From 800345591daa0ec0d916fa71032b78f4c4e225c9 Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Sat, 27 Jul 2013 03:21:12 -0500 Subject: [PATCH 01/17] Fix bug with use strongest session key feature (cherry picked from commit f4f89ac8e0f8583b7a2a3413fee5526a5b137d5b) Change-Id: I593b6ba7bdf050cc635baa463e741b584f0fa0bf --- kdc/kerberos5.c | 36 ++++++++++++++++++++++++------------ diff --git a/kdc/kerberos5.c b/kdc/kerberos5.c index ee6baf0..62bf0d8 100644 --- a/kdc/kerberos5.c +++ b/kdc/kerberos5.c @@ -131,8 +131,11 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, krb5_error_code ret; krb5_salt def_salt; krb5_enctype enctype = ETYPE_NULL; + krb5_enctype clientbest = (krb5_enctype)ETYPE_NULL; + const krb5_enctype *p; Key *key; - int i; + int i, k; + int client_offered_1des = 0; /* We'll want to avoid keys with v4 salted keys in the pre-auth case... */ ret = krb5_get_pw_salt(context, princ->entry.principal, &def_salt); @@ -142,10 +145,6 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, ret = KRB5KDC_ERR_ETYPE_NOSUPP; if (use_strongest_session_key) { - const krb5_enctype *p; - krb5_enctype clientbest = ETYPE_NULL; - int j; - /* * Pick the strongest key that the KDC, target service, and * client all support, using the local cryptosystem enctype @@ -164,11 +163,15 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, continue; /* check that the client supports it too */ - for (j = 0; j < len && enctype == ETYPE_NULL; j++) { - if (p[i] != etypes[j]) + for (k = 0; k < len && enctype == (krb5_enctype)ETYPE_NULL; k++) { + if (krb5_enctype_valid(context, etypes[k]) != 0 && + !_kdc_is_weak_exception(princ->entry.principal, etypes[k])) + continue; + if (etypes[k] == ETYPE_DES_CBC_CRC) + client_offered_1des = 1; + if (p[i] != etypes[k]) continue; - /* save best of union of { client, crypto system } */ - if (clientbest == ETYPE_NULL) + if (clientbest == (krb5_enctype)ETYPE_NULL) clientbest = p[i]; /* check target princ support */ ret = hdb_enctype2key(context, &princ->entry, p[i], &key); @@ -179,10 +182,19 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, enctype = p[i]; } } - if (clientbest != ETYPE_NULL && enctype == ETYPE_NULL) + if (clientbest != (krb5_enctype)ETYPE_NULL && + enctype == (krb5_enctype)ETYPE_NULL) { + ret = 0; enctype = clientbest; - else if (enctype == ETYPE_NULL) - ret = KRB5KDC_ERR_ETYPE_NOSUPP; + } else if (enctype == (krb5_enctype)ETYPE_NULL) { + if (client_offered_1des && + _kdc_is_weak_exception(princ->entry.principal, ETYPE_DES_CBC_CRC)) { + ret = 0; + enctype = ETYPE_DES_CBC_CRC; + } else { + ret = KRB5KDC_ERR_ETYPE_NOSUPP; + } + } if (ret == 0 && ret_enctype != NULL) *ret_enctype = enctype; if (ret == 0 && ret_key != NULL) -- 1.8.4 From e1dd757fe13c818dfb259b540d84345d9e20f98b Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Sat, 27 Jul 2013 16:51:01 -0500 Subject: [PATCH 02/17] Check all three DES types (cherry picked from commit 1f147f0fa66427c1976d5f88eb8bcdfe5f213287) --- kdc/kerberos5.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kdc/kerberos5.c b/kdc/kerberos5.c index 62bf0d8..7d16347 100644 --- a/kdc/kerberos5.c +++ b/kdc/kerberos5.c @@ -167,7 +167,9 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, if (krb5_enctype_valid(context, etypes[k]) != 0 && !_kdc_is_weak_exception(princ->entry.principal, etypes[k])) continue; - if (etypes[k] == ETYPE_DES_CBC_CRC) + if (etypes[k] == (krb5_enctype)ETYPE_DES_CBC_CRC || + etypes[k] == (krb5_enctype)ETYPE_DES_CBC_MD4 || + etypes[k] == (krb5_enctype)ETYPE_DES_CBC_MD5) client_offered_1des = 1; if (p[i] != etypes[k]) continue; -- 1.8.4 From 2a5a96d60ec464e831274fda3e3b6653de96196f Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Sat, 27 Jul 2013 17:29:54 -0500 Subject: [PATCH 03/17] When asking for the strongest key, get it right (cherry picked from commit 1826106ff4befe3e7dffc18692e40bd244c0d8d8) --- kdc/kerberos5.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kdc/kerberos5.c b/kdc/kerberos5.c index 7d16347..303736c 100644 --- a/kdc/kerberos5.c +++ b/kdc/kerberos5.c @@ -133,7 +133,7 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, krb5_enctype enctype = ETYPE_NULL; krb5_enctype clientbest = (krb5_enctype)ETYPE_NULL; const krb5_enctype *p; - Key *key; + Key *key = NULL; int i, k; int client_offered_1des = 0; @@ -175,6 +175,8 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, continue; if (clientbest == (krb5_enctype)ETYPE_NULL) clientbest = p[i]; + if (key) + continue; /* already picked a key below */ /* check target princ support */ ret = hdb_enctype2key(context, &princ->entry, p[i], &key); if (ret) -- 1.8.4 From a2d0f8e3ee350f7db48d7bcd6eed775ff1ace6e4 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Sat, 27 Jul 2013 20:02:16 -0400 Subject: [PATCH 04/17] _kdc_find_etype consolidation The 'use_strongest_session_key' block and its alternate should have similar behavior except for the order in which the enctype lists are processed. This patchset attempts to consolidate the exit processing and ensure that the inner loop enctype and key validation is the same. Bugs fixed: 1. In the 'use_strongest_session_key' case, the _kdc_is_weak_exception() test was applied during the client enctype loop which is only processed for acceptable enctypes. This test is moved to the local supported enctypes loop so as not to filter out weak keys when the service principal has an explicit exception. 2. In the 'use_strongest_session_key' case, the possibility of an enctype having keys with more than one salt was excluded. 3. In the 'use_strongest_session_key' case, the 'key' variable was not reset to NULL within each loop of the client enctype list. 4. In the '!use_strongest_session_key' case, the default salt test and is_preauth was inconsistent with the 'use_strongest_session_key' block. With this consolidation, if no enctype is selected and the service principal is permitted to use 1DES, then 1DES is selected. It doesn't matter whether 'use_strongest_session_key' is in use or not. Change-Id: Ib57264fc8bc23df64c70d39b4f6de48beeb54739 (cherry picked from commit 8f2d779663f4b1245cd53c3a593be94f5a616513) --- kdc/kerberos5.c | 93 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 48 insertions(+), 45 deletions(-) diff --git a/kdc/kerberos5.c b/kdc/kerberos5.c index 303736c..73b0252 100644 --- a/kdc/kerberos5.c +++ b/kdc/kerberos5.c @@ -130,12 +130,11 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, { krb5_error_code ret; krb5_salt def_salt; - krb5_enctype enctype = ETYPE_NULL; + krb5_enctype enctype = (krb5_enctype)ETYPE_NULL; krb5_enctype clientbest = (krb5_enctype)ETYPE_NULL; const krb5_enctype *p; Key *key = NULL; int i, k; - int client_offered_1des = 0; /* We'll want to avoid keys with v4 salted keys in the pre-auth case... */ ret = krb5_get_pw_salt(context, princ->entry.principal, &def_salt); @@ -158,51 +157,35 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, /* drive the search with local supported enctypes list */ p = krb5_kerberos_enctypes(context); - for (i = 0; p[i] != ETYPE_NULL && enctype == ETYPE_NULL; i++) { - if (krb5_enctype_valid(context, p[i]) != 0) + for (i = 0; + p[i] != (krb5_enctype)ETYPE_NULL && enctype == (krb5_enctype)ETYPE_NULL; + i++) { + if (krb5_enctype_valid(context, p[i]) != 0 && + !_kdc_is_weak_exception(princ->entry.principal, p[i])) continue; /* check that the client supports it too */ - for (k = 0; k < len && enctype == (krb5_enctype)ETYPE_NULL; k++) { - if (krb5_enctype_valid(context, etypes[k]) != 0 && - !_kdc_is_weak_exception(princ->entry.principal, etypes[k])) - continue; - if (etypes[k] == (krb5_enctype)ETYPE_DES_CBC_CRC || - etypes[k] == (krb5_enctype)ETYPE_DES_CBC_MD4 || - etypes[k] == (krb5_enctype)ETYPE_DES_CBC_MD5) - client_offered_1des = 1; + for (k = 0; k < len && enctype == (krb5_enctype)ETYPE_NULL; k++, key = NULL) { if (p[i] != etypes[k]) continue; if (clientbest == (krb5_enctype)ETYPE_NULL) clientbest = p[i]; - if (key) - continue; /* already picked a key below */ + /* check target princ support */ - ret = hdb_enctype2key(context, &princ->entry, p[i], &key); - if (ret) - continue; - if (is_preauth && !is_default_salt_p(&def_salt, key)) - continue; - enctype = p[i]; - } - } - if (clientbest != (krb5_enctype)ETYPE_NULL && - enctype == (krb5_enctype)ETYPE_NULL) { - ret = 0; - enctype = clientbest; - } else if (enctype == (krb5_enctype)ETYPE_NULL) { - if (client_offered_1des && - _kdc_is_weak_exception(princ->entry.principal, ETYPE_DES_CBC_CRC)) { - ret = 0; - enctype = ETYPE_DES_CBC_CRC; - } else { - ret = KRB5KDC_ERR_ETYPE_NOSUPP; + while (ret != 0 && + hdb_next_enctype2key(context, &princ->entry, + p[i], &key) == 0) { + if (key->key.keyvalue.length == 0) { + ret = KRB5KDC_ERR_NULL_KEY; + continue; + } + if (is_preauth && !is_default_salt_p(&def_salt, key)) + continue; + enctype = p[i]; + ret = 0; + } } } - if (ret == 0 && ret_enctype != NULL) - *ret_enctype = enctype; - if (ret == 0 && ret_key != NULL) - *ret_key = key; } else { /* * Pick the first key from the client's enctype list that is @@ -219,23 +202,43 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, !_kdc_is_weak_exception(princ->entry.principal, etypes[i])) continue; - while (hdb_next_enctype2key(context, &princ->entry, etypes[i], &key) == 0) { + while (ret != 0 && + hdb_next_enctype2key(context, &princ->entry, + etypes[i], &key) == 0) { if (key->key.keyvalue.length == 0) { ret = KRB5KDC_ERR_NULL_KEY; continue; } - if (ret_key != NULL) - *ret_key = key; - if (ret_enctype != NULL) - *ret_enctype = etypes[i]; + if (is_preauth && !is_default_salt_p(&def_salt, key)) + continue; + enctype = etypes[i]; ret = 0; - if (is_preauth && is_default_salt_p(&def_salt, key)) - goto out; } } } -out: + if (enctype == (krb5_enctype)ETYPE_NULL && + clientbest != (krb5_enctype)ETYPE_NULL) { + ret = 0; + enctype = clientbest; + } else if (enctype == (krb5_enctype)ETYPE_NULL) { + /* + * if the service principal is one for which there is a known 1DES + * exception and no other enctype matches both the client request and + * the service key list, provide a DES-CBC-CRC key. + */ + if (_kdc_is_weak_exception(princ->entry.principal, ETYPE_DES_CBC_CRC)) { + ret = 0; + enctype = ETYPE_DES_CBC_CRC; + } else { + ret = KRB5KDC_ERR_ETYPE_NOSUPP; + } + } + if (ret == 0 && ret_enctype != NULL) + *ret_enctype = enctype; + if (ret == 0 && ret_key != NULL) + *ret_key = key; + krb5_free_salt (context, def_salt); return ret; } -- 1.8.4 From 20090f7ba301453fc32bceda90125d043ff9210f Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Sun, 28 Jul 2013 14:48:09 -0400 Subject: [PATCH 05/17] _kdc_find_etype: do not return success if ret_key != NULL If _kdc_find_etype() is being called with 'ret_key' != NULL, the caller is attempting to find an actual principal key. If 'ret_key' is NULL then it is seeking a session key type. Only return an enctype that is not in the principal key list unless 'ret_key' is NULL. As part of this change remove 'clientbest' and the associated logic as it is both unnecessary and can produce an enctype for which the key cannot be returned. Change-Id: Iba319e95fc1eac139f00b0cce20e1249482d2c6f (cherry picked from commit 95f2abc1168f7050edc20af13f3f31ffd6fb8e69) --- kdc/kerberos5.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/kdc/kerberos5.c b/kdc/kerberos5.c index 73b0252..515934c 100644 --- a/kdc/kerberos5.c +++ b/kdc/kerberos5.c @@ -119,7 +119,7 @@ is_default_salt_p(const krb5_salt *default_salt, const Key *key) /* * return the first appropriate key of `princ' in `ret_key'. Look for * all the etypes in (`etypes', `len'), stopping as soon as we find - * one, but preferring one that has default salt + * one, but preferring one that has default salt. */ krb5_error_code @@ -131,7 +131,6 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, krb5_error_code ret; krb5_salt def_salt; krb5_enctype enctype = (krb5_enctype)ETYPE_NULL; - krb5_enctype clientbest = (krb5_enctype)ETYPE_NULL; const krb5_enctype *p; Key *key = NULL; int i, k; @@ -165,13 +164,13 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, continue; /* check that the client supports it too */ - for (k = 0; k < len && enctype == (krb5_enctype)ETYPE_NULL; k++, key = NULL) { + for (k = 0; k < len && enctype == (krb5_enctype)ETYPE_NULL; k++) { + if (p[i] != etypes[k]) continue; - if (clientbest == (krb5_enctype)ETYPE_NULL) - clientbest = p[i]; /* check target princ support */ + key = NULL; while (ret != 0 && hdb_next_enctype2key(context, &princ->entry, p[i], &key) == 0) { @@ -196,12 +195,13 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, * weak enctypes in krb5.conf and selects this key selection * algorithm, then we get exactly what RFC4120 says. */ - for(key = NULL, i = 0; ret != 0 && i < len; i++, key = NULL) { + for(i = 0; ret != 0 && i < len; i++) { if (krb5_enctype_valid(context, etypes[i]) != 0 && !_kdc_is_weak_exception(princ->entry.principal, etypes[i])) continue; + key = NULL; while (ret != 0 && hdb_next_enctype2key(context, &princ->entry, etypes[i], &key) == 0) { @@ -217,27 +217,27 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, } } - if (enctype == (krb5_enctype)ETYPE_NULL && - clientbest != (krb5_enctype)ETYPE_NULL) { - ret = 0; - enctype = clientbest; - } else if (enctype == (krb5_enctype)ETYPE_NULL) { + if (enctype == (krb5_enctype)ETYPE_NULL) { /* * if the service principal is one for which there is a known 1DES * exception and no other enctype matches both the client request and * the service key list, provide a DES-CBC-CRC key. */ - if (_kdc_is_weak_exception(princ->entry.principal, ETYPE_DES_CBC_CRC)) { + if (ret_key == NULL && + _kdc_is_weak_exception(princ->entry.principal, ETYPE_DES_CBC_CRC)) { ret = 0; enctype = ETYPE_DES_CBC_CRC; } else { ret = KRB5KDC_ERR_ETYPE_NOSUPP; } } - if (ret == 0 && ret_enctype != NULL) - *ret_enctype = enctype; - if (ret == 0 && ret_key != NULL) - *ret_key = key; + + if (ret == 0) { + if (ret_enctype != NULL) + *ret_enctype = enctype; + if (ret_key != NULL) + *ret_key = key; + } krb5_free_salt (context, def_salt); return ret; -- 1.8.4 From 33a3a172ad3cf53764388efb8767ce5793b49a41 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Mon, 29 Jul 2013 11:38:25 -0400 Subject: [PATCH 06/17] apply weak key exceptions to _kdc_get_preferred_key As part of the keytype validity checks within _kdc_get_preferred_key _kdc_is_weak_exception must be used to permit the afs/* principals to have only DES in the key list. Change-Id: I70801ce9b8c4d3f057542541ce11e06d195efd52 (cherry picked from commit 002a5acbf01efc2596a41b7685f03822b3895216) --- kdc/misc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/kdc/misc.c b/kdc/misc.c index 1b2c440..a09af50 100644 --- a/kdc/misc.c +++ b/kdc/misc.c @@ -140,8 +140,9 @@ _kdc_get_preferred_key(krb5_context context, if (config->use_strongest_server_key) { const krb5_enctype *p = krb5_kerberos_enctypes(context); - for (i = 0; p[i] != ETYPE_NULL; i++) { - if (krb5_enctype_valid(context, p[i]) != 0) + for (i = 0; p[i] != (krb5_enctype)ETYPE_NULL; i++) { + if (krb5_enctype_valid(context, p[i]) != 0 && + !_kdc_is_weak_exception(h->entry.principal, p[i])) continue; ret = hdb_enctype2key(context, &h->entry, p[i], key); if (ret != 0) @@ -154,8 +155,8 @@ _kdc_get_preferred_key(krb5_context context, *key = NULL; for (i = 0; i < h->entry.keys.len; i++) { - if (krb5_enctype_valid(context, h->entry.keys.val[i].key.keytype) - != 0) + if (krb5_enctype_valid(context, h->entry.keys.val[i].key.keytype) != 0 && + !_kdc_is_weak_exception(h->entry.principal, h->entry.keys.val[i].key.keytype)) continue; ret = hdb_enctype2key(context, &h->entry, h->entry.keys.val[i].key.keytype, key); -- 1.8.4 From 76bee4df58994d45852e2e8d5da7ec09bdc6f5d4 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Tue, 30 Jul 2013 10:46:20 -0400 Subject: [PATCH 07/17] _kdc_find_etype: prefer default salt for preauth if the query is "preauth" and the caller is seeking a Key, search try to find a Key that has the default salt but do not exclude keys that have a non-default salt. Move the assignment of 'ret' and 'enctype' before the preauth default salt test. If the only key of the given type is the non-default salt key, it should be used. If the caller is not seeking a Key, do not bother with the preauth test at all since the Key itself doesn't matter and we are simply seeking an enctype. Change-Id: I7cd37c579c0bfdd88bccfbc9eb5e5f55cd1910cb --- kdc/kerberos5.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/kdc/kerberos5.c b/kdc/kerberos5.c index 515934c..f4e60bb 100644 --- a/kdc/kerberos5.c +++ b/kdc/kerberos5.c @@ -171,17 +171,17 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, /* check target princ support */ key = NULL; - while (ret != 0 && - hdb_next_enctype2key(context, &princ->entry, + while (hdb_next_enctype2key(context, &princ->entry, p[i], &key) == 0) { if (key->key.keyvalue.length == 0) { ret = KRB5KDC_ERR_NULL_KEY; continue; } - if (is_preauth && !is_default_salt_p(&def_salt, key)) - continue; enctype = p[i]; ret = 0; + if (is_preauth && ret_key != NULL && + !is_default_salt_p(&def_salt, key)) + continue; } } } @@ -209,10 +209,11 @@ _kdc_find_etype(krb5_context context, krb5_boolean use_strongest_session_key, ret = KRB5KDC_ERR_NULL_KEY; continue; } - if (is_preauth && !is_default_salt_p(&def_salt, key)) - continue; enctype = etypes[i]; ret = 0; + if (is_preauth && ret_key != NULL && + !is_default_salt_p(&def_salt, key)) + continue; } } } -- 1.8.4 From d9b3691b0f993a4b80fddc7b2771209e3856c26a Mon Sep 17 00:00:00 2001 From: Ragnar Sundblad Date: Tue, 30 Jul 2013 12:21:54 -0400 Subject: [PATCH 08/17] tgs_make_reply: fix temp weak enctype exception The default heimdal KDC chokes when trying to encrypt a ticket with a weak server key that has a different type than the session key. The problem happens in the krb5_crypto_init function called from the _kdc_encode_reply function. The existing work-around of the problem temporarily enabled the weak enctype in case it was disabled but the principal was on the (hard-coded) exception list. Unfortunately the code used the keytype of the key encoded in the ticked (the session key) instead of the keytype of the key used to encrypt the ticket (the serverkey) thus enabling the incorrect encryption type if those two are different, for instance des-cbc-md5 and des-cbc-crc. Change-Id: Ia55dc344e3e5fc9ec1eb93c9e8ebb0a58c673d57 --- kdc/krb5tgs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c index a024efa..65bb4e8 100644 --- a/kdc/krb5tgs.c +++ b/kdc/krb5tgs.c @@ -969,10 +969,10 @@ tgs_make_reply(krb5_context context, goto out; } - if (krb5_enctype_valid(context, et.key.keytype) != 0 - && _kdc_is_weak_exception(server->entry.principal, et.key.keytype)) + if (krb5_enctype_valid(context, serverkey->keytype) != 0 + && _kdc_is_weak_exception(server->entry.principal, serverkey->keytype)) { - krb5_enctype_enable(context, et.key.keytype); + krb5_enctype_enable(context, serverkey->keytype); is_weak = 1; } @@ -993,7 +993,7 @@ tgs_make_reply(krb5_context context, serverkey, 0, replykey, rk_is_subkey, e_text, reply); if (is_weak) - krb5_enctype_disable(context, et.key.keytype); + krb5_enctype_disable(context, serverkey->keytype); out: free_TGS_REP(&rep); -- 1.8.4