[SSSD] [PATCH] Handle chauthtok with PAM_PRELIM_CHECK separately

Sumit Bose sbose at redhat.com
Wed Dec 16 13:31:47 UTC 2009


On Wed, Dec 16, 2009 at 02:03:44PM +0100, Sumit Bose wrote:
> On Wed, Dec 16, 2009 at 01:04:01PM +0100, Sumit Bose wrote:
> > Hi,
> > 
> > this patch should fix #326 "Missing checks in PAM_PRELIM_CHECK"
> > 
> > Currently we collect old and new password in pam_sss and send them
> > together to the sssd to change to password. But it is expected that
> > during the first call to pam_sm_chauthtok, when PAM_PRELIM_CHECK is set,
> > the old password is validated to give a feedback to the user before the
> > new password is requested.
> > 
> > Please check and test carefully.
> > 
> > bye,
> > Sumit
> 
> aaahh, sorry, I found a flaw in the changes for the Kerberos provider.
> 
> Please wait for a new version ...
> 

ok, new version attached, I also added Tomas suggestion.

bye,
Sumit
-------------- next part --------------
From 383234e5bf6d24d21772581ec8268dc763ccb8b2 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 16 Dec 2009 12:53:55 +0100
Subject: [PATCH] Handle chauthtok with PAM_PRELIM_CHECK separately

If pam_sm_chauthtok is called with the flag PAM_PRELIM_CHECK set we
generate a separate call to the sssd to validate the old password before
asking for a new password and sending the change password request.
---
 server/providers/data_provider_be.c |    1 +
 server/providers/krb5/krb5_auth.c   |   15 ++++++++++++-
 server/providers/krb5/krb5_child.c  |   36 ++++++++++++++++++++++++----------
 server/providers/ldap/ldap_auth.c   |   26 ++++++++++++++++++------
 server/providers/proxy.c            |   13 +++++++++++-
 server/responder/pam/pamsrv_cmd.c   |    6 +++++
 sss_client/pam_sss.c                |    2 +-
 sss_client/sss_cli.h                |    2 +-
 8 files changed, 78 insertions(+), 23 deletions(-)

diff --git a/server/providers/data_provider_be.c b/server/providers/data_provider_be.c
index 02d5a0d..05f3eaf 100644
--- a/server/providers/data_provider_be.c
+++ b/server/providers/data_provider_be.c
@@ -565,6 +565,7 @@ static int be_pam_handler(DBusMessage *message, struct sbus_connection *conn)
             target = BET_ACCESS;
             break;
         case SSS_PAM_CHAUTHTOK:
+        case SSS_PAM_CHAUTHTOK_PRELIM:
             target = BET_CHPASS;
             break;
         default:
diff --git a/server/providers/krb5/krb5_auth.c b/server/providers/krb5/krb5_auth.c
index 4581278..10e6797 100644
--- a/server/providers/krb5/krb5_auth.c
+++ b/server/providers/krb5/krb5_auth.c
@@ -398,9 +398,11 @@ static struct krb5_ctx *get_krb5_ctx(struct be_req *be_req)
                                        struct krb5_ctx);
             break;
         case SSS_PAM_CHAUTHTOK:
+        case SSS_PAM_CHAUTHTOK_PRELIM:
             return talloc_get_type(be_req->be_ctx->bet_info[BET_CHPASS].pvt_bet_data,
                                        struct krb5_ctx);
             break;
+            break;
         default:
             DEBUG(1, ("Unsupported PAM task.\n"));
             return NULL;
@@ -685,14 +687,16 @@ void krb5_pam_handler(struct be_req *be_req)
 
     pd = talloc_get_type(be_req->req_data, struct pam_data);
 
