[PATCH] s4:dsdb/common: samdb_result_parameters: fix bug in ldb_val to lsa_BinaryString conversation

Fri May 30 08:54:09 MDT 2014

Am 30.05.2014 01:29, schrieb Andrew Bartlett:
> On Tue, 2014-05-27 at 06:37 +0200, Stefan Gohmann wrote:
>> Hi Karmen,
>>
>> Am 27.05.2014 03:24, schrieb Kamen Mazdrashki:
>>> Hi Stefan,
>>>
>>>
>>> On Mon, May 26, 2014 at 11:03 PM, Stefan Gohmann <gohmann at univention.de
>>> <mailto:gohmann at univention.de>> wrote:
>>>
>>>     Hi Matthias,
>>>
>>>     Am 26.05.2014 14 <tel:26.05.2014%2014>:09, schrieb Matthias Dieter
>>>     Wallnöfer:
>>>     > Hi Stefan,
>>>     >
>>>     > it seems that no other one responded. I think that this is rather a
>>>     > problem with the "userParameters" attribute which we still do not
>>>     parse
>>>     > correctly I think.
>>>
>>>     thanks. I've re-checked this issue.
>>>
>>>     In my case the userParameters attribute is 105 bytes long. I think the
>>>     problem is that the length is odd and so there is one byte missing.
>>>
>>>     I was able to reproduce this issue while setting userParameters for a
>>>     user to the following value:
>>>
>>>     userParameters::
>>>     Q3R4Q2ZnUHJlc2VudCAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUAIaCAFDdHhDZmdQcmVzZW5045S15pSx5oiw44GiGAgBQ3R4Q2ZnRmxhZ3Mx44Cw44Gm44Cy44C5
>>>
>>>     and run
>>>       net user /domain username
>>>     from a Windows client.
>>>
>>>     Attached you'll find an updated patch which fixed the problem for me.
>>>
>>> IMHO, the correct fix should be:
>>> +s.array = talloc_array(mem_ctx, uint16_t, (val->length + 1) / 2);
>>> so we don't allocate  an extra bytes in 'normal' scenario when length is
>>> even.
>>
>> you are right. Please find a new patch attached.
> 
> This may correctly fill in the array without overwriting memory, but it
> just touches the tip of the iceburg here.  
> 
> For example, do we now leak 1 byte of uninitialised server memory?
> 
> How did an odd value get in here?  Probably via our DRS replication, but
> possibly via a SAMR call.  
> 
> I have to look up the full history, but until Samba 4.1, we really
> messed this up, because we treated the value as 'UTF8' in some places,
> particularly while replicating.  The values being sent are not valid
> UTF-16 sequences, so this would often fail. 
> 
> Then we added code to the schema to say that for this attribute, not to
> convert it to/from UTF-8.  This means that our LDAP interface now
> exposes the UTF-16 (or raw) formatted value.  This may or may not
> matter, Microsoft assures us LDAP should not be relied on for this
> attribute, but probably should change to a 'algorithmic UTF8' conversion
> of the raw bytes. 
> 
> The copying of the memory from  to the uint16_t array is also a problem,
> because this all assumes we never operate on a big-endian host.
> 
> Then there comes the issue of fixing up these 'utf8' formatted blobs in
> the database.  We have thoughts on how we could detect these, but we
> need to sit down and spend a day or two sorting it out in dbcheck,
> preferably with tests.  We need to expand these 'UTF8' sequences out
> into the raw bytes we now use as the native storage method.
> 
> We also need to sort out the classicupgrade codepath, and the script
> metze has been using to migrate customers to AD.
> 
> Finally we need to have our tests read and write a real (and not just
> short) value via the RPC-SAMR tests, and perhaps validate that with
> LDAP.
> 
> It is a lot of work, but you have sadly seen what happens when we leave
> this area as 'known broken' as we did for 4.1. 
> 
> I hope this clarifies this situation,
> 
> Andrew Bartlett
> 

Thanks for your explanation.

I did some more tests. If I use a plain Active Directory (Windows 2008
Server R2) without any Samba 4 connection, create a test user and set
the Windows Terminal Server Profile Path to '\\testserver\path', I get
the following value for userParameters:

ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUAUaCAFDdHhDZmdQcmVzZW5045S15pSx5oiw44GiGAgBQ3R4Q2ZnRmxhZ3Mx44Cw44Gm44Cy44C5EggBQ3R4U2hhZG9344Sw44Cw44Cw44CwKgIBQ3R4TWluRW5jcnlwdGlvbkxldmVs44SwICQBQ3R4V0ZQcm9maWxlUGF0aOaMteaMteOQt+OUtuOMt+OQt+OMt+OUtuOIt+OYt+OUtuOIt+aMteOAt+OEtuOQt+OgtuOAsA==

