On Tue, Feb 24, 2015 at 04:44:28PM +0100, Pavel Reichl wrote:
>
> On 02/23/2015 11:47 AM, Sumit Bose wrote:
> >On Fri, Feb 20, 2015 at 02:44:45PM +0100, Pavel Reichl wrote:
> >>On 02/20/2015 02:33 PM, Lukas Slebodnik wrote:
> >>>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
...
> >>index
> >>9f2e9ac34add13e40d316374094024afdcc4ae31..4e3f3510250b19b5f397125fa3e3a376e0d3701f
> >> 100644
> >>--- a/src/man/sssd-ldap.5.xml
> >>+++ b/src/man/sssd-ldap.5.xml
> >>@@ -1959,6 +1959,18 @@ ldap_access_filter = (employeeType=admin)
> >> ldap_account_expire_policy
> >> </para>
> >> <para>
> >>+ <emphasis>pwd_expire_policy</emphasis>:
> >>+ This option is useful if users are interested
> >>in
> >>+ seeing password expiration warning when
> >>authenticating
> >>+ using different method then passwords - for
> >>example
> >>+ SSH keys.
> >It's not about seeing a warning but about denying access based on an
> >expired password. I think you should be more clear here.
I'm sorry I wasn'T clear either. I meant to say 'it's not only about a
warning'. So you should mention both, the user will see a warning if the
password is about to expire and will be rejected if the password is
expired as with password authentication
> >
> >>+ </para>
> >>+ <para>
> >>+ Please note that 'access_provider = ldap' must
> >>+ be set for this feature to work. Also
> >>'ldap_pwd_policy'
> >>+ must be set to appropriate password policy.
> >>+ </para>
...
> >>+ return ret;
> >I think you have to modify the return code here to match the access
> >control expectations. check_pwexpire_policy() will e.g. return
> >ERR_PASSWORD_EXPIRED but the access control code expects
> >ERR_ACCESS_DENIED. As a result I see the following in the logs:
> In attached patch I modified sdap_access_done() instead:
>
> static void sdap_access_done(struct tevent_req *req)
> case ERR_ACCOUNT_EXPIRED:
> pam_status = PAM_ACCT_EXPIRED;
> break;
> + case ERR_PASSWORD_EXPIRED:
> + pam_status = PAM_PERM_DENIED;
> + break;
> default:
> DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access check
> result.\n");
> pam_status = PAM_SYSTEM_ERR;
>
> If you are OK with this approach would setting pam_status to
> PAM_AUTHTOK_EXPIRED be more appropriate?
That's a good point and it made me re-read to original tickets again. I
was under the impression that they ask for rejecting access as well, but
it looks thy only ask for a warning. Nevertheless I think it is a good
idea to reject access and it relates a bit to #2534 as well.
Nevertheless even if we reject your comment above made me think who we
should react is the password is expired, just reject or ask for a new
password as it would have been with password authentication.
As a result I wonder if it would make sense to add 3 options:
- pwd_expire_policy_reject: will warn and reject,
pam_status = PAM_PERM_DENIED
- pwd_expire_policy_warn: will only warn,
pam_status = PAM_SUCCESS
- pwd_expire_policy_renew: will warn and ask for new password
pam_status = PAM_NEW_AUTHTOK_REQD
What do you think?
bye,
Sumit
>
> >
> >(Mon Feb 23 11:00:07 2015) [sssd[be[ipa.devel]]]
> >[find_password_expiration_attributes] (0x4000): Found Kerberos password
> >expiration attributes.
> >(Mon Feb 23 11:00:07 2015) [sssd[be[ipa.devel]]] [check_pwexpire_kerberos]
> >(0x4000): Time info: tzname[0] [CET] tzname[1] [CEST] timezone [-3600]
> >daylight [1] now [1424685607] expire_time [1
> >421771276].
> >(Mon Feb 23 11:00:07 2015) [sssd[be[ipa.devel]]] [check_pwexpire_kerberos]
> >(0x0100): Kerberos password expired.
> >(Mon Feb 23 11:00:07 2015) [sssd[be[ipa.devel]]] [perform_pwexpire_policy]
> >(0x0080): check_pwexpire_policy returned 1432158224:[Password Expired]
> >.(Mon Feb 23 11:00:07 2015) [sssd[be[ipa.devel]]] [sdap_access_done]
> >(0x0020): Error retrieving access check result.
> >(Mon Feb 23 11:00:07 2015) [sssd[be[ipa.devel]]] [be_pam_handler_callback]
> >(0x0100): Backend returned: (3, 4, <NULL>) [Internal Error (Systemfehler)]
> >(Mon Feb 23 11:00:07 2015) [sssd[be[ipa.devel]]] [be_pam_handler_callback]
> >(0x0100): Sending result [4][ipa.devel]
> >(Mon Feb 23 11:00:07 2015) [sssd[be[ipa.devel]]] [be_pam_handler_callback]
> >(0x0100): Sent result [4][ipa.devel]
> >(Mon Feb 23 11:00:13 2015) [sssd[be[ipa.devel]]] [sbus_dispatch] (0x4000):
> >dbus conn: 0xb830ff80
> >(Mon Feb 23 11:00:13 2015) [sssd[be[ipa.devel]]] [sbus_dispatch] (0x4000):
> >Dispatching.
> >
> >So, access is denied, but with a generic system error.
> >
> >bye,
> >Sumit
> >_______________________________________________
> >sssd-devel mailing list
> >sssd-devel@lists.fedorahosted.org
> >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> Thanks!
> From e16c0e789ecb6b5c601839bdf58e95b1b24bba2d Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <prei...@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
> ---
> Makefile.am | 1 +
> src/providers/ldap/ldap_auth.c | 76
> ++++++++++++++++++++++++------------------
> src/providers/ldap/ldap_auth.h | 46 +++++++++++++++++++++++++
> 3 files changed, 91 insertions(+), 32 deletions(-)
> create mode 100644 src/providers/ldap/ldap_auth.h
>
> 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 \
> diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
> index
> 5a40c1359f138c42eb915e873fe21a50ab038e81..4035aaf58c23291eb8115ef320758ba7666ed4e2
> 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 = EINVAL;
> + }
> +
> + 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 == EINVAL) {
> + /* 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..5fbddd7087dc65ab8bd1df5fb57492d2fc26d0bb
> --- /dev/null
> +++ b/src/providers/ldap/ldap_auth.h
> @@ -0,0 +1,46 @@
> +/*
> + SSSD
> +
> + Copyright (C) Pavel Reichl <prei...@redhat.com> 2015
> +
> + 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 b9fb0b1c204b6b448e88b5d9fd0dbe2cee7469fa Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <prei...@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
> ---
> src/man/sssd-ldap.5.xml | 12 +++++++++++
> src/providers/ldap/ldap_access.c | 3 +++
> 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, 63 insertions(+), 1 deletion(-)
>
> diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml
> index
> 9f2e9ac34add13e40d316374094024afdcc4ae31..78136b675003c51454cffc6988c698ca683d9509
> 100644
> --- a/src/man/sssd-ldap.5.xml
> +++ b/src/man/sssd-ldap.5.xml
> @@ -1959,6 +1959,18 @@ ldap_access_filter = (employeeType=admin)
> ldap_account_expire_policy
> </para>
> <para>
> + <emphasis>pwd_expire_policy</emphasis>:
> + This option is useful if users are interested in
> + denying access in case that password is expired
> and
> + authentication is based on using different
> method then
> + passwords - for example SSH keys.
> + </para>
> + <para>
> + Please note that 'access_provider = ldap' must
> + be set for this feature to work. Also
> 'ldap_pwd_policy'
> + must be set to appropriate password policy.
> + </para>
> + <para>
> <emphasis>authorized_service</emphasis>: use
> the authorizedService attribute to determine
> access
> diff --git a/src/providers/ldap/ldap_access.c
> b/src/providers/ldap/ldap_access.c
> index
> 1913cd9a92342cc985d5c098f224c4fe8c58d465..958b89b020a727be3bbee428cf786d54af48298e
> 100644
> --- a/src/providers/ldap/ldap_access.c
> +++ b/src/providers/ldap/ldap_access.c
> @@ -96,6 +96,9 @@ static void sdap_access_done(struct tevent_req *req)
> case ERR_ACCOUNT_EXPIRED:
> pam_status = PAM_ACCT_EXPIRED;
> break;
> + case ERR_PASSWORD_EXPIRED:
> + pam_status = PAM_PERM_DENIED;
> + break;
> default:
> DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access check
> result.\n");
> pam_status = PAM_SYSTEM_ERR;
> diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
> index
> 4035aaf58c23291eb8115ef320758ba7666ed4e2..bdcc4505dc82cf3ca4bec9ce71ec6a9c28dd54e8
> 100644
> --- a/src/providers/ldap/ldap_auth.c
> +++ b/src/providers/ldap/ldap_auth.c
> @@ -47,6 +47,7 @@
> #include "providers/ldap/sdap_async.h"
> #include "providers/ldap/sdap_async_private.h"
> #include "providers/ldap/ldap_auth.h"
> +#include "providers/ldap/sdap_access.h"
>
> #define LDAP_PWEXPIRE_WARNING_TIME 0
>
> diff --git a/src/providers/ldap/ldap_init.c b/src/providers/ldap/ldap_init.c
> index
> 44333a9a3a45de16aaaf83fecaea4817cebc90d4..3b8db83300c43e5e5397cdf0c880a1d82effb4c2
> 100644
> --- a/src/providers/ldap/ldap_init.c
> +++ b/src/providers/ldap/ldap_init.c
> @@ -423,6 +423,9 @@ int sssm_ldap_access_init(struct be_ctx *bectx,
> access_ctx->access_rule[c] = LDAP_ACCESS_HOST;
> } else if (strcasecmp(order_list[c], LDAP_ACCESS_LOCK_NAME) == 0) {
> access_ctx->access_rule[c] = LDAP_ACCESS_LOCKOUT;
> + } else if (strcasecmp(order_list[c],
> + LDAP_ACCESS_EXPIRE_POLICY_NAME) == 0) {
> + access_ctx->access_rule[c] = LDAP_ACCESS_EXPIRE_POLICY;
> } else {
> DEBUG(SSSDBG_CRIT_FAILURE,
> "Unexpected access rule name [%s].\n", order_list[c]);
> diff --git a/src/providers/ldap/sdap_access.c
> b/src/providers/ldap/sdap_access.c
> index
> a6c882cae634f080b200fe75f51867e39192bcd9..b38b958f27c7ca8d463e789f14c96b5b2f11ad77
> 100644
> --- a/src/providers/ldap/sdap_access.c
> +++ b/src/providers/ldap/sdap_access.c
> @@ -39,10 +39,16 @@
> #include "providers/ldap/sdap_async.h"
> #include "providers/data_provider.h"
> #include "providers/dp_backend.h"
> +#include "providers/ldap/ldap_auth.h"
>
> #define PERMANENTLY_LOCKED_ACCOUNT "000001010000Z"
> #define MALFORMED_FILTER "Malformed access control filter [%s]\n"
>
> +static errno_t perform_pwexpire_policy(TALLOC_CTX *mem_ctx,
> + struct sss_domain_info *domain,
> + struct pam_data *pd,
> + struct sdap_options *opts);
> +
> static errno_t sdap_save_user_cache_bool(struct sss_domain_info *domain,
> const char *username,
> const char *attr_name,
> @@ -237,6 +243,11 @@ static errno_t sdap_access_check_next_rule(struct
> sdap_access_req_ctx *state,
> state->pd, state->user_entry);
> break;
>
> + case LDAP_ACCESS_EXPIRE_POLICY:
> + ret = perform_pwexpire_policy(state, state->domain, state->pd,
> + state->access_ctx->id_ctx->opts);
> + break;
> +
> case LDAP_ACCESS_SERVICE:
> ret = sdap_access_service( state->pd, state->user_entry);
> break;
> @@ -651,7 +662,6 @@ static errno_t sdap_account_expired_nds(struct pam_data
> *pd,
> return EOK;
> }
>
> -
> static errno_t sdap_account_expired(struct sdap_access_ctx *access_ctx,
> struct pam_data *pd,
> struct ldb_message *user_entry)
> @@ -702,6 +712,37 @@ static errno_t sdap_account_expired(struct
> sdap_access_ctx *access_ctx,
> return ret;
> }
>
> +static errno_t perform_pwexpire_policy(TALLOC_CTX *mem_ctx,
> + struct sss_domain_info *domain,
> + struct pam_data *pd,
> + struct sdap_options *opts)
> +{
> + enum pwexpire pw_expire_type;
> + void *pw_expire_data;
> + errno_t ret;
> + char *dn;
> +
> + ret = get_user_dn(mem_ctx, domain, opts, pd->user, &dn, &pw_expire_type,
> + &pw_expire_data);
> + if (ret != EOK) {
> + DEBUG(SSSDBG_MINOR_FAILURE, "get_user_dn returned %d:[%s].\n",
> + ret, sss_strerror(ret));
> + goto done;
> + }
> +
> + ret = check_pwexpire_policy(pw_expire_type, pw_expire_data, pd,
> + domain->pwd_expiration_warning);
> + if (ret != EOK) {
> + DEBUG(SSSDBG_MINOR_FAILURE,
> + "check_pwexpire_policy returned %d:[%s].\n",
> + ret, sss_strerror(ret));
> + goto done;
> + }
> +
> +done:
> + return ret;
> +}
> +
> struct sdap_access_filter_req_ctx {
> const char *username;
> const char *filter;
> diff --git a/src/providers/ldap/sdap_access.h
> b/src/providers/ldap/sdap_access.h
> index
> f085e619961198b887d65ed5ee0bc5cdd90d1b20..825fcffbc5fcd9413c9aa0dd79a8b87bd57d4dbc
> 100644
> --- a/src/providers/ldap/sdap_access.h
> +++ b/src/providers/ldap/sdap_access.h
> @@ -39,6 +39,7 @@
>
> #define LDAP_ACCESS_FILTER_NAME "filter"
> #define LDAP_ACCESS_EXPIRE_NAME "expire"
> +#define LDAP_ACCESS_EXPIRE_POLICY_NAME "pwd_expire_policy"
> #define LDAP_ACCESS_SERVICE_NAME "authorized_service"
> #define LDAP_ACCESS_HOST_NAME "host"
> #define LDAP_ACCESS_LOCK_NAME "lockout"
> @@ -57,6 +58,7 @@ enum ldap_access_rule {
> LDAP_ACCESS_SERVICE,
> LDAP_ACCESS_HOST,
> LDAP_ACCESS_LOCKOUT,
> + LDAP_ACCESS_EXPIRE_POLICY,
> LDAP_ACCESS_LAST
> };
>
> --
> 2.1.0
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
