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

[Nathaniel McCallum]

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/a78191ee5d31
> > > 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.

Nathaniel