[PATCH] s3-winbindd: Store schannel credentials in secrets.tdb

[Stefan (metze) Metzmacher]

Hi Christof,

> Andrew Bartlett <abartlet at samba.org> wrote on 09/19/2012 06:12:57 PM:
> 
>> On Wed, 2012-09-19 at 15:07 -0700, Christian Ambach wrote:
>>> On 09/19/2012 01:40 PM, Christof Schmitt wrote:
>>>>
>>>> Passing a dbwrap handle to the code is an easy change. What
>>>> complicated things was that my approach was to fetch a locked record
>>>> and keep it locked during the DC authentication. The code in
>>>> schannel_state_tdb.c does not keep the lock, so this needs to be
>>>> changed, or an additional lock would be required to guarantee
>>>> exclusive access to the DC during the authentication.
>>>
>>> You could add a _locked variant that returns the record in locked 
> state.
> 
> Here is a new patch series that switches schannel_state_tdb to dbwrap,
> adds _locked variants and uses those in winbindd_cm. With these
> patches, smbtorture base.bench now runs on a cluster without errors,
> this is the test where we first found this issue.
> 
>>>> A related question: cm_prepare_connection in
>>>> source3/winbindd/winbindd_cm.c already uses a mutex. Can someone
>>>> describe what this mutex protects?
>>>
>>> There are some comments in auth/auth_domain.c explaining the need for 
>>> the mutex:
>>>
>>> /* we use a mutex to prevent two connections at once - when a·
>>>     Win2k PDC get two connections where one hasn't completed a·
>>>     session setup yet it will send a TCP reset to the first·
>>>     connection (tridge) */
>>
>> To understand this, read 'reset on zero vc' in man smb.conf
>>
>>> /*
>>>   * With NT4.x DC's *all* authentication must be serialized to avoid
>>>   * ACCESS_DENIED errors if 2 auths are done from the same machine. 
> JRA.
>>>   */
>>
>> This to us not understanding the need for exactly this patch set, so a
>> finished patch set would remove this comment as obsolete, once this code
>> uses it as well. 
> 
> Thanks for the explanation. My understanding is that the schannel
> problem is different from the one when first establishing sessions, so
> we need to keep that mutex.

I have problems with the 3rd patch, because we can't hold a tdb lock
while doing network operations! This needs to be avoided because we need to
avoid deadlocks.

I think we need a model like we have for some file server tdbs
where we store the server_id of the process which "locks" the record,
and use the new dbwrap_watch_record_send/recv api to wait to get the lock.

Also for the netr_Authenticator credential chain the logic is much more
complex
for the clients, it's not enough to mutex the setup of the
netlogon_creds_CredentialState,
we also need to mutex the netlogon_creds_CredentialState->sequence etc.
And on the client we typically need to mutex arround network/ipc operations,
which should not be mutexed by a tdb lock.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120926/61fb9649/attachment.pgp>