diff --git a/components/policy/core/common/cloud/cloud_policy_validator.cc b/components/policy/core/common/cloud/cloud_policy_validator.cc index db2f6e570dcac..ce2056f7d7ed0 100644 --- a/components/policy/core/common/cloud/cloud_policy_validator.cc +++ b/components/policy/core/common/cloud/cloud_policy_validator.cc @@ -455,7 +455,8 @@ bool CloudPolicyValidatorBase::CheckNewPublicKeyVerificationSignature() { policy_->new_public_key_verification_data_signature(), em::PolicyFetchRequest::SHA256_RSA) && CheckDomainInPublicKeyVerificationData( - policy_->new_public_key_verification_data())) { + policy_->new_public_key_verification_data(), + policy_->new_public_key())) { UMA_HISTOGRAM_ENUMERATION(kMetricKeySignatureVerification, MetricKeySignatureVerification::kSuccess); // Signature verification succeeded - return success to the caller. @@ -543,7 +544,8 @@ std::string CloudPolicyValidatorBase::ExtractDomainFromPolicy() { } bool CloudPolicyValidatorBase::CheckDomainInPublicKeyVerificationData( - const std::string& new_public_key_verification_data) { + const std::string& new_public_key_verification_data, + const std::string& expected_new_public_key) { em::PublicKeyVerificationData public_key_data; if (!public_key_data.ParseFromString(new_public_key_verification_data)) { LOG_POLICY(ERROR, POLICY_FETCHING) @@ -551,6 +553,17 @@ bool CloudPolicyValidatorBase::CheckDomainInPublicKeyVerificationData( << "Failed to deserialize new public key."; return false; } + // The root-of-trust signature certifies a SPECIFIC key for a SPECIFIC + // domain. The certified key must be the same key that will be used to + // verify policy_data; otherwise an attacker can splice a captured + // root-signed blob onto a response carrying their own key. + if (public_key_data.new_public_key() != expected_new_public_key) { + LOG_POLICY(ERROR, POLICY_FETCHING) + << PolicyTypeLogPrefix(policy_type_, settings_entity_id_) + << "Key mismatch: root-certified key differs from supplied " + "new_public_key."; + return false; + } if (public_key_data.domain() != ExtractDomainFromPolicy()) { LOG_POLICY(ERROR, POLICY_FETCHING) << PolicyTypeLogPrefix(policy_type_, settings_entity_id_) @@ -631,7 +644,7 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckCachedKey() { if (VerifySignature(new_cached_key_, verification_key_.value(), new_cached_key_signature_, em::PolicyFetchRequest::SHA256_RSA) && - CheckDomainInPublicKeyVerificationData(new_cached_key_)) { + CheckDomainInPublicKeyVerificationData(new_cached_key_, cached_key_)) { UMA_HISTOGRAM_ENUMERATION(kMetricKeySignatureVerification, MetricKeySignatureVerification::kSuccess); // Signature verification succeeded - return success to the caller. diff --git a/components/policy/core/common/cloud/cloud_policy_validator.h b/components/policy/core/common/cloud/cloud_policy_validator.h index ba25f87be1c5a..485a0f540630b 100644 --- a/components/policy/core/common/cloud/cloud_policy_validator.h +++ b/components/policy/core/common/cloud/cloud_policy_validator.h @@ -348,10 +348,12 @@ class POLICY_EXPORT CloudPolicyValidatorBase { // empty string if the policy does not contain a username field. std::string ExtractDomainFromPolicy(); - // Returns if the domain from the new_public_key_verification_data matches - // the domain extracted from the |policy_|. + // Returns true iff the root-signed |new_public_key_verification_data| blob + // certifies exactly |expected_new_public_key| for the domain extracted from + // |policy_|. Both the domain AND the certified key must match. bool CheckDomainInPublicKeyVerificationData( - const std::string& new_public_key_verification_data); + const std::string& new_public_key_verification_data, + const std::string& expected_new_public_key); // Sets the owning domain used to verify new public keys, and ensures that // callers don't try to set conflicting values.