Introduce CompressionDictionaryTransportRequireKnownRootCert flag
We are observing potential issues with CompressionDictionaryTransport
feature that may be caused by network appliances that intercept network
traffic.
To investigate this issue, this CL introduces a new feature flag named
CompressionDictionaryTransportRequireKnownRootCert. If this feature is
enabled, Chromium will use stored shared dictionaries only when the
HTTPS connection's certificate is rooted by a well known root CA or when
the server is localhost.
Bug: 1413922
Change-Id: Iaaa9d4db9d335cde12632925c1e36b2a55a4cf9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5028336
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1232707}
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 43f1194..8e5ed0b2f 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -10586,6 +10586,15 @@
FEATURE_VALUE_TYPE(
network::features::kCompressionDictionaryTransportOverHttp1)},
+ {"enable-compression-dictionary-transport-require-known-root-cert",
+ flag_descriptions::kCompressionDictionaryTransportRequireKnownRootCertName,
+ flag_descriptions::
+ kCompressionDictionaryTransportRequireKnownRootCertDescription,
+ kOsAll,
+ FEATURE_VALUE_TYPE(
+ network::features::
+ kCompressionDictionaryTransportRequireKnownRootCert)},
+
{"enable-compute-pressure-rate-obfuscation-mitigation",
flag_descriptions::kComputePressureRateObfuscationMitigationName,
flag_descriptions::kComputePressureRateObfuscationMitigationDescription,
diff --git a/chrome/browser/browsing_data/browsing_data_model_browsertest.cc b/chrome/browser/browsing_data/browsing_data_model_browsertest.cc
index 5a7ddbc..f3f4044 100644
--- a/chrome/browser/browsing_data/browsing_data_model_browsertest.cc
+++ b/chrome/browser/browsing_data/browsing_data_model_browsertest.cc
@@ -281,7 +281,11 @@
{network::features::kCompressionDictionaryTransportOverHttp1, {}},
};
- std::vector<FeatureRef> disabled_features = {};
+ std::vector<FeatureRef> disabled_features = {
+ // Need to disable kCompressionDictionaryTransportRequireKnownRootCert
+ // because EmbeddedTestServer's certificate is not rooted at a standard
+ // CA root.
+ network::features::kCompressionDictionaryTransportRequireKnownRootCert};
#if BUILDFLAG(ENABLE_LIBRARY_CDMS)
enabled_features.push_back({media::kExternalClearKeyForTesting, {}});
diff --git a/chrome/browser/flag-metadata.json b/chrome/browser/flag-metadata.json
index 335124e..5ab4336e 100644
--- a/chrome/browser/flag-metadata.json
+++ b/chrome/browser/flag-metadata.json
@@ -2258,6 +2258,11 @@
"expiry_milestone": 130
},
{
+ "name": "enable-compression-dictionary-transport-require-known-root-cert",
+ "owners": [ "horo@chromium.org", "net-dev@chromium.org" ],
+ "expiry_milestone": 130
+ },
+ {
"name": "enable-compute-pressure-break-calibration-mitigation",
"owners": [ "arnaud.mandy@intel.com" ],
"expiry_milestone": 140
diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc
index 9eeed198..2ed8d531 100644
--- a/chrome/browser/flag_descriptions.cc
+++ b/chrome/browser/flag_descriptions.cc
@@ -922,6 +922,13 @@
"When this is enabled, Chromium can use stored shared dictionaries even "
"when the connection is using HTTP/1 for non-localhost requests.";
+const char kCompressionDictionaryTransportRequireKnownRootCertName[] =
+ "Compression dictionary transport require knwon root cert";
+const char kCompressionDictionaryTransportRequireKnownRootCertDescription[] =
+ "When this is enabled, Chromium can use stored shared dictionaries only "
+ "when the connection is using a well known root cert or when the server is "
+ "a localhost.";
+
const char kForceColorProfileSRGB[] = "sRGB";
const char kForceColorProfileP3[] = "Display P3 D65";
const char kForceColorProfileRec2020[] = "ITU-R BT.2020";
diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h
index 3a2ed8f..4bcbfed 100644
--- a/chrome/browser/flag_descriptions.h
+++ b/chrome/browser/flag_descriptions.h
@@ -515,6 +515,10 @@
extern const char kCompressionDictionaryTransportOverHttp1Name[];
extern const char kCompressionDictionaryTransportOverHttp1Description[];
+extern const char kCompressionDictionaryTransportRequireKnownRootCertName[];
+extern const char
+ kCompressionDictionaryTransportRequireKnownRootCertDescription[];
+
extern const char kUseDMSAAForTilesName[];
extern const char kUseDMSAAForTilesDescription[];
diff --git a/docs/experiments/compression-dictionary-transport.md b/docs/experiments/compression-dictionary-transport.md
index 65b14a2..72f7467 100644
--- a/docs/experiments/compression-dictionary-transport.md
+++ b/docs/experiments/compression-dictionary-transport.md
@@ -155,13 +155,18 @@
## Supported HTTP protocol
From Chrome 121, Chrome may not use stored shared dictionares when the
-connection is using HTTP/1 for non-localhost requests. This is a mitigation for
-a issue that some network appliances are interfering with HTTPS traffic by
-inspecting encrypted responses but failing to properly decode the shared
-dictionary encoded content.
+connection is using HTTP/1 for non-localhost requests. Also Chrome may not use
+shared dictionares when the HTTPS connection's certificate is not rooted by a
+well known root CA (eg: using a user installed root certificate). This is for
+an investigation for an issue that some network appliances are interfering with
+HTTPS traffic by inspecting encrypted responses but failing to properly decode
+the shared dictionary encoded content.
If you want to use shared dictionaries with HTTP/1, please enable
[chrome://flags/#enable-compression-dictionary-transport-over-http1][over-http1-flag].
+Also if you want to use shared dictionaries over the HTTPS connection which
+certificate is not rooted by a well known root CA, please disable
+[chrome://flags/#enable-compression-dictionary-transport-require-known-root-cert][require-known-root-ca-flag].
## Debugging
@@ -202,6 +207,7 @@
[backend-flag]: chrome://flags/#enable-compression-dictionary-transport-backend
[shared-zstd-flag]: chrome://flags/#enable-shared-zstd
[over-http1-flag]: chrome://flags/#enable-compression-dictionary-transport-over-http1
+[require-known-root-ca-flag]: chrome://flags/#enable-compression-dictionary-transport-require-known-root-cert
[shared_dictionary_readme]: ../../services/network/shared_dictionary/README.md#flags
[ot-blog]: https://developer.chrome.com/blog/origin-trials/
[ot-console]: https://developer.chrome.com/origintrials/#/trials/active
diff --git a/services/network/public/cpp/features.cc b/services/network/public/cpp/features.cc
index 100ca3d..1c61942 100644
--- a/services/network/public/cpp/features.cc
+++ b/services/network/public/cpp/features.cc
@@ -408,6 +408,13 @@
"CompressionDictionaryTransportOverHttp1",
base::FEATURE_DISABLED_BY_DEFAULT);
+// When this feature is enabled, Chromium will use stored shared dictionaries
+// only if the request URL is a localhost URL or the transport layer is using a
+// certificate rooted at a standard CA root.
+BASE_FEATURE(kCompressionDictionaryTransportRequireKnownRootCert,
+ "CompressionDictionaryTransportRequireKnownRootCert",
+ base::FEATURE_DISABLED_BY_DEFAULT);
+
BASE_FEATURE(kVisibilityAwareResourceScheduler,
"VisibilityAwareResourceScheduler",
base::FEATURE_DISABLED_BY_DEFAULT);
diff --git a/services/network/public/cpp/features.h b/services/network/public/cpp/features.h
index 93d9348..cbcf626 100644
--- a/services/network/public/cpp/features.h
+++ b/services/network/public/cpp/features.h
@@ -152,6 +152,8 @@
BASE_DECLARE_FEATURE(kCompressionDictionaryTransport);
COMPONENT_EXPORT(NETWORK_CPP)
BASE_DECLARE_FEATURE(kCompressionDictionaryTransportOverHttp1);
+COMPONENT_EXPORT(NETWORK_CPP)
+BASE_DECLARE_FEATURE(kCompressionDictionaryTransportRequireKnownRootCert);
// Enables visibility aware network service resource scheduler. When enabled,
// request may be prioritized or de-prioritized based on the visibility of
diff --git a/services/network/shared_dictionary/shared_dictionary_network_transaction.cc b/services/network/shared_dictionary/shared_dictionary_network_transaction.cc
index 1df02c92..c24a3d32 100644
--- a/services/network/shared_dictionary/shared_dictionary_network_transaction.cc
+++ b/services/network/shared_dictionary/shared_dictionary_network_transaction.cc
@@ -195,6 +195,13 @@
shared_dictionary_.reset();
return;
}
+ if (base::FeatureList::IsEnabled(
+ network::features::
+ kCompressionDictionaryTransportRequireKnownRootCert) &&
+ !cert_is_issued_by_known_root_ && !net::IsLocalhost(request_url)) {
+ shared_dictionary_.reset();
+ return;
+ }
// `is_shared_dictionary_read_allowed_callback_` triggers a notification of
// the shared dictionary usage to the browser process. So we need to call
diff --git a/services/network/shared_dictionary/shared_dictionary_network_transaction_unittest.cc b/services/network/shared_dictionary/shared_dictionary_network_transaction_unittest.cc
index ef56738..2fada8d 100644
--- a/services/network/shared_dictionary/shared_dictionary_network_transaction_unittest.cc
+++ b/services/network/shared_dictionary/shared_dictionary_network_transaction_unittest.cc
@@ -24,6 +24,7 @@
#include "services/network/shared_dictionary/shared_dictionary_manager.h"
#include "services/network/shared_dictionary/shared_dictionary_storage.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
namespace network {
@@ -380,6 +381,126 @@
EXPECT_EQ(kTestData, std::string(buf->data(), read_result));
}
+TEST_F(SharedDictionaryNetworkTransactionTest,
+ RequireKnownRootCertCheckFailure) {
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(
+ network::features::kCompressionDictionaryTransportRequireKnownRootCert);
+ DummySharedDictionaryManager manager(
+ base::MakeRefCounted<DummySharedDictionaryStorage>(
+ std::make_unique<DummySyncDictionary>(kTestDictionaryData)));
+
+ // Override MockTransaction to check that there is no sec-available-dictionary
+ // header.
+ net::MockTransaction new_mock_transaction = kBrotliDictionaryTestTransaction;
+ new_mock_transaction.handler =
+ kTestTransactionHandlerWithoutAvailableDictionary;
+ new_mock_transaction.transport_info.cert_is_issued_by_known_root = false;
+
+ net::AddMockTransaction(&new_mock_transaction);
+
+ net::MockHttpRequest request(new_mock_transaction);
+ SharedDictionaryNetworkTransaction transaction(manager,
+ CreateNetworkTransaction());
+ transaction.SetIsSharedDictionaryReadAllowedCallback(
+ base::BindRepeating([]() { return true; }));
+
+ net::TestCompletionCallback start_callback;
+ ASSERT_THAT(transaction.Start(&request, start_callback.callback(),
+ net::NetLogWithSource()),
+ net::test::IsError(net::ERR_IO_PENDING));
+ EXPECT_THAT(start_callback.WaitForResult(), net::test::IsError(net::OK));
+
+ scoped_refptr<net::IOBufferWithSize> buf =
+ base::MakeRefCounted<net::IOBufferWithSize>(kDefaultBufferSize);
+ net::TestCompletionCallback read_callback;
+ ASSERT_THAT(
+ transaction.Read(buf.get(), buf->size(), read_callback.callback()),
+ net::test::IsError(net::ERR_IO_PENDING));
+ int read_result = read_callback.WaitForResult();
+ EXPECT_THAT(read_result, kTestData.size());
+ EXPECT_EQ(kTestData, std::string(buf->data(), read_result));
+}
+
+TEST_F(SharedDictionaryNetworkTransactionTest,
+ RequireKnownRootCertCheckSuccess) {
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(
+ network::features::kCompressionDictionaryTransportRequireKnownRootCert);
+ DummySharedDictionaryManager manager(
+ base::MakeRefCounted<DummySharedDictionaryStorage>(
+ std::make_unique<DummySyncDictionary>(kTestDictionaryData)));
+
+ // The BrotliTestTransactionHandler `new_mock_transaction.handler` will check
+ // that the there is a correct sec-available-dictionary request header.
+ net::MockTransaction new_mock_transaction = kBrotliDictionaryTestTransaction;
+ new_mock_transaction.transport_info.cert_is_issued_by_known_root = true;
+
+ net::AddMockTransaction(&new_mock_transaction);
+
+ net::MockHttpRequest request(new_mock_transaction);
+ SharedDictionaryNetworkTransaction transaction(manager,
+ CreateNetworkTransaction());
+ transaction.SetIsSharedDictionaryReadAllowedCallback(
+ base::BindRepeating([]() { return true; }));
+
+ net::TestCompletionCallback start_callback;
+ ASSERT_THAT(transaction.Start(&request, start_callback.callback(),
+ net::NetLogWithSource()),
+ net::test::IsError(net::ERR_IO_PENDING));
+ EXPECT_THAT(start_callback.WaitForResult(), net::test::IsError(net::OK));
+
+ scoped_refptr<net::IOBufferWithSize> buf =
+ base::MakeRefCounted<net::IOBufferWithSize>(kDefaultBufferSize);
+ net::TestCompletionCallback read_callback;
+ ASSERT_THAT(
+ transaction.Read(buf.get(), buf->size(), read_callback.callback()),
+ net::test::IsError(net::ERR_IO_PENDING));
+ int read_result = read_callback.WaitForResult();
+ EXPECT_THAT(read_result, kTestData.size());
+ EXPECT_EQ(kTestData, std::string(buf->data(), read_result));
+}
+
+TEST_F(SharedDictionaryNetworkTransactionTest,
+ RequireKnownRootCertCheckSuccessForLocalhost) {
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitAndEnableFeature(
+ network::features::kCompressionDictionaryTransportRequireKnownRootCert);
+ DummySharedDictionaryManager manager(
+ base::MakeRefCounted<DummySharedDictionaryStorage>(
+ std::make_unique<DummySyncDictionary>(kTestDictionaryData)));
+
+ // The BrotliTestTransactionHandler `new_mock_transaction.handler` will check
+ // that the there is a correct sec-available-dictionary request header.
+ net::MockTransaction new_mock_transaction = kBrotliDictionaryTestTransaction;
+ new_mock_transaction.url = "http:///localhost:1234/test";
+ new_mock_transaction.transport_info.cert_is_issued_by_known_root = false;
+
+ net::AddMockTransaction(&new_mock_transaction);
+
+ net::MockHttpRequest request(new_mock_transaction);
+ SharedDictionaryNetworkTransaction transaction(manager,
+ CreateNetworkTransaction());
+ transaction.SetIsSharedDictionaryReadAllowedCallback(
+ base::BindRepeating([]() { return true; }));
+
+ net::TestCompletionCallback start_callback;
+ ASSERT_THAT(transaction.Start(&request, start_callback.callback(),
+ net::NetLogWithSource()),
+ net::test::IsError(net::ERR_IO_PENDING));
+ EXPECT_THAT(start_callback.WaitForResult(), net::test::IsError(net::OK));
+
+ scoped_refptr<net::IOBufferWithSize> buf =
+ base::MakeRefCounted<net::IOBufferWithSize>(kDefaultBufferSize);
+ net::TestCompletionCallback read_callback;
+ ASSERT_THAT(
+ transaction.Read(buf.get(), buf->size(), read_callback.callback()),
+ net::test::IsError(net::ERR_IO_PENDING));
+ int read_result = read_callback.WaitForResult();
+ EXPECT_THAT(read_result, kTestData.size());
+ EXPECT_EQ(kTestData, std::string(buf->data(), read_result));
+}
+
TEST_F(SharedDictionaryNetworkTransactionTest, NoMatchingDictionary) {
DummySharedDictionaryManager manager(
base::MakeRefCounted<DummySharedDictionaryStorage>(nullptr));
diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json
index b7153256..2d9f47d 100644
--- a/testing/variations/fieldtrial_testing_config.json
+++ b/testing/variations/fieldtrial_testing_config.json
@@ -4355,6 +4355,26 @@
]
}
],
+ "CompressionDictionaryTransportRequireKnownRootCert": [
+ {
+ "platforms": [
+ "android",
+ "chromeos",
+ "chromeos_lacros",
+ "linux",
+ "mac",
+ "windows"
+ ],
+ "experiments": [
+ {
+ "name": "Enabled",
+ "enable_features": [
+ "CompressionDictionaryTransportRequireKnownRootCert"
+ ]
+ }
+ ]
+ }
+ ],
"ConfigurableV8CodeCacheHotHours": [
{
"platforms": [
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 86cd368e..21f1c83 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -32511,6 +32511,8 @@
<int value="-1856902397" label="LoadingWithMojo:enabled"/>
<int value="-1856118202" label="AdvancedDocumentScanAPI:disabled"/>
<int value="-1855347512" label="FormControlsRefresh:disabled"/>
+ <int value="-1855339525"
+ label="CompressionDictionaryTransportRequireKnownRootCert:enabled"/>
<int value="-1854432127" label="ChromeHomePullToRefreshIphAtTop:disabled"/>
<int value="-1854372227" label="VrBrowsingExperimentalFeatures:enabled"/>
<int value="-1853877370" label="CastStreamingVp9:disabled"/>
@@ -32892,6 +32894,8 @@
<int value="-1660884825" label="ReadLaterNewBadgePromo:enabled"/>
<int value="-1659445938" label="RealboxSecondaryZeroSuggest:enabled"/>
<int value="-1658461830" label="SharedHighlightingRefinedBlocklist:enabled"/>
+ <int value="-1658154886"
+ label="CompressionDictionaryTransportRequireKnownRootCert:disabled"/>
<int value="-1657056532" label="FilesNewDirectoryTree:disabled"/>
<int value="-1656667928"
label="UseSyncInvalidationsForWalletAndOffer:disabled"/>