Control key pinning enforcement using a feature flag

Adds a feature flag that will be used to control static pinning enforcement. Flag is enabled by default on all platforms that currently enforce KP, and disabled by default on Android (where KP is currently not enforced). This also removes the BUILDFLAG check that disables KP on Android. This flag will be used to control the launch of KP enforcement on and can also serve as a killswitch for pinning. This CL also adds a timestamp in transport_security_static.pins, which is now used for freshness calculations (since BUILD_DATE cannot be reliably used on Android). Change-Id: Ib7564990ae153b775e5cc722d022cc420e022740 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3587708 Reviewed-by: David Benjamin <davidben@chromium.org> Auto-Submit: Carlos IL <carlosil@chromium.org> Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org> Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org> Commit-Queue: Carlos IL <carlosil@chromium.org> Reviewed-by: Clark DuVall <cduvall@chromium.org> Cr-Commit-Position: refs/heads/main@{#1002453}
chromium · May 12, 2022 · f12eac9 · f12eac9
1 parent 4256ab5
commit f12eac9
Show file tree

Hide file tree

Showing 24 changed files with 376 additions and 27 deletions.
diff --git a/content/browser/web_package/signed_exchange_request_handler_browsertest.cc b/content/browser/web_package/signed_exchange_request_handler_browsertest.cc
@@ -1839,6 +1839,10 @@ INSTANTIATE_TEST_SUITE_P(All,
 class SignedExchangePKPBrowserTest
     : public SignedExchangeRequestHandlerBrowserTest {
  public:
+  SignedExchangePKPBrowserTest() {
+    scoped_feature_list_.InitAndEnableFeature(
+        net::features::kStaticKeyPinningEnforcement);
+  }
   void SetUpOnMainThread() override {
     SignedExchangeRequestHandlerBrowserTest::SetUpOnMainThread();
 
@@ -1918,6 +1922,8 @@ class SignedExchangePKPBrowserTest
   // Only used when NetworkService is disabled. Accessed on IO thread.
   std::unique_ptr<net::ScopedTransportSecurityStateSource>
       transport_security_state_source_;
+
+  base::test::ScopedFeatureList scoped_feature_list_;
 };
 
 IN_PROC_BROWSER_TEST_P(SignedExchangePKPBrowserTest, PKPViolation) {

diff --git a/net/base/features.cc b/net/base/features.cc
@@ -293,5 +293,15 @@ const base::Feature kClampCookieExpiryTo400Days(
     "ClampCookieExpiryTo400Days",
     base::FEATURE_DISABLED_BY_DEFAULT);
 
+#if BUILDFLAG(IS_ANDROID)
+const base::Feature kStaticKeyPinningEnforcement(
+    "StaticKeyPinningEnforcement",
+    base::FEATURE_DISABLED_BY_DEFAULT);
+#else
+const base::Feature kStaticKeyPinningEnforcement(
+    "StaticKeyPinningEnforcement",
+    base::FEATURE_ENABLED_BY_DEFAULT);
+#endif
+
 }  // namespace features
 }  // namespace net
diff --git a/net/base/features.h b/net/base/features.h
@@ -430,6 +430,9 @@ NET_EXPORT extern const base::Feature kSandboxHttpCache;
 // future.
 NET_EXPORT extern const base::Feature kClampCookieExpiryTo400Days;
 
+// Controls whether static key pinning is enforced.
+NET_EXPORT extern const base::Feature kStaticKeyPinningEnforcement;
+
 }  // namespace features
 }  // namespace net
 

