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