system-proxy: Build //net/ntlm code
This CL adapts and builds the //net/ntlm code and tests with the
following changes:
1. Replace boringssl with openssl. boringssl is not available on the
platform.
2. Remove reference to the "net_string_util.h" file because it requires
the third_party library icu to build. Instead, the conversion is made
using base::ToUpperASCII.
3. For the test data, explicitly convert hex codes to char. On the
platform, the hex codes are converted to unsigned chars by default,
causing compilation issues when the value is assigned to a char.
4. Redefine the NET_EXPORT and NET_EXPORT_PRIVATE macros to do nothing.
BUG=b:173097096
TEST=P2_TEST_FILTER="Ntlm*" cros_workon_make
--board=eve system-proxy --test
Change-Id: I8c09a382e2542db2cf8e329d26fb7426621f2014
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2532006
Tested-by: Andreea-Elena Costinas <acostinas@google.com>
Commit-Queue: Andreea-Elena Costinas <acostinas@google.com>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
diff --git a/system-proxy/BUILD.gn b/system-proxy/BUILD.gn
index 18dd897..e88adcc 100644
--- a/system-proxy/BUILD.gn
+++ b/system-proxy/BUILD.gn
@@ -48,6 +48,7 @@
"libminijail",
"libpatchpanel-client",
"libpatchpanel-util",
+ "openssl",
# system_api depends on protobuf (or protobuf-lite). It must
# appear before protobuf or the linker flags won't be in the right
@@ -68,6 +69,16 @@
configs += [ ":target_defaults" ]
sources = [
"kerberos_client.cc",
+ "net/ntlm/ntlm.cc",
+ "net/ntlm/ntlm.h",
+ "net/ntlm/ntlm_buffer_reader.cc",
+ "net/ntlm/ntlm_buffer_reader.h",
+ "net/ntlm/ntlm_buffer_writer.cc",
+ "net/ntlm/ntlm_buffer_writer.h",
+ "net/ntlm/ntlm_client.cc",
+ "net/ntlm/ntlm_client.h",
+ "net/ntlm/ntlm_constants.cc",
+ "net/ntlm/ntlm_constants.h",
"protobuf_util.cc",
"sandboxed_worker.cc",
"system_proxy_adaptor.cc",
@@ -126,6 +137,11 @@
sources = [
"http_util_test.cc",
"kerberos_client_test.cc",
+ "net/ntlm/ntlm_buffer_reader_test.cc",
+ "net/ntlm/ntlm_buffer_writer_test.cc",
+ "net/ntlm/ntlm_client_test.cc",
+ "net/ntlm/ntlm_test.cc",
+ "net/ntlm/ntlm_test_data.h",
"proxy_connect_job_test.cc",
"server_proxy_test.cc",
"system_proxy_adaptor_test.cc",
diff --git a/system-proxy/net/README.md b/system-proxy/net/README.md
new file mode 100644
index 0000000..41d07135
--- /dev/null
+++ b/system-proxy/net/README.md
@@ -0,0 +1,53 @@
+# NET > NTLM
+
+This directory contains the net NTLM library which is a modified copy of
+Chromium's net NTLM code (net/ntlm). The current files are taken from milestone
+M88 of the code with the latest commit hash of
+`2fa09e20ad9e4a88207418cdffe83fd244ad6151`.
+
+The net NTLM library is used by System-proxy to generate NTLM authentication
+messages using the Chrome OS login password.
+
+## Modifications
+
+The code here is a modification of Chromium's net NTLM code. The modification is
+done to minimize the code imported. Currently, we're only interested in the
+`NtlmClient::GenerateAuthenticateMessage` method which is the entry point to
+the net NTLM stack.
+
+The modification process is done by importing the chromium //net/ntlm code with
+the original directory structure and the required changes to be compliant with the
+current clang presubmit checks (see CL:2532227), followed by CL:2532006
+containing the minimal amount of code necessary to build the code and successfully
+run the unit test.
+
+To verify that the build is successful, run the tests by entering the following
+command in `cros_sdk`:
+
+```shell
+P2_TEST_FILTER="Ntlm*" USE="-cros-debug" cros_workon_make --board=eve system-proxy --test
+```
+
+Below are the changes made:
+* Replace boringssl with openssl. boringssl is not available on the platform.
+* Remove reference to the "net_string_util.h" file because it requires the
+third_party library icu to build. Instead, use the locale insensitive
+`base::ToUpperASCII` moethod to converts UTF-16 strings to uppercase. This may
+cause issues since the browser does case sensitive conversions
+(https://crbug.com/1051924).
+* For the test data, explicitly convert hex codes to signed char. On the
+platform, the hex codes are converted to unsigned chars by default, causing
+compilation issues when the value is re-assigned to a signed char.
+* Redefine the `NET_EXPORT` and `NET_EXPORT_PRIVATE` macros to do nothing. These
+ macros are not necessary since the //net/ntlm code is not built as a component
+ build for System-proxy.
+* Fix clang presubmit errors (see CL:2532227):
+ * Update license headers and header guards.
+ * Add missing includes.
+ * Replace `DISALLOW_COPY_AND_ASSIGN` macro with explicitly deteled constructors.
+ * Remove file `/net/ntlm/ntlm_client_fuzzer.cc`.
+ * Format files `system-proxy/net/ntlm/ntlm.h` and `system-proxy/net/ntlm/ntlm_test_data.h`.
+ * Rename *_unittest.cc files to *_test.cc.
+ An alternative would have been to disable presubmit tests from running on the
+ imported files by modifying the `//platform2/PRESUBMIT.cfg` file and the associated
+ scripts which is more complex task than fixing the errors.
diff --git a/system-proxy/net/base/net_export.h b/system-proxy/net/base/net_export.h
new file mode 100644
index 0000000..d826a52
--- /dev/null
+++ b/system-proxy/net/base/net_export.h
@@ -0,0 +1,15 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SYSTEM_PROXY_NET_BASE_NET_EXPORT_H_
+#define SYSTEM_PROXY_NET_BASE_NET_EXPORT_H_
+
+// The chromium net module can be built as a component build which exposes
+// functionality to consumers using NET_EXPORT and NET_EXPORT_PRIVATE macros.
+// System-proxy builds the //net/ntlm code as part of libsystemproxy so
+// exporting the functionality is not necessary.
+#define NET_EXPORT
+#define NET_EXPORT_PRIVATE
+
+#endif // SYSTEM_PROXY_NET_BASE_NET_EXPORT_H_
diff --git a/system-proxy/net/ntlm/ntlm.cc b/system-proxy/net/ntlm/ntlm.cc
index 618e7fc..c22b2a2 100644
--- a/system-proxy/net/ntlm/ntlm.cc
+++ b/system-proxy/net/ntlm/ntlm.cc
@@ -2,24 +2,26 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "net/ntlm/ntlm.h"
+#include "system-proxy/net/ntlm/ntlm.h"
-#include <string>
+#include <algorithm>
+#include <string.h>
#include <utility>
+#include <crypto/scoped_openssl_types.h>
+#include <openssl/des.h>
+#include <openssl/hmac.h>
+#include <openssl/md4.h>
+#include <openssl/md5.h>
+
#include "base/check_op.h"
#include "base/containers/span.h"
#include "base/metrics/histogram_macros.h"
#include "base/notreached.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
-#include "net/base/net_string_util.h"
-#include "net/ntlm/ntlm_buffer_writer.h"
-#include "net/ntlm/ntlm_constants.h"
-#include "third_party/boringssl/src/include/openssl/des.h"
-#include "third_party/boringssl/src/include/openssl/hmac.h"
-#include "third_party/boringssl/src/include/openssl/md4.h"
-#include "third_party/boringssl/src/include/openssl/md5.h"
+#include "system-proxy/net/ntlm/ntlm_buffer_writer.h"
+#include "system-proxy/net/ntlm/ntlm_constants.h"
namespace net {
namespace ntlm {
@@ -213,8 +215,8 @@
DES_key_schedule key_schedule;
DES_set_odd_parity(key_block);
DES_set_key(key_block, &key_schedule);
- DES_ecb_encrypt(challenge_block, response_block, &key_schedule,
- DES_ENCRYPT);
+ DES_ecb_encrypt(const_cast<DES_cblock*>(challenge_block), response_block,
+ &key_schedule, DES_ENCRYPT);
}
}
@@ -295,9 +297,7 @@
base::span<uint8_t, kNtlmHashLen> v2_hash) {
// NOTE: According to [MS-NLMP] Section 3.3.2 only the username and not the
// domain is uppercased.
- base::string16 upper_username;
- bool result = ToUpper(username, &upper_username);
- DCHECK(result);
+ base::string16 upper_username = base::ToUpperASCII(username);
// TODO(https://crbug.com/1051924): Using a locale-sensitive upper casing
// algorithm is problematic. A more predictable approach is to only uppercase
@@ -344,7 +344,7 @@
base::span<const uint8_t, kProofInputLenV2> v2_input,
base::span<const uint8_t> target_info,
base::span<uint8_t, kNtlmProofLenV2> v2_proof) {
- bssl::ScopedHMAC_CTX ctx;
+ crypto::ScopedHMAC_CTX ctx(HMAC_CTX_new());
HMAC_Init_ex(ctx.get(), v2_hash.data(), kNtlmHashLen, EVP_md5(), NULL);
DCHECK_EQ(kNtlmProofLenV2, HMAC_size(ctx.get()));
HMAC_Update(ctx.get(), server_challenge.data(), kChallengeLen);
@@ -389,7 +389,7 @@
base::span<const uint8_t> challenge_msg,
base::span<const uint8_t> authenticate_msg,
base::span<uint8_t, kMicLenV2> mic) {
- bssl::ScopedHMAC_CTX ctx;
+ crypto::ScopedHMAC_CTX ctx(HMAC_CTX_new());
HMAC_Init_ex(ctx.get(), session_key.data(), kSessionKeyLenV2, EVP_md5(),
NULL);
DCHECK_EQ(kMicLenV2, HMAC_size(ctx.get()));
diff --git a/system-proxy/net/ntlm/ntlm.h b/system-proxy/net/ntlm/ntlm.h
index 934aa15..012a493 100644
--- a/system-proxy/net/ntlm/ntlm.h
+++ b/system-proxy/net/ntlm/ntlm.h
@@ -21,8 +21,8 @@
#include "base/containers/span.h"
#include "base/strings/string16.h"
#include "base/strings/string_piece.h"
-#include "net/base/net_export.h"
-#include "net/ntlm/ntlm_constants.h"
+#include "system-proxy/net/base/net_export.h"
+#include "system-proxy/net/ntlm/ntlm_constants.h"
namespace net {
namespace ntlm {
diff --git a/system-proxy/net/ntlm/ntlm_buffer_reader.cc b/system-proxy/net/ntlm/ntlm_buffer_reader.cc
index 02918c2..55a36eb 100644
--- a/system-proxy/net/ntlm/ntlm_buffer_reader.cc
+++ b/system-proxy/net/ntlm/ntlm_buffer_reader.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "net/ntlm/ntlm_buffer_reader.h"
+#include "system-proxy/net/ntlm/ntlm_buffer_reader.h"
#include <string.h>
#include <utility>
diff --git a/system-proxy/net/ntlm/ntlm_buffer_reader.h b/system-proxy/net/ntlm/ntlm_buffer_reader.h
index 0433f68..40a325f 100644
--- a/system-proxy/net/ntlm/ntlm_buffer_reader.h
+++ b/system-proxy/net/ntlm/ntlm_buffer_reader.h
@@ -12,8 +12,8 @@
#include <vector>
#include "base/containers/span.h"
-#include "net/base/net_export.h"
-#include "net/ntlm/ntlm_constants.h"
+#include "system-proxy/net/base/net_export.h"
+#include "system-proxy/net/ntlm/ntlm_constants.h"
namespace net {
namespace ntlm {
diff --git a/system-proxy/net/ntlm/ntlm_buffer_reader_test.cc b/system-proxy/net/ntlm/ntlm_buffer_reader_test.cc
index fc3334b..253eea8 100644
--- a/system-proxy/net/ntlm/ntlm_buffer_reader_test.cc
+++ b/system-proxy/net/ntlm/ntlm_buffer_reader_test.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "net/ntlm/ntlm_buffer_reader.h"
+#include "system-proxy/net/ntlm/ntlm_buffer_reader.h"
#include "base/stl_util.h"
#include "base/strings/utf_string_conversions.h"
diff --git a/system-proxy/net/ntlm/ntlm_buffer_writer.cc b/system-proxy/net/ntlm/ntlm_buffer_writer.cc
index 1f23a69..3cdf15b 100644
--- a/system-proxy/net/ntlm/ntlm_buffer_writer.cc
+++ b/system-proxy/net/ntlm/ntlm_buffer_writer.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "net/ntlm/ntlm_buffer_writer.h"
+#include "system-proxy/net/ntlm/ntlm_buffer_writer.h"
#include <string.h>
diff --git a/system-proxy/net/ntlm/ntlm_buffer_writer.h b/system-proxy/net/ntlm/ntlm_buffer_writer.h
index 84ce430..fadc346 100644
--- a/system-proxy/net/ntlm/ntlm_buffer_writer.h
+++ b/system-proxy/net/ntlm/ntlm_buffer_writer.h
@@ -16,8 +16,8 @@
#include "base/containers/span.h"
#include "base/strings/string16.h"
#include "base/strings/string_piece.h"
-#include "net/base/net_export.h"
-#include "net/ntlm/ntlm_constants.h"
+#include "system-proxy/net/base/net_export.h"
+#include "system-proxy/net/ntlm/ntlm_constants.h"
namespace net {
namespace ntlm {
diff --git a/system-proxy/net/ntlm/ntlm_buffer_writer_test.cc b/system-proxy/net/ntlm/ntlm_buffer_writer_test.cc
index 7bec964..3117c64 100644
--- a/system-proxy/net/ntlm/ntlm_buffer_writer_test.cc
+++ b/system-proxy/net/ntlm/ntlm_buffer_writer_test.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "net/ntlm/ntlm_buffer_writer.h"
+#include "system-proxy/net/ntlm/ntlm_buffer_writer.h"
#include <vector>
diff --git a/system-proxy/net/ntlm/ntlm_client.cc b/system-proxy/net/ntlm/ntlm_client.cc
index 9509722..af9db6b 100644
--- a/system-proxy/net/ntlm/ntlm_client.cc
+++ b/system-proxy/net/ntlm/ntlm_client.cc
@@ -2,7 +2,7 @@
// 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 "system-proxy/net/ntlm/ntlm_client.h"
#include <string.h>
#include <vector>
@@ -10,10 +10,10 @@
#include "base/check_op.h"
#include "base/containers/span.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"
-#include "net/ntlm/ntlm_constants.h"
+#include "system-proxy/net/ntlm/ntlm.h"
+#include "system-proxy/net/ntlm/ntlm_buffer_reader.h"
+#include "system-proxy/net/ntlm/ntlm_buffer_writer.h"
+#include "system-proxy/net/ntlm/ntlm_constants.h"
namespace net {
namespace ntlm {
diff --git a/system-proxy/net/ntlm/ntlm_client.h b/system-proxy/net/ntlm/ntlm_client.h
index 06c85f0..8011547 100644
--- a/system-proxy/net/ntlm/ntlm_client.h
+++ b/system-proxy/net/ntlm/ntlm_client.h
@@ -24,8 +24,8 @@
#include "base/containers/span.h"
#include "base/strings/string16.h"
#include "base/strings/string_piece.h"
-#include "net/base/net_export.h"
-#include "net/ntlm/ntlm_constants.h"
+#include "system-proxy/net/base/net_export.h"
+#include "system-proxy/net/ntlm/ntlm_constants.h"
namespace net {
namespace ntlm {
diff --git a/system-proxy/net/ntlm/ntlm_client_test.cc b/system-proxy/net/ntlm/ntlm_client_test.cc
index e4689a2..40c1911 100644
--- a/system-proxy/net/ntlm/ntlm_client_test.cc
+++ b/system-proxy/net/ntlm/ntlm_client_test.cc
@@ -2,7 +2,7 @@
// 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 "system-proxy/net/ntlm/ntlm_client.h"
#include <string>
#include <utility>
@@ -12,10 +12,10 @@
#include "base/stl_util.h"
#include "base/strings/string_util.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 "system-proxy/net/ntlm/ntlm.h"
+#include "system-proxy/net/ntlm/ntlm_buffer_reader.h"
+#include "system-proxy/net/ntlm/ntlm_buffer_writer.h"
+#include "system-proxy/net/ntlm/ntlm_test_data.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace net {
diff --git a/system-proxy/net/ntlm/ntlm_constants.cc b/system-proxy/net/ntlm/ntlm_constants.cc
index effc819..f23d8f9 100644
--- a/system-proxy/net/ntlm/ntlm_constants.cc
+++ b/system-proxy/net/ntlm/ntlm_constants.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "net/ntlm/ntlm_constants.h"
+#include "system-proxy/net/ntlm/ntlm_constants.h"
#include <utility>
diff --git a/system-proxy/net/ntlm/ntlm_constants.h b/system-proxy/net/ntlm/ntlm_constants.h
index 8dbcd96..55ee44b 100644
--- a/system-proxy/net/ntlm/ntlm_constants.h
+++ b/system-proxy/net/ntlm/ntlm_constants.h
@@ -11,7 +11,6 @@
#include <vector>
#include "base/stl_util.h"
-#include "net/base/net_export.h"
namespace net {
namespace ntlm {
@@ -130,7 +129,7 @@
// other AvPairs the value of these 2 fields is undefined and the payload
// is in the |buffer| field. For these fields the payload is copied verbatim
// and it's content is not read or validated in any way.
-struct NET_EXPORT_PRIVATE AvPair {
+struct AvPair {
AvPair();
AvPair(TargetInfoAvId avid, uint16_t avlen);
AvPair(TargetInfoAvId avid, std::vector<uint8_t> buffer);
diff --git a/system-proxy/net/ntlm/ntlm_test.cc b/system-proxy/net/ntlm/ntlm_test.cc
index 9be53da..0c387bf 100644
--- a/system-proxy/net/ntlm/ntlm_test.cc
+++ b/system-proxy/net/ntlm/ntlm_test.cc
@@ -11,7 +11,7 @@
//
// [1] https://msdn.microsoft.com/en-us/library/cc236621.aspx
-#include "net/ntlm/ntlm.h"
+#include "system-proxy/net/ntlm/ntlm.h"
#include <algorithm>
#include <iterator>
@@ -21,7 +21,7 @@
#include "base/stl_util.h"
#include "base/strings/string16.h"
#include "base/strings/utf_string_conversions.h"
-#include "net/ntlm/ntlm_test_data.h"
+#include "system-proxy/net/ntlm/ntlm_test_data.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace net {
diff --git a/system-proxy/net/ntlm/ntlm_test_data.h b/system-proxy/net/ntlm/ntlm_test_data.h
index 8198f77..cba2082 100644
--- a/system-proxy/net/ntlm/ntlm_test_data.h
+++ b/system-proxy/net/ntlm/ntlm_test_data.h
@@ -16,7 +16,7 @@
#ifndef SYSTEM_PROXY_NET_NTLM_NTLM_TEST_DATA_H_
#define SYSTEM_PROXY_NET_NTLM_NTLM_TEST_DATA_H_
-#include "net/ntlm/ntlm_constants.h"
+#include "system-proxy/net/ntlm/ntlm_constants.h"
namespace net {
namespace ntlm {
@@ -43,12 +43,60 @@
constexpr char kHostnameAscii[] = "COMPUTER";
// Test data obtained from [2].
-constexpr char kChannelBindings[] = {
- 't', 'l', 's', '-', 's', 'e', 'r', 'v', 'e', 'r', '-',
- 'e', 'n', 'd', '-', 'p', 'o', 'i', 'n', 't', ':', 0xea,
- 0x05, 0xfe, 0xfe, 0xcc, 0x6b, 0x0b, 0xd5, 0x71, 0xdb, 0xbc, 0x5b,
- 0xaa, 0x3e, 0xd4, 0x53, 0x86, 0xd0, 0x44, 0x68, 0x35, 0xf7, 0xb7,
- 0x4c, 0x85, 0x62, 0x1b, 0x99, 0x83, 0x47, 0x5f, 0x95, '\0'};
+constexpr char kChannelBindings[] = {'t',
+ 'l',
+ 's',
+ '-',
+ 's',
+ 'e',
+ 'r',
+ 'v',
+ 'e',
+ 'r',
+ '-',
+ 'e',
+ 'n',
+ 'd',
+ '-',
+ 'p',
+ 'o',
+ 'i',
+ 'n',
+ 't',
+ ':',
+ static_cast<char>(0xea),
+ static_cast<char>(0x05),
+ static_cast<char>(0xfe),
+ static_cast<char>(0xfe),
+ static_cast<char>(0xcc),
+ static_cast<char>(0x6b),
+ static_cast<char>(0x0b),
+ static_cast<char>(0xd5),
+ static_cast<char>(0x71),
+ static_cast<char>(0xdb),
+ static_cast<char>(0xbc),
+ static_cast<char>(0x5b),
+ static_cast<char>(0xaa),
+ static_cast<char>(0x3e),
+ static_cast<char>(0xd4),
+ static_cast<char>(0x53),
+ static_cast<char>(0x86),
+ static_cast<char>(0xd0),
+ static_cast<char>(0x44),
+ static_cast<char>(0x68),
+ static_cast<char>(0x35),
+ static_cast<char>(0xf7),
+ static_cast<char>(0xb7),
+ static_cast<char>(0x4c),
+ static_cast<char>(0x85),
+ static_cast<char>(0x62),
+ static_cast<char>(0x1b),
+ static_cast<char>(0x99),
+ static_cast<char>(0x83),
+ static_cast<char>(0x47),
+ static_cast<char>(0x5f),
+ static_cast<char>(0x95),
+ '\0'};
constexpr char kNtlmSpn[] = {'H', 'T', 'T', 'P', '/', 'S',
'e', 'r', 'v', 'e', 'r', '\0'};
