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

Thu May 8 17:57:58 UTC 2014

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

Nathaniel