Commit dd1fbeb3 authored by Pavol Marko's avatar Pavol Marko Committed by Commit Bot

Use ONC-provided per-extension certificates for sign-in profile extensions

If certificates have been specified in ONC policy to be used by a
sign-in screen extension, PolicyCertService now uses them.
The mapping between extension id (from policy) and StoragePartition is
performed in PolicyCertService (see comments in
PolicyCertService::GetPolicyCertificatesForStoragePartition).
Extension-specific certificates are only allowed if:
(*) The extension has isolated storage, i.e. it has its own StoragePartition
    and
(*) The Profile is using CertVerifierBuiltin (which is unconditionally true
    for the sign-in screen Profile since CL:1750004).

A browsertest has been added to ensure that the other StoragePartitions in the
sign-in screen Profile do not respect the additional extension-specific
certificate.

Bug: 939344
Test: browser_tests --gtest_filter=*PolicyProvidedCertsForSigninExtensionTest*
Change-Id: If29413049a46ee4f742718253dbbbc7ecdc31ae4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1702425
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: default avatarRyan Sleevi <rsleevi@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693789}
parent 6c323f54
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "chrome/browser/chromeos/policy/user_network_configuration_updater.h" #include "chrome/browser/chromeos/policy/user_network_configuration_updater.h"
#include "chrome/browser/chromeos/policy/user_network_configuration_updater_factory.h" #include "chrome/browser/chromeos/policy/user_network_configuration_updater_factory.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/net/profile_network_context_service.h" #include "chrome/browser/net/profile_network_context_service.h"
#include "chrome/browser/net/profile_network_context_service_factory.h" #include "chrome/browser/net/profile_network_context_service_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
...@@ -22,8 +23,11 @@ ...@@ -22,8 +23,11 @@
#include "components/user_manager/user_manager.h" #include "components/user_manager/user_manager.h"
#include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/storage_partition.h"
#include "extensions/browser/extension_util.h"
#include "net/cert/x509_certificate.h" #include "net/cert/x509_certificate.h"
#include "services/network/nss_temp_certs_cache_chromeos.h" #include "services/network/nss_temp_certs_cache_chromeos.h"
#include "url/gurl.h"
namespace policy { namespace policy {
...@@ -97,6 +101,85 @@ void PolicyCertService::GetPolicyCertificatesForStoragePartition( ...@@ -97,6 +101,85 @@ void PolicyCertService::GetPolicyCertificatesForStoragePartition(
*out_all_server_and_authority_certificates = *out_all_server_and_authority_certificates =
profile_wide_all_server_and_authority_certs_; profile_wide_all_server_and_authority_certs_;
*out_trust_anchors = profile_wide_trust_anchors_; *out_trust_anchors = profile_wide_trust_anchors_;
if (policy_certificate_provider_->GetExtensionIdsWithPolicyCertificates()
.empty()) {
return;
}
// The following code adds policy-provided extension specific certificates.
// Policy can specify these keyed by extension ID.
// In general, there is no direct mapping from a StoragePartition path to an
// extension ID, because extensions could be using the default
// StoragePartition of the Profile.
// However, for extensions with isolated storage, the extension will be in a
// StoragePartition that is exclusively used by this extension.
// Policy-provided extension specific certificates are thus only allowed for
// extensions with isolated storage.
// The following code checks those preconditions and attempts to find the
// extension ID (among extensions IDs with policy-provided certificates) that
// corresponds to |partition_path|.
// Only allow certificates that are specific to |partition_path| if the
// built-in certificate verifier is active. The platform certificate verifier
// is not able to isolate contexts from each other.
auto* profile_network_context =
ProfileNetworkContextServiceFactory::GetForContext(profile_);
if (!profile_network_context->using_builtin_cert_verifier()) {
LOG(ERROR) << "Ignoring extension-scoped policy certificates";
return;
}
base::FilePath default_storage_partition_path =
content::BrowserContext::GetDefaultStoragePartition(profile_)->GetPath();
// Among the extension IDs that have policy-provided certificates, attempt to
// find the extension ID which corresponds to |partition_path|.
// This is done by iterating the extension IDs because there's no trivial
// conversion from |partition_path| to extension ID as explained above.
std::string current_extension_id_with_policy_certificates;
std::set<std::string> extension_ids_with_policy_certificates =
policy_certificate_provider_->GetExtensionIdsWithPolicyCertificates();
for (const auto& extension_id : extension_ids_with_policy_certificates) {
const GURL extension_site =
extensions::util::GetSiteForExtensionId(extension_id, profile_);
// Only allow policy-provided certificates for extensions with isolated
// storage. Also sanity-check that it's not the default partition.
content::StoragePartition* extension_partition =
content::BrowserContext::GetStoragePartitionForSite(
profile_, extension_site, /*can_create=*/false);
if (!extension_partition)
continue;
if (!extensions::util::SiteHasIsolatedStorage(extension_site, profile_) ||
extension_partition->GetPath() == default_storage_partition_path) {
LOG(ERROR) << "Ignoring policy certificates for " << extension_id
<< " because it does not have isolated storage";
continue;
}
if (partition_path == extension_partition->GetPath()) {
current_extension_id_with_policy_certificates = extension_id;
break;
}
}
if (current_extension_id_with_policy_certificates.empty())
return;
net::CertificateList extension_all_server_and_authority_certificates =
policy_certificate_provider_->GetAllServerAndAuthorityCertificates(
chromeos::onc::CertificateScope::ForExtension(
current_extension_id_with_policy_certificates));
out_all_server_and_authority_certificates->insert(
out_all_server_and_authority_certificates->end(),
extension_all_server_and_authority_certificates.begin(),
extension_all_server_and_authority_certificates.end());
net::CertificateList extension_trust_anchors =
policy_certificate_provider_->GetWebTrustedCertificates(
chromeos::onc::CertificateScope::ForExtension(
current_extension_id_with_policy_certificates));
out_trust_anchors->insert(out_trust_anchors->end(),
extension_trust_anchors.begin(),
extension_trust_anchors.end());
} }
bool PolicyCertService::UsedPolicyCertificates() const { bool PolicyCertService::UsedPolicyCertificates() const {
......
...@@ -75,6 +75,10 @@ class PolicyCertService : public KeyedService, ...@@ -75,6 +75,10 @@ class PolicyCertService : public KeyedService,
// PolicyCertificateProvider::Observer: // PolicyCertificateProvider::Observer:
void OnPolicyProvidedCertsChanged() override; void OnPolicyProvidedCertsChanged() override;
// Fills *|out_all_server_and_authority_certificates| and *|out_trust_anchors|
// with policy-provided certificates that should be used when verifying a
// server certificate for Web requests from the StoragePartition identified by
// |partition_path|.
void GetPolicyCertificatesForStoragePartition( void GetPolicyCertificatesForStoragePartition(
const base::FilePath& partition_path, const base::FilePath& partition_path,
net::CertificateList* out_all_server_and_authority_certificates, net::CertificateList* out_all_server_and_authority_certificates,
......
...@@ -569,6 +569,9 @@ ProfileNetworkContextService::CreateNetworkContextParams( ...@@ -569,6 +569,9 @@ ProfileNetworkContextService::CreateNetworkContextParams(
network_context_params->use_builtin_cert_verifier = network_context_params->use_builtin_cert_verifier =
using_builtin_cert_verifier_; using_builtin_cert_verifier_;
bool profile_supports_policy_certs = false;
if (chromeos::ProfileHelper::IsSigninProfile(profile_))
profile_supports_policy_certs = true;
user_manager::UserManager* user_manager = user_manager::UserManager::Get(); user_manager::UserManager* user_manager = user_manager::UserManager::Get();
if (user_manager) { if (user_manager) {
const user_manager::User* user = const user_manager::User* user =
...@@ -580,16 +583,18 @@ ProfileNetworkContextService::CreateNetworkContextParams( ...@@ -580,16 +583,18 @@ ProfileNetworkContextService::CreateNetworkContextParams(
if (user && !user->username_hash().empty()) { if (user && !user->username_hash().empty()) {
network_context_params->username_hash = user->username_hash(); network_context_params->username_hash = user->username_hash();
network_context_params->nss_path = profile_->GetPath(); network_context_params->nss_path = profile_->GetPath();
if (policy::PolicyCertServiceFactory::CreateAndStartObservingForProfile( profile_supports_policy_certs = true;
profile_)) {
const policy::PolicyCertService* policy_cert_service =
policy::PolicyCertServiceFactory::GetForProfile(profile_);
network_context_params->initial_additional_certificates =
GetAdditionalCertificates(
policy_cert_service, GetPartitionPath(relative_partition_path));
}
} }
} }
if (profile_supports_policy_certs &&
policy::PolicyCertServiceFactory::CreateAndStartObservingForProfile(
profile_)) {
const policy::PolicyCertService* policy_cert_service =
policy::PolicyCertServiceFactory::GetForProfile(profile_);
network_context_params->initial_additional_certificates =
GetAdditionalCertificates(policy_cert_service,
GetPartitionPath(relative_partition_path));
}
#endif #endif
// Should be initialized with existing per-profile CORS access lists. // Should be initialized with existing per-profile CORS access lists.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment