[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
>>>>>>>>>>> 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
>>>>>>>> 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
after getting NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE in order to verify the
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.


More information about the samba-technical mailing list