On 06/09/2014 05:54 PM, Dmitri Pal wrote: > On 06/09/2014 10:03 AM, Nathaniel McCallum wrote: >> On Mon, 2014-06-09 at 09:01 -0400, Simo Sorce wrote: >>>>>>> From: "Martin Kosek" <[email protected]> >>>>>>> Given all sort of issues we get, I am thinking we should just revert it >>>>>>> unless >>>>>>> there is a quick fix available. >>> Instead of reverting I am thinking we may want to make this optional by >>> adding a configuration parameter that defaults to False for now. Once we can >>> manage better the password change we can turn it on by deault, in the >>> meanwhile admins can choose by themselves the lesser evil. >>> >>> Thoughts? >> I'm not a fan of introducing a new configuration parameter for a >> temporary workaround. >> >> My preference is to revert it and have a small project for the next >> release which handles all the "non-authenticated" corner cases. This >> would include: >> * Expired passwords >> * Password changes >> * Token syncing >> * Unauthenticated RPCs (rpcserver.py rework) >> * others? >> >> I think there is some value to be gained by thinking about these >> problems as a whole and devising a set of consistent mechanisms for >> them. >> >> Nathaniel >> >> _______________________________________________ >> Freeipa-devel mailing list >> [email protected] >> https://www.redhat.com/mailman/listinfo/freeipa-devel > +1 >
This is my preference as well, as written in other part of this thread. I reverted the patch, added an appropriate description (attached) and pushed to master. I updated the ticket, added Nathaniel's suggestions and moved it to needs triage. Thanks, Martin
From c41b782bc59cd0cb70cbd62d543f9c538109d410 Mon Sep 17 00:00:00 2001 From: Martin Kosek <[email protected]> Date: Tue, 10 Jun 2014 08:42:03 +0200 Subject: [PATCH] Revert "Check for password expiration in pre-bind" This reverts commit bfdbd3b6ad7c437e7dd293d2488b2d53f4ea7ba6. Forceful validation of password expiration date in a BIND pre-callback breaks LDAP password change extended operation as the password change is only allowed via authenticated (bound) channel. Passwords could be only changed via kadmin protocol. This change would thus break LDAP-only clients and Web UI password change hook. This patch will need to be revisited so that unauthenicated corner cases are also revisited. https://fedorahosted.org/freeipa/ticket/1539 --- daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 33 +++-------------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c index 6786c6ddb4de4158ec680e271cae29318bc150ce..23c7cb18c9a1cb5256254f20080c5d9aaec25579 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c @@ -1217,35 +1217,13 @@ static bool ipapwd_pre_bind_otp(const char *bind_dn, Slapi_Entry *entry, } static int ipapwd_authenticate(const char *dn, Slapi_Entry *entry, - const struct berval *credentials, - const char **errmsg) + const struct berval *credentials) { Slapi_Value **pwd_values = NULL; /* values of userPassword attribute */ Slapi_Value *value = NULL; Slapi_Attr *attr = NULL; - struct tm expire_tm; - char *expire; - char *p; int ret; - /* check the if the krbPrincipalKey attribute is present */ - ret = slapi_entry_attr_find(entry, "krbprincipalkey", &attr); - if (!ret) { - /* check that the password is not expired */ - expire = slapi_entry_attr_get_charptr(entry, "krbpasswordexpiration"); - if (expire) { - memset(&expire_tm, 0, sizeof (expire_tm)); - p = strptime(expire, "%Y%m%d%H%M%SZ", &expire_tm); - if (*p) { - LOG("Invalid expiration date string format"); - return 1; - } else if (time(NULL) > mktime(&expire_tm)) { - *errmsg = "The user password is expired"; - return 1; - } - } - } - /* retrieve userPassword attribute */ ret = slapi_entry_attr_find(entry, SLAPI_USERPWD_ATTR, &attr); if (ret) { @@ -1403,7 +1381,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb) static const char *attrs_list[] = { SLAPI_USERPWD_ATTR, "ipaUserAuthType", "krbprincipalkey", "uid", "krbprincipalname", "objectclass", "passwordexpirationtime", - "passwordhistory", "krbprincipalexpiration", "krbpasswordexpiration", + "passwordhistory", "krbprincipalexpiration", NULL }; struct berval *credentials = NULL; @@ -1416,7 +1394,6 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb) time_t expire_time; char *principal_expire = NULL; struct tm expire_tm; - const char *errmsg = NULL; /* get BIND parameters */ ret |= slapi_pblock_get(pb, SLAPI_BIND_TARGET, &dn); @@ -1477,12 +1454,10 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb) } /* Authenticate the user. */ - ret = ipapwd_authenticate(dn, entry, credentials, &errmsg); + ret = ipapwd_authenticate(dn, entry, credentials); if (ret) { slapi_entry_free(entry); - slapi_send_ldap_result(pb, LDAP_INVALID_CREDENTIALS, - NULL, errmsg, 0, NULL); - return 1; + return 0; } /* Attempt to handle a token synchronization request. */ -- 1.9.3
_______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
