new file mode 100644

@@ -0,0 +1,344 @@

+From e1caf6fb74981da62039846931ebdffed71309d1 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.

+

+ticket: 8643 (new)

+tags: pullup

+target_version: 1.16-next

+target_version: 1.15-next

+---

+ 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                            |  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 2420f2c2be..a59a65e8f6 100644

+--- a/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 535a1f309e..8b8420faa9 100644

+--- a/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 88a1704950..b7c9212cb2 100644

+--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c

+@@ -651,6 +651,107 @@ update_ldap_mod_auth_ind(krb5_context context, krb5_db_entry *entry,

+     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 217f2cdc3b..6e563b1032 100755

+--- a/src/tests/t_kdb.py

+@@ -203,6 +203,12 @@ def ldap_add(dn, objectclass, attrs=[]):

+ # 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 @@ def ldap_add(dn, objectclass, attrs=[]):

+            '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',

@@ -1,7 +1,7 @@

 Summary:        The Kerberos newtork authentication system

 Name:           krb5

 Version:        1.16

-Release:        1%{?dist}

+Release:        2%{?dist}

 License:        MIT

 URL:            http://web.mit.edu/kerberos/

 Group:          System Environment/Security

@@ -10,6 +10,7 @@ Distribution:   Photon

 Source0:        http://web.mit.edu/kerberos/www/dist/%{name}/1.16/%{name}-%{version}.tar.gz

 %define sha1    krb5=e1bd68d9121c337faf5dbd478d0a2b6998114fc7

 Patch0:         krb5-1.15-never-unload-mechanisms.patch

+Patch1:         krb5-CVE-2018-5730.patch

 Requires:       openssl

 Requires:       e2fsprogs-libs

 BuildRequires:  openssl-devel

@@ -37,6 +38,7 @@ These are the additional language files of krb5.

 %prep

 %setup -q

 %patch0 -p1

+%patch1 -p1

 %build

 cd src &&

@@ -118,6 +120,8 @@ rm -rf %{buildroot}/*

 %{_datarootdir}/locale/*

 %changelog

+*   Mon Aug 13 2018 Dweep Advani <dadvani@vmware.com> 1.16-2

+-   Fix for CVE-2018-5729 and CVE-2018-5730

 *   Wed Dec 13 2017 Xiaolin Li <xiaolinl@vmware.com> 1.16-1

 -   Update to version 1.16 to address CVE-2017-15088

 *   Thu Sep 28 2017 Xiaolin Li <xiaolinl@vmware.com> 1.15.2-1