Add unit tests for NTLMv1 portable implementation

net/http/http_auth_handler_ntlm_portable_unittest.cc

+// Copyright (c) 2017 The Chromium Authors. All rights reserved.

[asanka, 2017/06/23 21:29:10]

No "(c)"

[zentaro, 2017/07/05 17:57:42]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/http/http_auth_handler_ntlm.h"

[asanka, 2017/06/23 21:29:10]

The primary include file should http_auth_handler_ntlm_portable.h.

If the tests depend on http_auth_handler_ntlm.h, then it should be included in
the general include section below.

[zentaro, 2017/07/05 17:57:41]

There is no http_auth_handler_ntlm_portable.h so I moved this below.

+
+#include <string>
+
+#include "base/base64.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "net/base/test_completion_callback.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/http/http_auth_challenge_tokenizer.h"
+#include "net/http/http_request_info.h"
+#include "net/http/mock_allow_http_auth_preferences.h"
+#include "net/http/ntlm.h"
+#include "net/http/ntlm_buffer_reader.h"
+#include "net/http/ntlm_buffer_writer.h"
+#include "net/http/ntlm_client.h"
+#include "net/log/net_log_with_source.h"
+#include "net/ssl/ssl_info.h"
+#include "net/test/gtest_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+namespace net {
+
+#if defined(NTLM_PORTABLE)

[asanka, 2017/06/23 21:29:10]

This condition is not necessary and not correct.

The *_ntlm_portable_* stuff should only be getting built on platforms which use
it. It shouldn't be enforced here. 

Also, the NTLM_PORTABLE macro is used by HttpAuthHandlerNtlm to conditionalize
its implementation and shouldn't really be used outside of that. (I know it is,
but it shouldn't.)

[zentaro, 2017/07/05 17:57:42]

Done.

