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 <npmccal...@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 <npmccal...@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 <npmccal...@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 <npmccal...@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 <npmccal...@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.
From 6fc89d231ea0ccf062837c5477cf537075ea759e Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
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', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), 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', <type 'int'>, None)
 output: Output('failed', <type 'dict'>, None)
 output: Entry('result', <type 'dict'>, 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')
@@ -4016,7 +4018,7 @@ output: ListOfEntries('result', (<type 'list'>, <type 'tuple'>), Gettext('A list
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('truncated', <type 'bool'>, 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')
@@ -4024,6 +4026,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, 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: Flag('rights', autofill=True, default=False)
diff --git a/VERSION b/VERSION
index 36d3fe267d9d8e8da54f3baa9f0038bcb12bfaae..1eb126c7046e6f16cb49bfd6cc00f4df24cfc51f 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=166
-# Last change: tbabej - idviews: Add user certificate attribute to user ID overrides
+IPA_API_VERSION_MINOR=167
+# Last change: npmccallum - enable setting authinds on services
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index 644b07f463a008eff12d3ff36de9404c03cf8a69..1f7159d8619091f0ee7bceeb85b0faa35eba0004 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -411,7 +411,7 @@ class service(LDAPObject):
     permission_filter_objectclasses = ['ipaservice']
     search_attributes = ['krbprincipalname', 'managedby', 'ipakrbauthzdata']
     default_attributes = ['krbprincipalname', 'usercertificate', 'managedby',
-        'ipakrbauthzdata', 'memberof', 'ipaallowedtoperform']
+        'ipakrbauthzdata', 'memberof', 'ipaallowedtoperform', 'krbprincipalauthind']
     uuid_attribute = 'ipauniqueid'
     attribute_members = {
         'managedby': ['host'],
@@ -506,6 +506,14 @@ class service(LDAPObject):
             values=(u'MS-PAC', u'PAD', u'NONE'),
             csv=True,
         ),
+        StrEnum('krbprincipalauthind',
+            cli_name='auth_ind',
+            label=_('Authentication Indicators'),
+            doc=_('Authentication indicator whitelist'),
+            values=(u'otp', u'radius'),
+            multivalue=True,
+            required=False,
+        ),
     ) + ticket_flags_params
 
     def validate_ipakrbauthzdata(self, entry):
-- 
2.8.3

From 54f1ebdc8aa0259334b063463472ffeda20594c6 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Sun, 21 Feb 2016 19:44:19 -0500
Subject: [PATCH 4/5] Enable authentication indicators for OTP and RADIUS

If the user is configured for OTP or RADIUS authentication, insert the
relevant authentication indicator.

https://fedorahosted.org/freeipa/ticket/433
---
 daemons/ipa-kdb/ipa_kdb_principals.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index 910d55c4a3e3eecedc1f32eb9f724ae76f40aa57..d4adf27f2de7c7ccd050063e779a30fdae35bc83 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -512,7 +512,8 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
                                               krb5_db_entry **kentry,
                                               uint32_t *polmask)
 {
-    krb5_octet otp_string[] = {'o', 't', 'p', 0, '[', ']', 0 };
+    const krb5_octet rad_string[] = "otp\0[{\"indicators\": [\"radius\"]}]";
+    const krb5_octet otp_string[] = "otp\0[{\"indicators\": [\"otp\"]}]";
     struct ipadb_context *ipactx;
     enum ipadb_user_auth ua;
     LDAP *lcontext;
@@ -842,11 +843,16 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
     }
 
     /* If enabled, set the otp user string, enabling otp. */
-    if (ua & (IPADB_USER_AUTH_RADIUS | IPADB_USER_AUTH_OTP)) {
+    if (ua & IPADB_USER_AUTH_OTP) {
         kerr = ipadb_set_tl_data(entry, KRB5_TL_STRING_ATTRS,
                                  sizeof(otp_string), otp_string);
         if (kerr)
             goto done;
+    } else if (ua & IPADB_USER_AUTH_RADIUS) {
+        kerr = ipadb_set_tl_data(entry, KRB5_TL_STRING_ATTRS,
+                                 sizeof(rad_string), rad_string);
+        if (kerr)
+            goto done;
     }
 
     kerr = 0;
-- 
2.8.3

From 6d3b4a4a192d76bd8bc4e5109a00912513454024 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Sun, 21 Feb 2016 19:43:52 -0500
Subject: [PATCH 3/5] Return password-only preauth if passwords are allowed

Before this patch, if either password or password+otp were permitted,
only the otp preauth mech would be returned to the client. Now, the
client will receive either enc_ts or enc_chl in addition to otp.

https://fedorahosted.org/freeipa/ticket/433
---
 daemons/ipa-kdb/ipa_kdb_principals.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index 50278108769c9032a277f0b53f14380ad4257566..910d55c4a3e3eecedc1f32eb9f724ae76f40aa57 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -302,6 +302,8 @@ static void ipadb_validate_radius(struct ipadb_context *ipactx,
                                "ipatokenRadiusConfigLink");
     if (vals == NULL || vals[0] == NULL)
         *ua &= ~IPADB_USER_AUTH_RADIUS;
+    else
+        *ua = IPADB_USER_AUTH_RADIUS;
 
     if (vals != NULL)
         ldap_value_free_len(vals);
@@ -314,10 +316,6 @@ static void ipadb_validate_password(struct ipadb_context *ipactx,
     /* If no mechanisms are set, use password. */
     if (*ua == IPADB_USER_AUTH_NONE)
         *ua |= IPADB_USER_AUTH_PASSWORD;
-
-    /* If any other mechanism has passed validation, don't use password. */
-    else if (*ua & ~IPADB_USER_AUTH_PASSWORD)
-        *ua &= ~IPADB_USER_AUTH_PASSWORD;
 }
 
 static enum ipadb_user_auth ipadb_get_user_auth(struct ipadb_context *ipactx,
-- 
2.8.3

From 534f2d6cd6bea25712aaf8faec46c03786d4b118 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Thu, 12 May 2016 15:10:47 -0400
Subject: [PATCH 2/5] Ensure that ipa-otpd bind auths validate an OTP

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..022525b786705b4f58f861bc3b0a745ab8693755 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 c38d4915aa11dc329c1b2d08949eb1141117d5ae..4cbc17b998fd52e68d3c9b6baa83b2d7e93097c5 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h
@@ -55,6 +55,9 @@
  */
 #define OTP_SYNC_REQUEST_OID "2.16.840.1.113730.3.8.10.6"
 
+/* This control has no data. */
+#define OTP_REQUIRED_OID "2.16.840.1.113730.3.8.10.7"
+
 bool otpctrl_present(Slapi_PBlock *pb, const char *oid);
 
 bool otpctrl_sync_handle(const struct otp_config *cfg, Slapi_PBlock *pb,
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
index f41b1ac9d5208cff9c6542d639a59f8c8d44e264..5c700211b59e8707fa2d1d27347c73708dbbde61 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
@@ -1172,7 +1172,7 @@ done:
  *       validation.
  */
 static bool ipapwd_pre_bind_otp(const char *bind_dn, Slapi_Entry *entry,
-                                struct berval *creds)
+                                struct berval *creds, bool otpreq)
 {
     uint32_t auth_types;
 
@@ -1204,10 +1204,10 @@ static bool ipapwd_pre_bind_otp(const char *bind_dn, Slapi_Entry *entry,
             return false;
         }
 
-        /* If the user has no active tokens, succeed. */
+        /* With no tokens, succeed if tokens aren't required. */
         if (tokens[0] == NULL) {
             otp_token_free_array(tokens);
-            return true;
+            return !otpreq;
         }
 
         if (otp_token_validate_berval(tokens, creds, NULL)) {
@@ -1218,7 +1218,7 @@ static bool ipapwd_pre_bind_otp(const char *bind_dn, Slapi_Entry *entry,
         otp_token_free_array(tokens);
     }
 
-    return auth_types & OTP_CONFIG_AUTH_TYPE_PASSWORD;
+    return (auth_types & OTP_CONFIG_AUTH_TYPE_PASSWORD) && !otpreq;
 }
 
 static int ipapwd_authenticate(const char *dn, Slapi_Entry *entry,
@@ -1394,6 +1394,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
     char *dn = NULL;
     int method = 0;
     bool syncreq;
+    bool otpreq;
     int ret = 0;
     time_t current_time;
     time_t expire_time;
@@ -1451,7 +1452,8 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
 
     /* Try to do OTP first. */
     syncreq = otpctrl_present(pb, OTP_SYNC_REQUEST_OID);
-    if (!syncreq && !ipapwd_pre_bind_otp(dn, entry, credentials))
+    otpreq = otpctrl_present(pb, OTP_REQUIRED_OID);
+    if (!syncreq && !ipapwd_pre_bind_otp(dn, entry, credentials, otpreq))
         goto invalid_creds;
 
     /* Ensure that there is a password. */
@@ -1488,6 +1490,7 @@ int ipapwd_pre_init(Slapi_PBlock *pb)
     int ret;
 
     slapi_register_supported_control(OTP_SYNC_REQUEST_OID, SLAPI_OPERATION_BIND);
+    slapi_register_supported_control(OTP_REQUIRED_OID, SLAPI_OPERATION_BIND);
 
     ret = slapi_pblock_set(pb, SLAPI_PLUGIN_VERSION, SLAPI_PLUGIN_VERSION_01);
     if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, (void *)&ipapwd_plugin_desc);
-- 
2.8.3

From 872f611a2e602fb255477bf5e09414d7c03d94e8 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Thu, 12 May 2016 14:57:29 -0400
Subject: [PATCH 1/5] Rename syncreq.[ch] to otpctrl.[ch]

This gives us a place to handle all OTP related controls. Also,
genericize otpctrl_present() so that the OID can be specified as an
argument to the function call.

These changes are preparatory for the subsequent patches.

https://fedorahosted.org/freeipa/ticket/433
---
 daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am            |  2 +-
 .../ipa-slapi-plugins/ipa-pwd-extop/{syncreq.c => otpctrl.c}   |  8 ++++----
 .../ipa-slapi-plugins/ipa-pwd-extop/{syncreq.h => otpctrl.h}   | 10 +++++-----
 daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c              |  6 +++---
 4 files changed, 13 insertions(+), 13 deletions(-)
 rename daemons/ipa-slapi-plugins/ipa-pwd-extop/{syncreq.c => otpctrl.c} (94%)
 rename daemons/ipa-slapi-plugins/ipa-pwd-extop/{syncreq.h => otpctrl.h} (92%)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am b/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am
index 078ff9c8476b8547db58d2cf5c5d941846a3143c..46a649187504d6c76965dc72ae0032292a560a81 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am
@@ -47,7 +47,7 @@ libipa_pwd_extop_la_SOURCES = 		\
 	encoding.c			\
 	prepost.c			\
 	ipa_pwd_extop.c			\
-	syncreq.c			\
+	otpctrl.c			\
 	$(KRB5_UTIL_SRCS)		\
 	$(NULL)
 
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.c
similarity index 94%
rename from daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c
rename to daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.c
index 3a31529f7a69c0bb24a1cc845fa4f9da981be138..ce26abeeb9115a52d0f25d7138e11ed54e907204 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.c
@@ -38,19 +38,19 @@
  * END COPYRIGHT BLOCK **/
 
 #include "../libotp/otp_token.h"
-#include "syncreq.h"
+#include "otpctrl.h"
 
-bool sync_request_present(Slapi_PBlock *pb)
+bool otpctrl_present(Slapi_PBlock *pb, const char *oid)
 {
     LDAPControl **controls = NULL;
 
     if (slapi_pblock_get(pb, SLAPI_REQCONTROLS, &controls) != 0)
         return false;
 
-    return ldap_control_find(OTP_SYNC_REQUEST_OID, controls, NULL) != NULL;
+    return ldap_control_find(oid, controls, NULL) != NULL;
 }
 
-bool sync_request_handle(const struct otp_config *cfg, Slapi_PBlock *pb,
+bool otpctrl_sync_handle(const struct otp_config *cfg, Slapi_PBlock *pb,
                          const char *user_dn)
 {
     struct otp_token **tokens = NULL;
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.h b/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h
similarity index 92%
rename from daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.h
rename to daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h
index 98a97c4c9f6d2e6bf74f97fc93053b3aebbc7821..c38d4915aa11dc329c1b2d08949eb1141117d5ae 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.h
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h
@@ -38,8 +38,8 @@
  * END COPYRIGHT BLOCK **/
 
 
-#ifndef SYNCREQ_H_
-#define SYNCREQ_H_
+#ifndef OTPCTRL_H_
+#define OTPCTRL_H_
 
 #include "../libotp/otp_config.h"
 #include <stdbool.h>
@@ -55,9 +55,9 @@
  */
 #define OTP_SYNC_REQUEST_OID "2.16.840.1.113730.3.8.10.6"
 
-bool sync_request_present(Slapi_PBlock *pb);
+bool otpctrl_present(Slapi_PBlock *pb, const char *oid);
 
-bool sync_request_handle(const struct otp_config *cfg, Slapi_PBlock *pb,
+bool otpctrl_sync_handle(const struct otp_config *cfg, Slapi_PBlock *pb,
                          const char *user_dn);
 
-#endif /* SYNCREQ_H_ */
+#endif /* OTPCTRL_H_ */
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
index c1fc7fe3345e227054b8118b35d8f504fe957ea6..f41b1ac9d5208cff9c6542d639a59f8c8d44e264 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
@@ -62,7 +62,7 @@
 
 #include "ipapwd.h"
 #include "util.h"
-#include "syncreq.h"
+#include "otpctrl.h"
 
 #define IPAPWD_OP_NULL 0
 #define IPAPWD_OP_ADD 1
@@ -1450,7 +1450,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
     }
 
     /* Try to do OTP first. */
-    syncreq = sync_request_present(pb);
+    syncreq = otpctrl_present(pb, OTP_SYNC_REQUEST_OID);
     if (!syncreq && !ipapwd_pre_bind_otp(dn, entry, credentials))
         goto invalid_creds;
 
@@ -1466,7 +1466,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
     }
 
     /* Attempt to handle a token synchronization request. */
-    if (syncreq && !sync_request_handle(otp_config, pb, dn))
+    if (syncreq && !otpctrl_sync_handle(otp_config, pb, dn))
         goto invalid_creds;
 
     /* Attempt to write out kerberos keys for the user. */
-- 
2.8.3

-- 
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

Reply via email to