The background of this email is this bug:
https://fedorahosted.org/freeipa/ticket/4456

Attached are two patches which solve this issue for admin users (not
very helpful, I know). They depend on this fix in 389:
https://fedorahosted.org/389/ticket/47920

There are two outstanding issues:

1. 389 does not send the post read control for normal users. The
operation itself succeeds, but no control is sent.

The relevant sections from the log are attached. 389 is denying access
to the following attributes (* = valid, ! = invalid):
! objectClass
! ipatokenOTPalgorithm
! ipatokenOTPdigits
* ipatokenOTPkey
* ipatokenHOTPcounter
! ipatokenOwner
! managedBy
! ipatokenUniqueID

The ACIs allowing access to most of these attributes are here:
https://git.fedorahosted.org/cgit/freeipa.git/tree/install/share/default-aci.ldif#n90

Note that I am able to query the entry just fine (including all the
above invalidly restricted attributes). Hence, I know the ACIs are
working just fine.

Part of the strange thing is that in the post read control request, I
haven't indicated that I want *any* attributes returned (i.e. I want
just the DN). So I'm not sure why it is querying all the attributes. I
would suspect that the proper behavior would be to only check the ACIs
on attributes that will actually be returned.

2. The second patch (0002) modifies the ACI for normal user token
addition from this:

aci: (target = "ldap:///ipatokenuniqueid=*,cn=otp,$SUFFIX";)(targetfilter
= "(objectClass=ipaToken)")(version 3.0; acl "Users can create
self-managed tokens"; allow (add) userattr = "ipatokenOwner#SELFDN" and
userattr = "managedBy#SELFDN";)

To this:

aci: (target = "ldap:///ipatokenuniqueid=autogenerate,cn=otp,
$SUFFIX")(targetfilter = "(objectClass=ipaToken)")(version 3.0; acl
"Users can create self-managed tokens"; allow (add) userattr =
"ipatokenOwner#SELFDN" and userattr = "managedBy#SELFDN";)

The idea is to allow users to create tokens which will be expanded by
the UUID plugin. Unfortunately, after the UUID is expanded, the ACIs are
checked. Since the expanded UUID no longer matches the ACI, the addition
is denied. Is this truly the correct behavior? I would think that the
ACIs would be checked before the UUID plugin, not after.
From 7e9d847ec2d9b1b3829abbf3ec6961091d378fc7 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Wed, 8 Oct 2014 16:20:21 -0400
Subject: [PATCH 2/2] Use UUID plugin to generate ipaTokenUniqueIDs

This lets us to deny custom ipaTokenUniqueIDs to non-admin users.

https://fedorahosted.org/freeipa/ticket/4456
---
 install/share/Makefile.am                |  1 +
 install/share/default-aci.ldif           |  2 +-
 install/share/uuid-ipatokenuniqueid.ldif | 11 ++++++++
 install/updates/40-otp.update            |  3 +-
 ipalib/plugins/otptoken.py               | 47 ++++++++++++++++++--------------
 ipaserver/install/dsinstance.py          |  1 +
 6 files changed, 42 insertions(+), 23 deletions(-)
 create mode 100644 install/share/uuid-ipatokenuniqueid.ldif

diff --git a/install/share/Makefile.am b/install/share/Makefile.am
index 7d8ceb60e6374e133cfb6e3684bc307dbf313ce7..19bea40c872f148a3a7b8dcafca6e576429e3ace 100644
--- a/install/share/Makefile.am
+++ b/install/share/Makefile.am
@@ -63,6 +63,7 @@ app_DATA =				\
 	user_private_groups.ldif	\
 	host_nis_groups.ldif		\
 	uuid-ipauniqueid.ldif		\
+	uuid-ipatokenuniqueid.ldif	\
 	modrdn-krbprinc.ldif		\
 	entryusn.ldif			\
 	root-autobind.ldif		\
