Added a PolicyCertVerifier that uses the trust anchors from the ONC policies.

The MultiThreadedCertVerifier can optionally use a CertTrustAnchorProvider to
get a list of additional certificates to trust, without importing them into the
NSS database. This CL wraps the MultiThreadedCertVerifier with a custom verifier
that includes a trust anchor provider.

The trust anchor provider returns all the certificates from the user ONC policy
that have the Web trust flag. The PolicyCertVerifier also writes a preference
in the Profile once any such certificate is used.

This feature is currently behind a flag, until a warning UI is implemented.
The warning should be displayed if UsedPolicyCertificates() is true for the
given profile.

TBR=pneubeck@chromium.org
BUG=216495

NOTE: this is a reland of https://codereview.chromium.org/13035003, with tests fixed.


Review URL: https://chromiumcodereview.appspot.com/13532005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@192324 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 115 insertions, 58 deletions

diff --git a/chromeos/network/onc/onc_certificate_importer.cc b/chromeos/network/onc/onc_certificate_importer.cc
index 3407971..0dd9325 100644
--- a/chromeos/network/onc/onc_certificate_importer.cc
+++ b/chromeos/network/onc/onc_certificate_importer.cc
@@ -34,12 +34,13 @@ const char kX509CertificateHeader[] = "X509 CERTIFICATE";
 namespace chromeos {
 namespace onc {
 
-CertificateImporter::CertificateImporter(bool allow_web_trust)
-    : allow_web_trust_(allow_web_trust) {
+CertificateImporter::CertificateImporter(bool allow_trust_imports)
+    : allow_trust_imports_(allow_trust_imports) {
 }
 
 CertificateImporter::ParseResult CertificateImporter::ParseAndStoreCertificates(
-    const base::ListValue& certificates) {
+    const base::ListValue& certificates,
+    net::CertificateList* onc_trusted_certificates) {
   size_t successful_imports = 0;
   for (size_t i = 0; i < certificates.GetSize(); ++i) {
     const base::DictionaryValue* certificate = NULL;
@@ -48,7 +49,7 @@ CertificateImporter::ParseResult CertificateImporter::ParseAndStoreCertificates(
 
     VLOG(2) << "Parsing certificate at index " << i << ": " << *certificate;
 
-    if (!ParseAndStoreCertificate(*certificate)) {
+    if (!ParseAndStoreCertificate(*certificate, onc_trusted_certificates)) {
       ONC_LOG_ERROR(
           base::StringPrintf("Cannot parse certificate at index %zu", i));
     } else {
@@ -66,37 +67,6 @@ CertificateImporter::ParseResult CertificateImporter::ParseAndStoreCertificates(
   }
 }
 
-bool CertificateImporter::ParseAndStoreCertificate(
-    const base::DictionaryValue& certificate) {
-  // Get out the attributes of the given certificate.
-  std::string guid;
-  certificate.GetString(certificate::kGUID, &guid);
-  DCHECK(!guid.empty());
-
-  bool remove = false;
-  if (certificate.GetBoolean(kRemove, &remove) && remove) {
-    if (!DeleteCertAndKeyByNickname(guid)) {
-      ONC_LOG_ERROR("Unable to delete certificate");
-      return false;
-    } else {
-      return true;
-    }
-  }
-
-  // Not removing, so let's get the data we need to add this certificate.
-  std::string cert_type;
-  certificate.GetString(certificate::kType, &cert_type);
-  if (cert_type == certificate::kServer ||
-      cert_type == certificate::kAuthority) {
-    return ParseServerOrCaCertificate(cert_type, guid, certificate);
-  } else if (cert_type == certificate::kClient) {
-    return ParseClientCertificate(guid, certificate);
-  }
-
-  NOTREACHED();
-  return false;
-}
-
 // static
 void CertificateImporter::ListCertsWithNickname(const std::string& label,
                                                 net::CertificateList* result) {
@@ -155,11 +125,45 @@ bool CertificateImporter::DeleteCertAndKeyByNickname(const std::string& label) {
   return result;
 }
 
+bool CertificateImporter::ParseAndStoreCertificate(
+    const base::DictionaryValue& certificate,
+    net::CertificateList* onc_trusted_certificates) {
+  // Get out the attributes of the given certificate.
+  std::string guid;
+  certificate.GetString(certificate::kGUID, &guid);
+  DCHECK(!guid.empty());
+
+  bool remove = false;
+  if (certificate.GetBoolean(kRemove, &remove) && remove) {
+    if (!DeleteCertAndKeyByNickname(guid)) {
+      ONC_LOG_ERROR("Unable to delete certificate");
+      return false;
+    } else {
+      return true;
+    }
+  }
+
+  // Not removing, so let's get the data we need to add this certificate.
+  std::string cert_type;
+  certificate.GetString(certificate::kType, &cert_type);
+  if (cert_type == certificate::kServer ||
+      cert_type == certificate::kAuthority) {
+    return ParseServerOrCaCertificate(
+        cert_type, guid, certificate, onc_trusted_certificates);
+  } else if (cert_type == certificate::kClient) {
+    return ParseClientCertificate(guid, certificate);
+  }
+
+  NOTREACHED();
+  return false;
+}
+
 bool CertificateImporter::ParseServerOrCaCertificate(
     const std::string& cert_type,
     const std::string& guid,
-    const base::DictionaryValue& certificate) {
-  bool web_trust = false;
+    const base::DictionaryValue& certificate,
+    net::CertificateList* onc_trusted_certificates) {
+  bool web_trust_flag = false;
   const base::ListValue* trust_list = NULL;
   if (certificate.GetList(certificate::kTrust, &trust_list)) {
     for (size_t i = 0; i < trust_list->GetSize(); ++i) {
@@ -170,7 +174,7 @@ bool CertificateImporter::ParseServerOrCaCertificate(
       if (trust_type == certificate::kWeb) {
         // "Web" implies that the certificate is to be trusted for SSL
         // identification.
-        web_trust = true;
+        web_trust_flag = true;
       } else {
         ONC_LOG_ERROR("Certificate contains unknown trust type " + trust_type);
         return false;
@@ -178,9 +182,12 @@ bool CertificateImporter::ParseServerOrCaCertificate(
     }
   }
 
-  if (web_trust && !allow_web_trust_) {
-    LOG(WARNING) << "Web trust not granted for certificate: " << guid;
-    web_trust = false;
+  bool import_with_ssl_trust = false;
+  if (web_trust_flag) {
+    if (!allow_trust_imports_)
+      LOG(WARNING) << "Web trust not granted for certificate: " << guid;
+    else
+      import_with_ssl_trust = true;
   }
 
   std::string x509_data;
@@ -276,7 +283,7 @@ bool CertificateImporter::ParseServerOrCaCertificate(
   cert_list.push_back(x509_cert);
   net::NSSCertDatabase::ImportCertFailureList failures;
   bool success = false;
-  net::NSSCertDatabase::TrustBits trust = web_trust ?
+  net::NSSCertDatabase::TrustBits trust = import_with_ssl_trust ?
                                           net::NSSCertDatabase::TRUSTED_SSL :
                                           net::NSSCertDatabase::TRUST_DEFAULT;
   if (cert_type == certificate::kServer) {
@@ -295,6 +302,9 @@ bool CertificateImporter::ParseServerOrCaCertificate(
     return false;
   }
 
+  if (web_trust_flag && onc_trusted_certificates)
+    onc_trusted_certificates->push_back(x509_cert);
+
   return true;
 }
 
diff --git a/chromeos/network/onc/onc_certificate_importer.h b/chromeos/network/onc/onc_certificate_importer.h
index 5eddbb7..ebfb9c6 100644
--- a/chromeos/network/onc/onc_certificate_importer.h
+++ b/chromeos/network/onc/onc_certificate_importer.h
@@ -40,22 +40,21 @@ class CHROMEOS_EXPORT CertificateImporter {
 
   // During import with ParseCertificate(), Web trust is only applied to Server
   // and Authority certificates with the Trust attribute "Web" if the
-  // |allow_web_trust| permission is granted, otherwise the attribute is
+  // |allow_trust_imports| permission is granted, otherwise the attribute is
   // ignored.
-  explicit CertificateImporter(bool allow_web_trust);
+  explicit CertificateImporter(bool allow_trust_imports);
 
   // Parses and stores the certificates in |onc_certificates| into the
   // certificate store. If the "Remove" field of a certificate is enabled, then
   // removes the certificate from the store instead of importing. Returns the
   // result of the parse operation. In case of IMPORT_INCOMPLETE, some of the
   // certificates may be stored/removed successfully while others had errors.
+  // If |onc_trusted_certificates| is not NULL then it will be filled with the
+  // list of certificates that requested the Web trust flag.
   // If no error occurred, returns IMPORT_OK.
   ParseResult ParseAndStoreCertificates(
-      const base::ListValue& onc_certificates);
-
-  // Parses and stores/removes |certificate| in/from the certificate
-  // store. Returns true if the operation succeeded.
-  bool ParseAndStoreCertificate(const base::DictionaryValue& certificate);
+      const base::ListValue& onc_certificates,
+      net::CertificateList* onc_trusted_certificates);
 
   // Lists the certificates that have the string |label| as their certificate
   // nickname (exact match).
@@ -68,16 +67,24 @@ class CHROMEOS_EXPORT CertificateImporter {
   static bool DeleteCertAndKeyByNickname(const std::string& label);
 
  private:
-  bool ParseServerOrCaCertificate(const std::string& cert_type,
-                                  const std::string& guid,
-                                  const base::DictionaryValue& certificate);
+  // Parses and stores/removes |certificate| in/from the certificate
+  // store. Returns true if the operation succeeded.
+  bool ParseAndStoreCertificate(
+      const base::DictionaryValue& certificate,
+      net::CertificateList* onc_trusted_certificates);
+
+  bool ParseServerOrCaCertificate(
+      const std::string& cert_type,
+      const std::string& guid,
+      const base::DictionaryValue& certificate,
+      net::CertificateList* onc_trusted_certificates);
 
   bool ParseClientCertificate(const std::string& guid,
                               const base::DictionaryValue& certificate);
 
   // Whether certificates with Trust attribute "Web" should be stored with web
   // trust.
-  bool allow_web_trust_;
+  bool allow_trust_imports_;
 
   DISALLOW_COPY_AND_ASSIGN(CertificateImporter);
 };
diff --git a/chromeos/network/onc/onc_certificate_importer_unittest.cc b/chromeos/network/onc/onc_certificate_importer_unittest.cc
index 84f29f7..e8d800d 100644
--- a/chromeos/network/onc/onc_certificate_importer_unittest.cc
+++ b/chromeos/network/onc/onc_certificate_importer_unittest.cc
@@ -90,17 +90,21 @@ class ONCCertificateImporterTest : public testing::Test {
     certificates->GetDictionary(0, &certificate);
     certificate->GetStringWithoutPathExpansion(certificate::kGUID, guid);
 
+    web_trust_certificates_.clear();
     CertificateImporter importer(true /* allow web trust */);
     EXPECT_EQ(CertificateImporter::IMPORT_OK,
-              importer.ParseAndStoreCertificates(*certificates));
+              importer.ParseAndStoreCertificates(*certificates,
+                                                 &web_trust_certificates_));
 
-    net::CertificateList result_list;
-    CertificateImporter::ListCertsWithNickname(*guid, &result_list);
-    ASSERT_EQ(1ul, result_list.size());
-    EXPECT_EQ(expected_type, GetCertType(result_list[0]->os_cert_handle()));
+    result_list_.clear();
+    CertificateImporter::ListCertsWithNickname(*guid, &result_list_);
+    ASSERT_EQ(1ul, result_list_.size());
+    EXPECT_EQ(expected_type, GetCertType(result_list_[0]->os_cert_handle()));
   }
 
   scoped_refptr<net::CryptoModule> slot_;
+  net::CertificateList result_list_;
+  net::CertificateList web_trust_certificates_;
 
  private:
   net::CertificateList ListCertsInSlot(PK11SlotInfo* slot) {
@@ -135,6 +139,7 @@ class ONCCertificateImporterTest : public testing::Test {
 TEST_F(ONCCertificateImporterTest, AddClientCertificate) {
   std::string guid;
   AddCertificateFromFile("certificate-client.onc", net::USER_CERT, &guid);
+  EXPECT_TRUE(web_trust_certificates_.empty());
 
   SECKEYPrivateKeyList* privkey_list =
       PK11_ListPrivKeysInSlot(slot_->os_module_handle(), NULL, NULL);
@@ -179,6 +184,11 @@ TEST_F(ONCCertificateImporterTest, AddServerCertificate) {
   SECKEYPublicKeyList* pubkey_list =
       PK11_ListPublicKeysInSlot(slot_->os_module_handle(), NULL);
   EXPECT_FALSE(pubkey_list);
+
+  ASSERT_EQ(1u, web_trust_certificates_.size());
+  ASSERT_EQ(1u, result_list_.size());
+  EXPECT_TRUE(CERT_CompareCerts(result_list_[0]->os_cert_handle(),
+                                web_trust_certificates_[0]->os_cert_handle()));
 }
 
 TEST_F(ONCCertificateImporterTest, AddWebAuthorityCertificate) {
@@ -192,6 +202,25 @@ TEST_F(ONCCertificateImporterTest, AddWebAuthorityCertificate) {
   SECKEYPublicKeyList* pubkey_list =
       PK11_ListPublicKeysInSlot(slot_->os_module_handle(), NULL);
   EXPECT_FALSE(pubkey_list);
+
+  ASSERT_EQ(1u, web_trust_certificates_.size());
+  ASSERT_EQ(1u, result_list_.size());
+  EXPECT_TRUE(CERT_CompareCerts(result_list_[0]->os_cert_handle(),
+                                web_trust_certificates_[0]->os_cert_handle()));
+}
+
+TEST_F(ONCCertificateImporterTest, AddAuthorityCertificateWithoutWebTrust) {
+  std::string guid;
+  AddCertificateFromFile("certificate-authority.onc", net::CA_CERT, &guid);
+  EXPECT_TRUE(web_trust_certificates_.empty());
+
+  SECKEYPrivateKeyList* privkey_list =
+      PK11_ListPrivKeysInSlot(slot_->os_module_handle(), NULL, NULL);
+  EXPECT_FALSE(privkey_list);
+
+  SECKEYPublicKeyList* pubkey_list =
+      PK11_ListPublicKeysInSlot(slot_->os_module_handle(), NULL);
+  EXPECT_FALSE(pubkey_list);
 }
 
 class ONCCertificateImporterTestWithParam :
diff --git a/chromeos/test/data/network/certificate-authority.onc b/chromeos/test/data/network/certificate-authority.onc
new file mode 100644
index 0000000..3562250
--- /dev/null
+++ b/chromeos/test/data/network/certificate-authority.onc
@@ -0,0 +1,11 @@
+{
+  "Certificates": [
+    {
+      "GUID": "{f998f760-272b-6939-4c2beffe428697ab}",
+      "Type": "Authority",
+      "X509": "MIIDojCCAwugAwIBAgIJAKGvi5ZgEWDVMA0GCSqGSIb3DQEBBAUAMIGTMRUwEwYDVQQKEwxHb29nbGUsIEluYy4xETAPBgNVBAsTCENocm9tZU9TMSIwIAYJKoZIhvcNAQkBFhNnc3BlbmNlckBnb29nbGUuY29tMRowGAYDVQQHExFNb3VudGFpbiBWaWV3LCBDQTELMAkGA1UECBMCQ0ExCzAJBgNVBAYTAlVTMQ0wCwYDVQQDEwRsbWFvMB4XDTExMDMxNjIzNDcxMFoXDTEyMDMxNTIzNDcxMFowgZMxFTATBgNVBAoTDEdvb2dsZSwgSW5jLjERMA8GA1UECxMIQ2hyb21lT1MxIjAgBgkqhkiG9w0BCQEWE2dzcGVuY2VyQGdvb2dsZS5jb20xGjAYBgNVBAcTEU1vdW50YWluIFZpZXcsIENBMQswCQYDVQQIEwJDQTELMAkGA1UEBhMCVVMxDTALBgNVBAMTBGxtYW8wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMDX6BQz2JUzIAVjetiXxDznd2wdqVqVHfNkbSRW+xBywgqUaIXmFEGUol7VzPfmeFV8o8ok/eFlQB0h6ycqgwwMd0KjtJs2ys/k0F5GuN0G7fsgr+NRnhVgxj21yF6gYTN/8a9kscla/svdmp8ekexbALFnghbLBx3CgcqUxT+tAgMBAAGjgfswgfgwDAYDVR0TBAUwAwEB/zAdBgNVHQ4EFgQUbYygbSkl4kpjCNuxoezFGupA97UwgcgGA1UdIwSBwDCBvYAUbYygbSkl4kpjCNuxoezFGupA97WhgZmkgZYwgZMxFTATBgNVBAoTDEdvb2dsZSwgSW5jLjERMA8GA1UECxMIQ2hyb21lT1MxIjAgBgkqhkiG9w0BCQEWE2dzcGVuY2VyQGdvb2dsZS5jb20xGjAYBgNVBAcTEU1vdW50YWluIFZpZXcsIENBMQswCQYDVQQIEwJDQTELMAkGA1UEBhMCVVMxDTALBgNVBAMTBGxtYW+CCQChr4uWYBFg1TANBgkqhkiG9w0BAQQFAAOBgQCDq9wiQ4uVuf1CQU3sXfXCy1yqi5m8AsO9FxHvah5/SVFNwKllqTfedpCaWEswJ55YAojW9e+pY2Fh3Fo/Y9YkF88KCtLuBjjqDKCRLxF4LycjHODKyQQ7mN/t5AtP9yKOsNvWF+M4IfReg51kohau6FauQx87by5NIRPdkNPvkQ=="
+    }
+  ],
+  "NetworkConfigurations": [],
+  "Type": "UnencryptedConfiguration"
+}