[SSSD] [PATCH] Add retry option to pam_sss

Sumit Bose sbose at redhat.com
Fri May 7 19:39:52 UTC 2010


On Thu, May 06, 2010 at 03:05:09PM +0200, Jakub Hrozek wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 05/05/2010 04:13 PM, Jakub Hrozek wrote:
> > Sumit, can you give me some more information on how to test this?
> > 
> > From what I gathered, you should be asked for a password as many times
> > as the retry= option for pam_sss says. What I'm seeing is that pam_sss
> > does not ask again..just retries the same password
> 
> After a debugging session off-list we found out that my issue was not
> caused by error in the code but rather by the fact I was testing with
> sshd with PasswordAuthentication - as Sumit pointed out out in this case
> the user prompt and the pam conversation call is directly handled by
> sshd. ChallengeResponseAuthentication must be used in order to get the
> desired behaviour with retry= option.
> 
> So, nack to the original patch - we need to document the above in the
> pam_sss man page.

done

> 
> The code looks OK to me, just one nitpick while you are changing the
> patch - can you fix the indent after the "flags &=
> !FLAGS_USE_FIRST_PASS;" reset? Also I'm generally afraid of hardcoded

done

> constans like the "6" used in case of handling of the "retry="
> option..but in this case it is probably, we're unlikely to ever change
> the option and it is confined to the eval_argv() function.

done

New version attached.

bye,
Sumit

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAkvivoQACgkQHsardTLnvCU+ggCfdVtgOPOqyqfy4v7ezRiNYV9D
> xVkAoJ2ec1hRAjiBVMj5uNAZ7SXdmCVw
> =58qj
> -----END PGP SIGNATURE-----
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
-------------- next part --------------
From fabde503df784065f11b109e9f60bba86fec777e Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 28 Apr 2010 10:01:06 +0200
Subject: [PATCH] Add retry option to pam_sss

---
 src/man/pam_sss.8.xml    |   17 ++++
 src/sss_client/pam_sss.c |  239 ++++++++++++++++++++++++++++------------------
 2 files changed, 164 insertions(+), 92 deletions(-)

diff --git a/src/man/pam_sss.8.xml b/src/man/pam_sss.8.xml
index 9a68196..aed76e2 100644
--- a/src/man/pam_sss.8.xml
+++ b/src/man/pam_sss.8.xml
@@ -31,6 +31,9 @@
             <arg choice='opt'>
                 <replaceable>use_authtok</replaceable>
             </arg>
+            <arg choice='opt'>
+                <replaceable>retry=N</replaceable>
+            </arg>
         </cmdsynopsis>
     </refsynopsisdiv>
 
@@ -75,6 +78,20 @@
                     password module.</para>
                 </listitem>
             </varlistentry>
+            <varlistentry>
+                <term>
+                    <option>retry=N</option>
+                </term>
+                <listitem>
+                    <para>If specified the user is asked another N times for a
+                    password if authentication fails. Default is 0.</para>
+                    <para>Please note that this option might not work as
+                    expected if the application calling PAM handles the user
+                    dialog on its own. A typical example is
+                    <command>sshd</command> with
+                    <option>PasswordAuthentication</option>.</para>
+                </listitem>
+            </varlistentry>
         </variablelist>
     </refsect1>
 
diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index 8bd3c9e..e6ab477 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -39,6 +39,7 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <locale.h>
+#include <stdbool.h>
 
 #include <security/pam_modules.h>
 #include <security/pam_ext.h>
@@ -60,6 +61,8 @@
 #define PW_RESET_MSG_FILENAME_TEMPLATE SSSD_CONF_DIR"/customize/%s/pam_sss_pw_reset_message.%s"
 #define PW_RESET_MSG_MAX_SIZE 4096
 
+#define OPT_RETRY_KEY "retry="
+
 struct pam_items {
     const char* pam_service;
     const char* pam_user;
@@ -205,6 +208,9 @@ static void overwrite_and_free_pam_items(struct pam_items *pi)
         pi->pam_newauthtok = NULL;
     }
 
