From 97d7448c33e4aba91fb157c9cf0d186ea6d89f68 Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Wed, 25 Sep 2019 13:20:48 -0400 Subject: [PATCH] Fix KDC crash when logging PKINIT enctypes (CVE-2019-14844) --- ...able-flag-instead-of-denying-request.patch | 7 +- ...C-crash-when-logging-PKINIT-enctypes.patch | 32 ++++ ...5_cc_remove_cred-for-remaining-types.patch | 30 ++-- ...ype-names-in-KDC-logs-human-readable.patch | 137 +++++++++--------- ...beros-v4-support-vestiges-from-ccapi.patch | 60 ++++---- krb5.spec | 6 +- 6 files changed, 154 insertions(+), 118 deletions(-) create mode 100644 Fix-KDC-crash-when-logging-PKINIT-enctypes.patch diff --git a/Clear-forwardable-flag-instead-of-denying-request.patch b/Clear-forwardable-flag-instead-of-denying-request.patch index 4b29e0f..5cdb148 100644 --- a/Clear-forwardable-flag-instead-of-denying-request.patch +++ b/Clear-forwardable-flag-instead-of-denying-request.patch @@ -14,14 +14,14 @@ ticket: 7871 (cherry picked from commit 08e948cce2c79a3604066fcf7a64fc527456f83d) --- src/kdc/do_as_req.c | 19 ++------ - src/kdc/do_tgs_req.c | 56 ++++----------------- + src/kdc/do_tgs_req.c | 58 +++++----------------- src/kdc/kdc_util.c | 82 ++++++++++++++++++------------- src/kdc/kdc_util.h | 9 ++-- src/kdc/tgs_policy.c | 8 +-- src/tests/Makefile.in | 1 + src/tests/gcred.c | 28 ++++++++--- src/tests/t_kdcoptions.py | 100 ++++++++++++++++++++++++++++++++++++++ - 8 files changed, 189 insertions(+), 114 deletions(-) + 8 files changed, 190 insertions(+), 115 deletions(-) create mode 100644 src/tests/t_kdcoptions.py diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c @@ -146,9 +146,10 @@ index 587342a6c..1da099318 100644 - if (isflagset(request->kdc_options, KDC_OPT_REQUEST_ANONYMOUS) && - !isflagset(header_enc_tkt->flags, TKT_FLG_ANONYMOUS)) - clear(enc_tkt_reply.flags, TKT_FLG_ANONYMOUS); - +- - if (isflagset(request->kdc_options, KDC_OPT_POSTDATED)) { - setflag(enc_tkt_reply.flags, TKT_FLG_INVALID); ++ + if (isflagset(request->kdc_options, KDC_OPT_POSTDATED)) enc_tkt_reply.times.starttime = request->from; - } else diff --git a/Fix-KDC-crash-when-logging-PKINIT-enctypes.patch b/Fix-KDC-crash-when-logging-PKINIT-enctypes.patch new file mode 100644 index 0000000..4de01db --- /dev/null +++ b/Fix-KDC-crash-when-logging-PKINIT-enctypes.patch @@ -0,0 +1,32 @@ +From efbce403dcbcb4f76d6cbdbeb6a6bec2f4f533e3 Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Wed, 25 Sep 2019 12:57:56 -0400 +Subject: [PATCH] Fix KDC crash when logging PKINIT enctypes + +Commit a649279727490687d54becad91fde8cf7429d951 introduced a KDC crash +bug due to transposed strlcpy() arguments. Fix the argument order. + +This bug does not affect any MIT krb5 release, but affects the Fedora +krb5 packages due to backports. CVE-2019-14844 has been issued as a +result. + +ticket: 8772 +(cherry picked from commit 275c9a1aad36a1a7b56042f1a2c21c33e7d16eaf) +(cherry picked from commit 55353df13814c6d711a1d947dd6690b334269122) +--- + src/kdc/kdc_util.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c +index f2741090e..9dc64ddc5 100644 +--- a/src/kdc/kdc_util.c ++++ b/src/kdc/kdc_util.c +@@ -1093,7 +1093,7 @@ enctype_name(krb5_enctype ktype, char *buf, size_t buflen) + else + return krb5_enctype_to_name(ktype, FALSE, buf, buflen); + +- if (strlcpy(name, buf, buflen) >= buflen) ++ if (strlcpy(buf, name, buflen) >= buflen) + return ENOMEM; + return 0; + } diff --git a/Implement-krb5_cc_remove_cred-for-remaining-types.patch b/Implement-krb5_cc_remove_cred-for-remaining-types.patch index a656d57..430634a 100644 --- a/Implement-krb5_cc_remove_cred-for-remaining-types.patch +++ b/Implement-krb5_cc_remove_cred-for-remaining-types.patch @@ -266,13 +266,25 @@ index 8419f6ebf..98723fe2e 100644 - * with the time offsets, skip it. */ - while (krcursor->keys[krcursor->currkey] == krcursor->princ_id || - krcursor->keys[krcursor->currkey] == krcursor->offsets_id) { +- krcursor->currkey++; +- /* Check if we have now reached the end */ +- if (krcursor->currkey >= krcursor->numkeys) +- return KRB5_CC_END; +- } + /* Read the key; the right size buffer will be allocated and + * returned. */ + psize = keyctl_read_alloc(krcursor->keys[krcursor->currkey], + &payload); + if (psize != -1) { + krcursor->currkey++; -+ + +- /* Read the key; the right size buffer will be allocated and returned. */ +- psize = keyctl_read_alloc(krcursor->keys[krcursor->currkey], &payload); +- if (psize == -1) { +- DEBUG_PRINT(("Error reading key %d: %s\n", +- krcursor->keys[krcursor->currkey], +- strerror(errno))); +- return KRB5_FCC_NOFILE; + /* Unmarshal the cred using the file ccache version 4 format. */ + ret = k5_unmarshal_cred(payload, psize, 4, creds); + free(payload); @@ -285,22 +297,10 @@ index 8419f6ebf..98723fe2e 100644 + + /* The current key was unlinked, probably by a remove_cred call; move + * on to the next one. */ - krcursor->currkey++; -- /* Check if we have now reached the end */ -- if (krcursor->currkey >= krcursor->numkeys) -- return KRB5_CC_END; ++ krcursor->currkey++; } - -- /* Read the key; the right size buffer will be allocated and returned. */ -- psize = keyctl_read_alloc(krcursor->keys[krcursor->currkey], &payload); -- if (psize == -1) { -- DEBUG_PRINT(("Error reading key %d: %s\n", -- krcursor->keys[krcursor->currkey], -- strerror(errno))); -- return KRB5_FCC_NOFILE; -- } - krcursor->currkey++; -- + - /* Unmarshal the credential using the file ccache version 4 format. */ - ret = k5_unmarshal_cred(payload, psize, 4, creds); - free(payload); diff --git a/Make-etype-names-in-KDC-logs-human-readable.patch b/Make-etype-names-in-KDC-logs-human-readable.patch index 6fd40d7..7a76038 100644 --- a/Make-etype-names-in-KDC-logs-human-readable.patch +++ b/Make-etype-names-in-KDC-logs-human-readable.patch @@ -12,9 +12,9 @@ ticket: 8772 (new) (cherry picked from commit a649279727490687d54becad91fde8cf7429d951) --- src/kdc/kdc_log.c | 42 +++++++-------- - src/kdc/kdc_util.c | 131 +++++++++++++++++++++++---------------------- + src/kdc/kdc_util.c | 125 +++++++++++++++++++++++---------------------- src/kdc/kdc_util.h | 6 +-- - 3 files changed, 90 insertions(+), 89 deletions(-) + 3 files changed, 87 insertions(+), 86 deletions(-) diff --git a/src/kdc/kdc_log.c b/src/kdc/kdc_log.c index 4eec50373..b160ba21a 100644 @@ -132,57 +132,16 @@ index 0155c28c6..f5c581c82 100644 - * L10_2 = log10(2**x), rounded up; log10(2) ~= 0.301. - */ -#define L10_2(x) ((int)(((x * 301) + 999) / 1000)) -- --/* -- * Max length of sprintf("%ld") for an int of type T; includes leading -- * minus sign and terminating NUL. -- */ --#define D_LEN(t) (L10_2(sizeof(t) * CHAR_BIT) + 2) -- --void --ktypes2str(char *s, size_t len, int nktypes, krb5_enctype *ktype) +/* Wrapper of krb5_enctype_to_name() to include the PKINIT types. */ +static krb5_error_code +enctype_name(krb5_enctype ktype, char *buf, size_t buflen) - { -- int i; -- char stmp[D_LEN(krb5_enctype) + 1]; -- char *p; ++{ + char *name; - -- if (nktypes < 0 -- || len < (sizeof(" etypes {...}") + D_LEN(int))) { -- *s = '\0'; -- return; -- } ++ + if (buflen == 0) + return EINVAL; + *buf = '\0'; /* ensure these are always valid C-strings */ - -- snprintf(s, len, "%d etypes {", nktypes); -- for (i = 0; i < nktypes; i++) { -- snprintf(stmp, sizeof(stmp), "%s%ld", i ? " " : "", (long)ktype[i]); -- if (strlen(s) + strlen(stmp) + sizeof("}") > len) -- break; -- strlcat(s, stmp, len); -- } -- if (i < nktypes) { -- /* -- * We broke out of the loop. Try to truncate the list. -- */ -- p = s + strlen(s); -- while (p - s + sizeof("...}") > len) { -- while (p > s && *p != ' ' && *p != '{') -- *p-- = '\0'; -- if (p > s && *p == ' ') { -- *p-- = '\0'; -- continue; -- } -- } -- strlcat(s, "...", len); -- } -- strlcat(s, "}", len); -- return; ++ + /* rfc4556 recommends that clients wishing to indicate support for these + * pkinit algorithms include them in the etype field of the AS-REQ. */ + if (ktype == ENCTYPE_DSA_SHA1_CMS) @@ -201,47 +160,85 @@ index 0155c28c6..f5c581c82 100644 + name = "des-ede3-cbc-EnvOID"; + else + return krb5_enctype_to_name(ktype, FALSE, buf, buflen); -+ + +-/* +- * Max length of sprintf("%ld") for an int of type T; includes leading +- * minus sign and terminating NUL. +- */ +-#define D_LEN(t) (L10_2(sizeof(t) * CHAR_BIT) + 2) + if (strlcpy(name, buf, buflen) >= buflen) + return ENOMEM; + return 0; ++} + +-void +-ktypes2str(char *s, size_t len, int nktypes, krb5_enctype *ktype) ++char * ++ktypes2str(krb5_enctype *ktype, int nktypes) + { ++ struct k5buf buf; + int i; +- char stmp[D_LEN(krb5_enctype) + 1]; +- char *p; ++ char name[64]; + +- if (nktypes < 0 +- || len < (sizeof(" etypes {...}") + D_LEN(int))) { +- *s = '\0'; +- return; +- } ++ if (nktypes < 0) ++ return NULL; + +- snprintf(s, len, "%d etypes {", nktypes); ++ k5_buf_init_dynamic(&buf); ++ k5_buf_add_fmt(&buf, "%d etypes {", nktypes); + for (i = 0; i < nktypes; i++) { +- snprintf(stmp, sizeof(stmp), "%s%ld", i ? " " : "", (long)ktype[i]); +- if (strlen(s) + strlen(stmp) + sizeof("}") > len) +- break; +- strlcat(s, stmp, len); ++ enctype_name(ktype[i], name, sizeof(name)); ++ k5_buf_add_fmt(&buf, "%s%s(%ld)", i ? ", " : "", name, (long)ktype[i]); + } +- if (i < nktypes) { +- /* +- * We broke out of the loop. Try to truncate the list. +- */ +- p = s + strlen(s); +- while (p - s + sizeof("...}") > len) { +- while (p > s && *p != ' ' && *p != '{') +- *p-- = '\0'; +- if (p > s && *p == ' ') { +- *p-- = '\0'; +- continue; +- } +- } +- strlcat(s, "...", len); +- } +- strlcat(s, "}", len); +- return; ++ k5_buf_add(&buf, "}"); ++ return buf.data; } -void -rep_etypes2str(char *s, size_t len, krb5_kdc_rep *rep) +char * -+ktypes2str(krb5_enctype *ktype, int nktypes) ++rep_etypes2str(krb5_kdc_rep *rep) { - char stmp[sizeof("ses=") + D_LEN(krb5_enctype)]; -+ struct k5buf buf; -+ int i; -+ char name[64]; - +- - if (len < (3 * D_LEN(krb5_enctype) - + sizeof("etypes {rep= tkt= ses=}"))) { - *s = '\0'; - return; -+ if (nktypes < 0) -+ return NULL; -+ -+ k5_buf_init_dynamic(&buf); -+ k5_buf_add_fmt(&buf, "%d etypes {", nktypes); -+ for (i = 0; i < nktypes; i++) { -+ enctype_name(ktype[i], name, sizeof(name)); -+ k5_buf_add_fmt(&buf, "%s%s(%ld)", i ? ", " : "", name, (long)ktype[i]); - } -+ k5_buf_add(&buf, "}"); -+ return buf.data; -+} - -- snprintf(s, len, "etypes {rep=%ld", (long)rep->enc_part.enctype); -+char * -+rep_etypes2str(krb5_kdc_rep *rep) -+{ +- } + struct k5buf buf; + char name[64]; + krb5_enctype etype; -+ + +- snprintf(s, len, "etypes {rep=%ld", (long)rep->enc_part.enctype); + k5_buf_init_dynamic(&buf); + k5_buf_add(&buf, "etypes {rep="); + enctype_name(rep->enc_part.enctype, name, sizeof(name)); diff --git a/Remove-Kerberos-v4-support-vestiges-from-ccapi.patch b/Remove-Kerberos-v4-support-vestiges-from-ccapi.patch index 12c58a4..76c7b09 100644 --- a/Remove-Kerberos-v4-support-vestiges-from-ccapi.patch +++ b/Remove-Kerberos-v4-support-vestiges-from-ccapi.patch @@ -9,7 +9,7 @@ Subject: [PATCH] Remove Kerberos v4 support vestiges from ccapi src/ccapi/lib/ccapi_v2.c | 34 +-- src/ccapi/lib/win/OldCC/ccapi.h | 20 -- src/ccapi/server/ccs_ccache.c | 69 +----- - src/ccapi/test/test_ccapi_ccache.c | 223 +++----------------- + src/ccapi/test/test_ccapi_ccache.c | 227 +++----------------- src/ccapi/test/test_ccapi_constants.c | 2 - src/ccapi/test/test_ccapi_context.c | 3 - src/ccapi/test/test_ccapi_v2.c | 89 -------- @@ -20,7 +20,7 @@ Subject: [PATCH] Remove Kerberos v4 support vestiges from ccapi src/windows/kfwlogon/kfwlogon.h | 2 +- src/windows/leashdll/leash-int.h | 2 +- src/windows/lib/cacheapi.h | 53 +---- - 15 files changed, 98 insertions(+), 871 deletions(-) + 15 files changed, 100 insertions(+), 873 deletions(-) diff --git a/src/ccapi/common/cci_cred_union.c b/src/ccapi/common/cci_cred_union.c index 4c8981610..424a93dab 100644 @@ -760,29 +760,8 @@ index a0fd84af1..fe63e6710 100644 - cc_ccache_destroy(ccache); - ccache = NULL; - } -+ // replace v5 only ccache's principal -+ if (!err) { -+ err = cc_context_create_new_ccache(context, cc_credentials_v5, -+ "foo@BAZ.ORG", &ccache); -+ } -+ if (!err) { -+ check_once_cc_ccache_set_principal( -+ ccache, cc_credentials_v5, "foo/BAZ@BAR.ORG", ccNoError, -+ "replace v5 only ccache's principal (empty ccache)"); -+ } -+ else { -+ log_error( -+ "cc_context_create_new_ccache failed, can't complete test"); -+ failure_count++; -+ } - -+ // bad params -+ if (!err) { -+ check_once_cc_ccache_set_principal(ccache, cc_credentials_v5, -+ NULL, ccErrBadParam, -+ "NULL principal"); -+ } - +- +- - // empty ccache - - // replace v5 only ccache's principal @@ -858,6 +837,29 @@ index a0fd84af1..fe63e6710 100644 - // replace v4 only ccache's principal - - // add v5 principal to v4 only ccache ++ // replace v5 only ccache's principal ++ if (!err) { ++ err = cc_context_create_new_ccache(context, cc_credentials_v5, ++ "foo@BAZ.ORG", &ccache); ++ } ++ if (!err) { ++ check_once_cc_ccache_set_principal( ++ ccache, cc_credentials_v5, "foo/BAZ@BAR.ORG", ccNoError, ++ "replace v5 only ccache's principal (empty ccache)"); ++ } ++ else { ++ log_error( ++ "cc_context_create_new_ccache failed, can't complete test"); ++ failure_count++; ++ } ++ ++ // bad params ++ if (!err) { ++ check_once_cc_ccache_set_principal(ccache, cc_credentials_v5, ++ NULL, ccErrBadParam, ++ "NULL principal"); ++ } ++ + if (ccache) { + cc_ccache_destroy(ccache); + ccache = NULL; @@ -892,8 +894,7 @@ index a0fd84af1..fe63e6710 100644 } if (!err) { - check_once_cc_ccache_get_kdc_time_offset(ccache, cc_credentials_v5, &time_offset, ccNoError, "offset set for v5 but not v4"); -+ check_once_cc_ccache_get_kdc_time_offset(ccache, cc_credentials_v5, &time_offset, ccNoError, "offset set for v5"); - } +- } - if (!err) { - check_once_cc_ccache_get_kdc_time_offset(ccache, cc_credentials_v4, &time_offset, ccErrTimeOffsetNotSet, "asking for v4 offset when only v5 is set"); - } @@ -902,9 +903,10 @@ index a0fd84af1..fe63e6710 100644 - } - if (!err) { - check_once_cc_ccache_get_kdc_time_offset(ccache, cc_credentials_v4, &time_offset, ccNoError, "asking for v4 offset when v4 and v5 are set"); -- } -- ++ check_once_cc_ccache_get_kdc_time_offset(ccache, cc_credentials_v5, &time_offset, ccNoError, "offset set for v5"); + } +- check_once_cc_ccache_get_kdc_time_offset(ccache, cc_credentials_v5, NULL, ccErrBadParam, "NULL time_offset out param"); - check_once_cc_ccache_get_kdc_time_offset(ccache, cc_credentials_v4_v5, &time_offset, ccErrBadCredentialsVersion, "v4_v5 creds_vers in param (invalid)"); diff --git a/krb5.spec b/krb5.spec index 99818a0..72cfb81 100644 --- a/krb5.spec +++ b/krb5.spec @@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system Name: krb5 Version: 1.17 # for prerelease, should be e.g., 0.% {prerelease}.1% { ?dist } (without spaces) -Release: 14%{?dist} +Release: 15%{?dist} # lookaside-cached sources; two downloads and a build artifact Source0: https://web.mit.edu/kerberos/dist/krb5/1.16/krb5-%{version}%{prerelease}.tar.gz @@ -87,6 +87,7 @@ Patch114: Fix-potential-close-1-in-cc_file.c.patch Patch115: Check-more-errors-in-OpenSSL-crypto-backend.patch Patch116: Clear-forwardable-flag-instead-of-denying-request.patch Patch117: Add-dns_canonicalize_hostname-fallback-support.patch +Patch118: Fix-KDC-crash-when-logging-PKINIT-enctypes.patch License: MIT URL: http://web.mit.edu/kerberos/www/ @@ -723,6 +724,9 @@ exit 0 %{_libdir}/libkadm5srv_mit.so.* %changelog +* Wed Sep 25 2019 Robbie Harwood - 1.17-15 +- Fix KDC crash when logging PKINIT enctypes (CVE-2019-14844) + * Wed Apr 24 2019 Robbie Harwood - 1.17-14 - Add dns_canonicalize_hostname=fallback support