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.