+
+class HttpAuthHandlerNtlmPortableTest : public PlatformTest {
+ public:
+  HttpAuthHandlerNtlmPortableTest()
+      : domain_ascii_("THEDOMAIN"),
+        user_ascii_("someuser"),
+        password_ascii_("password") {
+    http_auth_preferences_.reset(new MockAllowHttpAuthPreferences());
+    factory_.reset(new HttpAuthHandlerNTLM::Factory());
+    factory_->set_http_auth_preferences(http_auth_preferences_.get());
+    creds_ =
+        AuthCredentials(base::ASCIIToUTF16(domain_ascii_ + "\\" + user_ascii_),
+                        base::ASCIIToUTF16(password_ascii_));
+  }
+
+  int CreateHandler() {
+    GURL gurl("https://foo.com");
+    SSLInfo null_ssl_info;
+
+    return factory_->CreateAuthHandlerFromString(
+        "NTLM", HttpAuth::AUTH_SERVER, null_ssl_info, gurl, NetLogWithSource(),
+        &auth_handler_);
+  }
+
+  std::string CreateType2Token(base::StringPiece message) {

[asanka, 2017/06/23 21:29:10]

Nit: doesn't really create a type2 token. Probably call it something more
generic, like 'GenerateNtlmAuthHeader()' or something.

[zentaro, 2017/07/05 17:57:41]

Done.

+    std::string output;
+    base::Base64Encode(message, &output);
+
+    return "NTLM " + output;
+  }
+
+  void HandleAnotherChallenge(const std::string& challenge,

[asanka, 2017/06/23 21:29:10]

It's easier to read, even though a bit verbose, if the function returned an
HttpAuth::AuthorizationResult.

I.e. the call site would look like:

   EXPECT_EQ(FOO, HandleAnotherChallenge(challenge));

That way it's clear what the input is and what the expected result is at the
call site. Also, it's much more convenient for the call site to switch to an
ASSERT_* if it doesn't make sense to proceed if this operation fails.

[zentaro, 2017/07/05 17:57:42]

Done.

+                              HttpAuth::AuthorizationResult expected_result) {
+    HttpAuthChallengeTokenizer tokenizer(challenge.begin(), challenge.end());
+    EXPECT_EQ(expected_result,
+              GetAuthHandler()->HandleAnotherChallenge(&tokenizer));
+  }
+
+  void HandleAnotherChallenge(const std::string& challenge) {
+    HandleAnotherChallenge(challenge, HttpAuth::AUTHORIZATION_RESULT_ACCEPT);
+  }
+
+  bool DecodeChallenge(const std::string& challenge, std::string* decoded) {
+    HttpAuthChallengeTokenizer tokenizer(challenge.begin(), challenge.end());
+    return base::Base64Decode(tokenizer.base64_param(), decoded);
+  }
+
+  int GenerateAuthToken(std::string* token) {
+    TestCompletionCallback callback;
+    HttpRequestInfo request_info;
+    return callback.GetResult(GetAuthHandler()->GenerateAuthToken(
+        GetCreds(), &request_info, callback.callback(), token));
+  }
+
+  void ReadBytesPayload(ntlm::NtlmBufferReader* reader,
+                        uint8_t* buffer,
+                        size_t len) {
+    // First read the security buffer.
+    ntlm::SecurityBuffer sec_buf;
+    EXPECT_TRUE(reader->ReadSecurityBuffer(&sec_buf));
+    EXPECT_EQ(sec_buf.length, len);

[asanka, 2017/06/23 21:29:10]

Should return early if this winds up not being the case. Also, return a bool or
something that the calling function can ASSERT_* on since it's likely that the
test can't proceed past this failure. Here and elsewhere.

[zentaro, 2017/07/05 17:57:41]

When I wrote most of the tests I didn't realize that EXPECT_* doesn't stop
execution. I'll just change everything to assert instead. It's not helping
anyone to continue once the test fails or to try and think about what happens if
you do or don't continue on every condition.

+    EXPECT_TRUE(reader->ReadBytesFrom(sec_buf, buffer));
+  }
+
+  // Reads bytes from a payload and assigns them to a string. This makes
+  // no assumptions about the underlying encoding.
+  void ReadStringPayload(ntlm::NtlmBufferReader* reader, std::string* str) {
+    ntlm::SecurityBuffer sec_buf;
+    EXPECT_TRUE(reader->ReadSecurityBuffer(&sec_buf));
+
+    uint8_t raw[sec_buf.length];
+    EXPECT_TRUE(reader->ReadBytesFrom(sec_buf, raw));
+
+    str->assign(reinterpret_cast<const char*>(raw), sec_buf.length);
+  }
+
+  // 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.
+  void ReadString16Payload(ntlm::NtlmBufferReader* reader,
+                           base::string16* str) {
+    ntlm::SecurityBuffer sec_buf;
+    EXPECT_TRUE(reader->ReadSecurityBuffer(&sec_buf));
+    EXPECT_EQ(0, sec_buf.length % 2);
+
+    uint8_t raw[sec_buf.length];
+    EXPECT_TRUE(reader->ReadBytesFrom(sec_buf, raw));
+
+#if IS_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);
+  }
+
+  int GetGenerateAuthTokenResult() {
+    std::string token;
+    return GenerateAuthToken(&token);
+  }
+
+  AuthCredentials* GetCreds() { return &creds_; }
+
+  HttpAuthHandlerNTLM* GetAuthHandler() {
+    return static_cast<HttpAuthHandlerNTLM*>(auth_handler_.get());
+  }
+
+  static void MockRandom(uint8_t* output, size_t n) {

[asanka, 2017/06/23 21:29:10]

Why not memset(output, 4, n) ? https://xkcd.com/221/

[zentaro, 2017/07/05 17:57:41]

Works for me. I copy/pasted it from
net/http/http_network_transaction_unittest.cc

+    static const uint8_t bytes[] = {0x55, 0x29, 0x66, 0x26,
+                                    0x6b, 0x9c, 0x73, 0x54};
+    static size_t current_byte = 0;
+    for (size_t i = 0; i < n; ++i) {
+      output[i] = bytes[current_byte++];
+      current_byte %= arraysize(bytes);
+    }
+  }
+
+  static std::string MockGetHostName() { return "MYHOSTNAME"; }
+
+ protected:
+  const std::string domain_ascii_;
+  const std::string user_ascii_;
+  const std::string password_ascii_;
+
+ private:
+  AuthCredentials creds_;
+  std::unique_ptr<HttpAuthHandler> auth_handler_;
+  std::unique_ptr<MockAllowHttpAuthPreferences> http_auth_preferences_;
+  std::unique_ptr<HttpAuthHandlerNTLM::Factory> factory_;
+};
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, SimpleConstruction) {
+  EXPECT_EQ(OK, CreateHandler());
+  ASSERT_TRUE(GetAuthHandler() != nullptr);
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, DoNotAllowDefaultCreds) {
+  EXPECT_EQ(OK, CreateHandler());
+  EXPECT_FALSE(GetAuthHandler()->AllowsDefaultCredentials());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, AllowsExplicitCredentials) {
+  EXPECT_EQ(OK, CreateHandler());
+  EXPECT_TRUE(GetAuthHandler()->AllowsExplicitCredentials());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, VerifyType1Message) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  std::string token;
+  EXPECT_EQ(OK, GenerateAuthToken(&token));
+  // The type 1 message generated is always the same. The only variable
+  // part of the message is the flags and Chrome always offers the same
+  // set of flags.
+  EXPECT_EQ("NTLM TlRMTVNTUAABAAAAB4IIAAAAAAAAAAAAAAAAAAAAAAA=", token);
+

