New issue
Advanced search Search tips

Issue 726080 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Mac
Pri: 2
Type: Bug-Security



Sign in to add a comment

NTLM implementation can have security downgraded by bad server

Project Member Reported by zentaro@google.com, May 24 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36

Steps to reproduce the problem:
Have a HTTP server with a custom NTLM implementation that clears the NTLM_NegotiateNTLM2Key (0x00080000) bit in the flags field of the challenge (aka type 2 message).

What is the expected behavior?
Browser/client still sends a response that uses extended session security (aka NTLM2).

What went wrong?
Browser is downgraded and sends a vanilla NTLMv1 response without extended session security. This response is moderately easy to derive the users password from.

The client should always use extended session security if it requested it in the negotiate (type 1) message. 

The bug is here https://chromium.googlesource.com/chromium/src/net/+/master/http/http_auth_handler_ntlm_portable.cc#550

It should not use the challenge flags from the server and should always take the true/if code path because Chrome always sends NTLM_NegotiateNTLM2Key during negotiate.

Did this work before? No 

Chrome version: 58.0.3029.110  Channel: stable
OS Version: 
Flash Version: 

I have a CL for a fix to send shortly. Additionally following up with firefox/mozilla since the code originally came from there.

Relevant parts of the latest NTLMSSP spec (Version 28.0).
https://msdn.microsoft.com/en-us/library/cc236621.aspx

2.2.2.5 (P Bit)

If both NTLMSSP_NEGOTIATE_EXTENDED_SESSIONSECURITY and NTLMSSP_NEGOTIATE_LM_KEY are requested, NTLMSSP_NEGOTIATE_EXTENDED_SESSIONSECURITY alone MUST be returned to the
client. NTLM v2 authentication session key generation MUST be supported by both the client and the DC in order to be used, and extended session security signing and sealing requires support from the client and the server in order to be used.

3.2.5.1.1 Server Side Pseudo Code

If (NTLMSSP_NEGOTIATE_EXTENDED_SESSIONSECURITY flag
        is set in NEGOTIATE.NegotiateFlags)
  Set the NTLMSSP_NEGOTIATE_EXTENDED_SESSIONSECURITY flag in
        CHALLENGE_MESSAGE.NegotiateFlags
ElseIf (NTLMSSP_NEGOTIATE_LM_KEY flag is set in 
        NEGOTIATE.NegotiateFlag)
  Set the NTLMSSP_NEGOTIATE_LM_KEY flag in
        CHALLENGE_MESSAGE.NegotiateFlags
EndIf

3.3.1 Client Side Pseudo Code

// Notice that it checks NegFlg not CHALLENGE_MESSAGE.NegotiateFlags
If (NTLMSSP_NEGOTIATE_EXTENDED_SESSIONSECURITY flag is set in NegFlg)
  // Details of NTLM2
Else
  // Details of vanilla NTLMv1
EndIf
 

Comment 1 by zentaro@google.com, May 24 2017

Note that in the Microsoft spec NTLMSSP_NEGOTIATE_EXTENDED_SESSIONSECURITY is the same as NTLM_NegotiateNTLM2Key in our code.

Comment 2 by sleevi@google.com, May 25 2017

Cc: asanka@chromium.org rsleevi@chromium.org
Components: Internals>Network>Auth
Labels: OS-Android OS-Mac
Project Member

Comment 4 by bugdroid1@chromium.org, May 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a6af46e2458df233a06576a074e198036822ce34

commit a6af46e2458df233a06576a074e198036822ce34
Author: zentaro <zentaro@chromium.org>
Date: Thu May 25 15:43:45 2017

Remove insecure code paths in portable NTLM.

- Removes some dead code
- Always use extended session security

BUG= chromium:726080 
TEST=unit tests and manual

Review-Url: https://codereview.chromium.org/2904043002
Cr-Commit-Position: refs/heads/master@{#474661}

[modify] https://crrev.com/a6af46e2458df233a06576a074e198036822ce34/net/http/http_auth_handler_ntlm_portable.cc

Comment 5 by kenrb@chromium.org, May 25 2017

Labels: Security_Severity-Low Security_Impact-Stable
Owner: zentaro@chromium.org
Status: Assigned (was: Unconfirmed)
Status: Fixed (was: Assigned)
Labels: M-60
Project Member

Comment 8 by sheriffbot@chromium.org, May 26 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: Release-0-M60
Project Member

Comment 10 by sheriffbot@chromium.org, Sep 1 2017

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment