kadmin: allow enforcing password quality on admin password change

This patch adds the "enforce_on_admin_set" configuration knob in the
[password_quality] section. When this is enabled, administrative password
changes via the kadmin or kpasswd protocols will be subject to password quality
checks. (An administrative password change is one where the authenticating
principal is different to the principal whose password is being changed.)

Note that kadmin running in local mode (-l) is unaffected by this patch.

diff --git a/doc/setup.texi b/doc/setup.texi
index d354049bcfc4701918444b756d5e349a9c0be69a..f0e2987841467c8e3254cf3209fdb981e98bec00 100644 (file)
--- a/doc/setup.texi
+++ b/doc/setup.texi
@@ -470,6 +470,15 @@ classes. Default value if not given is 3.
 The four different characters classes are, uppercase, lowercase,
 number, special characters.
 
+@item enforce_on_admin_set
+
+The enforce_on_admin_set check validates that administrative password changes
+via kpasswdd or kadmind are also subject to the password policy. Note that
+@command{kadmin} in local mode can still bypass these. An administrative
+password change is one where the identity of the authenticating principal
+differs from the subject of the password change. Default value if not given is
+true.
+
 @end itemize
 
 If you want to write your own shared object to check password

diff --git a/kadmin/server.c b/kadmin/server.c
index ccb6a7a991dbad1de8fefcc4feda3d68bc6c02e3..03605e504c502a1cadf241225b1833c8766d5c92 100644 (file)
--- a/kadmin/server.c
+++ b/kadmin/server.c
@@ -38,6 +38,9 @@ static kadm5_ret_t check_aliases(kadm5_server_context *,
                                  kadm5_principal_ent_rec *,
                                  kadm5_principal_ent_rec *);
 
+static krb5_boolean
+enforce_pwqual_on_admin_set_p(kadm5_server_context *contextp);
+
 static kadm5_ret_t
 kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
                 krb5_data *in, krb5_data *out)
@@ -178,6 +181,24 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
        }
        krb5_unparse_name_fixed(contextp->context, ent.principal,
                                name, sizeof(name));
+       if (enforce_pwqual_on_admin_set_p(contextp)) {
+           krb5_data pwd_data;
+           const char *pwd_reason;
+
+           pwd_data.data = password;
+           pwd_data.length = strlen(password);
+
+           pwd_reason = kadm5_check_password_quality (contextp->context,
+                                                      ent.principal, &pwd_data);
+           if (pwd_reason != NULL)
+               ret = KADM5_PASS_Q_DICT;
+           else
+               ret = 0;
+           if (ret) {
+               kadm5_free_principal_ent(kadm_handlep, &ent);
+               goto fail;
+           }
+       }
        krb5_warnx(contextp->context, "%s: %s %s", client, op, name);
        ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_ADD,
                                          ent.principal);
@@ -296,6 +317,8 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
        break;
     }
     case kadm_chpass:{
+       krb5_boolean is_self_cpw, allow_self_cpw;
+
        op = "CHPASS";
        ret = krb5_ret_principal(sp, &princ);
        if (ret)
@@ -314,21 +337,29 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
        krb5_warnx(contextp->context, "%s: %s %s", client, op, name);
 
        /*
-        * The change is allowed if at least one of:
-        *
-        * a) allowed by sysadmin
-        * b) it's for the principal him/herself and this was an
-        *    initial ticket, but then, check with the password quality
-        *    function.
-        * c) the user is on the CPW ACL.
+        * Change password requests are subject to ACLs unless the principal is
+        * changing their own password and the initial ticket flag is set, and
+        * the allow_self_change_password configuration option is TRUE.
         */
+       is_self_cpw =
+           krb5_principal_compare(contextp->context, contextp->caller, princ);
+       allow_self_cpw =
+           krb5_config_get_bool_default(contextp->context, NULL, TRUE,
+                                        "kadmin", "allow_self_change_password", NULL);
+       if (!(is_self_cpw && initial && allow_self_cpw)) {
+           ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_CPW, princ);
+           if (ret) {
+               krb5_free_principal(contextp->context, princ);
+               goto fail;
+           }
+       }
 
