[PATCH] Re: netlogon_creds_cli_validate() in master4-schannel
Stefan (metze) Metzmacher
metze at samba.org
Thu Dec 19 17:43:40 MST 2013
Am 20.12.2013 01:17, schrieb Andrew Bartlett:
> On Fri, 2013-12-20 at 00:46 +0100, Stefan (metze) Metzmacher wrote:
>> Hi Andrew,
>>
>>>>>>>>>>> Thanks! Are you able to do a wintest with this?
>>>>>>>>>>>
>>>>>>>>>>> I also want to do some tests with windows dcs.
>>>>>>>>>>>
>>>>>>>>>>> I important thing I want to verify is the behavior of
>>>>>>>>>>>
>>>>>>>>>>> invalidate_cm_connection(&domain->conn);
>>>>>>>>>>> + domain->conn.netlogon_force_reauth = true;
>>>>>>>>>>>
>>>>>>>>>>> in _wbint_CheckMachineAccount() and related code.
>>>>>>>>>>>
>>>>>>>>>>> Testing against a s4 dc showed that we are doing
>>>>>>>>>>> netr_ServerReqChallenge/netr_ServerAuthenticate3 over a connection
>>>>>>>>>>> with DCERPC_AUTH_TYPE_SCHANNEL/DCERPC_AUTH_LEVEL_PRIVACY and I'm not
>>>>>>>>>>> sure Windows also likes that.
>>>>>>>>>>>
>>>>>>>>>>> I think some combination of 'wbinfo -t' and 'wbinfo -c' triggered that.
>>>>>>>>>>>
>>>>>>>>>>> Günther can you also do some tests with your VMs?
>>>>>>>>>> I'll get Garming to give this a test against some real Windows VMs, and
>>>>>>>>>> yes, this is a very good excuse to get wintest running again.
>>>>>>>>>>
>>>>>>>>>> Andrew Bartlett
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> It appears to work just fine on my end.
>>>>>>>>
>>>>>>>> Against what windows versions did you test?
>>>>>>>
>>>>>>> Garming tested with 2008R2.
>>>>>>>
>>>>>>>> I've tested today against a w2012 dc and found that it works.
>>>>>>>>
>>>>>>>> I just found one bug when using net rpc testjoin, which triggered
>>>>>>>> a DCERPC_FAULT_SEC_PKG_ERROR.
>>>>>>>> This commit should fix the problem for now:
>>>>>>>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=88d3b57a7f744c4be39668031717df146eba7e6d
>>>>>>>> it's part of
>>>>>>>> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-schannel-ok
>>>>>>>> now.
>>>>>>>>
>>>>>>>> I've done some captures see
>>>>>>>> https://www.samba.org/~metze/ads/caps/netlogon/v4-0-schannel/20131213/
>>>>>>>>
>>>>>>>> I'll try to do some more testing on monday.
>>>>>>
>>>>>> I've also tested with Windows 2008 and will do with nt4 and windows 2000
>>>>>> and some samba versions.
>>>>>>
>>>>>> I have some updates in my
>>>>>> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-schannel-ok
>>>>>> branch.
>>>>>>
>>>>>> While testing with winbind sealed pipes = no, I noticed that we send the
>>>>>> same Authenticator again and again to a dc that returns NOT_IMPLEMENTED
>>>>>> to LogonGetCapabilities(). As this is the first request on each schannel
>>>>>> connection,
>>>>>> I think it's better to avoid this, as the session key is much more long
>>>>>> living now.
>>>>>>
>>>>>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=fa68a5814d7ad3fb48b22eaaad1bdb0ed2fc495c
>>>>>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=5df6c619f5670b71e04ab047a2d6f12073d376dc
>>>>>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=485ed1950affa3b9da0d78dc927c4185b2111e8c
>>>>>>
>>>>>> are the cleanup ups for this.
>>>>>>
>>>>>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=23896aefe5f50ba977167a85b1b6189dd65d03f0
>>>>>> got netlogon_creds_cli_open_global_db()
>>>>>> which is used in
>>>>>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=82e902bad329a0734ab2b4c1436f53c440cca4ef
>>>>>> which is used in
>>>>>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=46949116273667b65d7ac59f1d1a11ec9284f963
>>>>>>
>>>>>> This makes sure that the winbind parent opens the netlogon_creds_cli.tdb
>>>>>> and it doesn't get cleared
>>>>>> if a child was killed and a new one was started. This way we only do a
>>>>>> ServerChallenge/ServerAuthenticate
>>>>>> pair when winbindd is restarted or the dc gets restarted.
>>>>>
>>>>> I've looked over the changes individually, and the diff between what I
>>>>> last reviewed and your current tree. On that basis:
>>>>>
>>>>> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
>>>>>
>>>>> Thanks for all your great work here!
>>>>
>>>> After testing with NT4 4.0 SP6a I have some little changes.
>>>>
>>>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=e20b593ce69298f1dc7ec0ac126f865173d4f3c9
>>>>
>>>> Here I changed the comment, when getting NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE
>>>> in netlogon_creds_cli_check_caps(), NT 4.0 also returns that.
>>>
>>> I'm a bit worried about accepting that. This is the one error that can
>>> be trivially faked by an attacker. As you noticed we can do an auth2
>>> over SCHANNEL, could we use that to verify the flags instead in this
>>> case?
>>
>> If we get NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE, it means we don't
>> increment the schannel_state->seq_num. Which means the server would detect
>> an attacker with the next request. And we as client would also detect it
>> in the next response.
>
> Hmm. I'm cautious about this given Microsoft said they specifically
> needed this to avoid an unauthenticated reply.
I'm thinking about doing a dummy call that expected an authenticated
response
after getting NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE in order to verify the
seq_num
state immediately without waiting for the next "userspace" request.
>> So we could also remove this block:
>>
>> if (tmp & NETLOGON_NEG_SUPPORTS_AES) {
>> /*
>> * NT 4.0 and Old Samba servers need
>> * "disable aes schannel = yes"
>> */
>> status = NT_STATUS_DOWNGRADE_DETECTED;
>> tevent_req_nterror(req, status);
>> netlogon_creds_cli_check_cleanup(req, status);
>> return;
>> }
I think we should bail out there if we require strong or aes
and bail out on NT_STATUS_NOT_IMPLEMENTED if we require aes.
>> And maybe also the "disable aes schannel" option completely.
>>
>> What do you think?
>
> Either way, I would like "disable aes schannel" removed, if only because
> it seems to me to be a very confusing option.
>
> I also think that by the time Samba 4.2 is released, I really think that
> Samba DCs before bug #6099 would be very, very rare. Not only are they
> beyond security support, anyone who added even a trial Windows 7 desktop
> would have hit the issues and found it easier to upgrade the servers
> rather than face setting registry keys across the whole site.
>
> I understand wanting to be able to control this for testing, but for
> user experience can we safely roll this in to "require strong keys = no"
> somehow?
I think that's doable.
metze
More information about the samba-technical
mailing list