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