From 91465b2b091557c11d9824b34c1ae8a8b7d345b3 Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Mon, 23 Oct 2017 16:28:53 +0000 Subject: [PATCH 1/6] Drop dependency on python2-pyrad (dead upstream, broken with new python) --- krb5.spec | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/krb5.spec b/krb5.spec index df62457..5257456 100644 --- a/krb5.spec +++ b/krb5.spec @@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system Name: krb5 Version: 1.15.2 # for prerelease, should be e.g., 0.3.beta2% { ?dist } (without spaces) -Release: 2%{?dist} +Release: 3%{?dist} # lookaside-cached sources; two downloads and a build artifact Source0: https://web.mit.edu/kerberos/dist/krb5/1.15/krb5-%{version}%{prerelease}.tar.gz @@ -141,7 +141,6 @@ BuildRequires: perl-interpreter, dejagnu, tcl-devel BuildRequires: net-tools, rpcbind BuildRequires: hostname BuildRequires: iproute -BuildRequires: python2-pyrad BuildRequires: libverto-devel BuildRequires: openldap-devel BuildRequires: openssl-devel >= 0.9.8 @@ -745,6 +744,9 @@ exit 0 %{_libdir}/libkadm5srv_mit.so.* %changelog +* Mon Oct 23 2017 Robbie Harwood - 1.15.2-3 +- Drop dependency on python2-pyrad (dead upstream, broken with new python) + * Thu Sep 28 2017 Robbie Harwood - 1.15.2-2 - Add German translation From 1884c63c38f83b45af237ccb58a17b786a441f09 Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Tue, 24 Oct 2017 15:59:37 -0400 Subject: [PATCH 2/6] Fix CVE-2017-15088 (Buffer overflow in get_matching_data()) --- ...INIT-cert-matching-data-construction.patch | 105 ++++++++++++++++++ krb5.spec | 6 +- 2 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 Fix-PKINIT-cert-matching-data-construction.patch diff --git a/Fix-PKINIT-cert-matching-data-construction.patch b/Fix-PKINIT-cert-matching-data-construction.patch new file mode 100644 index 0000000..99b3db7 --- /dev/null +++ b/Fix-PKINIT-cert-matching-data-construction.patch @@ -0,0 +1,105 @@ +From 3fe07aaa6d8b6115aa19e2c04087352a5c87a568 Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Tue, 24 Oct 2017 15:33:37 -0400 +Subject: [PATCH] Fix PKINIT cert matching data construction + +Rewrite X509_NAME_oneline_ex() and its call sites to use dynamic +allocation and to perform proper error checking. + +(cherry picked from commit 1d8fb334a6256b9ddd3d4377a92c2441407d8a12) +--- + src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 63 ++++++++-------------- + 1 file changed, 21 insertions(+), 42 deletions(-) + +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +index 7fa2efd21..336102656 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +@@ -5139,33 +5139,23 @@ out: + return retval; + } + +-/* +- * Return a string format of an X509_NAME in buf where +- * size is an in/out parameter. On input it is the size +- * of the buffer, and on output it is the actual length +- * of the name. +- * If buf is NULL, returns the length req'd to hold name +- */ +-static char * +-X509_NAME_oneline_ex(X509_NAME * a, +- char *buf, +- unsigned int *size, +- unsigned long flag) ++static krb5_error_code ++rfc2253_name(X509_NAME *name, char **str_out) + { +- BIO *out = NULL; ++ BIO *b = NULL; ++ char *str; + +- out = BIO_new(BIO_s_mem ()); +- if (X509_NAME_print_ex(out, a, 0, flag) > 0) { +- if (buf != NULL && (*size) > (unsigned int) BIO_number_written(out)) { +- memset(buf, 0, *size); +- BIO_read(out, buf, (int) BIO_number_written(out)); +- } +- else { +- *size = BIO_number_written(out); +- } +- } +- BIO_free(out); +- return (buf); ++ *str_out = NULL; ++ b = BIO_new(BIO_s_mem()); ++ if (X509_NAME_print_ex(b, name, 0, XN_FLAG_SEP_COMMA_PLUS) < 0) ++ return ENOMEM; ++ str = calloc(BIO_number_written(b) + 1, 1); ++ if (str == NULL) ++ return ENOMEM; ++ BIO_read(b, str, BIO_number_written(b)); ++ BIO_free(b); ++ *str_out = str; ++ return 0; + } + + /* +@@ -5181,8 +5171,6 @@ crypto_cert_get_matching_data(krb5_context context, + krb5_principal *pkinit_sans =NULL, *upn_sans = NULL; + struct _pkinit_cert_data *cd = (struct _pkinit_cert_data *)ch; + unsigned int i, j; +- char buf[DN_BUF_LEN]; +- unsigned int bufsize = sizeof(buf); + + if (cd == NULL || cd->magic != CERT_MAGIC) + return EINVAL; +@@ -5195,23 +5183,14 @@ crypto_cert_get_matching_data(krb5_context context, + + md->ch = ch; + +- /* get the subject name (in rfc2253 format) */ +- X509_NAME_oneline_ex(X509_get_subject_name(cd->cred->cert), +- buf, &bufsize, XN_FLAG_SEP_COMMA_PLUS); +- md->subject_dn = strdup(buf); +- if (md->subject_dn == NULL) { +- retval = ENOMEM; ++ retval = rfc2253_name(X509_get_subject_name(cd->cred->cert), ++ &md->subject_dn); ++ if (retval) + goto cleanup; +- } +- +- /* get the issuer name (in rfc2253 format) */ +- X509_NAME_oneline_ex(X509_get_issuer_name(cd->cred->cert), +- buf, &bufsize, XN_FLAG_SEP_COMMA_PLUS); +- md->issuer_dn = strdup(buf); +- if (md->issuer_dn == NULL) { +- retval = ENOMEM; ++ retval = rfc2253_name(X509_get_issuer_name(cd->cred->cert), ++ &md->issuer_dn); ++ if (retval) + goto cleanup; +- } + + /* get the san data */ + retval = crypto_retrieve_X509_sans(context, cd->plgctx, cd->reqctx, diff --git a/krb5.spec b/krb5.spec index 5257456..e790908 100644 --- a/krb5.spec +++ b/krb5.spec @@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system Name: krb5 Version: 1.15.2 # for prerelease, should be e.g., 0.3.beta2% { ?dist } (without spaces) -Release: 3%{?dist} +Release: 4%{?dist} # lookaside-cached sources; two downloads and a build artifact Source0: https://web.mit.edu/kerberos/dist/krb5/1.15/krb5-%{version}%{prerelease}.tar.gz @@ -92,6 +92,7 @@ Patch68: Add-test-cert-with-no-extensions.patch Patch69: Add-PKINIT-test-case-for-generic-client-cert.patch Patch70: Add-hostname-based-ccselect-module.patch Patch71: Add-German-translation.patch +Patch72: Fix-PKINIT-cert-matching-data-construction.patch License: MIT URL: http://web.mit.edu/kerberos/www/ @@ -744,6 +745,9 @@ exit 0 %{_libdir}/libkadm5srv_mit.so.* %changelog +* Tue Oct 24 2017 Robbie Harwood - 1.15.2-4 +- Fix CVE-2017-15088 (Buffer overflow in get_matching_data()) + * Mon Oct 23 2017 Robbie Harwood - 1.15.2-3 - Drop dependency on python2-pyrad (dead upstream, broken with new python) From ee2993187ab2780f313a0e9125ceb5fe275f660e Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Mon, 29 Jan 2018 16:52:05 +0000 Subject: [PATCH 3/6] Process include directories in alphabetical order --- ...ed-directories-in-alphabetical-order.patch | 74 +++++++++++++++++++ krb5.spec | 6 +- 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 Process-included-directories-in-alphabetical-order.patch diff --git a/Process-included-directories-in-alphabetical-order.patch b/Process-included-directories-in-alphabetical-order.patch new file mode 100644 index 0000000..df80196 --- /dev/null +++ b/Process-included-directories-in-alphabetical-order.patch @@ -0,0 +1,74 @@ +From 5d5a6a48e9529fccac9e4c3487577276f8da69ef Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Mon, 29 Jan 2018 12:10:53 +0100 +Subject: [PATCH] Process included directories in alphabetical order + +readdir() and FindFirstFile()/FindNextFile() do not define any +ordering on the entries they return. Use sorted scandir() instead on +Unix-likes. + +(cherry picked from commit c2734538945d284a21bc8ad17404fca1eecdcf86) +--- + src/util/profile/prof_parse.c | 26 ++++++++++++++++---------- + 1 file changed, 16 insertions(+), 10 deletions(-) + +diff --git a/src/util/profile/prof_parse.c b/src/util/profile/prof_parse.c +index 1baceea9e..6c77f3a0c 100644 +--- a/src/util/profile/prof_parse.c ++++ b/src/util/profile/prof_parse.c +@@ -241,12 +241,18 @@ static int valid_name(const char *filename) + } + return 1; + } ++#ifndef _WIN32 ++static int valid_name_scandir(const struct dirent *d) ++{ ++ return valid_name(d->d_name); ++} ++#endif + + /* + * Include files within dirname. Only files with names ending in ".conf", or + * consisting entirely of alphanumeric characters, dashes, and underscores are + * included. This restriction avoids including editor backup files, .rpmsave +- * files, and the like. ++ * files, and the like. Files are processed in alphanumeric order. + */ + static errcode_t parse_include_dir(const char *dirname, + struct profile_node *root_section) +@@ -287,18 +293,17 @@ cleanup: + + #else /* not _WIN32 */ + +- DIR *dir; + char *pathname; + errcode_t retval = 0; +- struct dirent *ent; ++ struct dirent **namelist; ++ int num_ents, i; + +- dir = opendir(dirname); +- if (dir == NULL) ++ num_ents = scandir(dirname, &namelist, &valid_name_scandir, &alphasort); ++ if (num_ents == -1) + return PROF_FAIL_INCLUDE_DIR; +- while ((ent = readdir(dir)) != NULL) { +- if (!valid_name(ent->d_name)) +- continue; +- if (asprintf(&pathname, "%s/%s", dirname, ent->d_name) < 0) { ++ ++ for (i = 0; i < num_ents; i++) { ++ if (asprintf(&pathname, "%s/%s", dirname, namelist[i]->d_name) < 0) { + retval = ENOMEM; + break; + } +@@ -307,7 +312,8 @@ cleanup: + if (retval) + break; + } +- closedir(dir); ++ ++ free(namelist); + return retval; + #endif /* not _WIN32 */ + } diff --git a/krb5.spec b/krb5.spec index e790908..9f43f76 100644 --- a/krb5.spec +++ b/krb5.spec @@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system Name: krb5 Version: 1.15.2 # for prerelease, should be e.g., 0.3.beta2% { ?dist } (without spaces) -Release: 4%{?dist} +Release: 5%{?dist} # lookaside-cached sources; two downloads and a build artifact Source0: https://web.mit.edu/kerberos/dist/krb5/1.15/krb5-%{version}%{prerelease}.tar.gz @@ -93,6 +93,7 @@ Patch69: Add-PKINIT-test-case-for-generic-client-cert.patch Patch70: Add-hostname-based-ccselect-module.patch Patch71: Add-German-translation.patch Patch72: Fix-PKINIT-cert-matching-data-construction.patch +Patch73: Process-included-directories-in-alphabetical-order.patch License: MIT URL: http://web.mit.edu/kerberos/www/ @@ -745,6 +746,9 @@ exit 0 %{_libdir}/libkadm5srv_mit.so.* %changelog +* Mon Jan 29 2018 Robbie Harwood - 1.15.2-5 +- Process include directories in alphabetical order + * Tue Oct 24 2017 Robbie Harwood - 1.15.2-4 - Fix CVE-2017-15088 (Buffer overflow in get_matching_data()) From 702b0a90b5aaf5001227d93c6f65e1372c9af91c Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Mon, 12 Feb 2018 17:43:01 +0000 Subject: [PATCH 4/6] Fix leak in previous commit Resolves: #1540939 --- ...ed-directories-in-alphabetical-order.patch | 20 +++++++++++-------- krb5.spec | 6 +++++- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/Process-included-directories-in-alphabetical-order.patch b/Process-included-directories-in-alphabetical-order.patch index df80196..aeeae75 100644 --- a/Process-included-directories-in-alphabetical-order.patch +++ b/Process-included-directories-in-alphabetical-order.patch @@ -1,4 +1,4 @@ -From 5d5a6a48e9529fccac9e4c3487577276f8da69ef Mon Sep 17 00:00:00 2001 +From c7c44bbd80beabe7fb21f5fb6cfb9b57faa320f4 Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Mon, 29 Jan 2018 12:10:53 +0100 Subject: [PATCH] Process included directories in alphabetical order @@ -7,13 +7,13 @@ readdir() and FindFirstFile()/FindNextFile() do not define any ordering on the entries they return. Use sorted scandir() instead on Unix-likes. -(cherry picked from commit c2734538945d284a21bc8ad17404fca1eecdcf86) +(cherry picked from commit 4e8518baeedf376ae3e4ce302c9a138263d648df) --- - src/util/profile/prof_parse.c | 26 ++++++++++++++++---------- - 1 file changed, 16 insertions(+), 10 deletions(-) + src/util/profile/prof_parse.c | 30 ++++++++++++++++++++---------- + 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/util/profile/prof_parse.c b/src/util/profile/prof_parse.c -index 1baceea9e..6c77f3a0c 100644 +index 1baceea9e..309c27d07 100644 --- a/src/util/profile/prof_parse.c +++ b/src/util/profile/prof_parse.c @@ -241,12 +241,18 @@ static int valid_name(const char *filename) @@ -36,7 +36,7 @@ index 1baceea9e..6c77f3a0c 100644 */ static errcode_t parse_include_dir(const char *dirname, struct profile_node *root_section) -@@ -287,18 +293,17 @@ cleanup: +@@ -287,18 +293,19 @@ cleanup: #else /* not _WIN32 */ @@ -58,15 +58,19 @@ index 1baceea9e..6c77f3a0c 100644 - if (asprintf(&pathname, "%s/%s", dirname, ent->d_name) < 0) { + + for (i = 0; i < num_ents; i++) { -+ if (asprintf(&pathname, "%s/%s", dirname, namelist[i]->d_name) < 0) { ++ retval = asprintf(&pathname, "%s/%s", dirname, namelist[i]->d_name); ++ free(namelist[i]); ++ if (retval < 0) { retval = ENOMEM; break; } -@@ -307,7 +312,8 @@ cleanup: +@@ -307,7 +314,10 @@ cleanup: if (retval) break; } - closedir(dir); ++ for (i++; i < num_ents; i++) ++ free(namelist[i]); + + free(namelist); return retval; diff --git a/krb5.spec b/krb5.spec index 9f43f76..6fa2647 100644 --- a/krb5.spec +++ b/krb5.spec @@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system Name: krb5 Version: 1.15.2 # for prerelease, should be e.g., 0.3.beta2% { ?dist } (without spaces) -Release: 5%{?dist} +Release: 6%{?dist} # lookaside-cached sources; two downloads and a build artifact Source0: https://web.mit.edu/kerberos/dist/krb5/1.15/krb5-%{version}%{prerelease}.tar.gz @@ -746,6 +746,10 @@ exit 0 %{_libdir}/libkadm5srv_mit.so.* %changelog +* Mon Feb 12 2018 Robbie Harwood - 1.15.2-6 +- Fix leak in previous commit +- Resolves: #1540939 + * Mon Jan 29 2018 Robbie Harwood - 1.15.2-5 - Process include directories in alphabetical order From 466fd80d0edc5ad7a67157a247f9210f7e659047 Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Tue, 13 Feb 2018 16:11:51 +0000 Subject: [PATCH 5/6] Fix flaws in LDAP DN checking CVE-2018-5729, CVE-2018-5730 --- Fix-flaws-in-LDAP-DN-checking.patch | 346 ++++++++++++++++++++++++++++ krb5.spec | 7 +- 2 files changed, 352 insertions(+), 1 deletion(-) create mode 100644 Fix-flaws-in-LDAP-DN-checking.patch diff --git a/Fix-flaws-in-LDAP-DN-checking.patch b/Fix-flaws-in-LDAP-DN-checking.patch new file mode 100644 index 0000000..4a775bb --- /dev/null +++ b/Fix-flaws-in-LDAP-DN-checking.patch @@ -0,0 +1,346 @@ +From 27581397cd0d2f213c91bdf20ea9a6736f3e60dc Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Fri, 12 Jan 2018 11:43:01 -0500 +Subject: [PATCH] Fix flaws in LDAP DN checking + +KDB_TL_USER_INFO tl-data is intended to be internal to the LDAP KDB +module, and not used in disk or wire principal entries. Prevent +kadmin clients from sending KDB_TL_USER_INFO tl-data by giving it a +type number less than 256 and filtering out type numbers less than 256 +in kadm5_create_principal_3(). (We already filter out low type +numbers in kadm5_modify_principal()). + +In the LDAP KDB module, if containerdn and linkdn are both specified +in a put_principal operation, check both linkdn and the computed +standalone_principal_dn for container membership. To that end, factor +out the checks into helper functions and call them on all applicable +client-influenced DNs. + +CVE-2018-5729: + +In MIT krb5 1.6 or later, an authenticated kadmin user with permission +to add principals to an LDAP Kerberos database can cause a null +dereference in kadmind, or circumvent a DN container check, by +supplying tagged data intended to be internal to the database module. +Thanks to Sharwan Ram and Pooja Anil for discovering the potential +null dereference. + +CVE-2018-5730: + +In MIT krb5 1.6 or later, an authenticated kadmin user with permission +to add principals to an LDAP Kerberos database can circumvent a DN +containership check by supplying both a "linkdn" and "containerdn" +database argument, or by supplying a DN string which is a left +extension of a container DN string but is not hierarchically within +the container DN. + +ticket: 8643 (new) +tags: pullup +target_version: 1.16-next +target_version: 1.15-next + +(cherry picked from commit e1caf6fb74981da62039846931ebdffed71309d1) +--- + src/lib/kadm5/srv/svr_principal.c | 7 + + src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h | 2 +- + src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c | 200 +++++++++++---------- + src/tests/t_kdb.py | 11 ++ + 4 files changed, 125 insertions(+), 95 deletions(-) + +diff --git a/src/lib/kadm5/srv/svr_principal.c b/src/lib/kadm5/srv/svr_principal.c +index 2420f2c2b..a59a65e8f 100644 +--- a/src/lib/kadm5/srv/svr_principal.c ++++ b/src/lib/kadm5/srv/svr_principal.c +@@ -330,6 +330,13 @@ kadm5_create_principal_3(void *server_handle, + return KADM5_BAD_MASK; + if((mask & ~ALL_PRINC_MASK)) + return KADM5_BAD_MASK; ++ if (mask & KADM5_TL_DATA) { ++ for (tl_data_tail = entry->tl_data; tl_data_tail != NULL; ++ tl_data_tail = tl_data_tail->tl_data_next) { ++ if (tl_data_tail->tl_data_type < 256) ++ return KADM5_BAD_TL_TYPE; ++ } ++ } + + /* + * Check to see if the principal exists +diff --git a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h +index 535a1f309..8b8420faa 100644 +--- a/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h ++++ b/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h +@@ -141,7 +141,7 @@ extern int set_ldap_error (krb5_context ctx, int st, int op); + #define UNSTORE16_INT(ptr, val) (val = load_16_be(ptr)) + #define UNSTORE32_INT(ptr, val) (val = load_32_be(ptr)) + +-#define KDB_TL_USER_INFO 0x7ffe ++#define KDB_TL_USER_INFO 0xff + + #define KDB_TL_PRINCTYPE 0x01 + #define KDB_TL_PRINCCOUNT 0x02 +diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c +index 88a170495..b7c9212cb 100644 +--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c ++++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c +@@ -651,6 +651,107 @@ cleanup: + return ret; + } + ++static krb5_error_code ++check_dn_in_container(krb5_context context, const char *dn, ++ char *const *subtrees, unsigned int ntrees) ++{ ++ unsigned int i; ++ size_t dnlen = strlen(dn), stlen; ++ ++ for (i = 0; i < ntrees; i++) { ++ if (subtrees[i] == NULL || *subtrees[i] == '\0') ++ return 0; ++ stlen = strlen(subtrees[i]); ++ if (dnlen >= stlen && ++ strcasecmp(dn + dnlen - stlen, subtrees[i]) == 0 && ++ (dnlen == stlen || dn[dnlen - stlen - 1] == ',')) ++ return 0; ++ } ++ ++ k5_setmsg(context, EINVAL, _("DN is out of the realm subtree")); ++ return EINVAL; ++} ++ ++static krb5_error_code ++check_dn_exists(krb5_context context, ++ krb5_ldap_server_handle *ldap_server_handle, ++ const char *dn, krb5_boolean nonkrb_only) ++{ ++ krb5_error_code st = 0, tempst; ++ krb5_ldap_context *ldap_context = context->dal_handle->db_context; ++ LDAP *ld = ldap_server_handle->ldap_handle; ++ LDAPMessage *result = NULL, *ent; ++ char *attrs[] = { "krbticketpolicyreference", "krbprincipalname", NULL }; ++ char **values; ++ ++ LDAP_SEARCH_1(dn, LDAP_SCOPE_BASE, 0, attrs, IGNORE_STATUS); ++ if (st != LDAP_SUCCESS) ++ return set_ldap_error(context, st, OP_SEARCH); ++ ++ ent = ldap_first_entry(ld, result); ++ CHECK_NULL(ent); ++ ++ values = ldap_get_values(ld, ent, "krbticketpolicyreference"); ++ if (values != NULL) ++ ldap_value_free(values); ++ ++ values = ldap_get_values(ld, ent, "krbprincipalname"); ++ if (values != NULL) { ++ ldap_value_free(values); ++ if (nonkrb_only) { ++ st = EINVAL; ++ k5_setmsg(context, st, _("ldap object is already kerberized")); ++ goto cleanup; ++ } ++ } ++ ++cleanup: ++ ldap_msgfree(result); ++ return st; ++} ++ ++static krb5_error_code ++validate_xargs(krb5_context context, ++ krb5_ldap_server_handle *ldap_server_handle, ++ const xargs_t *xargs, const char *standalone_dn, ++ char *const *subtrees, unsigned int ntrees) ++{ ++ krb5_error_code st; ++ ++ if (xargs->dn != NULL) { ++ /* The supplied dn must be within a realm container. */ ++ st = check_dn_in_container(context, xargs->dn, subtrees, ntrees); ++ if (st) ++ return st; ++ /* The supplied dn must exist without Kerberos attributes. */ ++ st = check_dn_exists(context, ldap_server_handle, xargs->dn, TRUE); ++ if (st) ++ return st; ++ } ++ ++ if (xargs->linkdn != NULL) { ++ /* The supplied linkdn must be within a realm container. */ ++ st = check_dn_in_container(context, xargs->linkdn, subtrees, ntrees); ++ if (st) ++ return st; ++ /* The supplied linkdn must exist. */ ++ st = check_dn_exists(context, ldap_server_handle, xargs->linkdn, ++ FALSE); ++ if (st) ++ return st; ++ } ++ ++ if (xargs->containerdn != NULL && standalone_dn != NULL) { ++ /* standalone_dn (likely composed using containerdn) must be within a ++ * container. */ ++ st = check_dn_in_container(context, standalone_dn, subtrees, ntrees); ++ if (st) ++ return st; ++ } ++ ++ return 0; ++} ++ + krb5_error_code + krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + char **db_args) +@@ -662,12 +763,12 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + LDAPMessage *result=NULL, *ent=NULL; + char **subtreelist = NULL; + char *user=NULL, *subtree=NULL, *principal_dn=NULL; +- char **values=NULL, *strval[10]={NULL}, errbuf[1024]; ++ char *strval[10]={NULL}, errbuf[1024]; + char *filtuser=NULL; + struct berval **bersecretkey=NULL; + LDAPMod **mods=NULL; + krb5_boolean create_standalone=FALSE; +- krb5_boolean krb_identity_exists=FALSE, establish_links=FALSE; ++ krb5_boolean establish_links=FALSE; + char *standalone_principal_dn=NULL; + krb5_tl_data *tl_data=NULL; + krb5_key_data **keys=NULL; +@@ -860,24 +961,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + * any of the subtrees + */ + if (xargs.dn_from_kbd == TRUE) { +- /* make sure the DN falls in the subtree */ +- int dnlen=0, subtreelen=0; +- char *dn=NULL; +- krb5_boolean outofsubtree=TRUE; +- +- if (xargs.dn != NULL) { +- dn = xargs.dn; +- } else if (xargs.linkdn != NULL) { +- dn = xargs.linkdn; +- } else if (standalone_principal_dn != NULL) { +- /* +- * Even though the standalone_principal_dn is constructed +- * within this function, there is the containerdn input +- * from the user that can become part of the it. +- */ +- dn = standalone_principal_dn; +- } +- + /* Get the current subtree list if we haven't already done so. */ + if (subtreelist == NULL) { + st = krb5_get_subtree_info(ldap_context, &subtreelist, &ntrees); +@@ -885,81 +968,10 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + goto cleanup; + } + +- for (tre=0; tre= subtreelen) && (strcasecmp((dn + dnlen - subtreelen), subtreelist[tre]) == 0)) { +- outofsubtree = FALSE; +- break; +- } +- } +- } +- +- if (outofsubtree == TRUE) { +- st = EINVAL; +- k5_setmsg(context, st, _("DN is out of the realm subtree")); ++ st = validate_xargs(context, ldap_server_handle, &xargs, ++ standalone_principal_dn, subtreelist, ntrees); ++ if (st) + goto cleanup; +- } +- +- /* +- * dn value will be set either by dn, linkdn or the standalone_principal_dn +- * In the first 2 cases, the dn should be existing and in the last case we +- * are supposed to create the ldap object. so the below should not be +- * executed for the last case. +- */ +- +- if (standalone_principal_dn == NULL) { +- /* +- * If the ldap object is missing, this results in an error. +- */ +- +- /* +- * Search for krbprincipalname attribute here. +- * This is to find if a kerberos identity is already present +- * on the ldap object, in which case adding a kerberos identity +- * on the ldap object should result in an error. +- */ +- char *attributes[]={"krbticketpolicyreference", "krbprincipalname", NULL}; +- +- ldap_msgfree(result); +- result = NULL; +- LDAP_SEARCH_1(dn, LDAP_SCOPE_BASE, 0, attributes, IGNORE_STATUS); +- if (st == LDAP_SUCCESS) { +- ent = ldap_first_entry(ld, result); +- if (ent != NULL) { +- if ((values=ldap_get_values(ld, ent, "krbticketpolicyreference")) != NULL) { +- ldap_value_free(values); +- } +- +- if ((values=ldap_get_values(ld, ent, "krbprincipalname")) != NULL) { +- krb_identity_exists = TRUE; +- ldap_value_free(values); +- } +- } +- } else { +- st = set_ldap_error(context, st, OP_SEARCH); +- goto cleanup; +- } +- } +- } +- +- /* +- * If xargs.dn is set then the request is to add a +- * kerberos principal on a ldap object, but if +- * there is one already on the ldap object this +- * should result in an error. +- */ +- +- if (xargs.dn != NULL && krb_identity_exists == TRUE) { +- st = EINVAL; +- snprintf(errbuf, sizeof(errbuf), +- _("ldap object is already kerberized")); +- k5_setmsg(context, st, "%s", errbuf); +- goto cleanup; + } + + if (xargs.linkdn != NULL) { +diff --git a/src/tests/t_kdb.py b/src/tests/t_kdb.py +index 217f2cdc3..6e563b103 100755 +--- a/src/tests/t_kdb.py ++++ b/src/tests/t_kdb.py +@@ -203,6 +203,12 @@ if out != 'KRBTEST.COM\n': + # in the test LDAP server. + realm.run([kadminl, 'ank', '-randkey', '-x', 'dn=cn=krb5', 'princ1'], + expected_code=1, expected_msg='DN is out of the realm subtree') ++# Check that the DN container check is a hierarchy test, not a simple ++# suffix match (CVE-2018-5730). We expect this operation to fail ++# either way (because "xcn" isn't a valid DN tag) but the container ++# check should happen before the DN is parsed. ++realm.run([kadminl, 'ank', '-randkey', '-x', 'dn=xcn=t1,cn=krb5', 'princ1'], ++ expected_code=1, expected_msg='DN is out of the realm subtree') + realm.run([kadminl, 'ank', '-randkey', '-x', 'dn=cn=t2,cn=krb5', 'princ1']) + realm.run([kadminl, 'getprinc', 'princ1'], expected_msg='Principal: princ1') + realm.run([kadminl, 'ank', '-randkey', '-x', 'dn=cn=t2,cn=krb5', 'again'], +@@ -226,6 +232,11 @@ realm.run([kadminl, 'ank', '-randkey', '-x', 'containerdn=cn=t1,cn=krb5', + 'princ3']) + realm.run([kadminl, 'modprinc', '-x', 'containerdn=cn=t2,cn=krb5', 'princ3'], + expected_code=1, expected_msg='containerdn option not supported') ++# Verify that containerdn is checked when linkdn is also supplied ++# (CVE-2018-5730). ++realm.run([kadminl, 'ank', '-randkey', '-x', 'containerdn=cn=krb5', ++ '-x', 'linkdn=cn=t2,cn=krb5', 'princ4'], expected_code=1, ++ expected_msg='DN is out of the realm subtree') + + # Create and modify a ticket policy. + kldaputil(['create_policy', '-maxtktlife', '3hour', '-maxrenewlife', '6hour', diff --git a/krb5.spec b/krb5.spec index 6fa2647..749a64d 100644 --- a/krb5.spec +++ b/krb5.spec @@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system Name: krb5 Version: 1.15.2 # for prerelease, should be e.g., 0.3.beta2% { ?dist } (without spaces) -Release: 6%{?dist} +Release: 7%{?dist} # lookaside-cached sources; two downloads and a build artifact Source0: https://web.mit.edu/kerberos/dist/krb5/1.15/krb5-%{version}%{prerelease}.tar.gz @@ -94,6 +94,7 @@ Patch70: Add-hostname-based-ccselect-module.patch Patch71: Add-German-translation.patch Patch72: Fix-PKINIT-cert-matching-data-construction.patch Patch73: Process-included-directories-in-alphabetical-order.patch +Patch74: Fix-flaws-in-LDAP-DN-checking.patch License: MIT URL: http://web.mit.edu/kerberos/www/ @@ -746,6 +747,10 @@ exit 0 %{_libdir}/libkadm5srv_mit.so.* %changelog +* Tue Feb 13 2018 Robbie Harwood - 1.15.2-7 +- Fix flaws in LDAP DN checking +- CVE-2018-5729, CVE-2018-5730 + * Mon Feb 12 2018 Robbie Harwood - 1.15.2-6 - Fix leak in previous commit - Resolves: #1540939 From 16c9e72d3cd855bd7da1736d05771edbbcc904ea Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Thu, 29 Mar 2018 10:43:22 -0400 Subject: [PATCH 6/6] Continue after KRB5_CC_END in KCM cache iteration (cherry picked from commit 09f9308fd8aba5c2e9bc1144487d3d0e18bee37a) --- ...r-KRB5_CC_END-in-KCM-cache-iteration.patch | 42 +++++++++++++++++++ krb5.spec | 6 ++- 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 Continue-after-KRB5_CC_END-in-KCM-cache-iteration.patch diff --git a/Continue-after-KRB5_CC_END-in-KCM-cache-iteration.patch b/Continue-after-KRB5_CC_END-in-KCM-cache-iteration.patch new file mode 100644 index 0000000..999a5ba --- /dev/null +++ b/Continue-after-KRB5_CC_END-in-KCM-cache-iteration.patch @@ -0,0 +1,42 @@ +From 3001200ba4598aeb14511353a72dc746034280b1 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= +Date: Wed, 28 Mar 2018 18:27:06 +0200 +Subject: [PATCH] Continue after KRB5_CC_END in KCM cache iteration + +The KCM server returns KRB5_CC_END in response to a GET_CACHE_BY_UUID +request to indicate that the specified ccache uuid no longer exists. +In krb5_ptcursor_next(), ignore this error and continue the iteration, +as the Heimdal KCM client code does. + +In addition to addressing the case where a third party deletes a cache +between the GET_CACHE_UUID_LIST request and when we reach that uuid in +the iteration, this change also fixes a bug in kdestroy -A where the +caller deletes the primary cache and we later request it by uuid when +iterating over the list. + +[ghudson@mit.edu: rewrote commit message; edited comment] + +ticket: 8658 (new) +tags: pullup +target_version: 1.16-next +target_version: 1.15-next + +(cherry picked from commit 49087f5e6309f298f8898c35af6f4ade418ced60) +--- + src/lib/krb5/ccache/cc_kcm.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/src/lib/krb5/ccache/cc_kcm.c b/src/lib/krb5/ccache/cc_kcm.c +index b621ed33b..0d38b1839 100644 +--- a/src/lib/krb5/ccache/cc_kcm.c ++++ b/src/lib/krb5/ccache/cc_kcm.c +@@ -966,6 +966,9 @@ kcm_ptcursor_next(krb5_context context, krb5_cc_ptcursor cursor, + kcmreq_init(&req, KCM_OP_GET_CACHE_BY_UUID, NULL); + k5_buf_add_len(&req.reqbuf, id, KCM_UUID_LEN); + ret = kcmio_call(context, data->io, &req); ++ /* Continue if the cache has been deleted. */ ++ if (ret == KRB5_CC_END) ++ continue; + if (ret) + goto cleanup; + ret = kcmreq_get_name(&req, &name); diff --git a/krb5.spec b/krb5.spec index 749a64d..390f41d 100644 --- a/krb5.spec +++ b/krb5.spec @@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system Name: krb5 Version: 1.15.2 # for prerelease, should be e.g., 0.3.beta2% { ?dist } (without spaces) -Release: 7%{?dist} +Release: 8%{?dist} # lookaside-cached sources; two downloads and a build artifact Source0: https://web.mit.edu/kerberos/dist/krb5/1.15/krb5-%{version}%{prerelease}.tar.gz @@ -95,6 +95,7 @@ Patch71: Add-German-translation.patch Patch72: Fix-PKINIT-cert-matching-data-construction.patch Patch73: Process-included-directories-in-alphabetical-order.patch Patch74: Fix-flaws-in-LDAP-DN-checking.patch +Patch75: Continue-after-KRB5_CC_END-in-KCM-cache-iteration.patch License: MIT URL: http://web.mit.edu/kerberos/www/ @@ -747,6 +748,9 @@ exit 0 %{_libdir}/libkadm5srv_mit.so.* %changelog +* Sat May 05 2018 Lukas Slebodnik - 1.15.2-8 +- Continue after KRB5_CC_END in KCM cache iteration + * Tue Feb 13 2018 Robbie Harwood - 1.15.2-7 - Fix flaws in LDAP DN checking - CVE-2018-5729, CVE-2018-5730