-       if (krb5_config_get_bool_default(contextp->context, NULL, TRUE,
-                                        "kadmin", "allow_self_change_password", NULL)
-           && initial
-           && krb5_principal_compare (contextp->context, contextp->caller,
-                                      princ))
-       {
+       /*
+        * Change password requests are subject to password quality checks if
+        * the principal is changing their own password, or the enforce_on_admin_set
+        * configuration option is TRUE (the default).
+        */
+       if (is_self_cpw || enforce_pwqual_on_admin_set_p(contextp)) {
            krb5_data pwd_data;
            const char *pwd_reason;
 
@@ -341,13 +372,12 @@ kadmind_dispatch(void *kadm_handlep, krb5_boolean initial,
                ret = KADM5_PASS_Q_DICT;
            else
                ret = 0;
-       } else
-           ret = _kadm5_acl_check_permission(contextp, KADM5_PRIV_CPW, princ);
-
-       if(ret) {
-           krb5_free_principal(contextp->context, princ);
-           goto fail;
+           if (ret) {
+               krb5_free_principal(contextp->context, princ);
+               goto fail;
+           }
        }
+
        ret = kadm5_chpass_principal_3(kadm_handlep, princ, keepold, 0, NULL,
                                       password);
        krb5_free_principal(contextp->context, princ);
@@ -843,3 +873,11 @@ kadmind_loop(krb5_context contextp,
 
     return 0;
 }
+
+static krb5_boolean
+enforce_pwqual_on_admin_set_p(kadm5_server_context *contextp)
+{
+    return krb5_config_get_bool_default(contextp->context, NULL, TRUE,
+                                       "password_quality",
+                                       "enforce_on_admin_set", NULL);
+}

diff --git a/kpasswd/kpasswdd.c b/kpasswd/kpasswdd.c
index 967bf9383b346d05ffff2581ac29225da384d21e..cc115ce188daf65068201517ff1ce5570d1651f5 100644 (file)
--- a/kpasswd/kpasswdd.c
+++ b/kpasswd/kpasswdd.c
@@ -253,6 +253,7 @@ change (krb5_auth_context auth_context,
     krb5_data *pwd_data = NULL;
     char *tmp;
     ChangePasswdDataMS chpw;
+    krb5_boolean enforce_pwqual_on_admin_set;
 
     memset (&conf, 0, sizeof(conf));
     memset(&chpw, 0, sizeof(chpw));
@@ -366,12 +367,31 @@ change (krb5_auth_context auth_context,
        goto out;
     }
 
+    if (krb5_principal_compare(context, admin_principal, principal) == FALSE) {
+       ret = _kadm5_acl_check_permission(kadm5_handle, KADM5_PRIV_CPW,
+                                         principal);
+       if (ret) {
+           krb5_warn (context, ret,
+                      "Check ACL failed for %s for changing %s password",
+                      admin, client);
+           reply_priv (auth_context, s, sa, sa_size,
+                       KRB5_KPASSWD_HARDERROR, "permission denied");
+           goto out;
+       }
+       krb5_warnx (context, "%s is changing password for %s", admin, client);
+    }
+
     /*
-     * Check password quality if not changing as administrator
+     * Change password requests are subject to password quality checks if
+     * the principal is changing their own password, or the enforce_on_admin_set
+     * configuration option is TRUE (the default).
      */
-
-    if (krb5_principal_compare(context, admin_principal, principal) == TRUE) {
-
+    enforce_pwqual_on_admin_set =
+       krb5_config_get_bool_default(context, NULL, TRUE,
+                                    "password_quality",
+                                    "enforce_on_admin_set", NULL);
+    if (krb5_principal_compare(context, admin_principal, principal) == TRUE ||
+       enforce_pwqual_on_admin_set == TRUE) {
        pwd_reason = kadm5_check_password_quality (context, principal,
                                                   pwd_data);
        if (pwd_reason != NULL ) {
@@ -383,18 +403,6 @@ change (krb5_auth_context auth_context,
            goto out;
        }
        krb5_warnx (context, "Changing password for %s", client);
-    } else {
-       ret = _kadm5_acl_check_permission(kadm5_handle, KADM5_PRIV_CPW,
-                                         principal);
-       if (ret) {
-           krb5_warn (context, ret,
-                      "Check ACL failed for %s for changing %s password",
-                      admin, client);
-           reply_priv (auth_context, s, sa, sa_size,
-                       KRB5_KPASSWD_HARDERROR, "permission denied");
-           goto out;
-       }
-       krb5_warnx (context, "%s is changing password for %s", admin, client);
     }
 
     ret = krb5_data_realloc(pwd_data, pwd_data->length + 1);

diff --git a/lib/krb5/verify_krb5_conf.c b/lib/krb5/verify_krb5_conf.c
index 7528f3f7bdbd31130d3b30af0e386eefac296e65..232f2db18a8ac631df806ba98365ccc13699fe2d 100644 (file)
--- a/lib/krb5/verify_krb5_conf.c
+++ b/lib/krb5/verify_krb5_conf.c
@@ -633,6 +633,7 @@ struct entry kcm_entries[] = {
 };
 
 struct entry password_quality_entries[] = {
+    { "enforce_on_admin_set", krb5_config_string, check_boolean, 0 },
     { "check_function", krb5_config_string, NULL, 0 },
     { "check_library", krb5_config_string, NULL, 0 },
     { "external_program", krb5_config_string, NULL, 0 },

diff --git a/tests/kdc/check-kadmin.in b/tests/kdc/check-kadmin.in
index 5272319009919c24e838d188b75b41086de01e88..106ef70c9448693a27b59c2326446c0a77e193dd 100644 (file)
--- a/tests/kdc/check-kadmin.in
+++ b/tests/kdc/check-kadmin.in
@@ -59,7 +59,7 @@ kinit="${kinit} -c $cache ${afs_no_afslog}"
 kgetcred="${kgetcred} -c $cache"
 kdestroy="${kdestroy} -c $cache ${afs_no_unlog}"
 
-foopassword="foo"
+foopassword="fooLongPasswordYo123;"
 
 KRB5_CONFIG="${objdir}/krb5.conf"
 export KRB5_CONFIG
@@ -256,6 +256,11 @@ env KRB5CCNAME=${cache} \
 ${kadmin} -p foo/admin@${R} add -p "$foopassword" --use-defaults kaka@${R} || 
        { echo "kadmin failed $?"; cat messages.log ; exit 1; }
 
+echo "kadmin"
+env KRB5CCNAME=${cache} \
+${kadmin} -p foo/admin@${R} add -p abc --use-defaults kaka@${R} &&
+       { echo "kadmin succeeded $?"; cat messages.log ; exit 1; }
+
 #----------------------------------
 ${kadmind} -d &
 kadmpid=$!

diff --git a/tests/ldap/check-ldap.in b/tests/ldap/check-ldap.in
index 9d6d7f305b4af79286b1eda4b790752f37f5c511..b99c951032bbd8c282376503bac21bc96988bd2c 100644 (file)
--- a/tests/ldap/check-ldap.in
+++ b/tests/ldap/check-ldap.in
@@ -54,6 +54,8 @@ kgetcred="${TESTS_ENVIRONMENT} ../../kuser/kgetcred -c $cache"
 kadmin="${TESTS_ENVIRONMENT} ../../kadmin/kadmin -l -r $R"
 kdc="${TESTS_ENVIRONMENT} ../../kdc/kdc --addresses=localhost -P $port"
 
+foopassword="fooLongPasswordYo123;"
+
 testfailed="echo test failed; exit 1"
 
 KRB5_CONFIG="${objdir}/krb5.conf"
@@ -102,8 +104,8 @@ ${kadmin} \
     --realm-max-renewable-life=1month \
     ${R} || exit 1
 
-${kadmin} add -p foo --use-defaults foo@${R} || exit 1
-${kadmin} add -p foo --use-defaults bar@${R} || exit 1
+${kadmin} add -p "$foopassword" --use-defaults foo@${R} || exit 1
+${kadmin} add -p "$foopassword" --use-defaults bar@${R} || exit 1
 ${kadmin} add -p kaka --use-defaults ${server}@${R} || exit 1
 
 ${kadmin} cpw --random-password bar@${R} > /dev/null || exit 1
@@ -111,11 +113,11 @@ ${kadmin} cpw --random-password bar@${R} > /dev/null || exit 1
 ${kadmin} cpw --random-password bar@${R} > /dev/null || exit 1
 
 ${kadmin} cpw --random-password suser@${R} > /dev/null|| exit 1
-${kadmin} cpw --password=foo suser@${R} || exit 1
+${kadmin} cpw --password="$foopassword" suser@${R} || exit 1
 
 ${kadmin} list '*' > /dev/null || exit 1
 
-echo foo > ${objdir}/foopassword
+echo "$foopassword" > ${objdir}/foopassword
 
 echo Starting kdc
 ${kdc} --detach --testing || { echo "kdc failed to start"; exit 1; }