0

Remove CertVerifierService feature

The feature is enabled by default and hasn't caused any meaningful
regression, so this removes the CertVerifierService feature.

This requires un-parameterizing some tests that were parameterized on
the CertVerifierService feature.

CertVerifierParams in the NetworkContext was a union of
CertVerifierServiceRemoteParams and CertVerifierCreationParams, but now
the Network Service has no use for the CertVerifierCreationParams as it
won't be creating a CertVerifier in-process anymore. So the union is
removed and NetworkContextParams just contains a
CertVerifierServiceRemoteParams directly, which requires some type
changes all over the codebase.

Bug: 1015134, 1111472
Change-Id: I722346f20dda6161b40cfec039d6adff35878ef5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2674530
Commit-Queue: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#851400}
This commit is contained in:
Matthew Denton
2021-02-06 01:58:54 +00:00
committed by Chromium LUCI CQ
parent 76a2ec46d0
commit 82c211f615
20 changed files with 155 additions and 585 deletions

@ -139,8 +139,7 @@ void ChromeBrowserMainPartsMac::PostMainMessageLoopStart() {
MacStartupProfiler::POST_MAIN_MESSAGE_LOOP_START);
ChromeBrowserMainPartsPosix::PostMainMessageLoopStart();
if (base::FeatureList::IsEnabled(network::features::kCertVerifierService) &&
base::FeatureList::IsEnabled(
if (base::FeatureList::IsEnabled(
net::features::kCertVerifierBuiltinFeature)) {
net::InitializeTrustStoreMacCache();
}

@ -516,12 +516,10 @@ bool IsCertInCertificateList(
// Allows testing if user policy provided trust roots take effect, without
// having device policy.
// The parameter specifies whether the CertVerifierService is enabled.
class PolicyProvidedCertsRegularUserTest
: public InProcessBrowserTest,
public ::testing::WithParamInterface<bool> {
class PolicyProvidedCertsRegularUserTest : public InProcessBrowserTest {
protected:
PolicyProvidedCertsRegularUserTest() = default;
~PolicyProvidedCertsRegularUserTest() = default;
~PolicyProvidedCertsRegularUserTest() override = default;
void SetUpCommandLine(base::CommandLine* command_line) override {
InProcessBrowserTest::SetUpCommandLine(command_line);
@ -530,14 +528,6 @@ class PolicyProvidedCertsRegularUserTest
}
void SetUpInProcessBrowserTestFixture() override {
if (GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
network::features::kCertVerifierService);
} else {
scoped_feature_list_.InitAndDisableFeature(
network::features::kCertVerifierService);
}
InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
ASSERT_NO_FATAL_FAILURE(
@ -562,8 +552,6 @@ class PolicyProvidedCertsRegularUserTest
MultiProfilePolicyProviderHelper multi_profile_policy_helper_;
base::test::ScopedFeatureList scoped_feature_list_;
UserPolicyCertsHelper user_policy_certs_helper_;
// A NSSCertDatabase is needed for the tests that do something with
@ -572,7 +560,7 @@ class PolicyProvidedCertsRegularUserTest
std::unique_ptr<net::NSSCertDatabase> test_nss_cert_db_;
};
IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsRegularUserTest, TrustAnchorApplied) {
IN_PROC_BROWSER_TEST_F(PolicyProvidedCertsRegularUserTest, TrustAnchorApplied) {
user_policy_certs_helper_.SetRootCertONCUserPolicy(
multi_profile_policy_helper_.profile_1(),
multi_profile_policy_helper_.policy_for_profile_1());
@ -581,7 +569,7 @@ IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsRegularUserTest, TrustAnchorApplied) {
user_policy_certs_helper_.server_cert()));
}
IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsRegularUserTest,
IN_PROC_BROWSER_TEST_F(PolicyProvidedCertsRegularUserTest,
PrimaryProfileTrustAnchorDoesNotLeak) {
ASSERT_NO_FATAL_FAILURE(multi_profile_policy_helper_.CreateSecondProfile());
@ -596,7 +584,7 @@ IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsRegularUserTest,
user_policy_certs_helper_.server_cert()));
}
IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsRegularUserTest,
IN_PROC_BROWSER_TEST_F(PolicyProvidedCertsRegularUserTest,
SecondaryProfileTrustAnchorDoesNotLeak) {
ASSERT_NO_FATAL_FAILURE(multi_profile_policy_helper_.CreateSecondProfile());
@ -618,7 +606,7 @@ IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsRegularUserTest,
user_policy_certs_helper_.server_cert()));
}
IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsRegularUserTest,
IN_PROC_BROWSER_TEST_F(PolicyProvidedCertsRegularUserTest,
UntrustedIntermediateAuthorityApplied) {
// Sanity check: Apply ONC policy which does not mention the intermediate
// authority.
@ -641,7 +629,7 @@ IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsRegularUserTest,
user_policy_certs_helper_.server_cert_by_intermediate()));
}
IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsRegularUserTest,
IN_PROC_BROWSER_TEST_F(PolicyProvidedCertsRegularUserTest,
AuthorityAvailableThroughNetworkCertLoader) {
// Set |NetworkCertLoader| to use a test NSS database - otherwise, it is not
// properly initialized because |UserSessionManager| only sets the primary
@ -668,15 +656,10 @@ IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsRegularUserTest,
chromeos::NetworkCertLoader::Get()->authority_certs()));
}
INSTANTIATE_TEST_SUITE_P(All,
PolicyProvidedCertsRegularUserTest,
::testing::Bool());
// Base class for testing policy-provided trust roots with device-local
// accounts. Needs device policy.
class PolicyProvidedCertsDeviceLocalAccountTest
: public DevicePolicyCrosBrowserTest,
public ::testing::WithParamInterface<bool> {
: public DevicePolicyCrosBrowserTest {
public:
PolicyProvidedCertsDeviceLocalAccountTest() {
// Use the same testing slot as private and public slot for testing.
@ -691,14 +674,6 @@ class PolicyProvidedCertsDeviceLocalAccountTest
virtual void SetupDevicePolicy() = 0;
void SetUpInProcessBrowserTestFixture() override {
if (GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
network::features::kCertVerifierService);
} else {
scoped_feature_list_.InitAndDisableFeature(
network::features::kCertVerifierService);
}
DevicePolicyCrosBrowserTest::SetUpInProcessBrowserTestFixture();
ASSERT_NO_FATAL_FAILURE(user_policy_certs_helper_.Initialize());
@ -725,8 +700,6 @@ class PolicyProvidedCertsDeviceLocalAccountTest
command_line->AppendSwitch(chromeos::switches::kOobeSkipPostLogin);
}
base::test::ScopedFeatureList scoped_feature_list_;
chromeos::LocalPolicyTestServerMixin local_policy_mixin_{&mixin_host_};
const AccountId device_local_account_id_ =
@ -782,7 +755,7 @@ class PolicyProvidedCertsPublicSessionTest
// TODO(https://crbug.com/874831): Re-enable this after the source of the
// flakiness has been identified.
IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsPublicSessionTest,
IN_PROC_BROWSER_TEST_F(PolicyProvidedCertsPublicSessionTest,
DISABLED_AllowedInPublicSession) {
StartLogin();
chromeos::test::WaitForPrimaryUserSessionStart();
@ -799,28 +772,10 @@ IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsPublicSessionTest,
user_policy_certs_helper_.server_cert()));
}
INSTANTIATE_TEST_SUITE_P(All,
PolicyProvidedCertsPublicSessionTest,
::testing::Bool());
class PolicyProvidedCertsOnUserSessionInitTest
: public LoginPolicyTestBase,
public ::testing::WithParamInterface<bool> {
class PolicyProvidedCertsOnUserSessionInitTest : public LoginPolicyTestBase {
protected:
PolicyProvidedCertsOnUserSessionInitTest() {}
void SetUpInProcessBrowserTestFixture() override {
if (GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
network::features::kCertVerifierService);
} else {
scoped_feature_list_.InitAndDisableFeature(
network::features::kCertVerifierService);
}
LoginPolicyTestBase::SetUpInProcessBrowserTestFixture();
}
void GetMandatoryPoliciesValue(base::DictionaryValue* policy) const override {
std::string user_policy_blob = GetTestCertsFileContents(kRootCaCertOnc);
policy->SetKey(key::kOpenNetworkConfiguration,
@ -843,13 +798,12 @@ class PolicyProvidedCertsOnUserSessionInitTest
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
DISALLOW_COPY_AND_ASSIGN(PolicyProvidedCertsOnUserSessionInitTest);
};
// Verifies that the policy-provided trust root is active as soon as the user
// session starts.
IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsOnUserSessionInitTest,
IN_PROC_BROWSER_TEST_F(PolicyProvidedCertsOnUserSessionInitTest,
TrustAnchorsAvailableImmediatelyAfterSessionStart) {
// Load the certificate which is only OK if the policy-provided authority is
// actually trusted.
@ -868,27 +822,13 @@ IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsOnUserSessionInitTest,
EXPECT_EQ(net::OK, VerifyTestServerCert(active_user_profile(), server_cert));
}
INSTANTIATE_TEST_SUITE_P(All,
PolicyProvidedCertsOnUserSessionInitTest,
::testing::Bool());
// Testing policy-provided client cert import.
class PolicyProvidedClientCertsTest
: public DevicePolicyCrosBrowserTest,
public ::testing::WithParamInterface<bool> {
class PolicyProvidedClientCertsTest : public DevicePolicyCrosBrowserTest {
protected:
PolicyProvidedClientCertsTest() {}
~PolicyProvidedClientCertsTest() override {}
void SetUpInProcessBrowserTestFixture() override {
if (GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
network::features::kCertVerifierService);
} else {
scoped_feature_list_.InitAndDisableFeature(
network::features::kCertVerifierService);
}
// Set up the mock policy provider.
EXPECT_CALL(provider_, IsInitializationComplete(testing::_))
.WillRepeatedly(testing::Return(true));
@ -919,11 +859,10 @@ class PolicyProvidedClientCertsTest
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
MockConfigurationPolicyProvider provider_;
};
IN_PROC_BROWSER_TEST_P(PolicyProvidedClientCertsTest, ClientCertsImported) {
IN_PROC_BROWSER_TEST_F(PolicyProvidedClientCertsTest, ClientCertsImported) {
// Sanity check: we don't expect the client certificate to be present before
// setting the user ONC policy.
EXPECT_FALSE(
@ -934,8 +873,6 @@ IN_PROC_BROWSER_TEST_P(PolicyProvidedClientCertsTest, ClientCertsImported) {
IsCertInNSSDatabase(browser()->profile(), kClientCertSubjectCommonName));
}
INSTANTIATE_TEST_SUITE_P(All, PolicyProvidedClientCertsTest, ::testing::Bool());
// TODO(https://crbug.com/874937): Add a test case for a kiosk session.
// Class for testing policy-provided extensions in the sign-in profile.
@ -943,8 +880,7 @@ INSTANTIATE_TEST_SUITE_P(All, PolicyProvidedClientCertsTest, ::testing::Bool());
// |kSigninScreenExtension1|. Force-installs |kSigninScreenExtension1| and
// |kSigninScreenExtension2| into the sign-in profile.
class PolicyProvidedCertsForSigninExtensionTest
: public SigninProfileExtensionsPolicyTestBase,
public ::testing::WithParamInterface<bool> {
: public SigninProfileExtensionsPolicyTestBase {
protected:
// Use DEV channel as sign-in screen extensions are currently usable there.
PolicyProvidedCertsForSigninExtensionTest()
@ -952,14 +888,6 @@ class PolicyProvidedCertsForSigninExtensionTest
~PolicyProvidedCertsForSigninExtensionTest() override = default;
void SetUpInProcessBrowserTestFixture() override {
if (GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
network::features::kCertVerifierService);
} else {
scoped_feature_list_.InitAndDisableFeature(
network::features::kCertVerifierService);
}
// Apply |kRootCaCert| for |kSigninScreenExtension1| in Device ONC policy.
base::FilePath test_certs_path = GetTestCertsPath();
std::string x509_contents;
@ -1055,8 +983,6 @@ class PolicyProvidedCertsForSigninExtensionTest
return onc_dict;
}
base::test::ScopedFeatureList scoped_feature_list_;
DISALLOW_COPY_AND_ASSIGN(PolicyProvidedCertsForSigninExtensionTest);
}; // namespace policy
@ -1070,7 +996,7 @@ class PolicyProvidedCertsForSigninExtensionTest
// Verification of all these aspects has been intentionally put into one test,
// so if the verification result leaks (e.g. due to accidentally reusing
// caches), the test is able to catch that.
IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsForSigninExtensionTest,
IN_PROC_BROWSER_TEST_F(PolicyProvidedCertsForSigninExtensionTest,
ActiveOnlyInSelectedExtension) {
chromeos::OobeScreenWaiter(chromeos::OobeBaseTest::GetFirstSigninScreen())
.Wait();
@ -1111,8 +1037,4 @@ IN_PROC_BROWSER_TEST_P(PolicyProvidedCertsForSigninExtensionTest,
server_cert_));
}
INSTANTIATE_TEST_SUITE_P(All,
PolicyProvidedCertsForSigninExtensionTest,
::testing::Bool());
} // namespace policy

@ -82,22 +82,10 @@ class NetLogPlatformBrowserTestBase : public PlatformBrowserTest {
};
// This is an integration test to ensure that CertVerifyProc netlog events
// continue to be logged once cert verification is moved out of the network
// service process. (See crbug.com/1015134 and crbug.com/1040681.)
class CertVerifyProcNetLogBrowserTest
: public NetLogPlatformBrowserTestBase,
public testing::WithParamInterface<bool> {
// continue to be logged even though cert verification is no longer performed in
// the network process.
class CertVerifyProcNetLogBrowserTest : public NetLogPlatformBrowserTestBase {
public:
void SetUpInProcessBrowserTestFixture() override {
if (GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
network::features::kCertVerifierService);
} else {
scoped_feature_list_.InitAndDisableFeature(
network::features::kCertVerifierService);
}
}
void SetUpOnMainThread() override {
PlatformBrowserTest::SetUpOnMainThread();
@ -142,11 +130,10 @@ class CertVerifyProcNetLogBrowserTest
const std::string kTestHost = "netlog-example.a.test";
protected:
base::test::ScopedFeatureList scoped_feature_list_;
net::EmbeddedTestServer https_server_{net::EmbeddedTestServer::TYPE_HTTPS};
};
IN_PROC_BROWSER_TEST_P(CertVerifyProcNetLogBrowserTest, Test) {
IN_PROC_BROWSER_TEST_F(CertVerifyProcNetLogBrowserTest, Test) {
ASSERT_TRUE(https_server_.Start());
// Request using a unique host name to ensure that the cert verification wont
@ -164,7 +151,3 @@ IN_PROC_BROWSER_TEST_P(CertVerifyProcNetLogBrowserTest, Test) {
base::RunLoop().RunUntilIdle();
content::FlushNetworkServiceInstanceForTesting();
}
INSTANTIATE_TEST_SUITE_P(CertVerifierService,
CertVerifyProcNetLogBrowserTest,
::testing::Bool());

@ -512,21 +512,13 @@ class ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest
public testing::WithParamInterface<bool> {
public:
void SetUpInProcessBrowserTestFixture() override {
std::vector<base::Feature> enabled_features, disabled_features;
if (use_builtin_cert_verifier()) {
enabled_features.push_back(net::features::kCertVerifierBuiltinFeature);
} else {
disabled_features.push_back(net::features::kCertVerifierBuiltinFeature);
}
if (enable_cert_verifier_service()) {
enabled_features.push_back(network::features::kCertVerifierService);
test_cert_verifier_service_factory_.emplace();
content::SetCertVerifierServiceFactoryForTesting(
&test_cert_verifier_service_factory_.value());
} else {
disabled_features.push_back(network::features::kCertVerifierService);
}
scoped_feature_list_.InitWithFeatures(enabled_features, disabled_features);
scoped_feature_list_.InitWithFeatureState(
net::features::kCertVerifierBuiltinFeature,
use_builtin_cert_verifier());
content::SetCertVerifierServiceFactoryForTesting(
&test_cert_verifier_service_factory_);
policy::PolicyTest::SetUpInProcessBrowserTestFixture();
}
@ -535,22 +527,20 @@ class ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest
}
void SetUpOnMainThread() override {
if (enable_cert_verifier_service()) {
test_cert_verifier_service_factory_->ReleaseAllCertVerifierParams();
}
test_cert_verifier_service_factory_.ReleaseAllCertVerifierParams();
}
void ExpectUseBuiltinCertVerifierCorrectUsingCertVerifierService(
void ExpectUseBuiltinCertVerifierCorrect(
network::mojom::CertVerifierCreationParams::CertVerifierImpl
use_builtin_cert_verifier) {
ASSERT_TRUE(enable_cert_verifier_service());
ASSERT_TRUE(test_cert_verifier_service_factory_);
EXPECT_EQ(1ul, test_cert_verifier_service_factory_->num_captured_params());
ASSERT_EQ(1ul, test_cert_verifier_service_factory_.num_captured_params());
ASSERT_TRUE(test_cert_verifier_service_factory_.GetParamsAtIndex(0)
->creation_params);
EXPECT_EQ(use_builtin_cert_verifier,
test_cert_verifier_service_factory_->GetParamsAtIndex(0)
test_cert_verifier_service_factory_.GetParamsAtIndex(0)
->creation_params->use_builtin_cert_verifier);
// Send it to the actual CertVerifierServiceFactory.
test_cert_verifier_service_factory_->ReleaseNextCertVerifierParams();
test_cert_verifier_service_factory_.ReleaseNextCertVerifierParams();
}
Profile* CreateNewProfile() {
@ -566,104 +556,22 @@ class ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest
}
bool use_builtin_cert_verifier() const { return GetParam(); }
bool enable_cert_verifier_service() const {
return enable_cert_verifier_service_;
}
void set_enable_cert_verifier_service(bool enable_cv_service) {
enable_cert_verifier_service_ = enable_cv_service;
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
bool enable_cert_verifier_service_ = false;
// Used if enable_cert_verifier_service() returns true.
base::Optional<cert_verifier::TestCertVerifierServiceFactoryImpl>
cert_verifier::TestCertVerifierServiceFactoryImpl
test_cert_verifier_service_factory_;
};
IN_PROC_BROWSER_TEST_P(
ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest,
Test) {
ProfileNetworkContextService* profile_network_context_service =
ProfileNetworkContextServiceFactory::GetForContext(browser()->profile());
base::FilePath empty_relative_partition_path;
{
network::mojom::NetworkContextParams network_context_params;
network::mojom::CertVerifierCreationParams cert_verifier_creation_params;
profile_network_context_service->ConfigureNetworkContextParams(
/*in_memory=*/false, empty_relative_partition_path,
&network_context_params, &cert_verifier_creation_params);
EXPECT_EQ(use_builtin_cert_verifier()
? network::mojom::CertVerifierCreationParams::
CertVerifierImpl::kBuiltin
: network::mojom::CertVerifierCreationParams::
CertVerifierImpl::kSystem,
cert_verifier_creation_params.use_builtin_cert_verifier);
}
#if BUILDFLAG(BUILTIN_CERT_VERIFIER_POLICY_SUPPORTED)
// If the BuiltinCertificateVerifierEnabled policy is set it should override
// the feature flag.
policy::PolicyMap policies;
SetPolicy(&policies, policy::key::kBuiltinCertificateVerifierEnabled,
base::Value(true));
UpdateProviderPolicy(policies);
{
network::mojom::NetworkContextParams network_context_params;
network::mojom::CertVerifierCreationParams cert_verifier_creation_params;
profile_network_context_service->ConfigureNetworkContextParams(
/*in_memory=*/false, empty_relative_partition_path,
&network_context_params, &cert_verifier_creation_params);
EXPECT_EQ(
network::mojom::CertVerifierCreationParams::CertVerifierImpl::kBuiltin,
cert_verifier_creation_params.use_builtin_cert_verifier);
}
SetPolicy(&policies, policy::key::kBuiltinCertificateVerifierEnabled,
base::Value(false));
UpdateProviderPolicy(policies);
{
network::mojom::NetworkContextParams network_context_params;
network::mojom::CertVerifierCreationParams cert_verifier_creation_params;
profile_network_context_service->ConfigureNetworkContextParams(
/*in_memory=*/false, empty_relative_partition_path,
&network_context_params, &cert_verifier_creation_params);
EXPECT_EQ(
network::mojom::CertVerifierCreationParams::CertVerifierImpl::kSystem,
cert_verifier_creation_params.use_builtin_cert_verifier);
}
#endif // BUILDFLAG(BUILTIN_CERT_VERIFIER_POLICY_SUPPORTED)
}
INSTANTIATE_TEST_SUITE_P(
All,
ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest,
::testing::Bool());
class
ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTestWithService
: public ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest {
public:
ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTestWithService() {
set_enable_cert_verifier_service(true);
}
~ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTestWithService()
override = default;
};
IN_PROC_BROWSER_TEST_P(
ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTestWithService,
Test) {
{
content::BrowserContext::GetDefaultStoragePartition(CreateNewProfile())
->GetNetworkContext();
ExpectUseBuiltinCertVerifierCorrectUsingCertVerifierService(
ExpectUseBuiltinCertVerifierCorrect(
use_builtin_cert_verifier()
? network::mojom::CertVerifierCreationParams::CertVerifierImpl::
kBuiltin
@ -683,7 +591,7 @@ IN_PROC_BROWSER_TEST_P(
content::BrowserContext::GetDefaultStoragePartition(CreateNewProfile())
->GetNetworkContext();
ExpectUseBuiltinCertVerifierCorrectUsingCertVerifierService(
ExpectUseBuiltinCertVerifierCorrect(
network::mojom::CertVerifierCreationParams::CertVerifierImpl::kBuiltin);
}
@ -695,7 +603,7 @@ IN_PROC_BROWSER_TEST_P(
content::BrowserContext::GetDefaultStoragePartition(CreateNewProfile())
->GetNetworkContext();
ExpectUseBuiltinCertVerifierCorrectUsingCertVerifierService(
ExpectUseBuiltinCertVerifierCorrect(
network::mojom::CertVerifierCreationParams::CertVerifierImpl::kSystem);
}
#endif // BUILDFLAG(BUILTIN_CERT_VERIFIER_POLICY_SUPPORTED)
@ -703,7 +611,7 @@ IN_PROC_BROWSER_TEST_P(
INSTANTIATE_TEST_SUITE_P(
All,
ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTestWithService,
ProfileNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest,
::testing::Bool());
#endif // BUILDFLAG(BUILTIN_CERT_VERIFIER_FEATURE_SUPPORTED)

@ -484,12 +484,10 @@ INSTANTIATE_TEST_SUITE_P(
#if BUILDFLAG(BUILTIN_CERT_VERIFIER_FEATURE_SUPPORTED)
class SystemNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest
: public policy::PolicyTest,
public testing::WithParamInterface<std::tuple<bool, bool>> {
public testing::WithParamInterface<bool> {
public:
SystemNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest() {
bool use_builtin_cert_verifier;
std::tie(use_builtin_cert_verifier, enable_cert_verification_service_) =
GetParam();
bool use_builtin_cert_verifier = GetParam();
cert_verifier_impl_ = use_builtin_cert_verifier
? network::mojom::CertVerifierCreationParams::
CertVerifierImpl::kBuiltin
@ -498,22 +496,14 @@ class SystemNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest
}
void SetUpInProcessBrowserTestFixture() override {
std::vector<base::Feature> enabled_features, disabled_features;
if (cert_verifier_impl_ == network::mojom::CertVerifierCreationParams::
CertVerifierImpl::kBuiltin) {
enabled_features.push_back(net::features::kCertVerifierBuiltinFeature);
} else {
disabled_features.push_back(net::features::kCertVerifierBuiltinFeature);
}
if (enable_cert_verification_service_) {
enabled_features.push_back(network::features::kCertVerifierService);
test_cert_verifier_service_factory_.emplace();
content::SetCertVerifierServiceFactoryForTesting(
&test_cert_verifier_service_factory_.value());
} else {
disabled_features.push_back(network::features::kCertVerifierService);
}
scoped_feature_list_.InitWithFeatures(enabled_features, disabled_features);
scoped_feature_list_.InitWithFeatureState(
net::features::kCertVerifierBuiltinFeature,
cert_verifier_impl_ == network::mojom::CertVerifierCreationParams::
CertVerifierImpl::kBuiltin);
content::SetCertVerifierServiceFactoryForTesting(
&test_cert_verifier_service_factory_);
policy::PolicyTest::SetUpInProcessBrowserTestFixture();
}
@ -522,9 +512,7 @@ class SystemNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest
}
void SetUpOnMainThread() override {
if (enable_cert_verification_service_) {
test_cert_verifier_service_factory_->ReleaseAllCertVerifierParams();
}
test_cert_verifier_service_factory_.ReleaseAllCertVerifierParams();
}
void ExpectUseBuiltinCertVerifierCorrect(
@ -533,27 +521,14 @@ class SystemNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest
use_builtin_cert_verifier) {
ASSERT_TRUE(network_context_params_ptr);
ASSERT_TRUE(network_context_params_ptr->cert_verifier_params);
if (enable_cert_verification_service_) {
EXPECT_TRUE(
network_context_params_ptr->cert_verifier_params->is_remote_params());
ASSERT_TRUE(test_cert_verifier_service_factory_);
ASSERT_EQ(1ul,
test_cert_verifier_service_factory_->num_captured_params());
ASSERT_TRUE(test_cert_verifier_service_factory_->GetParamsAtIndex(0)
->creation_params);
EXPECT_EQ(use_builtin_cert_verifier,
test_cert_verifier_service_factory_->GetParamsAtIndex(0)
->creation_params->use_builtin_cert_verifier);
// Send it to the actual CertVerifierServiceFactory.
test_cert_verifier_service_factory_->ReleaseNextCertVerifierParams();
} else {
ASSERT_TRUE(network_context_params_ptr->cert_verifier_params
->is_creation_params());
EXPECT_EQ(use_builtin_cert_verifier,
network_context_params_ptr->cert_verifier_params
->get_creation_params()
->use_builtin_cert_verifier);
}
ASSERT_EQ(1ul, test_cert_verifier_service_factory_.num_captured_params());
ASSERT_TRUE(test_cert_verifier_service_factory_.GetParamsAtIndex(0)
->creation_params);
EXPECT_EQ(use_builtin_cert_verifier,
test_cert_verifier_service_factory_.GetParamsAtIndex(0)
->creation_params->use_builtin_cert_verifier);
// Send it to the actual CertVerifierServiceFactory.
test_cert_verifier_service_factory_.ReleaseNextCertVerifierParams();
}
network::mojom::CertVerifierCreationParams::CertVerifierImpl
@ -564,11 +539,9 @@ class SystemNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest
private:
network::mojom::CertVerifierCreationParams::CertVerifierImpl
cert_verifier_impl_;
bool enable_cert_verification_service_;
base::test::ScopedFeatureList scoped_feature_list_;
// Used if |enable_cert_verification_service_| set to true.
base::Optional<cert_verifier::TestCertVerifierServiceFactoryImpl>
cert_verifier::TestCertVerifierServiceFactoryImpl
test_cert_verifier_service_factory_;
};
@ -615,5 +588,5 @@ IN_PROC_BROWSER_TEST_P(
INSTANTIATE_TEST_SUITE_P(
All,
SystemNetworkContextServiceCertVerifierBuiltinFeaturePolicyTest,
::testing::Combine(::testing::Bool(), ::testing::Bool()));
::testing::Bool());
#endif // BUILDFLAG(BUILTIN_CERT_VERIFIER_FEATURE_SUPPORTED)

@ -49,7 +49,6 @@ static const char kOCSPTestCertPolicy[] = "1.3.6.1.4.1.11129.2.4.1";
} // namespace
class OCSPBrowserTest : public PlatformBrowserTest,
public ::testing::WithParamInterface<bool>,
public network::mojom::SSLConfigClient {
public:
OCSPBrowserTest() = default;
@ -76,19 +75,6 @@ class OCSPBrowserTest : public PlatformBrowserTest,
base::nullopt);
}
void SetUpInProcessBrowserTestFixture() override {
std::vector<base::Feature> enabled_features;
std::vector<base::Feature> disabled_features;
disabled_features.push_back(blink::features::kMixedContentAutoupgrade);
if (GetParam()) {
enabled_features.push_back(network::features::kCertVerifierService);
} else {
disabled_features.push_back(network::features::kCertVerifierService);
}
scoped_feature_list_.InitWithFeatures(enabled_features, disabled_features);
}
void SetUpOnMainThread() override {
network::mojom::NetworkContextParamsPtr context_params =
g_browser_process->system_network_context_manager()
@ -96,23 +82,12 @@ class OCSPBrowserTest : public PlatformBrowserTest,
last_ssl_config_ = *context_params->initial_ssl_config;
receiver_.Bind(std::move(context_params->ssl_config_client_receiver));
if (GetParam() || content::IsInProcessNetworkService()) {
// TODO(https://crbug.com/1085233): when the CertVerifierService is moved
// out of process, the ScopedTestEVPolicy needs to be instantiated in
// that process.
ev_test_policy_ = std::make_unique<net::ScopedTestEVPolicy>(
net::EVRootCAMetadata::GetInstance(), kTestRootCertHash,
kOCSPTestCertPolicy);
} else {
content::GetNetworkService()->BindTestInterface(
network_service_test_.BindNewPipeAndPassReceiver());
mojo::ScopedAllowSyncCallForTesting allow_sync_call;
EXPECT_TRUE(network_service_test_->SetEVPolicy(
std::vector<uint8_t>(
kTestRootCertHash.data,
kTestRootCertHash.data + sizeof(kTestRootCertHash.data)),
kOCSPTestCertPolicy));
}
// TODO(https://crbug.com/1085233): when the CertVerifierService is moved
// out of process, the ScopedTestEVPolicy needs to be instantiated in
// that process.
ev_test_policy_ = std::make_unique<net::ScopedTestEVPolicy>(
net::EVRootCAMetadata::GetInstance(), kTestRootCertHash,
kOCSPTestCertPolicy);
}
// Sets the policy identified by |policy_name| to be true, ensuring
@ -206,8 +181,6 @@ class OCSPBrowserTest : public PlatformBrowserTest,
testing::NiceMock<policy::MockConfigurationPolicyProvider> policy_provider_;
std::unique_ptr<net::ScopedTestEVPolicy> ev_test_policy_;
base::test::ScopedFeatureList scoped_feature_list_;
mojo::Remote<network::mojom::NetworkServiceTest> network_service_test_;
base::RepeatingClosure ssl_config_updated_callback_;
network::mojom::SSLConfig last_ssl_config_;
@ -216,7 +189,7 @@ class OCSPBrowserTest : public PlatformBrowserTest,
// Visits a page with revocation checking set to the default value (disabled)
// and a revoked OCSP response.
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPRevokedButNotChecked) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, TestHTTPSOCSPRevokedButNotChecked) {
// OCSP checking is disabled by default.
EXPECT_FALSE(last_ssl_config().rev_checking_enabled);
EXPECT_FALSE(g_browser_process->system_network_context_manager()
@ -237,7 +210,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPRevokedButNotChecked) {
}
// Visits a page with revocation checking enabled and a valid OCSP response.
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPOk) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, TestHTTPSOCSPOk) {
EnableRevocationChecking();
net::EmbeddedTestServer::ServerCertificateConfig ok_cert_config;
@ -258,7 +231,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPOk) {
}
// Visits a page with revocation checking enabled and a revoked OCSP response.
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPRevoked) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, TestHTTPSOCSPRevoked) {
EnableRevocationChecking();
net::EmbeddedTestServer::ServerCertificateConfig revoked_cert_config;
@ -278,7 +251,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPRevoked) {
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPInvalid) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, TestHTTPSOCSPInvalid) {
EnableRevocationChecking();
net::EmbeddedTestServer::ServerCertificateConfig invalid_cert_config;
@ -296,7 +269,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPInvalid) {
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPIntermediateValid) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, TestHTTPSOCSPIntermediateValid) {
EnableRevocationChecking();
net::EmbeddedTestServer::ServerCertificateConfig
@ -324,7 +297,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPIntermediateValid) {
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest,
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest,
TestHTTPSOCSPIntermediateResponseOldButStillValid) {
EnableRevocationChecking();
@ -352,7 +325,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest,
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest,
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest,
TestHTTPSOCSPIntermediateResponseTooOld) {
EnableRevocationChecking();
@ -387,7 +360,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest,
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPIntermediateRevoked) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, TestHTTPSOCSPIntermediateRevoked) {
EnableRevocationChecking();
net::EmbeddedTestServer::ServerCertificateConfig cert_config;
@ -421,7 +394,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPIntermediateRevoked) {
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPValidStapled) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, TestHTTPSOCSPValidStapled) {
if (!ssl_test_util::SystemSupportsOCSPStapling()) {
LOG(WARNING)
<< "Skipping test because system doesn't support OCSP stapling";
@ -452,7 +425,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPValidStapled) {
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPRevokedStapled) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, TestHTTPSOCSPRevokedStapled) {
if (!ssl_test_util::SystemSupportsOCSPStapling()) {
LOG(WARNING)
<< "Skipping test because system doesn't support OCSP stapling";
@ -483,7 +456,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPRevokedStapled) {
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPOldStapledAndInvalidAIA) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, TestHTTPSOCSPOldStapledAndInvalidAIA) {
if (!ssl_test_util::SystemSupportsOCSPStapling()) {
LOG(WARNING)
<< "Skipping test because system doesn't support OCSP stapling";
@ -512,7 +485,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPOldStapledAndInvalidAIA) {
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPOldStapledButValidAIA) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, TestHTTPSOCSPOldStapledButValidAIA) {
if (!ssl_test_util::SystemSupportsOCSPStapling()) {
LOG(WARNING)
<< "Skipping test because system doesn't support OCSP stapling";
@ -544,7 +517,7 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, TestHTTPSOCSPOldStapledButValidAIA) {
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, HardFailOnOCSPInvalid) {
IN_PROC_BROWSER_TEST_F(OCSPBrowserTest, HardFailOnOCSPInvalid) {
if (!ssl_test_util::SystemSupportsHardFailRevocationChecking()) {
LOG(WARNING) << "Skipping test because system doesn't support hard fail "
<< "revocation checking";
@ -589,11 +562,9 @@ IN_PROC_BROWSER_TEST_P(OCSPBrowserTest, HardFailOnOCSPInvalid) {
EXPECT_TRUE(cert_status & net::CERT_STATUS_REV_CHECKING_ENABLED);
}
INSTANTIATE_TEST_SUITE_P(/* no prefix */, OCSPBrowserTest, ::testing::Bool());
using AIABrowserTest = OCSPBrowserTest;
IN_PROC_BROWSER_TEST_P(AIABrowserTest, TestHTTPSAIA) {
IN_PROC_BROWSER_TEST_F(AIABrowserTest, TestHTTPSAIA) {
net::EmbeddedTestServer::ServerCertificateConfig cert_config;
cert_config.intermediate = net::EmbeddedTestServer::IntermediateType::kByAIA;
@ -601,5 +572,3 @@ IN_PROC_BROWSER_TEST_P(AIABrowserTest, TestHTTPSAIA) {
ssl_test_util::CheckAuthenticatedState(
chrome_test_utils::GetActiveWebContents(this), AuthState::NONE);
}
INSTANTIATE_TEST_SUITE_P(/* no prefix */, AIABrowserTest, ::testing::Bool());

@ -624,24 +624,13 @@ GetCertVerifierServiceFactory() {
} // namespace
network::mojom::CertVerifierParamsPtr GetCertVerifierParams(
network::mojom::CertVerifierServiceRemoteParamsPtr GetCertVerifierParams(
network::mojom::CertVerifierCreationParamsPtr
cert_verifier_creation_params) {
if (!base::FeatureList::IsEnabled(network::features::kCertVerifierService)) {
return network::mojom::CertVerifierParams::NewCreationParams(
std::move(cert_verifier_creation_params));
}
auto cv_service_remote_params =
network::mojom::CertVerifierServiceRemoteParams::New();
// Create a cert verifier service.
cv_service_remote_params->cert_verifier_service =
GetNewCertVerifierServiceRemote(GetCertVerifierServiceFactory(),
std::move(cert_verifier_creation_params));
return network::mojom::CertVerifierParams::NewRemoteParams(
std::move(cv_service_remote_params));
return network::mojom::CertVerifierServiceRemoteParams::New(
GetNewCertVerifierServiceRemote(
GetCertVerifierServiceFactory(),
std::move(cert_verifier_creation_params)));
}
void SetCertVerifierServiceFactoryForTesting(

@ -2472,10 +2472,9 @@ void StoragePartitionImpl::InitNetworkContext() {
devtools_instrumentation::ApplyNetworkContextParamsOverrides(
browser_context_, context_params.get());
DCHECK(!context_params->cert_verifier_params)
<< "|cert_verifier_params| should not be set in the NetworkContextParams,"
"as they will be replaced with either the newly configured "
"|cert_verifier_creation_params| or with a new pipe to the "
"CertVerifierService.";
<< "|cert_verifier_params| should not be set in the "
"NetworkContextParams, as they will be replaced with a new pipe to "
"the CertVerifierService.";
context_params->cert_verifier_params =
GetCertVerifierParams(std::move(cert_verifier_creation_params));

@ -90,9 +90,9 @@ GetNetworkTaskRunner();
//
// Otherwise, |cert_verifier_creation_params| will just be placed directly into
// the CertVerifierParams to configure an in-network-service CertVerifier.
CONTENT_EXPORT network::mojom::CertVerifierParamsPtr GetCertVerifierParams(
network::mojom::CertVerifierCreationParamsPtr
cert_verifier_creation_params);
CONTENT_EXPORT network::mojom::CertVerifierServiceRemoteParamsPtr
GetCertVerifierParams(network::mojom::CertVerifierCreationParamsPtr
cert_verifier_creation_params);
// Sets the CertVerifierServiceFactory used to instantiate
// CertVerifierServices.

@ -37,9 +37,9 @@ mojo::PendingRemote<mojom::CertVerifierService> GetNewCertVerifierServiceRemote(
} // namespace
class NetworkContextTest : public testing::Test {
class NetworkContextWithRealCertVerifierTest : public testing::Test {
public:
explicit NetworkContextTest(
explicit NetworkContextWithRealCertVerifierTest(
base::test::TaskEnvironment::TimeSource time_source =
base::test::TaskEnvironment::TimeSource::DEFAULT)
: task_environment_(base::test::TaskEnvironment::MainThreadType::IO,
@ -47,7 +47,7 @@ class NetworkContextTest : public testing::Test {
network_change_notifier_(
net::NetworkChangeNotifier::CreateMockIfNeeded()),
network_service_(network::NetworkService::CreateForTesting()) {}
~NetworkContextTest() override = default;
~NetworkContextWithRealCertVerifierTest() override = default;
std::unique_ptr<network::NetworkContext> CreateContextWithParams(
network::mojom::NetworkContextParamsPtr context_params) {
@ -58,16 +58,10 @@ class NetworkContextTest : public testing::Test {
std::move(context_params));
}
network::mojom::CertVerifierParamsPtr GetCertVerifierParams(
network::mojom::CertVerifierServiceRemoteParamsPtr GetCertVerifierParams(
network::mojom::CertVerifierCreationParamsPtr
cert_verifier_creation_params =
network::mojom::CertVerifierCreationParams::New()) {
if (!base::FeatureList::IsEnabled(
network::features::kCertVerifierService)) {
return network::mojom::CertVerifierParams::NewCreationParams(
std::move(cert_verifier_creation_params));
}
if (!cert_verifier_service_factory_) {
cert_verifier_service_factory_ =
std::make_unique<CertVerifierServiceFactoryImpl>(
@ -75,17 +69,13 @@ class NetworkContextTest : public testing::Test {
.BindNewPipeAndPassReceiver());
}
auto cv_service_remote_params =
network::mojom::CertVerifierServiceRemoteParams::New();
// Create a cert verifier service.
cv_service_remote_params->cert_verifier_service =
GetNewCertVerifierServiceRemote(
cert_verifier_service_factory_.get(),
std::move(cert_verifier_creation_params));
auto cert_verifier_service_remote = GetNewCertVerifierServiceRemote(
cert_verifier_service_factory_.get(),
std::move(cert_verifier_creation_params));
return network::mojom::CertVerifierParams::NewRemoteParams(
std::move(cv_service_remote_params));
return network::mojom::CertVerifierServiceRemoteParams::New(
std::move(cert_verifier_service_remote));
}
network::mojom::NetworkService* network_service() const {
@ -160,28 +150,7 @@ std::unique_ptr<network::TestURLLoaderClient> FetchRequest(
} // namespace
class UseCertVerifierBuiltinTest : public NetworkContextTest,
public testing::WithParamInterface<bool> {
public:
UseCertVerifierBuiltinTest() = default;
~UseCertVerifierBuiltinTest() override = default;
void SetUp() override {
if (GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
network::features::kCertVerifierService);
} else {
scoped_feature_list_.InitAndDisableFeature(
network::features::kCertVerifierService);
}
NetworkContextTest::SetUp();
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
};
TEST_P(UseCertVerifierBuiltinTest, UseCertVerifierBuiltin) {
TEST_F(NetworkContextWithRealCertVerifierTest, UseCertVerifierBuiltin) {
net::EmbeddedTestServer test_server(net::EmbeddedTestServer::TYPE_HTTPS);
net::test_server::RegisterDefaultHandlers(&test_server);
ASSERT_TRUE(test_server.Start());
@ -217,27 +186,15 @@ TEST_P(UseCertVerifierBuiltinTest, UseCertVerifierBuiltin) {
}
}
INSTANTIATE_TEST_SUITE_P(All, UseCertVerifierBuiltinTest, ::testing::Bool());
class NetworkContextCertVerifierBuiltinFeatureFlagTest
: public NetworkContextTest,
public testing::WithParamInterface<std::tuple<bool, bool>> {
: public NetworkContextWithRealCertVerifierTest,
public testing::WithParamInterface<bool> {
public:
NetworkContextCertVerifierBuiltinFeatureFlagTest()
: use_builtin_cert_verifier_feature_(std::get<0>(GetParam())),
use_cert_verifier_service_feature_(std::get<1>(GetParam())) {
std::vector<base::Feature> enabled_features, disabled_features;
if (use_builtin_cert_verifier_feature_) {
enabled_features.push_back(net::features::kCertVerifierBuiltinFeature);
} else {
disabled_features.push_back(net::features::kCertVerifierBuiltinFeature);
}
if (use_cert_verifier_service_feature_) {
enabled_features.push_back(network::features::kCertVerifierService);
} else {
disabled_features.push_back(network::features::kCertVerifierService);
}
scoped_feature_list_.InitWithFeatures(enabled_features, disabled_features);
: use_builtin_cert_verifier_feature_(GetParam()) {
scoped_feature_list_.InitWithFeatureState(
net::features::kCertVerifierBuiltinFeature,
use_builtin_cert_verifier_feature_);
}
bool use_builtin_cert_verifier_feature() const {
@ -246,7 +203,6 @@ class NetworkContextCertVerifierBuiltinFeatureFlagTest
private:
const bool use_builtin_cert_verifier_feature_;
const bool use_cert_verifier_service_feature_;
base::test::ScopedFeatureList scoped_feature_list_;
};
@ -281,7 +237,6 @@ TEST_P(NetworkContextCertVerifierBuiltinFeatureFlagTest,
INSTANTIATE_TEST_SUITE_P(All,
NetworkContextCertVerifierBuiltinFeatureFlagTest,
::testing::Combine(::testing::Bool(),
::testing::Bool()));
::testing::Bool());
#endif // BUILDFLAG(BUILTIN_CERT_VERIFIER_FEATURE_SUPPORTED)
} // namespace cert_verifier

@ -47,8 +47,7 @@ const base::FilePath::CharType kServicesTestData[] =
// Base class for any tests that just need a NetworkService and a
// TaskEnvironment, and to create NetworkContexts using the NetworkService.
// Parametrized on whether or not the CertVerifierService feature is enabled.
class NetworkServiceIntegrationTest : public testing::TestWithParam<bool> {
class NetworkServiceIntegrationTest : public testing::Test {
public:
NetworkServiceIntegrationTest()
: task_environment_(base::test::TaskEnvironment::MainThreadType::IO),
@ -57,50 +56,22 @@ class NetworkServiceIntegrationTest : public testing::TestWithParam<bool> {
cert_verifier_service_remote_.BindNewPipeAndPassReceiver()) {}
~NetworkServiceIntegrationTest() override = default;
void SetUp() override {
if (GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
network::features::kCertVerifierService);
} else {
scoped_feature_list_.InitAndDisableFeature(
network::features::kCertVerifierService);
}
}
void DestroyService() { service_.reset(); }
network::mojom::NetworkContextParamsPtr CreateNetworkContextParams() {
network::mojom::CertVerifierCreationParamsPtr
cert_verifier_creation_params =
network::mojom::CertVerifierCreationParams::New();
network::mojom::CertVerifierParamsPtr cert_verifier_params;
if (base::FeatureList::IsEnabled(network::features::kCertVerifierService)) {
mojo::PendingRemote<mojom::CertVerifierService> cv_service_remote;
mojo::PendingRemote<mojom::CertVerifierService> cv_service_remote;
auto cv_service_remote_params =
network::mojom::CertVerifierServiceRemoteParams::New();
// Create a cert verifier service.
cert_verifier_service_impl_.GetNewCertVerifierForTesting(
cv_service_remote.InitWithNewPipeAndPassReceiver(),
std::move(cert_verifier_creation_params),
&cert_net_fetcher_url_loader_);
cv_service_remote_params->cert_verifier_service =
std::move(cv_service_remote);
cert_verifier_params =
network::mojom::CertVerifierParams::NewRemoteParams(
std::move(cv_service_remote_params));
} else {
cert_verifier_params =
network::mojom::CertVerifierParams::NewCreationParams(
std::move(cert_verifier_creation_params));
}
// Create a cert verifier service.
cert_verifier_service_impl_.GetNewCertVerifierForTesting(
cv_service_remote.InitWithNewPipeAndPassReceiver(),
network::mojom::CertVerifierCreationParams::New(),
&cert_net_fetcher_url_loader_);
network::mojom::NetworkContextParamsPtr params =
network::mojom::NetworkContextParams::New();
params->cert_verifier_params = std::move(cert_verifier_params);
params->cert_verifier_params =
network::mojom::CertVerifierServiceRemoteParams::New(
std::move(cv_service_remote));
// Use a fixed proxy config, to avoid dependencies on local network
// configuration.
params->initial_proxy_config =
@ -160,7 +131,6 @@ class NetworkServiceIntegrationTest : public testing::TestWithParam<bool> {
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
base::test::TaskEnvironment task_environment_;
std::unique_ptr<network::NetworkService> service_;
@ -198,7 +168,7 @@ class NetworkServiceCRLSetTest : public NetworkServiceIntegrationTest {
};
// Verifies CRLSets take effect if configured on the service.
TEST_P(NetworkServiceCRLSetTest, CRLSetIsApplied) {
TEST_F(NetworkServiceCRLSetTest, CRLSetIsApplied) {
CreateNetworkContext(CreateNetworkContextParams());
uint32_t options =
@ -241,7 +211,7 @@ TEST_P(NetworkServiceCRLSetTest, CRLSetIsApplied) {
// Verifies CRLSets configured before creating a new network context are
// applied to that network context.
TEST_P(NetworkServiceCRLSetTest, CRLSetIsPassedToNewContexts) {
TEST_F(NetworkServiceCRLSetTest, CRLSetIsPassedToNewContexts) {
// Send a CRLSet that blocks the leaf cert, even while no NetworkContexts
// exist.
std::string crl_set_bytes;
@ -270,7 +240,7 @@ TEST_P(NetworkServiceCRLSetTest, CRLSetIsPassedToNewContexts) {
}
// Verifies newer CRLSets (by sequence number) are applied.
TEST_P(NetworkServiceCRLSetTest, CRLSetIsUpdatedIfNewer) {
TEST_F(NetworkServiceCRLSetTest, CRLSetIsUpdatedIfNewer) {
// Send a CRLSet that only allows the root cert if it matches a known SPKI
// hash (that matches the test server chain)
std::string crl_set_bytes;
@ -326,7 +296,7 @@ TEST_P(NetworkServiceCRLSetTest, CRLSetIsUpdatedIfNewer) {
// Verifies that attempting to send an older CRLSet (by sequence number)
// does not apply to existing or new contexts.
TEST_P(NetworkServiceCRLSetTest, CRLSetDoesNotDowngrade) {
TEST_F(NetworkServiceCRLSetTest, CRLSetDoesNotDowngrade) {
// Send a CRLSet that blocks the root certificate by subject name.
std::string crl_set_bytes;
ASSERT_TRUE(base::ReadFileToString(net::GetTestCertsDirectory().AppendASCII(
@ -398,7 +368,6 @@ TEST_P(NetworkServiceCRLSetTest, CRLSetDoesNotDowngrade) {
net::CERT_STATUS_REVOKED);
}
INSTANTIATE_TEST_SUITE_P(All, NetworkServiceCRLSetTest, ::testing::Bool());
#endif // !defined(OS_IOS) && !defined(OS_ANDROID)
// TODO(crbug.com/860189): AIA tests fail on iOS
@ -447,7 +416,7 @@ class NetworkServiceAIATest : public NetworkServiceIntegrationTest {
net::EmbeddedTestServer test_server_;
};
TEST_P(NetworkServiceAIATest, MAYBE(AIAFetching)) {
TEST_F(NetworkServiceAIATest, MAYBE(AIAFetching)) {
PerformAIATest();
}
@ -455,10 +424,9 @@ TEST_P(NetworkServiceAIATest, MAYBE(AIAFetching)) {
// backing the CertNetFetcherURLLoader disconnects.
// Only relevant if testing with the CertVerifierService, and the underlying
// CertVerifier uses the CertNetFetcher.
TEST_P(NetworkServiceAIATest,
TEST_F(NetworkServiceAIATest,
MAYBE(AIAFetchingWithURLLoaderFactoryDisconnect)) {
if (!base::FeatureList::IsEnabled(network::features::kCertVerifierService) ||
!cert_net_fetcher_url_loader()) {
if (!cert_net_fetcher_url_loader()) {
// TODO(crbug.com/1015706): Switch to GTEST_SKIP().
LOG(WARNING) << "Skipping AIA reconnection test because the underlying "
"cert verifier does not use a CertNetFetcherURLLoader.";
@ -475,5 +443,4 @@ TEST_P(NetworkServiceAIATest,
PerformAIATest();
}
INSTANTIATE_TEST_SUITE_P(All, NetworkServiceAIATest, ::testing::Bool());
} // namespace cert_verifier

@ -32,8 +32,7 @@ namespace cert_verifier {
// Base class for any tests that just need a NetworkService and a
// TaskEnvironment, and to create NetworkContexts using the NetworkService.
class SSLConfigServiceMojoTestWithCertVerifier
: public testing::TestWithParam<bool> {
class SSLConfigServiceMojoTestWithCertVerifier : public testing::Test {
public:
SSLConfigServiceMojoTestWithCertVerifier()
: task_environment_(base::test::TaskEnvironment::MainThreadType::IO),
@ -42,50 +41,22 @@ class SSLConfigServiceMojoTestWithCertVerifier
cert_verifier_service_remote_.BindNewPipeAndPassReceiver()) {}
~SSLConfigServiceMojoTestWithCertVerifier() override = default;
void SetUp() override {
if (GetParam()) {
scoped_feature_list_.InitAndEnableFeature(
network::features::kCertVerifierService);
} else {
scoped_feature_list_.InitAndDisableFeature(
network::features::kCertVerifierService);
}
}
void DestroyService() { service_.reset(); }
network::mojom::NetworkContextParamsPtr CreateNetworkContextParams() {
network::mojom::CertVerifierCreationParamsPtr
cert_verifier_creation_params =
network::mojom::CertVerifierCreationParams::New();
network::mojom::CertVerifierParamsPtr cert_verifier_params;
if (base::FeatureList::IsEnabled(network::features::kCertVerifierService)) {
mojo::PendingRemote<mojom::CertVerifierService> cv_service_remote;
mojo::PendingRemote<mojom::CertVerifierService> cv_service_remote;
auto cv_service_remote_params =
network::mojom::CertVerifierServiceRemoteParams::New();
// Create a cert verifier service.
cert_verifier_service_impl_.GetNewCertVerifierForTesting(
cv_service_remote.InitWithNewPipeAndPassReceiver(),
std::move(cert_verifier_creation_params),
&cert_net_fetcher_url_loader_);
cv_service_remote_params->cert_verifier_service =
std::move(cv_service_remote);
cert_verifier_params =
network::mojom::CertVerifierParams::NewRemoteParams(
std::move(cv_service_remote_params));
} else {
cert_verifier_params =
network::mojom::CertVerifierParams::NewCreationParams(
std::move(cert_verifier_creation_params));
}
// Create a cert verifier service.
cert_verifier_service_impl_.GetNewCertVerifierForTesting(
cv_service_remote.InitWithNewPipeAndPassReceiver(),
network::mojom::CertVerifierCreationParams::New(),
&cert_net_fetcher_url_loader_);
network::mojom::NetworkContextParamsPtr params =
network::mojom::NetworkContextParams::New();
params->cert_verifier_params = std::move(cert_verifier_params);
params->cert_verifier_params =
network::mojom::CertVerifierServiceRemoteParams::New(
std::move(cv_service_remote));
// Use a fixed proxy config, to avoid dependencies on local network
// configuration.
params->initial_proxy_config =
@ -116,7 +87,6 @@ class SSLConfigServiceMojoTestWithCertVerifier
private:
base::test::TaskEnvironment task_environment_;
std::unique_ptr<network::NetworkService> service_;
base::test::ScopedFeatureList scoped_feature_list_;
mojo::Remote<network::mojom::NetworkContext> network_context_remote_;
std::unique_ptr<network::NetworkContext> network_context_;
@ -127,7 +97,7 @@ class SSLConfigServiceMojoTestWithCertVerifier
};
#if !defined(OS_IOS) && !defined(OS_ANDROID)
TEST_P(SSLConfigServiceMojoTestWithCertVerifier, CRLSetIsApplied) {
TEST_F(SSLConfigServiceMojoTestWithCertVerifier, CRLSetIsApplied) {
mojo::Remote<network::mojom::SSLConfigClient> ssl_config_client;
network::mojom::NetworkContextParamsPtr context_params =
@ -193,9 +163,6 @@ TEST_P(SSLConfigServiceMojoTestWithCertVerifier, CRLSetIsApplied) {
net::test::IsError(net::ERR_CERT_REVOKED));
}
INSTANTIATE_TEST_SUITE_P(All,
SSLConfigServiceMojoTestWithCertVerifier,
::testing::Bool());
#endif // !defined(OS_IOS) && !defined(OS_ANDROID)
} // namespace cert_verifier

@ -23,13 +23,8 @@ namespace {
// CRLSet.
scoped_refptr<net::CRLSet> ParseCRLSet(std::string crl_set) {
scoped_refptr<net::CRLSet> result;
if (base::FeatureList::IsEnabled(network::features::kCertVerifierService)) {
if (!net::CRLSet::ParseAndStoreUnparsedData(std::move(crl_set), &result))
return nullptr;
} else {
if (!net::CRLSet::Parse(std::move(crl_set), &result))
return nullptr;
}
if (!net::CRLSet::ParseAndStoreUnparsedData(std::move(crl_set), &result))
return nullptr;
return result;
}

@ -88,7 +88,6 @@
#include "services/network/proxy_config_service_mojo.h"
#include "services/network/proxy_lookup_request.h"
#include "services/network/proxy_resolving_socket_factory_mojo.h"
#include "services/network/public/cpp/cert_verifier/cert_verifier_creation.h"
#include "services/network/public/cpp/cert_verifier/mojo_cert_verifier.h"
#include "services/network/public/cpp/content_security_policy/content_security_policy.h"
#include "services/network/public/cpp/features.h"
@ -1869,46 +1868,22 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext(
const base::CommandLine* command_line =
base::CommandLine::ForCurrentProcess();
DCHECK(
g_cert_verifier_for_testing ||
!base::FeatureList::IsEnabled(network::features::kCertVerifierService) ||
(params_->cert_verifier_params &&
params_->cert_verifier_params->is_remote_params()))
<< "If cert verification service is on, the creator of the "
"NetworkContext should pass CertVerifierServiceRemoteParams.";
std::unique_ptr<net::CertVerifier> cert_verifier;
if (g_cert_verifier_for_testing) {
cert_verifier = std::make_unique<WrappedTestingCertVerifier>();
} else {
if (params_->cert_verifier_params &&
params_->cert_verifier_params->is_remote_params()) {
// base::Unretained() is safe below because |this| will own
// |cert_verifier|.
// TODO(https://crbug.com/1085233): this cert verifier should deal with
// disconnections if the CertVerifierService is run outside of the browser
// process.
cert_verifier = std::make_unique<cert_verifier::MojoCertVerifier>(
std::move(params_->cert_verifier_params->get_remote_params()
->cert_verifier_service),
std::move(url_loader_factory_for_cert_net_fetcher),
base::BindRepeating(
&NetworkContext::CreateURLLoaderFactoryForCertNetFetcher,
base::Unretained(this)));
} else {
mojom::CertVerifierCreationParams* creation_params = nullptr;
if (params_->cert_verifier_params &&
params_->cert_verifier_params->is_creation_params()) {
creation_params =
params_->cert_verifier_params->get_creation_params().get();
}
if (IsUsingCertNetFetcher())
cert_net_fetcher_ =
base::MakeRefCounted<net::CertNetFetcherURLRequest>();
cert_verifier = CreateCertVerifier(creation_params, cert_net_fetcher_);
}
DCHECK(params_->cert_verifier_params);
// base::Unretained() is safe below because |this| will own
// |cert_verifier|.
// TODO(https://crbug.com/1085233): this cert verifier should deal with
// disconnections if the CertVerifierService is run outside of the browser
// process.
cert_verifier = std::make_unique<cert_verifier::MojoCertVerifier>(
std::move(params_->cert_verifier_params->cert_verifier_service),
std::move(url_loader_factory_for_cert_net_fetcher),
base::BindRepeating(
&NetworkContext::CreateURLLoaderFactoryForCertNetFetcher,
base::Unretained(this)));
#if BUILDFLAG(IS_CT_SUPPORTED)
std::vector<scoped_refptr<const net::CTLogVerifier>> ct_logs;

@ -325,14 +325,6 @@ void NetworkService::Initialize(mojom::NetworkServiceParamsPtr params,
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
#if defined(OS_MAC)
if (!base::FeatureList::IsEnabled(network::features::kCertVerifierService) &&
base::FeatureList::IsEnabled(
net::features::kCertVerifierBuiltinFeature)) {
net::InitializeTrustStoreMacCache();
}
#endif
// Set-up the global port overrides.
if (command_line->HasSwitch(switches::kExplicitlyAllowedPorts)) {
std::string allowed_ports =

@ -149,12 +149,6 @@ const base::Feature kRequestInitiatorSiteLockEnfocement = {
"RequestInitiatorSiteLockEnfocement",
base::FEATURE_ENABLED_BY_DEFAULT};
// When the CertVerifierService is enabled, certificate verification will not be
// performed in the network service, but will instead be brokered to a separate
// cert verification service potentially running in a different process.
const base::Feature kCertVerifierService{"CertVerifierService",
base::FEATURE_ENABLED_BY_DEFAULT};
// Enables preprocessing requests with the Trust Tokens API Fetch flags set,
// and handling their responses, according to the protocol.
// (See https://github.com/WICG/trust-token-api.)

@ -55,8 +55,6 @@ COMPONENT_EXPORT(NETWORK_CPP)
extern const base::Feature kDisableKeepaliveFetch;
COMPONENT_EXPORT(NETWORK_CPP)
extern const base::Feature kRequestInitiatorSiteLockEnfocement;
COMPONENT_EXPORT(NETWORK_CPP)
extern const base::Feature kCertVerifierService;
COMPONENT_EXPORT(NETWORK_CPP)
extern const base::Feature kTrustTokens;

@ -146,13 +146,6 @@ struct CertVerifierServiceRemoteParams {
cert_verifier_service;
};
// Contains the parameters necessary to either connect to a CertVerifierService
// or create a net::CertVerifier in the network service itself.
union CertVerifierParams {
CertVerifierServiceRemoteParams remote_params;
CertVerifierCreationParams creation_params;
};
// Client to update the custom proxy config.
interface CustomProxyConfigClient {
OnCustomProxyConfigUpdated(CustomProxyConfig proxy_config);
@ -417,9 +410,8 @@ struct NetworkContextParams {
[EnableIf=is_ct_supported]
mojo_base.mojom.Time ct_log_update_time;
// Contains either a pipe to a CertVerifierService, or parameters used to
// instantiate a net::CertVerifier in-process.
CertVerifierParams? cert_verifier_params;
// Contains a pipe to a CertVerifierService.
CertVerifierServiceRemoteParams cert_verifier_params;
// Initial additional certificates that will be used for certificate
// validation.

@ -24,20 +24,13 @@ FakeTestCertVerifierParamsFactory::~FakeTestCertVerifierParamsFactory() =
default;
// static
mojom::CertVerifierParamsPtr
mojom::CertVerifierServiceRemoteParamsPtr
FakeTestCertVerifierParamsFactory::GetCertVerifierParams() {
if (!base::FeatureList::IsEnabled(network::features::kCertVerifierService)) {
return mojom::CertVerifierParams::NewCreationParams(
mojom::CertVerifierCreationParams::New());
}
auto remote_params = mojom::CertVerifierServiceRemoteParams::New();
mojo::PendingRemote<cert_verifier::mojom::CertVerifierService> cv_remote;
mojo::MakeSelfOwnedReceiver(
std::make_unique<FakeTestCertVerifierParamsFactory>(),
cv_remote.InitWithNewPipeAndPassReceiver());
remote_params->cert_verifier_service = std::move(cv_remote);
return mojom::CertVerifierParams::NewRemoteParams(std::move(remote_params));
return mojom::CertVerifierServiceRemoteParams::New(std::move(cv_remote));
}
void FakeTestCertVerifierParamsFactory::Verify(

@ -24,7 +24,7 @@ class FakeTestCertVerifierParamsFactory
FakeTestCertVerifierParamsFactory();
~FakeTestCertVerifierParamsFactory() override;
static mojom::CertVerifierParamsPtr GetCertVerifierParams();
static mojom::CertVerifierServiceRemoteParamsPtr GetCertVerifierParams();
private:
// cert_verifier::mojom::CertVerifierService implementation: