Enable client certificate patterns in device ONC policy

chromeos/network/client_cert_resolver_unittest.cc

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..e5376ebb147b3b1e476dd25bc77347bdd22e184d 100644
--- a/chromeos/network/client_cert_resolver_unittest.cc
+++ b/chromeos/network/client_cert_resolver_unittest.cc
@@ -55,14 +55,15 @@ class ClientCertResolverTest : public testing::Test,
  public:
   ClientCertResolverTest()
       : network_properties_changed_count_(0),
+        cert_loader_(nullptr),

[emaxx, 2017/04/20 20:10:39]

nit: It's generally advisable to move onto C++11 in-class initializers in such
straightforward cases. (If you take this change on, please also update other
members that are initialized here.)

[pmarko, 2017/04/24 14:49:56]

Done. I've left scoped_task_scheduler(&message_loop) in the initializer list
because it's the only non-trivial one.

         service_test_(nullptr),
         profile_test_(nullptr),
-        cert_loader_(nullptr),
         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(
@@ -137,6 +138,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());
@@ -221,7 +232,8 @@ class ClientCertResolverTest : public testing::Test,
   // 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 +263,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 */);
   }
 
@@ -277,6 +289,7 @@ class ClientCertResolverTest : public testing::Test,
   std::string test_cert_id_;
   std::unique_ptr<base::SimpleTestClock> test_clock_;
   std::unique_ptr<ClientCertResolver> client_cert_resolver_;
+  CertLoader* cert_loader_;
 
  private:
   // ClientCertResolver::Observer:
@@ -287,7 +300,6 @@ class ClientCertResolverTest : public testing::Test,
 
   ShillServiceClient::TestInterface* service_test_;
   ShillProfileClient::TestInterface* profile_test_;
-  CertLoader* cert_loader_;
   std::unique_ptr<NetworkStateHandler> network_state_handler_;
   std::unique_ptr<NetworkProfileHandler> network_profile_handler_;
   std::unique_ptr<NetworkConfigurationHandler> network_config_handler_;
@@ -298,6 +310,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 +323,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.
@@ -348,7 +361,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 +386,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 +403,7 @@ TEST_F(ClientCertResolverTest, ExpiringCertificate) {
   base::RunLoop().RunUntilIdle();
 
   SetupNetworkHandlers();
-  SetupPolicyMatchingIssuerPEM("");
+  SetupPolicyMatchingIssuerPEM(onc::ONC_SOURCE_USER_POLICY, "");
   base::RunLoop().RunUntilIdle();
 
   StartCertLoader();
@@ -414,13 +427,76 @@ TEST_F(ClientCertResolverTest, ExpiringCertificate) {
   EXPECT_EQ(std::string(), pkcs11_id);
 }
 
+TEST_F(ClientCertResolverTest, UserPolicyUsesSystemToken) {
+  SetupTestCertInSystemToken("client_1");
+  SetupWifi();
+  base::RunLoop().RunUntilIdle();
+
+  SetupNetworkHandlers();
+  SetupPolicyMatchingIssuerPEM(onc::ONC_SOURCE_USER_POLICY, "");
+  base::RunLoop().RunUntilIdle();
+
+  StartCertLoader();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1U, cert_loader_->system_cert_list().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, DevicePolicyUsesSystemToken) {
+  SetupTestCertInSystemToken("client_1");
+  SetupWifi();
+  base::RunLoop().RunUntilIdle();
+
+  SetupNetworkHandlers();
+  SetupPolicyMatchingIssuerPEM(onc::ONC_SOURCE_DEVICE_POLICY, "");
+  base::RunLoop().RunUntilIdle();
+
+  StartCertLoader();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1U, cert_loader_->system_cert_list().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, DevicePolicyDoesNotUseUserToken) {
+  SetupTestCerts("client_1", false /* import issuer */);
+  SetupWifi();
+  base::RunLoop().RunUntilIdle();
+
+  SetupNetworkHandlers();
+  SetupPolicyMatchingIssuerPEM(onc::ONC_SOURCE_DEVICE_POLICY, "");
+  base::RunLoop().RunUntilIdle();
+
+  network_properties_changed_count_ = 0;
+  StartCertLoader();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(0U, cert_loader_->system_cert_list().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, 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 +513,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);