[Freeipa-devel] [HELP] Regular users should not be able to add OTP tokens with custom name

[Martin Kosek]

On 10/10/2014 05:43 PM, Nathaniel McCallum wrote:
> As a result of this ongoing conversation, I have opened two 389 bugs:
> 
> 1. Post Read - https://fedorahosted.org/389/ticket/47924
> 2. UUID ACIs - https://fedorahosted.org/389/ticket/47925
> 
> On Wed, 2014-10-08 at 17:46 -0400, Nathaniel McCallum wrote:
>> 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.

Given the number of issues we have with this patch on the DS side, it is likely
we will need to go some simpler way for FreeIPA 4.1, in terms just of hiding
the option where appropriate.

Also, few comments to your current patch set (though the patches themselves
will probably not land in 4.1):

Patch 0001:
- while it may work (after DS fixes), it adds PostRead for all our commands,
not just otptoken-add. I would rather have some attribute like
"read_dn_from_postread" on LDAPObject which defaults to False to make sure
there is no regression or performance hit with other LDAP calls.
- Wider adoption of the PostRead call would be left for future, when we stop
doing post-execute LDAP read call and only rely on PostRead result.

Patch 0002:
- "cn=IPA Token Unique IDs,cn=IPA UUID,cn=plugins,cn=config" should be created
in LDAP update files only. Currently it will not be created on upgrades.

Martin