[SSSD] [PATCH] print password complexity hint when change password failed

Jakub Hrozek jhrozek at redhat.com
Sun Aug 11 20:49:42 UTC 2013


On Sun, Aug 11, 2013 at 10:47:13PM +0200, Jakub Hrozek wrote:
> On Wed, Jul 17, 2013 at 01:18:27PM +0200, Jakub Hrozek wrote:
> > On Tue, Jul 16, 2013 at 08:03:04PM +0200, Jakub Hrozek wrote:
> > > On Tue, Jul 16, 2013 at 10:25:34AM +0200, Pavel Březina wrote:
> > > > On 07/10/2013 04:32 PM, Jakub Hrozek wrote:
> > > > >On Fri, Jun 21, 2013 at 02:49:51PM +0200, Pavel Březina wrote:
> > > > >>Now the output looks like:
> > > > >>
> > > > >>$ su test-user
> > > > >>Password:
> > > > >>Password expired. Change your password now.
> > > > >>Current Password:
> > > > >>New password:
> > > > >>Retype new password:
> > > > >>Password change failed. Please make sure the password meets the complexity
> > > > >>constraints.
> > > > >>su: incorrect password
> > > > >
> > > > >This works but I think the change is too broad, the hint would now be
> > > > >printed for any password change failure without a specific message and
> > > > >that might be misleading.
> > > > >
> > > > >The reason why msg is zero-sized is that the user_error_message variable
> > > > >in changepw_child() is overwritten with result_string which is "". So
> > > > >one change might be to not overwrite user_error_message with empty
> > > > >string.
> > > > 
> > > > For some reason Kerberos returns empty string but result_string.length = 30.
> > > > 
> > > > >The other change would be in changepw_child() -- we could
> > > > >special-case when krb5_change_password() returns result_code 4
> > > > >(Password change rejected) and then instead of sending
> > > > >SSS_PAM_USER_INFO_CHPASS_ERROR send a new code
> > > > >(SSS_PAM_USER_INFO_CHPASS_REJECTED?) that would instruct the user to
> > > > >check password complexity settings.
> > > > 
> > > > How about this? I avoided new error code and chose to fill server message in
> > > > provider. The output now looks like:
> > > > 
> > > > Password change failed. Server message: Please make sure the password meets
> > > > the complexity constraints.
> > > 
> > > This works for me, I tested a couple of usual scenarios to make sure we
> > > don't print this message too frequently, but seems like everything is
> > > working fine.
> > > 
> > > Ack
> > 
> > Pushed to master and sssd-1-10
> 
> FYI, attached is a backport of the patch on the 1.9 branch. I think the
> backport was trivial enough so that I pushed it, but retroactive review
> would be welcome, too.
> 
> Also, the patch was already verified by a customer.

Let's try it again, now with the patch attached.
-------------- next part --------------
>From 658e2757fa9559f5dff6d475ae50496429e3d89f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
Date: Mon, 15 Jul 2013 13:44:31 +0200
Subject: [PATCH] print hint about password complexity when new password is
 rejected

https://fedorahosted.org/sssd/ticket/1827
---
 src/providers/krb5/krb5_child.c |  8 +++++++-
 src/providers/ldap/sdap_async.c | 16 +++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 73575b88b5b6d36fa8c755463a3c32b0c3e9deb0..e4dc49327db1fadac52fa77ffad07c8cc2d1bba8 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1185,7 +1185,7 @@ static errno_t changepw_child(int fd, struct krb5_req *kr)
             }
         }
 
-        if (result_string.length > 0) {
+        if (result_string.length > 0 && result_string.data[0] != '\0') {
             DEBUG(1, ("krb5_change_password failed [%d][%.*s].\n", result_code,
                       result_string.length, result_string.data));
             talloc_free(user_error_message);
@@ -1194,6 +1194,12 @@ static errno_t changepw_child(int fd, struct krb5_req *kr)
             if (user_error_message == NULL) {
                 DEBUG(1, ("talloc_strndup failed.\n"));
             }
+        } else if (result_code == KRB5_KPASSWD_SOFTERROR) {
+            user_error_message = talloc_strdup(kr->pd, "Please make sure the "
+                                 "password meets the complexity constraints.");
+            if (user_error_message == NULL) {
+                DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strndup failed.\n"));
+            }
         }
 
         if (user_error_message != NULL) {
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index 73b2b64af012a1ea69717628e86cf6f784c4ffad..32763d2e6b92bc34393ce895e95f5cab3f32e55d 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -641,7 +641,21 @@ static void sdap_exop_modify_passwd_done(struct sdap_op *op,
     DEBUG(3, ("ldap_extended_operation result: %s(%d), %s\n",
             sss_ldap_err2string(state->result), state->result, errmsg));
 
-    if (state->result != LDAP_SUCCESS) {
+    switch (state->result) {
+    case LDAP_SUCCESS:
+        ret = EOK;
+        break;
+    case LDAP_CONSTRAINT_VIOLATION:
+        state->user_error_message = talloc_strdup(state,
+            "Please make sure the password meets the complexity constraints.");
+        if (state->user_error_message == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup failed\n"));
+            ret = ENOMEM;
+            goto done;
+        }
+        ret = EIO;
+        goto done;
+    default:
         if (errmsg) {
             state->user_error_message = talloc_strdup(state, errmsg);
             if (state->user_error_message == NULL) {
-- 
1.8.3.1



More information about the sssd-devel mailing list