CVE-2018-5730 (C, published 2018-03-06T20:29Z)
Description: value: MIT krb5 1.6 or later allows an authenticated kadmin with permission to add principals to an LDAP Kerberos database to 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.
Severity: MEDIUM (CVSS v3 base score: 3.8)
Weakness: CWE-90 \u2014 Improper Neutralization of Special Elements used in an LDAP Query ('LDAP Injection')
The product constructs all or part of an LDAP query using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended LDAP query when it is sent to a downstream component.
Repository: https://github.com/krb5/krb5
Files changed:
src/lib/kadm5/srv/svr_principal.c
src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.h
src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
src/tests/t_kdb.py
Commit message: 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


Vulnerable code:
#define  KDB_TL_USER_INFO      0x7ffe
    char                        **values=NULL, *strval[10]={NULL}, errbuf[1024];
    krb5_boolean                krb_identity_exists=FALSE, establish_links=FALSE;
        /* 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;
        }

        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"));
        }

        /*
         * 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;

Fixed code:
    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;
        }
    }
#define  KDB_TL_USER_INFO      0xff
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;
}

    char                        *strval[10]={NULL}, errbuf[1024];
    krb5_boolean                establish_links=FALSE;
        st = validate_xargs(context, ldap_server_handle, &xargs,
                            standalone_principal_dn, subtreelist, ntrees);
        if (st)
# 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')
# 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')