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.

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

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);
+}

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);

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 },

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=$!

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; }