[SSSD] [PATCH] Improvements for LDAP Password Policy support
Sumit Bose
sbose at redhat.com
Fri Mar 19 16:40:31 UTC 2010
On Thu, Mar 18, 2010 at 03:12:14PM +0100, Ralf Haferkamp wrote:
> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce:
> > On Wed, 17 Mar 2010 15:33:38 +0100
> >
> > Ralf Haferkamp <rhafer at suse.de> wrote:
> > > Hi,
> > >
> > > here's another set of enhancements to the LDAP Password Policy
> > > support in the PAM module and the LDAP backend. The PAM module now
> > > issues warning when either the grace counter or the expire counter
> > > of the LDAP Ppolicy Control in > 0.
> > >
> > > Addtionally I changed the detection for Ppolicy support of the LDAP
> > > Server a bit. If the Server returned the Ppolicy Control in the Bind
> > > Response ppolicy support is assumed.
> > >
> > > I left the original check for "pwdAttribute" intact, though I think
> > > it doesn't make a lot of sense. The "pwdAttribute" LDAP Attribute is
> > > usually not part of the user's entry, it's part of the Entry that
> > > contains the Policy. Addtionally it might be protected by ACLs and
> > > not be returned for anonymous (without losing any functionality).
> >
> > Ralf,
> > patch looks mostly good, but there are some heavy coding style
> > violations.
> >
> > if ( condition ){ is not good, it should be: is (condition) {
> >
> > same for some functions foo( ccc ); is not good, use foo(ccc);
> >
> > Ie, no space in parenthesis, a space only after keywords, and
> > always before the opening {
> >
> > Don't use ++x, but x++ if possible.
> >
> > Also there is at least one place where the return of talloc is not
> > checked.
> >
> > Finally please try to keep the 80 columns limit where possible.
>
> Updated patch attached. I think I fixed the coding style issues.
> Additionally I just noticed that my orginal patch broke password resets
> via LDAP password policies. This should be fixed now as well.
>
> regards,
> Ralf
I have tested this new version and didn't find any issues. Password
resets are working, too.
It would be nice if you can fix the whitespace errors:
Applying: Improvements for LDAP Password Policy support
/home/sbose/wip/freeipa/sssd/.git/rebase-apply/patch:187: trailing whitespace.
DEBUG(4,
/home/sbose/wip/freeipa/sssd/.git/rebase-apply/patch:291: trailing whitespace.
ret = snprintf(user_msg, sizeof(user_msg),
/home/sbose/wip/freeipa/sssd/.git/rebase-apply/patch:300: trailing whitespace.
/home/sbose/wip/freeipa/sssd/.git/rebase-apply/patch:337: trailing whitespace.
ret = snprintf(user_msg, sizeof(user_msg),
/home/sbose/wip/freeipa/sssd/.git/rebase-apply/patch:344: trailing whitespace.
warning: squelched 2 whitespace errors
warning: 7 lines add whitespace errors.
bye,
Sumit
> From 0b1f123f20f4944425d70b02bb346b78176d0abe Mon Sep 17 00:00:00 2001
> From: Ralf Haferkamp <rhafer at suse.de>
> Date: Fri, 12 Mar 2010 10:54:40 +0100
> Subject: [PATCH] Improvements for LDAP Password Policy support
>
> Display warnings about remaining grace logins and password
> expiration to the user, when LDAP Password Policies are used.
>
> Improved detection if LDAP Password policies are supported by
> LDAP Server.
> ---
> src/providers/ldap/ldap_auth.c | 52 +++++++++++++++++-
> src/providers/ldap/sdap.h | 5 ++
> src/providers/ldap/sdap_async.h | 6 ++-
> src/providers/ldap/sdap_async_connection.c | 53 +++++++++++++++----
> src/sss_client/pam_sss.c | 82 ++++++++++++++++++++++++++++
> src/sss_client/sss_cli.h | 23 ++++++---
> 6 files changed, 201 insertions(+), 20 deletions(-)
>
> diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
> index 5228703..b572bf2 100644
> --- a/src/providers/ldap/ldap_auth.c
> +++ b/src/providers/ldap/ldap_auth.c
> @@ -7,6 +7,7 @@
> Sumit Bose <sbose at redhat.com>
>
> Copyright (C) 2008 Red Hat
> + Copyright (C) 2010, rhafer at suse.de, Novell Inc.
>
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> @@ -135,6 +136,39 @@ static errno_t check_pwexpire_shadow(struct spwd *spwd, time_t now,
> return EOK;
> }
>
> +static errno_t check_pwexpire_ldap(struct pam_data *pd,
> + struct sdap_ppolicy_data *ppolicy,
> + enum sdap_result *result)
> +{
> + if (ppolicy->grace > 0 || ppolicy->expire > 0) {
> + uint32_t *data;
> + uint32_t *ptr;
> +
> + data = talloc_size(pd, 2* sizeof(uint32_t));
> + if (*data == NULL) {
> + DEBUG(1, ("talloc_size failed.\n"));
> + return ENOMEM;
> + }
> +
> + ptr = data;
> + if (ppolicy->grace > 0) {
> + *ptr = SSS_PAM_USER_INFO_GRACE_LOGIN;
> + ptr++;
> + *ptr = ppolicy->grace;
> + } else if (ppolicy->expire > 0) {
> + *ptr = SSS_PAM_USER_INFO_EXPIRE_WARN;
> + ptr++;
> + *ptr = ppolicy->expire;
> + }
> +
> + pam_add_response(pd, SSS_PAM_USER_INFO, 2* sizeof(uint32_t),
> + (uint8_t*)data);
> + }
> +
> + *result = SDAP_AUTH_SUCCESS;
> + return EOK;
> +}
> +
> static errno_t string_to_shadowpw_days(const char *s, long *d)
> {
> long l;
> @@ -569,8 +603,15 @@ static void auth_bind_user_done(struct tevent_req *subreq)
> struct auth_state *state = tevent_req_data(req,
> struct auth_state);
> int ret;
> -
> - ret = sdap_auth_recv(subreq, &state->result);
> + struct sdap_ppolicy_data *ppolicy;
> +
> + ret = sdap_auth_recv(subreq, state, &state->result, &ppolicy);
> + if (ppolicy != NULL) {
> + DEBUG(9,("Found ppolicy data, "
> + "assuming LDAP password policies are active.\n"));
> + state->pw_expire_type = PWEXPIRE_LDAP_PASSWORD_POLICY;
> + state->pw_expire_data = ppolicy;
> + }
> talloc_zfree(subreq);
> if (ret) {
> tevent_req_error(req, ret);
> @@ -960,6 +1001,13 @@ static void sdap_pam_auth_done(struct tevent_req *req)
> }
> break;
> case PWEXPIRE_LDAP_PASSWORD_POLICY:
> + ret = check_pwexpire_ldap(state->pd, pw_expire_data, &result);
> + if (ret != EOK) {
> + DEBUG(1, ("check_pwexpire_ldap failed.\n"));
> + state->pd->pam_status = PAM_SYSTEM_ERR;
> + goto done;
> + }
> + break;
> case PWEXPIRE_NONE:
> break;
> default:
> diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
> index 007185f..f0e345e 100644
> --- a/src/providers/ldap/sdap.h
> +++ b/src/providers/ldap/sdap.h
> @@ -85,6 +85,11 @@ struct sdap_service {
> char *uri;
> };
>
> +struct sdap_ppolicy_data {
> + int grace;
> + int expire;
> +};
> +
> #define SYSDB_SHADOWPW_LASTCHANGE "shadowLastChange"
> #define SYSDB_SHADOWPW_MIN "shadowMin"
> #define SYSDB_SHADOWPW_MAX "shadowMax"
> diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h
> index 3c52d23..888df6b 100644
> --- a/src/providers/ldap/sdap_async.h
> +++ b/src/providers/ldap/sdap_async.h
> @@ -76,7 +76,11 @@ struct tevent_req *sdap_auth_send(TALLOC_CTX *memctx,
> const char *user_dn,
> const char *authtok_type,
> struct dp_opt_blob authtok);
> -int sdap_auth_recv(struct tevent_req *req, enum sdap_result *result);
> +
> +int sdap_auth_recv(struct tevent_req *req,
> + TALLOC_CTX *memctx,
> + enum sdap_result *result,
> + struct sdap_ppolicy_data **ppolicy);
>
> struct tevent_req *sdap_get_initgr_send(TALLOC_CTX *memctx,
> struct tevent_context *ev,
> diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
> index fe8a501..4a75c1d 100644
> --- a/src/providers/ldap/sdap_async_connection.c
> +++ b/src/providers/ldap/sdap_async_connection.c
> @@ -4,6 +4,7 @@
> Async LDAP Helper routines
>
> Copyright (C) Simo Sorce <ssorce at redhat.com> - 2009
> + Copyright (C) 2010, rhafer at suse.de, Novell Inc.
>
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> @@ -278,6 +279,7 @@ struct simple_bind_state {
> struct sdap_op *op;
>
> struct sdap_msg *reply;
> + struct sdap_ppolicy_data *ppolicy;
> int result;
> };
>
> @@ -401,6 +403,7 @@ static void simple_bind_done(struct sdap_op *op,
>
> if (response_controls == NULL) {
> DEBUG(5, ("Server returned no controls.\n"));
> + state->ppolicy = NULL;
> } else {
> for (c = 0; response_controls[c] != NULL; c++) {
> DEBUG(9, ("Server returned control [%s].\n",
> @@ -420,12 +423,30 @@ static void simple_bind_done(struct sdap_op *op,
> DEBUG(7, ("Password Policy Response: expire [%d] grace [%d] "
> "error [%s].\n", pp_expire, pp_grace,
> ldap_passwordpolicy_err2txt(pp_error)));
> -
> - if ((state->result == LDAP_SUCCESS &&
> - (pp_error == PP_changeAfterReset || pp_grace > 0)) ||
> - (state->result == LDAP_INVALID_CREDENTIALS &&
> - pp_error == PP_passwordExpired ) ) {
> - DEBUG(4, ("User must set a new password.\n"));
> + state->ppolicy = talloc(state, struct sdap_ppolicy_data);
> + if (state->ppolicy == NULL) {
> + DEBUG(1, ("talloc failed.\n"));
> + ret = ENOMEM;
> + goto done;
> + }
> + state->ppolicy->grace = pp_grace;
> + state->ppolicy->expire = pp_expire;
> + if (state->result == LDAP_SUCCESS) {
> + if (pp_error == PP_changeAfterReset) {
> + DEBUG(4, ("Password was reset. "
> + "User must set a new password.\n"));
> + state->result = LDAP_X_SSSD_PASSWORD_EXPIRED;
> + } else if (pp_grace > 0) {
> + DEBUG(4, ("Password expired. "
> + "[%d] grace logins remaining.\n", pp_grace));
> + } else if (pp_expire > 0) {
> + DEBUG(4, ("Password will expire in [%d] seconds.\n",
> + pp_expire));
> + }
> + } else if (state->result == LDAP_INVALID_CREDENTIALS &&
> + pp_error == PP_passwordExpired) {
> + DEBUG(4,
> + ("Password expired user must set a new password.\n"));
> state->result = LDAP_X_SSSD_PASSWORD_EXPIRED;
> }
> }
> @@ -446,7 +467,10 @@ done:
> }
> }
>
> -static int simple_bind_recv(struct tevent_req *req, int *ldaperr)
> +static int simple_bind_recv(struct tevent_req *req,
> + TALLOC_CTX *memctx,
> + int *ldaperr,
> + struct sdap_ppolicy_data **ppolicy)
> {
> struct simple_bind_state *state = tevent_req_data(req,
> struct simple_bind_state);
> @@ -455,6 +479,7 @@ static int simple_bind_recv(struct tevent_req *req, int *ldaperr)
> TEVENT_REQ_RETURN_ON_ERROR(req);
>
> *ldaperr = state->result;
> + *ppolicy = talloc_steal(memctx, state->ppolicy);
> return EOK;
> }
>
> @@ -704,6 +729,7 @@ int sdap_kinit_recv(struct tevent_req *req, enum sdap_result *result)
> struct sdap_auth_state {
> const char *user_dn;
> struct berval pw;
> + struct sdap_ppolicy_data *ppolicy;
>
> int result;
> bool is_sasl;
> @@ -766,8 +792,9 @@ static void sdap_auth_done(struct tevent_req *subreq)
>
> if (state->is_sasl) {
> ret = sasl_bind_recv(subreq, &state->result);
> + state->ppolicy = NULL;
> } else {
> - ret = simple_bind_recv(subreq, &state->result);
> + ret = simple_bind_recv(subreq, state, &state->result, &state->ppolicy);
> }
> if (ret != EOK) {
> tevent_req_error(req, ret);
> @@ -777,7 +804,10 @@ static void sdap_auth_done(struct tevent_req *subreq)
> tevent_req_done(req);
> }
>
> -int sdap_auth_recv(struct tevent_req *req, enum sdap_result *result)
> +int sdap_auth_recv(struct tevent_req *req,
> + TALLOC_CTX *memctx,
> + enum sdap_result *result,
> + struct sdap_ppolicy_data **ppolicy)
> {
> struct sdap_auth_state *state = tevent_req_data(req,
> struct sdap_auth_state);
> @@ -785,6 +815,9 @@ int sdap_auth_recv(struct tevent_req *req, enum sdap_result *result)
> *result = SDAP_ERROR;
> TEVENT_REQ_RETURN_ON_ERROR(req);
>
> + if (ppolicy != NULL) {
> + *ppolicy = talloc_steal(memctx, state->ppolicy);
> + }
> switch (state->result) {
> case LDAP_SUCCESS:
> *result = SDAP_AUTH_SUCCESS;
> @@ -1078,7 +1111,7 @@ static void sdap_cli_auth_done(struct tevent_req *subreq)
> enum sdap_result result;
> int ret;
>
> - ret = sdap_auth_recv(subreq, &result);
> + ret = sdap_auth_recv(subreq, NULL, &result, NULL);
> talloc_zfree(subreq);
> if (ret) {
> tevent_req_error(req, ret);
> diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
> index 2ba6f15..013a4e5 100644
> --- a/src/sss_client/pam_sss.c
> +++ b/src/sss_client/pam_sss.c
> @@ -3,6 +3,7 @@
> Sumit Bose <sbose at redhat.com>
>
> Copyright (C) 2009 Red Hat
> + Copyright (C) 2010, rhafer at suse.de, Novell Inc.
>
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU Lesser General Public License as published by
> @@ -436,6 +437,81 @@ static int user_info_offline_auth(pam_handle_t *pamh, size_t buflen,
> return PAM_SUCCESS;
> }
>
> +static int user_info_grace_login(pam_handle_t *pamh,
> + size_t buflen,
> + uint8_t *buf)
> +{
> + int ret;
> + uint32_t grace;
> + char user_msg[256];
> +
> + if (buflen != 2* sizeof(uint32_t)) {
> + D(("User info response data has the wrong size"));
> + return PAM_BUF_ERR;
> + }
> + memcpy(&grace, buf + sizeof(uint32_t), sizeof(uint32_t));
> + ret = snprintf(user_msg, sizeof(user_msg),
> + _("Your password has expired. "
> + "You have %d grace login(s) remaining."),
> + grace);
> + if (ret < 0 || ret >= sizeof(user_msg)) {
> + D(("snprintf failed."));
> + return PAM_SYSTEM_ERR;
> + }
> + ret = do_pam_conversation(pamh, PAM_TEXT_INFO, user_msg, NULL, NULL);
> +
> + if (ret != PAM_SUCCESS) {
> + D(("do_pam_conversation failed."));
> + return PAM_SYSTEM_ERR;
> + }
> +
> + return PAM_SUCCESS;
> +}
> +
> +#define MINSEC 60
> +#define HOURSEC (60*MINSEC)
> +#define DAYSEC (24*HOURSEC)
> +static int user_info_expire_warn(pam_handle_t *pamh,
> + size_t buflen,
> + uint8_t *buf)
> +{
> + int ret;
> + uint32_t expire;
> + char user_msg[256];
> + const char* unit="second(s)";
> +
> + if (buflen != 2* sizeof(uint32_t)) {
> + D(("User info response data has the wrong size"));
> + return PAM_BUF_ERR;
> + }
> + memcpy(&expire, buf + sizeof(uint32_t), sizeof(uint32_t));
> + if (expire >= DAYSEC) {
> + expire /= DAYSEC;
> + unit = "day(s)";
> + } else if (expire >= HOURSEC) {
> + expire /= HOURSEC;
> + unit = "hour(s)";
> + } else if (expire >= MINSEC) {
> + expire /= MINSEC;
> + unit = "minute(s)";
> + }
> +
> + ret = snprintf(user_msg, sizeof(user_msg),
> + _("Your password will expire in %d %s."), expire, unit);
> + if (ret < 0 || ret >= sizeof(user_msg)) {
> + D(("snprintf failed."));
> + return PAM_SYSTEM_ERR;
> + }
> + ret = do_pam_conversation(pamh, PAM_TEXT_INFO, user_msg, NULL, NULL);
> +
> + if (ret != PAM_SUCCESS) {
> + D(("do_pam_conversation failed."));
> + return PAM_SYSTEM_ERR;
> + }
> +
> + return PAM_SUCCESS;
> +}
> +
> static int user_info_offline_auth_delayed(pam_handle_t *pamh, size_t buflen,
> uint8_t *buf)
> {
> @@ -563,6 +639,12 @@ static int eval_user_info_response(pam_handle_t *pamh, size_t buflen,
> case SSS_PAM_USER_INFO_OFFLINE_AUTH:
> ret = user_info_offline_auth(pamh, buflen, buf);
> break;
> + case SSS_PAM_USER_INFO_GRACE_LOGIN:
> + ret = user_info_grace_login(pamh, buflen, buf);
> + break;
> + case SSS_PAM_USER_INFO_EXPIRE_WARN:
> + ret = user_info_expire_warn(pamh, buflen, buf);
> + break;
> case SSS_PAM_USER_INFO_OFFLINE_AUTH_DELAYED:
> ret = user_info_offline_auth_delayed(pamh, buflen, buf);
> break;
> diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h
> index 2edd158..e1420c8 100644
> --- a/src/sss_client/sss_cli.h
> +++ b/src/sss_client/sss_cli.h
> @@ -377,13 +377,22 @@ enum user_info_type {
> * possible to change the password while
> * the system is offline. This message
> * is generated by the PAM responder. */
> - SSS_PAM_USER_INFO_CHPASS_ERROR /**< Tell the user that a password change
> - * failed and optionally give a reason.
> - * @param Size of the message as unsigned
> - * 32-bit integer value. A value of 0
> - * indicates that no message is following.
> - * @param String with the specified
> - * length. */
> + SSS_PAM_USER_INFO_CHPASS_ERROR, /**< Tell the user that a password change
> + * failed and optionally give a reason.
> + * @param Size of the message as unsigned
> + * 32-bit integer value. A value of 0
> + * indicates that no message is following.
> + * @param String with the specified
> + * length. */
> + SSS_PAM_USER_INFO_GRACE_LOGIN, /**< Warn the user that the password is
> + * expired and inform about the remaining
> + * number of grace logins.
> + * @param The number of remaining grace
> + * logins as uint32_t */
> + SSS_PAM_USER_INFO_EXPIRE_WARN /**< Warn the user that the password will
> + * expire soon.
> + * @param Number of seconds before the user's
> + * password will expire. */
> };
> /**
> * @}
> --
> 1.6.4.2
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
More information about the sssd-devel
mailing list