[Freeipa-devel] [PATCH 0049] Add support for protected tokens
Nathaniel McCallum
npmccallum at redhat.com
Fri Jun 6 17:04:37 UTC 2014
On Thu, 2014-06-05 at 08:45 +0200, Jan Cholasta wrote:
> On 28.5.2014 22:44, Nathaniel McCallum wrote:
> > On Mon, 2014-05-26 at 16:57 +0200, Jan Cholasta wrote:
> >> On 13.5.2014 19:12, Nathaniel McCallum wrote:
> >>> On Tue, 2014-05-13 at 16:33 +0200, Jan Cholasta wrote:
> >>>> On 12.5.2014 21:02, 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).
> >>>>>>
> >>>>>> 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 ?
> >>>>>>
> >>>>>> In any case I prefer the sytnax that uses managedby, as it has more
> >>>>>> potential.
> >>>>>
> >>>>> Attached is a new version of the patch which implements the feature
> >>>>> using managedBy instead of ipatokenProtected. One important thing needs
> >>>>> to be said about this patch. I am not exposing managedBy in either the
> >>>>> UI, the CLI or LDAP (ACI). Do we care about this? If yes, should I
> >>>>> expose this similar to owner or as a relationship?
> >>>>
> >>>> I would expose it, as a relationship. (Note that ipatokenowner should
> >>>> ideally be represented as a relationship too, but the framework does not
> >>>> support 1-to-many relationships ATM.)
> >>>
> >>> So since this is a 1-to-many relationship we shouldn't expose it?
> >>>
> >>> Or should I do it like owner is currently done?
> >>
> >> Do it like managedby is done in the host plugin (see
> >> "attribute_members", "relationships", etc.)
> >>
> >>>
> >>>>
> >>>> Just curious, why are the ACIs divided like this:
> >>>>
> >>>> aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs =
> >>>> "objectclass || description || 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";)
> >>>> 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";)
> >>>> 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";)
> >>>>
> >>>> IMHO you could make them less complex by dividing them like this:
> >>>>
> >>>> aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs =
> >>>> "objectclass || description || ipatokenUniqueID || ipatokenDisabled ||
> >>>> ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel
> >>>> || ipatokenSerial || ipatokenOwner || ipatokenOTPalgorithm ||
> >>>> ipatokenOTPdigits || ipatokenTOTPtimeStep")(version 3.0; acl "Owner can
> >>>> read token details"; allow (read, search, compare) userattr =
> >>>> "ipatokenOwner#USERDN";)
> >>>> aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs =
> >>>> "objectclass || description || ipatokenUniqueID || ipatokenDisabled ||
> >>>> ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel
> >>>> || ipatokenSerial || ipatokenOwner || ipatokenOTPalgorithm ||
> >>>> ipatokenOTPdigits || ipatokenTOTPtimeStep")(version 3.0; acl "Managers
> >>>> can read token details"; allow (read, search, compare) userattr =
> >>>> "managedBy#USERDN";)
> >>>
> >>> The first set is organized by objectClass. The second by userattr. I
> >>> have no strong opinion on this matter, though performance is probably a
> >>> consideration. Do any DS guys want to chime in?
> >>
> >> I would still like to know someone else's opinion on this, but if
> >> there's none, let's keep it your way.
> >>
> >>>
> >>>> Would it make sense to keep --protected as a flag in otptoken-add as a
> >>>> shortcut for "entry_attrs['managedby'] = None"?
> >>>
> >>> I can't think of a use case for this. The only use case I *can* think of
> >>> is an admin creating a non-protected token for a user.
> >>
> >> OK.
> >>
> >>>
> >>>> Would it make sense to default managedby to the current bind DN in
> >>>> otptoken-add, even if it's not a user DN? (Do we want to allow running
> >>>> otptoken-add by hosts/services/other non-users?)
> >>>
> >>> No idea. Dmitri?
> >>
> >> We can add this later if necessary.
> >>
> >>>
> >>>> Is orphaning a token when a user is deleted only if it is not managed by
> >>>> any other users the intended behavior? It just seems sort of strange to
> >>>> me, since it changes the token from unprotected to protected.
> >>>
> >>> I don't think that is the behavior. We orphan the token if the owner is
> >>> not listed as a manager. If the owner is listed as a manager, we delete
> >>> the token.
> >>>
> >>> Put another way, protected tokens are orphaned and unprotected tokens
> >>> are deleted.
> >>>
> >>> If I didn't implement that, please point out my bug.
> >>
> >> Sorry, my bad, your code is right. You can make it simpler, though:
> >>
> >> orphan = [x for x in token.get('managedby', []) if x == dn]
> >>
> >> (The "len() == 0" check is not necessary and using list comprehensions
> >> makes the code more readable than using filter.)
> >
> > The attached version fixes all the above issues. Two issues that may
> > remain:
> > 1. There is no option to set managedBy during otptoken-add or
> > otptoken-mod. This is probably okay.
>
> Yes. I guess this bit is not needed anymore:
>
> # If owner was not specified, default to the person adding
> this token.
> - if 'ipatokenowner' not in entry_attrs:
> + # If managedby was not specified, attempt a sensible default.
> + if 'ipatokenowner' not in entry_attrs or 'managedby' not in
> entry_attrs:
> result = self.api.Command.user_find(whoami=True)['result']
> if result:
> cur_uid = result[0]['uid'][0]
> - entry_attrs.setdefault('ipatokenowner', cur_uid)
> + prev_uid = entry_attrs.setdefault('ipatokenowner', cur_uid)
> + if cur_uid == prev_uid:
> + entry_attrs.setdefault('managedby', result[0]['dn'])
This code is still needed. Read the patch description. And, FYI,
'managedby' is correct in this case, not 'managedby_user'. I tested.
> > 2. I can't figure out how to get the framework to actually show
> > managedBy in command output (like otptoken-show). This means you can
> > add/remove relationships using otptoken-add-managedby and
> > otptoken-remove-managedby, but you can't actually see the list of
> > managers. What am I missing?
>
> In the hbacrule or selinuxusermap plugins it is done by adding an
> "invisible" param to the object plugin, like this:
>
> Str('managedby_user?',
> label=_('Manager'),
> flags=['no_create', 'no_update', 'no_search'],
> ),
Done.
> >
> > Also, it would be helpful if someone with DS expertise could answer the
> > question about the performance of the ACI structure options as listed
> > above.
We still need this. Ludwig?
> You should update the code in user_del to use managedby_user instead of
> managedby, otherwise it won't really work.
Done.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-npmccallum-0049a.2-Add-support-for-managedBy-to-tokens.patch
Type: text/x-patch
Size: 25028 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140606/ed3d922b/attachment.bin>
More information about the Freeipa-devel
mailing list