[PATCH] Re: netlogon_creds_cli_validate() in master4-schannel
Stefan (metze) Metzmacher
metze at samba.org
Thu Dec 19 16:46:07 MST 2013
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.
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;
}
And maybe also the "disable aes schannel" option completely.
What do you think?
>> I tested with this settings:
>>
>> # NT 4.0 sp6a
>> security = domain
>> workgroup = NT4DOM
>> require strong key = no
>> disable aes schannel = yes
>> client NTLMv2 auth = no
>>
>> I also tested if "client schannel = no" still works.
>> I had to change netlogon_creds_cli_auth_send() and
>> we always start with try_auth3 and try_auth2, we also
>> set require_auth2 if require any of NETLOGON_NEG_ARCFOUR,
>> NETLOGON_NEG_STRONG_KEYS, NETLOGON_NEG_PASSWORD_SET2,
>> NETLOGON_NEG_SUPPORTS_AES. I may add NETLOGON_NEG_AUTHENTICATED_RPC
>> after testing against 3.0.37, it seems that NT 4.0 and Samba 3.0.37 both
>> support Authenticate2.
>
> Yes, NT 4.0 and 3.0.37 both support Authenticate2, it's been in the
> protocol as long as I have been involved in Samba.
Ok, then I think I'll require auth2 and add
NETLOGON_NEG_AUTHENTICATED_RPC to
the required flags unless we have "client schannel = no".
Later we could think about removing this parameter completely.
>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=9447e1d665991af78d71223ca099a9a8a49d88ce
>> Here I explained where "disable aes schannel = yes" might be needed.
>
> I don't want to get in the way of this getting into master, but this
> explanation makes me feel like the parameter is poorly named.
It was modeled to behave like a windows client that doesn't support aes.
As written above we might be able to remove it.
>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=c78e78cea8b9f25e79407c12a60410e755d05f58
>> Here I fixed the flags we require with "reject md5 servers = yes".
>>
>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=371fa99fd1ae88898839d0c4a26bd5a3ae4e0a39
>> Here I fixed the flags we require with "require strong key = yes" (the
>> default)
>> and that "require strong key = no" is needed for Samba < 3.5.
>
> I think you pasted the wrong link here.
https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=35fa79d6873fb20fa78e75c4a6566f46cb9a3a32
> Do we really need all of "disable aes schannel", "require strong key"
> and "reject md5 servers", and can we rename these to have some link to
> the NETLOGON pipe that they apply to? As they are, users will set them
> randomly without really understanding them.
I can add a "netlogon" prefix to them, but I think we need all of them.
metze
More information about the samba-technical
mailing list