| Subject: | LDAP key data encoder/decoder does not treat KrbKey salt as optional |
In 1.7, we made most ASN.1 encoders table-driven, including the LDAP key
sequence encoder. In this conversion, the KrbSalt salt field
(containing the salt value) was properly treated as optional, but the
KrbKey salt field (containing the KrbSalt sequence) was not. Even if a
krb5_key_data has key_data_ver == 1, we encode a KrbSalt representation
of key_data_length[1], key_data_contents[1]/key_data_length[1].
Luckily, just about every krb5_key_data constructor in the code base
starts out with zeroed memory, so we just wind up encoding a salt type
of 0 (which corresponds to the normal salt) and no salt value. The only
constructors I could find which don't do this are kadm5_get_principal
and ulog_conv_2dbentry. The former never constructs a krb5_key_data
value which is stored in a KDB (because it is being placed in a
kadm5_principal_ent_rec, not a krb5_db_entry), and the latter is never
stored in an LDAP KDB because LDAP KDBs can't be iprop slaves.
In 1.11 we made all ASN.1 decoders table-driven, using the same tables
as the encoders. This made the KrbKey salt field mandatory when
decoding, causing us to be unable to decode most key data generated by
version 1.6. See also #7918 for a secondary bug created during this
conversion.
If we simply fix the encoder and decoder to treat the KrbKey salt field
as mandatory, we will start generating key data encodings which
unpatched 1.11 and 1.12 can't decode. So we probably want to encode a
KrbSalt containing type 0 and no value when key_data_ver is 1 (but not
by assuming that key_data_type[1] and key_data_length[1] are meaningful)
for compatibility. The decoder should go back to treating the KrbKey
salt field as optional.
sequence encoder. In this conversion, the KrbSalt salt field
(containing the salt value) was properly treated as optional, but the
KrbKey salt field (containing the KrbSalt sequence) was not. Even if a
krb5_key_data has key_data_ver == 1, we encode a KrbSalt representation
of key_data_length[1], key_data_contents[1]/key_data_length[1].
Luckily, just about every krb5_key_data constructor in the code base
starts out with zeroed memory, so we just wind up encoding a salt type
of 0 (which corresponds to the normal salt) and no salt value. The only
constructors I could find which don't do this are kadm5_get_principal
and ulog_conv_2dbentry. The former never constructs a krb5_key_data
value which is stored in a KDB (because it is being placed in a
kadm5_principal_ent_rec, not a krb5_db_entry), and the latter is never
stored in an LDAP KDB because LDAP KDBs can't be iprop slaves.
In 1.11 we made all ASN.1 decoders table-driven, using the same tables
as the encoders. This made the KrbKey salt field mandatory when
decoding, causing us to be unable to decode most key data generated by
version 1.6. See also #7918 for a secondary bug created during this
conversion.
If we simply fix the encoder and decoder to treat the KrbKey salt field
as mandatory, we will start generating key data encodings which
unpatched 1.11 and 1.12 can't decode. So we probably want to encode a
KrbSalt containing type 0 and no value when key_data_ver is 1 (but not
by assuming that key_data_type[1] and key_data_length[1] are meaningful)
for compatibility. The decoder should go back to treating the KrbKey
salt field as optional.