From b84669f83f6eb14e3e5f12f8ec741957b7ba6a50 Mon Sep 17 00:00:00 2001 From: William Brown Date: Jul 15 2019 23:43:17 +0000 Subject: Ticket 49421 - Implement password hash upgrade on bind. Bug Description: As time goes on, password hash mechanisms change and need to become more resistant to brute force and other attacks. However long lived, and service passwords do not change frequently - and in fact, frequent password changes is a security anti-pattern which is now discouraged. As a result, it's important to be able to improve the cryptographic strength and resitance of our passwords for users as time goes on. Fix Description: We can implement this because during a bind operation we have short amount of access to the plaintext password - we then use that to upgrade the content of the hash. This builds on Emanuel's proof of concept to improve the testing of the feature, as well as to avoid updating clear/crypt due to potential application integrations. https://pagure.io/389-ds-base/issue/49421 Author: Emanuel Rietveld William Brown Review by: mreynolds, mhonek (Thanks!) --- diff --git a/dirsrvtests/tests/suites/password/pwd_upgrade_on_bind.py b/dirsrvtests/tests/suites/password/pwd_upgrade_on_bind.py new file mode 100644 index 0000000..e826e6e --- /dev/null +++ b/dirsrvtests/tests/suites/password/pwd_upgrade_on_bind.py @@ -0,0 +1,140 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2019 William Brown +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- +# +import ldap +import pytest +from lib389.topologies import topology_st +from lib389.idm.user import UserAccounts +from lib389._constants import (DEFAULT_SUFFIX, PASSWORD) + +def test_password_hash_on_upgrade(topology_st): + """If a legacy password hash is present, assert that on a correct bind + the hash is "upgraded" to the latest-and-greatest hash format on the + server. + + Assert also that password FAILURE does not alter the password. + + :id: 42cf99e6-454d-46f5-8f1c-8bb699864a07 + :setup: Single instance + :steps: 1. Set a password hash in SSHA256, and hash to pbkdf2 statically + 2. Test a faulty bind + 3. Assert the PW is SSHA256 + 4. Test a correct bind + 5. Assert the PW is PBKDF2 + :expectedresults: + 1. Successfully set the values + 2. The bind fails + 3. The PW is SSHA256 + 4. The bind succeeds + 5. The PW is PBKDF2 + """ + # Make sure the server is set to pkbdf + topology_st.standalone.config.set('passwordStorageScheme', 'PBKDF2_SHA256') + topology_st.standalone.config.set('nsslapd-allow-hashed-passwords', 'on') + topology_st.standalone.config.set('nsslapd-enable-upgrade-hash', 'on') + + users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX) + user = users.create_test_user() + # Static version of "password" in SSHA256. + user.set('userPassword', "{SSHA256}9eliEQgjfc4Fcj1IXZtc/ne1GRF+OIjz/NfSTX4f7HByGMQrWHLMLA==") + # Attempt to bind with incorrect password. + with pytest.raises(ldap.INVALID_CREDENTIALS): + badconn = user.bind('badpassword') + # Check the pw is SSHA256 + up = user.get_attr_val_utf8('userPassword') + assert up.startswith('{SSHA256}') + + # Bind with correct. + conn = user.bind(PASSWORD) + # Check the pw is now PBKDF2! + up = user.get_attr_val_utf8('userPassword') + assert up.startswith('{PBKDF2_SHA256}') + +def test_password_hash_on_upgrade_clearcrypt(topology_st): + """In some deploymentes, some passwords MAY be in clear or crypt which have + specific possible application integrations allowing the read value to be + processed by other entities. We avoid upgrading these two, to prevent + breaking these integrations. + + :id: 27712492-a4bf-4ea9-977b-b4850ddfb628 + :setup: Single instance + :steps: 1. Set a password hash in CLEAR, and hash to pbkdf2 statically + 2. Test a correct bind + 3. Assert the PW is CLEAR + 4. Set the password to CRYPT + 5. Test a correct bind + 6. Assert the PW is CLEAR + :expectedresults: + 1. Successfully set the values + 2. The bind succeeds + 3. The PW is CLEAR + 4. The set succeeds + 4. The bind succeeds + 5. The PW is CRYPT + """ + # Make sure the server is set to pkbdf + topology_st.standalone.config.set('nsslapd-allow-hashed-passwords', 'on') + topology_st.standalone.config.set('nsslapd-enable-upgrade-hash', 'on') + + users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX) + user = users.create_test_user(1001) + + topology_st.standalone.config.set('passwordStorageScheme', 'CLEAR') + user.set('userPassword', "password") + topology_st.standalone.config.set('passwordStorageScheme', 'PBKDF2_SHA256') + + conn = user.bind(PASSWORD) + up = user.get_attr_val_utf8('userPassword') + assert up.startswith('password') + + user.set('userPassword', "{crypt}I0S3Ry62CSoFg") + conn = user.bind(PASSWORD) + up = user.get_attr_val_utf8('userPassword') + assert up.startswith('{crypt}') + +def test_password_hash_on_upgrade_disable(topology_st): + """If a legacy password hash is present, assert that on a correct bind + the hash is "upgraded" to the latest-and-greatest hash format on the + server. But some people may not like this, so test that we can disable + the feature too! + + :id: ed315145-a3d1-4f17-b04c-73d3638e7ade + :setup: Single instance + :steps: 1. Set a password hash in SSHA256, and hash to pbkdf2 statically + 2. Test a faulty bind + 3. Assert the PW is SSHA256 + 4. Test a correct bind + 5. Assert the PW is SSHA256 + :expectedresults: + 1. Successfully set the values + 2. The bind fails + 3. The PW is SSHA256 + 4. The bind succeeds + 5. The PW is SSHA256 + """ + # Make sure the server is set to pkbdf + topology_st.standalone.config.set('passwordStorageScheme', 'PBKDF2_SHA256') + topology_st.standalone.config.set('nsslapd-allow-hashed-passwords', 'on') + topology_st.standalone.config.set('nsslapd-enable-upgrade-hash', 'off') + + users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX) + user = users.create_test_user(1002) + # Static version of "password" in SSHA256. + user.set('userPassword', "{SSHA256}9eliEQgjfc4Fcj1IXZtc/ne1GRF+OIjz/NfSTX4f7HByGMQrWHLMLA==") + # Attempt to bind with incorrect password. + with pytest.raises(ldap.INVALID_CREDENTIALS): + badconn = user.bind('badpassword') + # Check the pw is SSHA256 + up = user.get_attr_val_utf8('userPassword') + assert up.startswith('{SSHA256}') + + # Bind with correct. + conn = user.bind(PASSWORD) + # Check the pw is NOT upgraded! + up = user.get_attr_val_utf8('userPassword') + assert up.startswith('{SSHA256}') diff --git a/ldap/servers/slapd/bind.c b/ldap/servers/slapd/bind.c index d2a89ba..1532f36 100644 --- a/ldap/servers/slapd/bind.c +++ b/ldap/servers/slapd/bind.c @@ -763,7 +763,13 @@ do_bind(Slapi_PBlock *pb) } } - update_pw_encoding(pb, bind_target_entry, sdn, cred.bv_val); + /* + * If required, update the pw hash to the "current setting" on bind + * if it was successful. + */ + if (config_get_enable_upgrade_hash()) { + update_pw_encoding(pb, bind_target_entry, sdn, cred.bv_val); + } bind_credentials_set(pb_conn, authtype, slapi_ch_strdup(slapi_sdn_get_ndn(sdn)), diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c index 81df130..7131ab0 100644 --- a/ldap/servers/slapd/libglobs.c +++ b/ldap/servers/slapd/libglobs.c @@ -249,6 +249,7 @@ slapi_int_t init_malloc_mmap_threshold; #endif slapi_onoff_t init_extract_pem; slapi_onoff_t init_ignore_vattrs; +slapi_onoff_t init_enable_upgrade_hash; static int isInt(ConfigVarType type) @@ -1232,8 +1233,11 @@ static struct config_get_and_set NULL, 0, (void **)&global_slapdFrontendConfig.tls_check_crl, CONFIG_SPECIAL_TLS_CHECK_CRL, (ConfigGetFunc)config_get_tls_check_crl, - "none" /* Allow reset to this value */} - + "none" /* Allow reset to this value */}, + {CONFIG_ENABLE_UPGRADE_HASH, config_set_enable_upgrade_hash, + NULL, 0, + (void **)&global_slapdFrontendConfig.enable_upgrade_hash, + CONFIG_ON_OFF, (ConfigGetFunc)config_get_enable_upgrade_hash, &init_enable_upgrade_hash} /* End config */ }; @@ -1751,6 +1755,18 @@ FrontendConfig_init(void) #endif init_extract_pem = cfg->extract_pem = LDAP_ON; + /* + * Default upgrade hash to on - this is an important security step, meaning that old + * or legacy hashes are upgraded on bind. It means we are proactive in securing accounts + * that may have infrequent on no password changes (which is current best practice in + * computer security). + * + * A risk is that some accounts may use clear/crypt for other application integrations + * where the hash is "read" from the account. To avoid this, these two hashes are NEVER + * upgraded - in other words, "ON" means only MD5, SHA*, are upgraded to the "current" + * scheme set in cn=config + */ + init_enable_upgrade_hash = cfg->enable_upgrade_hash = LDAP_ON; /* Done, unlock! */ CFG_UNLOCK_WRITE(cfg); @@ -7589,6 +7605,30 @@ config_set_enable_nunc_stans(const char *attrname, char *value, char *errorbuf, return retVal; } +int32_t +config_get_enable_upgrade_hash() +{ + int32_t retVal; + slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig(); + CFG_LOCK_READ(slapdFrontendConfig); + retVal = slapdFrontendConfig->enable_upgrade_hash; + CFG_UNLOCK_READ(slapdFrontendConfig); + + return retVal; +} + +int32_t +config_set_enable_upgrade_hash(const char *attrname, char *value, char *errorbuf, int32_t apply) +{ + int32_t retVal = LDAP_SUCCESS; + slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig(); + + retVal = config_set_onoff(attrname, value, + &(slapdFrontendConfig->enable_upgrade_hash), + errorbuf, apply); + return retVal; +} + static char * config_initvalue_to_onoff(struct config_get_and_set *cgas, char *initvalbuf, size_t initvalbufsize) { diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h index b3167b8..2fa4136 100644 --- a/ldap/servers/slapd/proto-slap.h +++ b/ldap/servers/slapd/proto-slap.h @@ -588,6 +588,9 @@ int config_get_malloc_mmap_threshold(void); int config_get_maxsimplepaged_per_conn(void); int config_get_extract_pem(void); +int32_t config_get_enable_upgrade_hash(void); +int32_t config_set_enable_upgrade_hash(const char *attrname, char *value, char *errorbuf, int apply); + int is_abspath(const char *); char *rel2abspath(char *); char *rel2abspath_ext(char *, char *); diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index a618b2a..2d2c985 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -661,6 +661,13 @@ update_pw_info(Slapi_PBlock *pb, char *old_pw) "Param error - no password entry/target dn/operation\n"); return -1; } + + /* If we have been requested to skip updating this data, check now */ + if (slapi_operation_is_flag_set(operation, OP_FLAG_ACTION_SKIP_PWDPOLICY)) { + /* No action required! */ + return 0; + } + internal_op = slapi_operation_is_flag_set(operation, SLAPI_OP_FLAG_INTERNAL); target_dn = slapi_sdn_get_ndn(sdn); pwpolicy = new_passwdPolicy(pb, target_dn); @@ -3255,7 +3262,7 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) * success ( 0 ) * operationsError ( -1 ), */ -int update_pw_encoding(Slapi_PBlock *orig_pb, Slapi_Entry *e, Slapi_DN *sdn, char *cleartextpassword) { +int32_t update_pw_encoding(Slapi_PBlock *orig_pb, Slapi_Entry *e, Slapi_DN *sdn, char *cleartextpassword) { char *dn = (char *)slapi_sdn_get_ndn(sdn); Slapi_Attr *pw = NULL; Slapi_Value **password_values = NULL; @@ -3264,10 +3271,13 @@ int update_pw_encoding(Slapi_PBlock *orig_pb, Slapi_Entry *e, Slapi_DN *sdn, cha Slapi_Mods smods; char *hashed_val = NULL; Slapi_PBlock *pb = NULL; - int res = 0; + int32_t res = 0; slapi_mods_init(&smods, 0); + /* + * Does the entry have a pw? + */ if (e == NULL || slapi_entry_attr_find(e, SLAPI_USERPWD_ATTR, &pw) != 0 || pw == NULL) { slapi_log_err(SLAPI_LOG_WARNING, "update_pw_encoding", "Could not read password attribute on '%s'\n", @@ -3292,6 +3302,9 @@ int update_pw_encoding(Slapi_PBlock *orig_pb, Slapi_Entry *e, Slapi_DN *sdn, cha goto free_and_return; } + /* + * Get the current system pw policy. + */ pwpolicy = new_passwdPolicy(orig_pb, dn); if (pwpolicy == NULL || pwpolicy->pw_storagescheme == NULL) { slapi_log_err(SLAPI_LOG_WARNING, @@ -3301,11 +3314,26 @@ int update_pw_encoding(Slapi_PBlock *orig_pb, Slapi_Entry *e, Slapi_DN *sdn, cha goto free_and_return; } + /* + * If the scheme is the same as current, do nothing! + */ curpwsp = pw_val2scheme((char *)slapi_value_get_string(password_values[0]), NULL, 1); if (curpwsp != NULL && strcmp(curpwsp->pws_name, pwpolicy->pw_storagescheme->pws_name) == 0) { res = 0; // Nothing to do goto free_and_return; } + /* + * If the scheme is clear or crypt, we also do nothing to prevent breaking some application + * integrations. See pwdstorage.h + */ + if (strcmp(curpwsp->pws_name, "CLEAR") == 0 || strcmp(curpwsp->pws_name, "CRYPT") == 0) { + res = 0; // Nothing to do + goto free_and_return; + } + + /* + * We are commited to upgrading the hash content now! + */ hashed_val = slapi_encode_ext(NULL, NULL, cleartextpassword, pwpolicy->pw_storagescheme->pws_name); if (hashed_val == NULL) { @@ -3329,7 +3357,7 @@ int update_pw_encoding(Slapi_PBlock *orig_pb, Slapi_Entry *e, Slapi_DN *sdn, cha NULL, /* UniqueID */ pw_get_componentID(), /* PluginID */ OP_FLAG_SKIP_MODIFIED_ATTRS & - OP_FLAG_REPLICATED); /* Flags */ + OP_FLAG_ACTION_SKIP_PWDPOLICY); /* Flags */ slapi_modify_internal_pb(pb); slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, &res); diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index e691ea9..7f3a056 100644 --- a/ldap/servers/slapd/slap.h +++ b/ldap/servers/slapd/slap.h @@ -2198,6 +2198,7 @@ typedef struct _slapdEntryPoints #define CONFIG_MODDN_ACI_ATTRIBUTE "nsslapd-moddn-aci" #define CONFIG_GLOBAL_BACKEND_LOCK "nsslapd-global-backend-lock" #define CONFIG_ENABLE_NUNC_STANS "nsslapd-enable-nunc-stans" +#define CONFIG_ENABLE_UPGRADE_HASH "nsslapd-enable-upgrade-hash" #define CONFIG_CONFIG_ATTRIBUTE "nsslapd-config" #define CONFIG_INSTDIR_ATTRIBUTE "nsslapd-instancedir" #define CONFIG_SCHEMADIR_ATTRIBUTE "nsslapd-schemadir" @@ -2531,6 +2532,7 @@ typedef struct _slapdFrontendConfig int malloc_mmap_threshold; /* mallopt M_MMAP_THRESHOLD */ #endif slapi_onoff_t extract_pem; /* If "on", export key/cert as pem files */ + slapi_onoff_t enable_upgrade_hash; /* If on, upgrade hashes for PW at bind */ } slapdFrontendConfig_t; /* possible values for slapdFrontendConfig_t.schemareplace */ diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index d26397d..c54bb98 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -5385,8 +5385,6 @@ int slapi_value_find(void *plugin, struct berval **vals, struct berval *v); */ #define SLAPI_USERPWD_ATTR "userpassword" int slapi_pw_find_sv(Slapi_Value **vals, const Slapi_Value *v); -int update_pw_encoding(Slapi_PBlock *orig_pb, Slapi_Entry *e, Slapi_DN *sdn, char *cleartextpassword); - /* value encoding encoding */ /* checks if the value is encoded with any known algorithm*/ int slapi_is_encoded(char *value); diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h index 4161002..66493dc 100644 --- a/ldap/servers/slapd/slapi-private.h +++ b/ldap/servers/slapd/slapi-private.h @@ -426,6 +426,9 @@ char *slapi_filter_to_string_internal(const struct slapi_filter *f, char *buf, s #define OP_FLAG_BULK_IMPORT 0x800000 /* operation is bulk import */ #define OP_FLAG_NOOP 0x01000000 /* operation results from urp and * should be ignored */ +#define OP_FLAG_ACTION_SKIP_PWDPOLICY 0x02000000 /* Skip applying pw policy rules to the password + * change operation, as it's from an upgrade on + * bind rather than a normal password change */ /* reverse search states */ #define REV_STARTED 1 @@ -803,6 +806,9 @@ void task_cleanup(void); int pw_rever_encode(Slapi_Value **vals, char *attr_name); int pw_rever_decode(char *cipher, char **plain, const char *attr_name); +int32_t update_pw_encoding(Slapi_PBlock *orig_pb, Slapi_Entry *e, Slapi_DN *sdn, char *cleartextpassword); + + /* config routines */ int slapi_config_get_readonly(void);