Fix flaws in LDAP DN checking
CVE-2018-5729, CVE-2018-5730
This commit is contained in:
parent
c4848e3332
commit
392309c493
346
Fix-flaws-in-LDAP-DN-checking.patch
Normal file
346
Fix-flaws-in-LDAP-DN-checking.patch
Normal file
@ -0,0 +1,346 @@
|
||||
From 27581397cd0d2f213c91bdf20ea9a6736f3e60dc 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
|
||||
|
||||
(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<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 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',
|
@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system
|
||||
Name: krb5
|
||||
Version: 1.16
|
||||
# for prerelease, should be e.g., 0.% {prerelease}.1% { ?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.16/krb5-%{version}%{prerelease}.tar.gz
|
||||
@ -61,6 +61,7 @@ Patch34: krb5-1.9-debuginfo.patch
|
||||
Patch35: krb5-1.11-run_user_0.patch
|
||||
Patch36: krb5-1.11-kpasswdtest.patch
|
||||
Patch37: Process-included-directories-in-alphabetical-order.patch
|
||||
Patch38: Fix-flaws-in-LDAP-DN-checking.patch
|
||||
|
||||
License: MIT
|
||||
URL: http://web.mit.edu/kerberos/www/
|
||||
@ -714,6 +715,10 @@ exit 0
|
||||
%{_libdir}/libkadm5srv_mit.so.*
|
||||
|
||||
%changelog
|
||||
* Tue Feb 13 2018 Robbie Harwood <rharwood@redhat.com> - 1.16-7
|
||||
- Fix flaws in LDAP DN checking
|
||||
- CVE-2018-5729, CVE-2018-5730
|
||||
|
||||
* Mon Feb 12 2018 Robbie Harwood <rharwood@redhat.com> - 1.16-6
|
||||
- Fix a leak in the previous commit
|
||||
- Restore dist macro that was accidentally removed
|
||||
|
Loading…
Reference in New Issue
Block a user