SCHANNEL crypto failures with s3-dcerpc: avoid talloc_move on schannel creds in cli_rpc_pipe_open_schannel_with_key().

[Andrew Bartlett]

On Tue, 2010-08-24 at 10:47 +1000, Andrew Bartlett wrote:
> On Mon, 2010-08-23 at 19:04 -0500, Günther Deschner wrote:
> > The branch, master has been updated
> >        via  898c612... s3-dcerpc: avoid talloc_move on schannel creds in cli_rpc_pipe_open_schannel_with_key().
> >       from  33060f6... Final part of fix for bug #7636 - winbind internal error, backtrace.
> > 
> > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> > 
> > 
> > - Log -----------------------------------------------------------------
> > commit 898c6123355a3a11ec17f0396c4cb3018c75c184
> > Author: Günther Deschner <gd at samba.org>
> > Date:   Mon Aug 23 16:02:23 2010 +0200
> > 
> >     s3-dcerpc: avoid talloc_move on schannel creds in cli_rpc_pipe_open_schannel_with_key().
> >     
> >     Initially, the schannel creds were talloc memduped, then, during the netlogon
> >     creds client merge (baf7274fed2f1ae7a9e3a57160bf5471566e636c) they were first
> >     talloc_referenced and then later (53765c81f726a8c056cc4e57004592dd489975c9)
> >     talloc_moved.
> >     
> >     The issue with using talloc_move here is that users of that function in winbind
> >     will only be able to have two schanneled connections, as the cached schannel
> >     credentials pointer from the netlogon pipe will be set to NULL. Do a deep copy
> >     of the struct instead.
> 
> Is this really correct?  I would have said that talloc_reference() is
> the right thing to do here, as it is shared state.  If one connection
> does any call that updates the credentials chain, then the other
> connections state must reflect that update, otherwise their calls
> (SamLogon and other calls that do the credential chaining) will fail -
> see how we have do this via a tdb on the server. 

Günther,

Can you please look into this before we loose the context here?  I fear
that if we leave the code like this, we will be back here again to fix
it up in just another few months, with more weird 'credential check
failure' messages. 

The cryptographic state MUST be shared between all callers that use the
same netbios name.  If we do not share this on the memory pointers we
will instead have to share it in a TDB or similar instead.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100828/2e2a900a/attachment.pgp>