[Freeipa-users] Patch for ipa-sam: ipa-server-trust-ad samba server valid users =@groupname

[Alexander Bokovoy]

On Thu, 06 Mar 2014, Jason Woods wrote:
>Hi all,
>
>I am quite aware that installing ipa-server-trust-ad and using the
>samba as a file server is as unsupported as one can get... but I really
>needed a Samba server integrated with IPA (damn Mac OS and Windows). I
>don't actually have a Windows environment but this seemed to bootstrap
>enough of the requirements to get it working
>
>Bit of a story for those who have time to read and maybe battling
>similiar, or just skip to after the log for the fix+patch :)
>* ipaNTSecurityIdentifier ended up missing because I didn't use
>--setsid and NT hash missing because I did not do a ipa passwd reset
>* As a result, experienced user not found or invalid password, and
>after debug level 5 I had about 500M of core dumps (sorry don't have
>them anymore)
>* Ran ipa-adtrust-install again with --setsid and reset some passwords
>and things started looking better, could connect, all good, NT hash was
>there and ipaNTSecurityIdentifier there (ldapsearch <3)
>* Then next problem was when I added "valid users = @groupname" to
>share config. No longer could connect even if member of the group!
>* Turned out ipNTGroupAttr was missing from some groups - thus had to
>register the ldif for the ipa-setsid task
For the record, it is ipa-adtrust-install --add-sids and the task is
called sidgen task.

>I noticed the "Could not find uidNumber" - turns out ipa-sam was being
>asked to turn SID into ID and was successfully finding it but needed to
>work out whether it was a group or a user. To do this, it searches the
>objectClass for "ipNTGroupAttr" - if it finds it, it looks for
>gidNumber, otherwise it looks for uidNumber. However, the objectClass
>added by ipa-setsid is "ipntgroupattr" and ipa-sam was using "strncmp".
>
>I've fixed this with a patch to use strncasecmp. Might not be the best
>fix... maybe ipa-sam should be modified to have the attributes lower
>case for comparison? But this was simplest patch. Comments/feedback
>welcome and maybe I'll have time to do alternative fix if felt better?
You are absolutely on spot here, thanks!

Since we are comparing values of the attribute, we are on our own and
cannot rely on attribute name canonicalization here. This means
strncasecmp() is for the job. I've looked at other options like using
ber_bvcmp() macro but we are really can't guarantee that objectClass
attribute values are in any specific string case because the only 
matching rule defined for them is objectIdentifierMatch -- we would have
to turn the value to oid first and then compare which is probably too
much for this specific case.


>Versions:
>RHEL 6.4 3.0.0-37
>Code in master branch appears to show the same issue
>
>References:
>freeipa/daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen.h
>around line 54-55: lowercase objectClass addition
>freeipa/daemons/ipa-sam/ipa_sam.c
>around line 688: case sensitive comparison to ipaNTGroupAttrs
>
>Patch for master branch:
>diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
>index 1ca504d..c5e8b39 100644
>--- a/daemons/ipa-sam/ipa_sam.c
>+++ b/daemons/ipa-sam/ipa_sam.c
>@@ -750,7 +750,7 @@ static bool ldapsam_sid_to_id(struct pdb_methods *methods,
> 	}
>
> 	for (c = 0; values[c] != NULL; c++) {
>-		if (strncmp(LDAP_OBJ_GROUPMAP, values[c]->bv_val,
>+		if (strncasecmp(LDAP_OBJ_GROUPMAP, values[c]->bv_val,
> 			                       values[c]->bv_len) == 0) {
> 			break;
> 		}
>
>Patch for RHEL 6.5 3.0.0-37:
>--- a/daemons/ipa-sam/ipa_sam.c	2014-03-06 19:30:15.994792879 +0000
>+++ b/daemons/ipa-sam/ipa_sam.c	2014-03-06 19:35:34.966791637 +0000
>@@ -685,7 +685,7 @@
> 	}
>
> 	for (c = 0; values[c] != NULL; c++) {
>-		if (strncmp(LDAP_OBJ_GROUPMAP, values[c]->bv_val,
>+		if (strncasecmp(LDAP_OBJ_GROUPMAP, values[c]->bv_val,
> 			                       values[c]->bv_len) == 0) {
> 			break;
> 		}
>
This is valid bug. Could you please raise it in bugzilla.redhat.com or,
alternatively, at FreeIPA's trac?

-- 
/ Alexander Bokovoy