diff --git a/install/share/default-aci.ldif b/install/share/default-aci.ldif
index af7eedb0b92375f893a61ad1fb6e2d7b176389f9..7b6519b291dbaaa075e949317154c047da8f32ce 100644
--- a/install/share/default-aci.ldif
+++ b/install/share/default-aci.ldif
@@ -96,4 +96,4 @@ aci: (targetfilter = "(objectClass=ipatokenTOTP)")(targetattrs = "ipatokenOTPalg
 aci: (targetfilter = "(objectClass=ipatokenHOTP)")(targetattrs = "ipatokenOTPalgorithm || ipatokenOTPdigits")(version 3.0; acl "Users/managers can see HOTP details"; allow (read, search, compare) userattr = "ipatokenOwner#USERDN" or userattr = "managedBy#USERDN";)
 aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs = "description || ipatokenDisabled || ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel || ipatokenSerial")(version 3.0; acl "Managers can write basic token info"; allow (write) userattr = "managedBy#USERDN";)
 aci: (targetfilter = "(objectClass=ipaToken)")(version 3.0; acl "Managers can delete tokens"; allow (delete) userattr = "managedBy#USERDN";)
-aci: (target = "ldap:///ipatokenuniqueid=*,cn=otp,$SUFFIX";)(targetfilter = "(objectClass=ipaToken)")(version 3.0; acl "Users can create self-managed tokens"; allow (add) userattr = "ipatokenOwner#SELFDN" and userattr = "managedBy#SELFDN";)
+aci: (target = "ldap:///ipatokenuniqueid=autogenerate,cn=otp,$SUFFIX";)(targetfilter = "(objectClass=ipaToken)")(version 3.0; acl "Users can create self-managed tokens"; allow (add) userattr = "ipatokenOwner#SELFDN" and userattr = "managedBy#SELFDN";)
diff --git a/install/share/uuid-ipatokenuniqueid.ldif b/install/share/uuid-ipatokenuniqueid.ldif
new file mode 100644
index 0000000000000000000000000000000000000000..738f57f5f0534eb6d2a883fa888eea96c9ad28ec
--- /dev/null
+++ b/install/share/uuid-ipatokenuniqueid.ldif
@@ -0,0 +1,11 @@
+# add plugin configuration for ipatokenuniqueid
+dn: cn=IPA Token Unique IDs,cn=IPA UUID,cn=plugins,cn=config
+changetype: add
+objectclass: top
+objectclass: extensibleObject
+cn: IPA Token Unique IDs
+ipaUuidAttr: ipaTokenUniqueID
+ipaUuidMagicRegen: autogenerate
+ipaUuidFilter: (objectclass=ipaToken)
+ipaUuidScope: $SUFFIX
+ipaUuidEnforce: FALSE
diff --git a/install/updates/40-otp.update b/install/updates/40-otp.update
index caa21c380618dd6450205b0883c6165e9ac40967..fc75129c0ff9fc61b656cc6da40847866107e496 100644
--- a/install/updates/40-otp.update
+++ b/install/updates/40-otp.update
@@ -9,12 +9,13 @@ remove: aci:'(targetfilter = "(objectClass=ipaToken)")(targetattrs = "objectclas
 remove: aci:'(targetfilter = "(objectClass=ipaToken)")(targetattrs = "ipatokenUniqueID || description || ipatokenOwner || ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel || ipatokenSerial")(version 3.0; acl "Users can write basic token info"; allow (write) userattr = "ipatokenOwner#USERDN";)'
 remove: aci:'(targetfilter = "(objectClass=ipatokenTOTP)")(targetattrs = "ipatokenOTPkey || ipatokenOTPalgorithm || ipatokenOTPdigits || ipatokenTOTPclockOffset || ipatokenTOTPtimeStep")(version 3.0; acl "Users can add TOTP token secrets"; allow (write, search) userattr = "ipatokenOwner#USERDN";)'
 remove: aci:'(targetfilter = "(objectClass=ipatokenHOTP)")(targetattrs = "ipatokenOTPkey || ipatokenOTPalgorithm || ipatokenOTPdigits || ipatokenHOTPcounter")(version 3.0; acl "Users can add HOTP token secrets"; allow (write, search) userattr = "ipatokenOwner#USERDN";)'
