Skip to content

Commit 04038bf

Browse files
committed
Support keyless principals in LDAP [CVE-2014-5354]
Operations like "kadmin -q 'addprinc -nokey foo'" or "kadmin -q 'purgekeys -all foo'" result in principal entries with no keys present, so krb5_encode_krbsecretkey() would just return NULL, which then got unconditionally dereferenced in krb5_add_ber_mem_ldap_mod(). Apply some fixes to krb5_encode_krbsecretkey() to handle zero-key principals better, correct the test for an allocation failure, and slightly restructure the cleanup handler to be shorter and more appropriate for the usage. Once it no longer short-circuits when n_key_data is zero, it will produce an array of length two with both entries NULL, which is treated as an empty list by the LDAP library, the correct behavior for a keyless principal. However, attributes with empty values are only handled by the LDAP library for Modify operations, not Add operations (which only get a sequence of Attribute, with no operation field). Therefore, only add an empty krbprincipalkey to the modlist when we will be performing a Modify, and not when we will be performing an Add, which is conditional on the (misspelled) create_standalone_prinicipal boolean. CVE-2014-5354: In MIT krb5, when kadmind is configured to use LDAP for the KDC database, an authenticated remote attacker can cause a NULL dereference by inserting into the database a principal entry which contains no long-term keys. In order for the LDAP KDC backend to translate a principal entry from the database abstraction layer into the form expected by the LDAP schema, the principal's keys are encoded into a NULL-terminated array of length-value entries to be stored in the LDAP database. However, the subroutine which produced this array did not correctly handle the case where no keys were present, returning NULL instead of an empty array, and the array was unconditionally dereferenced while adding to the list of LDAP operations to perform. Versions of MIT krb5 prior to 1.12 did not expose a way for principal entries to have no long-term key material, and therefore are not vulnerable. CVSSv2 Vector: AV:N/AC:M/Au:S/C:N/I:N/A:P/E:H/RL:OF/RC:C ticket: 8041 (new) tags: pullup target_version: 1.13.1 subject: kadmind with ldap backend crashes when putting keyless entries
1 parent e8df045 commit 04038bf

File tree

1 file changed

+17
-8
lines changed

1 file changed

+17
-8
lines changed

Diff for: src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c

+17-8
Original file line numberDiff line numberDiff line change
@@ -406,14 +406,14 @@ krb5_encode_krbsecretkey(krb5_key_data *key_data_in, int n_key_data,
406406
int num_versions = 1;
407407
int i, j, last;
408408
krb5_error_code err = 0;
409-
krb5_key_data *key_data;
409+
krb5_key_data *key_data = NULL;
410410

411-
if (n_key_data <= 0)
411+
if (n_key_data < 0)
412412
return NULL;
413413

414414
/* Make a shallow copy of the key data so we can alter it. */
415415
key_data = k5calloc(n_key_data, sizeof(*key_data), &err);
416-
if (key_data_in == NULL)
416+
if (key_data == NULL)
417417
goto cleanup;
418418
memcpy(key_data, key_data_in, n_key_data * sizeof(*key_data));
419419

@@ -467,9 +467,8 @@ krb5_encode_krbsecretkey(krb5_key_data *key_data_in, int n_key_data,
467467
free(key_data);
468468
if (err != 0) {
469469
if (ret != NULL) {
470-
for (i = 0; i <= num_versions; i++)
471-
if (ret[i] != NULL)
472-
free (ret[i]);
470+
for (i = 0; ret[i] != NULL; i++)
471+
free (ret[i]);
473472
free (ret);
474473
ret = NULL;
475474
}
@@ -1036,9 +1035,19 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry,
10361035
bersecretkey = krb5_encode_krbsecretkey (entry->key_data,
10371036
entry->n_key_data, mkvno);
10381037

1039-
if ((st=krb5_add_ber_mem_ldap_mod(&mods, "krbprincipalkey",
1040-
LDAP_MOD_REPLACE | LDAP_MOD_BVALUES, bersecretkey)) != 0)
1038+
if (bersecretkey == NULL) {
1039+
st = ENOMEM;
10411040
goto cleanup;
1041+
}
1042+
/* An empty list of bervals is only accepted for modify operations,
1043+
* not add operations. */
1044+
if (bersecretkey[0] != NULL || !create_standalone_prinicipal) {
1045+
st = krb5_add_ber_mem_ldap_mod(&mods, "krbprincipalkey",
1046+
LDAP_MOD_REPLACE | LDAP_MOD_BVALUES,
1047+
bersecretkey);
1048+
if (st != 0)
1049+
goto cleanup;
1050+
}
10421051

10431052
if (!(entry->mask & KADM5_PRINCIPAL)) {
10441053
memset(strval, 0, sizeof(strval));

0 commit comments

Comments
 (0)