From 69886f73781b126b3e0f991c870146e91c95c9db Mon Sep 17 00:00:00 2001 From: Francois Marier Date: Wed, 5 Apr 2023 17:42:54 -0700 Subject: [PATCH 1/2] Enable SCT enforcement (fixes brave/brave-browser#22482) This makes Brave follow the same Certificate Transparency policy as Chrome for TLS certificates. It also excludes Brave hostnames which are involved with browser updates in order to ensure that updates always work even if the certificate transparency code breaks in the future. --- .../certificate_transparency_browsertest.cc | 134 ++++++++++++++++++ .../net/profile_network_context_service.cc | 20 +++ .../net/system_network_context_manager.cc | 7 +- ...t-profile_network_context_service.cc.patch | 12 ++ test/BUILD.gn | 1 + 5 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 browser/ssl/certificate_transparency_browsertest.cc create mode 100644 chromium_src/chrome/browser/net/profile_network_context_service.cc create mode 100644 patches/chrome-browser-net-profile_network_context_service.cc.patch diff --git a/browser/ssl/certificate_transparency_browsertest.cc b/browser/ssl/certificate_transparency_browsertest.cc new file mode 100644 index 000000000000..41d0e9002cde --- /dev/null +++ b/browser/ssl/certificate_transparency_browsertest.cc @@ -0,0 +1,134 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +// Based on chromium's +// chrome/browser/ssl/certificate_transparency_browsertest.cc under this +// license: +// +// Copyright 2022 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/run_loop.h" +#include "chrome/browser/net/system_network_context_manager.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ssl/cert_verifier_browser_test.h" +#include "chrome/browser/ssl/ssl_browsertest_util.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/ui_test_utils.h" +#include "components/policy/core/common/mock_configuration_policy_provider.h" +#include "content/public/browser/storage_partition.h" +#include "content/public/test/browser_test.h" +#include "crypto/sha2.h" +#include "net/base/hash_value.h" +#include "net/cert/asn1_util.h" +#include "net/cert/x509_util.h" +#include "net/dns/mock_host_resolver.h" +#include "net/test/cert_test_util.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "net/test/test_data_directory.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +// Returns the Sha256 hash of the SPKI of |cert|. +net::HashValue GetSPKIHash(const CRYPTO_BUFFER* cert) { + base::StringPiece spki_bytes; + EXPECT_TRUE(net::asn1::ExtractSPKIFromDERCert( + net::x509_util::CryptoBufferAsStringPiece(cert), &spki_bytes)); + net::HashValue sha256(net::HASH_VALUE_SHA256); + crypto::SHA256HashString(spki_bytes, sha256.data(), crypto::kSHA256Length); + return sha256; +} + +} // namespace + +// Class used to run browser tests that verify SSL UI triggered due to +// Certificate Transparency verification failures/successes. +class CertificateTransparencyBrowserTest : public CertVerifierBrowserTest { + public: + CertificateTransparencyBrowserTest() + : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) { + SystemNetworkContextManager::SetEnableCertificateTransparencyForTesting( + true); + } + + CertificateTransparencyBrowserTest( + const CertificateTransparencyBrowserTest&) = delete; + CertificateTransparencyBrowserTest& operator=( + const CertificateTransparencyBrowserTest&) = delete; + + ~CertificateTransparencyBrowserTest() override {} + + void SetUpOnMainThread() override { + CertVerifierBrowserTest::SetUpOnMainThread(); + host_resolver()->AddRule("*", "127.0.0.1"); + https_server_.AddDefaultHandlers(GetChromeTestDataDir()); + } + + void SetUp() override { + policy_provider_.SetDefaultReturns( + /*is_initialization_complete_return=*/true, + /*is_first_policy_load_complete_return=*/true); + CertVerifierBrowserTest::SetUp(); + } + + void SetUpCertVerifier() { + content::StoragePartition* partition = + browser()->profile()->GetDefaultStoragePartition(); + partition->GetNetworkContext()->SetCTLogListAlwaysTimelyForTesting(); + + ASSERT_TRUE(https_server()->Start()); + + net::CertVerifyResult verify_result; + verify_result.verified_cert = + net::ImportCertFromFile(net::GetTestCertsDirectory(), "may_2018.pem"); + ASSERT_TRUE(verify_result.verified_cert); + verify_result.is_issued_by_known_root = true; + verify_result.public_key_hashes.push_back( + GetSPKIHash(verify_result.verified_cert->cert_buffer())); + + mock_cert_verifier()->AddResultForCert( + https_server()->GetCertificate().get(), verify_result, net::OK); + } + + net::EmbeddedTestServer* https_server() { return &https_server_; } + + private: + net::EmbeddedTestServer https_server_; + + testing::NiceMock policy_provider_; +}; + +IN_PROC_BROWSER_TEST_F(CertificateTransparencyBrowserTest, EnforcedByDefault) { + SetUpCertVerifier(); + + // Normal non-exempt URL + ASSERT_TRUE(ui_test_utils::NavigateToURL( + browser(), https_server()->GetURL("/ssl/google.html"))); + + ssl_test_util::CheckSecurityState( + browser()->tab_strip_model()->GetActiveWebContents(), + net::CERT_STATUS_CERTIFICATE_TRANSPARENCY_REQUIRED, + security_state::DANGEROUS, + ssl_test_util::AuthState::SHOWING_INTERSTITIAL); +} + +IN_PROC_BROWSER_TEST_F(CertificateTransparencyBrowserTest, ExemptedHost) { + SetUpCertVerifier(); + + // URL exempted from SCT requirements + ASSERT_TRUE(ui_test_utils::NavigateToURL( + browser(), https_server()->GetURL("sct-exempted.bravesoftware.com", + "/ssl/google.html"))); + + ssl_test_util::CheckSecurityState( + browser()->tab_strip_model()->GetActiveWebContents(), + ssl_test_util::CertError::NONE, security_state::SECURE, + ssl_test_util::AuthState::NONE); +} diff --git a/chromium_src/chrome/browser/net/profile_network_context_service.cc b/chromium_src/chrome/browser/net/profile_network_context_service.cc new file mode 100644 index 000000000000..db48ebc2d07d --- /dev/null +++ b/chromium_src/chrome/browser/net/profile_network_context_service.cc @@ -0,0 +1,20 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ +static const char* kBraveCTExcludedHosts[] = { + // Critical endpoints that shouldn't require SCTs so they always work + "laptop-updates.brave.com", + "updates.bravesoftware.com", + "updates-cdn.bravesoftware.com", + // Test host for manual testing + "sct-exempted.bravesoftware.com", +}; + +#define BRAVE_PROFILE_NETWORK_CONTEXT_SERVICE_GET_CT_POLICY \ + for (const auto* host : kBraveCTExcludedHosts) { \ + excluded.push_back(host); \ + } + +#include "src/chrome/browser/net/profile_network_context_service.cc" +#undef BRAVE_PROFILE_NETWORK_CONTEXT_SERVICE_GET_CT_POLICY diff --git a/chromium_src/chrome/browser/net/system_network_context_manager.cc b/chromium_src/chrome/browser/net/system_network_context_manager.cc index f8de42a9e757..d43489ef6d6d 100644 --- a/chromium_src/chrome/browser/net/system_network_context_manager.cc +++ b/chromium_src/chrome/browser/net/system_network_context_manager.cc @@ -1,8 +1,13 @@ /* Copyright (c) 2019 The Brave Authors. All rights reserved. * This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ + * You can obtain one at https://mozilla.org/MPL/2.0/. */ +#include "build/branding_buildflags.h" #include "brave/services/network/public/cpp/system_request_handler.h" +// This is currently necessary in order to enable Certificate Transparency +// enforcement (brave-browser#22482). +#undef BUILDFLAG_INTERNAL_GOOGLE_CHROME_BRANDING +#define BUILDFLAG_INTERNAL_GOOGLE_CHROME_BRANDING() (1) #include "src/chrome/browser/net/system_network_context_manager.cc" diff --git a/patches/chrome-browser-net-profile_network_context_service.cc.patch b/patches/chrome-browser-net-profile_network_context_service.cc.patch new file mode 100644 index 000000000000..d41c86f04be1 --- /dev/null +++ b/patches/chrome-browser-net-profile_network_context_service.cc.patch @@ -0,0 +1,12 @@ +diff --git a/chrome/browser/net/profile_network_context_service.cc b/chrome/browser/net/profile_network_context_service.cc +index 81851bafbadd5b2c617b01bd6405d47cefa9f49f..2b6ce02cd639feac8560d0723902a5a815276ac7 100644 +--- a/chrome/browser/net/profile_network_context_service.cc ++++ b/chrome/browser/net/profile_network_context_service.cc +@@ -491,6 +491,7 @@ network::mojom::CTPolicyPtr ProfileNetworkContextService::GetCTPolicy() { + std::vector excluded_legacy_spkis( + TranslateStringArray(ct_excluded_legacy_spkis)); + ++ BRAVE_PROFILE_NETWORK_CONTEXT_SERVICE_GET_CT_POLICY + return network::mojom::CTPolicy::New(std::move(required), std::move(excluded), + std::move(excluded_spkis), + std::move(excluded_legacy_spkis)); diff --git a/test/BUILD.gn b/test/BUILD.gn index ff7df19a811b..a6a5091c88c9 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -1089,6 +1089,7 @@ test("brave_browser_tests") { if (!is_android) { sources += [ "//brave/browser/brave_resources_browsertest.cc", + "//brave/browser/ssl/certificate_transparency_browsertest.cc", "//brave/browser/ui/views/toolbar/wallet_button_notification_source_browsertest.cc", ] deps += [ From cf63b4a2fa5f6086b49770f4274dbac3cf81d34e Mon Sep 17 00:00:00 2001 From: Francois Marier Date: Wed, 5 Apr 2023 21:18:38 -0700 Subject: [PATCH 2/2] Re-enable upstream CT policy browser test --- test/filters/browser_tests.filter | 1 - 1 file changed, 1 deletion(-) diff --git a/test/filters/browser_tests.filter b/test/filters/browser_tests.filter index 95060db566e7..88366ff3198d 100644 --- a/test/filters/browser_tests.filter +++ b/test/filters/browser_tests.filter @@ -816,7 +816,6 @@ -CaptivePortalBlockingPageTest.* -CaptivePortalBrowserTest.* -CertificateReportingServiceBrowserTest.* --CertificateTransparencyPolicyTest.CertificateTransparencyEnforcementDisabledForUrls -ChromeAcceptHeaderTest.Check -ChromeBackForwardCacheBrowserTest.* -ChromeBrowsingDataLifetimeManagerShutdownTest.*