https://fedorahosted.org/sssd/ticket/1773
Commit bf8cce77a35cb0a3cdb0d21fb9c39b7b6372bc11 broke pam_pwd_expiration_warning because it started treating its value as if it was seconds, but in fact it's days. I tested the path that hits parse_krb5_child_response() and the one that goes through check_pwexpire_ldap() but I'm not entirely sure how to test the LDAP authentication that uses check_pwexpire_kerberos(). Could anyone educate me (or test the patch as part of review maybe?). I also don't like that the Kerberos password expiration is checked in the provider and the LDAP password expiration in the responder. I think this kind of logic belongs to the responder only. I filed #1774 to get it fixed, but since this patch is intended for the 1.9.4 release that is going to happen after Wednesday, I think we can do the refactor separately.
>From ac9fab1f52338e720145cc6967db950a1515f707 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <[email protected]> Date: Sat, 19 Jan 2013 18:50:34 +0100 Subject: [PATCH] Convert the value of pwd_exp_warning to seconds https://fedorahosted.org/sssd/ticket/1773 Commit bf8cce77a35cb0a3cdb0d21fb9c39b7b6372bc11 broke pam_pwd_expiration_warning because it started treating its value as if it was seconds, but in fact it's days. --- src/providers/krb5/krb5_child_handler.c | 11 ++++++----- src/providers/ldap/ldap_auth.c | 24 +++++++++++++++--------- src/util/auth_utils.h | 12 ++++++++++++ 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c index 5adbcf700ebf02a2aa524af3cecab51e543b4986..f5a7f263870af3aa2a3557373172d32091efae2a 100644 --- a/src/providers/krb5/krb5_child_handler.c +++ b/src/providers/krb5/krb5_child_handler.c @@ -24,6 +24,7 @@ #include "util/util.h" #include "util/child_common.h" +#include "util/auth_utils.h" #include "providers/krb5/krb5_common.h" #include "providers/krb5/krb5_auth.h" #include "src/providers/krb5/krb5_utils.h" @@ -482,6 +483,7 @@ parse_krb5_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, ssize_t len, struct krb5_child_response *res; const char *upn = NULL; size_t upn_len; + int expiration_warning; if ((size_t) len < sizeof(int32_t)) { DEBUG(SSSDBG_CRIT_FAILURE, ("message too short.\n")); @@ -490,9 +492,8 @@ parse_krb5_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, ssize_t len, memset(&tgtt, 0, sizeof(struct tgt_times)); - if (pwd_exp_warning < 0) { - pwd_exp_warning = KERBEROS_PWEXPIRE_WARNING_TIME; - } + expiration_warning = pam_pwd_warning_to_sec(pwd_exp_warning, + KERBEROS_PWEXPIRE_WARNING_TIME); /* A buffer with the following structure is expected. * int32_t status of the request (required) @@ -556,8 +557,8 @@ parse_krb5_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, ssize_t len, if (*msg_subtype == SSS_PAM_USER_INFO_EXPIRE_WARN) { expiration = (uint32_t *)&buf[p+sizeof(uint32_t)]; - if (pwd_exp_warning > 0 && - difftime(pwd_exp_warning, *expiration) < 0.0) { + if (expiration_warning > 0 && + difftime(expiration_warning, *expiration) < 0.0) { skip = true; } } diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c index aa07fbb8f03a7d17663f5c9bcc0c1514652defdc..2974aa8c6c09c4b5f229c1f08f6f522367ffd05f 100644 --- a/src/providers/ldap/ldap_auth.c +++ b/src/providers/ldap/ldap_auth.c @@ -42,6 +42,7 @@ #include "util/util.h" #include "util/user_info_msg.h" +#include "util/auth_utils.h" #include "db/sysdb.h" #include "providers/ldap/ldap_common.h" #include "providers/ldap/sdap_async.h" @@ -128,11 +129,12 @@ static errno_t check_pwexpire_kerberos(const char *expire_date, time_t now, } else { *result = SDAP_AUTH_SUCCESS; - if (pwd_exp_warning >= 0) { - expiration_warning = pwd_exp_warning; - } else { - expiration_warning = KERBEROS_PWEXPIRE_WARNING_TIME; - } + expiration_warning = pam_pwd_warning_to_sec(pwd_exp_warning, + KERBEROS_PWEXPIRE_WARNING_TIME); + DEBUG(SSSDBG_TRACE_LIBS, + ("expire_time = %d\texpiration_warning = %d\n", + expire_time, expiration_warning)); + if (pd != NULL && (difftime(now + expiration_warning, expire_time) > 0.0 || expiration_warning == 0)) { @@ -213,10 +215,9 @@ static errno_t check_pwexpire_ldap(struct pam_data *pd, uint32_t *data; uint32_t *ptr; int ret; + int expiration_warning; - if (pwd_exp_warning < 0) { - pwd_exp_warning = 0; - } + expiration_warning = pam_pwd_warning_to_sec(pwd_exp_warning, 0); data = talloc_size(pd, 2* sizeof(uint32_t)); if (data == NULL) { @@ -230,7 +231,12 @@ static errno_t check_pwexpire_ldap(struct pam_data *pd, ptr++; *ptr = ppolicy->grace; } else if (ppolicy->expire > 0) { - if (pwd_exp_warning != 0 && ppolicy->expire > pwd_exp_warning) { + DEBUG(SSSDBG_TRACE_LIBS, + ("ppolicy->expire = %d\texpiration_warning = %d\n", + ppolicy->expire, expiration_warning)); + + if (expiration_warning != 0 && + ppolicy->expire > expiration_warning) { /* do not warn */ goto done; } diff --git a/src/util/auth_utils.h b/src/util/auth_utils.h index e9e60a085c386c1354732a816f8d982c5e6bf23f..b3c788c8ab50821130716cda112768525e8e590f 100644 --- a/src/util/auth_utils.h +++ b/src/util/auth_utils.h @@ -40,3 +40,15 @@ static inline int cached_login_pam_status(int auth_res) return PAM_SYSTEM_ERR; } + +static inline int pam_pwd_warning_to_sec(int pwd_exp_warning, int defval) +{ + if (pwd_exp_warning < 0) { + return defval; + } + + /* pwd_exp_warning is a number in *days*, but ppolicy->expire + * that comes from the server is in *seconds* + */ + return pwd_exp_warning * 24 * 60 * 60; +} -- 1.8.1
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