diff --git a/net/http/transport_security_state.cc b/net/http/transport_security_state.cc
@@ -413,8 +413,7 @@ TransportSecurityState::TransportSecurityState(
           features::kPartitionExpectCTStateByNetworkIsolationKey)) {
 // Static pinning is only enabled for official builds to make sure that
 // others don't end up with pins that cannot be easily updated.
-#if !BUILDFLAG(GOOGLE_CHROME_BRANDING) || BUILDFLAG(IS_ANDROID) || \
-    BUILDFLAG(IS_IOS)
+#if !BUILDFLAG(GOOGLE_CHROME_BRANDING) || BUILDFLAG(IS_IOS)
   enable_static_pins_ = false;
   enable_static_expect_ct_ = false;
 #endif
@@ -1179,8 +1178,10 @@ bool TransportSecurityState::GetStaticPKPState(const std::string& host,
                                                PKPState* pkp_result) const {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
-  if (!enable_static_pins_ || !IsStaticPKPListTimely())
+  if (!enable_static_pins_ || !IsStaticPKPListTimely() ||
+      !base::FeatureList::IsEnabled(features::kStaticKeyPinningEnforcement)) {
     return false;
+  }
 
   PreloadResult result;
   if (host_pins_.has_value()) {
@@ -1641,10 +1642,18 @@ bool TransportSecurityState::IsCTLogListTimely() const {
 }
 
 bool TransportSecurityState::IsStaticPKPListTimely() const {
+  if (pins_list_always_timely_for_testing_) {
+    return true;
+  }
+
   // If the list has not been updated via component updater, freshness depends
-  // on build freshness.
+  // on the compiled-in list freshness.
   if (!host_pins_.has_value()) {
-    return IsBuildTimely();
+#if BUILDFLAG(INCLUDE_TRANSPORT_SECURITY_STATE_PRELOAD_LIST)
+    return (base::Time::Now() - kPinsListTimestamp).InDays() < 70;
+#else
+    return false;
+#endif
   }
   DCHECK(!key_pins_list_last_update_time_.is_null());
   // Else, we use the last update time.

diff --git a/net/http/transport_security_state.h b/net/http/transport_security_state.h
@@ -642,6 +642,12 @@ class NET_EXPORT TransportSecurityState {
   // The number of cached ExpectCTState entries.
   size_t num_expect_ct_entries_for_testing() const;
 
+  // Sets whether pinning list timestamp freshness should be ignored for
+  // testing.
+  void SetPinningListAlwaysTimelyForTesting(bool always_timely) {
+    pins_list_always_timely_for_testing_ = always_timely;
+  }
+
   // The number of cached STSState entries.
   size_t num_sts_entries() const;
 
@@ -813,6 +819,8 @@ class NET_EXPORT TransportSecurityState {
   base::Time key_pins_list_last_update_time_;
   std::vector<PinSet> pinsets_;
 
+  bool pins_list_always_timely_for_testing_ = false;
+
   THREAD_CHECKER(thread_checker_);
 };
 

diff --git a/net/http/transport_security_state_static.pins b/net/http/transport_security_state_static.pins
@@ -43,6 +43,10 @@
 #   hash function for preloaded entries again (we have already done so once).
 #
 
+# Last updated: 2022-04-25 00:00 UTC
+PinsListTimestamp
+1650844800
+
 TestSPKI
 sha256/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
 

diff --git a/net/http/transport_security_state_static.template b/net/http/transport_security_state_static.template
@@ -11,8 +11,12 @@
 
 #include <iterator>
 
+#include "base/time/time.h"
 #include "net/http/transport_security_state_source.h"
 
+// This is the time at which the key pins list was last updated.
+const base::Time kPinsListTimestamp = base::Time::FromTimeT([[PINS_LIST_TIMESTAMP]]);
+
 // These are SubjectPublicKeyInfo hashes for public key pinning. The
 // hashes are SHA256 digests.
 [[SPKI_HASHES]]

diff --git a/net/http/transport_security_state_static_unittest.pins b/net/http/transport_security_state_static_unittest.pins
@@ -5,6 +5,12 @@
 # This is a HSTS pins file used by the unittests. For more information on the
 # content and format see the comments in transport_security_state_static.pins.
 
+# Having a timestamp is required by the parser, but this timestamp is not
+# otherwise used by tests (since they override the behavior by setting
+# pins_list_always_timely_for_testing_
+PinsListTimestamp
+0
+
 TestSPKI1
 sha256/AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQE=
 

diff --git a/net/http/transport_security_state_static_unittest_default.pins b/net/http/transport_security_state_static_unittest_default.pins
@@ -5,6 +5,12 @@
 # For use with transport_security_state_static_unittest_default.json.
 # The format of this file is identical to transport_security_state_static.pins.
 
+# Having a timestamp is required by the parser, but this timestamp is not
+# otherwise used by tests (since they override the behavior by setting
+# pins_list_always_timely_for_testing_
+PinsListTimestamp
+0
+
 TestSPKI1
 sha256/w3y7Yg3RzkAyhCeBoLHm71YRnuuUW87AAR/DVpLMTw4=