On Tue, Dec 02, 2014 at 09:12:25AM -0500, Nathaniel McCallum wrote:
On Tue, 2014-12-02 at 10:22 +0100, Jakub Hrozek wrote:
> On Mon, Dec 01, 2014 at 05:16:49PM -0500, Nathaniel McCallum wrote:
> > On Mon, 2014-12-01 at 22:15 +0100, Jakub Hrozek wrote:
> > > Hi,
> > >
> > > the attached patch fixes chpass for OTP users for me. I hope looking at
> > > the ipaUserAuthType attribute is acceptable.
> > >
> > > The attribute is undocumented on purpose -- I don't see a reason for
the
> > > user to set this attribute to a different value and the desription would
> > > just clutter the (already too complex) sssd-ldap man page.
> > >
> > > I'm open to adding the attribute to the configAPI, though.
> >
> > I'm very thankful for a quick fix!
> >
> > I haven't tested it. However, part of me is loathe to special case OTP
> > in this way. I admit, this case isn't bad.
>
> I know, I'm not thrilled by the special-case either. One of the reasons
> I CC-ed you directly was to get another opinion -- if there is another
> way of fixing the bug, I'm all for it.
>
> > But ipaUserAuthType does not
> > technically indicate which method was used, only which methods are
> > possible.
>
> Right. Is there a way to see what method was used prior to actually
> using the authtok? I know the conversation function returns an indicator
> of OTP being used, but only after it was sent over the wire.
I'm pretty sure that krb5_child returns whether or not OTP was used
during authentication.
Hmm, I don't have the backtrace right now, but I think the prompter is
invoked after krb5_get_init_creds_password() which is when we send the
authtoken...so at that point the OTP is already consumed.
We implemented this feature so that password
caches are invalidated.
Yep, the krb5_child returns "I used an OTP" to sssd_be so that sssd_be
doesn't cache the password. However, wasn't there a replay bug with
HOTPs in IPA that made this whole reuse possible?
> > For instance, if ipaUserAuthType == "otp" and "password",
the
> > user could use either one (not currently, but that is the plan).
>
> Hm, are you sure we can't use either already? That's how it worked for
> me during my testing.
It currently works for LDAP bind but not kinit. The reason is that MIT
kerberos doesn't support preauth sets. We are instead designing a new
preauth mech to handle this logic.
> >
> > It seems to me that reusing credentials is always wrong.
>
> I can see why it's wrong with OTPs, but what's wrong about using a
> long-term password twice in a row?
Nothing, if you can guarantee the credential is always a long-term
password. It is this guarantee that I find difficult except in a narrow
set of cases.
Yes, I agree we should refactor the chpass code, but I don't think it's
the right time now that the sssd-1-12 branch is already used by
downstreams. Let's plan this task as a high priority for the next
(sssd-1-13) release.
> > What is CHAUTHTOK anyway?
>
> Change auth token. In SSSD we have this whole operation split into two
> phases - SSS_PAM_CHAUTHTOK_PRELIM and SSS_PAM_CHAUTHTOK.
>
> The former maps onto PAM subsystem's PAM_PRELIM_CHECK and its purpose is
> to just try to acquire "kadmin/changepw" using the old password so that
> in case the old password was expired or the user was locked out, he would
> get a friendly error message back and the state of the sssd deamon or the
> cached users are not changed in any way. If the SSS_PAM_CHAUTHTOK_PRELIM
> succeeds, we then use the old password again to acquire the
"kadmin/changepw"
> principal which is then used when calling krb5_change_password().
>
> I agree it would be ideal to keep around the creds for "kadmin/changepw"
> somehow to avoid calling the same code twice, but currently that's not
> possible, the krb5_child code is one-shot. In the next release, we could
> either change the code to keep the child around, or maybe transfer the
> "kadmin/changepw" credentials back to the caller in some encoding..but
> I'm afraid that's out of scope for 7.1
>
> Would you agree that we work on this issue along with the other problems
> related to OTPs such as:
>
https://fedorahosted.org/sssd/ticket/2335
> and:
>
https://fedorahosted.org/sssd/ticket/2278
Yup. Though functional password changes are high priority.
yes, but for 1.13...
Given the discussion, what changes do you propose to the patch to get it
merged to sssd-1-12 (aka master) ?