Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP2:GA
krb5.17090
0011-Fix-flaws-in-LDAP-DN-checking.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 0011-Fix-flaws-in-LDAP-DN-checking.patch of Package krb5.17090
From 71fedac4e7ffddc9d256812697721a7794134f02 Mon Sep 17 00:00:00 2001 From: Greg Hudson <ghudson@mit.edu> 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. (cherry picked from commit e1caf6fb74981da62039846931ebdffed71309d1) ticket: 8643 version_fixed: 1.15.3 (cherry picked from commit a2df94b6bb22ecf6e77c5044cff0f627b4fb30cf) --- src/lib/kadm5/srv/svr_principal.c | 7 + src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h | 2 +- .../kdb/ldap/libkdb_ldap/ldap_principal2.c | 200 ++++++++++-------- src/tests/t_kdb.py | 14 ++ 4 files changed, 128 insertions(+), 95 deletions(-) diff --git a/src/lib/kadm5/srv/svr_principal.c b/src/lib/kadm5/srv/svr_principal.c index 8f4da0e52..a35c24900 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 06b477537..0c19804ad 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 7ba53f959..0f32cc1d8 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<ntrees; ++tre) { - if (subtreelist[tre] == NULL || strlen(subtreelist[tre]) == 0) { - outofsubtree = FALSE; - break; - } else { - dnlen = strlen (dn); - subtreelen = strlen(subtreelist[tre]); - if ((dnlen >= 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 185225afa..001905f5f 100755 --- a/src/tests/t_kdb.py +++ b/src/tests/t_kdb.py @@ -171,6 +171,14 @@ out = realm.run([kadminl, 'ank', '-randkey', '-x', 'dn=cn=krb5', 'princ1'], expected_code=1) if 'DN is out of the realm subtree' not in out: fail('Unexpected kadmin.local output for out-of-realm dn') +# 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. +out = realm.run([kadminl, 'ank', '-randkey', '-x', 'dn=xcn=t1,cn=krb5', + 'princ1'], expected_code=1) +if 'DN is out of the realm subtree' not in out: + fail('Unexpected kadmin.local output for non-hierarchy dn prefix') realm.run([kadminl, 'ank', '-randkey', '-x', 'dn=cn=t2,cn=krb5', 'princ1']) out = realm.run([kadminl, 'getprinc', 'princ1']) if 'Principal: princ1' not in out: @@ -208,6 +216,12 @@ out = realm.run([kadminl, 'modprinc', '-x', 'containerdn=cn=t2,cn=krb5', 'princ3'], expected_code=1) if 'containerdn option not supported' not in out: fail('Unexpected kadmin.local output trying to reset containerdn') +# Verify that containerdn is checked when linkdn is also supplied +# (CVE-2018-5730). +out = realm.run([kadminl, 'ank', '-randkey', '-x', 'containerdn=cn=krb5', + '-x', 'linkdn=cn=t2,cn=krb5', 'princ4'], expected_code=1) +if 'DN is out of the realm subtree' not in out: + fail('Unexpected kadmin.local output for linkdn+containerdn test') # Create and modify a ticket policy. kldaputil(['create_policy', '-maxtktlife', '3hour', '-maxrenewlife', '6hour', -- 2.20.1
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
