Replace NTLMv1 implementation with a functionally equivalent one.

net/ntlm/ntlm_client.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/ntlm/ntlm_client.h"
+
+#include <string.h>
+
+#include "base/logging.h"
+#include "base/strings/utf_string_conversions.h"
+#include "net/ntlm/ntlm.h"
+#include "net/ntlm/ntlm_buffer_reader.h"
+#include "net/ntlm/ntlm_buffer_writer.h"
+
+namespace net {
+namespace ntlm {
+
+namespace {
+// Parses the challenge message and returns the |challenge_flags| and writes
+// the |server_challenge| into the supplied buffer.
+// |server_challenge| must contain at least 8 bytes.
+static bool ParseChallengeMessage(const uint8_t* challenge_message,

[asanka, 2017/07/14 16:52:39]

|static| is redundant within an anonymous namespace.

[zentaro, 2017/07/19 15:20:12]

Done.

+                                  size_t challenge_message_len,
+                                  NegotiateFlags* challenge_flags,
+                                  uint8_t* server_challenge) {
+  NtlmBufferReader challenge_reader(challenge_message, challenge_message_len);
+
+  return challenge_reader.MatchMessageHeader(MessageType::kChallenge) &&
+         challenge_reader.SkipSecurityBufferWithValidation() &&
+         challenge_reader.ReadFlags(challenge_flags) &&
+         challenge_reader.ReadBytes(server_challenge, kChallengeLen);
+}
+
+static bool WriteAuthenticateMessage(NtlmBufferWriter* authenticate_writer,
+                                     SecurityBuffer lm_payload,
+                                     SecurityBuffer ntlm_payload,
+                                     SecurityBuffer domain_payload,
+                                     SecurityBuffer username_payload,
+                                     SecurityBuffer hostname_payload,
+                                     NegotiateFlags authenticate_flags) {
+  return authenticate_writer->WriteMessageHeader(MessageType::kAuthenticate) &&
+         authenticate_writer->WriteSecurityBuffer(lm_payload) &&
+         authenticate_writer->WriteSecurityBuffer(ntlm_payload) &&
+         authenticate_writer->WriteSecurityBuffer(domain_payload) &&
+         authenticate_writer->WriteSecurityBuffer(username_payload) &&
+         authenticate_writer->WriteSecurityBuffer(hostname_payload) &&
+         authenticate_writer->WriteSecurityBuffer(
+             SecurityBuffer(kAuthenticateHeaderLenV1, 0)) &&
+         authenticate_writer->WriteFlags(authenticate_flags);
+}
+
+static bool WriteResponsePayloads(NtlmBufferWriter* authenticate_writer,
+                                  const uint8_t* lm_response,
+                                  size_t lm_response_len,
+                                  const uint8_t* ntlm_response,
+                                  size_t ntlm_response_len) {
+  return authenticate_writer->WriteBytes(lm_response, lm_response_len) &&
+         authenticate_writer->WriteBytes(ntlm_response, ntlm_response_len);
+}
+
+static bool WriteStringPayloads(NtlmBufferWriter* authenticate_writer,
+                                bool is_unicode,
+                                const base::string16& domain,
+                                const base::string16& username,
+                                const std::string hostname) {

[asanka, 2017/07/14 16:52:39]

Use a |const std::string&| here. Otherwise you are forcing a copy at the
callsite at GenerateAuthenticateMessage().

[zentaro, 2017/07/19 15:20:12]

Done.

+  if (is_unicode) {
+    return authenticate_writer->WriteUtf16String(domain) &&
+           authenticate_writer->WriteUtf16String(username) &&
+           authenticate_writer->WriteUtf8AsUtf16String(hostname);
+  } else {
+    return authenticate_writer->WriteUtf16AsUtf8String(domain) &&
+           authenticate_writer->WriteUtf16AsUtf8String(username) &&
+           authenticate_writer->WriteUtf8String(hostname);
+  }
+}
+
+// Returns the size in bytes of a string16 depending whether unicode
+// was negotiated.
+static size_t GetStringPayloadLength(const base::string16& str,
+                                     bool is_unicode) {
+  if (is_unicode)
+    return str.length() * 2;
+
+  // When |WriteUtf16AsUtf8String| is called with a |base::string16|, the string
+  // is converted to UTF8. Do the conversion to ensure that the character
+  // count is correct.
+  return base::UTF16ToUTF8(str).length();
+}
+
+// Returns the size in bytes of a std::string depending whether unicode
+// was negotiated.
+static size_t GetStringPayloadLength(const std::string& str, bool is_unicode) {
+  if (!is_unicode)
+    return str.length();
+
+  return base::UTF8ToUTF16(str).length() * 2;
+}
+
+}  // namespace
+
+NtlmClient::NtlmClient() : negotiate_flags_(kNegotiateMessageFlags) {

[asanka, 2017/07/14 16:52:39]

why store negotiate_flags_ here? It's essentially immutable.

[zentaro, 2017/07/19 15:20:12]

In the next cl for v2 - I allow it along with a features bitset (to toggle
MIC/EPA etc) to be optionally passed to the constructor if the caller wants to
specify different flags.

+  // Just generate the negotiate message once and hold on to it. It never
+  // changes and in a NTLMv2 it's used as an input
+  // to the Message Integrity Check in the Authenticate message.
+  GenerateNegotiateMessage();
+}
+
+NtlmClient::~NtlmClient() {}
+
+void NtlmClient::GetNegotiateMessage(uint8_t** negotiate_message,
+                                     size_t* negotiate_message_len) const {
+  *negotiate_message_len = kNegotiateMessageLen;
+  *negotiate_message = static_cast<uint8_t*>(malloc(kNegotiateMessageLen));
+  memcpy(*negotiate_message, negotiate_message_.get(), kNegotiateMessageLen);
+}
+
+void NtlmClient::GenerateNegotiateMessage() {
+  NtlmBufferWriter writer(kNegotiateMessageLen);
+  bool result =
+      writer.WriteMessageHeader(MessageType::kNegotiate) &&
+      writer.WriteFlags(negotiate_flags_) &&
+      writer.WriteSecurityBuffer(SecurityBuffer(kNegotiateMessageLen, 0)) &&
+      writer.WriteSecurityBuffer(SecurityBuffer(kNegotiateMessageLen, 0)) &&
+      writer.IsEndOfBuffer();
+
+  DCHECK(result);
+
+  negotiate_message_.reset(new uint8_t[writer.GetLength()]);

[asanka, 2017/07/14 16:52:39]

Why create a new copy of the writer's buffer?

[zentaro, 2017/07/19 15:20:12]

N/A - It's a string now.

+  memcpy(negotiate_message_.get(), writer.GetBuffer().data(),
+         writer.GetLength());
+}
+
+bool NtlmClient::GenerateAuthenticateMessage(
+    const base::string16& domain,
+    const base::string16& username,
+    const base::string16& password,
+    const std::string& hostname,
+    const uint8_t* client_challenge,
+    const uint8_t* challenge_message,
+    size_t challenge_message_len,
+    uint8_t** authenticate_message,
+    size_t* authenticate_message_len) const {
+  *authenticate_message = nullptr;
+  *authenticate_message_len = 0;
+  NegotiateFlags challenge_flags;
+  uint8_t server_challenge[kChallengeLen];
+
+  if (!ParseChallengeMessage(challenge_message, challenge_message_len,
+                             &challenge_flags, server_challenge)) {
+    return false;
+  }
+
+  // Calculate the responses for the authenticate message.
+  uint8_t lm_response[kResponseLenV1];
+  uint8_t ntlm_response[kResponseLenV1];
+
+  // Always use extended session security even if the server tries to downgrade.
+  NegotiateFlags authenticate_flags = (challenge_flags & negotiate_flags_) |
+                                      NegotiateFlags::kExtendedSessionSecurity;
+
+  // Generate the LM and NTLM responses.
+  GenerateResponsesV1WithSessionSecurity(
+      password, server_challenge, client_challenge, lm_response, ntlm_response);
+
+  // Calculate all the payload lengths and offsets.
+  bool is_unicode =
+      static_cast<bool>(authenticate_flags & NegotiateFlags::kUnicode);

[asanka, 2017/07/14 16:52:39]

bool is_unicode = (authenticate_flags & NegotiateFlags::kUnicode) ==
NegotiateFlags::kUnicode

[zentaro, 2017/07/19 15:20:12]

Done.

+
+  SecurityBuffer lm_info;
+  SecurityBuffer ntlm_info;
+  SecurityBuffer domain_info;
+  SecurityBuffer username_info;
+  SecurityBuffer hostname_info;
+  CalculatePayloadLayout(is_unicode, domain, username, hostname, &lm_info,
+                         &ntlm_info, &domain_info, &username_info,
+                         &hostname_info, authenticate_message_len);
+
+  // Write the authenticate message header.
+  NtlmBufferWriter authenticate_writer(*authenticate_message_len);
+  bool writer_result = WriteAuthenticateMessage(
+      &authenticate_writer, lm_info, ntlm_info, domain_info, username_info,
+      hostname_info, authenticate_flags);
+  DCHECK(writer_result);
+  DCHECK(authenticate_writer.GetCursor() == GetAuthenticateHeaderLength());

[asanka, 2017/07/14 16:52:39]

DCHECK_EQ here and elsewhere.

[zentaro, 2017/07/19 15:20:12]

Done.

+
+  // Write the response payloads.

[asanka, 2017/07/14 16:52:39]

many of these comments seem unnecessary since the method names are just as
descriptive.

[zentaro, 2017/07/19 15:20:12]

Deleted.

+  writer_result =
+      WriteResponsePayloads(&authenticate_writer, lm_response, lm_info.length,
+                            ntlm_response, ntlm_info.length);
+  DCHECK(writer_result);
+  DCHECK(authenticate_writer.GetCursor() == domain_info.offset);
+
+  // Write the string field payloads.
+  writer_result = WriteStringPayloads(&authenticate_writer, is_unicode, domain,
+                                      username, hostname);
+  DCHECK(writer_result);
+  DCHECK(authenticate_writer.IsEndOfBuffer());
+  DCHECK(*authenticate_message_len == authenticate_writer.GetLength());
+
+  *authenticate_message =
+      static_cast<uint8_t*>(malloc(authenticate_writer.GetLength()));

[asanka, 2017/07/14 16:52:39]

let's avoid unnecessary buffer copies.

[zentaro, 2017/07/19 15:20:12]

N/A string now.

+  memcpy(*authenticate_message, authenticate_writer.GetBuffer().data(),
+         authenticate_writer.GetLength());
+  return true;
+}
+
+void NtlmClient::CalculatePayloadLayout(
+    bool is_unicode,
+    const base::string16& domain,
+    const base::string16& username,
+    const std::string& hostname,
+    SecurityBuffer* lm_info,
+    SecurityBuffer* ntlm_info,
+    SecurityBuffer* domain_info,
+    SecurityBuffer* username_info,
+    SecurityBuffer* hostname_info,
+    size_t* authenticate_message_len) const {
+  size_t upto = GetAuthenticateHeaderLength();
+
+  lm_info->offset = upto;
+  lm_info->length = kResponseLenV1;
+  upto += lm_info->length;
+
+  ntlm_info->offset = upto;
+  ntlm_info->length = GetNtlmResponseLength();
+  upto += ntlm_info->length;
+
+  domain_info->offset = upto;
+  domain_info->length = GetStringPayloadLength(domain, is_unicode);
+  upto += domain_info->length;
+
+  username_info->offset = upto;
+  username_info->length = GetStringPayloadLength(username, is_unicode);
+  upto += username_info->length;
+
+  hostname_info->offset = upto;
+  hostname_info->length = GetStringPayloadLength(hostname, is_unicode);
+  upto += hostname_info->length;
+
+  *authenticate_message_len = upto;
+}
+
+size_t NtlmClient::GetAuthenticateHeaderLength() const {
+  return kAuthenticateHeaderLenV1;
+}
+
+size_t NtlmClient::GetNtlmResponseLength() const {
+  return kResponseLenV1;
+}
+
+}  // namespace ntlm
+}  // namespace net