-    if (pd->cmd != SSS_PAM_AUTHENTICATE && pd->cmd != SSS_PAM_CHAUTHTOK) {
+    if (pd->cmd != SSS_PAM_AUTHENTICATE && pd->cmd != SSS_PAM_CHAUTHTOK &&
+        pd->cmd != SSS_PAM_CHAUTHTOK_PRELIM) {
         DEBUG(4, ("krb5 does not handles pam task %d.\n", pd->cmd));
         pam_status = PAM_SUCCESS;
         dp_err = DP_ERR_OK;
         goto done;
     }
 
-    if (be_is_offline(be_req->be_ctx) && pd->cmd == SSS_PAM_CHAUTHTOK) {
+    if (be_is_offline(be_req->be_ctx) &&
+        (pd->cmd == SSS_PAM_CHAUTHTOK || pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM)) {
         DEBUG(9, ("Password changes are not possible while offline.\n"));
         pam_status = PAM_AUTHINFO_UNAVAIL;
         dp_err = DP_ERR_OFFLINE;
@@ -958,6 +962,12 @@ static void krb5_child_done(struct tevent_req *req)
         pd->pam_status = *msg_status;
     }
 
+    if (*msg_status == PAM_SUCCESS && pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM) {
+        pam_status = PAM_SUCCESS;
+        dp_err = DP_ERR_OK;
+        goto done;
+    }
+
     pref_len = strlen(CCACHE_ENV_NAME)+1;
     if (*msg_len > pref_len &&
         strncmp((const char *) &buf[p], CCACHE_ENV_NAME"=", pref_len) == 0) {
@@ -1047,6 +1057,7 @@ static void krb5_save_ccname_done(struct tevent_req *req)
 
         switch(pd->cmd) {
             case SSS_PAM_AUTHENTICATE:
+            case SSS_PAM_CHAUTHTOK_PRELIM:
                 password = talloc_size(be_req, pd->authtok_size + 1);
                 if (password != NULL) {
                     memcpy(password, pd->authtok, pd->authtok_size);
diff --git a/server/providers/krb5/krb5_child.c b/server/providers/krb5/krb5_child.c
index 2f48574..6eb420b 100644
--- a/server/providers/krb5/krb5_child.c
+++ b/server/providers/krb5/krb5_child.c
@@ -299,19 +299,24 @@ static struct response *prepare_response_message(struct krb5_req *kr,
     }
 
     if (kerr == 0) {
-        if (kr->ccname == NULL) {
-            DEBUG(1, ("Error obtaining ccname.\n"));
-            return NULL;
-        }
+        if(kr->pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM) {
+            ret = pack_response_packet(resp, PAM_SUCCESS, PAM_USER_INFO,
+                                       "success");
+        } else {
+            if (kr->ccname == NULL) {
+                DEBUG(1, ("Error obtaining ccname.\n"));
+                return NULL;
+            }
 
-        msg = talloc_asprintf(kr, "%s=%s",CCACHE_ENV_NAME, kr->ccname);
-        if (msg == NULL) {
-            DEBUG(1, ("talloc_asprintf failed.\n"));
-            return NULL;
-        }
+            msg = talloc_asprintf(kr, "%s=%s",CCACHE_ENV_NAME, kr->ccname);
+            if (msg == NULL) {
+                DEBUG(1, ("talloc_asprintf failed.\n"));
+                return NULL;
+            }
 
-        ret = pack_response_packet(resp, PAM_SUCCESS, PAM_ENV_ITEM, msg);
-        talloc_zfree(msg);
+            ret = pack_response_packet(resp, PAM_SUCCESS, PAM_ENV_ITEM, msg);
+            talloc_zfree(msg);
+        }
     } else {
         krb5_msg = sss_krb5_get_error_message(krb5_error_ctx, kerr);
         if (krb5_msg == NULL) {
@@ -528,6 +533,14 @@ static errno_t changepw_child(int fd, struct krb5_req *kr)
     talloc_zfree(pass_str);
     memset(kr->pd->authtok, 0, kr->pd->authtok_size);
 
+    if (kr->pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM) {
+        DEBUG(9, ("Initial authentication for change password operation "
+                  "successfull.\n"));
+        krb5_free_cred_contents(kr->ctx, kr->creds);
+        pam_status = PAM_SUCCESS;
+        goto sendresponse;
+    }
+
     newpass_str = talloc_strndup(kr, (const char *) kr->pd->newauthtok,
                               kr->pd->newauthtok_size);
     if (newpass_str == NULL) {
@@ -824,6 +837,7 @@ static int krb5_setup(struct pam_data *pd, const char *user_princ_str,
             }
             break;
         case SSS_PAM_CHAUTHTOK:
+        case SSS_PAM_CHAUTHTOK_PRELIM:
             kr->child_req = changepw_child;
             break;
         default:
diff --git a/server/providers/ldap/ldap_auth.c b/server/providers/ldap/ldap_auth.c
index 88e637b..28b3240 100644
--- a/server/providers/ldap/ldap_auth.c
+++ b/server/providers/ldap/ldap_auth.c
@@ -661,7 +661,7 @@ void sdap_pam_chpass_handler(struct be_req *breq)
 
     pd->pam_status = PAM_SYSTEM_ERR;
 
-    if (pd->cmd != SSS_PAM_CHAUTHTOK) {
+    if (pd->cmd != SSS_PAM_CHAUTHTOK && pd->cmd != SSS_PAM_CHAUTHTOK_PRELIM) {
         DEBUG(2, ("chpass target was called by wrong pam command.\n"));
         goto done;
     }
@@ -677,12 +677,15 @@ void sdap_pam_chpass_handler(struct be_req *breq)
     if (!state->password) goto done;
     talloc_set_destructor((TALLOC_CTX *)state->password,
                           password_destructor);
-    state->new_password = talloc_strndup(state,
-                                         (char *)pd->newauthtok,
-                                         pd->newauthtok_size);
-    if (!state->new_password) goto done;
-    talloc_set_destructor((TALLOC_CTX *)state->new_password,
-                          password_destructor);
+
+    if (pd->cmd == SSS_PAM_CHAUTHTOK) {
+        state->new_password = talloc_strndup(state,
+                                             (char *)pd->newauthtok,
+                                             pd->newauthtok_size);
+        if (!state->new_password) goto done;
+        talloc_set_destructor((TALLOC_CTX *)state->new_password,
+                              password_destructor);
+    }
 
     authtok.data = (uint8_t *)state->password;
     authtok.length = strlen(state->password);
@@ -717,6 +720,14 @@ static void sdap_auth4chpass_done(struct tevent_req *req)
         goto done;
     }
 
+    if (result == SDAP_AUTH_SUCCESS &&
+        state->pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM) {
+        DEBUG(9, ("Initial authentication for change password operation "
+                  "successful.\n"));
+        state->pd->pam_status = PAM_SUCCESS;
+        goto done;
+    }
+
     if (result == SDAP_AUTH_SUCCESS) {
         switch (pw_expire_type) {
             case PWEXPIRE_SHADOW:
@@ -851,6 +862,7 @@ void sdap_pam_auth_handler(struct be_req *breq)
 
     switch (pd->cmd) {
     case SSS_PAM_AUTHENTICATE:
+    case SSS_PAM_CHAUTHTOK_PRELIM:
 
         state = talloc_zero(breq, struct sdap_pam_auth_state);
         if (!state) goto done;
diff --git a/server/providers/proxy.c b/server/providers/proxy.c
index ef52ae9..080479c 100644
--- a/server/providers/proxy.c
+++ b/server/providers/proxy.c
@@ -137,6 +137,7 @@ static void proxy_pam_handler(struct be_req *req) {
                                   struct proxy_auth_ctx);
             break;
         case SSS_PAM_CHAUTHTOK:
+        case SSS_PAM_CHAUTHTOK_PRELIM:
             ctx = talloc_get_type(req->be_ctx->bet_info[BET_CHPASS].pvt_bet_data,
                                   struct proxy_auth_ctx);
             break;
@@ -207,12 +208,22 @@ static void proxy_pam_handler(struct be_req *req) {
                     cache_auth_data = true;
                 }
                 break;
+            case SSS_PAM_CHAUTHTOK_PRELIM:
+                if (pd->priv != 1) {
+                    auth_data->authtok_size = pd->authtok_size;
+                    auth_data->authtok = pd->authtok;
+                    pam_status = pam_authenticate(pamh, 0);
+                } else {
+                    pam_status = PAM_SUCCESS;
+                }
+                break;
             default:
                 DEBUG(1, ("unknown PAM call"));
                 pam_status=PAM_ABORT;
         }
 
-        DEBUG(4, ("Pam result: [%d][%s]\n", pam_status, pam_strerror(pamh, pam_status)));
+        DEBUG(4, ("Pam result: [%d][%s]\n", pam_status,
+                  pam_strerror(pamh, pam_status)));
 
         if (pam_status == PAM_AUTHINFO_UNAVAIL) {
             be_mark_offline(req->be_ctx);
diff --git a/server/responder/pam/pamsrv_cmd.c b/server/responder/pam/pamsrv_cmd.c
index 4479926..69cbf55 100644
--- a/server/responder/pam/pamsrv_cmd.c
+++ b/server/responder/pam/pamsrv_cmd.c
@@ -1051,6 +1051,11 @@ static int pam_cmd_chauthtok(struct cli_ctx *cctx) {
     return pam_forwarder(cctx, SSS_PAM_CHAUTHTOK);
 }
 
+static int pam_cmd_chauthtok_prelim(struct cli_ctx *cctx) {
+    DEBUG(4, ("entering pam_cmd_chauthtok_prelim\n"));
+    return pam_forwarder(cctx, SSS_PAM_CHAUTHTOK_PRELIM);
+}
+
 struct cli_protocol_version *register_cli_protocol_version(void)
 {
     static struct cli_protocol_version pam_cli_protocol_version[] = {
@@ -1073,6 +1078,7 @@ struct sss_cmd_table *get_pam_cmds(void)
         {SSS_PAM_OPEN_SESSION, pam_cmd_open_session},
         {SSS_PAM_CLOSE_SESSION, pam_cmd_close_session},
         {SSS_PAM_CHAUTHTOK, pam_cmd_chauthtok},
+        {SSS_PAM_CHAUTHTOK_PRELIM, pam_cmd_chauthtok_prelim},
         {SSS_CLI_NULL, NULL}
     };
 
diff --git a/sss_client/pam_sss.c b/sss_client/pam_sss.c
index 1c4aed0..951a1dc 100644
--- a/sss_client/pam_sss.c
+++ b/sss_client/pam_sss.c
@@ -814,7 +814,7 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh,
                 return ret;
             }
             if (pam_flags & PAM_PRELIM_CHECK) {
-                return ret;
+                task = SSS_PAM_CHAUTHTOK_PRELIM;
             }
             break;
         case SSS_PAM_ACCT_MGMT:
diff --git a/sss_client/sss_cli.h b/sss_client/sss_cli.h
index b036aec..7c18e1c 100644
--- a/sss_client/sss_cli.h
+++ b/sss_client/sss_cli.h
@@ -128,7 +128,7 @@ enum sss_cli_command {
     SSS_PAM_OPEN_SESSION     = 0x00F4,
     SSS_PAM_CLOSE_SESSION    = 0x00F5,
     SSS_PAM_CHAUTHTOK        = 0x00F6,
-    SSS_PAM_CHAUTHTOK_PRELIM = 0x00F6,
+    SSS_PAM_CHAUTHTOK_PRELIM = 0x00F7,
 
 };
 
-- 
1.6.5.2



More information about the sssd-devel mailing list