Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1772)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chromeos/network/client_cert_resolver_unittest.cc

Issue 2828713002: Enable client certificate patterns in device ONC policy (Closed)
Patch Set: Addressed comments - more DCHECKs, use PostTask..WithReply in client_cert_resolver.cc. Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chromeos/network/client_cert_resolver.cc ('k') | chromeos/network/client_cert_util.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chromeos/network/client_cert_resolver_unittest.cc
diff --git a/chromeos/network/client_cert_resolver_unittest.cc b/chromeos/network/client_cert_resolver_unittest.cc
index 8caac7a44c1cd4d72d7bb6148270dc3be46bae26..06576480f7a6c4eb15a87b3965e7bb724550807b 100644
--- a/chromeos/network/client_cert_resolver_unittest.cc
+++ b/chromeos/network/client_cert_resolver_unittest.cc
@@ -53,16 +53,12 @@ const char* kUserHash = "user_hash";
class ClientCertResolverTest : public testing::Test,
public ClientCertResolver::Observer {
public:
- ClientCertResolverTest()
- : network_properties_changed_count_(0),
- service_test_(nullptr),
- profile_test_(nullptr),
- cert_loader_(nullptr),
- scoped_task_scheduler_(&message_loop_) {}
+ ClientCertResolverTest() : scoped_task_scheduler_(&message_loop_) {}
~ClientCertResolverTest() override {}
void SetUp() override {
ASSERT_TRUE(test_nssdb_.is_open());
+ ASSERT_TRUE(test_system_nssdb_.is_open());
// Use the same DB for public and private slot.
test_nsscertdb_.reset(new net::NSSCertDatabaseChromeOS(
@@ -86,10 +82,12 @@ class ClientCertResolverTest : public testing::Test,
}
void TearDown() override {
- client_cert_resolver_->RemoveObserver(this);
+ if (client_cert_resolver_)
+ client_cert_resolver_->RemoveObserver(this);
client_cert_resolver_.reset();
test_clock_.reset();
- network_state_handler_->Shutdown();
+ if (network_state_handler_)
+ network_state_handler_->Shutdown();
managed_config_handler_.reset();
network_config_handler_.reset();
network_profile_handler_.reset();
@@ -137,6 +135,16 @@ class ClientCertResolverTest : public testing::Test,
ASSERT_TRUE(test_client_cert_.get());
}
+ void SetupTestCertInSystemToken(const std::string& prefix) {
+ test_nsscertdb_->SetSystemSlot(
+ crypto::ScopedPK11Slot(PK11_ReferenceSlot(test_system_nssdb_.slot())));
+
+ test_client_cert_ = net::ImportClientCertAndKeyFromFile(
+ net::GetTestCertsDirectory(), prefix + ".pem", prefix + ".pk8",
+ test_system_nssdb_.slot());
+ ASSERT_TRUE(test_client_cert_.get());
+ }
+
void SetupNetworkHandlers() {
network_state_handler_ = NetworkStateHandler::InitializeForTest();
network_profile_handler_.reset(new NetworkProfileHandler());
@@ -184,7 +192,7 @@ class ClientCertResolverTest : public testing::Test,
// Sets up a policy with a certificate pattern that matches any client cert
// with a certain Issuer CN. It will match the test client cert.
- void SetupPolicyMatchingIssuerCN() {
+ void SetupPolicyMatchingIssuerCN(onc::ONCSource onc_source) {
const char* kTestPolicy =
"[ { \"GUID\": \"wifi_stub\","
" \"Name\": \"wifi_stub\","
@@ -213,15 +221,40 @@ class ClientCertResolverTest : public testing::Test,
base::ListValue* policy = nullptr;
ASSERT_TRUE(policy_value->GetAsList(&policy));
+ std::string user_hash =
+ onc_source == onc::ONC_SOURCE_USER_POLICY ? kUserHash : "";
managed_config_handler_->SetPolicy(
- onc::ONC_SOURCE_USER_POLICY, kUserHash, *policy,
+ onc_source, user_hash, *policy,
base::DictionaryValue() /* no global network config */);
}
+ void SetupCertificateConfigMatchingIssuerCN(
+ onc::ONCSource onc_source,
+ client_cert::ClientCertConfig* client_cert_config) {
+ const char* kTestOncPattern =
+ "{"
+ " \"Issuer\": {"
+ " \"CommonName\": \"B CA\""
+ " }"
+ "}";
+ std::string error;
+ std::unique_ptr<base::Value> onc_pattern_value =
+ base::JSONReader::ReadAndReturnError(
+ kTestOncPattern, base::JSON_ALLOW_TRAILING_COMMAS, nullptr, &error);
+ ASSERT_TRUE(onc_pattern_value) << error;
+
+ base::DictionaryValue* onc_pattern_dict;
+ onc_pattern_value->GetAsDictionary(&onc_pattern_dict);
+
+ client_cert_config->onc_source = onc_source;
+ client_cert_config->pattern.ReadFromONCDictionary(*onc_pattern_dict);
+ }
+
// Sets up a policy with a certificate pattern that matches any client cert
// that is signed by the test CA cert (stored in |test_ca_cert_pem_|). In
// particular it will match the test client cert.
- void SetupPolicyMatchingIssuerPEM(const std::string& identity) {
+ void SetupPolicyMatchingIssuerPEM(onc::ONCSource onc_source,
+ const std::string& identity) {
const char* kTestPolicyTemplate =
"[ { \"GUID\": \"wifi_stub\","
" \"Name\": \"wifi_stub\","
@@ -251,10 +284,10 @@ class ClientCertResolverTest : public testing::Test,
base::ListValue* policy = nullptr;
ASSERT_TRUE(policy_value->GetAsList(&policy));
+ std::string user_hash =
+ onc_source == onc::ONC_SOURCE_USER_POLICY ? kUserHash : "";
managed_config_handler_->SetPolicy(
- onc::ONC_SOURCE_USER_POLICY,
- kUserHash,
- *policy,
+ onc_source, user_hash, *policy,
base::DictionaryValue() /* no global network config */);
}
@@ -273,10 +306,11 @@ class ClientCertResolverTest : public testing::Test,
properties->GetStringWithoutPathExpansion(prop_name, prop_value);
}
- int network_properties_changed_count_;
+ int network_properties_changed_count_ = 0;
std::string test_cert_id_;
std::unique_ptr<base::SimpleTestClock> test_clock_;
std::unique_ptr<ClientCertResolver> client_cert_resolver_;
+ CertLoader* cert_loader_ = nullptr;
private:
// ClientCertResolver::Observer:
@@ -285,9 +319,8 @@ class ClientCertResolverTest : public testing::Test,
++network_properties_changed_count_;
}
- ShillServiceClient::TestInterface* service_test_;
- ShillProfileClient::TestInterface* profile_test_;
- CertLoader* cert_loader_;
+ ShillServiceClient::TestInterface* service_test_ = nullptr;
+ ShillProfileClient::TestInterface* profile_test_ = nullptr;
std::unique_ptr<NetworkStateHandler> network_state_handler_;
std::unique_ptr<NetworkProfileHandler> network_profile_handler_;
std::unique_ptr<NetworkConfigurationHandler> network_config_handler_;
@@ -298,6 +331,7 @@ class ClientCertResolverTest : public testing::Test,
scoped_refptr<net::X509Certificate> test_client_cert_;
std::string test_ca_cert_pem_;
crypto::ScopedTestNSSDB test_nssdb_;
+ crypto::ScopedTestNSSDB test_system_nssdb_;
std::unique_ptr<net::NSSCertDatabaseChromeOS> test_nsscertdb_;
DISALLOW_COPY_AND_ASSIGN(ClientCertResolverTest);
@@ -310,7 +344,7 @@ TEST_F(ClientCertResolverTest, NoMatchingCertificates) {
base::RunLoop().RunUntilIdle();
network_properties_changed_count_ = 0;
SetupNetworkHandlers();
- SetupPolicyMatchingIssuerPEM("");
+ SetupPolicyMatchingIssuerPEM(onc::ONC_SOURCE_USER_POLICY, "");
base::RunLoop().RunUntilIdle();
// Verify that no client certificate was configured.
@@ -327,7 +361,7 @@ TEST_F(ClientCertResolverTest, MatchIssuerCNWithoutIssuerInstalled) {
base::RunLoop().RunUntilIdle();
SetupNetworkHandlers();
- SetupPolicyMatchingIssuerCN();
+ SetupPolicyMatchingIssuerCN(onc::ONC_SOURCE_USER_POLICY);
base::RunLoop().RunUntilIdle();
network_properties_changed_count_ = 0;
@@ -348,7 +382,7 @@ TEST_F(ClientCertResolverTest, ResolveOnCertificatesLoaded) {
base::RunLoop().RunUntilIdle();
SetupNetworkHandlers();
- SetupPolicyMatchingIssuerPEM("");
+ SetupPolicyMatchingIssuerPEM(onc::ONC_SOURCE_USER_POLICY, "");
base::RunLoop().RunUntilIdle();
network_properties_changed_count_ = 0;
@@ -373,7 +407,7 @@ TEST_F(ClientCertResolverTest, ResolveAfterPolicyApplication) {
// Policy application will trigger the ClientCertResolver.
network_properties_changed_count_ = 0;
- SetupPolicyMatchingIssuerPEM("");
+ SetupPolicyMatchingIssuerPEM(onc::ONC_SOURCE_USER_POLICY, "");
base::RunLoop().RunUntilIdle();
// Verify that the resolver positively matched the pattern in the policy with
@@ -390,7 +424,7 @@ TEST_F(ClientCertResolverTest, ExpiringCertificate) {
base::RunLoop().RunUntilIdle();
SetupNetworkHandlers();
- SetupPolicyMatchingIssuerPEM("");
+ SetupPolicyMatchingIssuerPEM(onc::ONC_SOURCE_USER_POLICY, "");
base::RunLoop().RunUntilIdle();
StartCertLoader();
@@ -414,13 +448,130 @@ TEST_F(ClientCertResolverTest, ExpiringCertificate) {
EXPECT_EQ(std::string(), pkcs11_id);
}
+TEST_F(ClientCertResolverTest, UserPolicyUsesSystemToken) {
+ SetupTestCertInSystemToken("client_1");
+ SetupWifi();
+ base::RunLoop().RunUntilIdle();
+
+ SetupNetworkHandlers();
+ SetupPolicyMatchingIssuerCN(onc::ONC_SOURCE_USER_POLICY);
+ base::RunLoop().RunUntilIdle();
+
+ StartCertLoader();
+ base::RunLoop().RunUntilIdle();
+ EXPECT_EQ(1U, cert_loader_->system_certs().size());
+
+ // Verify that the resolver positively matched the pattern in the policy with
+ // the test client cert and configured the network.
+ std::string pkcs11_id;
+ GetServiceProperty(shill::kEapCertIdProperty, &pkcs11_id);
+ EXPECT_EQ(test_cert_id_, pkcs11_id);
+}
+
+TEST_F(ClientCertResolverTest, UserPolicyUsesSystemTokenSync) {
+ SetupTestCertInSystemToken("client_1");
+ StartCertLoader();
+ base::RunLoop().RunUntilIdle();
+
+ client_cert::ClientCertConfig client_cert_config;
+ SetupCertificateConfigMatchingIssuerCN(onc::ONC_SOURCE_USER_POLICY,
+ &client_cert_config);
+
+ base::DictionaryValue shill_properties;
+ ClientCertResolver::ResolveCertificatePatternSync(
+ client_cert::CONFIG_TYPE_EAP, client_cert_config, &shill_properties);
+ std::string pkcs11_id;
+ shill_properties.GetStringWithoutPathExpansion(shill::kEapCertIdProperty,
+ &pkcs11_id);
+ EXPECT_EQ(test_cert_id_, pkcs11_id);
+}
+
+TEST_F(ClientCertResolverTest, DevicePolicyUsesSystemToken) {
+ SetupTestCertInSystemToken("client_1");
+ SetupWifi();
+ base::RunLoop().RunUntilIdle();
+
+ SetupNetworkHandlers();
+ SetupPolicyMatchingIssuerCN(onc::ONC_SOURCE_USER_POLICY);
+ base::RunLoop().RunUntilIdle();
+
+ StartCertLoader();
+ base::RunLoop().RunUntilIdle();
+ EXPECT_EQ(1U, cert_loader_->system_certs().size());
+
+ // Verify that the resolver positively matched the pattern in the policy with
+ // the test client cert and configured the network.
+ std::string pkcs11_id;
+ GetServiceProperty(shill::kEapCertIdProperty, &pkcs11_id);
+ EXPECT_EQ(test_cert_id_, pkcs11_id);
+}
+
+TEST_F(ClientCertResolverTest, DevicePolicyUsesSystemTokenSync) {
+ SetupTestCertInSystemToken("client_1");
+ StartCertLoader();
+ base::RunLoop().RunUntilIdle();
+
+ client_cert::ClientCertConfig client_cert_config;
+ SetupCertificateConfigMatchingIssuerCN(onc::ONC_SOURCE_DEVICE_POLICY,
+ &client_cert_config);
+
+ base::DictionaryValue shill_properties;
+ ClientCertResolver::ResolveCertificatePatternSync(
+ client_cert::CONFIG_TYPE_EAP, client_cert_config, &shill_properties);
+ std::string pkcs11_id;
+ shill_properties.GetStringWithoutPathExpansion(shill::kEapCertIdProperty,
+ &pkcs11_id);
+ EXPECT_EQ(test_cert_id_, pkcs11_id);
+}
+
+TEST_F(ClientCertResolverTest, DevicePolicyDoesNotUseUserToken) {
+ SetupTestCerts("client_1", false /* do not import the issuer */);
+ SetupWifi();
+ base::RunLoop().RunUntilIdle();
+
+ SetupNetworkHandlers();
+ SetupPolicyMatchingIssuerCN(onc::ONC_SOURCE_DEVICE_POLICY);
+ base::RunLoop().RunUntilIdle();
+
+ network_properties_changed_count_ = 0;
+ StartCertLoader();
+ base::RunLoop().RunUntilIdle();
+ EXPECT_EQ(0U, cert_loader_->system_certs().size());
+
+ // Verify that no client certificate was configured.
+ std::string pkcs11_id;
+ GetServiceProperty(shill::kEapCertIdProperty, &pkcs11_id);
+ EXPECT_EQ(std::string(), pkcs11_id);
+ EXPECT_EQ(1, network_properties_changed_count_);
+ EXPECT_FALSE(client_cert_resolver_->IsAnyResolveTaskRunning());
+}
+
+TEST_F(ClientCertResolverTest, DevicePolicyDoesNotUseUserTokenSync) {
+ SetupTestCerts("client_1", false /* do not import the issuer */);
+ StartCertLoader();
+ base::RunLoop().RunUntilIdle();
+
+ client_cert::ClientCertConfig client_cert_config;
+ SetupCertificateConfigMatchingIssuerCN(onc::ONC_SOURCE_DEVICE_POLICY,
+ &client_cert_config);
+
+ base::DictionaryValue shill_properties;
+ ClientCertResolver::ResolveCertificatePatternSync(
+ client_cert::CONFIG_TYPE_EAP, client_cert_config, &shill_properties);
+ std::string pkcs11_id;
+ shill_properties.GetStringWithoutPathExpansion(shill::kEapCertIdProperty,
+ &pkcs11_id);
+ EXPECT_EQ(std::string(), pkcs11_id);
+}
+
TEST_F(ClientCertResolverTest, PopulateIdentityFromCert) {
SetupTestCerts("client_3", true /* import issuer */);
SetupWifi();
base::RunLoop().RunUntilIdle();
SetupNetworkHandlers();
- SetupPolicyMatchingIssuerPEM("${CERT_SAN_EMAIL}");
+ SetupPolicyMatchingIssuerPEM(onc::ONC_SOURCE_USER_POLICY,
+ "${CERT_SAN_EMAIL}");
base::RunLoop().RunUntilIdle();
network_properties_changed_count_ = 0;
@@ -437,7 +588,8 @@ TEST_F(ClientCertResolverTest, PopulateIdentityFromCert) {
// Verify that after changing the ONC policy to request a variant of the
// Microsoft Universal Principal Name field instead, the correct value is
// substituted into the shill service entry.
- SetupPolicyMatchingIssuerPEM("upn-${CERT_SAN_UPN}-suffix");
+ SetupPolicyMatchingIssuerPEM(onc::ONC_SOURCE_USER_POLICY,
+ "upn-${CERT_SAN_UPN}-suffix");
base::RunLoop().RunUntilIdle();
GetServiceProperty(shill::kEapIdentityProperty, &identity);
« no previous file with comments | « chromeos/network/client_cert_resolver.cc ('k') | chromeos/network/client_cert_util.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698