onc: Allow specifying scope for policy-provided certificates

Allow ONC policy to specify a Scope for policy-provided certificates. Currently, all consumers only use certificates with profile-wide scope (no Scope specified in ONC). Using extension-scoped certificates will be done in a follow-up CL. Bug: 939344 Test: chromeos_unittests --gtest_filter=*OncParsedCertificates* Change-Id: Ic5b824d3d7fc36b89a2695f5ec0d1099a1600171 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1702308 Reviewed-by: Matt Mueller <mattm@chromium.org> Reviewed-by: Joshua Pawlicki <waffles@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Alexander Hendrich <hendrich@chromium.org> Commit-Queue: Pavol Marko <pmarko@chromium.org> Cr-Commit-Position: refs/heads/master@{#686538}
chromium · Aug 13, 2019 · c14be9c · c14be9c
1 parent c77fe32
commit c14be9c
Show file tree

Hide file tree

Showing 32 changed files with 617 additions and 60 deletions.
diff --git a/chrome/browser/certificate_manager_model.cc b/chrome/browser/certificate_manager_model.cc
@@ -36,6 +36,7 @@
 #include "chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.h"
 #include "chrome/browser/chromeos/policy/user_network_configuration_updater.h"
 #include "chrome/browser/chromeos/policy/user_network_configuration_updater_factory.h"
+#include "chromeos/network/onc/certificate_scope.h"
 #include "chromeos/network/policy_certificate_provider.h"
 #endif
 
@@ -308,11 +309,13 @@ class CertsSourcePolicy : public CertificateManagerModel::CertsSource,
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     switch (mode_) {
       case Mode::kPolicyCertsWithoutWebTrust:
-        RefreshImpl(policy_certs_provider_->GetCertificatesWithoutWebTrust(),
+        RefreshImpl(policy_certs_provider_->GetCertificatesWithoutWebTrust(
+                        chromeos::onc::CertificateScope::Default()),
                     false /* policy_web_trusted */);
         break;
       case Mode::kPolicyCertsWithWebTrust:
-        RefreshImpl(policy_certs_provider_->GetWebTrustedCertificates(),
+        RefreshImpl(policy_certs_provider_->GetWebTrustedCertificates(
+                        chromeos::onc::CertificateScope::Default()),
                     true /* policy_web_trusted */);
         break;
       default:

diff --git a/chrome/browser/certificate_manager_model_unittest.cc b/chrome/browser/certificate_manager_model_unittest.cc
@@ -18,6 +18,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/certificate_provider/certificate_provider.h"
+#include "chromeos/network/onc/certificate_scope.h"
 #include "chromeos/network/policy_certificate_provider.h"
 #endif
 
@@ -202,7 +203,11 @@ class FakePolicyCertificateProvider
     observer_list_.RemoveObserver(observer);
   }
 
-  net::CertificateList GetAllServerAndAuthorityCertificates() const override {
+  net::CertificateList GetAllServerAndAuthorityCertificates(
+      const chromeos::onc::CertificateScope& scope) const override {
+    // The CertificateManagerModel only retrieves profile-wide certificates.
+    EXPECT_EQ(chromeos::onc::CertificateScope::Default(), scope);
+
     net::CertificateList merged;
     merged.insert(merged.end(), web_trusted_certs_.begin(),
                   web_trusted_certs_.end());
@@ -211,20 +216,36 @@ class FakePolicyCertificateProvider
     return merged;
   }
 
-  net::CertificateList GetAllAuthorityCertificates() const override {
+  net::CertificateList GetAllAuthorityCertificates(
+      const chromeos::onc::CertificateScope& scope) const override {
     // This function is not called by CertificateManagerModel.
     NOTREACHED();
     return net::CertificateList();
   }
 
-  net::CertificateList GetWebTrustedCertificates() const override {
+  net::CertificateList GetWebTrustedCertificates(
+      const chromeos::onc::CertificateScope& scope) const override {
+    // The CertificateManagerModel only retrieves profile-wide certificates.
+    EXPECT_EQ(chromeos::onc::CertificateScope::Default(), scope);
+
     return web_trusted_certs_;
   }
 
-  net::CertificateList GetCertificatesWithoutWebTrust() const override {
+  net::CertificateList GetCertificatesWithoutWebTrust(
+      const chromeos::onc::CertificateScope& scope) const override {
+    // The CertificateManagerModel only retrieves profile-wide certificates.
+    EXPECT_EQ(chromeos::onc::CertificateScope::Default(), scope);
+
     return not_web_trusted_certs_;
   }
 
+  const std::set<std::string>& GetExtensionIdsWithPolicyCertificates()
+      const override {
+    // This function is not called by CertificateManagerModel.
+    NOTREACHED();
+    return kNoExtensions;
+  }
+
   void SetPolicyProvidedCertificates(
       const net::CertificateList& web_trusted_certs,
       const net::CertificateList& not_web_trusted_certs) {
@@ -242,6 +263,7 @@ class FakePolicyCertificateProvider
                      true /* check_empty */>::Unchecked observer_list_;
   net::CertificateList web_trusted_certs_;
   net::CertificateList not_web_trusted_certs_;
+  const std::set<std::string> kNoExtensions = {};
 };
 
 class FakeExtensionCertificateProvider : public chromeos::CertificateProvider {

diff --git a/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc b/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc
@@ -80,6 +80,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/upstart/upstart_client.h"
 #include "chromeos/login/login_state/login_state.h"
+#include "chromeos/network/onc/certificate_scope.h"
 #include "chromeos/network/proxy/proxy_config_service_impl.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "chromeos/timezone/timezone_resolver.h"

diff --git a/chrome/browser/chromeos/policy/network_configuration_updater.cc b/chrome/browser/chromeos/policy/network_configuration_updater.cc
@@ -14,6 +14,7 @@
 #include "net/cert/x509_certificate.h"
 #include "net/cert/x509_util_nss.h"
 
+using chromeos::onc::CertificateScope;
 using chromeos::onc::OncParsedCertificates;
 
 namespace policy {
@@ -24,19 +25,37 @@ namespace {
 using ServerOrAuthorityCertPredicate = base::RepeatingCallback<bool(
     const OncParsedCertificates::ServerOrAuthorityCertificate& cert)>;
 
+// Returns a filtered copy of |sever_or_authority_certificates|. The filtered
+// copy will contain  a certificate from the input iff it matches |scope| and
+// executing |predicate| on it returned true.
 net::CertificateList GetFilteredCertificateListFromOnc(
     const std::vector<OncParsedCertificates::ServerOrAuthorityCertificate>&
         server_or_authority_certificates,
+    const CertificateScope& scope,
     ServerOrAuthorityCertPredicate predicate) {
   net::CertificateList certificates;
   for (const auto& server_or_authority_cert :
        server_or_authority_certificates) {
-    if (predicate.Run(server_or_authority_cert))
+    if (server_or_authority_cert.scope() == scope &&
+        predicate.Run(server_or_authority_cert))
       certificates.push_back(server_or_authority_cert.certificate());
   }
   return certificates;
 }
 
+// Returns all extension IDs that were used in a Scope of a one of the
+// |server_or_authority_certificates|.
+std::set<std::string> CollectExtensionIds(
+    const std::vector<OncParsedCertificates::ServerOrAuthorityCertificate>&
+        server_or_authority_certificates) {
+  std::set<std::string> extension_ids;
+  for (const auto& cert : server_or_authority_certificates) {
+    if (cert.scope().is_extension_scoped())
+      extension_ids.insert(cert.scope().extension_id());
+  }
+  return extension_ids;
+}
+
 }  // namespace
 
 NetworkConfigurationUpdater::~NetworkConfigurationUpdater() {
@@ -72,21 +91,22 @@ void NetworkConfigurationUpdater::RemovePolicyProvidedCertsObserver(
 }
 
 net::CertificateList
-NetworkConfigurationUpdater::GetAllServerAndAuthorityCertificates() const {
+NetworkConfigurationUpdater::GetAllServerAndAuthorityCertificates(
+    const CertificateScope& scope) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return GetFilteredCertificateListFromOnc(
-      certs_->server_or_authority_certificates(),
+      certs_->server_or_authority_certificates(), scope,
       base::BindRepeating(
           [](const OncParsedCertificates::ServerOrAuthorityCertificate& cert) {
             return true;
           }));
 }
 
-net::CertificateList NetworkConfigurationUpdater::GetAllAuthorityCertificates()
-    const {
+net::CertificateList NetworkConfigurationUpdater::GetAllAuthorityCertificates(
+    const CertificateScope& scope) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return GetFilteredCertificateListFromOnc(
-      certs_->server_or_authority_certificates(),
+      certs_->server_or_authority_certificates(), scope,
       base::BindRepeating(
           [](const OncParsedCertificates::ServerOrAuthorityCertificate& cert) {
             return cert.type() ==
@@ -95,28 +115,34 @@ net::CertificateList NetworkConfigurationUpdater::GetAllAuthorityCertificates()
           }));
 }
 
-net::CertificateList NetworkConfigurationUpdater::GetWebTrustedCertificates()
-    const {
+net::CertificateList NetworkConfigurationUpdater::GetWebTrustedCertificates(
+    const CertificateScope& scope) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return GetFilteredCertificateListFromOnc(
-      certs_->server_or_authority_certificates(),
+      certs_->server_or_authority_certificates(), scope,
       base::BindRepeating(
           [](const OncParsedCertificates::ServerOrAuthorityCertificate& cert) {
             return cert.web_trust_requested();
           }));
 }
 
 net::CertificateList
-NetworkConfigurationUpdater::GetCertificatesWithoutWebTrust() const {
+NetworkConfigurationUpdater::GetCertificatesWithoutWebTrust(
+    const CertificateScope& scope) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return GetFilteredCertificateListFromOnc(
-      certs_->server_or_authority_certificates(),
+      certs_->server_or_authority_certificates(), scope,
       base::BindRepeating(
           [](const OncParsedCertificates::ServerOrAuthorityCertificate& cert) {
             return !cert.web_trust_requested();
           }));
 }
 
+const std::set<std::string>&
+NetworkConfigurationUpdater::GetExtensionIdsWithPolicyCertificates() const {
+  return extension_ids_with_policy_certificates_;
+}
+
 NetworkConfigurationUpdater::NetworkConfigurationUpdater(
     onc::ONCSource onc_source,
     std::string policy_key,
@@ -288,6 +314,8 @@ void NetworkConfigurationUpdater::ImportCertificates(
     return;
 
   certs_ = std::move(incoming_certs);
+  extension_ids_with_policy_certificates_ =
+      CollectExtensionIds(certs_->server_or_authority_certificates());
 
   if (client_certs_changed)
     ImportClientCertificates();

diff --git a/chrome/browser/chromeos/policy/network_configuration_updater.h b/chrome/browser/chromeos/policy/network_configuration_updater.h
@@ -6,13 +6,15 @@
 #define CHROME_BROWSER_CHROMEOS_POLICY_NETWORK_CONFIGURATION_UPDATER_H_
 
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "base/sequence_checker.h"
+#include "chromeos/network/onc/certificate_scope.h"
 #include "chromeos/network/onc/onc_parsed_certificates.h"
 #include "chromeos/network/policy_certificate_provider.h"
 #include "components/onc/onc_constants.h"
@@ -54,10 +56,17 @@ class NetworkConfigurationUpdater : public chromeos::PolicyCertificateProvider,
       chromeos::PolicyCertificateProvider::Observer* observer) override;
   void RemovePolicyProvidedCertsObserver(
       chromeos::PolicyCertificateProvider::Observer* observer) override;
-  net::CertificateList GetAllServerAndAuthorityCertificates() const override;
-  net::CertificateList GetAllAuthorityCertificates() const override;
-  net::CertificateList GetWebTrustedCertificates() const override;
-  net::CertificateList GetCertificatesWithoutWebTrust() const override;
+  net::CertificateList GetAllServerAndAuthorityCertificates(
+      const chromeos::onc::CertificateScope& scope) const override;
+  net::CertificateList GetAllAuthorityCertificates(
+      const chromeos::onc::CertificateScope& scope) const override;
+  net::CertificateList GetWebTrustedCertificates(
+      const chromeos::onc::CertificateScope& scope) const override;
+  net::CertificateList GetCertificatesWithoutWebTrust(
+      const chromeos::onc::CertificateScope& scope) const override;
+
+  const std::set<std::string>& GetExtensionIdsWithPolicyCertificates()
+      const override;
 
  protected:
   NetworkConfigurationUpdater(
@@ -149,6 +158,7 @@ class NetworkConfigurationUpdater : public chromeos::PolicyCertificateProvider,
 
   // Holds certificates from the last parsed ONC policy.
   std::unique_ptr<chromeos::onc::OncParsedCertificates> certs_;
+  std::set<std::string> extension_ids_with_policy_certificates_;
 
   // Observer list for notifying about ONC-provided server and CA certificate
   // changes.