[Freeipa-devel] Patch for #1539

[Nathaniel McCallum]

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.

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?

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.

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.

Nathaniel