[Pki-devel] [PATCH] 0027..0029 support external authorization LDAP server

[Fraser Tweedale]

On Fri, Mar 13, 2015 at 08:10:11PM +0700, Endi Sukma Dewata wrote:
> On 3/13/2015 2:00 PM, Fraser Tweedale wrote:
> >>>4. When constructing a DN the attribute values should be escaped with
> >>>LDAPUtil.escapeRDNValue() to prevent problems with DN special characters.
> >>>
> >>>5. When constructing LDAP filter the attribute values should be escaped
> >>>with LDAPUtil.escapeFilter() to prevent problems with LDAP filter
> >>>special characters.
> >>>
> >I have escaped end-user-supplied values, but not config values.
> 
> Do you mean these lines?
> 1. return mGroups + "," + mGroupsBaseDN;
> 2. "(&(objectclass=" + mGroupObjectClass + ")(" + filter + "))",
> 3. filter = k + "=" + userdn;
> 
> I think line #1 is fine because the values in those variables are already in
> the DN format, not because they are config values.
> 
> Line #2 is fine too because the mGroupObjectClass is probably filter-safe
> due to object class naming restrictions.
> 
> For line #3 I think the userdn still needs to be escaped. DNs and filters
> have different sets of special characters (see LDAPUtil.java) so a DN may
> not necessarily be filter-safe regardless how the value was obtained.
> 
> >>9. I see the "gid" attribute being set in TokenAuthentication. Probably it
> >>should be changed to IAuthToken.GID so we know where it's set and used.
> >>Alternatively we can replace it with IAuthToken.GROUPS that contains just a
> >>single group, and remove IAuthToken.GID.
> >>
> >Good suggestion; implemented.
> 
> Just one more thing, the "no gid in authToken" message probably should be
> replaced with "no groups in authToken".
> 
> Once these are addressed, ACK. Thanks!
> 
Thanks Endi, amended patch pushed to master (f98e599).

Cheers,
Fraser