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!
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).
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.
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'.
bye, Sumit
On Thu, Feb 19, 2015 at 06:16:10PM +0100, 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).
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.
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'.
Another comment:
diff --git a/Makefile.am b/Makefile.am index 8cb815b..239199b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -563,6 +563,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 \
bye, Sumit
bye, Sumit _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
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 _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thanks for comments and testing. Please see updated patchset (only second patch differs).
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@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@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@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
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 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@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@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@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@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thanks Lukas, that wasn't very smart from me. Updated patch attached.
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 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@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@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@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@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thanks Lukas, that wasn't very smart from me. Updated patch attached.
Thank you, please find comments in-line:
From ac6de4dd1410d3aa91378afc145dfe915b47292b Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@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
...
+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;
We rarely return -1 as error code. I think EINVAL would be more suitable or if you want a specific error code for the unknown type you can add an SSSD internal error code..
- }
- return ret;
+}
...
@@ -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;
see above, a SSSD internal error code might be better here.
} }
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@redhat.com
I'm not sure how important it is but typically we add the year here as well, see COPYING file.
- 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 9a561a5fb61d6c7ce44d4081c2f50bb1be4ea0c3 Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@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_auth.c | 1 + src/providers/ldap/ldap_init.c | 3 +++ src/providers/ldap/sdap_access.c | 43 +++++++++++++++++++++++++++++++++++++++- src/providers/ldap/sdap_access.h | 2 ++ 5 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml 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.
</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
...
+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.",
typo, '.' after \n
ret, sss_strerror(ret));
goto done;
- }
+done:
- 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:
(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
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 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@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@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@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@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thanks Lukas, that wasn't very smart from me. Updated patch attached.
Thank you, please find comments in-line:
Thanks I updated patches accordingly to your comments. Please see comments in-line.
From ac6de4dd1410d3aa91378afc145dfe915b47292b Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@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
...
+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;
We rarely return -1 as error code. I think EINVAL would be more suitable or if you want a specific error code for the unknown type you can add an SSSD internal error code..
- }
- return ret;
+}
...
@@ -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;
see above, a SSSD internal error code might be better here.
} }
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@redhat.com
I'm not sure how important it is but typically we add the year here as well, see COPYING file.
Sure, fixed. I copied it from some other file. For example sdap_async.h is missing year to. I am happy to check all source files if you think its a good idea.
- 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 9a561a5fb61d6c7ce44d4081c2f50bb1be4ea0c3 Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@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_auth.c | 1 + src/providers/ldap/ldap_init.c | 3 +++ src/providers/ldap/sdap_access.c | 43 +++++++++++++++++++++++++++++++++++++++- src/providers/ldap/sdap_access.h | 2 ++ 5 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml 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.
</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
...
+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.",
typo, '.' after \n
ret, sss_strerror(ret));
goto done;
- }
+done:
- 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?
(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!
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;
default: DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access checkbreak;
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 preichl@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 preichl@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 preichl@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;
default: DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access check result.\n"); pam_status = PAM_SYSTEM_ERR;break;
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
On 02/27/2015 12:01 PM, Sumit Bose wrote:
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
I think you are right, I just wonder if it would be better to create one option
pwd_expire_policy_action which would have a value from {reject, warn, renew}
Default value would be reject?
(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 preichl@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,
{ const char *mark; const char *val;enum pwexpire *type, void **data)
@@ -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 preichl@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 preichl@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
On Fri, Feb 27, 2015 at 03:17:41PM +0100, Pavel Reichl wrote:
On 02/27/2015 12:01 PM, Sumit Bose wrote:
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;
default: DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access checkbreak;
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
I think you are right, I just wonder if it would be better to create one option
pwd_expire_policy_action which would have a value from {reject, warn, renew}
Default value would be reject?
But this would mean to add a new option for a quite limited use-case.
Jakub, do you have a preference?
bye, Sumit
On 02/27/2015 04:28 PM, Sumit Bose wrote:
On Fri, Feb 27, 2015 at 03:17:41PM +0100, Pavel Reichl wrote:
On 02/27/2015 12:01 PM, Sumit Bose wrote:
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
I think you are right, I just wonder if it would be better to create one option
pwd_expire_policy_action which would have a value from {reject, warn, renew}
Default value would be reject?
But this would mean to add a new option for a quite limited use-case.
Oh, I see now. I thought that you proposed 3 new boolean option not just substitution for pwd_expire_policy.
I understand now and I agree with your proposal. Please ignore my previous message.
Jakub, do you have a preference?
bye, Sumit _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Fri, Feb 27, 2015 at 04:28:39PM +0100, Sumit Bose wrote:
On Fri, Feb 27, 2015 at 03:17:41PM +0100, Pavel Reichl wrote:
On 02/27/2015 12:01 PM, Sumit Bose wrote:
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;
default: DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access checkbreak;
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
I think you are right, I just wonder if it would be better to create one option
pwd_expire_policy_action which would have a value from {reject, warn, renew}
Default value would be reject?
But this would mean to add a new option for a quite limited use-case.
Jakub, do you have a preference?
I fell into the same trap as Pavel :-) I didn't like adding a new option, but new values for existing options sound good to me.
On 02/27/2015 12:01 PM, Sumit Bose wrote:
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
Thanks for comments. Please see attached patches.
On Sun, Mar 01, 2015 at 08:29:50PM +0100, Pavel Reichl wrote:
On 02/27/2015 12:01 PM, Sumit Bose wrote:
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;
default: DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access checkbreak;
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
Thanks for comments. Please see attached patches.
Thank you for your patience. Patches are looking good, working as expected and pass CI (http://sssd-ci.duckdns.org/logs/job/8/63/summary.html) so ACK.
Nevertheless I can imagine that there will be request for a 'Password is expired' warning for the pwd_expire_policy_warn case (and maybe a syslog messages for the pwd_expire_policy_reject case). Would you mind to file a ticket about this so that we are prepared?
bye, Sumit
On Tue, Mar 03, 2015 at 06:27:54PM +0100, Sumit Bose wrote:
On Sun, Mar 01, 2015 at 08:29:50PM +0100, Pavel Reichl wrote:
On 02/27/2015 12:01 PM, Sumit Bose wrote:
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;
default: DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access checkbreak;
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
Thanks for comments. Please see attached patches.
Thank you for your patience. Patches are looking good, working as expected and pass CI (http://sssd-ci.duckdns.org/logs/job/8/63/summary.html) so ACK.
Nevertheless I can imagine that there will be request for a 'Password is expired' warning for the pwd_expire_policy_warn case (and maybe a syslog messages for the pwd_expire_policy_reject case). Would you mind to file a ticket about this so that we are prepared?
bye, Sumit
* master: * cdaa29d2c5724a4c72bfa0f42284ccfac3d5a464 * c9b0071bfcb8eb8c71e40248de46d23aceecc0f3 * sssd-1-12: * 8b353dd2b90b7ab222acdea726ab7e8681752237 * d3f82e944dc5dab3812700a245deec4aa3245b21
On 03/03/2015 06:27 PM, Sumit Bose wrote:
Thank you for your patience. Patches are looking good, working as expected and pass CI (http://sssd-ci.duckdns.org/logs/job/8/63/summary.html) so ACK.
Nevertheless I can imagine that there will be request for a 'Password is expired' warning for the pwd_expire_policy_warn case (and maybe a syslog messages for the pwd_expire_policy_reject case). Would you mind to file a ticket about this so that we are prepared?
bye, Sumit
Gratitude for patience is on my side, thanks!
Ticket filed - https://fedorahosted.org/sssd/ticket/2594
Bye.
sssd-devel@lists.fedorahosted.org