+remove: aci:'(target = "ldap:///ipatokenuniqueid=*,cn=otp,$SUFFIX";)(targetfilter = "(objectClass=ipaToken)")(version 3.0; acl "Users can create self-managed tokens"; allow (add) userattr = "ipatokenOwner#SELFDN" and userattr = "managedBy#SELFDN";)'
 add: aci:'(targetfilter = "(objectClass=ipaToken)")(targetattrs = "objectclass || description || managedBy || ipatokenUniqueID || ipatokenDisabled || ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel || ipatokenSerial || ipatokenOwner")(version 3.0; acl "Users/managers can read basic token info"; allow (read, search, compare) userattr = "ipatokenOwner#USERDN" or userattr = "managedBy#USERDN";)'
 add: aci:'(targetfilter = "(objectClass=ipatokenTOTP)")(targetattrs = "ipatokenOTPalgorithm || ipatokenOTPdigits || ipatokenTOTPtimeStep")(version 3.0; acl "Users/managers can see TOTP details"; allow (read, search, compare) userattr = "ipatokenOwner#USERDN" or userattr = "managedBy#USERDN";)'
 add: aci:'(targetfilter = "(objectClass=ipatokenHOTP)")(targetattrs = "ipatokenOTPalgorithm || ipatokenOTPdigits")(version 3.0; acl "Users/managers can see HOTP details"; allow (read, search, compare) userattr = "ipatokenOwner#USERDN" or userattr = "managedBy#USERDN";)'
 add: aci:'(targetfilter = "(objectClass=ipaToken)")(targetattrs = "description || ipatokenDisabled || ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel || ipatokenSerial")(version 3.0; acl "Managers can write basic token info"; allow (write) userattr = "managedBy#USERDN";)'
 add: aci:'(targetfilter = "(objectClass=ipaToken)")(version 3.0; acl "Managers can delete tokens"; allow (delete) userattr = "managedBy#USERDN";)'
-add: aci:'(target = "ldap:///ipatokenuniqueid=*,cn=otp,$SUFFIX";)(targetfilter = "(objectClass=ipaToken)")(version 3.0; acl "Users can create self-managed tokens"; allow (add) userattr = "ipatokenOwner#SELFDN" and userattr = "managedBy#SELFDN";)'
+add: aci:'(target = "ldap:///ipatokenuniqueid=autogenerate,cn=otp,$SUFFIX";)(targetfilter = "(objectClass=ipaToken)")(version 3.0; acl "Users can create self-managed tokens"; allow (add) userattr = "ipatokenOwner#SELFDN" and userattr = "managedBy#SELFDN";)'
 
 dn: cn=radiusproxy,$SUFFIX
 default: objectClass: nsContainer
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index 1bd85d4b952dc51ea800ed37c49b3c50aeb31492..48215ecfd956585c7920efdcfd8d5802abfbf317 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -254,10 +254,10 @@ class otptoken_add(LDAPCreate):
     )
 
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
-        # Fill in a default UUID when not specified.
+        # Generate a UUID when not specified.
         if entry_attrs.get('ipatokenuniqueid', None) is None:
-            entry_attrs['ipatokenuniqueid'] = str(uuid.uuid4())
-            dn = DN("ipatokenuniqueid=%s" % entry_attrs['ipatokenuniqueid'], dn)
+            entry_attrs['ipatokenuniqueid'] = u'autogenerate'
+            dn = DN("ipatokenuniqueid=autogenerate", dn)
 
         if not _check_interval(options.get('ipatokennotbefore', None),
                                options.get('ipatokennotafter', None)):
@@ -285,36 +285,41 @@ class otptoken_add(LDAPCreate):
         # Resolve the owner's dn
         _normalize_owner(self.api.Object.user, entry_attrs)
 
+        return dn
+
+    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
+        # Get the key
+        key = entry_attrs.get('ipatokenotpkey', [options['ipatokenotpkey'],])[0]
+
         # Get the issuer for the URI
-        owner = entry_attrs.get('ipatokenowner', None)
         issuer = api.env.realm
-        if owner is not None:
-            try:
-                issuer = ldap.get_entry(owner, ['krbprincipalname'])['krbprincipalname'][0]
-            except (NotFound, IndexError):
-                pass
+        try:
+            owner = entry_attrs.get('ipatokenowner', [])[0]
+            issuer = ldap.get_entry(owner, ['krbprincipalname'])['krbprincipalname'][0]
+        except (NotFound, IndexError):
+            pass
 
         # Build the URI parameters
         args = {}
         args['issuer'] = issuer
