From: Robbie Harwood Date: Tue, 17 Dec 2019 22:37:41 +0000 (-0500) Subject: Fix LDAP policy enforcement of pw_expiration X-Git-Tag: krb5-1.18-beta1~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F1016%2Fhead;p=thirdparty%2Fkrb5.git Fix LDAP policy enforcement of pw_expiration In the LDAP backend, the change mask is used to determine what LDAP attributes to update. As a result, password expiration was not set from policy when running during addprinc, among other issues. However, when the mask did not contain KADM5_PRINCIPAL, pw_expiration would be applied regardless, which meant that (for instance) changing the password would cause the password application to be applied. Remove the check for KADM5_PRINCIPAL, and fix the mask to contain KADM5_PW_EXPIRATION where appropriate. Add a regression test to t_kdb.py. [ghudson@mit.edu: also set KADM5_ATTRIBUTES for randkey and setkey since they both unset KRB5_KDB_REQUIRES_PWCHANGE; edited comments and commit message] ticket: 8861 (new) tags: pullup target_version: 1.17-next --- diff --git a/src/lib/kadm5/srv/svr_principal.c b/src/lib/kadm5/srv/svr_principal.c index 6ee262e899..53ecbe1bc0 100644 --- a/src/lib/kadm5/srv/svr_principal.c +++ b/src/lib/kadm5/srv/svr_principal.c @@ -356,6 +356,11 @@ kadm5_create_principal_3(void *server_handle, kdb = calloc(1, sizeof(*kdb)); if (kdb == NULL) return ENOMEM; + + /* In all cases the principal entry is new and key data is set; let the + * database provider know. */ + kdb->mask = mask | KADM5_KEY_DATA | KADM5_PRINCIPAL; + memset(&adb, 0, sizeof(osa_princ_ent_rec)); /* @@ -405,14 +410,12 @@ kadm5_create_principal_3(void *server_handle, kdb->expiration = handle->params.expiration; kdb->pw_expiration = 0; - if (have_polent) { - if(polent.pw_max_life) - kdb->pw_expiration = ts_incr(now, polent.pw_max_life); - else - kdb->pw_expiration = 0; - } - if ((mask & KADM5_PW_EXPIRATION)) + if (mask & KADM5_PW_EXPIRATION) { kdb->pw_expiration = entry->pw_expiration; + } else if (have_polent && polent.pw_max_life) { + kdb->mask |= KADM5_PW_EXPIRATION; + kdb->pw_expiration = ts_incr(now, polent.pw_max_life); + } kdb->last_success = 0; kdb->last_failed = 0; @@ -503,9 +506,6 @@ kadm5_create_principal_3(void *server_handle, adb.policy = entry->policy; } - /* In all cases key and the principal data is set, let the database provider know */ - kdb->mask = mask | KADM5_KEY_DATA | KADM5_PRINCIPAL ; - /* store the new db entry */ ret = kdb_put_entry(handle, kdb, &adb); @@ -601,6 +601,9 @@ kadm5_modify_principal(void *server_handle, if (ret) return(ret); + /* Let the mask propagate to the database provider. */ + kdb->mask = mask; + /* * This is pretty much the same as create ... */ @@ -616,11 +619,15 @@ kadm5_modify_principal(void *server_handle, free(adb.policy); adb.policy = strdup(entry->policy); } - if (have_pol) { + + if (mask & KADM5_PW_EXPIRATION) { + kdb->pw_expiration = entry->pw_expiration; + } else if (have_pol) { /* set pw_max_life based on new policy */ + kdb->mask |= KADM5_PW_EXPIRATION; if (pol.pw_max_life) { ret = krb5_dbe_lookup_last_pwd_change(handle->context, kdb, - &(kdb->pw_expiration)); + &kdb->pw_expiration); if (ret) goto done; kdb->pw_expiration = ts_incr(kdb->pw_expiration, pol.pw_max_life); @@ -642,8 +649,6 @@ kadm5_modify_principal(void *server_handle, kdb->max_life = entry->max_life; if ((mask & KADM5_PRINC_EXPIRE_TIME)) kdb->expiration = entry->princ_expire_time; - if (mask & KADM5_PW_EXPIRATION) - kdb->pw_expiration = entry->pw_expiration; if (mask & KADM5_MAX_RLIFE) kdb->max_renewable_life = entry->max_renewable_life; @@ -682,9 +687,6 @@ kadm5_modify_principal(void *server_handle, kdb->fail_auth_count = 0; } - /* let the mask propagate to the database provider */ - kdb->mask = mask; - ret = k5_kadm5_hook_modify(handle->context, handle->hook_handles, KADM5_HOOK_STAGE_PRECOMMIT, entry, mask); if (ret) @@ -1362,6 +1364,11 @@ kadm5_chpass_principal_3(void *server_handle, if ((ret = kdb_get_entry(handle, principal, &kdb, &adb))) return(ret); + /* We will always be changing the key data, attributes, auth failure count, + * and password expiration time. */ + kdb->mask = KADM5_KEY_DATA | KADM5_ATTRIBUTES | KADM5_FAIL_AUTH_COUNT | + KADM5_PW_EXPIRATION; + ret = apply_keysalt_policy(handle, adb.policy, n_ks_tuple, ks_tuple, &new_n_ks_tuple, &new_ks_tuple); if (ret) @@ -1407,6 +1414,7 @@ kadm5_chpass_principal_3(void *server_handle, if (ret) goto done; + kdb->pw_expiration = 0; if ((adb.aux_attributes & KADM5_POLICY)) { /* the policy was loaded before */ @@ -1439,10 +1447,6 @@ kadm5_chpass_principal_3(void *server_handle, if (pol.pw_max_life) kdb->pw_expiration = ts_incr(now, pol.pw_max_life); - else - kdb->pw_expiration = 0; - } else { - kdb->pw_expiration = 0; } #ifdef USE_PASSWORD_SERVER @@ -1481,11 +1485,6 @@ kadm5_chpass_principal_3(void *server_handle, /* unlock principal on this KDC */ kdb->fail_auth_count = 0; - /* key data and attributes changed, let the database provider know */ - kdb->mask = KADM5_KEY_DATA | KADM5_ATTRIBUTES | - KADM5_FAIL_AUTH_COUNT; - /* | KADM5_CPW_FUNCTION */ - if (hist_added) kdb->mask |= KADM5_KEY_HIST; @@ -1560,6 +1559,11 @@ kadm5_randkey_principal_3(void *server_handle, if ((ret = kdb_get_entry(handle, principal, &kdb, &adb))) return(ret); + /* We will always be changing the key data, attributes, auth failure count, + * and password expiration time. */ + kdb->mask = KADM5_KEY_DATA | KADM5_ATTRIBUTES | KADM5_FAIL_AUTH_COUNT | + KADM5_PW_EXPIRATION; + ret = apply_keysalt_policy(handle, adb.policy, n_ks_tuple, ks_tuple, &new_n_ks_tuple, &new_ks_tuple); if (ret) @@ -1599,14 +1603,10 @@ kadm5_randkey_principal_3(void *server_handle, if (ret) goto done; } - if (have_pol) { - if (pol.pw_max_life) - kdb->pw_expiration = ts_incr(now, pol.pw_max_life); - else - kdb->pw_expiration = 0; - } else { - kdb->pw_expiration = 0; - } + + kdb->pw_expiration = 0; + if (have_pol && pol.pw_max_life) + kdb->pw_expiration = ts_incr(now, pol.pw_max_life); ret = krb5_dbe_update_last_pwd_change(handle->context, kdb, now); if (ret) @@ -1624,10 +1624,6 @@ kadm5_randkey_principal_3(void *server_handle, goto done; } - /* key data changed, let the database provider know */ - kdb->mask = KADM5_KEY_DATA | KADM5_FAIL_AUTH_COUNT; - /* | KADM5_RANDKEY_USED */; - ret = k5_kadm5_hook_chpass(handle->context, handle->hook_handles, KADM5_HOOK_STAGE_PRECOMMIT, principal, keepold, new_n_ks_tuple, new_ks_tuple, NULL); @@ -1763,6 +1759,11 @@ kadm5_setkey_principal_4(void *server_handle, krb5_principal principal, if (ret) return ret; + /* We will always be changing the key data, attributes, auth failure count, + * and password expiration time. */ + kdb->mask = KADM5_KEY_DATA | KADM5_ATTRIBUTES | KADM5_FAIL_AUTH_COUNT | + KADM5_PW_EXPIRATION; + if (kvno == 0) { /* Pick the next kvno. */ for (i = 0; i < kdb->n_key_data; i++) { @@ -1864,14 +1865,10 @@ kadm5_setkey_principal_4(void *server_handle, krb5_principal principal, if (ret) goto done; } - if (have_pol) { - if (pol.pw_max_life) - kdb->pw_expiration = ts_incr(now, pol.pw_max_life); - else - kdb->pw_expiration = 0; - } else { - kdb->pw_expiration = 0; - } + + kdb->pw_expiration = 0; + if (have_pol && pol.pw_max_life) + kdb->pw_expiration = ts_incr(now, pol.pw_max_life); ret = krb5_dbe_update_last_pwd_change(handle->context, kdb, now); if (ret) @@ -1880,9 +1877,6 @@ kadm5_setkey_principal_4(void *server_handle, krb5_principal principal, /* Unlock principal on this KDC. */ kdb->fail_auth_count = 0; - /* key data changed, let the database provider know */ - kdb->mask = KADM5_KEY_DATA | KADM5_FAIL_AUTH_COUNT; - ret = kdb_put_entry(handle, kdb, &adb); if (ret) goto done; diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c index 564093fbde..1d0726707f 100644 --- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c +++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c @@ -1230,19 +1230,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, goto cleanup; } - if (!(entry->mask & KADM5_PRINCIPAL)) { - memset(strval, 0, sizeof(strval)); - if ((strval[0]=getstringtime(entry->pw_expiration)) == NULL) - goto cleanup; - if ((st=krb5_add_str_mem_ldap_mod(&mods, - "krbpasswordexpiration", - LDAP_MOD_REPLACE, strval)) != 0) { - free (strval[0]); - goto cleanup; - } - free (strval[0]); - } - /* Update last password change whenever a new key is set */ { krb5_timestamp last_pw_changed; diff --git a/src/tests/t_kdb.py b/src/tests/t_kdb.py index 9fff20404c..03ee70f47c 100755 --- a/src/tests/t_kdb.py +++ b/src/tests/t_kdb.py @@ -495,6 +495,23 @@ else: realm.run([kadminl, 'modprinc', '-pwexpire', '2040-02-03', 'user']) realm.run([kadminl, 'getprinc', 'user'], expected_msg=' 2040\n') +# Regression test for #8861 (pw_expiration policy enforcement). +mark('pw_expiration propogation') +# Create a policy with a max life and verify its application. +realm.run([kadminl, 'addpol', '-maxlife', '1s', 'pw_e']) +realm.run([kadminl, 'addprinc', '-policy', 'pw_e', '-pw', 'password', + 'pwuser']) +out = realm.run([kadminl, 'getprinc', 'pwuser'], + expected_msg='Password expiration date: ') +if 'Password expiration date: [never]' in out: + fail('pw_expiration not applied at principal creation') +# Unset the policy max life and verify its application during password +# change. +realm.run([kadminl, 'modpol', '-maxlife', '0', 'pw_e']) +realm.run([kadminl, 'cpw', '-pw', 'password_', 'pwuser']) +realm.run([kadminl, 'getprinc', 'pwuser'], + expected_msg='Password expiration date: [never]') + realm.stop() # Briefly test dump and load.