Hello, we are using samba 3.0 with two uid: LDAP attribute (multivalue): dn: uid=justauser,ou=Users,ou=company,dc=tld ... uid: justauser uid: justauser@something.company.tld ... This is working fine with 3.0 but in our samba 3.2.8 tests we got the following error: "attribute uid has 2 values, expected only one init_sam_from_ldap: No uid attribute found for this user!" uid is a multivalue an from the LDAP view this is OK and because of other applications using the LDAP its not possible to remove one of the uid's to have only one.:( IMHO samba 3.2 should support uid multivalue like 3.0 or is there a reason I cannot see why this isn't allowed anymore? Thank you. Regards. Noèl
Just as additional info: http://lists.samba.org/archive/samba-technical/2008-September/061452.html He was lucky he could remove additional uids.;)
Hmm. Okay. We now check that in lib/smbldap.c:313. The question is: How should we figure out the right value in the multivalue field? Samba 3.0 just used the first value that came back from ldapsearch, but this is not really deterministic. Volker
(In reply to comment #2) > Hmm. Okay. We now check that in lib/smbldap.c:313. The question is: How should > we figure out the right value in the multivalue field? Samba 3.0 just used the > first value that came back from ldapsearch, but this is not really > deterministic. We don't. We assume that uid is a single valued attribute (and so does a lot of other software). We should just consider this scenario invalid, if Noèl wants to keep using this schema, IMHO, he has to come up with his own patch to samba that is able to pick an choose the right value for him.
Well, this used to work in 3.0, where we did not check. So you could call that a regression. Volker
(In reply to comment #4) > Well, this used to work in 3.0, where we did not check. So you could call that > a regression. Personally I think it was a bug in 3.0.x, that we did not check. So from my POV we just fixed bad behavior. Simo.
The LDAP RFCs seem to make "uid" a multi-value attribute. Why shouldn't we support that? For those who have a "safe" directory, this does not matter. The others get the non-deterministic behaviour? Volker
(In reply to comment #6) > The LDAP RFCs seem to make "uid" a multi-value attribute. Why shouldn't we > support that? For those who have a "safe" directory, this does not matter. The > others get the non-deterministic behaviour? Which one do you pick ? Order is not defined in LDAP, so you might get any value as the first one. That would make the behaviour unpredictable. I personally think an unpredictable behaviour is worst than throwing an error if multiple uids are found. Unless there is a way we can pick the right value somehow.
Use the shortest value? Or the longest? Or do an alphabetic sort and use the first one? Thinking about it, I like the "alphabetically smallest" one :-) Volker
Created attachment 3990 [details] patch The attached patch compiles but is completely untested. It implements the idea to pick the alphabetically smallest value of the multiple ones in the "uid" attribute. Noel, can you test that? Simo, what do you think? Thanks, Volker
(In reply to comment #9) > Created an attachment (id=3990) [edit] > patch > > The attached patch compiles but is completely untested. It implements the idea > to pick the alphabetically smallest value of the multiple ones in the "uid" > attribute. > > Noel, can you test that? > > Simo, what do you think? I am not sure that picking the alphabetically smallest value is always the right choice, but at least it is predictable. I was thinking that another way (possibly more useful) could be to provide a new parameter were you can set a regular expression to match the value you want to use, and fall back to alphabetic sorting if no value or more than one value match. This way I could have a regex like (.*)[^@]* and that would match only entries that do not contain the @ sign (the regex may not be correct, but you get the idea :-). But I understand that implementing regex matching would take a bigger patch and new options, and I don't have time to implement it. So as far as I am concerned alphabetic sorting is probably better than nothing for now.
Thanks Volker. I will test it today and report the results.
Noel, any updates here? Volker
Sorry for failing my own dates.:( I tried to build 3.2.8 with your patch but got the following error: Compiling lib/winbind_util.c Compiling lib/smbldap.c lib/smbldap.c: In function `smbldap_talloc_smallest_attribute': lib/smbldap.c:355: error: too many arguments to function `pull_utf8_talloc' lib/smbldap.c:367: error: too many arguments to function `pull_utf8_talloc' The following command failed: i386-linux-gcc -I. -I/tmp/buildd/samba-3.2.8/source -O -D_SAMBA_BUILD_=3 -I/tmp/buildd/samba-3.2.8/source/iniparser/src -Iinclude -I./include -I. -I. -I./lib/replace -I./lib/talloc -I./lib/tdb/include -I./libaddns -I./librpc -DHAVE_CONFIG_H -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -Iinclude -I./include -I. -I. -I./lib/replace -I./lib/talloc -I./lib/tdb/include -I./libaddns -I./librpc -I./popt -DLDAP_DEPRECATED -I/include -I/tmp/buildd/samba-3.2.8/source/lib -D_SAMBA_BUILD_=3 -fPIC -c lib/smbldap.c -o lib/smbldap.o make[1]: *** [lib/smbldap.o] Error 1 make[1]: Leaving directory `/tmp/buildd/samba-3.2.8/source' make: *** [build-stamp] Error 2
Created attachment 4042 [details] patch Sorry. I've just built the attached new patch in a 3.2.8 successfully. Volker
Hello Volker, thank you for the updated patch. I tested it today with 3.2.8 and is working fine. The users with two uid attributes can connect and use the server. We will use this patch with 3.2.8 but integration into samba (3.3.x, no need from our side for 3.2) would be great. Thank you.
Created attachment 4145 [details] patch for 3.3 Is this a candidate for 3.3? Volker
Will be in 3.4. Nobody was found who would vote this in for 3.3. Sorry, Volker
As this is a regression, I would vote for picking this patch for 3.3!
Jeremy, would you like to review the patch? Thanks!
Ok, patch looks good - does exactly what it says. Thanks, Jeremy.
Pushed, will be included in 3.3.5. Closing out bug report. Thanks for reporting!
All LDAP-Server I've seen yet (AD and OpenLDAP 2.x.x) return the values in the order they were stored. There're Samba 3.0 setups which rely on the first uid being used. As that's also what nss_ldap will return. Should we add an option so select between "alphabetically smallest" and "first" value. Or should be just restore the Samba 3.0.x behavior?
A discussion on IRC; 14:10 <@idra> metze, ldap does not guarantee any order, relying on the order of the attributes is just plain wrong 14:14 < metze> what LDAP do you know that doesn't return the same order that's stored? 14:14 <@idra> metze, I don't know of specific examples, but you just rely on specific implementations behavior 14:15 <@idra> metze, however if nss_ldap always picks the first we should probably do the same otherwise things could break 14:15 < blingme> is this for enumeration, or selection of a specific account? 14:15 < metze> I think both 14:16 < blingme> well, multi-valued or not should only affect enumeration 14:16 < blingme> for authn/authz of a specific account, the filter, and/or matched result extension should be enough? 14:17 < blingme> (but, I'm not familiar with samba internals) 14:17 < metze> we search for (uid=%s) which will match any uid of the account 14:18 < metze> and then use the first (in 3.0.x) uid of the result to fill in the account name of the user 14:19 < metze> so the user can login with any uid from the client, but samba only uses the primary uid 14:19 < metze> e.g. for %U substitutions 14:24 < blingme> would it make sense to check if the attribute name is an/the RDN ? 14:25 < blingme> (in the example from the user, this is the case) 14:34 < metze> maybe a good idea, I'll check
As far as I can see, nss_ldap just returns the first value. I'd preferr to just do the same...
My view is that we should be exactly as broken as nss_ldap (because it is common software paring), after the administrator sets an option. (If we must support this at all). The e-mail address login thing is neat however, and I've had folks asking about it on IRC (migration from AD). But I wish we had stayed strict - it's just asking for trouble.
Created attachment 5160 [details] New patch for v3-3 Is that ok for v3-3?
Looks good to me
Pushed Metze's patches to v3-3-test. Re-assigning to Metze to add the same patch for v3-4-test.
Created attachment 5174 [details] Patch for v3-5
Created attachment 5175 [details] Patch for v3-4
Can we get that into 3.4.6 and 3.5.0rc2?
Yes, pushed to v3-5-test and v3-4-test. Closing out bug report. Thanks!