Added support for signed policy blobs on desktop.

components/policy/core/common/cloud/cloud_policy_validator.cc

Index: components/policy/core/common/cloud/cloud_policy_validator.cc
diff --git a/components/policy/core/common/cloud/cloud_policy_validator.cc b/components/policy/core/common/cloud/cloud_policy_validator.cc
index 5bbc6bc7c99f468ecc7423f7ab058b1b068c9623..f85561dc675a38ea16ae1b681f7bbfc51dfced4d 100644
--- a/components/policy/core/common/cloud/cloud_policy_validator.cc
+++ b/components/policy/core/common/cloud/cloud_policy_validator.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind_helpers.h"
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/histogram.h"
 #include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
@@ -28,6 +29,23 @@ const uint8 kSignatureAlgorithm[] = {
     0xf7, 0x0d, 0x01, 0x01, 0x05, 0x05, 0x00
 };
 
+const char kMetricPolicyKeyVerification[] = "Enterprise.PolicyKeyVerification";
+
+enum MetricPolicyKeyVerification {
+  // UMA metric recorded when the client has no verification key.
+  METRIC_POLICY_KEY_VERIFICATION_KEY_MISSING,
+  // Recorded when the policy being verified has no key signature (e.g. policy
+  // fetched before the server supported the verification key).
+  METRIC_POLICY_KEY_VERIFICATION_SIGNATURE_MISSING,
+  // Recorded when the key signature did not match the expected value (in
+  // theory, this should only happen after key rotation or if the policy cached
+  // on disk has been modified).
+  METRIC_POLICY_KEY_VERIFICATION_FAILED,
+  // Recorded when key verification succeeded.
+  METRIC_POLICY_KEY_VERIFICATION_SUCCEEDED,
+  METRIC_POLICY_KEY_VERIFICATION_SIZE  // Must be the last.
+};
+
 }  // namespace
 
 CloudPolicyValidatorBase::~CloudPolicyValidatorBase() {}
@@ -83,16 +101,22 @@ void CloudPolicyValidatorBase::ValidatePayload() {
   validation_flags_ |= VALIDATE_PAYLOAD;
 }
 