[asanka, 2017/06/23 21:29:10]

The remainder of the test is basically testing the NtlmBufferReader
implementation. :-/ The above comparison against a static string is sufficient
to verify the Type1 token generation.

[zentaro, 2017/07/05 17:57:41]

Done.

+  // Poke into the message to verify the fields inside are expected.
+  std::string decoded;
+  EXPECT_TRUE(DecodeChallenge(token, &decoded));
+
+  ntlm::NtlmBufferReader reader(decoded);
+  EXPECT_TRUE(reader.MatchMessageHeader(ntlm::MessageType::NEGOTIATE));
+  ntlm::NegotiateFlags flags;
+  EXPECT_TRUE(reader.ReadFlags(&flags));
+  EXPECT_EQ(ntlm::NEGOTIATE_MESSAGE_FLAGS, flags);
+  EXPECT_TRUE(reader.MatchEmptySecurityBuffer());
+  EXPECT_TRUE(reader.MatchEmptySecurityBuffer());
+  EXPECT_TRUE(reader.IsEndOfBuffer());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, EmptyTokenFails) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  // The encoded token for a type 2 message can't be empty.
+  HandleAnotherChallenge("NTLM", HttpAuth::AUTHORIZATION_RESULT_REJECT);
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, InvalidBase64Encoding) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  // Token isn't valid base64.
+  HandleAnotherChallenge("NTLM !!!!!!!!!!!!!");
+  EXPECT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, CantChangeSchemeMidway) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  // Can't switch to a different auth scheme in the middle of the process.
+  HandleAnotherChallenge(
+      "Negotiate "
+      "TlRMTVNTUAACAAAADAAMADgAAAAFgokCXziKeNIPIDYAAAAAAAAAAIYAhgBEAAAABgOAJQAA"

[asanka, 2017/06/23 21:29:10]

The token is irrelevant. Let's go with something shorter.

[zentaro, 2017/07/05 17:57:41]

Done.

+      "AA9aAEUATgBEAE8ATQACAAwAWgBFAE4ARABPAE0AAQAMAFoARQBOAEQAQwAxAAQAFAB6AGUA"
+      "bgBkAG8AbQAuAGwAbwBjAAMAIgBaAGUAbgBEAEMAMQAuAHoAZQBuAGQAbwBtAC4AbABvAGMA"
+      "BQAUAHoAZQBuAGQAbwBtAC4AbABvAGMABwAIAN6N+IxGwNIBAAAAAA==",
+      HttpAuth::AUTHORIZATION_RESULT_INVALID);
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, Type2MessageTooShort) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  // Fail because the minimum size valid message is 32 bytes.
+  char raw[31];
+  HandleAnotherChallenge(CreateType2Token(base::StringPiece(raw, sizeof(raw))));

[asanka, 2017/06/23 21:29:10]

The memory bots may complain about this because the generated token is likely
based on uninitialized memory. ("branch conditionalized on uninitialized value"
or somesuch).

