Allow ${CERT_SAN_EMAIL} and ${CERT_SAN_UPN} in the ONC Identity field

Currently enterprise customers can specify ${LOGIN_ID} or ${LOGIN_EMAIL}
to tell Chrome OS to substitute user identity information into an EAP
configuration.  However, in some installations, the login ID for the
Chromebook does not match the login ID for the EAP-TLS wireless network;
instead, the EAP-TLS identity is stored in the subjectAltName field
in the client certificate.  Add code to Chrome to allow this
field to be extracted if so configured in CPanel.

BUG=549659
TEST=`chromeos_unittests`
TEST=manually configure EAP-TLS network in CPanel, then watch
     the `freeradius -X` logs during connection

Review URL: https://codereview.chromium.org/1717123002

Cr-Commit-Position: refs/heads/master@{#379056}

4 files changed, 114 insertions, 32 deletions

diff --git a/chromeos/network/client_cert_resolver.cc b/chromeos/network/client_cert_resolver.cc
index 3d438d8..b6671bb 100644
--- a/chromeos/network/client_cert_resolver.cc
+++ b/chromeos/network/client_cert_resolver.cc
@@ -14,6 +14,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
+#include "base/strings/string_util.h"
 #include "base/task_runner.h"
 #include "base/threading/worker_pool.h"
 #include "base/time/clock.h"
@@ -25,6 +26,8 @@
 #include "dbus/object_path.h"
 #include "net/cert/scoped_nss_types.h"
 #include "net/cert/x509_certificate.h"
+#include "net/cert/x509_util_nss.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
@@ -34,11 +37,13 @@ struct ClientCertResolver::NetworkAndMatchingCert {
   NetworkAndMatchingCert(const std::string& network_path,
                          client_cert::ConfigType config_type,
                          const std::string& cert_id,
-                         int slot_id)
+                         int slot_id,
+                         const std::string& configured_identity)
       : service_path(network_path),
         cert_config_type(config_type),
         pkcs11_id(cert_id),
-        key_slot_id(slot_id) {}
+        key_slot_id(slot_id),
+        identity(configured_identity) {}
 
   std::string service_path;
   client_cert::ConfigType cert_config_type;
@@ -48,6 +53,12 @@ struct ClientCertResolver::NetworkAndMatchingCert {
 
   // The id of the slot containing the certificate and the private key.
   int key_slot_id;
+
+  // The ONC WiFi.EAP.Identity field can contain variables like
+  // ${CERT_SAN_EMAIL} which are expanded by ClientCertResolver.
+  // |identity| stores a copy of this string after the substitution
+  // has been done.
+  std::string identity;
 };
 
 typedef std::vector<ClientCertResolver::NetworkAndMatchingCert>
@@ -197,6 +208,8 @@ void FindCertificateMatches(const net::CertificateList& certs,
                      MatchCertWithPattern(it->cert_config.pattern));
     std::string pkcs11_id;
     int slot_id = -1;
+    std::string identity;
+
     if (cert_it == client_certs.end()) {
       VLOG(1) << "Couldn't find a matching client cert for network "
               << it->service_path;
@@ -211,9 +224,38 @@ void FindCertificateMatches(const net::CertificateList& certs,
         // the worst case the user can remove the problematic cert.
         continue;
       }
+
+      // If the policy specifies an identity containing ${CERT_SAN_xxx},
+      // see if the cert contains a suitable subjectAltName that can be
+      // stuffed into the shill properties.
+      identity = it->cert_config.policy_identity;
+      std::vector<std::string> names;
+
+      size_t offset = identity.find(onc::substitutes::kCertSANEmail, 0);
+      if (offset != std::string::npos) {
+        std::vector<std::string> names;
+        net::x509_util::GetRFC822SubjectAltNames(
+            cert_it->cert->os_cert_handle(), &names);
+        if (!names.empty()) {
+          base::ReplaceSubstringsAfterOffset(
+              &identity, offset, onc::substitutes::kCertSANEmail, names[0]);
+        }
+      }
+
+      offset = identity.find(onc::substitutes::kCertSANUPN, 0);
+      if (offset != std::string::npos) {
+        std::vector<std::string> names;
+        net::x509_util::GetUPNSubjectAltNames(cert_it->cert->os_cert_handle(),
+                                              &names);
+        if (!names.empty()) {
+          base::ReplaceSubstringsAfterOffset(
+              &identity, offset, onc::substitutes::kCertSANUPN, names[0]);
+        }
+      }
     }
     matches->push_back(ClientCertResolver::NetworkAndMatchingCert(
-        it->service_path, it->cert_config.location, pkcs11_id, slot_id));
+        it->service_path, it->cert_config.location, pkcs11_id, slot_id,
+        identity));
   }
 }
 
