[Freeipa-devel] [PATCH 0049] Add support for protected tokens

[Rob Crittenden]

Nathaniel McCallum wrote:
> On Thu, 2014-05-08 at 13:51 -0400, Simo Sorce wrote:
>> On Thu, 2014-05-08 at 12:26 -0400, Nathaniel McCallum wrote:
>>> On Wed, 2014-05-07 at 11:17 -0400, Simo Sorce wrote:
>>>> On Wed, 2014-05-07 at 09:54 -0400, Dmitri Pal wrote:
>>>>> On 05/07/2014 09:05 AM, Nathaniel McCallum wrote:
>>>>>> On Wed, 2014-05-07 at 11:42 +0200, Jan Cholasta wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 6.5.2014 17:08, Nathaniel McCallum wrote:
>>>>>>>> On Tue, 2014-05-06 at 09:49 -0400, Nathaniel McCallum wrote:
>>>>>>>>> On Mon, 2014-05-05 at 12:42 -0400, Nathaniel McCallum wrote:
>>>>>>>>>> This also constitutes a rethinking of the token ACIs after the
>>>>>>>>>> introduction of SELFDN support.
>>>>>>>>>>
>>>>>>>>>> Admins, as before, have full access to all token permissions.
>>>>>>>>>>
>>>>>>>>>> Normal users have read/search/compare access to all of the non-secret
>>>>>>>>>> data for tokens assigned to them, whether protected or non-protected.
>>>>>>>>>> Users can add or delete non-protected tokens and modify most of their
>>>>>>>>>> metadata. However they cannot create, delete or modify protected tokens.
>>>>>>>>>> Regardless of whether the token is protected or not, users cannot change
>>>>>>>>>> a token's ownership or unique identity.
>>>>>>>>>>
>>>>>>>>>> In contrast, admins can create protected tokens. This protects the token
>>>>>>>>>> from deletion or modification when assigned to users. Additionally, when
>>>>>>>>>> a user account is deleted, the assigned non-protected tokens are deleted
>>>>>>>>>> but the protected tokens are merely orphaned. This permits the token to
>>>>>>>>>> be reassigned without having to recreate it. This last point is
>>>>>>>>>> particularly useful in the case of hardware tokens.
>>>>>>>>>>
>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/4228
>>>>>>>>>>
>>>>>>>>>> NOTE: This patch depends on my patch 0048.
>>>>>>>>> This new version makes ipatokenDisabled visible for token owners. It is
>>>>>>>>> also writable if the token is non-protected. This additionally fixes:
>>>>>>>>>
>>>>>>>>> https://fedorahosted.org/freeipa/ticket/4259
>>>>>>>> This new version changes the way the default value of protected is setup
>>>>>>>> in accordance with the changes made for the review of my patch 0048.2.
>>>>>>>>
>>>>>>>> Nathaniel
>>>>>>> Is using the ipatokenprotected attribute the final design?
>>>>>> No. Alternate designs are welcome. The code is easy enough to modify.
>>>>>>
>>>>>>> I did not dig too deep into this, but I think it might be better to
>>>>>>> instead use the managedby attribute on a token to limit who can delete
>>>>>>> (or administer in other way) the token. On otptoken-add, managedby would
>>>>>>> be set to the "whoami" user DN, unless run with --protected, in which
>>>>>>> case managedby would be left empty. Then, when deleting a user, the
>>>>>>> token would be deleted only if the user manages the token.
>>>>>> It seems to me that the mechanics of this are roughly the same as
>>>>>> protected, just with a different syntax. The cost of this is more
>>>>>> complex ACIs. In particular, we'd have to use two userdn clauses (is
>>>>>> this possible?) instead of a simple filter. If there is a clear benefit,
>>>>>> we can justify the more obtuse syntax.
>>>>>
>>>>> We usually try not to create new attributes until it is fully justified.
>>>>> I would like Simo to chime in on this.
>>>>
>>>> I would also prefer to reuse existing attributes and mechanism if
>>>> possible and if it will not preclude future work.
>>>>
>>>> In this case, it "sounds" like managed-by has the appropriate meaning:
>>>> "who manages the token ?" -> myself, admin, other, none ?
>>>>
>>>> Nathaniel can you send 2 lines showing the difference in ACIs between
>>>> using managed-by vs a new attribute ?
>>>
>>> These are the ACIs using the protected mechanism:
>>>
>>> aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs =
>>> "objectclass || description || ipatokenUniqueID || ipatokenDisabled ||
>>> ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel
>>> || ipatokenSerial || ipatokenOwner || ipatokenProtected")(version 3.0;
>>> acl "Users can read basic token info"; allow (read, search, compare)
>>> userattr = "ipatokenOwner#USERDN";)
>>>
>>> aci: (targetfilter = "(objectClass=ipatokenTOTP)")(targetattrs =
>>> "ipatokenOTPalgorithm || ipatokenOTPdigits ||
>>> ipatokenTOTPtimeStep")(version 3.0; acl "Users can see TOTP details";
>>> allow (read, search, compare) userattr = "ipatokenOwner#USERDN";)
>>>
>>> aci: (targetfilter = "(objectClass=ipatokenHOTP)")(targetattrs =
>>> "ipatokenOTPalgorithm || ipatokenOTPdigits")(version 3.0; acl "Users can
>>> see HOTP details"; allow (read, search, compare) userattr =
>>> "ipatokenOwner#USERDN";)
>>>
>>> aci: (targetfilter =
>>> "(&(objectClass=ipaToken)(!(ipatokenProtected=TRUE)))")(targetattrs =
>>> "description || ipatokenDisabled || ipatokenNotBefore ||
>>> ipatokenNotAfter || ipatokenVendor || ipatokenModel ||
>>> ipatokenSerial")(version 3.0; acl "Users can write basic token info";
>>> allow (write) userattr = "ipatokenOwner#USERDN";)
>>>
>>> aci: (target = "ldap:///ipatokenuniqueid=*,cn=otp,$SUFFIX")(targetfilter
>>> = "(&(objectClass=ipaToken)(!(ipatokenProtected=TRUE))))")(version 3.0;
>>> acl "Users can create and delete tokens"; allow (add, delete) userattr =
>>> "ipatokenOwner#SELFDN";)
>>>
>>> This is what they look like using managedBy (I have not tested this):
>>>
>>> aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs =
>>> "objectclass || description || ipatokenUniqueID || ipatokenDisabled ||
>>> ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel
>>> || ipatokenSerial || ipatokenOwner || ipatokenProtected")(version 3.0;
>>> acl "Users can read basic token info"; allow (read, search, compare)
>>> userattr = "ipatokenOwner#USERDN"; allow (read, search, compare)
>>> userattr = "managedBy#USERDN";)
>>>
>>> aci: (targetfilter = "(objectClass=ipatokenTOTP)")(targetattrs =
>>> "ipatokenOTPalgorithm || ipatokenOTPdigits ||
>>> ipatokenTOTPtimeStep")(version 3.0; acl "Users can see TOTP details";
>>> allow (read, search, compare) userattr = "ipatokenOwner#USERDN"; allow
>>> (read, search, compare) userattr = "managedBy#USERDN";)
>>>
>>> aci: (targetfilter = "(objectClass=ipatokenHOTP)")(targetattrs =
>>> "ipatokenOTPalgorithm || ipatokenOTPdigits")(version 3.0; acl "Users can
>>> see HOTP details"; allow (read, search, compare) userattr =
>>> "ipatokenOwner#USERDN"; allow (read, search, compare) 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";)
>>>
>>> In short:
>>> 1. Owner and manager get read, search and compare.
>>> 2. Manager gets write (to select attributes) and delete.
>>> 3. Users can create self-managed tokens for themselves only.
>>>
>>> The otptoken-add command should gain the following defaults:
>>> 1. The owner defaults to the user adding the token.
>>> 2. If owner == user adding token, managedBy defaults to owner.
>>> 3. Otherwise, managedBy defaults to None.
>>>
>>> This means that if neither owner nor managedBy are specified, the
>>> default is a self-owned, self-managed token. Likewise, if a different
>>> owner is specified, no manager is assumed.
>>>
>>> rcrit expresses worry that ipalib's ACI parser may not handle the above
>>> syntax. This will become clear during testing if we want this approach.
>>>
>>> Does this look sane?
>>
>> I am not entirely sure your ACI syntax is always right for the second
>> set. and perhaps we want to duplicate ACIs in some cases (once for owner
>> once for manager).
>
> In the patch I am preparing, I migrated it to use the more terse boolean
> OR. It should be correct. But I will test it thoroughly.
>
>> I think the read ACIs do not need to list managedby ? Do we plan to have
>> a manager that is another regular user but not the owner nor an admin ?
>
> Even if we don't plan on it, someone else will use it for that. And
> managers can't manage what they can't see. One thing I did leave out is
> adding management to groups: userattr = "managedBy#GROUPDN".
>
>> In any case I prefer the sytnax that uses managedby, as it has more
>> potential.
>
> I think I'm inclined to agree.
>
> Is there some aid to understand how to use the managedBy attribute in
> the plugin code? In existing code it all seems to be used as part of a
> relationship, which is rather less than clear (and very magical).

I don't think there are any docs beyond the DS ACI docs that describe 
it. We use it both as a list of DNs (USERDN) for hosts managing hosts 
and services and as a group DN when delegating DNS.

rob