Let's take a valid Type2 message and truncate it for this test.

[zentaro, 2017/07/05 17:57:41]

Done.

+  EXPECT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, Type2MessageNoSig) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  // Fail because the first bytes don't match "NTLMSSP\0"
+  char raw[32];

[asanka, 2017/06/23 21:29:11]

Same as before. Use a known initialized buffer for tests.

Also, the failure could be due to anything. Let's start with a minimal valid
message, and test by changing individual fields thereof. That should increase
our confidence that the ERR_UNEXPECTED is due to the invalid signature.

[zentaro, 2017/07/05 17:57:41]

Done.

+  memset(raw, 0, ntlm::SIGNATURE_LEN);
+  HandleAnotherChallenge(CreateType2Token(base::StringPiece(raw, sizeof(raw))));
+  EXPECT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, Type2WrongMessageType) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  // Fail because the message type should be MessageType::CHALLENGE (0x00000002)
+  ntlm::NtlmBufferWriter writer(32);
+  EXPECT_TRUE(writer.WriteMessageHeader(ntlm::MessageType::NEGOTIATE));
+
+  HandleAnotherChallenge(CreateType2Token(writer.GetBuffer()));
+  EXPECT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, MinimalStructurallyValidType2) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  ntlm::NtlmBufferWriter writer(ntlm::CHALLENGE_HEADER_LEN);
+  EXPECT_TRUE(writer.WriteMessageHeader(ntlm::MessageType::CHALLENGE));
+
+  // A message with both length and offset equal zero is not forbidden
+  // by the spec (2.2.1.2) however it is not what is recommended.
+  // But test it anyway.
+  EXPECT_TRUE(writer.WriteSecurityBuffer(ntlm::SecurityBuffer()));
+
+  HandleAnotherChallenge(CreateType2Token(writer.GetBuffer()));
+  EXPECT_EQ(OK, GetGenerateAuthTokenResult());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, Type2MessageWithNoTargetName) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  ntlm::NtlmBufferWriter writer(ntlm::CHALLENGE_HEADER_LEN);
+  EXPECT_TRUE(writer.WriteMessageHeader(ntlm::MessageType::CHALLENGE));
+
+  // 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.
+  EXPECT_TRUE(writer.WriteSecurityBuffer(
+      ntlm::SecurityBuffer(ntlm::CHALLENGE_HEADER_LEN, 0)));
+
+  HandleAnotherChallenge(CreateType2Token(writer.GetBuffer()));
+  EXPECT_EQ(OK, GetGenerateAuthTokenResult());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, Type2MessageWithTargetName) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  // One extra byte is provided for target name.
+  ntlm::NtlmBufferWriter writer(ntlm::CHALLENGE_HEADER_LEN + 1);
+  EXPECT_TRUE(writer.WriteMessageHeader(ntlm::MessageType::CHALLENGE));
+
+  // The target name field is 1 byte long.
+  EXPECT_TRUE(writer.WriteSecurityBuffer(
+      ntlm::SecurityBuffer(ntlm::CHALLENGE_HEADER_LEN, 1)));

[asanka, 2017/06/23 21:29:11]

Note that you are relying on NtlmBufferWriter initializing the extra byte. This
is not the case in the dependent patchset. I added comment in the CL that
introduces NtlmBufferWriter to address that.

[zentaro, 2017/07/05 17:57:41]

N/A now.