-void CloudPolicyValidatorBase::ValidateSignature(const std::vector<uint8>& key,
-                                                 bool allow_key_rotation) {
+void CloudPolicyValidatorBase::ValidateSignature(
+    const std::string& key,
+    const std::string& verification_key,
+    const std::string& key_signature,
+    bool allow_key_rotation) {
   validation_flags_ |= VALIDATE_SIGNATURE;
-  key_ = std::string(reinterpret_cast<const char*>(vector_as_array(&key)),
-                     key.size());
+  set_verification_key(verification_key);
+  key_ = key;
+  key_signature_ = key_signature;
   allow_key_rotation_ = allow_key_rotation;
 }
 
-void CloudPolicyValidatorBase::ValidateInitialKey() {
+void CloudPolicyValidatorBase::ValidateInitialKey(
+    const std::string& verification_key) {
   validation_flags_ |= VALIDATE_INITIAL_KEY;
+  set_verification_key(verification_key);
 }
 
 void CloudPolicyValidatorBase::ValidateAgainstCurrentPolicy(
@@ -210,6 +234,65 @@ void CloudPolicyValidatorBase::RunChecks() {
   }
 }
 
+// Verifies the |new_public_key_verification_signature| for the |new_public_key|
+// in the policy blob.
+bool CloudPolicyValidatorBase::CheckNewPublicKeyVerificationSignature() {
+  // If there's no local verification key, then just return true (no
+  // validation possible).
+  if (verification_key_.empty()) {
+    UMA_HISTOGRAM_ENUMERATION(kMetricPolicyKeyVerification,
+                              METRIC_POLICY_KEY_VERIFICATION_KEY_MISSING,
+                              METRIC_POLICY_KEY_VERIFICATION_SIZE);
+    return true;
+  }
+
+  if (!policy_->has_new_public_key_verification_signature()) {
+    // Policy does not contain a verification signature, so log an error.
+    LOG(ERROR) << "Policy is missing public_key_verification_signature";
+    UMA_HISTOGRAM_ENUMERATION(kMetricPolicyKeyVerification,
+                              METRIC_POLICY_KEY_VERIFICATION_SIGNATURE_MISSING,
+                              METRIC_POLICY_KEY_VERIFICATION_SIZE);
+    // TODO(atwilson): Return an error on failed signature verification once
+    // our test servers and unittests are returning policy with a verification
+    // signature (http://crbug.com/275291).
+    return true;
+  }
+
+  if (!CheckVerificationKeySignature(
+          policy_->new_public_key(),
+          verification_key_,
+          policy_->new_public_key_verification_signature())) {
+    LOG(ERROR) << "Signature verification failed";
+    UMA_HISTOGRAM_ENUMERATION(kMetricPolicyKeyVerification,
+                              METRIC_POLICY_KEY_VERIFICATION_FAILED,
+                              METRIC_POLICY_KEY_VERIFICATION_SIZE);
+    // TODO(atwilson): Update this code to include the domain name in the
+    // signature, and return an error once the server starts returning this.
+    return true;
+  }
+  // Signature verification succeeded - return success to the caller.
+  UMA_HISTOGRAM_ENUMERATION(kMetricPolicyKeyVerification,
+                            METRIC_POLICY_KEY_VERIFICATION_SUCCEEDED,
+                            METRIC_POLICY_KEY_VERIFICATION_SIZE);
+  return true;
+}
+
+bool CloudPolicyValidatorBase::CheckVerificationKeySignature(
+    const std::string& key,
+    const std::string& verification_key,
+    const std::string& signature) {
+  // TODO(atwilson): Update this routine to include the domain name in the
+  // signed data.
+  return VerifySignature(key, verification_key, signature);
+}
+
+void CloudPolicyValidatorBase::set_verification_key(
+    const std::string& verification_key) {
+  // Make sure we aren't overwriting the verification key with a different key.
+  DCHECK(verification_key_.empty() || verification_key_ == verification_key);
+  verification_key_ = verification_key;
+}
+
 CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckSignature() {
   const std::string* signature_key = &key_;
   if (policy_->has_new_public_key() && allow_key_rotation_) {
@@ -217,9 +300,14 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckSignature() {
     if (!policy_->has_new_public_key_signature() ||
         !VerifySignature(policy_->new_public_key(), key_,
                          policy_->new_public_key_signature())) {
-      LOG(ERROR) << "New public key signature verification failed";
+      LOG(ERROR) << "New public key rotation signature verification failed";
       return VALIDATION_BAD_SIGNATURE;
     }
+
+    if (!CheckNewPublicKeyVerificationSignature()) {
+      LOG(ERROR) << "New public key root verification failed";
+      return VALIDATION_BAD_KEY_VERIFICATION_SIGNATURE;
+    }
   }
 
   if (!policy_->has_policy_data_signature() ||
@@ -229,6 +317,16 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckSignature() {
     return VALIDATION_BAD_SIGNATURE;
   }
 
+  // If a key verification signature is available, then verify the base signing
+  // key as well.
+  if (!key_signature_.empty() &&
+      !CheckVerificationKeySignature(key_, verification_key_, key_signature_)) {

[Mattias Nissler (ping if slow), 2014/01/31 21:00:35]

Wouldn't we fail here for the case where the verification key gets rotated on
Chrome update and we load a cached blob? Should the initial load after startup
for that case fail or not? Do we need to handle this more graceful?

I guess could keep the old key in the binary as well and allow that for
verification of cached blobs for a while. If we decide to do that, it makes
sense to write code (and tests!) for this already, s.t. the key rotation can be
accomplished by just changing the hard-coded constants. Probably material for a
follow-up CL though.

[Andrew T Wilson (Slow), 2014/02/02 11:31:58]

When we do key rotation, we'll need to add code on the client to support old +
new keys and plumb that through. I haven't done this yet, but it's not hard - I
was planning to defer this work until such time as we need to do our first key
rotation.

+    LOG(ERROR) << "Verification key signature verification failed";
+    // TODO(atwilson): Update to return an error once the server is properly
+    // generating a verification signature (http://crbug.com/275291).
+    // return VALIDATION_BAD_KEY_VERIFICATION_SIGNATURE;
+  }
+
   return VALIDATION_OK;
 }
 
@@ -241,6 +339,10 @@ CloudPolicyValidatorBase::Status CloudPolicyValidatorBase::CheckInitialKey() {
     return VALIDATION_BAD_INITIAL_SIGNATURE;
   }
 
+  if (!CheckNewPublicKeyVerificationSignature()) {
+    LOG(ERROR) << "Initial policy root signature validation failed";
+    return VALIDATION_BAD_KEY_VERIFICATION_SIGNATURE;
+  }
   return VALIDATION_OK;
 }