URL: https://github.com/SSSD/sssd/pull/483
Author: sumit-bose
 Title: #483: Password change with two factor authentication
Action: opened

PR body:
"""
If two factor authentication is enforced both authentication factors are
needed to update or change the long term password. This means that during
the PAM chauthok operation it has to be determined if two factor
authentication is enable for the user and the user must be prompted
accordingly.

Typically in the first step of the chauthok operation (PAM_PRELIM_CHECK)
the current password is verified before asking the user for a new password.
With two factor authentication this has to be skipped because the one-time
factor would then be invalid to authenticate the actual password change.

In krb5_child if the long term Kerberos password is expired the available
pre-authentication types for the user should be checked by requesting the
kadmin/changepw principal.

Related to https://pagure.io/SSSD/sssd/issue/3585
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/483/head:pr483
git checkout pr483
From eaa40a8dd287c24135df3bb96a6150cefc1d729d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 18 Dec 2017 15:39:32 +0100
Subject: [PATCH 1/2] krb5_child: check preauth types if password is expired

If the long term Kerberos password is expired the available
pre-authentication types for the user should be checked by requesting
the kadmin/changepw principal. This is e.g. needed for 2-factor
authentication where the user not only has to specific the current
long password but an one-time password as well.

Related to https://pagure.io/SSSD/sssd/issue/3585
---
 src/providers/krb5/krb5_child.c | 102 ++++++++++++++++++++++++++--------------
 1 file changed, 68 insertions(+), 34 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 7ee6c34eb..97a6015e5 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1532,9 +1532,11 @@ static krb5_error_code get_and_save_tgt(struct krb5_req *kr,
                                         password_or_responder(password),
                                         sss_krb5_prompter, kr, 0,
                                         NULL, kr->options);
-    if (kr->pd->cmd == SSS_PAM_PREAUTH) {
-        /* Any errors are ignored during pre-auth, only data is collected to
-         * be send back to the client.*/
+    if (kr->pd->cmd == SSS_PAM_PREAUTH && kerr != KRB5KDC_ERR_KEY_EXP) {
+        /* Any errors except KRB5KDC_ERR_KEY_EXP are ignored during pre-auth,
+         * only data is collected to be send back to the client.
+         * KRB5KDC_ERR_KEY_EXP must be handled separately to figure out the
+         * possible authentication methods to update the password. */
         DEBUG(SSSDBG_TRACE_FUNC,
               "krb5_get_init_creds_password returned [%d] during pre-auth.\n",
               kerr);
@@ -1731,12 +1733,14 @@ static errno_t changepw_child(struct krb5_req *kr, bool prelim)
 
     DEBUG(SSSDBG_TRACE_LIBS, "Password change operation\n");
 
-    ret = sss_authtok_get_password(kr->pd->authtok, &password, NULL);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              "Failed to fetch current password [%d] %s.\n",
-                  ret, strerror(ret));
-        return ERR_NO_CREDS;
+    if (sss_authtok_get_type(kr->pd->authtok) == SSS_AUTHTOK_TYPE_PASSWORD) {
+        ret = sss_authtok_get_password(kr->pd->authtok, &password, NULL);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "Failed to fetch current password [%d] %s.\n",
+                      ret, strerror(ret));
+            return ERR_NO_CREDS;
+        }
     }
 
     if (!prelim) {
@@ -1893,6 +1897,40 @@ static errno_t changepw_child(struct krb5_req *kr, bool prelim)
     return map_krb5_error(kerr);
 }
 
+static errno_t pam_add_prompting(struct krb5_req *kr)
+{
+    int ret;
+
+    /* add OTP tokeninfo messge if available */
+    if (kr->otp) {
+        ret = k5c_attach_otp_info_msg(kr);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "k5c_attach_otp_info_msg failed.\n");
+            return ret;
+        }
+    }
+
+    if (kr->password_prompting) {
+        ret = pam_add_response(kr->pd, SSS_PASSWORD_PROMPTING, 0, NULL);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "pam_add_response failed.\n");
+            return ret;
+        }
+    }
+
+    if (kr->pkinit_prompting) {
+        ret = pam_add_response(kr->pd, SSS_CERT_AUTH_PROMPTING, 0,
+                               NULL);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "pam_add_response failed.\n");
+            return ret;
+        }
+    }
+
+    return EOK;
+}
+
 static errno_t tgt_req_child(struct krb5_req *kr)
 {
     const char *password = NULL;
@@ -1928,31 +1966,10 @@ static errno_t tgt_req_child(struct krb5_req *kr)
 
     if (kerr != KRB5KDC_ERR_KEY_EXP) {
         if (kr->pd->cmd == SSS_PAM_PREAUTH) {
-            /* add OTP tokeninfo messge if available */
-            if (kr->otp) {
-                ret = k5c_attach_otp_info_msg(kr);
-                if (ret != EOK) {
-                    DEBUG(SSSDBG_CRIT_FAILURE,
-                          "k5c_attach_otp_info_msg failed.\n");
-                    goto done;
-                }
-            }
-
-            if (kr->password_prompting) {
-                ret = pam_add_response(kr->pd, SSS_PASSWORD_PROMPTING, 0, NULL);
-                if (ret != EOK) {
-                    DEBUG(SSSDBG_CRIT_FAILURE, "pam_add_response failed.\n");
-                    goto done;
-                }
-            }
-
-            if (kr->pkinit_prompting) {
-                ret = pam_add_response(kr->pd, SSS_CERT_AUTH_PROMPTING, 0,
-                                       NULL);
-                if (ret != EOK) {
-                    DEBUG(SSSDBG_CRIT_FAILURE, "pam_add_response failed.\n");
-                    goto done;
-                }
+            ret = pam_add_prompting(kr);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_CRIT_FAILURE, "pam_add_prompting failed.\n");
+                goto done;
             }
         } else {
             if (kerr == 0) {
@@ -1985,6 +2002,23 @@ static errno_t tgt_req_child(struct krb5_req *kr)
                                         kr->options);
 
     krb5_free_cred_contents(kr->ctx, kr->creds);
+
+    if (kr->pd->cmd == SSS_PAM_PREAUTH) {
+        /* Any errors are ignored during pre-auth, only data is collected to
+         * be send back to the client. Even if the password is expired we
+         * should now know which authentication methods are available to
+         * update the password. */
+        DEBUG(SSSDBG_TRACE_FUNC,
+              "krb5_get_init_creds_password returned [%d] during pre-auth, "
+              "ignored.\n", kerr);
+        ret = pam_add_prompting(kr);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "pam_add_prompting failed.\n");
+            goto done;
+        }
+        goto done;
+    }
+
     if (kerr == 0) {
         ret = ERR_CREDS_EXPIRED;
 

From 0a231ba3d5c5950c4212b2fa0e079c9bd7064bf7 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 18 Dec 2017 15:45:21 +0100
Subject: [PATCH 2/2] pam_sss: password change with two factor authentication

If two factor authentication is enforced both authentication factors are
needed to update or change the long term password. This means that
during the PAM chauthok operation it has to be determined if two factor
authentication is enable for the user and the user must be prompted
accordingly.

Typically in the first step of the chauthok operation (PAM_PRELIM_CHECK)
the current password is verified before asking the user for a new
password. With two factor authentication this has to be skipped because
the one-time factor would then be invalid to authenticate the actual
password change.

Related to https://pagure.io/SSSD/sssd/issue/3585
---
 src/sss_client/pam_sss.c | 193 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 174 insertions(+), 19 deletions(-)

diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index 1dc51ea05..e9413750c 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -63,6 +63,9 @@
 
 #define PWEXP_FLAG "pam_sss:password_expired_flag"
 #define FD_DESTRUCTOR "pam_sss:fd_destructor"
+#define PAM_SSS_AUTHOK_TYPE "pam_sss:authtok_type"
+#define PAM_SSS_AUTHOK_SIZE "pam_sss:authtok_size"
+#define PAM_SSS_AUTHOK_DATA "pam_sss:authtok_data"
 
 #define PW_RESET_MSG_FILENAME_TEMPLATE SSSD_CONF_DIR"/customize/%s/pam_sss_pw_reset_message.%s"
 #define PW_RESET_MSG_MAX_SIZE 4096
@@ -2079,6 +2082,102 @@ static int get_authtok_for_authentication(pam_handle_t *pamh,
     return PAM_SUCCESS;
 }
 
+static int check_authtok_data(pam_handle_t *pamh, struct pam_items *pi)
+{
+    int pam_status;
+    int *authtok_type;
+    size_t *authtok_size;
+    char *authtok_data;
+
+    pam_status = pam_get_data(pamh, PAM_SSS_AUTHOK_TYPE,
+                              (const void **) &authtok_type);
+    if (pam_status != PAM_SUCCESS) {
+        D(("pam_get_data failed."));
+        return EIO;
+    }
+
+    pam_status = pam_get_data(pamh, PAM_SSS_AUTHOK_SIZE,
+                              (const void **) &authtok_size);
+    if (pam_status != PAM_SUCCESS) {
+        D(("pam_get_data failed."));
+        return EIO;
+    }
+
+    pam_status = pam_get_data(pamh, PAM_SSS_AUTHOK_DATA,
+                              (const void **) &authtok_data);
+    if (pam_status != PAM_SUCCESS) {
+        D(("pam_get_data failed."));
+        return EIO;
+    }
+
+    pi->pam_authtok = malloc(*authtok_size);
+    if (pi->pam_authtok == NULL) {
+        D(("malloc failed."));
+        return ENOMEM;
+    }
+    memcpy(pi->pam_authtok, authtok_data, *authtok_size);
+
+    pi->pam_authtok_type = *authtok_type;
+    pi->pam_authtok_size = *authtok_size;
+
+    return 0;
+}
+
+static int keep_authtok_data(pam_handle_t *pamh, struct pam_items *pi)
+{
+    int pam_status;
+    int *authtok_type;
+    size_t *authtok_size;
+    char *authtok_data;
+
+    authtok_type = malloc(sizeof(int));
+    if (authtok_type == NULL) {
+        D(("malloc failed."));
+        return ENOMEM;
+    }
+    *authtok_type = pi->pam_authtok_type;
+
+    pam_status = pam_set_data(pamh, PAM_SSS_AUTHOK_TYPE, authtok_type,
+                              free_exp_data);
+    if (pam_status != PAM_SUCCESS) {
+        free(authtok_type);
+        D(("pam_set_data failed."));
+        return EIO;
+    }
+
+    authtok_size = malloc(sizeof(size_t));
+    if (authtok_size == NULL) {
+        D(("malloc failed."));
+        return ENOMEM;
+    }
+    *authtok_size = pi->pam_authtok_size;
+
+    pam_status = pam_set_data(pamh, PAM_SSS_AUTHOK_SIZE, authtok_size,
+                              free_exp_data);
+    if (pam_status != PAM_SUCCESS) {
+        free(authtok_size);
+        D(("pam_set_data failed."));
+        return EIO;
+    }
+
+    authtok_data = malloc(pi->pam_authtok_size);
+    if (authtok_data == NULL) {
+        D(("malloc failed."));
+        return ENOMEM;
+    }
+    memcpy(authtok_data, pi->pam_authtok, pi->pam_authtok_size);
+
+    pam_status = pam_set_data(pamh, PAM_SSS_AUTHOK_DATA, authtok_data,
+                              free_exp_data);
+    if (pam_status != PAM_SUCCESS) {
+        free(authtok_data);
+        D(("pam_set_data failed."));
+        return EIO;
+    }
+
+    return 0;
+}
+
 static int get_authtok_for_password_change(pam_handle_t *pamh,
                                            struct pam_items *pi,
                                            uint32_t flags,
@@ -2092,10 +2191,22 @@ static int get_authtok_for_password_change(pam_handle_t *pamh,
      * pam_sss work e.g. with pam_cracklib */
     if (pam_flags & PAM_PRELIM_CHECK) {
         if ( (getuid() != 0 || exp_data ) && !(flags & FLAGS_USE_FIRST_PASS)) {
-            ret = prompt_password(pamh, pi, _("Current Password: "));
-            if (ret != PAM_SUCCESS) {
-                D(("failed to get password from user"));
-                return ret;
+            if (flags & FLAGS_USE_2FA
+                    || (pi->otp_vendor != NULL && pi->otp_token_id != NULL
+                            && pi->otp_challenge != NULL)) {
+                if (pi->password_prompting) {
+                    ret = prompt_2fa(pamh, pi, _("First Factor (Current Password): "),
+                                     _("Second Factor (optional): "));
+                } else {
+                    ret = prompt_2fa(pamh, pi, _("First Factor (Current Password): "),
+                                     _("Second Factor: "));
+                }
+            } else {
+                ret = prompt_password(pamh, pi, _("Current Password: "));
+                if (ret != PAM_SUCCESS) {
+                    D(("failed to get password from user"));
+                    return ret;
+                }
             }
 
             ret = pam_set_item(pamh, PAM_OLDAUTHTOK, pi->pam_authtok);
@@ -2105,28 +2216,38 @@ static int get_authtok_for_password_change(pam_handle_t *pamh,
                    pam_strerror(pamh,ret)));
                    return ret;
             }
+
+            if (pi->pam_authtok_type == SSS_AUTHTOK_TYPE_2FA) {
+                ret = keep_authtok_data(pamh, pi);
+                if (ret != 0) {
+                    D(("Failed to store authtok data to pam handle. Password "
+                       "change might fail."));
+                }
+            }
         }
 
         return PAM_SUCCESS;
     }
 
-    if (pi->pamstack_oldauthtok == NULL) {
-        if (getuid() != 0) {
-            D(("no password found for chauthtok"));
-            return PAM_BUF_ERR;
+    if (check_authtok_data(pamh, pi) != 0) {
+        if (pi->pamstack_oldauthtok == NULL) {
+            if (getuid() != 0) {
+                D(("no password found for chauthtok"));
+                return PAM_BUF_ERR;
+            } else {
+                pi->pam_authtok_type = SSS_AUTHTOK_TYPE_EMPTY;
+                pi->pam_authtok = NULL;
+                pi->pam_authtok_size = 0;
+            }
         } else {
-            pi->pam_authtok_type = SSS_AUTHTOK_TYPE_EMPTY;
-            pi->pam_authtok = NULL;
-            pi->pam_authtok_size = 0;
-        }
-    } else {
-        pi->pam_authtok = strdup(pi->pamstack_oldauthtok);
-        if (pi->pam_authtok == NULL) {
-            D(("strdup failed"));
-            return PAM_BUF_ERR;
+            pi->pam_authtok = strdup(pi->pamstack_oldauthtok);
+            if (pi->pam_authtok == NULL) {
+                D(("strdup failed"));
+                return PAM_BUF_ERR;
+            }
+            pi->pam_authtok_type = SSS_AUTHTOK_TYPE_PASSWORD;
+            pi->pam_authtok_size = strlen(pi->pam_authtok);
         }
-        pi->pam_authtok_type = SSS_AUTHTOK_TYPE_PASSWORD;
-        pi->pam_authtok_size = strlen(pi->pam_authtok);
     }
 
     if (flags & FLAGS_USE_AUTHTOK) {
@@ -2306,6 +2427,31 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh,
                 }
                 break;
             case SSS_PAM_CHAUTHTOK:
+                /*
+                 * Only do preauth if
+                 * - PAM_PRELIM_CHECK is set
+                 * - FLAGS_USE_FIRST_PASS is not set
+                 * - no password is on the stack or FLAGS_PROMPT_ALWAYS is set
+                 * - preauth indicator file exists.
+                 */
+                if ( (pam_flags & PAM_PRELIM_CHECK)
+                        && !(flags & FLAGS_USE_FIRST_PASS)
+                        && (pi.pam_authtok == NULL
+                                || (flags & FLAGS_PROMPT_ALWAYS))
+                        && access(PAM_PREAUTH_INDICATOR, F_OK) == 0) {
+                    pam_status = send_and_receive(pamh, &pi, SSS_PAM_PREAUTH,
+                                                  quiet_mode);
+                    if (pam_status != PAM_SUCCESS) {
+                        D(("send_and_receive returned [%d] during pre-auth",
+                           pam_status));
+                        /*
+                         * Since we are only interested in the result message
+                         * and will always use password authentication
+                         * as a fallback, errors can be ignored here.
+                         */
+                    }
+                }
+
                 ret = get_authtok_for_password_change(pamh, &pi, flags, pam_flags);
                 if (ret != PAM_SUCCESS) {
                     D(("failed to get tokens for password change: %s",
@@ -2313,7 +2459,16 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh,
                     overwrite_and_free_pam_items(&pi);
                     return ret;
                 }
+
                 if (pam_flags & PAM_PRELIM_CHECK) {
+                    if (pi.pam_authtok_type == SSS_AUTHTOK_TYPE_2FA) {
+                        /* We cannot validate the credentials with an OTP
+                         * token value during PAM_PRELIM_CHECK because it
+                         * would be invalid for the actual password change. So
+                         * we are done. */
+
+                        return PAM_SUCCESS;
+                    }
                     task = SSS_PAM_CHAUTHTOK_PRELIM;
                 }
                 break;
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to