handling whitespaces in netgroup triples

[Josef Cejka]

Hi,
I have noticed that sssd does not trim whitespaces from strings while
parsing netgroup triples. Comment inside code explains that it follow
the nss_ldap implementation:

src/db/sysdb_search.c:
1687 /* This function splits a three-tuple into three strings
1688  * It assumes that any whitespace between the parentheses
1689  * and commas are intentional and does not attempt to
1690  * strip them out. Leading and trailing whitespace is
1691  * ignored.
1692  *
1693  * This behavior is compatible with nss_ldap's
1694  * implementation.
1695  */
1696 static errno_t sysdb_netgr_split_triple(TALLOC_CTX *mem_ctx,

Don't know which nss_ldap implementation is referenced, the one from
PADL trim the spaces (and glibc too):

https://github.com/PADL/nss_ldap/blob/154730b5a2b58a4212e419b498476fcb5
a60de7b/ldap-netgrp.c#L251

Is the difference intended?

I run into it when LDAP server returns some of the host strings with
spaces around which works with nss_ldap form PADL, but not with sssd.

Fix is easy, but I'm afraid that it could break some other corner cases
like netgroup rules for users with leading/trailing whitespaces in
username.

What's your opinion on it?
Thanks

[Jakub Hrozek]

On Thu, May 03, 2018 at 02:44:40PM +0200, Josef Cejka wrote:
> Hi,
> I have noticed that sssd does not trim whitespaces from strings while
> parsing netgroup triples. Comment inside code explains that it follow
> the nss_ldap implementation:
>
> src/db/sysdb_search.c:
> 1687 /* This function splits a three-tuple into three strings
> 1688  * It assumes that any whitespace between the parentheses
> 1689  * and commas are intentional and does not attempt to
> 1690  * strip them out. Leading and trailing whitespace is
> 1691  * ignored.
> 1692  *
> 1693  * This behavior is compatible with nss_ldap's
> 1694  * implementation.
> 1695  */
> 1696 static errno_t sysdb_netgr_split_triple(TALLOC_CTX *mem_ctx,
>
> Don't know which nss_ldap implementation is referenced, the one from
> PADL trim the spaces (and glibc too):
>
> https://github.com/PADL/nss_ldap/blob/154730b5a2b58a4212e419b498476fcb5
> a60de7b/ldap-netgrp.c#L251
>
> Is the difference intended?
>
> I run into it when LDAP server returns some of the host strings with
> spaces around which works with nss_ldap form PADL, but not with sssd.
>
> Fix is easy, but I'm afraid that it could break some other corner cases
> like netgroup rules for users with leading/trailing whitespaces in
> username.
>
> What's your opinion on it?

I agree with trimming. I don't think anyone should be using leading
or trailing whitespace and SSSD even sanitizes them. I also
checked the nss-pam-ldapd code and they also trim the whitespaces.

Can you submit a github PR at https://github.com/SSSD/sssd/pulls ?

[Jakub Hrozek]

On Sun, May 06, 2018 at 03:31:03PM +0200, Jakub Hrozek wrote:
> On Thu, May 03, 2018 at 02:44:40PM +0200, Josef Cejka wrote:
> > Hi,
> > I have noticed that sssd does not trim whitespaces from strings while
> > parsing netgroup triples. Comment inside code explains that it follow
> > the nss_ldap implementation:
> >
> > src/db/sysdb_search.c:
> > 1687 /* This function splits a three-tuple into three strings
> > 1688  * It assumes that any whitespace between the parentheses
> > 1689  * and commas are intentional and does not attempt to
> > 1690  * strip them out. Leading and trailing whitespace is
> > 1691  * ignored.
> > 1692  *
> > 1693  * This behavior is compatible with nss_ldap's
> > 1694  * implementation.
> > 1695  */
> > 1696 static errno_t sysdb_netgr_split_triple(TALLOC_CTX *mem_ctx,
> >
> > Don't know which nss_ldap implementation is referenced, the one from
> > PADL trim the spaces (and glibc too):
> >
> > https://github.com/PADL/nss_ldap/blob/154730b5a2b58a4212e419b498476fcb5
> > a60de7b/ldap-netgrp.c#L251
> >
> > Is the difference intended?
> >
> > I run into it when LDAP server returns some of the host strings with
> > spaces around which works with nss_ldap form PADL, but not with sssd.
> >
> > Fix is easy, but I'm afraid that it could break some other corner cases
> > like netgroup rules for users with leading/trailing whitespaces in
> > username.
> >
> > What's your opinion on it?
>
> I agree with trimming. I don't think anyone should be using leading
> or trailing whitespace and SSSD even sanitizes them. I also
> checked the nss-pam-ldapd code and they also trim the whitespaces.
>
> Can you submit a github PR at https://github.com/SSSD/sssd/pulls ?

...and I forgot to CC you earlier, sorry.