On Fri, 2012-02-24 at 11:39 +0100, Jakub Hrozek wrote:
On Thu, Feb 23, 2012 at 08:50:15PM -0500, Stephen Gallagher wrote:
> On Thu, 2012-02-23 at 18:58 +0100, Jakub Hrozek wrote:
> > On Thu, Feb 23, 2012 at 06:23:46PM +0100, Jakub Hrozek wrote:
> > >
https://fedorahosted.org/sssd/ticket/1168
> > >
> > > The actual base for the map entry is the entry DN, so it's not really
> > > multiple search bases..still, the change is useful to honor the extra
> > > filter and scope.
> >
> > Sorry, I forgot to squash a commit with a fix.
>
> Ack to this code for master and sssd-1-8, but with a caveat.
>
> While reviewing this patch, I identified an issue with the original
> code: we shouldn't be passing the orig_dn through sss_filter_sanitize()
> in automntmaps_process_members_send(). This is only supposed to be done
> for values that are being passed into an LDAP filter. By sanitizing this
> here, you're actually escaping some characters that would render the
> resulting string unparseable as a DN. It's safe to trust what we get
> back from the sysdb here. The only time we need to do any sanitizing is
> if we are constructing a DN format string from unknown components (like
> we do sometimes in the sysdb with the sysdb_dn_sanitize() function).
>
> The only reason this was working is because we just happened not to hit
> any DNs that contain an asterisk, open/close-parenthesis or a backslash.
>
> Please submit a fix for this issue and I'll push them together.
>
The DN does not come from sysdb, it has just been retreived from LDAP with
sdap_get_generic_recv(). See sdap_get_automntmap_process(). Don't we need
to sanitize values that come directly from LDAP?
Not exactly. We only need to sanitize components that are going into
search filters with sss_filter_sanitize(). This is so the search filter
doesn't break if a DN or other value happens to have parens or an
asterisk, etc. in it (since those are reserved for special use in a
filter). But a pure DN string being passed into the search_base field of
sdap_get_generic_send() needs to be passed as-is, or else if it DOES
contain a special character, it will fail.
There's also the sysdb_dn_sanitize() function (which internally just
calls ldb_dn_escape_value()) for those cases where we're constructing a
new DN from unknown sources. Such as creating a sysdb DN out of a
username. We'd need to pass the username through sysdb_dn_sanitize() in
order to make sure it escaped any DN-sensitive values.
But neither of these conditions applies for this code. Please remove the
sss_filter_sanitize() so we don't break for DNs with parens in them.