From dc475c320677d767d33603afb1bf4067fd16cfb1 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Tue, 17 Dec 2019 17:37:41 -0500
Subject: [PATCH] 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]
(cherry picked from commit 6b004dd5739bded71be4290c11e7ac3a816c7e09)
ticket: 8861
version_fixed: 1.17.2
(cherry picked from commit 90a13e6d9763c510339c7a40fbc4ecb30041f421)
---
src/lib/kadm5/srv/svr_principal.c | 89 +++++++++----------
.../kdb/ldap/libkdb_ldap/ldap_principal2.c | 13 ---
src/tests/t_kdb.py | 17 ++++
3 files changed, 60 insertions(+), 59 deletions(-)
diff --git a/src/lib/kadm5/srv/svr_principal.c b/src/lib/kadm5/srv/svr_principal.c
index a44d53f03..50db8bbcd 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;
@@ -499,9 +502,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);
@@ -597,6 +597,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 ...
*/
@@ -612,11 +615,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);
@@ -638,8 +645,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;
@@ -678,9 +683,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)
@@ -1358,6 +1360,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)
@@ -1403,6 +1410,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 */
@@ -1435,10 +1443,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
@@ -1477,11 +1481,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;
@@ -1556,6 +1555,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)
@@ -1593,14 +1597,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)
@@ -1618,10 +1618,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);
@@ -1872,6 +1868,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++) {
@@ -1973,14 +1974,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)
diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
index b7c9212cb..8b98a4482 100644
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
@@ -1233,19 +1233,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 319687ff3..38a2fd020 100755
--- a/src/tests/t_kdb.py
+++ b/src/tests/t_kdb.py
@@ -467,6 +467,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.