Decoding via python shows the following:

>>>
len(base64.decodestring('ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUAUaCAFDdHhDZmdQcmVzZW5045S15pSx5oiw44GiGAgBQ3R4Q2ZnRmxhZ3Mx44Cw44Gm44Cy44C5EggBQ3R4U2hhZG9344Sw44Cw44Cw44CwKgIBQ3R4TWluRW5jcnlwdGlvbkxldmVs44SwICQBQ3R4V0ZQcm9maWxlUGF0aOaMteaMteOQt+OUtuOMt+OQt+OMt+OUtuOIt+OYt+OUtuOIt+aMteOAt+OEtuOQt+OgtuOAsA=='))
229
>>>
base64.decodestring('ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUAUaCAFDdHhDZmdQcmVzZW5045S15pSx5oiw44GiGAgBQ3R4Q2ZnRmxhZ3Mx44Cw44Gm44Cy44C5EggBQ3R4U2hhZG9344Sw44Cw44Cw44CwKgIBQ3R4TWluRW5jcnlwdGlvbkxldmVs44SwICQBQ3R4V0ZQcm9maWxlUGF0aOaMteaMteOQt+OUtuOMt+OQt+OMt+OUtuOIt+OYt+OUtuOIt+aMteOAt+OEtuOQt+OgtuOAsA==')
'
P\x05\x1a\x08\x01CtxCfgPresent\xe3\x94\xb5\xe6\x94\xb1\xe6\x88\xb0\xe3\x81\xa2\x18\x08\x01CtxCfgFlags1\xe3\x80\xb0\xe3\x81\xa6\xe3\x80\xb2\xe3\x80\xb9\x12\x08\x01CtxShadow\xe3\x84\xb0\xe3\x80\xb0\xe3\x80\xb0\xe3\x80\xb0*\x02\x01CtxMinEncryptionLevel\xe3\x84\xb0
$\x01CtxWFProfilePath\xe6\x8c\xb5\xe6\x8c\xb5\xe3\x90\xb7\xe3\x94\xb6\xe3\x8c\xb7\xe3\x90\xb7\xe3\x8c\xb7\xe3\x94\xb6\xe3\x88\xb7\xe3\x98\xb7\xe3\x94\xb6\xe3\x88\xb7\xe6\x8c\xb5\xe3\x80\xb7\xe3\x84\xb6\xe3\x90\xb7\xe3\xa0\xb6\xe3\x80\xb0'
>>>
base64.decodestring('ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUAUaCAFDdHhDZmdQcmVzZW5045S15pSx5oiw44GiGAgBQ3R4Q2ZnRmxhZ3Mx44Cw44Gm44Cy44C5EggBQ3R4U2hhZG9344Sw44Cw44Cw44CwKgIBQ3R4TWluRW5jcnlwdGlvbkxldmVs44SwICQBQ3R4V0ZQcm9maWxlUGF0aOaMteaMteOQt+OUtuOMt+OQt+OMt+OUtuOIt+OYt+OUtuOIt+aMteOAt+OEtuOQt+OgtuOAsA==').encode('hex')
'20202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202050051a080143747843666750726573656e74e394b5e694b1e688b0e381a2180801437478436667466c61677331e380b0e381a6e380b2e380b9120801437478536861646f77e384b0e380b0e380b0e380b02a02014374784d696e456e6372797074696f6e4c6576656ce384b0202401437478574650726f66696c6550617468e68cb5e68cb5e390b7e394b6e38cb7e390b7e38cb7e394b6e388b7e398b7e394b6e388b7e68cb5e380b7e384b6e390b7e3a0b6e380b0'
>>>

Opening the user in MMC shows an unicode value for the attribute
userParameters in the attribute editor. And the substring
CtxWFProfilePath doesn't look like UTF-16.

I think if I join Samba 4 into this AD domain and try to login against
Samba 4, the rpc server will get stuck. The patch should fix this case.
Or am I missing something?

Thanks,
Stefan

-- 
Stefan Gohmann
Head of Software Engineering

Univention GmbH
be open.
Mary-Somerville-Str.1
28359 Bremen
Tel. : +49 421 22232-0
Fax :  +49 421 22232-99

gohmann at univention.de
http://www.univention.de

Geschäftsführer: Peter H. Ganten
HRB 20755 Amtsgericht Bremen
Steuer-Nr.: 71-597-02876

  * Englisch - erkannt
  * Englisch
  * Deutsch

  * Englisch
  * Deutsch

 <javascript:void(0);>