+
+  HandleAnotherChallenge(CreateType2Token(writer.GetBuffer()));
+  EXPECT_EQ(OK, GetGenerateAuthTokenResult());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, NoTargetNameOverflowFromOffset) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  ntlm::NtlmBufferWriter writer(ntlm::CHALLENGE_HEADER_LEN);
+  EXPECT_TRUE(writer.WriteMessageHeader(ntlm::MessageType::CHALLENGE));
+
+  // Claim that the target name field is 1 byte long and outside
+  // the buffer.
+  EXPECT_TRUE(writer.WriteSecurityBuffer(
+      ntlm::SecurityBuffer(ntlm::CHALLENGE_HEADER_LEN, 1)));
+
+  // 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.
+  HandleAnotherChallenge(CreateType2Token(writer.GetBuffer()));
+  EXPECT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, NoTargetNameOverflowFromLength) {
+  EXPECT_EQ(OK, CreateHandler());
+
+  // Message has 1 extra byte of space after the header for the
+  // target name.
+  ntlm::NtlmBufferWriter writer(ntlm::CHALLENGE_HEADER_LEN + 1);
+  EXPECT_TRUE(writer.WriteMessageHeader(ntlm::MessageType::CHALLENGE));
+  // Claim that the target name field is 2 bytes long but
+  // there is only 1 byte of space.
+  EXPECT_TRUE(writer.WriteSecurityBuffer(
+      ntlm::SecurityBuffer(ntlm::CHALLENGE_HEADER_LEN, 2)));
+
+  // 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.
+  HandleAnotherChallenge(CreateType2Token(writer.GetBuffer()));
+  EXPECT_EQ(ERR_UNEXPECTED, GetGenerateAuthTokenResult());
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, Type3RespectsUnicode) {
+  HttpAuthHandlerNTLM::ScopedProcSetter proc_setter(MockRandom,
+                                                    MockGetHostName);
+  EXPECT_EQ(OK, CreateHandler());
+
+  // Generate the type 2 message from the server.
+  ntlm::NtlmBufferWriter writer(ntlm::CHALLENGE_HEADER_LEN);
+  EXPECT_TRUE(writer.WriteMessageHeader(ntlm::MessageType::CHALLENGE));
+  // No target name. Chrome doesn't use it anyway.
+  EXPECT_TRUE(writer.WriteSecurityBuffer(
+      ntlm::SecurityBuffer(ntlm::CHALLENGE_HEADER_LEN, 0)));
+  // Set the unicode flag.
+  EXPECT_TRUE(writer.WriteFlags(ntlm::NegotiateFlags::UNICODE));
+
+  std::string token;
+  HandleAnotherChallenge(CreateType2Token(writer.GetBuffer()));
+  EXPECT_EQ(OK, GenerateAuthToken(&token));
+
+  // Validate the type 3 message
+  std::string decoded;
+  EXPECT_TRUE(DecodeChallenge(token, &decoded));
+  ntlm::NtlmBufferReader reader(decoded);
+  EXPECT_TRUE(reader.MatchMessageHeader(ntlm::MessageType::AUTHENTICATE));
+
+  // Skip the LM and NTLM Hash fields. This test isn't testing that.
+  EXPECT_TRUE(reader.SkipSecurityBuffer());

[asanka, 2017/06/23 21:29:10]

SkipSecurityBufferWithValidation() ?

[zentaro, 2017/07/05 17:57:41]

Done.

+  EXPECT_TRUE(reader.SkipSecurityBuffer());
+  base::string16 domain;
+  base::string16 username;
+  base::string16 hostname;
+  ReadString16Payload(&reader, &domain);
+  EXPECT_EQ(base::ASCIIToUTF16(domain_ascii_), domain);
+  ReadString16Payload(&reader, &username);
+  EXPECT_EQ(base::ASCIIToUTF16(user_ascii_), username);
+  ReadString16Payload(&reader, &hostname);
+  EXPECT_EQ(base::ASCIIToUTF16(MockGetHostName()), hostname);
+
+  // Skip the session key which isn't used.
+  EXPECT_TRUE(reader.SkipSecurityBufferWithValidation());
+
+  // Verify the unicode flag is set.
+  ntlm::NegotiateFlags flags;
+  EXPECT_TRUE(reader.ReadFlags(&flags));
+  EXPECT_EQ(ntlm::NegotiateFlags::UNICODE,
+            flags & ntlm::NegotiateFlags::UNICODE);
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, Type3WithoutUnicode) {
+  HttpAuthHandlerNTLM::ScopedProcSetter proc_setter(MockRandom,
+                                                    MockGetHostName);
+  EXPECT_EQ(OK, CreateHandler());
+
+  // Generate the type 2 message from the server.
+  ntlm::NtlmBufferWriter writer(ntlm::CHALLENGE_HEADER_LEN);
+  EXPECT_TRUE(writer.WriteMessageHeader(ntlm::MessageType::CHALLENGE));
+  // No target name. Chrome doesn't use it anyway.
+  EXPECT_TRUE(writer.WriteSecurityBuffer(
+      ntlm::SecurityBuffer(ntlm::CHALLENGE_HEADER_LEN, 0)));
+  // Set the OEM flag.
+  EXPECT_TRUE(writer.WriteFlags(ntlm::NegotiateFlags::OEM));
+
+  std::string token;
+  HandleAnotherChallenge(CreateType2Token(writer.GetBuffer()));
+  EXPECT_EQ(OK, GenerateAuthToken(&token));
+
+  // Validate the type 3 message
+  std::string decoded;
+  EXPECT_TRUE(DecodeChallenge(token, &decoded));
+  ntlm::NtlmBufferReader reader(decoded);
+  EXPECT_TRUE(reader.MatchMessageHeader(ntlm::MessageType::AUTHENTICATE));
+
+  // Skip the 2 hash fields. This test isn't testing that.
+  EXPECT_TRUE(reader.SkipSecurityBufferWithValidation());
+  EXPECT_TRUE(reader.SkipSecurityBufferWithValidation());
+  std::string domain;
+  std::string username;
+  std::string hostname;
+  ReadStringPayload(&reader, &domain);
+  EXPECT_EQ(domain_ascii_, domain);
+  ReadStringPayload(&reader, &username);
+  EXPECT_EQ(user_ascii_, username);
+  ReadStringPayload(&reader, &hostname);
+  EXPECT_EQ(MockGetHostName(), hostname);
+
+  // Skip the session key which isn't used.
+  EXPECT_TRUE(reader.SkipSecurityBufferWithValidation());
+
+  // Verify the unicode flag is not set and OEM flag is.
+  ntlm::NegotiateFlags flags;
+  EXPECT_TRUE(reader.ReadFlags(&flags));
+  EXPECT_EQ(ntlm::NegotiateFlags::NONE, flags & ntlm::NegotiateFlags::UNICODE);
+  EXPECT_EQ(ntlm::NegotiateFlags::OEM, flags & ntlm::NegotiateFlags::OEM);
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, Type3UnicodeNoSessionSecurity) {
+  // Verify that the client won't be downgraded if the server clears
+  // the session security flag.
+  HttpAuthHandlerNTLM::ScopedProcSetter proc_setter(MockRandom,
+                                                    MockGetHostName);
+  EXPECT_EQ(OK, CreateHandler());
+
+  // Generate the type 2 message from the server.
+  ntlm::NtlmBufferWriter writer(ntlm::CHALLENGE_HEADER_LEN);
+  EXPECT_TRUE(writer.WriteMessageHeader(ntlm::MessageType::CHALLENGE));
+  // No target name. Chrome doesn't use it anyway.
+  EXPECT_TRUE(writer.WriteSecurityBuffer(
+      ntlm::SecurityBuffer(ntlm::CHALLENGE_HEADER_LEN, 0)));
+  // Set the unicode but not the session security flag.
+  EXPECT_TRUE(writer.WriteFlags(ntlm::NegotiateFlags::UNICODE));
+
+  uint8_t server_challenge[] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17};
+  EXPECT_EQ(arraysize(server_challenge), ntlm::CHALLENGE_LEN);
+  EXPECT_TRUE(writer.WriteBytes(server_challenge, ntlm::CHALLENGE_LEN));
+  EXPECT_TRUE(writer.IsEndOfBuffer());
+
+  std::string token;
+  HandleAnotherChallenge(CreateType2Token(writer.GetBuffer()));
+  EXPECT_EQ(OK, GenerateAuthToken(&token));
+
+  // Validate the type 3 message
+  std::string decoded;
+  EXPECT_TRUE(DecodeChallenge(token, &decoded));
+  ntlm::NtlmBufferReader reader(decoded);
+  EXPECT_TRUE(reader.MatchMessageHeader(ntlm::MessageType::AUTHENTICATE));
+
+  // Read the LM and NTLM Response Payloads.
+  uint8_t expected_lm_response[ntlm::RESPONSE_V1_LEN];
+  uint8_t expected_ntlm_response[ntlm::RESPONSE_V1_LEN];
+  uint8_t actual_lm_response[ntlm::RESPONSE_V1_LEN];
+  uint8_t actual_ntlm_response[ntlm::RESPONSE_V1_LEN];
+
+  ReadBytesPayload(&reader, actual_lm_response, ntlm::RESPONSE_V1_LEN);
+  ReadBytesPayload(&reader, actual_ntlm_response, ntlm::RESPONSE_V1_LEN);
+
+  // Session security also uses a client generated challenge so
+  // use the mock to get the same value that the implementation
+  // would get.
+  uint8_t client_challenge[ntlm::CHALLENGE_LEN];
+  MockRandom(client_challenge, ntlm::CHALLENGE_LEN);
+
+  ntlm::GenerateResponsesV1WithSS(base::ASCIIToUTF16(password_ascii_),

[asanka, 2017/06/23 21:29:11]

While this makes sense for now since the test is exercising two implementations,
it won't make sense when we cut over to a single implementation. At that point
we'd be verifying that the new HttpAuthHandlerNtlmPortable winds up calling
GenerateResponses* at some point. Beyond that, we are comparing the output of
GenerateResponses* with itself.

Let's  test against fixed hand verified static output rather than ones that are
generated by the implementation we are testing.

It's a bit hard to verify the output token against a fixed static token since
the protocol doesn't provide a canonical representation. So I'm not against
using a NtlmBufferReader. But at least the individual fields should be fixed.

[zentaro, 2017/07/05 17:57:41]

Are you OK leaving this for this CL? 

We have validated GenerateResponsesV1WithSS against the test data in the spec.
Most of the content/output tests in this file actually belong against the lower
level NtlmClient class and are moved there in the next CL which switches out the
implementation. In that CL I will add tests against the verbatim binary messages
in the spec. We don't have enough control over the current implementation to be
able to generate the exact spec version of the messages.

HttpAuthHandlerNtlm is only really responsible for base64 encoding and the
challenge response mechanism but this was the only place to put it for now.

+                                  server_challenge, client_challenge,
+                                  expected_lm_response, expected_ntlm_response);
+
+  // Verify that the client still generated a response that uses
+  // session security.
+  EXPECT_EQ(0, memcmp(expected_lm_response, actual_lm_response,
+                      ntlm::RESPONSE_V1_LEN));
+  EXPECT_EQ(0, memcmp(expected_ntlm_response, actual_ntlm_response,
+                      ntlm::RESPONSE_V1_LEN));
+
+  base::string16 domain;
+  base::string16 username;
+  base::string16 hostname;
+  ReadString16Payload(&reader, &domain);
+  EXPECT_EQ(base::ASCIIToUTF16(domain_ascii_), domain);
+  ReadString16Payload(&reader, &username);
+  EXPECT_EQ(base::ASCIIToUTF16(user_ascii_), username);
+  ReadString16Payload(&reader, &hostname);
+  EXPECT_EQ(base::ASCIIToUTF16(MockGetHostName()), hostname);
+
+  // Skip the session key which isn't used.
+  EXPECT_TRUE(reader.SkipSecurityBufferWithValidation());
+
+  // Verify the unicode flag is set.
+  ntlm::NegotiateFlags flags;
+  EXPECT_TRUE(reader.ReadFlags(&flags));
+  EXPECT_EQ(ntlm::NegotiateFlags::UNICODE,
+            flags & ntlm::NegotiateFlags::UNICODE);
+}
+
+TEST_F(HttpAuthHandlerNtlmPortableTest, Type3UnicodeWithSessionSecurity) {
+  HttpAuthHandlerNTLM::ScopedProcSetter proc_setter(MockRandom,
+                                                    MockGetHostName);
+  EXPECT_EQ(OK, CreateHandler());
+
+  // Generate the type 2 message from the server.
+  ntlm::NtlmBufferWriter writer(ntlm::CHALLENGE_HEADER_LEN);
+  EXPECT_TRUE(writer.WriteMessageHeader(ntlm::MessageType::CHALLENGE));
+  // No target name. Chrome doesn't use it anyway.
+  EXPECT_TRUE(writer.WriteSecurityBuffer(
+      ntlm::SecurityBuffer(ntlm::CHALLENGE_HEADER_LEN, 0)));
+  // Set the unicode and session security flag.
+  EXPECT_TRUE(
+      writer.WriteFlags((ntlm::NegotiateFlags::UNICODE |
+                         ntlm::NegotiateFlags::EXTENDED_SESSIONSECURITY)));
+
+  uint8_t server_challenge[] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17};
+  EXPECT_EQ(arraysize(server_challenge), ntlm::CHALLENGE_LEN);
+  EXPECT_TRUE(writer.WriteBytes(server_challenge, ntlm::CHALLENGE_LEN));
+  EXPECT_TRUE(writer.IsEndOfBuffer());
+
+  std::string token;
+  HandleAnotherChallenge(CreateType2Token(writer.GetBuffer()));
+  EXPECT_EQ(OK, GenerateAuthToken(&token));
+
+  // Validate the type 3 message
+  std::string decoded;
+  EXPECT_TRUE(DecodeChallenge(token, &decoded));
+  ntlm::NtlmBufferReader reader(decoded);
+  EXPECT_TRUE(reader.MatchMessageHeader(ntlm::MessageType::AUTHENTICATE));
+
+  // Read the LM and NTLM Response Payloads.
+  uint8_t expected_lm_response[ntlm::RESPONSE_V1_LEN];
+  uint8_t expected_ntlm_response[ntlm::RESPONSE_V1_LEN];
+  uint8_t actual_lm_response[ntlm::RESPONSE_V1_LEN];
+  uint8_t actual_ntlm_response[ntlm::RESPONSE_V1_LEN];
+
+  ReadBytesPayload(&reader, actual_lm_response, ntlm::RESPONSE_V1_LEN);
+  ReadBytesPayload(&reader, actual_ntlm_response, ntlm::RESPONSE_V1_LEN);
+
+  // Session security also uses a client generated challenge so
+  // use the mock to get the same value that the implementation
+  // would get.
+  uint8_t client_challenge[ntlm::CHALLENGE_LEN];
+  MockRandom(client_challenge, ntlm::CHALLENGE_LEN);
+
+  ntlm::GenerateResponsesV1WithSS(base::ASCIIToUTF16(password_ascii_),
+                                  server_challenge, client_challenge,
+                                  expected_lm_response, expected_ntlm_response);
+
+  EXPECT_EQ(0, memcmp(expected_lm_response, actual_lm_response,
+                      ntlm::RESPONSE_V1_LEN));
+  EXPECT_EQ(0, memcmp(expected_ntlm_response, actual_ntlm_response,
+                      ntlm::RESPONSE_V1_LEN));
+
+  base::string16 domain;
+  base::string16 username;
+  base::string16 hostname;
+  ReadString16Payload(&reader, &domain);
+  EXPECT_EQ(base::ASCIIToUTF16(domain_ascii_), domain);
+  ReadString16Payload(&reader, &username);
+  EXPECT_EQ(base::ASCIIToUTF16(user_ascii_), username);
+  ReadString16Payload(&reader, &hostname);
+  EXPECT_EQ(base::ASCIIToUTF16(MockGetHostName()), hostname);
+
+  // Skip the session key which isn't used.

[asanka, 2017/06/23 21:29:10]

?

[zentaro, 2017/07/05 17:57:42]

Clarified the comment a bit. AFAIK it's only for the connectionless mode for the
sealing/signing stuff. We don't send NTLMSSP_NEGOTIATE_KEY_EXCH so it should be
empty.

+  EXPECT_TRUE(reader.SkipSecurityBufferWithValidation());
+
+  // Verify the unicode flag is set.
+  ntlm::NegotiateFlags flags;
+  EXPECT_TRUE(reader.ReadFlags(&flags));
+  EXPECT_EQ(ntlm::NegotiateFlags::UNICODE,
+            flags & ntlm::NegotiateFlags::UNICODE);
+}
+
+#endif  // defined(NTLM_PORTABLE)
+
+}  // namespace net