... As a result, "wbinfo --user-info", "wbinfo --group-info", "getent passwd" 'domain\user' fail, "getent group" shows empty domain groups, "valid users = +domain\group" doesn't work. The reason is winbindd incorrectly construct (objectSid=%s) filter while queris AD LDAP. It sends something like objectSid=\01\05\00\00\00\00\00\05\15\00\00\00²\05╔\0EО┌\09▒HM\29\0B\22\10\00\00 instead of objectSid=S-1-5-21-245695901-2433319663-187256136-4130 So, LDAP answers with 0 objects. //////////////////////// My patch is: ///////////////////////// --- source3/winbindd/winbindd_ads.c 2012-09-07 20:41:57.000000000 +0300 +++ source3/winbindd/winbindd_ads.c 2012-09-07 21:27:39.000000000 +0300 @@ -549,7 +549,7 @@ return NT_STATUS_SERVER_DISABLED; } - sidstr = ldap_encode_ndr_dom_sid(talloc_tos(), sid); + sidstr = sid_string_talloc(talloc_tos(), sid); ret = asprintf(&ldap_exp, "(objectSid=%s)", sidstr); TALLOC_FREE(sidstr); @@ -1059,7 +1059,7 @@ goto done; } - if ((sidbinstr = ldap_encode_ndr_dom_sid(talloc_tos(), group_sid)) == NULL) { + if ((sidbinstr = sid_string_talloc(talloc_tos(), group_sid)) == NULL) { status = NT_STATUS_NO_MEMORY; goto done; } //////////////////////// END of patch ///////////////////////// Some more pieces of code possibly need to be patched in the same manner, I did'n ispected more than troubled me.
This page: http://www.pcreview.co.uk/forums/re-objectsid-ldap-search-t1458615.html disagrees with you. The relevant part: -------------------------------------------------------- The objectSid attribute is binary-valued, so to search on it, you have to use the binary value of the SID. Binary values are represented in LDAP search filters as \xx, where "xx" are two hexadecimal digits. The details of LDAP search filters are covered in RFC 2254 (available at http://www.ietf.org/rfc/rfc2254.txt). For example, suppose your SID in string form was S-1-5-21-2562418665-3218585558-1813906818-1576. In binary form, this is {01,05,00,00,00,00,00,05,15,00,00,00,e9,67,bb,98,d6,b7,d7,bf,82,05,1e,6c,28,06,00,00}, so the LDAP search filter would be: (objectSid=\01\05\00\00\00\00\00\05\15\00\00\00\e9\67\bb\98\d6\b7\d7\bf\82\05\1e\6c\28\06\00\00) Thanks, Matthew Rimer [MSFT] -------------------------------------------------------- So we'll need to do some testing here. Jeremy.
Here is the function that is actually encoding the SID binary blob into a string for the LDAP query: /* encode a blob as a RFC2254 binary string, escaping any non-printable or '\' characters */ char *ldb_binary_encode(void *mem_ctx, struct ldb_val val) { size_t i; char *ret; size_t len = val.length; unsigned char *buf = val.data; for (i=0;i<val.length;i++) { if (!isprint(buf[i]) || strchr(" *()\\&|!\"", buf[i])) { len += 2; } } ret = talloc_array(mem_ctx, char, len+1); if (ret == NULL) return NULL; len = 0; for (i=0;i<val.length;i++) { if (!isprint(buf[i]) || strchr(" *()\\&|!\"", buf[i])) { snprintf(ret+len, 4, "\\%02X", buf[i]); len += 3; } else { ret[len++] = buf[i]; } } ret[len] = 0; return ret; } I'm suspicious of the isprint() function on your platform, as it seems to be returning "true" for characters like ² ╔ etc., which probably should not be printable.
Although according to : https://www.ietf.org/rfc/rfc2254.txt the only characters we must encode are: Character ASCII value --------------------------- * 0x2a ( 0x28 ) 0x29 \ 0x5c NUL 0x00
> Here is the function that is actually encoding the SID binary blob into a > string for the LDAP query: > > /* > encode a blob as a RFC2254 binary string, escaping any > non-printable or '\' characters > */ > char *ldb_binary_encode(void *mem_ctx, struct ldb_val val) > { ... > } > > I'm suspicious of the isprint() function on your platform, as it seems to be > returning "true" for characters like ² ╔ etc., which probably should not be > printable. 1. ОК, but how do we explain that the changes I've done solves the problem? 2. Why LDAPTLS_REQCERT=never ldapsearch -H ldaps://dc1.1plus1.corp/ -b dc=1plus1,dc=corp -D s.korsak@1plus1.corp -W '(objectSid=S-1-5-21-245695901-2433319663-187256136-500)' returns whole Administrator's record inspite of STRING (not BINARY) syntax in the filter? May be my DCs configuration is screwed and does not accept binary objectSids, but only ascii? The platform is FreeBSD/amd64 8.3-RELEASE. I'll check tomorrow what isprint() returns. Man page is here: http://www.freebsd.org/cgi/man.cgi?query=isprint&apropos=0&sektion=3&manpath=FreeBSD+8.3-RELEASE&arch=amd64&format=html I'll also check what LDAPTLS_REQCERT=never net ads -k search '(objectSid=S-1-5-21-245695901-2433319663-187256136-500)' sends do LDAP server. I beieve it sends string "S-...", because it works with default installation, while winbindd does not.
Well you are correct... Hmmm. I used a simple bind ldapsearch request using a text based query: ldapsearch -x -LLL -H ldap://172.27.69.195 -b 'dc=AAA,dc=BBB,dc=CCC,dc=com' -D 'Administrator@AAA.BBB.CCC.com' -w 'password' '(objectSid=S-1-5-32-548)' and did a wireshark capture of the packets, and it does clearly show the SID being sent as plaintext over the wire. I got back: dn: CN=Account Operators,CN=Builtin,DC=AAA,DC=BBB,DC=CCC,DC=com objectClass: top objectClass: group cn: Account Operators description: Members can administer domain user and group accounts distinguishedName: CN=Account Operators,CN=Builtin,DC=AAA,DC=BBB,DC=CCC,DC=com instanceType: 4 whenCreated: 20120320161211.0Z whenChanged: 20120320162722.0Z uSNCreated: 12360 uSNChanged: 12708 name: Account Operators objectGUID:: 3TFx+gv1MkKJ1XsC/TExPQ== objectSid:: AQIAAAAAAAUgAAAAJAIAAA== The objectSID reply is coming back as the binary representation, but it certainly seems the query for objectSid should be done as text.
Oh, not so fast. Looks like the query can be done by both text, *OR* by binary representation. Sid "S-1-5-32-548" is a binary string of: "\01\02\00\00\00\00\00\05\20\00\00\00\24\02\00\00" (as read from the wireshark return). So doing the query as: ldapsearch -x -LLL -H ldap://172.27.69.195 -b 'dc=AAA,dc=BBB,dc=CCC,dc=com' -D 'Administrator@AAA.BBB.CCC.com' -w 'password' '(objectSid=\01\02\00\00\00\00\00\05\20\00\00\00\24\02\00\00)' returns exactly the same result: dn: CN=Account Operators,CN=Builtin,DC=AAA,DC=BBB,DC=CCC,DC=com objectClass: top objectClass: group cn: Account Operators description: Members can administer domain user and group accounts distinguishedName: CN=Account Operators,CN=Builtin,DC=AAA,DC=BBB,DC=CCC,DC=com instanceType: 4 whenCreated: 20120320161211.0Z whenChanged: 20120320162722.0Z uSNCreated: 12360 uSNChanged: 12708 name: Account Operators objectGUID:: 3TFx+gv1MkKJ1XsC/TExPQ== objectSid:: AQIAAAAAAAUgAAAAJAIAAA== So what we're doing in winbindd should certainly work.
Yep, just to confirm I tried with a mix of \XX and ascii values and was able to retrieve my own account details via objectSid query. You need to look closely at the encoding of what is going over the wire on your platform. I can give you a simple patch that will force \XX encoding of all values, but they certainly should be allowed.
Yep, on this DC my SID is: objectSid=S-1-5-21-4068062641-1244396112-413245233-1103 which I can also retrieve as : objectSid=\01\05\00\00\00\00\00\05\15\00\00\00\b1\b5y\f2P\fa\2bJ\31\9f\a1\18O\04\00\00
(In reply to comment #8) > Yep, on this DC my SID is: > > objectSid=S-1-5-21-4068062641-1244396112-413245233-1103 > > which I can also retrieve as : > > objectSid=\01\05\00\00\00\00\00\05\15\00\00\00\b1\b5y\f2P\fa\2bJ\31\9f\a1\18O\04\00\00 So can I. Both variants returns "Account Operators" record in my environment, this: LDAPTLS_REQCERT=never ldapsearch -H ldaps://dc1.1plus1.corp/ -b dc=1plus1,dc=corp -D s.korsak@1plus1.corp -W '(objectSid=S-1-5-32-548)' and this: LDAPTLS_REQCERT=never ldapsearch -H ldaps://dc1.1plus1.corp/ -b dc=1plus1,dc=corp -D s.korsak@1plus1.corp -W '(objectSid=\01\02\00\00\00\00\00\05\20\00\00\00\24\02\00\00)' So the only suspicion I have now is ldb_binary_encode() returns somewhat wrong "binary string". Maybe it does not convert some characters because of LC_xxx environment. I'll research this tomorrow.
Oh, I think I know what this is !!! Your isprint() is returning true for the characters like ² ╔, and I wonder if Windows is mapping them incorrectly ! I have a quick patch which might help. Hang on a minute.
Created attachment 7864 [details] Test patch. Give this a try. If forces all characters outside of the US-ASCII range (and the prohibited ones) to be encoded as \XX.
Created attachment 7865 [details] Fixed test patch :-) Got 0x32 and 0x20 mixed up for space char :-). Try this.
Created attachment 7866 [details] One more fix.. Belt and braces :-). Fixes both implementations of ldb_binary_encode() to avoid using isprint().
(In reply to comment #13) > Created attachment 7866 [details] > One more fix.. > > Belt and braces :-). Fixes both implementations of ldb_binary_encode() to avoid > using isprint(). Unpatched winbindd works OK if started with LC_ALL=C environment. It founds user: Search for (objectSid=\01\05\00\00\00\00\00\05\15\00\00\00\9D\05\A5\0E\EF\82\09\91HM\29\0B\A4\14\00\00) in <dc=1PLUS1,dc=CORP> gave 1 replies If LC_ALL=uk_UA.CP1251, it fails: Search for (objectSid=\01\05\00\00\00\00\00\05\15\00\00\00²\05╔\0EО┌\09▒HM\29\0Bє\14\00\00) in <dc=1PLUS1,dc=CORP> gave 0 replies Patched winbindd now works OK inspite of locale settings. Thank you, Jeremy!
Created attachment 7874 [details] git-am fix for 3.6.x
Comment on attachment 7874 [details] git-am fix for 3.6.x Same code went into master.
Created attachment 7876 [details] git-am fix for 3.5.next Same fix for 3.5.next.
Jeremy, sorry, but this patch seems subobtimal to me. The check against concrete numbers is repeated verbatim in multiple places. And if you want to restrict to ascii, why don't you use "isascii()" ? That would read much less arbitrary. Cheers - Michael
Hmm, ok isascii is reported deprecated and not portable in the manpages... Is there an alternative existing function to provide what you want to achieve? Or could we at least create our own function? Cheers - Michael
It's a fair cop :-). I've factored out to a function in master, I'll re-spin the patches to do the same. FYI - can't use isascii as the restriction we need isn't the same as that set. Cheers, Jeremy.
Created attachment 7879 [details] List of SIDs in textual format
Created attachment 7880 [details] List of SIDs in bianry format
Created attachment 7881 [details] objectSid.text.txt
Created attachment 7882 [details] objectSid.bin.txt
I've created list of SIDs in my AD that differ by LSbyte, 0 to 255. The textual version is in sids0toff.text.txt (the 1st attach), binary version is in objectSid.bin.txt (the 2nd attach). Then I've executed, LDAPTLS_REQCERT=never ldapsearch -H ldaps://dc1.1plus1.corp/ -b dc=1plus1,dc=corp -D s.korsak@1plus1.corp -y .ldappass -f sids0toff.text.txt '(objectSid=%s)' objectSid | grep ^objectSid > objectSid.text.txt LDAPTLS_REQCERT=never ldapsearch -H ldaps://dc1.1plus1.corp/ -b dc=1plus1,dc=corp -D s.korsak@1plus1.corp -y .ldappass -f sids0toff.bin.txt '(objectSid=%s)' objectSid | grep ^objectSid > objectSid.bin.txt The results are competely equal, see two last attaches (objectSid.text.txt & objectSid.bin.txt). dc1.1plus1.corp runs Windows Server 2008 R2 Standard 7601 Service Pack 1. Do you know any official reference that describes the exact format of objectSid filter? Must it be S-txt-txt... or backslash escaped-sequence? In the last case, what bytes Must be esacaped, what bytes Must Not be escaped, and what bytes format has no matter? My case only shows that in AD v.2008 environment we can safely use textual or fully-backslash-escaped SIDs in LDAP '(objectSid=%s)' filter.
Created attachment 7883 [details] git-am fix for 3.6.next Ok, as requested by Michael, abstracted the decision into a fn call.
Created attachment 7884 [details] (correct) git-am fix for 3.6.next. Uses functions as requested by obnox.
Created attachment 7885 [details] git-am fix for 3.5.next. Uses functions as requested.
For the completeness I've tested what characters must be converted to \XX sequence to be accepted by AD LDAP in '(objectSid=%s)' filter. The result surprised me very much - 250 of 256 are accepted as-is. Only \00 (nul), \28 (left parenthesis), \29 (right parenthesis), \2A (asterisk) \5C (backslash) caused "Bad search filter" error or "Nothig found" result. But the latest four, being backslashed - \(, \), \*, \\ were accepted by LDAP and returned correct object. Only nul must be definitely written as \00, but it may quite be caused by openldap's internal coding (C-style End of String, etc...). This conclusion is independent of LC_xxx locale setting. So It's a puzzle why the original (unpatched) winbindd could not fetch info from LDAP sending not-escaped objectSid filter. Cheers!
I agree your tests show what is expected, but I've always abided by the "be conservative in what you send" code, so I think we're safer with this patch (it's already in master). Jeremy.
> So It's a puzzle why the original (unpatched) winbindd could not fetch info > from LDAP sending not-escaped objectSid filter. > A possible key is that openldap converts raw characters to \XX sequences itself. The behavior of the "net" tool indirectly confirms such guess because it doesn't find user so: net ads search '(objectSid=\01\05\00\00\00\00\00\05\15\00\00\00\9D\05\A5\0E\EF\82\09\91\48\4D\29\0Bя\22\00\00)' while ldapsearch '(objectSid=\01\05\00\00\00\00\00\05\15\00\00\00\9D\05\A5\0E\EF\82\09\91\48\4D\29\0Bя\22\00\00)' finds. > I agree your tests show what is expected, but I've always abided by the "be > conservative in what you send" code, so I think we're safer with this patch > (it's already in master). > Jeremy. I have no objection against the patch. It does what it should. I just think about wouldn't it be safer to remap all 256 characters into \XX sequence. The byte overtraffic will be negligible, the packet overtraffic will be missing.
I've tcpdump-ed what ldapsearch sends to LDAP server. 1. ldapsearch '(objectSid=S-1-5-21-245695901-2433319663-187256136-8959)' : 000001b0 01 00 02 01 00 02 01 00 01 01 00 a3 39 04 09 6f |...........Ј9..o| 000001c0 62 6a 65 63 74 53 69 64 04 2c 53 2d 31 2d 35 2d |bjectSid.,S-1-5-| 000001d0 32 31 2d 32 34 35 36 39 35 39 30 31 2d 32 34 33 |21-245695901-243| 000001e0 33 33 31 39 36 36 33 2d 31 38 37 32 35 36 31 33 |3319663-18725613| 000001f0 36 2d 38 39 35 39 30 00 43 af 51 50 5a 59 04 00 |6-89590.CЇQPZY..| Just cleartext, isn't it ? 2. ldapsearch '(objectSid=\01\05\00\00\00\00\00\05\15\00\00\00\9D\05\A5\0E\EF\82\09\91\48\4D\29\0B\FF\22\00\00)' 000001b0 01 00 02 01 00 02 01 00 01 01 00 a3 29 04 09 6f |...........Ј)..o| 000001c0 62 6a 65 63 74 53 69 64 04 1c 01 05 00 00 00 00 |bjectSid........| 000001d0 00 05 15 00 00 00 9d 05 a5 0e ef 82 09 91 48 4d |......ќ.Ґ.п‚.‘HM| 000001e0 29 0b ff 22 00 00 30 00 41 ad 51 50 b4 a4 0b 00 |).я"..0.AQPґ¤..| Note bytes 01ca-01e5. There are no backslashes - just sequence of bytes. 3. ldapsearch '(objectSid=\01\05\00\00\00\00\00\05\15\00\00\00\9D\05\A5\0E\EF\82\09\91\48\4D\29\0Bя\22\00\00)' (one byte is unescaped) Dump is the same, as in paragraph 2. What sends "net ads search" is a mystery, because it unconditionally uses StartTLS. So openldap client or library deals with escaping-unescaping, LDAP server just gets raw byte sequence. According to http://tools.ietf.org/html/rfc4515#section-3 we must escape octets "*" (\2a), "(" (\28), ")" (\29), "\" (\\5c), NUL (\00) and all octets greater than 0x7F (\80 to \ff). Other octets MAY be escaped.
Comment on attachment 7884 [details] (correct) git-am fix for 3.6.next. This reduces the code duplication from 4 to 2. ;-) Thanks Jeremy
Comment on attachment 7885 [details] git-am fix for 3.5.next. This reduces the code duplication from 4 to 2. ;-) Thanks Jeremy
==> Karolin for inclusion into the releases...
Pushed to v3-5-test and v3-6-test. Closing out bug report. Thanks!