Re: [Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-26 Thread Martin Basti 



On 26.05.2016 17:36, Nathaniel McCallum wrote:

Martin, can we get patches 1-4 pushed? I'll submit patch 5 again to the
list after a rebase for further discussion.

Here it is, pushed to master:
* cd9bc84240c99ed744e5ee44db18d925a5292ffd Rename syncreq.[ch] to 
otpctrl.[ch]
* 168a6c7d4778a2a3c729e3ac24e4ad9dfacb46c0 Ensure that ipa-otpd bind 
auths validate an OTP
* 204200d73bb135cb7b9b31b8f1ba5268d73094a5 Return password-only preauth 
if passwords are allowed
* 8f356a4305a9aa74aacae36806d6e8ed1b765245 Enable authentication 
indicators for OTP and RADIUS




On Wed, 2016-05-25 at 13:32 +0200, Sumit Bose wrote:

On Tue, May 24, 2016 at 12:21:43PM -0400, Nathaniel McCallum wrote:

New versions again. This time I just removed the stray "TODO:
assign
OID" line in the commit as it no longer applies.

ACK to patches 1-4.

Patch 5 can be committed independently and needs some additional
discussion, see below.

bye,
Sumit


On Tue, 2016-05-24 at 12:08 -0400, Nathaniel McCallum wrote:

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:

...


 From c9e2b50248493fb5a283cf8c88c8e20c312d6348 Mon Sep 17
00:00:00
2001
From: Nathaniel McCallum 
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?



--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-26 Thread Nathaniel McCallum 
Martin, can we get patches 1-4 pushed? I'll submit patch 5 again to the
list after a rebase for further discussion.

On Wed, 2016-05-25 at 13:32 +0200, Sumit Bose wrote:
> On Tue, May 24, 2016 at 12:21:43PM -0400, Nathaniel McCallum wrote:
> > New versions again. This time I just removed the stray "TODO:
> > assign
> > OID" line in the commit as it no longer applies.
> 
> ACK to patches 1-4.
> 
> Patch 5 can be committed independently and needs some additional
> discussion, see below.
> 
> bye,
> Sumit
> 
> > 
> > On Tue, 2016-05-24 at 12:08 -0400, Nathaniel McCallum wrote:
> > > 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:
> 
> ...
> 
> > > > > From c9e2b50248493fb5a283cf8c88c8e20c312d6348 Mon Sep 17
> > > > > 00:00:00
> > > > > 2001
> > > > > From: Nathaniel McCallum 
> > > > > 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?
> > > 

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-25 Thread Sumit Bose 
On Tue, May 24, 2016 at 12:21:43PM -0400, Nathaniel McCallum wrote:
> New versions again. This time I just removed the stray "TODO: assign
> OID" line in the commit as it no longer applies.

ACK to patches 1-4.

Patch 5 can be committed independently and needs some additional
discussion, see below.

bye,
Sumit

> 
> On Tue, 2016-05-24 at 12:08 -0400, Nathaniel McCallum wrote:
> > 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:

...

> > > > From c9e2b50248493fb5a283cf8c88c8e20c312d6348 Mon Sep 17 00:00:00
> > > > 2001
> > > > From: Nathaniel McCallum 
> > > > 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?
> > 

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-24 Thread Nathaniel McCallum 
New versions again. This time I just removed the stray "TODO: assign
OID" line in the commit as it no longer applies.

On Tue, 2016-05-24 at 12:08 -0400, Nathaniel McCallum wrote:
> 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/t
> > > > > ic
> > > > > 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  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 
> > > 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

Re: [Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-24 Thread Nathaniel McCallum 
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  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 
> > 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 
> > 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 
> > Date: Sun, 21 Feb 2016 19:43:52 -0500
> > Subject: [PATCH 3/5] Return password-only preauth if passwords are
> > allowed
> > 
>

Re: [Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-24 Thread Simo Sorce 
On Tue, 2016-05-24 at 15:25 +0200, Sumit Bose wrote:
> >  #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 ?


@@ -446,6 +446,9 @@ IPA Extensions and Controls OIDs
 
 2.16.840.1.113730.3.8.10.6 Token Resynchronization Control OID
 
+2.16.840.1.113730.3.8.10.7 Token Required Control OID
+Control to signal an OTP bind is required
+
 


-- 
Simo Sorce * Red Hat, Inc * New York

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-24 Thread Sumit Bose 
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/ticket/
> > > 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  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 
> 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.

> From 356daafb001bd868f37f6f0b58338bd0c4da135c Mon Sep 17 00:00:00 2001
> From: Nathaniel McCallum 
> 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 
> 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 
> 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

Re: [Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-12 Thread Nathaniel McCallum 
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/ticket/
> > 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!

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  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 
Date: Wed, 4 May 2016 17:08:45 -0400
Subject: [PATCH 5/5] Enable service authentication indicator management

https://fedorahosted.org/freeipa/ticket/433
---
 API.txt   |  9 ++---
 VERSION   |  4 ++--
 ipalib/plugins/service.py | 10 +-
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/API.txt b/API.txt
index b2aec7313b6b9496179beddb68e4a0f5a09608bf..7bf4cba0d29e89afbfd465f3f30d9c3de7701465 100644
--- a/API.txt
+++ b/API.txt
@@ -3888,7 +3888,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
 command: service_add
-args: 1,11,3
+args: 1,12,3
 arg: Str('krbprincipalname', attribute=True, cli_name='principal', multivalue=False, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -3896,6 +3896,7 @@ option: Flag('force', autofill=True, default=False)
 option: StrEnum('ipakrbauthzdata', attribute=True, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE'))
 option: Bool('ipakrbokasdelegate', attribute=False, cli_name='ok_as_delegate', multivalue=False, required=False)
 option: Bool('ipakrbrequirespreauth', attribute=False, cli_name='requires_pre_auth', multivalue=False, required=False)
+option: StrEnum('krbprincipalauthind', attribute=True, cli_name='auth_ind', multivalue=True, required=False, values=(u'otp', u'radius'))
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
@@ -3998,10 +3999,11 @@ output: Output('completed', , None)
 output: Output('failed', , None)
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 command: service_find
-args: 1,11,4
+args: 1,12,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True,

Re: [Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-11 Thread Pavel Vomacka 



On 05/06/2016 02:44 PM, 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/ticket/5872
I've already sent the patch to the ML. I use the API doc string as a 
tooltip for the authentication indicator field in webui and now there is 
only "Authentication indicator whitelist" and I think that we might 
provide more information in webui. So there are two solutions I can 
write my own tooltip text or we can extend the API doc string for this 
new option. Actually, there is another one - leave it as it is. What do 
you think would be better?


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).
I tried to apply your patches and the last patch (93) needs change in 
VERSION file. IPA_API_VERSION_MINOR is the same as in master. So it 
needs to be incremented.


Pavel^3


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.

bye,
Sumit


Nathaniel


--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-06 Thread Sumit Bose 
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/ticket/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.

bye,
Sumit

> 
> Nathaniel

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCHES 0089-0093] Authentication Indicators

2016-05-04 Thread Nathaniel McCallum 
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/ticket/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).

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.

NathanielFrom 047a8846fb5582ac1a1451c106ebf74079c3609f Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Wed, 4 May 2016 17:08:45 -0400
Subject: [PATCH 5/5] Enable managing authentication indicators on services

TODO: actually test this patch
---
 API.txt   | 9 ++---
 VERSION   | 4 ++--
 ipalib/plugins/service.py | 8 
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/API.txt b/API.txt
index 3598b08198cae536754259f7463669052efa3f86..9a563520db7b171ff655e081358876259d4a2753 100644
--- a/API.txt
+++ b/API.txt
@@ -3862,7 +3862,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
 command: service_add
-args: 1,11,3
+args: 1,12,3
 arg: Str('krbprincipalname', attribute=True, cli_name='principal', multivalue=False, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -3870,6 +3870,7 @@ option: Flag('force', autofill=True, default=False)
 option: StrEnum('ipakrbauthzdata', attribute=True, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE'))
 option: Bool('ipakrbokasdelegate', attribute=False, cli_name='ok_as_delegate', multivalue=False, required=False)
 option: Bool('ipakrbrequirespreauth', attribute=False, cli_name='requires_pre_auth', multivalue=False, required=False)
+option: StrEnum('krbprincipalauthind', attribute=True, cli_name='auth_ind', multivalue=True, required=False, values=(u'otp', u'radius'))
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
@@ -3972,10 +3973,11 @@ output: Output('completed', , None)
 output: Output('failed', , None)
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 command: service_find
-args: 1,11,4
+args: 1,12,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, query=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE'))
+option: StrEnum('krbprincipalauthind', attribute=True, autofill=False, cli_name='auth_ind', multivalue=True, query=True, required=False, values=(u'otp', u'radius'))
 option: Str('krbprincipalname', attribute=True, autofill=False, cli_name='principal', multivalue=False, primary_key=True, query=True, required=False)
 option: Str('man_by_host*', cli_name='man_by_hosts', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
@@ -3990,7 +3992,7 @@ output: ListOfEntries('result', (, ), Gettext('A list
 output: Output('summary', (, ), None)
 output: Output('truncated', , None)
 command: service_mod
-args: 1,12,3
+args: 1,13,3
 arg: Str('krbprincipalname', attribute=True, cli_name='principal', multivalue=False, primary_key=True, query=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -3998,6 +4000,7 @@ option: Str('delattr*', cli_name='delattr', exclude='webui')
 option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE'))
 option: Bool('ipakrbokasdelegate', attribute=False, autofill=False, cli_name='ok_as_delegate', multivalue=False, required=False)
 option: Bool('ipakrbrequirespreauth', attribute=False, autofill=False, cli_name='requires_pre_auth', multivalue=False, required=False)
+option: StrEnum('krbprincipalauthind', attribute=True, autofill=False,