[PATCH] Fix for a bug in MacOS X Sierra NTLMv2 processing.

[Jeremy Allison]

On Sat, Jun 24, 2017 at 01:21:13PM +0200, Stefan Metzmacher wrote:
> Hi Jeremy,
> 
> I've created https://bugzilla.samba.org/show_bug.cgi?id=12862
> for this.

Thanks !

> What about the attached patch?

I originally thought about putting it there, but doesn't this also
adds it on the server side if the client neglects to offer it
(gensec_ntlmssp_server_negotiate() calls ntlmssp_handle_neg_flags()
as well as ntlmssp_client_challenge()) ?

Ah, never mind - it's gated on "ntlmssp_state->use_ntlmv2"
which is only set true in client setup.

Pushed with an additional comment explaining that !

Jeremy.

> >>> Found at the plugfest. The Apple MacOS X Sierra SMB2
> >>> server has a bug. It only supports NTLMv2 but doesn't
> >>> negotiate it in the chal_flags returned to the client.
> >>>
> >>> Windows clients work as use NTLMv2 by default and ignore
> >>> the negotiate but. Here is a patch that adds a tunable
> >>> ntlmssp_client:force ntlmv2 (default off) that allows
> >>> smbclient, libsmbclient and associated tools to still
> >>> connect to the MacOS X Sierra SMB2 server.
> >>>
> >>> I'm ambivilent about this - this is a server bug, but
> >>> until they fix it no Samba client tools can connect to
> >>> this server without this fix.
> >>
> >> We can safely force on the NTLM2 flag if we are using NTLMv2 as a
> >> client (which isn't negotiated, it is configured).  It is required, so
> >> we don't really gain anything by giving an error vs proceeding: we may
> >> as well force it and see how we go.
> > 
> > This is also how Windows clients behave.
> > 
> >>> ntlmssp_handle_neg_flags: Got challenge flags[0x22810205] - possible downgrade detected! missing_flags[0x00080000] - NT code 0x80090302
> >>>   NTLMSSP_NEGOTIATE_EXTENDED_SESSIONSECURITY
> >>> SPNEGO(ntlmssp) login failed: NT code 0x80090302
> >>> session setup failed: NT code 0x80090302
> >>>
> >>> Should I log a Samba bug ? Do we want this patch ?
> >>
> >> I don't see why we want the option, but we should log the bug and have
> >> a simpler patch to just force it on.
> >>
> >> Naturally, get any final patch past metze :-)
> > 
> > Yeah, we could remove the:
> > 
> > gensec_setting_bool(gensec_security->settings,
> > +                                       "ntlmssp_client",
> > +                                       "force ntlmv2",
> > +                                       false)) {
> > 
> > part and just force it on if ntlmssp_state->use_ntlmv2
> > is true.
> > 
> > Metze, comments ?
> 
> What about the attached patch.
> 
> 

> From 1da97d8bb89bc9d4c73195f87ee9d4396ae0c633 Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <metze at samba.org>
> Date: Sat, 24 Jun 2017 13:16:03 +0200
> Subject: [PATCH] auth/ntlmssp: enforce NTLMSSP_NEGOTIATE_NTLM2 for the NTLMv2
>  client case
> 
> Some server may not announce the NTLMSSP_NEGOTIATE_NTLM2
> (a.k.a. NTLMSSP_NEGOTIATE_EXTENDED_SESSIONSECURITY) bit.
> 
> But if we're acting as a client using NTLMv2 we need to
> implies this flag, because it's not really a negotiationable
> in that case.
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12862
> 
> Signed-off-by: Stefan Metzmacher <metze at samba.org>
> ---
>  auth/ntlmssp/ntlmssp_util.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/auth/ntlmssp/ntlmssp_util.c b/auth/ntlmssp/ntlmssp_util.c
> index 4ae6101..1d48095 100644
> --- a/auth/ntlmssp/ntlmssp_util.c
> +++ b/auth/ntlmssp/ntlmssp_util.c
> @@ -75,6 +75,24 @@ NTSTATUS ntlmssp_handle_neg_flags(struct ntlmssp_state *ntlmssp_state,
>  {
>  	uint32_t missing_flags = ntlmssp_state->required_flags;
>  
> +	if (ntlmssp_state->use_ntlmv2) {
> +		/*
> +		 * Using NTLMv2 as a client implies
> +		 * using NTLMSSP_NEGOTIATE_NTLM2
> +		 * (NTLMSSP_NEGOTIATE_EXTENDED_SESSIONSECURITY)
> +		 *
> +		 * Even if the server has a bug an not announce
> +		 * it, we need to assume it's present.
> +		 *
> +		 * Note that we also have the flag
> +		 * in ntlmssp_state->required_flags,
> +		 * see gensec_ntlmssp_client_start().
> +		 *
> +		 * See bug #12862.
> +		 */
> +		flags |= NTLMSSP_NEGOTIATE_NTLM2;
> +	}
> +
>  	if (flags & NTLMSSP_NEGOTIATE_UNICODE) {
>  		ntlmssp_state->neg_flags |= NTLMSSP_NEGOTIATE_UNICODE;
>  		ntlmssp_state->neg_flags &= ~NTLMSSP_NEGOTIATE_OEM;
> -- 
> 1.9.1
>