+    pi->pamstack_authtok = NULL;
+    pi->pamstack_oldauthtok = NULL;
+
     free(pi->domain_name);
 }
 
@@ -1201,8 +1207,10 @@ static int prompt_new_password(pam_handle_t *pamh, struct pam_items *pi)
 }
 
 static void eval_argv(pam_handle_t *pamh, int argc, const char **argv,
-                      uint32_t *flags)
+                      uint32_t *flags, int *retries)
 {
+    char *ep;
+
     for (; argc-- > 0; ++argv) {
         if (strcmp(*argv, "forward_pass") == 0) {
             *flags |= FLAGS_FORWARD_PASS;
@@ -1210,6 +1218,28 @@ static void eval_argv(pam_handle_t *pamh, int argc, const char **argv,
             *flags |= FLAGS_USE_FIRST_PASS;
         } else if (strcmp(*argv, "use_authtok") == 0) {
             *flags |= FLAGS_USE_AUTHTOK;
+        } else if (strncmp(*argv, OPT_RETRY_KEY, strlen(OPT_RETRY_KEY)) == 0) {
+            if (*(*argv+6) == '\0') {
+                logger(pamh, LOG_ERR, "Missing argument to option retry.");
+                *retries = 0;
+            } else {
+                errno = 0;
+                *retries = strtol(*argv+6, &ep, 10);
+                if (errno != 0) {
+                    D(("strtol failed [%d][%s]", errno, strerror(errno)));
+                    *retries = 0;
+                }
+                if (*ep != '\0') {
+                    logger(pamh, LOG_ERR, "Argument to option retry contains "
+                                          "extra characters.");
+                    *retries = 0;
+                }
+                if (*retries < 0) {
+                    logger(pamh, LOG_ERR, "Argument to option retry must not "
+                                          "be negative.");
+                    *retries = 0;
+                }
+            }
         } else {
             logger(pamh, LOG_WARNING, "unknown option: %s", *argv);
         }
@@ -1334,12 +1364,14 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh,
     struct pam_items pi;
     uint32_t flags = 0;
     int *exp_data;
+    bool retry = false;
+    int retries = 0;
 
     bindtextdomain(PACKAGE, LOCALEDIR);
 
     D(("Hello pam_sssd: %d", task));
 
-    eval_argv(pamh, argc, argv, &flags);
+    eval_argv(pamh, argc, argv, &flags, &retries);
 
     ret = get_pam_items(pamh, &pi);
     if (ret != PAM_SUCCESS) {
@@ -1347,106 +1379,129 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh,
         return ret;
     }
 
-    switch(task) {
-        case SSS_PAM_AUTHENTICATE:
-            ret = get_authtok_for_authentication(pamh, &pi, flags);
-            if (ret != PAM_SUCCESS) {
-                D(("failed to get authentication token: %s",
-                   pam_strerror(pamh, ret)));
-                return ret;
-            }
-            break;
-        case SSS_PAM_CHAUTHTOK:
-            ret = get_authtok_for_password_change(pamh, &pi, flags, pam_flags);
-            if (ret != PAM_SUCCESS) {
-                D(("failed to get tokens for password change: %s",
-                   pam_strerror(pamh, ret)));
-                return ret;
-            }
-            if (pam_flags & PAM_PRELIM_CHECK) {
-                task = SSS_PAM_CHAUTHTOK_PRELIM;
-            }
-            break;
-        case SSS_PAM_ACCT_MGMT:
-        case SSS_PAM_SETCRED:
-        case SSS_PAM_OPEN_SESSION:
-        case SSS_PAM_CLOSE_SESSION:
-            break;
-        default:
-            D(("Illegal task [%d]", task));
-            return PAM_SYSTEM_ERR;
-    }
-
-    pam_status = send_and_receive(pamh, &pi, task);
-
-    switch (task) {
-        case SSS_PAM_AUTHENTICATE:
-            /* We allow sssd to send the return code PAM_NEW_AUTHTOK_REQD during
-             * authentication, see sss_cli.h for details */
-            if (pam_status == PAM_NEW_AUTHTOK_REQD) {
-                D(("Authtoken expired, trying to change it"));
-
-                exp_data = malloc(sizeof(int));
-                if (exp_data == NULL) {
-                    D(("malloc failed."));
-                    pam_status = PAM_BUF_ERR;
-                    break;
-                }
-                *exp_data = 1;
+    do {
+        retry = false;
 
-                pam_status = pam_set_data(pamh, PWEXP_FLAG, exp_data,
-                                          free_exp_data);
-                if (pam_status != PAM_SUCCESS) {
-                    D(("pam_set_data failed."));
-                }
-            }
-            break;
-        case SSS_PAM_ACCT_MGMT:
-            if (pam_status == PAM_SUCCESS &&
-                pam_get_data(pamh, PWEXP_FLAG, (const void **) &exp_data) ==
-                                                                  PAM_SUCCESS) {
-                ret = do_pam_conversation(pamh, PAM_TEXT_INFO,
-                               _("Password expired. Change your password now."),
-                               NULL, NULL);
+        switch(task) {
+            case SSS_PAM_AUTHENTICATE:
+                ret = get_authtok_for_authentication(pamh, &pi, flags);
                 if (ret != PAM_SUCCESS) {
-                    D(("do_pam_conversation failed."));
+                    D(("failed to get authentication token: %s",
+                       pam_strerror(pamh, ret)));
+                    return ret;
                 }
-                pam_status = PAM_NEW_AUTHTOK_REQD;
-            }
-            break;
-        case SSS_PAM_CHAUTHTOK:
-            if (pam_status != PAM_SUCCESS && pam_status != PAM_USER_UNKNOWN) {
-                ret = pam_set_item(pamh, PAM_AUTHTOK, NULL);
+                break;
+            case SSS_PAM_CHAUTHTOK:
+                ret = get_authtok_for_password_change(pamh, &pi, flags, pam_flags);
                 if (ret != PAM_SUCCESS) {
-                    D(("Failed to unset PAM_AUTHTOK [%s]",
-                       pam_strerror(pamh,ret)));
+                    D(("failed to get tokens for password change: %s",
+                       pam_strerror(pamh, ret)));
+                    return ret;
                 }
-                ret = pam_set_item(pamh, PAM_OLDAUTHTOK, NULL);
-                if (ret != PAM_SUCCESS) {
-                    D(("Failed to unset PAM_OLDAUTHTOK [%s]",
-                       pam_strerror(pamh,ret)));
+                if (pam_flags & PAM_PRELIM_CHECK) {
+                    task = SSS_PAM_CHAUTHTOK_PRELIM;
                 }
-            }
-            break;
-        case SSS_PAM_CHAUTHTOK_PRELIM:
-            if (pam_status == PAM_PERM_DENIED && pi.pam_authtok_size == 0 &&
-                getuid() == 0 &&
-                pam_get_data(pamh, PWEXP_FLAG, (const void **) &exp_data) !=
-                                                                  PAM_SUCCESS) {
-
-                ret = select_pw_reset_message(pamh, &pi);
-                if (ret != 0) {
-                    D(("select_pw_reset_message failed.\n"));
+                break;
+            case SSS_PAM_ACCT_MGMT:
+            case SSS_PAM_SETCRED:
+            case SSS_PAM_OPEN_SESSION:
+            case SSS_PAM_CLOSE_SESSION:
+                break;
+            default:
+                D(("Illegal task [%d]", task));
+                return PAM_SYSTEM_ERR;
+        }
+
+        pam_status = send_and_receive(pamh, &pi, task);
+
+        switch (task) {
+            case SSS_PAM_AUTHENTICATE:
+                /* We allow sssd to send the return code PAM_NEW_AUTHTOK_REQD during
+                 * authentication, see sss_cli.h for details */
+                if (pam_status == PAM_NEW_AUTHTOK_REQD) {
+                    D(("Authtoken expired, trying to change it"));
+
+                    exp_data = malloc(sizeof(int));
+                    if (exp_data == NULL) {
+                        D(("malloc failed."));
+                        pam_status = PAM_BUF_ERR;
+                        break;
+                    }
+                    *exp_data = 1;
+
+                    pam_status = pam_set_data(pamh, PWEXP_FLAG, exp_data,
+                                              free_exp_data);
+                    if (pam_status != PAM_SUCCESS) {
+                        D(("pam_set_data failed."));
+                    }
                 }
-            }
-        default:
-            /* nothing to do */
-            break;
-    }
+                break;
+            case SSS_PAM_ACCT_MGMT:
+                if (pam_status == PAM_SUCCESS &&
+                    pam_get_data(pamh, PWEXP_FLAG, (const void **) &exp_data) ==
+                                                                      PAM_SUCCESS) {
+                    ret = do_pam_conversation(pamh, PAM_TEXT_INFO,
+                                   _("Password expired. Change your password now."),
+                                   NULL, NULL);
+                    if (ret != PAM_SUCCESS) {
+                        D(("do_pam_conversation failed."));
+                    }
+                    pam_status = PAM_NEW_AUTHTOK_REQD;
+                }
+                break;
+            case SSS_PAM_CHAUTHTOK:
+                if (pam_status != PAM_SUCCESS && pam_status != PAM_USER_UNKNOWN) {
+                    ret = pam_set_item(pamh, PAM_AUTHTOK, NULL);
+                    if (ret != PAM_SUCCESS) {
+                        D(("Failed to unset PAM_AUTHTOK [%s]",
+                           pam_strerror(pamh,ret)));
+                    }
+                    ret = pam_set_item(pamh, PAM_OLDAUTHTOK, NULL);
+                    if (ret != PAM_SUCCESS) {
+                        D(("Failed to unset PAM_OLDAUTHTOK [%s]",
+                           pam_strerror(pamh,ret)));
+                    }
+                }
+                break;
+            case SSS_PAM_CHAUTHTOK_PRELIM:
+                if (pam_status == PAM_PERM_DENIED && pi.pam_authtok_size == 0 &&
+                    getuid() == 0 &&
+                    pam_get_data(pamh, PWEXP_FLAG, (const void **) &exp_data) !=
+                                                                      PAM_SUCCESS) {
+
+                    ret = select_pw_reset_message(pamh, &pi);
+                    if (ret != 0) {
+                        D(("select_pw_reset_message failed.\n"));
+                    }
+                }
+            default:
+                /* nothing to do */
+                break;
+        }
 
+        overwrite_and_free_pam_items(&pi);
 
+        D(("retries [%d].", retries));
 
-    overwrite_and_free_pam_items(&pi);
+        if (pam_status != PAM_SUCCESS &&
+            (task == SSS_PAM_AUTHENTICATE || task == SSS_PAM_CHAUTHTOK_PRELIM) &&
+            retries > 0) {
+            retry = true;
+            retries--;
+
+            flags &= !FLAGS_USE_FIRST_PASS;
+            ret = pam_set_item(pamh, PAM_AUTHTOK, NULL);
+            if (ret != PAM_SUCCESS) {
+                D(("Failed to unset PAM_AUTHTOK [%s]",
+                   pam_strerror(pamh,ret)));
+            }
+            ret = pam_set_item(pamh, PAM_OLDAUTHTOK, NULL);
+            if (ret != PAM_SUCCESS) {
+                D(("Failed to unset PAM_OLDAUTHTOK [%s]",
+                   pam_strerror(pamh,ret)));
+            }
+        }
+    } while(retry);
 
     return pam_status;
 }
-- 
1.6.6.1



More information about the sssd-devel mailing list