Replace NTLMv1 implementation with a functionally equivalent one.

net/ntlm/ntlm_client_unittest.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>
+
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "build/build_config.h"
+#include "net/ntlm/ntlm.h"
+#include "net/ntlm/ntlm_buffer_reader.h"
+#include "net/ntlm/ntlm_buffer_writer.h"
+#include "net/ntlm/ntlm_test_data.h"
+#include "testing/platform_test.h"
+
+namespace net {
+namespace ntlm {
+
+namespace {
+
+void GetNegotiateMessage(const NtlmClient& client,
+                         std::unique_ptr<uint8_t[]>* negotiate_msg,
+                         size_t* negotiate_msg_len) {
+  uint8_t* negotiate_msg_ptr = nullptr;
+  client.GetNegotiateMessage(&negotiate_msg_ptr, negotiate_msg_len);
+  negotiate_msg->reset(negotiate_msg_ptr);

[asanka, 2017/07/12 20:38:37]

as mentioned elsewhere, this assumes that negotiate_msg_ptr was allocated via
new[]. Just leaving this note here so we don't forget to check this file when we
address it.

[zentaro, 2017/07/13 20:27:18]

Specialized the unique_ptr to call free.

+}
+
+bool GenerateAuthMsg(const NtlmClient& client,
+                     const uint8_t* challenge_msg,
+                     size_t challenge_msg_len,
+                     std::unique_ptr<uint8_t[]>* authenticate_msg,
+                     size_t* authenticate_msg_len) {
+  uint8_t* authenticate_msg_ptr = nullptr;
+  bool result = client.GenerateAuthenticateMessage(
+      NTLM_DOMAIN, NTLM_USER, NTLM_PASSWORD, NTLM_HOSTNAME_ASCII,
+      CLIENT_CHALLENGE, challenge_msg, challenge_msg_len, &authenticate_msg_ptr,
+      authenticate_msg_len);
+  if (result)
+    authenticate_msg->reset(authenticate_msg_ptr);
+
+  return result;
+}
+
+bool GenerateAuthMsg(const NtlmClient& client,
+                     const NtlmBufferWriter& challenge_writer,
+                     std::unique_ptr<uint8_t[]>* authenticate_msg,
+                     size_t* authenticate_msg_len) {
+  base::StringPiece piece(challenge_writer.GetBuffer());
+
+  return GenerateAuthMsg(client, reinterpret_cast<const uint8_t*>(piece.data()),
+                         piece.length(), authenticate_msg,
+                         authenticate_msg_len);
+}
+
+bool GetAuthMsgResult(const NtlmClient& client,
+                      const NtlmBufferWriter& challenge_writer) {
+  std::unique_ptr<uint8_t[]> authenticate_msg;
+  size_t authenticate_msg_len;
+  return GenerateAuthMsg(client, challenge_writer, &authenticate_msg,
+                         &authenticate_msg_len);
+}
+
+bool ReadBytesPayload(NtlmBufferReader* reader, uint8_t* buffer, size_t len) {
+  SecurityBuffer sec_buf;
+  return reader->ReadSecurityBuffer(&sec_buf) && (sec_buf.length == len) &&
+         reader->ReadBytesFrom(sec_buf, buffer);
+}
+
+// Reads bytes from a payload and assigns them to a string. This makes
+// no assumptions about the underlying encoding.
+bool ReadStringPayload(NtlmBufferReader* reader, std::string* str) {
+  SecurityBuffer sec_buf;
+  if (!reader->ReadSecurityBuffer(&sec_buf))
+    return false;
+
+  uint8_t raw[sec_buf.length];
+  if (!reader->ReadBytesFrom(sec_buf, raw))
+    return false;
+
+  str->assign(reinterpret_cast<const char*>(raw), sec_buf.length);
+  return true;
+}
+
+// Reads bytes from a payload and assigns them to a string16. This makes
+// no assumptions about the underlying encoding. This will fail if there
+// are an odd number of bytes in the payload.
+bool ReadString16Payload(NtlmBufferReader* reader, base::string16* str) {
+  SecurityBuffer sec_buf;
+  if (!reader->ReadSecurityBuffer(&sec_buf) || (sec_buf.length % 2 != 0))
+    return false;
+
+  uint8_t raw[sec_buf.length];
+  if (!reader->ReadBytesFrom(sec_buf, raw))
+    return false;
+
+#if defined(ARCH_CPU_BIG_ENDIAN)
+  for (size_t i = 0; i < sec_buf.length; i += 2) {
+    std::swap(raw[i], raw[i + 1]);
+  }
+#endif
+
+  str->assign(reinterpret_cast<const base::char16*>(raw), sec_buf.length / 2);
+  return true;
+}
+
+}  // namespace
+
+TEST(NtlmClientTest, VerifyNegotiateMessageV1) {
+  NtlmClient client;
+
+  std::unique_ptr<uint8_t[]> negotiate_msg;
+  size_t negotiate_msg_len;
+  GetNegotiateMessage(client, &negotiate_msg, &negotiate_msg_len);
+
+  ASSERT_EQ(NEGOTIATE_MESSAGE_LEN, negotiate_msg_len);
+  ASSERT_EQ(0, memcmp(EXPECTED_NEGOTIATE_MSG, negotiate_msg.get(),
+                      NEGOTIATE_MESSAGE_LEN));
+}
+
+TEST(NtlmClientTest, MinimalStructurallyValidChallenge) {
+  NtlmClient client;
+
+  NtlmBufferWriter writer(MIN_CHALLENGE_HEADER_LEN);
+  ASSERT_TRUE(
+      writer.WriteBytes(MIN_CHALLENGE_MESSAGE, MIN_CHALLENGE_HEADER_LEN));
+
+  ASSERT_TRUE(GetAuthMsgResult(client, writer));
+}
+
+TEST(NtlmClientTest, MinimalStructurallyValidChallengeZeroOffset) {
+  NtlmClient client;
+
+  // The spec (2.2.1.2) states that the length SHOULD be 0 and the offset
+  // SHOULD be where the payload would be if it was present. This is the
+  // expected response from a compliant server when no target name is sent.
+  // In reality the offset should always be ignored if the length is zero.
+  // Also implementations often just write zeros.
+  uint8_t raw[MIN_CHALLENGE_HEADER_LEN];
+  memcpy(raw, MIN_CHALLENGE_MESSAGE, MIN_CHALLENGE_HEADER_LEN);
+  // Modify the default valid message to overwrite the offset to zero.
+  raw[16] = 0x00;
+
+  NtlmBufferWriter writer(MIN_CHALLENGE_HEADER_LEN);
+  ASSERT_TRUE(writer.WriteBytes(raw, arraysize(raw)));
+
+  ASSERT_TRUE(GetAuthMsgResult(client, writer));
+}
+
+TEST(NtlmClientTest, ChallengeMsgTooShort) {
+  NtlmClient client;
+
+  // Fail because the minimum size valid message is 32 bytes.
+  NtlmBufferWriter writer(MIN_CHALLENGE_HEADER_LEN - 1);
+  ASSERT_TRUE(
+      writer.WriteBytes(MIN_CHALLENGE_MESSAGE, MIN_CHALLENGE_HEADER_LEN - 1));
+  ASSERT_FALSE(GetAuthMsgResult(client, writer));
+}
+
+TEST(NtlmClientTest, ChallengeMsgNoSig) {
+  NtlmClient client;
+
+  // Fail because the first 8 bytes don't match "NTLMSSP\0"
+  uint8_t raw[MIN_CHALLENGE_HEADER_LEN];
+  memcpy(raw, MIN_CHALLENGE_MESSAGE, MIN_CHALLENGE_HEADER_LEN);
+  // Modify the default valid message to overwrite the last byte of the
+  // signature.
+  raw[7] = 0xff;
+  NtlmBufferWriter writer(MIN_CHALLENGE_HEADER_LEN);
+  ASSERT_TRUE(writer.WriteBytes(raw, arraysize(raw)));
+  ASSERT_FALSE(GetAuthMsgResult(client, writer));
+}
+
+TEST(NtlmClientTest, ChallengeMsgWrongMessageType) {
+  NtlmClient client;
+
+  // Fail because the message type should be MessageType::CHALLENGE (0x00000002)
+  uint8_t raw[MIN_CHALLENGE_HEADER_LEN];
+  memcpy(raw, MIN_CHALLENGE_MESSAGE, MIN_CHALLENGE_HEADER_LEN);
+  // Modify the message type.
+  raw[8] = 0x03;
+
+  NtlmBufferWriter writer(MIN_CHALLENGE_HEADER_LEN);
+  ASSERT_TRUE(writer.WriteBytes(raw, arraysize(raw)));
+
+  ASSERT_FALSE(GetAuthMsgResult(client, writer));
+}
+
+TEST(NtlmClientTest, ChallengeWithNoTargetName) {
+  NtlmClient client;
+
+  // The spec (2.2.1.2) states that the length SHOULD be 0 and the offset
+  // SHOULD be where the payload would be if it was present. This is the
+  // expected response from a compliant server when no target name is sent.
+  // In reality the offset should always be ignored if the length is zero.
+  // Also implementations often just write zeros.
+  uint8_t raw[MIN_CHALLENGE_HEADER_LEN];
+  memcpy(raw, MIN_CHALLENGE_MESSAGE, MIN_CHALLENGE_HEADER_LEN);
+  // Modify the default valid message to overwrite the offset to zero.
+  raw[16] = 0x00;
+
+  NtlmBufferWriter writer(MIN_CHALLENGE_HEADER_LEN);
+  ASSERT_TRUE(writer.WriteBytes(raw, arraysize(raw)));
+
+  ASSERT_TRUE(GetAuthMsgResult(client, writer));
+}
+
+TEST(NtlmClientTest, Type2MessageWithTargetName) {
+  NtlmClient client;
+
+  // One extra byte is provided for target name.
+  uint8_t raw[MIN_CHALLENGE_HEADER_LEN + 1];
+  memcpy(raw, MIN_CHALLENGE_MESSAGE, MIN_CHALLENGE_HEADER_LEN);
+  // Modify the default valid message to indicate 1 byte is present in the
+  // target name payload.
+  raw[12] = 0x01;
+  raw[14] = 0x01;
+  // Put something in the target name.
+  raw[32] = 'Z';
+
+  NtlmBufferWriter writer(CHALLENGE_HEADER_LEN + 1);
+  ASSERT_TRUE(writer.WriteBytes(raw, arraysize(raw)));
+
+  ASSERT_TRUE(GetAuthMsgResult(client, writer));
+}
+
+TEST(NtlmClientTest, NoTargetNameOverflowFromOffset) {
+  NtlmClient client;
+
+  uint8_t raw[MIN_CHALLENGE_HEADER_LEN];
+  memcpy(raw, MIN_CHALLENGE_MESSAGE, MIN_CHALLENGE_HEADER_LEN);
+  // Modify the default valid message to claim that the target name field is 1
+  // byte long overrunning the end of the message message.
+  raw[12] = 0x01;
+  raw[14] = 0x01;
+
+  NtlmBufferWriter writer(MIN_CHALLENGE_HEADER_LEN);
+  ASSERT_TRUE(writer.WriteBytes(raw, arraysize(raw)));
+
+  // The above malformed message could cause an implementation to read outside
+  // the message buffer because the offset is past the end of the message.
+  // Verify it gets rejected.
+  ASSERT_FALSE(GetAuthMsgResult(client, writer));
+}
+
+TEST(NtlmClientTest, NoTargetNameOverflowFromLength) {
+  NtlmClient client;
+
+  // Message has 1 extra byte of space after the header for the target name.
+  // One extra byte is provided for target name.
+  uint8_t raw[MIN_CHALLENGE_HEADER_LEN + 1];
+  memcpy(raw, MIN_CHALLENGE_MESSAGE, MIN_CHALLENGE_HEADER_LEN);
+  // Modify the default valid message to indicate 2 bytes are present in the
+  // target name payload (however there is only space for 1).
+  raw[12] = 0x02;
+  raw[14] = 0x02;
+  // Put something in the target name.
+  raw[32] = 'Z';
+
+  NtlmBufferWriter writer(MIN_CHALLENGE_HEADER_LEN + 1);
+  ASSERT_TRUE(writer.WriteBytes(raw, arraysize(raw)));
+
+  // The above malformed message could cause an implementation
+  // to read outside the message buffer because the length is
+  // longer than available space. Verify it gets rejected.
+  ASSERT_FALSE(GetAuthMsgResult(client, writer));
+}
+
+TEST(NtlmClientTest, Type3UnicodeWithSessionSecuritySpecTest) {
+  NtlmClient client;
+
+  std::unique_ptr<uint8_t[]> auth_msg;
+  size_t auth_msg_len;
+  ASSERT_TRUE(GenerateAuthMsg(client, V1_CHALLENGE_MSG,
+                              arraysize(V1_CHALLENGE_MSG), &auth_msg,
+                              &auth_msg_len));
+
+  ASSERT_EQ(arraysize(EXPECTED_V1_AUTHENTICATE_MSG), auth_msg_len);
+  ASSERT_EQ(0,
+            memcmp(EXPECTED_V1_AUTHENTICATE_MSG, auth_msg.get(), auth_msg_len));
+}
+
+TEST(NtlmClientTest, Type3WithoutUnicode) {
+  NtlmClient client;
+
+  std::unique_ptr<uint8_t[]> auth_msg;
+  size_t auth_msg_len;
+  ASSERT_TRUE(GenerateAuthMsg(client, MIN_CHALLENGE_MESSAGE_NO_UNICODE,
+                              MIN_CHALLENGE_HEADER_LEN, &auth_msg,
+                              &auth_msg_len));
+
+  NtlmBufferReader reader(auth_msg.get(), auth_msg_len);
+  ASSERT_TRUE(reader.MatchMessageHeader(MessageType::AUTHENTICATE));
+
+  // Read the LM and NTLM Response Payloads.
+  uint8_t actual_lm_response[RESPONSE_V1_LEN];
+  uint8_t actual_ntlm_response[RESPONSE_V1_LEN];
+
+  ASSERT_TRUE(ReadBytesPayload(&reader, actual_lm_response, RESPONSE_V1_LEN));
+  ASSERT_TRUE(ReadBytesPayload(&reader, actual_ntlm_response, RESPONSE_V1_LEN));
+
+  ASSERT_EQ(0, memcmp(EXPECTED_V1_WITH_SS_LM_RESPONSE, actual_lm_response,
+                      RESPONSE_V1_LEN));
+  ASSERT_EQ(0, memcmp(EXPECTED_V1_WITH_SS_NTLM_RESPONSE, actual_ntlm_response,
+                      RESPONSE_V1_LEN));
+
+  std::string domain;
+  std::string username;
+  std::string hostname;
+  ASSERT_TRUE(ReadStringPayload(&reader, &domain));
+  ASSERT_EQ(NTLM_DOMAIN_ASCII, domain);
+  ASSERT_TRUE(ReadStringPayload(&reader, &username));
+  ASSERT_EQ(NTLM_USER_ASCII, username);
+  ASSERT_TRUE(ReadStringPayload(&reader, &hostname));
+  ASSERT_EQ(NTLM_HOSTNAME_ASCII, hostname);
+
+  // The session key is not used in HTTP. Since NTLMSSP_NEGOTIATE_KEY_EXCH
+  // was not sent this is empty.
+  ASSERT_TRUE(reader.MatchEmptySecurityBuffer());
+
+  // Verify the unicode flag is not set and OEM flag is.
+  NegotiateFlags flags;
+  ASSERT_TRUE(reader.ReadFlags(&flags));
+  ASSERT_EQ(NegotiateFlags::NONE, flags & NegotiateFlags::UNICODE);
+  ASSERT_EQ(NegotiateFlags::OEM, flags & NegotiateFlags::OEM);
+}
+
+TEST(NtlmClientTest, ClientDoesNotDowngradeSessionSecurity) {
+  NtlmClient client;
+
+  std::unique_ptr<uint8_t[]> auth_msg;
+  size_t auth_msg_len;
+  ASSERT_TRUE(GenerateAuthMsg(client, MIN_CHALLENGE_MESSAGE_NO_SS,
+                              MIN_CHALLENGE_HEADER_LEN, &auth_msg,
+                              &auth_msg_len));
+
+  NtlmBufferReader reader(auth_msg.get(), auth_msg_len);
+  ASSERT_TRUE(reader.MatchMessageHeader(MessageType::AUTHENTICATE));
+
+  // Read the LM and NTLM Response Payloads.
+  uint8_t actual_lm_response[RESPONSE_V1_LEN];
+  uint8_t actual_ntlm_response[RESPONSE_V1_LEN];
+
+  ASSERT_TRUE(ReadBytesPayload(&reader, actual_lm_response, RESPONSE_V1_LEN));
+  ASSERT_TRUE(ReadBytesPayload(&reader, actual_ntlm_response, RESPONSE_V1_LEN));
+
+  // The important part of this test is that even though the
+  // server told the client to drop session security. The client
+  // DID NOT drop it.
+  ASSERT_EQ(0, memcmp(EXPECTED_V1_WITH_SS_LM_RESPONSE, actual_lm_response,
+                      RESPONSE_V1_LEN));
+  ASSERT_EQ(0, memcmp(EXPECTED_V1_WITH_SS_NTLM_RESPONSE, actual_ntlm_response,
+                      RESPONSE_V1_LEN));
+
+  base::string16 domain;
+  base::string16 username;
+  base::string16 hostname;
+  ASSERT_TRUE(ReadString16Payload(&reader, &domain));
+  ASSERT_EQ(NTLM_DOMAIN, domain);
+  ASSERT_TRUE(ReadString16Payload(&reader, &username));
+  ASSERT_EQ(NTLM_USER, username);
+  ASSERT_TRUE(ReadString16Payload(&reader, &hostname));
+  ASSERT_EQ(NTLM_HOSTNAME, hostname);
+
+  // The session key is not used in HTTP. Since NTLMSSP_NEGOTIATE_KEY_EXCH
+  // was not sent this is empty.
+  ASSERT_TRUE(reader.MatchEmptySecurityBuffer());
+
+  // Verify the unicode and session security flag is set.
+  NegotiateFlags flags;
+  ASSERT_TRUE(reader.ReadFlags(&flags));
+  ASSERT_EQ(NegotiateFlags::UNICODE, flags & NegotiateFlags::UNICODE);
+  ASSERT_EQ(NegotiateFlags::EXTENDED_SESSIONSECURITY,
+            flags & NegotiateFlags::EXTENDED_SESSIONSECURITY);
+}
+
+}  // namespace ntlm
+}  // namespace net