-        args['secret'] = base64.b32encode(entry_attrs['ipatokenotpkey'])
-        args['digits'] = entry_attrs['ipatokenotpdigits']
-        args['algorithm'] = entry_attrs['ipatokenotpalgorithm']
+        args['secret'] = base64.b32encode(key)
+        args['digits'] = entry_attrs['ipatokenotpdigits'][0]
+        args['algorithm'] = entry_attrs['ipatokenotpalgorithm'][0]
         if options['type'] == 'totp':
-            args['period'] = entry_attrs['ipatokentotptimestep']
+            args['period'] = entry_attrs['ipatokentotptimestep'][0]
         elif options['type'] == 'hotp':
-            args['counter'] = entry_attrs['ipatokenhotpcounter']
+            args['counter'] = entry_attrs['ipatokenhotpcounter'][0]
 
         # Build the URI
-        label = urllib.quote(entry_attrs['ipatokenuniqueid'])
+        label = urllib.quote(entry_attrs['ipatokenuniqueid'][0])
         parameters = urllib.urlencode(args)
-        uri = u'otpauth://%s/%s:%s?%s' % (options['type'], issuer, label, parameters)
-        setattr(context, 'uri', uri)
+        entry_attrs['uri'] = u'otpauth://%s/%s:%s?%s' % (
+            options['type'],
+            issuer,
+            label,
+            parameters
+        )
 
-        return dn
-
-    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
-        entry_attrs['uri'] = getattr(context, 'uri')
         _convert_owner(self.api.Object.user, entry_attrs, options)
         return super(otptoken_add, self).post_callback(ldap, dn, entry_attrs, *keys, **options)
 
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index c9a1c15e9495108382f6e2e8a58f6cc4e8f79c98..25733c1cf8fe83b128b46a90d3eeadec9158114e 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -540,6 +540,7 @@ class DsInstance(service.Service):
     def __config_uuid_module(self):
         self._ldap_mod("uuid-conf.ldif")
         self._ldap_mod("uuid-ipauniqueid.ldif", self.sub_dict)
+        self._ldap_mod("uuid-ipatokenuniqueid.ldif", self.sub_dict)
 
     def __config_modrdn_module(self):
         self._ldap_mod("modrdn-conf.ldif")
-- 
2.1.0

From 37f7dad90ed58c2adf522322fdc535d6235804a2 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Wed, 8 Oct 2014 14:39:18 -0400
Subject: [PATCH 1/2] Use post read control to get new DN after add

This will handle the case where the DN is changed as a result of
the add operation. The most common case of this is the IPA UUID
plugin.
---
 ipalib/plugins/baseldap.py |  2 +-
 ipapython/ipaldap.py       | 10 +++++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index e589a53219766a2aa41bbb4abef7fc3a12fcc267..f8c1b82fd03af471a4711088d556705ad01847f6 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1133,7 +1133,7 @@ class LDAPCreate(BaseLDAPCommand, crud.Create):
         _check_limit_object_class(self.api.Backend.ldap2.schema.attribute_types(self.obj.disallow_object_classes), entry_attrs.keys(), allow_only=False)
 
         try:
