[Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators
Nathaniel McCallum
npmccallum at redhat.com
Tue May 24 16:08:28 UTC 2016
I have attached new versions of the patches. Comments below.
On Tue, 2016-05-24 at 15:25 +0200, Sumit Bose wrote:
> On Thu, May 12, 2016 at 05:33:26PM -0400, Nathaniel McCallum wrote:
> > On Fri, 2016-05-06 at 14:44 +0200, Sumit Bose wrote:
> > > On Wed, May 04, 2016 at 05:33:55PM -0400, Nathaniel McCallum
> > > wrote:
> > > > This series of patches implements authentication indicator
> > > > insertion,
> > > > evaluation and management in FreeIPA. Besides these patches,
> > > > two
> > > > other
> > > > patches are needed to round out support.
> > > >
> > > > First, we need a UI patch: https://fedorahosted.org/freeipa/tic
> > > > ket/
> > > > 5872
> > > >
> > > > Second, we need a SSSD patch to handle the new case where
> > > > multiple
> > > > responders are set (when either 1FA or 2FA can be used).
> > >
> > > I've already some initial work done here and will continue with
> > > your
> > > patches.
> > >
> > > >
> > > > Please note that the last patch in this series (0093) is
> > > > untested
> > > > and
> > > > simply represents my desire to get these patches off of my hard
> > > > disk
> > > > before I take a long weekend. This patch also requires mrogers'
> > > > patch
> > > > 0001 (already merged to master).
> > > >
> > > > Also worthy of note is the need for an OID for the
> > > > authentication
> > > > control. Hopefully Simo can assign this after we agree that
> > > > this
> > > > control method is sufficient. One question I had was whether or
> > > > not
> > > > it
> > > > would be possible to send the control only on UNIX sockets
> > > > (0089;
> > > > report_auth_method()).
> > > >
> > > > Please review the approaches taken here. I plan to hit this
> > > > hard on
> > > > Monday.
> > >
> > > I'm on a conference next week and currently busy preparing my
> > > presentation. I will give you feedback in the following week.
> >
> > Thanks!
>
> sorry for the delay, I was distracted because on some of my VMs OTP
> in
> general does not work anymore. I assume some issue, maybe with
> libverto
> on 32bit systems, but I'm still debugging.
>
> Nevertheless I was able to successfully test the patches in 64bits.
>
> Simo, please see OID assignment request below.
>
> >
> > The attached patches offer the latest version of the work. The only
> > major outstanding item that I see is OID assignment (which we can
> > do
> > just before committing).
> >
> > I have tested the full stack both for appropriate approvals and
> > denials
> > across all possible scenarios. In short it works.
> >
> > The easiest way to test this is as following:
> >
> > # After Clean Install of FreeIPA
> > $ kinit admin
> >
> > # Add a service allowed by either 1FA or 2FA
> > $ ipa service-add ANY/ipa.example.com
> > $ ipa-getkeytab -p ANY/ipa.example.com -k /tmp/any.keytab
> >
> > # Add a service allowed only by 2FA
> > $ ipa service-add OTP/ipa.example.com --auth-ind=otp
> > $ ipa-getkeytab -p OTP/ipa.example.com -k /tmp/otp.keytab
> >
> > # Add the test user
> > $ ipa user-add test --user-auth-type=otp --user-auth-type=password
> > $ ipa passwd test
> > $ kinit test
> >
> > # Try to get tickets for the services
> > $ kvno ANY/ipa.example.com # Expected success
> > $ kvno OTP/ipa.example.com # Expected failure
> >
> > # Add a token and login with 2FA
> > $ ipa otptoken-add
> > $ kinit -T <ccache> test # Log in with 2FA
> >
> > # Try to get tickets for the services
> > $ kvno ANY/ipa.example.com #
> > Expected success
> > $ kvno OTP/ipa.example.com # Expected success
>
> > From c9e2b50248493fb5a283cf8c88c8e20c312d6348 Mon Sep 17 00:00:00
> > 2001
> > From: Nathaniel McCallum <npmccallum at redhat.com>
> > Date: Wed, 4 May 2016 17:08:45 -0400
> > Subject: [PATCH 5/5] Enable service authentication indicator
> > management
> >
>
> For me the patch looks good, but it would be nice if someone more
> used
> to our usage of python can have a short look to see if all
> conventioens
> are met. ACK for the functionality.
I would like for us to merge the first four patches first and then
concentrate on this one.
In particular, the following issue needs to be discussed. We currently
only have two, hard-coded indicator values: otp and radius. Thus, we
use a StrEnum for this property. However, in the long-term, I'd like to
have more flexibility; such as per-token indicators. This implies
String.
Is there some way to do StrEnum now and easily migrate to String later?
I think this will break API. Thoughts?
> > From 356daafb001bd868f37f6f0b58338bd0c4da135c Mon Sep 17 00:00:00
> > 2001
> > From: Nathaniel McCallum <npmccallum at redhat.com>
> > Date: Sun, 21 Feb 2016 19:44:19 -0500
> > Subject: [PATCH 4/5] Enable authentication indicators for OTP and
> > RADIUS
> >
>
> ACK
>
> > From c33d0d2af5284a6eb5e50a4f5864f94fa8b3cf21 Mon Sep 17 00:00:00
> > 2001
> > From: Nathaniel McCallum <npmccallum at redhat.com>
> > Date: Sun, 21 Feb 2016 19:43:52 -0500
> > Subject: [PATCH 3/5] Return password-only preauth if passwords are
> > allowed
> >
>
> ACK, on the client krb5_responder_list_questions() return both
> "password" and "otp" if the user is configured for both.
>
> Btw, what is the right way for a client to skip "otp" and only do
> "password" should something like krb5_responder_otp_set_answer(ctx,
> rctx, i, NULL, NULL); work ?
>
> > From 42768f63cdfd47ff3ac0bcdc228fb363b421e2b2 Mon Sep 17 00:00:00
> > 2001
> > From: Nathaniel McCallum <npmccallum at redhat.com>
> > Date: Thu, 12 May 2016 15:10:47 -0400
> > Subject: [PATCH 2/5] Ensure that ipa-otpd bind auths validate an
> > OTP
>
> ACK. Depending on the user setting LDAP simple bind does work with
> password, password+token or both.
>
> >
> > Before this patch, if the user was configured for either OTP or
> > password
> > it was possible to do a 1FA authentication through ipa-otpd.
> > Because this
> > correctly respected the configuration, it is not a security error.
> >
> > However, once we begin to insert authentication indicators into the
> > Kerberos tickets, we cannot allow 1FA authentications through this
> > code path. Otherwise the ticket would contain a 2FA indicator when
> > only 1FA was actually performed.
> >
> > To solve this problem, we have ipa-otpd send a critical control
> > during
> > the bind operation which informs the LDAP server that it *MUST*
> > validate
> > an OTP token for authentication to be successful. Next, we
> > implement
> > support for this control in the ipa-pwd-extop plugin. The end
> > result is
> > that the bind operation will always fail if the control is present
> > and
> > no OTP is validated.
> >
> > TODO: assign an OID
> >
> > https://fedorahosted.org/freeipa/ticket/433
> > ---
> > daemons/ipa-otpd/bind.c | 5 ++++-
> > daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h | 3 +++
> > daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 13 ++++++++---
> > --
> > 3 files changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/daemons/ipa-otpd/bind.c b/daemons/ipa-otpd/bind.c
> > index
> > c985ccd7e73c6e8182cafcef49fd9873ab3340ea..022525b786705b4f58f861bc3
> > b0a745ab8693755 100644
> > --- a/daemons/ipa-otpd/bind.c
> > +++ b/daemons/ipa-otpd/bind.c
> > @@ -26,9 +26,12 @@
> > */
> >
> > #include "internal.h"
> > +#include "../ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h"
> >
> > static void on_bind_writable(verto_ctx *vctx, verto_ev *ev)
> > {
> > + LDAPControl control = { OTP_REQUIRED_OID, {}, true };
> > + LDAPControl *ctrls[] = { &control, NULL };
> > struct otpd_queue *push = &ctx.stdio.responses;
> > const krb5_data *data;
> > struct berval cred;
> > @@ -55,7 +58,7 @@ static void on_bind_writable(verto_ctx *vctx,
> > verto_ev *ev)
> > cred.bv_val = data->data;
> > cred.bv_len = data->length;
> > i = ldap_sasl_bind(verto_get_private(ev), item->user.dn,
> > LDAP_SASL_SIMPLE,
> > - &cred, NULL, NULL, &item->msgid);
> > + &cred, ctrls, NULL, &item->msgid);
> > if (i != LDAP_SUCCESS) {
> > otpd_log_err(errno, "Unable to initiate bind: %s",
> > ldap_err2string(i));
> > verto_break(ctx.vctx);
> > diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h
> > b/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h
> > index
> > 0b5cbc586118fe43b6e9ac823179174a7b3ba91e..184962095029edec15dac8007
> > 21eb63b3353ec50 100644
> > --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h
> > +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h
> > @@ -53,6 +53,9 @@
> > */
> > #define OTP_SYNC_REQUEST_OID "2.16.840.1.113730.3.8.10.6"
> >
> > +/* This control has no data. */
> > +#define OTP_REQUIRED_OID "1.2.3.4.5.6.7.8.9"
> > +
>
> Simo, can you assign a proper OID for OTP_REQUIRED_OID ?
I have added the assigned OID now.
> > bool otpctrl_present(Slapi_PBlock *pb, const char *oid);
> >
>
> > From ec1c3e3ef0e3b06c6fd93b3d4709b55f28e12873 Mon Sep 17 00:00:00
> > 2001
> > From: Nathaniel McCallum <npmccallum at redhat.com>
> > Date: Thu, 12 May 2016 14:57:29 -0400
> > Subject: [PATCH 1/5] Rename syncreq.[ch] to otpctrl.[ch]
> >
>
> ...
>
> > index
> > 98a97c4c9f6d2e6bf74f97fc93053b3aebbc7821..0b5cbc586118fe43b6e9ac823
> > 179174a7b3ba91e 100644
> > --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.h
> > +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h
> > @@ -37,9 +37,7 @@
> > * All rights reserved.
> > * END COPYRIGHT BLOCK **/
> >
> > -
> > -#ifndef SYNCREQ_H_
> > -#define SYNCREQ_H_
> > +#pragma once
> >
>
> I would prefer to keep the old way for now and discuss on the list if
> we
> should move to '#pragma once'. If we can get an agreement we can
> switch
> to '#pragma once' completely later.
I have used guards in the latest patch. I now have another patch on the
list to migrate all files from using guards to pragma.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-Enable-service-authentication-indicator-management.patch
Type: text/x-patch
Size: 6254 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/c025df92/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Enable-authentication-indicators-for-OTP-and-RADIUS.patch
Type: text/x-patch
Size: 1986 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/c025df92/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Return-password-only-preauth-if-passwords-are-allowe.patch
Type: text/x-patch
Size: 1676 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/c025df92/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Ensure-that-ipa-otpd-bind-auths-validate-an-OTP.patch
Type: text/x-patch
Size: 5577 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/c025df92/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Rename-syncreq.-ch-to-otpctrl.-ch.patch
Type: text/x-patch
Size: 4960 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/c025df92/attachment-0004.bin>
More information about the Freeipa-devel
mailing list