[SSSD] [PATCH] libwbclient: SSSD implementation

Jakub Hrozek jhrozek at redhat.com
Sun Aug 17 18:43:25 UTC 2014


On Wed, Jul 23, 2014 at 12:34:32PM +0200, Sumit Bose wrote:
> new version attached.
> 
> bye,
> Sumit

I admit I haven't reviewed the filed copied from Winbind too closely,
I just quickly read them to know what's going on in these modules. In the
review, I mostly compared the sssd implementation to the winbind one.

Do you expect the files copied from Samba to change? Most of the files I
check in the Samba tree didn't change since 2012 or even 2010 so I
expect we're safe..

I think the dlopen to open the nss_sss module is perfectly fine, in fact
I prefer it over linking.  That's how the module is used by the glibc
after all.

A question -- in sssd's copy_group_entry() you only create the gr_mem
array when there are some members. Winbind seems to always create an
array which contains just a NULL sentinel if the group has no members.
Does that matter? If not, let's leave the code as-is, I just wanted to
check.

In nss_to_wbc(), would it make sense to also translate
NSS_STATUS_UNAVAIL to WBC_ERR_WINBIND_NOT_AVAILABLE ? Sure, there is no
winbind, so the return code might better be called something like
WBC_ERR_DATA_SOURCE_NOT_AVAILABLE or similar, but my question is more if
handling the NSS_STATUS_UNAVAIL NSS code is expected.

wbcGetgrnam and wbcGetgrnam have wrong comments, but this trivial bug is
also in Samba.

Are you sure the asprintf() call in wbcLookupName is safe? Could this
enable someone to trash the stack with a long enough name?

I agree with the comment in wbcLookupSid() about returning the name and
domain separately. Do you want to file a ticket? Alternatively, we can
make a function that parses a FQDN into a (name,domain) tuple public if
you expect this functionality to be needed elsewhere?

sss_client/libwbclient/wbc_sssd_internal.h seems to use a DEVELOPER
macro, do you expect this macro to be enabled with modifying CFLAGS?
It's OK to leave it there I guess, I'm not aware of any similar macro in
SSSD..

In wbclient.pc.in can you mention this is wbclient API using SSSD?

That's all I have. The code looks very solid and clean overall --
thanks!



More information about the sssd-devel mailing list