On (20/02/15 14:23), Pavel Reichl wrote:
> On 02/19/2015 06:16 PM, Sumit Bose wrote:
>> On Tue, Feb 17, 2015 at 10:56:43PM +0100, Pavel Reichl wrote:
>>> Hello,
>>>
>>> please see attached patches resolving
>>>
https://fedorahosted.org/sssd/ticket/2167
>>>
>>> 1st patch contains some minor refactoring useful for 2nd patch.
>>>
>>> How to test:
>>> On OpenLDAP following attributes should be set:
>>> * shadowMax - how many days after shadowLastChange will password expire
>>> * shadowWarning - how many days before password expires should warning be
>>> displayed.
>>>
>>> In sssd.conf domain section should contain something similar to:
>>>
>>> access_provider = ldap
>>> ldap_access_order = expire, expire_policy
>>> ldap_account_expire_policy = shadow
>>> ldap_pwd_policy = shadow
>>>
>>> Thanks!
>> Hi Pavel,
>>
>> I've tested the patch with FreeIPA and ldap_pwd_policy = mit_kerberos
>> and it is working as expected. Nevertheless I have a few comments.
>>
>> First, I would recommend to rename the option 'expire_policy' is
>> ambiguous because it is not clear which expiration is meant, account or
>> password. Since it is tightly related to ldap_pwd_policy I would prefer
>> 'pwd_policy' or 'pwd_expire_policy' ('ldap_pwd_policy'
might not be a
>> good choice be because then we have an option name and an option value
>> with the same name which might be confusing).
> done
>> My main comment is about moving the check if configured. Although I see
>> the point of doing the check only once I think it should be done in the
>> auth and access provider. Since it is run from the cache there shouldn't
>> be any great delay for the user. This will make your patch easier and
>> the check is still done for applications only calling pam_authenticate.
> I did it to avoid duplicate output for services like ssh when using
> passwords. I should have consult that.
>> In the man page you should explicitly mention that ldap_pwd_policy must
>> be set to make the new option work, because the default for
>> ldap_pwd_policy is 'none'.
> dobe
>> bye,
>> Sumit
> Thanks for comments and testing. Please see updated patchset (only second
> patch differs).
>
>From 8551fd21aa91be1f9632d08c86a30d433a104db0 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl(a)redhat.com>
> Date: Mon, 16 Feb 2015 18:56:25 -0500
> Subject: [PATCH 1/2] SDAP: refactor pwexpire policy
>
> Move part of pwexpire policy code to a separate function.
>
> Relates to:
>
https://fedorahosted.org/sssd/ticket/2167
> ---
> src/providers/ldap/ldap_auth.c | 76 ++++++++++++++++++++++++------------------
> src/providers/ldap/ldap_auth.h | 46 +++++++++++++++++++++++++
> 2 files changed, 90 insertions(+), 32 deletions(-)
> create mode 100644 src/providers/ldap/ldap_auth.h
>
> diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
> index
5a40c1359f138c42eb915e873fe21a50ab038e81..1f1add53713201c83a13928ebb66f6bd130d7bac 100644
> --- a/src/providers/ldap/ldap_auth.c
> +++ b/src/providers/ldap/ldap_auth.c
> @@ -46,16 +46,10 @@
> #include "providers/ldap/ldap_common.h"
> #include "providers/ldap/sdap_async.h"
> #include "providers/ldap/sdap_async_private.h"
> +#include "providers/ldap/ldap_auth.h"
>
> #define LDAP_PWEXPIRE_WARNING_TIME 0
>
> -enum pwexpire {
> - PWEXPIRE_NONE = 0,
> - PWEXPIRE_LDAP_PASSWORD_POLICY,
> - PWEXPIRE_KERBEROS,
> - PWEXPIRE_SHADOW
> -};
> -
> static errno_t add_expired_warning(struct pam_data *pd, long exp_time)
> {
> int ret;
> @@ -248,10 +242,41 @@ done:
> return ret;
> }
>
> -static errno_t find_password_expiration_attributes(TALLOC_CTX *mem_ctx,
> - const struct ldb_message *msg,
> - struct dp_option *opts,
> - enum pwexpire *type, void **data)
> +errno_t check_pwexpire_policy(enum pwexpire pw_expire_type,
> + void *pw_expire_data,
> + struct pam_data *pd,
> + int pwd_expiration_warning)
> +{
> + errno_t ret;
> +
> + switch (pw_expire_type) {
> + case PWEXPIRE_SHADOW:
> + ret = check_pwexpire_shadow(pw_expire_data, time(NULL), pd);
> + break;
> + case PWEXPIRE_KERBEROS:
> + ret = check_pwexpire_kerberos(pw_expire_data, time(NULL), pd,
> + pwd_expiration_warning);
> + break;
> + case PWEXPIRE_LDAP_PASSWORD_POLICY:
> + ret = check_pwexpire_ldap(pd, pw_expire_data,
> + pwd_expiration_warning);
> + break;
> + case PWEXPIRE_NONE:
> + ret = EOK;
> + break;
> + default:
> + DEBUG(SSSDBG_CRIT_FAILURE, "Unknown password expiration
type.\n");
> + ret = -1;
> + }
> +
> + return ret;
> +}
> +
> +static errno_t
> +find_password_expiration_attributes(TALLOC_CTX *mem_ctx,
> + const struct ldb_message *msg,
> + struct dp_option *opts,
> + enum pwexpire *type, void **data)
> {
> const char *mark;
> const char *val;
> @@ -492,7 +517,7 @@ static int get_user_dn_recv(TALLOC_CTX *mem_ctx, struct
tevent_req *req,
> return EOK;
> }
>
> -static int get_user_dn(TALLOC_CTX *memctx,
> +int get_user_dn(TALLOC_CTX *memctx,
> struct sss_domain_info *domain,
> struct sdap_options *opts,
> const char *username,
> @@ -998,7 +1023,7 @@ static void sdap_auth4chpass_done(struct tevent_req *req)
> case PWEXPIRE_NONE:
> break;
> default:
> - DEBUG(SSSDBG_CRIT_FAILURE, "Unknow pasword expiration
type.\n");
> + DEBUG(SSSDBG_CRIT_FAILURE, "Unknown password expiration
type.\n");
> state->pd->pam_status = PAM_SYSTEM_ERR;
> goto done;
> }
> @@ -1247,25 +1272,12 @@ static void sdap_pam_auth_done(struct tevent_req *req)
> talloc_zfree(req);
>
> if (ret == EOK) {
> - switch (pw_expire_type) {
> - case PWEXPIRE_SHADOW:
> - ret = check_pwexpire_shadow(pw_expire_data, time(NULL), state->pd);
> - break;
> - case PWEXPIRE_KERBEROS:
> - ret = check_pwexpire_kerberos(pw_expire_data, time(NULL),
> - state->pd,
> -
be_ctx->domain->pwd_expiration_warning);
> - break;
> - case PWEXPIRE_LDAP_PASSWORD_POLICY:
> - ret = check_pwexpire_ldap(state->pd, pw_expire_data,
> -
be_ctx->domain->pwd_expiration_warning);
> - break;
> - case PWEXPIRE_NONE:
> - break;
> - default:
> - DEBUG(SSSDBG_CRIT_FAILURE, "Unknow pasword expiration
type.\n");
> - state->pd->pam_status = PAM_SYSTEM_ERR;
> - goto done;
> + ret = check_pwexpire_policy(pw_expire_type, pw_expire_data, state->pd,
> + be_ctx->domain->pwd_expiration_warning);
> + if (ret == -1) {
> + /* Unknown password expiration type. */
> + state->pd->pam_status = PAM_SYSTEM_ERR;
> + goto done;
> }
> }
>
> diff --git a/src/providers/ldap/ldap_auth.h b/src/providers/ldap/ldap_auth.h
> new file mode 100644
> index
0000000000000000000000000000000000000000..1038ddc7d136db925f2e9416c848e4505c45f4cf
> --- /dev/null
> +++ b/src/providers/ldap/ldap_auth.h
> @@ -0,0 +1,46 @@
> +/*
> + SSSD
> +
> + Copyright (C) Pavel Reichl <preichl(a)redhat.com>
> +
> + 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
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <
http://www.gnu.org/licenses/>.
> +*/
> +
> +#ifndef _LDAP_AUTH_H_
> +#define _LDAP_AUTH_H_
> +
> +#include "config.h"
> +
> +enum pwexpire {
> + PWEXPIRE_NONE = 0,
> + PWEXPIRE_LDAP_PASSWORD_POLICY,
> + PWEXPIRE_KERBEROS,
> + PWEXPIRE_SHADOW
> +};
> +
> +int get_user_dn(TALLOC_CTX *memctx,
> + struct sss_domain_info *domain,
> + struct sdap_options *opts,
> + const char *username,
> + char **user_dn,
> + enum pwexpire *user_pw_expire_type,
> + void **user_pw_expire_data);
> +
> +errno_t check_pwexpire_policy(enum pwexpire pw_expire_type,
> + void *pw_expire_data,
> + struct pam_data *pd,
> + errno_t checkb);
> +
> +
> +#endif /* _LDAP_AUTH_H_ */
> --
> 2.1.0
>
>From add61e4819be0c013da2c452cc86d0d98d803440 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl(a)redhat.com>
> Date: Wed, 18 Feb 2015 01:03:40 -0500
> Subject: [PATCH 2/2] SDAP: enable change phase of pw expire policy check
>
> Implement new option which does checking password expiration policy
> in accounting phase.
>
> This allows SSSD to issue shadow expiration warning even if alternate
> authentication method is used.
>
> Resolves:
>
https://fedorahosted.org/sssd/ticket/2167
> ---
> Makefile.am | 1 +
> src/man/sssd-ldap.5.xml | 12 +++++++++++
> src/providers/ldap/ldap_auth.c | 1 +
> src/providers/ldap/ldap_init.c | 3 +++
> src/providers/ldap/sdap_access.c | 43 +++++++++++++++++++++++++++++++++++++++-
> src/providers/ldap/sdap_access.h | 2 ++
> 6 files changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index
70278550fd22b820528f9613d30e11a723ea6c5c..35e3b39027f1163078a61a0528df2306367d6187 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -562,6 +562,7 @@ dist_noinst_HEADERS = \
> src/providers/ldap/sdap_autofs.h \
> src/providers/ldap/sdap_id_op.h \
> src/providers/ldap/ldap_opts.h \
> + src/providers/ldap/ldap_auth.h \
> src/providers/ldap/sdap_range.h \
> src/providers/ldap/sdap_users.h \
> src/providers/ldap/sdap_dyndns.h \
This change should be part of 1st patch otherwise our CI will complain that
make distcheck fails after applying 1st patch.
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel