Revert 192102 "Added a PolicyCertVerifier that uses the trust an..."

> 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.
> 
> BUG=216495
> 
> Review URL: https://codereview.chromium.org/13035003

TBR=joaodasilva@chromium.org
Review URL: https://codereview.chromium.org/13581002

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

4 files changed, 58 insertions, 115 deletions

diff --git a/chromeos/network/onc/onc_certificate_importer.cc b/chromeos/network/onc/onc_certificate_importer.cc
index 0dd9325..3407971 100644
--- a/chromeos/network/onc/onc_certificate_importer.cc
+++ b/chromeos/network/onc/onc_certificate_importer.cc
@@ -34,13 +34,12 @@ const char kX509CertificateHeader[] = "X509 CERTIFICATE";
 namespace chromeos {
 namespace onc {
 
-CertificateImporter::CertificateImporter(bool allow_trust_imports)
-    : allow_trust_imports_(allow_trust_imports) {
+CertificateImporter::CertificateImporter(bool allow_web_trust)
+    : allow_web_trust_(allow_web_trust) {
 }
 
 CertificateImporter::ParseResult CertificateImporter::ParseAndStoreCertificates(
-    const base::ListValue& certificates,
-    net::CertificateList* onc_trusted_certificates) {
+    const base::ListValue& certificates) {
   size_t successful_imports = 0;
   for (size_t i = 0; i < certificates.GetSize(); ++i) {
     const base::DictionaryValue* certificate = NULL;
@@ -49,7 +48,7 @@ CertificateImporter::ParseResult CertificateImporter::ParseAndStoreCertificates(
 
     VLOG(2) << "Parsing certificate at index " << i << ": " << *certificate;
 
-    if (!ParseAndStoreCertificate(*certificate, onc_trusted_certificates)) {
+    if (!ParseAndStoreCertificate(*certificate)) {
       ONC_LOG_ERROR(
           base::StringPrintf("Cannot parse certificate at index %zu", i));
     } else {
@@ -67,6 +66,37 @@ 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) {
@@ -125,45 +155,11 @@ 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,
-    net::CertificateList* onc_trusted_certificates) {
-  bool web_trust_flag = false;
+    const base::DictionaryValue& certificate) {
+  bool web_trust = false;
   const base::ListValue* trust_list = NULL;
   if (certificate.GetList(certificate::kTrust, &trust_list)) {
     for (size_t i = 0; i < trust_list->GetSize(); ++i) {
@@ -174,7 +170,7 @@ bool CertificateImporter::ParseServerOrCaCertificate(
       if (trust_type == certificate::kWeb) {
         // "Web" implies that the certificate is to be trusted for SSL
         // identification.
-        web_trust_flag = true;
+        web_trust = true;
       } else {
         ONC_LOG_ERROR("Certificate contains unknown trust type " + trust_type);
         return false;
@@ -182,12 +178,9 @@ bool CertificateImporter::ParseServerOrCaCertificate(
     }
   }
 
-  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;
+  if (web_trust && !allow_web_trust_) {
+    LOG(WARNING) << "Web trust not granted for certificate: " << guid;
+    web_trust = false;
   }
 
   std::string x509_data;
@@ -283,7 +276,7 @@ bool CertificateImporter::ParseServerOrCaCertificate(
   cert_list.push_back(x509_cert);
   net::NSSCertDatabase::ImportCertFailureList failures;
   bool success = false;
-  net::NSSCertDatabase::TrustBits trust = import_with_ssl_trust ?
+  net::NSSCertDatabase::TrustBits trust = web_trust ?
                                           net::NSSCertDatabase::TRUSTED_SSL :
                                           net::NSSCertDatabase::TRUST_DEFAULT;
   if (cert_type == certificate::kServer) {
@@ -302,9 +295,6 @@ 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 ebfb9c6..5eddbb7 100644
--- a/chromeos/network/onc/onc_certificate_importer.h
+++ b/chromeos/network/onc/onc_certificate_importer.h
@@ -40,21 +40,22 @@ 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_trust_imports| permission is granted, otherwise the attribute is
+  // |allow_web_trust| permission is granted, otherwise the attribute is
   // ignored.
-  explicit CertificateImporter(bool allow_trust_imports);
+  explicit CertificateImporter(bool allow_web_trust);
 
   // 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,
-      net::CertificateList* onc_trusted_certificates);
+      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);
 
   // Lists the certificates that have the string |label| as their certificate
   // nickname (exact match).
@@ -67,24 +68,16 @@ class CHROMEOS_EXPORT CertificateImporter {
   static bool DeleteCertAndKeyByNickname(const std::string& label);
 
  private:
-  // 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 ParseServerOrCaCertificate(const std::string& cert_type,
+                                  const std::string& guid,
+                                  const base::DictionaryValue& certificate);
 
   bool ParseClientCertificate(const std::string& guid,
                               const base::DictionaryValue& certificate);
 
   // Whether certificates with Trust attribute "Web" should be stored with web
   // trust.
-  bool allow_trust_imports_;
+  bool allow_web_trust_;
 
   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 e8d800d..84f29f7 100644
--- a/chromeos/network/onc/onc_certificate_importer_unittest.cc
+++ b/chromeos/network/onc/onc_certificate_importer_unittest.cc
@@ -90,21 +90,17 @@ 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,
-                                                 &web_trust_certificates_));
+              importer.ParseAndStoreCertificates(*certificates));
 
-    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()));
+    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()));
   }
 
   scoped_refptr<net::CryptoModule> slot_;
-  net::CertificateList result_list_;
-  net::CertificateList web_trust_certificates_;
 
  private:
   net::CertificateList ListCertsInSlot(PK11SlotInfo* slot) {
@@ -139,7 +135,6 @@ 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);
@@ -184,11 +179,6 @@ 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) {
@@ -202,25 +192,6 @@ 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
deleted file mode 100644
index 3562250..0000000
--- a/chromeos/test/data/network/certificate-authority.onc
+++ /dev/null
@@ -1,11 +0,0 @@
-{
-  "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"
-}