On Mon, Oct 05, 2009 at 06:48:14AM -0400, Simo Sorce wrote:
> On Mon, 2009-10-05 at 10:45 +0200, Sumit Bose wrote:
> > - currently PAM_AUTHTOK_EXPIRED is returned if the password is expired
> > regardless of the supplied password is correct or not. Would it be
> > better to return a different error if the password is wrong?
>
> We should return an auth error if the password is wrong I guess
> (assuming we know at the same that the password is wrong and the real
> password is expired).
>
> We shouldn't expose to the casual attacker that the password is expired.
This is fixed in the new version of 0001 by trying to get a change
password ticket.
>
> > - currently the pam_sss only asks the new password, because the
> > current/old password is already known. Typically pam modules are
> > asking for the current password for a second time (because the
> > password is not know anymore) and the for the new one. I think this
> > behaviour if often irritation people. Which version shall we use?
>
> Not sure, but as long as wee keep password change requests within the
> auth module we can avoid asking for the current password once again, the
> user just provided it, asking for it again adds nothing to the security
> of the operation.
>
> I have a questions though (haven't looked at the patch yet). Do you send
> back any message to the user before asking for the new password ?
>
Now a message is send to the user in the new version of 0003.
0002 is unchanged.
> Simo.
>
bye,
Sumit
>From 19ef4a95e16bca78abe49d71cdc78bf55e335fb3 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 2 Oct 2009 13:50:20 +0200
Subject: [PATCH 1/3] handle expired password during authentication
---
server/providers/krb5/krb5_child.c | 27 +++++++++++++++++++++++++--
1 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/server/providers/krb5/krb5_child.c
b/server/providers/krb5/krb5_child.c
index 6f69840..7649406 100644
--- a/server/providers/krb5/krb5_child.c
+++ b/server/providers/krb5/krb5_child.c
@@ -359,14 +359,37 @@ static errno_t tgt_req_child(int fd, struct krb5_req *kr)
}
kerr = get_and_save_tgt(kr, pass_str);
+
+ /* If the password is expired the KDC will always return
+ KRB5KDC_ERR_KEY_EXP regardless if the supplied password is correct or
+ not. In general the password can still be used to get a changepw ticket.
+ So we validate the password by trying to get a changepw ticket. */
+ if (kerr == KRB5KDC_ERR_KEY_EXP) {
+ kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,
+ pass_str, NULL, NULL, 0,
+ kr->krb5_ctx->changepw_principle,
+ kr->options);
+ krb5_free_cred_contents(kr->ctx, kr->creds);
+ if (kerr == 0) {
+ kerr = KRB5KDC_ERR_KEY_EXP;
+ }
+ }
+
memset(pass_str, 0, kr->pd->authtok_size);
talloc_zfree(pass_str);
memset(kr->pd->authtok, 0, kr->pd->authtok_size);
if (kerr != 0) {
KRB5_DEBUG(1, kerr);
- if (kerr == KRB5_KDC_UNREACH) {
- pam_status = PAM_AUTHINFO_UNAVAIL;
+ switch (kerr) {
+ case KRB5_KDC_UNREACH:
+ pam_status = PAM_AUTHINFO_UNAVAIL;
+ break;
+ case KRB5KDC_ERR_KEY_EXP:
+ pam_status = PAM_AUTHTOK_EXPIRED;
+ break;
+ default:
+ pam_status = PAM_SYSTEM_ERR;
}
}
--
1.6.2.5
>From 24b40cd4036ae32c7f2edc0d8615c9d13ba6ce1d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 2 Oct 2009 16:03:02 +0200
Subject: [PATCH 2/3] move password handling into subroutines
---
sss_client/pam_sss.c | 188 +++++++++++++++++++++++++++++++-------------------
1 files changed, 117 insertions(+), 71 deletions(-)
diff --git a/sss_client/pam_sss.c b/sss_client/pam_sss.c
index 9a1d441..eec25ab 100644
--- a/sss_client/pam_sss.c
+++ b/sss_client/pam_sss.c
@@ -651,6 +651,111 @@ static void eval_argv(pam_handle_t *pamh, int argc, const
char **argv,
return;
}
+static int get_authtok_for_authentication(pam_handle_t *pamh,
+ struct pam_items *pi,
+ uint32_t flags)
+{
+ int ret;
+
+ if (flags & FLAGS_USE_FIRST_PASS) {
+ pi->pam_authtok_type = SSS_AUTHTOK_TYPE_PASSWORD;
+ pi->pam_authtok = strdup(pi->pamstack_authtok);
+ if (pi->pam_authtok == NULL) {
+ D(("option use_first_pass set, but no password found"));
+ return PAM_BUF_ERR;
+ }
+ pi->pam_authtok_size = strlen(pi->pam_authtok);
+ } else {
+ ret = prompt_password(pamh, pi);
+ if (ret != PAM_SUCCESS) {
+ D(("failed to get password from user"));
+ return ret;
+ }
+
+ if (flags & FLAGS_FORWARD_PASS) {
+ ret = pam_set_item(pamh, PAM_AUTHTOK, pi->pam_authtok);
+ if (ret != PAM_SUCCESS) {
+ D(("Failed to set PAM_AUTHTOK [%s], "
+ "authtok may not be available for other modules",
+ pam_strerror(pamh,ret)));
+ }
+ }
+ }
+
+ return PAM_SUCCESS;
+}
+
+static int get_authtok_for_password_change(pam_handle_t *pamh,
+ struct pam_items *pi,
+ uint32_t flags,
+ int pam_flags)
+{
+ int ret;
+
+ /* we query for the old password during PAM_PRELIM_CHECK to make
+ * pam_sss work e.g. with pam_cracklib */
+ if (pam_flags & PAM_PRELIM_CHECK) {
+ if (getuid() != 0 && !(flags & FLAGS_USE_FIRST_PASS)) {
+ ret = prompt_password(pamh, pi);
+ if (ret != PAM_SUCCESS) {
+ D(("failed to get password from user"));
+ return ret;
+ }
+
+ ret = pam_set_item(pamh, PAM_OLDAUTHTOK, pi->pam_authtok);
+ if (ret != PAM_SUCCESS) {
+ D(("Failed to set PAM_OLDAUTHTOK [%s], "
+ "oldauthtok may not be available",
+ pam_strerror(pamh,ret)));
+ return ret;
+ }
+ }
+
+ return PAM_SUCCESS;
+ }
+
+ if (getuid() != 0) {
+ pi->pam_authtok_type = SSS_AUTHTOK_TYPE_PASSWORD;
+ pi->pam_authtok = strdup(pi->pamstack_oldauthtok);
+ if (pi->pam_authtok == NULL) {
+ D(("no password found for chauthtok"));
+ return PAM_BUF_ERR;
+ }
+ pi->pam_authtok_size = strlen(pi->pam_authtok);
+ } else {
+ pi->pam_authtok_type = SSS_AUTHTOK_TYPE_EMPTY;
+ pi->pam_authtok = NULL;
+ pi->pam_authtok_size = 0;
+ }
+
+ if (flags & FLAGS_USE_AUTHTOK) {
+ pi->pam_newauthtok_type = SSS_AUTHTOK_TYPE_PASSWORD;
+ pi->pam_newauthtok = strdup(pi->pamstack_authtok);
+ if (pi->pam_newauthtok == NULL) {
+ D(("option use_authtok set, but no new password found"));
+ return PAM_BUF_ERR;
+ }
+ pi->pam_newauthtok_size = strlen(pi->pam_newauthtok);
+ } else {
+ ret = prompt_new_password(pamh, pi);
+ if (ret != PAM_SUCCESS) {
+ D(("failed to get new password from user"));
+ return ret;
+ }
+
+ if (flags & FLAGS_FORWARD_PASS) {
+ ret = pam_set_item(pamh, PAM_AUTHTOK, pi->pam_newauthtok);
+ if (ret != PAM_SUCCESS) {
+ D(("Failed to set PAM_AUTHTOK [%s], "
+ "oldauthtok may not be available",
+ pam_strerror(pamh,ret)));
+ }
+ }
+ }
+
+ return PAM_SUCCESS;
+}
+
static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh,
int pam_flags, int argc, const char **argv)
{
@@ -673,82 +778,23 @@ static int pam_sss(enum sss_cli_command task,
pam_handle_t *pamh,
switch(task) {
case SSS_PAM_AUTHENTICATE:
- if (flags & FLAGS_USE_FIRST_PASS) {
- pi.pam_authtok_type = SSS_AUTHTOK_TYPE_PASSWORD;
- pi.pam_authtok = strdup(pi.pamstack_authtok);
- if (pi.pam_authtok == NULL) {
- D(("option use_first_pass set, but no password found"));
- return PAM_BUF_ERR;
- }
- pi.pam_authtok_size = strlen(pi.pam_authtok);
- } else {
- prompt_password(pamh, &pi);
-
- if (flags & FLAGS_FORWARD_PASS) {
- ret = pam_set_item(pamh, PAM_AUTHTOK, pi.pam_authtok);
- if (ret != PAM_SUCCESS) {
- D(("Failed to set PAM_AUTHTOK [%s], "
- "authtok may not be available for other modules",
- pam_strerror(pamh,ret)));
- }
- }
+ 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:
- /* we query for the old password during PAM_PRELIM_CHECK to make
- * pam_sss work e.g. with pam_cracklib */
- if (pam_flags & PAM_PRELIM_CHECK) {
- if (getuid() != 0 && !(flags & FLAGS_USE_FIRST_PASS)) {
- prompt_password(pamh, &pi);
-
- ret = pam_set_item(pamh, PAM_OLDAUTHTOK, pi.pam_authtok);
- if (ret != PAM_SUCCESS) {
- D(("Failed to set PAM_OLDAUTHTOK [%s], "
- "oldauthtok may not be available",
- pam_strerror(pamh,ret)));
- return ret;
- }
- }
-
- return PAM_SUCCESS;
- }
-
- if (getuid() != 0) {
- pi.pam_authtok_type = SSS_AUTHTOK_TYPE_PASSWORD;
- pi.pam_authtok = strdup(pi.pamstack_oldauthtok);
- if (pi.pam_authtok == NULL) {
- D(("no password found for chauthtok"));
- return PAM_BUF_ERR;
- }
- pi.pam_authtok_size = strlen(pi.pam_authtok);
- } else {
- pi.pam_authtok_type = SSS_AUTHTOK_TYPE_EMPTY;
- pi.pam_authtok = NULL;
- pi.pam_authtok_size = 0;
+ 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 (flags & FLAGS_USE_AUTHTOK) {
- pi.pam_newauthtok_type = SSS_AUTHTOK_TYPE_PASSWORD;
- pi.pam_newauthtok = strdup(pi.pamstack_authtok);
- if (pi.pam_newauthtok == NULL) {
- D(("option use_authtok set, but no new password found"));
- return PAM_BUF_ERR;
- }
- pi.pam_newauthtok_size = strlen(pi.pam_newauthtok);
- } else {
- prompt_new_password(pamh, &pi);
-
- if (flags & FLAGS_FORWARD_PASS) {
- ret = pam_set_item(pamh, PAM_AUTHTOK, pi.pam_newauthtok);
- if (ret != PAM_SUCCESS) {
- D(("Failed to set PAM_AUTHTOK [%s], "
- "oldauthtok may not be available",
- pam_strerror(pamh,ret)));
- }
- }
+ if (pam_flags & PAM_PRELIM_CHECK) {
+ return ret;
}
-
break;
case SSS_PAM_ACCT_MGMT:
case SSS_PAM_SETCRED:
--
1.6.2.5
>From 16fee9a04ed985fd9e7fe97416b2745c28094501 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 5 Oct 2009 10:23:38 +0200
Subject: [PATCH 3/3] ask for new password if password is expired
---
sss_client/pam_sss.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/sss_client/pam_sss.c b/sss_client/pam_sss.c
index eec25ab..5ac9e2f 100644
--- a/sss_client/pam_sss.c
+++ b/sss_client/pam_sss.c
@@ -158,6 +158,21 @@ static size_t add_string_item(enum pam_item_type type,
const char *str,
return rp;
}
+static void overwrite_and_free_authtoks(struct pam_items *pi)
+{
+ if (pi->pam_authtok != NULL) {
+ _pam_overwrite_n((void *)pi->pam_authtok, pi->pam_authtok_size);
+ free((void *)pi->pam_authtok);
+ pi->pam_authtok = NULL;
+ }
+
+ if (pi->pam_newauthtok != NULL) {
+ _pam_overwrite_n((void *)pi->pam_newauthtok, pi->pam_newauthtok_size);
+ free((void *)pi->pam_newauthtok);
+ pi->pam_newauthtok = NULL;
+ }
+}
+
static int pack_message_v3(struct pam_items *pi, size_t *size,
uint8_t **buffer) {
int len;
@@ -210,16 +225,10 @@ static int pack_message_v3(struct pam_items *pi, size_t
*size,
rp += add_authtok_item(PAM_ITEM_AUTHTOK, pi->pam_authtok_type,
pi->pam_authtok, pi->pam_authtok_size, &buf[rp]);
- _pam_overwrite_n((void *)pi->pam_authtok, pi->pam_authtok_size);
- free((void *)pi->pam_authtok);
- pi->pam_authtok = NULL;
rp += add_authtok_item(PAM_ITEM_NEWAUTHTOK, pi->pam_newauthtok_type,
pi->pam_newauthtok, pi->pam_newauthtok_size,
&buf[rp]);
- _pam_overwrite_n((void *)pi->pam_newauthtok, pi->pam_newauthtok_size);
- free((void *)pi->pam_newauthtok);
- pi->pam_newauthtok = NULL;
((uint32_t *)(&buf[rp]))[0] = END_OF_PAM_REQUEST;
rp += sizeof(uint32_t);
@@ -806,7 +815,31 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t
*pamh,
return PAM_SYSTEM_ERR;
}
- return send_and_receive(pamh, &pi, task);
+ ret = send_and_receive(pamh, &pi, task);
+
+ if (ret == PAM_AUTHTOK_EXPIRED && task == SSS_PAM_AUTHENTICATE) {
+ D(("Authtoken expired, trying to change it"));
+ ret = do_pam_conversation(pamh, PAM_ERROR_MSG,
+ _("Password has expired."), NULL, NULL);
+ if (ret != PAM_SUCCESS) {
+ D(("do_pam_conversation failed."));
+ return PAM_SYSTEM_ERR;
+ }
+
+ pi.pamstack_oldauthtok = pi.pam_authtok;
+ 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;
+ }
+
+ ret = send_and_receive(pamh, &pi, SSS_PAM_CHAUTHTOK);
+ }
+
+ overwrite_and_free_authtoks(&pi);
+
+ return ret;
}
PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc,
--
1.6.2.5
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