@@ -502,6 +544,10 @@ void ClientCertResolver::ConfigureCertificates(NetworkCertMatches* matches) {
                                       it->key_slot_id,
                                       it->pkcs11_id,
                                       &shill_properties);
+      if (!it->identity.empty()) {
+        shill_properties.SetStringWithoutPathExpansion(
+            shill::kEapIdentityProperty, it->identity);
+      }
     }
     network_properties_changed_ = true;
     DBusThreadManager::Get()->GetShillServiceClient()->
diff --git a/chromeos/network/client_cert_resolver_unittest.cc b/chromeos/network/client_cert_resolver_unittest.cc
index 98123a3..2f8002d 100644
--- a/chromeos/network/client_cert_resolver_unittest.cc
+++ b/chromeos/network/client_cert_resolver_unittest.cc
@@ -108,10 +108,10 @@ class ClientCertResolverTest : public testing::Test,
   // Imports a client certificate. Its PKCS#11 ID is stored in |test_cert_id_|.
   // If |import_issuer| is true, also imports the CA cert (stored as PEM in
   // test_ca_cert_pem_) that issued the client certificate.
-  void SetupTestCerts(bool import_issuer) {
+  void SetupTestCerts(const std::string& prefix, bool import_issuer) {
     // Load a CA cert.
     net::CertificateList ca_cert_list = net::CreateCertificateListFromFile(
-        net::GetTestCertsDirectory(), "client_1_ca.pem",
+        net::GetTestCertsDirectory(), prefix + "_ca.pem",
         net::X509Certificate::FORMAT_AUTO);
     ASSERT_TRUE(!ca_cert_list.empty());
     net::X509Certificate::GetPEMEncoded(ca_cert_list[0]->os_cert_handle(),
@@ -127,11 +127,9 @@ class ClientCertResolverTest : public testing::Test,
     }
 
     // Import a client cert signed by that CA.
-    test_client_cert_ =
-        net::ImportClientCertAndKeyFromFile(net::GetTestCertsDirectory(),
-                                            "client_1.pem",
-                                            "client_1.pk8",
-                                            test_nssdb_.slot());
+    test_client_cert_ = net::ImportClientCertAndKeyFromFile(
+        net::GetTestCertsDirectory(), prefix + ".pem", prefix + ".pk8",
+        test_nssdb_.slot());
     ASSERT_TRUE(test_client_cert_.get());
   }
 
@@ -220,7 +218,7 @@ 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() {
+  void SetupPolicyMatchingIssuerPEM(const std::string& identity) {
     const char* kTestPolicyTemplate =
         "[ { \"GUID\": \"wifi_stub\","
         "    \"Name\": \"wifi_stub\","
@@ -229,6 +227,7 @@ class ClientCertResolverTest : public testing::Test,
         "      \"Security\": \"WPA-EAP\","
         "      \"SSID\": \"wifi_ssid\","
         "      \"EAP\": {"
+        "        \"Identity\": \"%s\","
         "        \"Outer\": \"EAP-TLS\","
         "        \"ClientCertType\": \"Pattern\","
         "        \"ClientCertPattern\": {"
@@ -237,8 +236,8 @@ class ClientCertResolverTest : public testing::Test,
         "      }"
         "    }"
         "} ]";
-    std::string policy_json =
-        base::StringPrintf(kTestPolicyTemplate, test_ca_cert_pem_.c_str());
+    std::string policy_json = base::StringPrintf(
+        kTestPolicyTemplate, identity.c_str(), test_ca_cert_pem_.c_str());
 
     std::string error;
     scoped_ptr<base::Value> policy_value = base::JSONReader::ReadAndReturnError(
@@ -260,14 +259,14 @@ class ClientCertResolverTest : public testing::Test,
         kWifiStub, shill::kStateProperty, base::StringValue(state)));
   }
 
-  void GetClientCertProperties(std::string* pkcs11_id) {
-    pkcs11_id->clear();
+  void GetServiceProperty(const std::string& prop_name,
+                          std::string* prop_value) {
+    prop_value->clear();
     const base::DictionaryValue* properties =
         service_test_->GetServiceProperties(kWifiStub);
     if (!properties)
       return;
-    properties->GetStringWithoutPathExpansion(shill::kEapCertIdProperty,
-                                              pkcs11_id);
+    properties->GetStringWithoutPathExpansion(prop_name, prop_value);
   }
 
   int network_properties_changed_count_;
@@ -299,25 +298,25 @@ class ClientCertResolverTest : public testing::Test,
 };
 
 TEST_F(ClientCertResolverTest, NoMatchingCertificates) {
-  SetupTestCerts(false /* do not import the issuer */);
+  SetupTestCerts("client_1", false /* do not import the issuer */);
   StartCertLoader();
   SetupWifi();
   base::RunLoop().RunUntilIdle();
   network_properties_changed_count_ = 0;
   SetupNetworkHandlers();
-  SetupPolicyMatchingIssuerPEM();
+  SetupPolicyMatchingIssuerPEM("");
   base::RunLoop().RunUntilIdle();
 
   // Verify that no client certificate was configured.
   std::string pkcs11_id;
-  GetClientCertProperties(&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, MatchIssuerCNWithoutIssuerInstalled) {
-  SetupTestCerts(false /* do not import the issuer */);
+  SetupTestCerts("client_1", false /* do not import the issuer */);
   SetupWifi();
   base::RunLoop().RunUntilIdle();
 
@@ -332,18 +331,18 @@ TEST_F(ClientCertResolverTest, MatchIssuerCNWithoutIssuerInstalled) {
   // Verify that the resolver positively matched the pattern in the policy with
   // the test client cert and configured the network.
   std::string pkcs11_id;
-  GetClientCertProperties(&pkcs11_id);
+  GetServiceProperty(shill::kEapCertIdProperty, &pkcs11_id);
   EXPECT_EQ(test_cert_id_, pkcs11_id);
   EXPECT_EQ(1, network_properties_changed_count_);
 }
 
 TEST_F(ClientCertResolverTest, ResolveOnCertificatesLoaded) {
-  SetupTestCerts(true /* import issuer */);
+  SetupTestCerts("client_1", true /* import issuer */);
   SetupWifi();
   base::RunLoop().RunUntilIdle();
 
   SetupNetworkHandlers();
-  SetupPolicyMatchingIssuerPEM();
+  SetupPolicyMatchingIssuerPEM("");
   base::RunLoop().RunUntilIdle();
 
   network_properties_changed_count_ = 0;
@@ -353,13 +352,13 @@ TEST_F(ClientCertResolverTest, ResolveOnCertificatesLoaded) {
   // Verify that the resolver positively matched the pattern in the policy with
   // the test client cert and configured the network.
   std::string pkcs11_id;
-  GetClientCertProperties(&pkcs11_id);
+  GetServiceProperty(shill::kEapCertIdProperty, &pkcs11_id);
   EXPECT_EQ(test_cert_id_, pkcs11_id);
   EXPECT_EQ(1, network_properties_changed_count_);
 }
 
 TEST_F(ClientCertResolverTest, ResolveAfterPolicyApplication) {
-  SetupTestCerts(true /* import issuer */);
+  SetupTestCerts("client_1", true /* import issuer */);
   SetupWifi();
   base::RunLoop().RunUntilIdle();
   StartCertLoader();
@@ -368,24 +367,24 @@ TEST_F(ClientCertResolverTest, ResolveAfterPolicyApplication) {
 
   // Policy application will trigger the ClientCertResolver.
   network_properties_changed_count_ = 0;
-  SetupPolicyMatchingIssuerPEM();
+  SetupPolicyMatchingIssuerPEM("");
   base::RunLoop().RunUntilIdle();
 
   // Verify that the resolver positively matched the pattern in the policy with
   // the test client cert and configured the network.
   std::string pkcs11_id;
-  GetClientCertProperties(&pkcs11_id);
+  GetServiceProperty(shill::kEapCertIdProperty, &pkcs11_id);
   EXPECT_EQ(test_cert_id_, pkcs11_id);
   EXPECT_EQ(1, network_properties_changed_count_);
 }
 
 TEST_F(ClientCertResolverTest, ExpiringCertificate) {
-  SetupTestCerts(true /* import issuer */);
+  SetupTestCerts("client_1", true /* import issuer */);
   SetupWifi();
   base::RunLoop().RunUntilIdle();
 
   SetupNetworkHandlers();
-  SetupPolicyMatchingIssuerPEM();
+  SetupPolicyMatchingIssuerPEM("");
   base::RunLoop().RunUntilIdle();
 
   StartCertLoader();
@@ -397,7 +396,7 @@ TEST_F(ClientCertResolverTest, ExpiringCertificate) {
   // Verify that the resolver positively matched the pattern in the policy with
   // the test client cert and configured the network.
   std::string pkcs11_id;
-  GetClientCertProperties(&pkcs11_id);
+  GetServiceProperty(shill::kEapCertIdProperty, &pkcs11_id);
   EXPECT_EQ(test_cert_id_, pkcs11_id);
 
   // Verify that, after the certificate expired and the network disconnection
@@ -405,8 +404,39 @@ TEST_F(ClientCertResolverTest, ExpiringCertificate) {
   test_clock_->SetNow(base::Time::Max());
   SetWifiState(shill::kStateOffline);
   base::RunLoop().RunUntilIdle();
-  GetClientCertProperties(&pkcs11_id);
+  GetServiceProperty(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}");
+  base::RunLoop().RunUntilIdle();
+
+  network_properties_changed_count_ = 0;
+  StartCertLoader();
+  base::RunLoop().RunUntilIdle();
+
+  // Verify that the resolver read the subjectAltName email field from the
+  // cert, and wrote it into the shill service entry.
+  std::string identity;
+  GetServiceProperty(shill::kEapIdentityProperty, &identity);
+  EXPECT_EQ("santest@example.com", identity);
+  EXPECT_EQ(1, network_properties_changed_count_);
+
+  // 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");
+  base::RunLoop().RunUntilIdle();
+
+  GetServiceProperty(shill::kEapIdentityProperty, &identity);
+  EXPECT_EQ("upn-santest@ad.corp.example.com-suffix", identity);
+  EXPECT_EQ(2, network_properties_changed_count_);
+}
+
 }  // namespace chromeos
diff --git a/chromeos/network/client_cert_util.cc b/chromeos/network/client_cert_util.cc
index f440dcb..0fface7 100644
--- a/chromeos/network/client_cert_util.cc
+++ b/chromeos/network/client_cert_util.cc
@@ -41,6 +41,9 @@ std::string GetStringFromDictionary(const base::DictionaryValue& dict,
 void GetClientCertTypeAndPattern(
     const base::DictionaryValue& dict_with_client_cert,
     ClientCertConfig* cert_config) {
+  dict_with_client_cert.GetStringWithoutPathExpansion(
+      ::onc::eap::kIdentity, &cert_config->policy_identity);
+
   using namespace ::onc::client_cert;
   dict_with_client_cert.GetStringWithoutPathExpansion(
       kClientCertType, &cert_config->client_cert_type);
diff --git a/chromeos/network/client_cert_util.h b/chromeos/network/client_cert_util.h
index c1fd42b..6b41413 100644
--- a/chromeos/network/client_cert_util.h
+++ b/chromeos/network/client_cert_util.h
@@ -46,6 +46,9 @@ struct CHROMEOS_EXPORT ClientCertConfig {
 
   // If |client_cert_type| equals kPattern, this contains the pattern.
   CertificatePattern pattern;
+
+  // The value of kIdentity, to enable substitutions.
+  std::string policy_identity;
 };
 
 // Returns true only if any fields set in this pattern match exactly with