summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorWes Cilldhaire <wes@sol1.com.au>2018-02-05 12:07:25 +1100
committerPatrice Clement <monsieurp@gentoo.org>2018-02-05 22:50:03 +0100
commite71df7341cdaa0a4cc2aeff56496ce1724b921d2 (patch)
tree1cdc955f9304b56096b736cc1809e920bcbd8ceb /net-nds/389-ds-base/files
parentdev-python/async: remove dead HOMEPAGE (diff)
downloadgentoo-e71df7341cdaa0a4cc2aeff56496ce1724b921d2.tar.gz
gentoo-e71df7341cdaa0a4cc2aeff56496ce1724b921d2.tar.bz2
gentoo-e71df7341cdaa0a4cc2aeff56496ce1724b921d2.zip
net-nds/389-ds-base: patch against CVE-2017-15135 in 1.3.6.8.
* Patch and revbump to 1.3.6.8 to address CVE-2017-15135 * Update copyright line in all versions for 2018 Bug: https://bugs.gentoo.org/645706 Acked-by: wibrown@redhat.com Package-Manager: Portage-2.3.20, Repoman-2.3.6 Closes: https://github.com/gentoo/gentoo/pull/7078
Diffstat (limited to 'net-nds/389-ds-base/files')
-rw-r--r--net-nds/389-ds-base/files/389-ds-base-1.3.6-backport-invalid-password-mig.patch376
1 files changed, 376 insertions, 0 deletions
diff --git a/net-nds/389-ds-base/files/389-ds-base-1.3.6-backport-invalid-password-mig.patch b/net-nds/389-ds-base/files/389-ds-base-1.3.6-backport-invalid-password-mig.patch
new file mode 100644
index 000000000000..b4ba70a2fb5f
--- /dev/null
+++ b/net-nds/389-ds-base/files/389-ds-base-1.3.6-backport-invalid-password-mig.patch
@@ -0,0 +1,376 @@
+From cefec5714cf0fdec4aa582a5fe020ef80d6024cd Mon Sep 17 00:00:00 2001
+From: William Brown <firstyear@redhat.com>
+Date: Thu, 18 Jan 2018 11:27:58 +1000
+Subject: [PATCH] Ticket bz1525628 1.3.6 backport - invalid password migration
+ causes unauth bind
+
+Bug Description: Slapi_ct_memcmp expects both inputs to be
+at LEAST size n. If they are not, we only compared UP to n.
+
+Invalid migrations of passwords (IE {CRYPT}XX) would create
+a pw which is just salt and no hash. ct_memcmp would then
+only verify the salt bits and would allow the authentication.
+
+This relies on an administrative mistake both of allowing
+password migration (nsslapd-allow-hashed-passwords) and then
+subsequently migrating an INVALID password to the server.
+
+Fix Description: slapi_ct_memcmp now access n1, n2 size
+and will FAIL if they are not the same, but will still compare
+n bytes, where n is the "longest" memory, to the first byte
+of the other to prevent length disclosure of the shorter
+value (generally the mis-migrated password)
+
+https://bugzilla.redhat.com/show_bug.cgi?id=1525628
+
+Author: wibrown
+
+Review by: ???
+---
+ .../bz1525628_ct_memcmp_invalid_hash_test.py | 56 ++++++++++++++++++++
+ ldap/servers/plugins/pwdstorage/clear_pwd.c | 4 +-
+ ldap/servers/plugins/pwdstorage/crypt_pwd.c | 4 +-
+ ldap/servers/plugins/pwdstorage/md5_pwd.c | 36 ++++++-------
+ ldap/servers/plugins/pwdstorage/sha_pwd.c | 18 +++++--
+ ldap/servers/plugins/pwdstorage/smd5_pwd.c | 60 +++++++++++-----------
+ ldap/servers/slapd/ch_malloc.c | 36 +++++++++++--
+ ldap/servers/slapd/slapi-plugin.h | 2 +-
+ 8 files changed, 155 insertions(+), 61 deletions(-)
+ create mode 100644 dirsrvtests/tests/suites/password/bz1525628_ct_memcmp_invalid_hash_test.py
+
+diff --git a/dirsrvtests/tests/suites/password/bz1525628_ct_memcmp_invalid_hash_test.py b/dirsrvtests/tests/suites/password/bz1525628_ct_memcmp_invalid_hash_test.py
+new file mode 100644
+index 0000000..2f38384
+--- /dev/null
++++ b/dirsrvtests/tests/suites/password/bz1525628_ct_memcmp_invalid_hash_test.py
+@@ -0,0 +1,56 @@
++# --- BEGIN COPYRIGHT BLOCK ---
++# Copyright (C) 2018 Red Hat, Inc.
++# All rights reserved.
++#
++# License: GPL (version 3 or any later version).
++# See LICENSE for details.
++# --- END COPYRIGHT BLOCK ---
++#
++
++import ldap
++import pytest
++import logging
++from lib389.topologies import topology_st
++from lib389._constants import PASSWORD, DEFAULT_SUFFIX
++
++from lib389.idm.user import UserAccounts, TEST_USER_PROPERTIES
++
++logging.getLogger(__name__).setLevel(logging.DEBUG)
++log = logging.getLogger(__name__)
++
++def test_invalid_hash_fails(topology_st):
++ """When given a malformed hash from userpassword migration
++ slapi_ct_memcmp would check only to the length of the shorter
++ field. This affects some values where it would ONLY verify
++ the salt is valid, and thus would allow any password to bind.
++
++ :id: 8131c029-7147-47db-8d03-ec5db2a01cfb
++ :setup: Standalone Instance
++ :steps:
++ 1. Create a user
++ 2. Add an invalid password hash (truncated)
++ 3. Attempt to bind
++ :expectedresults:
++ 1. User is added
++ 2. Invalid pw hash is added
++ 3. Bind fails
++ """
++ log.info("Running invalid hash test")
++
++ # Allow setting raw password hashes for migration.
++ topology_st.standalone.config.set('nsslapd-allow-hashed-passwords', 'on')
++
++ users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX)
++ user = users.create(properties=TEST_USER_PROPERTIES)
++ user.set('userPassword', '{CRYPT}XX')
++
++ # Attempt to bind. This should fail.
++ with pytest.raises(ldap.INVALID_CREDENTIALS):
++ user.bind(PASSWORD)
++ with pytest.raises(ldap.INVALID_CREDENTIALS):
++ user.bind('XX')
++ with pytest.raises(ldap.INVALID_CREDENTIALS):
++ user.bind('{CRYPT}XX')
++
++ log.info("PASSED")
++
+diff --git a/ldap/servers/plugins/pwdstorage/clear_pwd.c b/ldap/servers/plugins/pwdstorage/clear_pwd.c
+index b9b362d..050e60d 100644
+--- a/ldap/servers/plugins/pwdstorage/clear_pwd.c
++++ b/ldap/servers/plugins/pwdstorage/clear_pwd.c
+@@ -39,7 +39,7 @@ clear_pw_cmp( const char *userpwd, const char *dbpwd )
+ * However, even if the first part of userpw matches dbpwd, but len !=, we
+ * have already failed anyawy. This prevents substring matching.
+ */
+- if (slapi_ct_memcmp(userpwd, dbpwd, len_dbp) != 0) {
++ if (slapi_ct_memcmp(userpwd, dbpwd, len_user, len_dbp) != 0) {
+ result = 1;
+ }
+ } else {
+@@ -51,7 +51,7 @@ clear_pw_cmp( const char *userpwd, const char *dbpwd )
+ * dbpwd to itself. We have already got result == 1 if we are here, so we are
+ * just trying to take up time!
+ */
+- if (slapi_ct_memcmp(dbpwd, dbpwd, len_dbp)) {
++ if (slapi_ct_memcmp(dbpwd, dbpwd, len_dbp, len_dbp)) {
+ /* Do nothing, we have the if to fix a coverity check. */
+ }
+ }
+diff --git a/ldap/servers/plugins/pwdstorage/crypt_pwd.c b/ldap/servers/plugins/pwdstorage/crypt_pwd.c
+index dfd5af9..5fcff13 100644
+--- a/ldap/servers/plugins/pwdstorage/crypt_pwd.c
++++ b/ldap/servers/plugins/pwdstorage/crypt_pwd.c
+@@ -56,13 +56,13 @@ crypt_close(Slapi_PBlock *pb __attribute__((unused)))
+ int
+ crypt_pw_cmp( const char *userpwd, const char *dbpwd )
+ {
+- int rc;
++ int32_t rc;
+ char *cp;
+ PR_Lock(cryptlock);
+ /* we use salt (first 2 chars) of encoded password in call to crypt() */
+ cp = crypt( userpwd, dbpwd );
+ if (cp) {
+- rc= slapi_ct_memcmp( dbpwd, cp, strlen(dbpwd));
++ rc = slapi_ct_memcmp(dbpwd, cp, strlen(dbpwd), strlen(cp));
+ } else {
+ rc = -1;
+ }
+diff --git a/ldap/servers/plugins/pwdstorage/md5_pwd.c b/ldap/servers/plugins/pwdstorage/md5_pwd.c
+index b279946..2e1c472 100644
+--- a/ldap/servers/plugins/pwdstorage/md5_pwd.c
++++ b/ldap/servers/plugins/pwdstorage/md5_pwd.c
+@@ -30,13 +30,13 @@
+ int
+ md5_pw_cmp( const char *userpwd, const char *dbpwd )
+ {
+- int rc=-1;
+- char * bver;
+- PK11Context *ctx=NULL;
+- unsigned int outLen;
+- unsigned char hash_out[MD5_HASH_LEN];
+- unsigned char b2a_out[MD5_HASH_LEN*2]; /* conservative */
+- SECItem binary_item;
++ int32_t rc = -1;
++ char *bver;
++ PK11Context *ctx = NULL;
++ unsigned int outLen;
++ unsigned char hash_out[MD5_HASH_LEN];
++ unsigned char b2a_out[MD5_HASH_LEN * 2]; /* conservative */
++ SECItem binary_item;
+
+ ctx = PK11_CreateDigestContext(SEC_OID_MD5);
+ if (ctx == NULL) {
+@@ -51,17 +51,17 @@ md5_pw_cmp( const char *userpwd, const char *dbpwd )
+ PK11_DigestFinal(ctx, hash_out, &outLen, sizeof hash_out);
+ PK11_DestroyContext(ctx, 1);
+
+- /* convert the binary hash to base64 */
+- binary_item.data = hash_out;
+- binary_item.len = outLen;
+- bver = NSSBase64_EncodeItem(NULL, (char *)b2a_out, sizeof b2a_out, &binary_item);
+- /* bver points to b2a_out upon success */
+- if (bver) {
+- rc = slapi_ct_memcmp(bver,dbpwd, strlen(dbpwd));
+- } else {
+- slapi_log_err(SLAPI_LOG_PLUGIN, MD5_SUBSYSTEM_NAME,
+- "Could not base64 encode hashed value for password compare");
+- }
++ /* convert the binary hash to base64 */
++ binary_item.data = hash_out;
++ binary_item.len = outLen;
++ bver = NSSBase64_EncodeItem(NULL, (char *)b2a_out, sizeof b2a_out, &binary_item);
++ /* bver points to b2a_out upon success */
++ if (bver) {
++ rc = slapi_ct_memcmp(bver, dbpwd, strlen(dbpwd), strlen(bver));
++ } else {
++ slapi_log_err(SLAPI_LOG_PLUGIN, MD5_SUBSYSTEM_NAME,
++ "Could not base64 encode hashed value for password compare");
++ }
+ loser:
+ return rc;
+ }
+diff --git a/ldap/servers/plugins/pwdstorage/sha_pwd.c b/ldap/servers/plugins/pwdstorage/sha_pwd.c
+index 5f41c5b..c9db896 100644
+--- a/ldap/servers/plugins/pwdstorage/sha_pwd.c
++++ b/ldap/servers/plugins/pwdstorage/sha_pwd.c
+@@ -49,7 +49,7 @@ sha_pw_cmp (const char *userpwd, const char *dbpwd, unsigned int shaLen )
+ char userhash[MAX_SHA_HASH_SIZE];
+ char quick_dbhash[MAX_SHA_HASH_SIZE + SHA_SALT_LENGTH + 3];
+ char *dbhash = quick_dbhash;
+- struct berval salt;
++ struct berval salt = {0};
+ PRUint32 hash_len;
+ unsigned int secOID;
+ char *schemeName;
+@@ -120,10 +120,20 @@ sha_pw_cmp (const char *userpwd, const char *dbpwd, unsigned int shaLen )
+ }
+
+ /* the proof is in the comparison... */
+- if ( hash_len >= shaLen ) {
+- result = slapi_ct_memcmp( userhash, dbhash, shaLen );
++ if (hash_len >= shaLen) {
++ /*
++ * This say "if the hash has a salt IE >, OR if they are equal, check the hash component ONLY.
++ * This is why we repeat shaLen twice, even though it seems odd. If you have a dbhast of ssha
++ * it's len is 28, and the userpw is 20, but 0 - 20 is the sha, and 21-28 is the salt, which
++ * has already been processed into userhash.
++ * The case where dbpwd is truncated is handled above in "invalid base64" arm.
++ */
++ result = slapi_ct_memcmp(userhash, dbhash, shaLen, shaLen);
+ } else {
+- result = slapi_ct_memcmp( userhash, dbhash + OLD_SALT_LENGTH, hash_len - OLD_SALT_LENGTH );
++ /* This case is for if the salt is at the START, which only applies to DS40B1 case.
++ * May never be a valid check...
++ */
++ result = slapi_ct_memcmp(userhash, dbhash + OLD_SALT_LENGTH, shaLen, hash_len - OLD_SALT_LENGTH);
+ }
+
+ loser:
+diff --git a/ldap/servers/plugins/pwdstorage/smd5_pwd.c b/ldap/servers/plugins/pwdstorage/smd5_pwd.c
+index 2e9d195..f6b4bb4 100644
+--- a/ldap/servers/plugins/pwdstorage/smd5_pwd.c
++++ b/ldap/servers/plugins/pwdstorage/smd5_pwd.c
+@@ -52,35 +52,37 @@ smd5_pw_cmp( const char *userpwd, const char *dbpwd )
+ /*
+ * Decode hash stored in database.
+ */
+- hash_len = pwdstorage_base64_decode_len(dbpwd, 0);
+- if ( hash_len >= sizeof(quick_dbhash) ) { /* get more space: */
+- dbhash = (char*) slapi_ch_calloc( hash_len + 1, sizeof(char) );
+- if ( dbhash == NULL ) goto loser;
+- } else {
+- memset( quick_dbhash, 0, sizeof(quick_dbhash) );
+- }
+-
+- hashresult = PL_Base64Decode( dbpwd, 0, dbhash );
+- if (NULL == hashresult) {
+- slapi_log_err(SLAPI_LOG_PLUGIN, SALTED_MD5_SUBSYSTEM_NAME,
+- "smd5_pw_cmp: userPassword \"%s\" is the wrong length "
+- "or is not properly encoded BASE64\n", dbpwd );
+- goto loser;
+- }
+-
+- salt.bv_val = (void*)(dbhash + MD5_LENGTH); /* salt starts after hash value */
+- salt.bv_len = hash_len - MD5_LENGTH; /* remaining bytes must be salt */
+-
+- /* create the hash */
+- memset( userhash, 0, sizeof(userhash) );
+- PK11_DigestBegin(ctx);
+- PK11_DigestOp(ctx, (const unsigned char *)userpwd, strlen(userpwd));
+- PK11_DigestOp(ctx, (unsigned char*)(salt.bv_val), salt.bv_len);
+- PK11_DigestFinal(ctx, userhash, &outLen, sizeof userhash);
+- PK11_DestroyContext(ctx, 1);
+-
+- /* Compare everything up to the salt. */
+- rc = slapi_ct_memcmp( userhash, dbhash, MD5_LENGTH );
++ hash_len = pwdstorage_base64_decode_len(dbpwd, 0);
++ if (hash_len >= sizeof(quick_dbhash)) { /* get more space: */
++ dbhash = (char *)slapi_ch_calloc(hash_len + 1, sizeof(char));
++ if (dbhash == NULL)
++ goto loser;
++ } else {
++ memset(quick_dbhash, 0, sizeof(quick_dbhash));
++ }
++
++ hashresult = PL_Base64Decode(dbpwd, 0, dbhash);
++ if (NULL == hashresult) {
++ slapi_log_err(SLAPI_LOG_PLUGIN, SALTED_MD5_SUBSYSTEM_NAME,
++ "smd5_pw_cmp: userPassword \"%s\" is the wrong length "
++ "or is not properly encoded BASE64\n",
++ dbpwd);
++ goto loser;
++ }
++
++ salt.bv_val = (void *)(dbhash + MD5_LENGTH); /* salt starts after hash value */
++ salt.bv_len = hash_len - MD5_LENGTH; /* remaining bytes must be salt */
++
++ /* create the hash */
++ memset(userhash, 0, sizeof(userhash));
++ PK11_DigestBegin(ctx);
++ PK11_DigestOp(ctx, (const unsigned char *)userpwd, strlen(userpwd));
++ PK11_DigestOp(ctx, (unsigned char *)(salt.bv_val), salt.bv_len);
++ PK11_DigestFinal(ctx, userhash, &outLen, sizeof userhash);
++ PK11_DestroyContext(ctx, 1);
++
++ /* Compare everything up to the salt. */
++ rc = slapi_ct_memcmp(userhash, dbhash, MD5_LENGTH, MD5_LENGTH);
+
+ loser:
+ if ( dbhash && dbhash != quick_dbhash ) slapi_ch_free_string( (char **)&dbhash );
+diff --git a/ldap/servers/slapd/ch_malloc.c b/ldap/servers/slapd/ch_malloc.c
+index 52ccb64..66cb692 100644
+--- a/ldap/servers/slapd/ch_malloc.c
++++ b/ldap/servers/slapd/ch_malloc.c
+@@ -343,8 +343,8 @@ slapi_ch_smprintf(const char *fmt, ...)
+
+ /* Constant time memcmp. Does not shortcircuit on failure! */
+ /* This relies on p1 and p2 both being size at least n! */
+-int
+-slapi_ct_memcmp( const void *p1, const void *p2, size_t n)
++int32_t
++slapi_ct_memcmp(const void *p1, const void *p2, size_t n1, size_t n2)
+ {
+ int result = 0;
+ const unsigned char *_p1 = (const unsigned char *)p1;
+@@ -354,9 +354,35 @@ slapi_ct_memcmp( const void *p1, const void *p2, size_t n)
+ return 2;
+ }
+
+- for (size_t i = 0; i < n; i++) {
+- if (_p1[i] ^ _p2[i]) {
+- result = 1;
++ if (n1 == n2) {
++ for (size_t i = 0; i < n1; i++) {
++ if (_p1[i] ^ _p2[i]) {
++ result = 1;
++ }
++ }
++ } else {
++ const unsigned char *_pa;
++ const unsigned char *_pb;
++ size_t nl;
++ if (n2 > n1) {
++ _pa = _p2;
++ _pb = _p2;
++ nl = n2;
++ } else {
++ _pa = _p1;
++ _pb = _p1;
++ nl = n1;
++ }
++ /* We already fail as n1 != n2 */
++ result = 3;
++ for (size_t i = 0; i < nl; i++) {
++ if (_pa[i] ^ _pb[i]) {
++ /*
++ * If we don't mutate result here, dead code elimination
++ * we remove for loop.
++ */
++ result = 4;
++ }
+ }
+ }
+ return result;
+diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
+index d37bc63..2c5c4ce 100644
+--- a/ldap/servers/slapd/slapi-plugin.h
++++ b/ldap/servers/slapd/slapi-plugin.h
+@@ -5859,7 +5859,7 @@ char * slapi_ch_smprintf(const char *fmt, ...)
+ * \param n length in bytes of the content of p1 AND p2.
+ * \return 0 on match. 1 on non-match. 2 on presence of NULL pointer in p1 or p2.
+ */
+-int slapi_ct_memcmp( const void *p1, const void *p2, size_t n);
++int32_t slapi_ct_memcmp(const void *p1, const void *p2, size_t n1, size_t n2);
+
+ /*
+ * syntax plugin routines
+--
+1.8.3.1
+