On 06/09/2014 08:21 AM, Martin Kosek wrote: > On 06/06/2014 05:47 PM, Nathaniel McCallum wrote: >> On Fri, 2014-06-06 at 11:43 -0400, Simo Sorce wrote: >>> On Fri, 2014-06-06 at 11:06 -0400, Nathaniel McCallum wrote: >>>> On Fri, 2014-06-06 at 08:00 -0400, Simo Sorce wrote: >>>>> On Fri, 2014-06-06 at 10:30 +0200, Martin Kosek wrote: >>>>>> On 05/31/2014 03:27 AM, Simo Sorce wrote: >>>>>>> I have rebased theold patch attached to the ticket, unfortunately I >>>>>>> haven't had time to test it yet, but didn't want to lose it in some >>>>>>> branch. >>>>>>> >>>>>>> Simo. >>>>>> >>>>>> I tested the patch and it worked fine, code also reads OK. Thus, I am >>>>>> willing >>>>>> to ACK it. >>>>>> >>>>>> I am just wondering if there is any scenario we could have missed, but I >>>>>> did >>>>>> not find any. In there is no push back against the patch I may just push >>>>>> it. >>>> >>>>> The only thing I would draw attention to is the fact that now I am >>>>> sending back the error directly once we have a negative return from the >>>>> function in which expiration is checked (ipapwd_authenticate). >>>>> >>>>> I could not see why we did, in fact, not do that before and I meant >>>>> asking Nathaniel if he had an explicit reason why we do not, as he is >>>>> the last one that did some significant refactoring in the bind preop >>>>> plugin. >>>> >>>> In the current design, if ipapwd_authenticate() fails, we defer to 389ds >>>> for the actual response to the client. The purpose for this is so that >>>> verification of the first factor always behaves the same, regardless of >>>> what is in pre-bind. >>>> >>>> So ipapwd_authenticate() is not actually the "final" authentication. It >>>> is a preliminary authentication to determine if the user should be able >>>> to write kerberos keys and perform OTP sync. So checking expiration in >>>> pre-bind only protects these two operations. >>>> >>>> I presume that 389ds also checks password expiration. If this assumption >>>> is true, ipapwd_authenticate() SHOULD NOT return any response to the >>>> client. It should simply fail key-write/otp-sync silently and let dirsrv >>>> return the error to the client. >>> >>> 389ds would check it if we were using 389ds native password policies but >>> we are not. So we need to check on our own, which is what this patch >>> implements. >>> >>>> The important point here is that so long as 389ds is verifying password >>>> expiration, using a correct-but-expired password will should not result >>>> in a bind in the current code. It will result in kerberos key writing >>>> and OTP sync. Do we actually care about protecting kerberos key writing >>>> and OTP sync from correct-but-expired passwords? >>> >>> No, but that's not the point. >>> >>>> If 389ds does not check password expiration, then we probably need to >>>> patch upstream 389ds or, at the very least, return an error to the >>>> client. >>> >>> It is not a 389ds bug, it is just an integration issue due to the fact >>> IPA uses a different schema for password policies (for compatibility >>> with the Kerberos schema). >>> >>>> Otherwise, if we don't care about protecting kerberos key writing and >>>> OTP sync from correct-but-expired passwords, this patch is entirely >>>> unnecessary. >>>> >>>> Otherwise, the patch is necessary, but should skip kerberos key writing >>>> and OTP sync and fall through silently to 389ds. >>> >>> If we fall through to 389ds then authentication will succeed. >>> >>> So from this discussion it seem to me we achieve the goal of the patch >>> and returning an error directly is ok. >>> >>> Unless Nathaniel has something *against* returning an error in this >>> place I think we are good to go. >> >> Looks good to me. ACK. >> >> Nathaniel >> > > Ok, thanks for discussion and double checking! > > Pushed to master: bfdbd3b6ad7c437e7dd293d2488b2d53f4ea7ba6 > > Martin
I just read a question in the ticket from alonbl about how password change should work on the LDAP level: https://fedorahosted.org/freeipa/ticket/1539#comment:29 Are we ready to say that from now on, expired passwords cannot be changed via LDAP? I.e. this workflow will no longer work: $ ipa user-add --first=Foo --last Bar expired --random -------------------- Added user "expired" -------------------- User login: expired First name: Foo Last name: Bar Full name: Foo Bar Display name: Foo Bar Initials: FB Home directory: /home/expired GECOS: Foo Bar Login shell: /bin/sh Kerberos principal: expi...@mkosek-fedora20.test Email address: expi...@mkosek-fedora20.test Random password: qiCjofo.2pfT UID: 1170600026 GID: 1170600026 Password: True Member of groups: ipausers Kerberos keys available: True $ ldappasswd -h `hostname` -D uid=expired,cn=users,cn=accounts,dc=mkosek-fedora20,dc=test -x -w qiCjofo.2pfT uid=expired,cn=users,cn=accounts,dc=mkosek-fedora20,dc=test -a qiCjofo.2pfT -s Secret123 ldap_bind: Invalid credentials (49) additional info: The user password is expired Thanks, Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel