[PATCH] allow passdb backend to change trusted domain object password with clear text

[Alexander Bokovoy]

On ti, 11 huhti 2017, Alexander Bokovoy via samba-technical wrote:
> On ti, 11 huhti 2017, Andrew Bartlett wrote:
> > On Mon, 2017-04-10 at 16:51 +0300, Alexander Bokovoy wrote:
> > > On ma, 10 huhti 2017, Andrew Bartlett wrote:
> > > > On Mon, 2017-04-10 at 10:22 +0300, Alexander Bokovoy wrote:
> > > > > On pe, 07 huhti 2017, Jeremy Allison wrote:
> > > > > > On Fri, Apr 07, 2017 at 11:08:44PM +0300, Alexander Bokovoy
> > > > > > wrote:
> > > > > > 
> > > > > > > I'll see what I can do there but this code is a copy/paste
> > > > > > > from
> > > > > > > another
> > > > > > > helper we have for NT/LM hash pass-through. Guenther already
> > > > > > > asked me to
> > > > > > > consider how I can going these two functions in a common
> > > > > > > piece
> > > > > > > that
> > > > > > > could be called for both cases, so I'll do refactoring for
> > > > > > > this
> > > > > > > too.
> > > > > > 
> > > > > > Thanks. That other code might be wrong too :-).
> > > > > 
> > > > > Attached is the patch that unifies NT hash/plain text pass-
> > > > > through.
> > > > > This is a more concise change that doesn't duplicate existing
> > > > > code.
> > > > 
> > > > How certain are we that our plaintext password is null terminated?
> > > > 
> > > > +	cr.creds.password = (const char*) plaintext.data;
> > > 
> > > Good question. A plain text password is a data blob represented as up
> > > to
> > > 256 WCHARs. It is UTF-16 coded on wire and we have its length from
> > > the
> > > buffer.  SetUserInfo2 SAMR call chain in decode_pw_buffer() does
> > > explicitly
> > > expect 512+4 bytes in the buffer. It then calls
> > > convert_string_talloc() to
> > > convert it to UNIX charset passing the correct value of the plaintext
> > > password length. However, convert_string_talloc() expects the length
> > > of
> > > input string *including* the terminating null and we pass just the
> > > string length.
> > > 
> > > convert_string_talloc() then explicitly null-terminates the resulting
> > > string by adding two nulls. In most cases UNIX charset is UTF-8, so
> > > we
> > > get null-terminated UTF-8 string down to PASSDB layer.
> > > 
> > > However, MS-SAMR does not limit what does the password should
> > > contain.
> > > It says it is 'userPassword' value. Either 'userPassword' or
> > > 'unicodePwd' cannot contain null characters according to MS-ADTS
> > > 3.1.1.3.1.5 because they must be proper UTF-8 and UTF-16 strings
> > > accordingly. 
> > > 
> > > So I think we are good here,
> > 
> > Please copy this into a comment, so we remember and an auditor can be
> > re-assured. 
> Re-sent with updated comment.
Could please anyone to review updated patch (in previous email)?

-- 
/ Alexander Bokovoy