[Freeipa-devel] [REVIEW] Intial stab towards Authentication Indicators

Fri Feb 26 14:30:46 UTC 2016

On Thu, 2016-02-25 at 16:51 -0500, Simo Sorce wrote:
> On Thu, 2016-02-25 at 16:13 -0500, Nathaniel McCallum wrote:
> > 
> > On Thu, 2016-02-25 at 12:19 -0500, Nathaniel McCallum wrote:
> > > 
> > > On Thu, 2016-02-25 at 10:49 -0500, Simo Sorce wrote:
> > > > 
> > > > 
> > > > On Thu, 2016-02-25 at 10:32 -0500, Nathaniel McCallum wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On Wed, 2016-02-24 at 09:55 -0500, Nathaniel McCallum wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Sun, 2016-02-21 at 20:50 -0500, Simo Sorce wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On Sun, 2016-02-21 at 20:20 -0500, Nathaniel McCallum
> > > > > > > wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > https://github.com/npmccallum/freeipa/pull/1
> > > > > > > > 
> > > > > > > > The above (pseudo) pull request contains four patches
> > > > > > > > against
> > > > > > > > FreeIPA
> > > > > > > > to enable the insertion of Authentication Indicators
> > > > > > > > into
> > > > > > > > Kerberos
> > > > > > > > tickets. The basic flow looks like this.
> > > > > > > > 
> > > > > > > > First, we patch ipa-pwd-extop to return a control
> > > > > > > > indicating
> > > > > > > > what
> > > > > > > > authentication method succeeded resulting in a
> > > > > > > > successful
> > > > > > > > bind.
> > > > > > > > 
> > > > > > > > Second, we patch ipa-otpd to check the returned control
> > > > > > > > to
> > > > > > > > ensure
> > > > > > > > that
> > > > > > > > the bind resulted from an otp validation.
> > > > > > > > 
> > > > > > > > Third, we patch ipa-kdb to enable the KDC to return
> > > > > > > > either
> > > > > > > > the
> > > > > > > > encrypted timestamp or encrypted challenge preauth
> > > > > > > > mechanism
> > > > > > > > when
> > > > > > > > the
> > > > > > > > user is configured for optional 2FA logins. Clients can
> > > > > > > > then
> > > > > > > > decide
> > > > > > > > whether to do 1FA or 2FA login (for kinit, sane
> > > > > > > > behavior
> > > > > > > > already
> > > > > > > > exists).
> > > > > > > > 
> > > > > > > > Forth, we patch ipa-kdb again to insert hard-coded
> > > > > > > > authentication
> > > > > > > > indicators for either OTP or RADIUS.
> > > > > > > > 
> > > > > > > > Some explanation is required for the first two patches.
> > > > > > > > Currently,
> > > > > > > > it
> > > > > > > > is possible to do a 1FA through the otp
> > > > > > > > preauthentication
> > > > > > > > mechanism
> > > > > > > > if
> > > > > > > > the user is configured for doing optional 2FA. However,
> > > > > > > > because
> > > > > > > > we
> > > > > > > > want
> > > > > > > > to insert an authentication indicator in this code
> > > > > > > > path, we
> > > > > > > > need
> > > > > > > > to
> > > > > > > > guarantee that a request going through the otp preauth
> > > > > > > > mechanism
> > > > > > > > actually validates an OTP. This is the purpose of the
> > > > > > > > control.
> > > > > > > > 
> > > > > > > > Items still on the TODO list:
> > > > > > > > 
> > > > > > > >   * Authentication Indicator enforcement
> > > > > > > >     - Upstream libkrb5 needs to grow funcs for reading
> > > > > > > > indicators
> > > > > > > >     - Schema change to add indicators multi-value attr
> > > > > > > > to
> > > > > > > > services
> > > > > > > >     - ipa-kdb needs to implement check_policy_tgs()
> > > > > > > > 
> > > > > > > > 
> > > > > > > >   * SSSD needs to learn to handle optional 2FA
> > > > > > > > 
> > > > > > > > I will write up a project page for all of this
> > > > > > > > tomorrow.
> > > > > > > > But
> > > > > > > > this
> > > > > > > > small
> > > > > > > > code basically amounts to my brainstorming. It is not
> > > > > > > > ready
> > > > > > > > for
> > > > > > > > merge,
> > > > > > > > just basic review.
> > > > > > > > 
> > > > > > > It looks mostly ok, however the LDAP control part needs
> > > > > > > to be
> > > > > > > done
> > > > > > > as
> > > > > > > a
> > > > > > > request/response pair.
> > > > > > > A client that wishes to know what kind of authentication
> > > > > > > happened
> > > > > > > should
> > > > > > > send a request control, and only in that case , the
> > > > > > > server
> > > > > > > will
> > > > > > > send
> > > > > > > the
> > > > > > > associated reply control with the requested information.
> > > > > > I just pushed a new version of the control (now merged into
> > > > > > a
> > > > > > single
> > > > > > patch): https://github.com/npmccallum/freeipa/commit/a78191
> > > > > > ee5d
> > > > > > 31
> > > > > > e1de
> > > > > > 39
> > > > > > f28eb637f66199da7e9225
> > > > > > 
> > > > > > In this version the client sends a critical control with no
> > > > > > content
> > > > > > indicating that the server must validate an OTP. If the
> > > > > > LDAP
> > > > > > server
> > > > > > doesn't support the control (for whatever reason), bind
> > > > > > will
> > > > > > fail. If
> > > > > > the LDAP server doesn't validate an OTP (for whatever
> > > > > > reason),
> > > > > > bind
> > > > > > will fail.
> > > > > > 
> > > > > > This approach is simpler and doesn't require a
> > > > > > request/response
> > > > > > control
> > > > > > pair.
> > > > > I need some design advice. My goal here is that we need a way
> > > > > to
> > > > > expose
> > > > > the authentication indicators to services in the FreeIPA
> > > > > UI/CLI.
> > > > > 
> > > > > Here is the good news: users can already set these values in
> > > > > FreeIPA
> > > > > using kadmin. They do this by simply setting the require_auth
> > > > > string on
> > > > > the target service principal. Our kdb plugin then encodes
> > > > > these
> > > > > with
> > > > > the rest of the tl_data into the krbExtraData attribute.
> > > > > 
> > > > > I see two approaches here. First, we can try to manipulate
> > > > > the
> > > > > krbExtraData attribute directly. Second, we can create a
> > > > > separate
> > > > > attribute for the authentication indicator strings and then
> > > > > synthesize
> > > > > the tl_data internally in kdb. We would have to do this for
> > > > > both
> > > > > reads
> > > > > and writes so as not to break existing kdb functionality.
> > > > > 
> > > > > The trade-off that I see is that the first method complicates
> > > > > the
> > > > > python framework side where the second method complicates the
> > > > > kdb
> > > > > plugin.
> > > > > 
> > > > > A third option, which I doubt is even possible, is to use
> > > > > kadmin
> > > > > to
> > > > > manipulate this option rather than modifying LDAP directly.
> > > > > 
> > > > > Thoughts?
> > > > We should translate it, we need that to allow to delegate
> > > > access
> > > > only
> > > > to
> > > > the specific attribute via our standard means.
> > > > 
> > > > We already do this for other tl_data entries.
> > > > 
> > > > The krbExtraData access cannot always be delegated because it
> > > > would
> > > > be
> > > > open ended. also it is really obnoxious to have to manipulate
> > > > ASN.1
> > > > stuff in the framework.
> > > > 
> > > > kadmin could be used at some point, but we'd still want to have
> > > > this
> > > > attribute extracted in order to be able to grant access control
> > > > individually, as our ACL system and delegation system is more
> > > > fine
> > > > grained than what kadmin can offer.
> > > After discussing this with MIT, Simo and Matt, it seems that the
> > > best
> > > option is to update the (MIT) upstream krbPrincipal objectClass
> > > to
> > > have
> > > a new attribute. The reason for this is twofold. First, it has
> > > upstream
> > > value. Second, we don't have good objectClass to attach the new
> > > attribute to inside FreeIPA.
> > > 
> > > So the current plan is that Matt will create a patch for storing
> > > auth
> > > indicators (specifically, the "required_auth" strings) in a new
> > > multi-
> > > value string attribute on krbPrincipal objects. The
> > > get_principal()
> > > KDB
> > > hook will read "required_auth" from krbExtraData or (if present,
> > > preferred) the new attribute. In turn, the put_principal() KDB
> > > hook
> > > will store "required_auth" in the new attribute. This will allow
> > > the
> > > transparent migration of any data currently stored in
> > > krbExtraData.
> > > 
> > > As part of this process, Matt will also refactor put_principal()
> > > into
> > > smaller functions (it is currently 800+ LOC).
> > > 
> > > Once we have an attribute in upstream krbPrincipal, we will use
> > > this
> > > attribute exclusively in our KDB plugin.
> > I have started a project page:
> > http://www.freeipa.org/page/V4/AuthenticationIndicators
> > 
> > We are still waiting on some details. But the general shape of
> > things
> > is there. Please review. :)
> LGTM so far.
> 
> Questions:
> - Should the control specify what kind of auth specifically should be
> required ?

I had also wondered that. I'm certainly not against it. But I'd
probably prefer a simple utf8 string value to avoid parsing complexity.

> - Will it make sense in future to have different strength otp-like
> second factors and have ipa-otpd be able to specify which one it is
> expecting to be validated ?

I'm personally hoping that we can deprecate ipa-otpd after SPAKE lands.
Post-SPAKE validations will require a method for validating OTP-only
(excluding password). This will probably be an extop. The same will be
true for all new second factors.

I'm thus not sure if we'll ever add more second factors to the existing
simple bind mechanism.

> - Even if ipa-otpd will not grow such a feature, I see this control
> could be useful for pure LDAP auth clients, so perhaps a different
> kind
> of client may want to set this control ? Perhaps one day we can have
> a
> way to do GSSAPI auth and check that the AI on the ldap ticket was a
> 2FA
> and then DS will refuse login if the otp AI was missing on the ticket
> it
> received and the control requires it ? (could be used for the IPA UI
> connection to LDAP maybe ?)

That seems to me like a decision LDAP can make internally. No?