krb5 commit: Treat LDAP KrbKey salt field as optional

Greg Hudson ghudson at MIT.EDU
Thu Jun 5 11:43:06 EDT 2014


https://github.com/krb5/krb5/commit/fb5cd8df0dbd04dac4f610e68cba5b80a3cb8d48
commit fb5cd8df0dbd04dac4f610e68cba5b80a3cb8d48
Author: Greg Hudson <ghudson at mit.edu>
Date:   Fri May 23 19:58:41 2014 -0400

    Treat LDAP KrbKey salt field as optional
    
    Per the ASN.1 definition, the KrbKey salt field is optional.  Since
    1.7, we have been treating it as mandatory in the encoder; since 1.11,
    we have been treating it as mandatory in the decoder.  Mostly by luck,
    we have been encoding a salt type of 0 when key_data_ver is 1, but we
    really should not be looking at key_data_type[1] or key_data_length[1]
    in this situation.  Treat the salt field as optional in the encoder
    and decoder.  Although the previous commit ensures that we continue to
    always encode a salt (without any dangerous assumptions about
    krb5_key_data constructors), this change will allow us to decode key
    data encoded by 1.6 without salt fields.
    
    This also fixes issue #7918, by properly setting key_data_ver to 2 if
    a salt type but no salt value is present.  It is difficult to get the
    decoder to actually assign 2 to key_data_ver just because the salt
    field is there, so take care of that in asn1_decode_sequence_of_keys.
    
    Adjust kdbtest.c to match the new behavior by setting key_data_ver to
    2 in both test keys.
    
    ticket: 7919
    target_version: 1.12.2
    tags: pullup

 src/lib/krb5/asn.1/ldap_key_seq.c                  |   19 ++++++++++++++++---
 src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c |    6 ++++--
 src/tests/kdbtest.c                                |    2 +-
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/lib/krb5/asn.1/ldap_key_seq.c b/src/lib/krb5/asn.1/ldap_key_seq.c
index 69ad847..deb4705 100644
--- a/src/lib/krb5/asn.1/ldap_key_seq.c
+++ b/src/lib/krb5/asn.1/ldap_key_seq.c
@@ -57,14 +57,14 @@ DEFCOUNTEDSTRINGTYPE(ui2_octetstring, unsigned char *, krb5_ui_2,
                      ASN1_OCTETSTRING);
 
 static int
-is_salt_present(const void *p)
+is_value_present(const void *p)
 {
     const krb5_key_data *val = p;
     return (val->key_data_length[1] != 0);
 }
 DEFCOUNTEDTYPE(krbsalt_salt, krb5_key_data, key_data_contents[1],
                key_data_length[1], ui2_octetstring);
-DEFOPTIONALTYPE(krbsalt_salt_if_present, is_salt_present, NULL, krbsalt_salt);
+DEFOPTIONALTYPE(krbsalt_salt_if_present, is_value_present, NULL, krbsalt_salt);
 DEFFIELD(krbsalt_0, krb5_key_data, key_data_type[1], 0, int16);
 DEFCTAGGEDTYPE(krbsalt_1, 1, krbsalt_salt_if_present);
 static const struct atype_info *krbsalt_fields[] = {
@@ -80,7 +80,20 @@ static const struct atype_info *encryptionkey_fields[] = {
 };
 DEFSEQTYPE(encryptionkey, krb5_key_data, encryptionkey_fields);
 
-DEFCTAGGEDTYPE(key_data_0, 0, krbsalt);
+static int
+is_salt_present(const void *p)
+{
+    const krb5_key_data *val = p;
+    return val->key_data_ver > 1;
+}
+static void
+no_salt(void *p)
+{
+    krb5_key_data *val = p;
+    val->key_data_ver = 1;
+}
+DEFOPTIONALTYPE(key_data_salt_if_present, is_salt_present, no_salt, krbsalt);
+DEFCTAGGEDTYPE(key_data_0, 0, key_data_salt_if_present);
 DEFCTAGGEDTYPE(key_data_1, 1, encryptionkey);
 #if 0 /* We don't support this field currently.  */
 DEFCTAGGEDTYPE(key_data_2, 2, s2kparams),
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
index 482066f..b9bd059 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
@@ -383,8 +383,10 @@ asn1_decode_sequence_of_keys(krb5_data *in, krb5_key_data **out,
     /* Set kvno and key_data_ver in each key_data element. */
     for (i = 0; i < p->n_key_data; i++) {
         p->key_data[i].key_data_kvno = p->kvno;
-        p->key_data[i].key_data_ver =
-            (p->key_data[i].key_data_length[1] == 0) ? 1 : 2;
+        /* The decoder sets key_data_ver to 1 if no salt is present, but leaves
+         * it at 0 if salt is present. */
+        if (p->key_data[i].key_data_ver == 0)
+            p->key_data[i].key_data_ver = 2;
     }
 
     *out = p->key_data;
diff --git a/src/tests/kdbtest.c b/src/tests/kdbtest.c
index 64f28bb..d211265 100644
--- a/src/tests/kdbtest.c
+++ b/src/tests/kdbtest.c
@@ -120,7 +120,7 @@ static krb5_key_data keys[] = {
           U("expsalt") }
     },
     {
-        1,                          /* key_data_ver */
+        2,                          /* key_data_ver */
         2,                          /* key_data_kvno */
         { ENCTYPE_AES128_CTS_HMAC_SHA1_96, 0 },
         { 16, 0 },


More information about the cvs-krb5 mailing list