[PATCH] s4:dsdb/common: samdb_result_parameters: fix bug in ldb_val to lsa_BinaryString conversation
gohmann at univention.de
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
>>> > 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
>>> > 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:
>>> 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
>> 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
> 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:
Decoding via python shows the following:
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?
Head of Software Engineering
Tel. : +49 421 22232-0
Fax : +49 421 22232-99
gohmann at univention.de
Geschäftsführer: Peter H. Ganten
HRB 20755 Amtsgericht Bremen
* Englisch - erkannt
More information about the samba-technical