-            self._exc_wrapper(keys, options, ldap.add_entry)(entry_attrs)
+            entry_attrs.dn = self._exc_wrapper(keys, options, ldap.add_entry)(entry_attrs)
         except errors.NotFound:
             parent = self.obj.parent_object
             if parent:
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 1702daa253d7eb568c27f66fda1810b4661656ad..9f6d4a9cbc4d965265cd0a86cdc8817aee566662 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -45,6 +45,7 @@ from ipapython.ipautil import (
 from ipapython.ipa_log_manager import log_mgr
 from ipapython.dn import DN, RDN
 from ipapython.dnsutil import DNSName
+from ldap.controls.readentry import PostReadControl
 
 # Global variable to define SASL auth
 SASL_GSSAPI = ldap.sasl.sasl({}, 'GSSAPI')
@@ -1588,11 +1589,18 @@ class LDAPClient(object):
         # remove all [] values (python-ldap hates 'em)
         attrs = dict((k, v) for k, v in entry.raw.iteritems() if v)
 
+        ctrl = PostReadControl()
         with self.error_handler():
-            self.conn.add_s(entry.dn, attrs.items())
+            ctrls = self.conn.add_ext_s(entry.dn, attrs.items(), [ctrl])[3]
 
         entry.reset_modlist()
 
+        for ctrl in ctrls:
+            if isinstance(ctrl, PostReadControl):
+                return DN(ctrl.dn)
+
+        return entry.dn
+
     def update_entry_rdn(self, dn, new_rdn, del_old=True):
         """
         Update entry's relative distinguished name.
-- 
2.1.0

[08/Oct/2014:16:54:39 -0400] NSACLPlugin - conn=24 op=1 (main): Deny read on 
entry(ipatokenuniqueid=52001946-4f2d-11e4-9127-7831c1d63a78,cn=otp,dc=example,dc=com).attr(objectClass)
 to uid=otp,cn=users,cn=accounts,dc=example,dc=com: no aci matched the subject 
by aci(19): aciname= "Admin can manage any entry", acidn="dc=example,dc=com"
[08/Oct/2014:16:54:39 -0400] NSACLPlugin - conn=24 op=1 (main): Deny read on 
entry(ipatokenuniqueid=52001946-4f2d-11e4-9127-7831c1d63a78,cn=otp,dc=example,dc=com).attr(ipatokenOTPalgorithm)
 to uid=otp,cn=users,cn=accounts,dc=example,dc=com: no aci matched the subject 
by aci(19): aciname= "Admin can manage any entry", acidn="dc=example,dc=com"
[08/Oct/2014:16:54:39 -0400] NSACLPlugin - conn=24 op=1 (main): Deny read on 
entry(ipatokenuniqueid=52001946-4f2d-11e4-9127-7831c1d63a78,cn=otp,dc=example,dc=com).attr(ipatokenOTPdigits)
 to uid=otp,cn=users,cn=accounts,dc=example,dc=com: no aci matched the subject 
by aci(19): aciname= "Admin can manage any entry", acidn="dc=example,dc=com"
[08/Oct/2014:16:54:39 -0400] NSACLPlugin - conn=24 op=1 (main): Deny read on 
entry(ipatokenuniqueid=52001946-4f2d-11e4-9127-7831c1d63a78,cn=otp,dc=example,dc=com).attr(ipatokenOTPkey)
 to uid=otp,cn=users,cn=accounts,dc=example,dc=com: no aci matched the subject 
by aci(19): aciname= "Admin can manage any entry", acidn="dc=example,dc=com"
[08/Oct/2014:16:54:39 -0400] NSACLPlugin - conn=24 op=1 (main): Deny read on 
entry(ipatokenuniqueid=52001946-4f2d-11e4-9127-7831c1d63a78,cn=otp,dc=example,dc=com).attr(ipatokenHOTPcounter)
 to uid=otp,cn=users,cn=accounts,dc=example,dc=com: no aci matched the subject 
by aci(19): aciname= "Admin can manage any entry", acidn="dc=example,dc=com"
[08/Oct/2014:16:54:39 -0400] NSACLPlugin - conn=24 op=1 (main): Deny read on 
entry(ipatokenuniqueid=52001946-4f2d-11e4-9127-7831c1d63a78,cn=otp,dc=example,dc=com).attr(ipatokenOwner)
 to uid=otp,cn=users,cn=accounts,dc=example,dc=com: no aci matched the subject 
by aci(19): aciname= "Admin can manage any entry", acidn="dc=example,dc=com"
[08/Oct/2014:16:54:40 -0400] NSACLPlugin - conn=24 op=1 (main): Deny read on 
entry(ipatokenuniqueid=52001946-4f2d-11e4-9127-7831c1d63a78,cn=otp,dc=example,dc=com).attr(managedBy)
 to uid=otp,cn=users,cn=accounts,dc=example,dc=com: no aci matched the subject 
by aci(19): aciname= "Admin can manage any entry", acidn="dc=example,dc=com"
[08/Oct/2014:16:54:40 -0400] NSACLPlugin - conn=24 op=1 (main): Deny read on 
entry(ipatokenuniqueid=52001946-4f2d-11e4-9127-7831c1d63a78,cn=otp,dc=example,dc=com).attr(ipatokenUniqueID)
 to uid=otp,cn=users,cn=accounts,dc=example,dc=com: no aci matched the subject 
by aci(19): aciname= "Admin can manage any entry", acidn="dc=example,dc=com"
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to