[Freeipa-devel] [PATCH] krb 1.12's OTP-Over-RADIUS

Wed May 1 13:33:45 UTC 2013

Below is my first stab at ACLs. They don't actually work right, but I'm not sure what I've done wrong. The basic gist is that nobody gets any permissions by default. Admins get full permissions and users get limited permissions for their own tokens. Any help would be appreciated.

Nathaniel

dn: $SUFFIX
changetype: modify
add: aci
aci: (targetattrs = "ipatokenRadiusConfigLink || ipatokenRadiusUserName")(version 3.0; acl "RADIUS user configuration is priviledged"; deny (all) userdn = "ldap:///all";)
aci: (targetattrs = "ipatokenRadiusConfigLink || ipatokenRadiusUserName")(version 3.0; acl "Admins can manage RADIUS user configuration"; allow (all) groupdn="ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";)
aci: (targetfilter = "(objectClass=ipatokenRadiusConfiguration)")(targetattrs = "*")(version 3.0; acl "RADIUS configuration is priviledged"; deny (all) userdn = "ldap:///all";)
aci: (targetfilter = "(objectClass=ipatokenRadiusConfiguration)")(targetattrs = "*")(version 3.0; acl "Admins can manage RADIUS configuration"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";)
aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs = "*")(version 3.0; acl "Token configuration is priviledged"; deny (all) userdn = "ldap:///all";)
aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs = "*")(version 3.0; acl "Admins can manage token configuration"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";)
aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs = "ipatokenUniqueID || description || ipatokenOwner || ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel || ipatokenSerial")(version 3.0; acl "Users can read/add basic token info"; allow (read, add, search, compare) userattr = "ipatokenOwner#USERDN";)
aci: (targetfilter = "(objectClass=ipatokenTOTP)")(targetattrs = "*")(version 3.0; acl "TOTP Token configuration is priviledged"; deny (all) userdn = "ldap:///all";)
aci: (targetfilter = "(objectClass=ipatokenTOTP)")(targetattrs = "*")(version 3.0; acl "Admins can manage TOTP token configuration"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";)
aci: (targetfilter = "(objectClass=ipatokenTOTP)")(targetattrs = "ipatokenOTPkey || ipatokenOTPalgorithm || ipatokenOTPdigits || ipatokenTOTPclockOffset || ipatokenTOTPtimeStep")(version 3.0; acl "Users can add TOTP token secrets"; allow (add, search) userattr = "ipatokenOwner#USERDN";)
-

----- Original Message -----
From: "Nathaniel McCallum" <npmccallum at redhat.com>
To: "Rob Crittenden" <rcritten at redhat.com>
Cc: freeipa-devel at redhat.com
Sent: Tuesday, April 30, 2013 5:46:33 PM
Subject: Re: [Freeipa-devel] [PATCH] krb 1.12's OTP-Over-RADIUS

On Fri, 2013-04-26 at 18:30 -0400, Rob Crittenden wrote:
> Nathaniel McCallum wrote:
> > On Fri, 2013-04-12 at 17:39 -0400, Nathaniel McCallum wrote:
> >> On Fri, 2013-04-12 at 11:53 -0400, Nathaniel McCallum wrote:
> >>> On Fri, 2013-04-12 at 11:34 -0400, Nathaniel McCallum wrote:
> >>>> On Thu, 2013-04-11 at 14:48 -0400, Rob Crittenden wrote:
> >>>>> Nathaniel McCallum wrote:
> >>>>>> On Wed, 2013-04-10 at 15:35 -0400, Rob Crittenden wrote:
> >>>>>>> I'm not sure how I'd test it if I got it built.
> >>>>>>
> >>>>>> I'm working on this. I hope to have a clear answer next week. Bear with
> >>>>>> me...
> >>>>>>
> >>>>>>> Overall looks really good.
> >>>>>>
> >>>>>> I've split up the patch into multiple commits. I've also added .update
> >>>>>> files and a patch for ipa-kdb to feed krb5 the right user string.
> >>>>>>
> >>>>>> https://github.com/npmccallum/freeipa/commits/otp
> >>>>>>
> >>>>>> Please take a look. I *think* I've got everything worked out so far with
> >>>>>> the exception of bug numbers / urls. Should every patch have a separate
> >>>>>> bug and a link to the design page?
> >>>>>
> >>>>> The ticket should go into every commit. I'd probably put the design link
> >>>>> there too, just for completeness. Future bug fixes, et all aren't going
> >>>>> to require the design page, but since these commits are all related to
> >>>>> the initial feature it will be nice to have.
> >>>>>
> >>>>> You can have multiple patches on the same ticket/bug.
> >>>>
> >>>> https://github.com/npmccallum/freeipa/commits/otp
> >>>>
> >>>> All four commits now have bug numbers and design page links. I'm adding
> >>>> the design page link to the tickets as we speak.
> >>>>
> >>>> Remaining issues (AFAICS):
> >>>> 1. The daemon (ipa-otpd) runs as root and binds anonymously
> >>>> 2. ipatokenRadiusSecret is readable by an anonymous bind
> >>> 3. ipatokenT?OTP.* are readable by an anonymous bind
> >>>
> >>> In the case of both #2 and #3, only admins should have RW. ipa-otpd
> >>> needs read access to ipatokenRadiusSecret. The DS bind plugin below (#2)
> >>> needs read access to ipatokenT?OTP.*.
> >>>
> >>>> Outstanding pieces:
> >>>> 1. CLI tool -- https://fedorahosted.org/freeipa/ticket/3368
> >>>> 2. DS bind plugin -- https://fedorahosted.org/freeipa/ticket/3367
> >>>> 3. UI -- https://fedorahosted.org/freeipa/ticket/3369
> >>>> 4. Self Service UI -- https://fedorahosted.org/freeipa/ticket/3370
> >>>>
> >>>> #1 and #2 are within the scope of F19 and should hopefully land shortly
> >>>> (in separate commits). #3 and #4 are probably $nextrelease.
> >>>>
> >>
> >> FYI - Here is an RPM with all of the code so far:
> >> http://koji.fedoraproject.org/koji/taskinfo?taskID=5247029
> >
> > Updated RPMs, containing the new 389DS bind plugin and build for F19,
> > are here:
> > http://koji.fedoraproject.org/koji/taskinfo?taskID=5270926
> >
> > Nathaniel
> 
> BuildRequires needed for whatever provides krad.h

It is there already. I updated the spec file to have a hard dependency
on krb5 >= 1.11.

> A bunch of the files declare functions in each other. Is it cleaner to 
> put these into an include file? I'm gathering that this will always be 
> self-contained, so maybe this is ok.

Most of the functions are only needed in one other file. I thought this
was cleaner. I'm also open to suggestions on how to better split the
files so there are less includes...

> In entry_attr_get_berval() is it worth pointing out that there is no 
> need to free the value, or is that assumed because it uses 
> slapi_value_get_berval()?

I would hope it is assumed by const.

> If we detect that there is clock drift should we log it? Will we ever 
> try to report to the client (e.g. future enhancement)?

There are lots of clock related enhancements that should be low-hanging
fruit.

> I wonder if the NSPR-version of some functions should be used since this 
> is running inside 389-ds, like PL_strcasecmp for strcasecmp()

I did not fix this since strcasecmp() is available everywhere.

> ops.c:
> 
> pedantic: lack of spacing between if and parens

Fixed.

> sha384 is an allowed type only in otp.c. Is that needed?

The RFC doesn't specify sha384 support. However, adding support for it
is somewhat obvious. In the spirit of "being liberal in what you
receive, strict in what you send" I added sha384 support in auth.c.

Nathaniel

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel at redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel