Added parsing of the attestation challenge response protobuf.

This allows callers to optimize if the challenge bytes are already
incorporated into their protocol.

BUG=chromium:260504
TEST=unit, manual

Review URL: https://chromiumcodereview.appspot.com/24088004

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

5 files changed, 88 insertions, 29 deletions

diff --git a/chrome/browser/chromeos/attestation/attestation_signed_data.proto b/chrome/browser/chromeos/attestation/attestation_signed_data.proto
new file mode 100644
index 0000000..7377a4d
--- /dev/null
+++ b/chrome/browser/chromeos/attestation/attestation_signed_data.proto
@@ -0,0 +1,17 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+package chromeos.attestation;
+
+// The message used by the attestation service for challenge response.
+message SignedData {
+  // The data which was signed.
+  optional bytes data = 1;
+  // The signature of the data field.
+  optional bytes signature = 2;
+}
diff --git a/chrome/browser/chromeos/attestation/platform_verification_flow.cc b/chrome/browser/chromeos/attestation/platform_verification_flow.cc
index 392389f..5eb462e 100644
--- a/chrome/browser/chromeos/attestation/platform_verification_flow.cc
+++ b/chrome/browser/chromeos/attestation/platform_verification_flow.cc
@@ -8,6 +8,7 @@
 #include "base/logging.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/chromeos/attestation/attestation_ca_client.h"
+#include "chrome/browser/chromeos/attestation/attestation_signed_data.pb.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/system/statistics_provider.h"
@@ -40,6 +41,14 @@ void DBusCallback(const base::Callback<void(bool)>& on_success,
     on_failure.Run();
   }
 }
+
+// A helper to call a ChallengeCallback with an error result.
+void ReportError(
+    const chromeos::attestation::PlatformVerificationFlow::ChallengeCallback&
+        callback,
+    chromeos::attestation::PlatformVerificationFlow::Result error) {
+  callback.Run(error, std::string(), std::string(), std::string());
+}
 }  // namespace
 
 namespace chromeos {
@@ -116,7 +125,7 @@ void PlatformVerificationFlow::ChallengePlatformKey(
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   if (!IsAttestationEnabled(web_contents)) {
     LOG(INFO) << "PlatformVerificationFlow: Feature disabled.";
-    callback.Run(POLICY_REJECTED, std::string(), std::string());
+    ReportError(callback, POLICY_REJECTED);
     return;
   }
   BoolDBusMethodCallback dbus_callback = base::Bind(
@@ -127,7 +136,7 @@ void PlatformVerificationFlow::ChallengePlatformKey(
                  service_id,
                  challenge,
                  callback),
-      base::Bind(callback, INTERNAL_ERROR, std::string(), std::string()));
+      base::Bind(&ReportError, callback, INTERNAL_ERROR));
   cryptohome_client_->TpmAttestationIsEnrolled(dbus_callback);
 }
 
@@ -205,16 +214,16 @@ void PlatformVerificationFlow::OnConsentResponse(
     if (consent_response == CONSENT_RESPONSE_NONE) {
       // No user response - do not proceed and do not modify any settings.
       LOG(WARNING) << "PlatformVerificationFlow: No response from user.";
-      callback.Run(USER_REJECTED, std::string(), std::string());
+      ReportError(callback, USER_REJECTED);
       return;
     }
     if (!UpdateSettings(web_contents, consent_type, consent_response)) {
-      callback.Run(INTERNAL_ERROR, std::string(), std::string());
+      ReportError(callback, INTERNAL_ERROR);
       return;
     }
     if (consent_response == CONSENT_RESPONSE_DENY) {
       LOG(INFO) << "PlatformVerificationFlow: User rejected request.";
-      callback.Run(USER_REJECTED, std::string(), std::string());
+      ReportError(callback, USER_REJECTED);
       return;
     }
   }
@@ -243,13 +252,14 @@ void PlatformVerificationFlow::OnCertificateReady(
     const std::string& certificate) {
   if (!operation_success) {
     LOG(WARNING) << "PlatformVerificationFlow: Failed to certify platform.";
-    callback.Run(PLATFORM_NOT_VERIFIED, std::string(), std::string());
+    ReportError(callback, PLATFORM_NOT_VERIFIED);
     return;
   }
   cryptohome::AsyncMethodCaller::DataCallback cryptohome_callback = base::Bind(
       &PlatformVerificationFlow::OnChallengeReady,
       weak_factory_.GetWeakPtr(),
       certificate,
+      challenge,
       callback);
   std::string key_name = kContentProtectionKeyPrefix;
   key_name += service_id;
@@ -261,16 +271,26 @@ void PlatformVerificationFlow::OnCertificateReady(
 
 void PlatformVerificationFlow::OnChallengeReady(
     const std::string& certificate,
+    const std::string& challenge,
     const ChallengeCallback& callback,
     bool operation_success,
     const std::string& response_data) {
   if (!operation_success) {
     LOG(ERROR) << "PlatformVerificationFlow: Failed to sign challenge.";
-    callback.Run(INTERNAL_ERROR, std::string(), std::string());
+    ReportError(callback, INTERNAL_ERROR);
+    return;
+  }
+  SignedData signed_data_pb;
+  if (response_data.empty() || !signed_data_pb.ParseFromString(response_data)) {
+    LOG(ERROR) << "PlatformVerificationFlow: Failed to parse response data.";
+    ReportError(callback, INTERNAL_ERROR);
     return;
   }
+  callback.Run(SUCCESS,
+               signed_data_pb.data(),
+               signed_data_pb.signature(),
+               certificate);
   LOG(INFO) << "PlatformVerificationFlow: Platform successfully verified.";
-  callback.Run(SUCCESS, response_data, certificate);
 }
 
 PrefService* PlatformVerificationFlow::GetPrefs(
diff --git a/chrome/browser/chromeos/attestation/platform_verification_flow.h b/chrome/browser/chromeos/attestation/platform_verification_flow.h
index 6a99c08..8ba3b87 100644
--- a/chrome/browser/chromeos/attestation/platform_verification_flow.h
+++ b/chrome/browser/chromeos/attestation/platform_verification_flow.h
@@ -89,15 +89,16 @@ class PlatformVerificationFlow {
   };
 
   // This callback will be called when a challenge operation completes.  If
-  // |result| is SUCCESS then |challenge_response| holds the challenge response
-  // as specified by the protocol.  The |platform_key_certificate| is for the
-  // key which was used to create the challenge response.  This key may be
-  // generated on demand and is not guaranteed to persist across multiple calls
-  // to this method.  Both the response and the certificate are opaque to
-  // the browser; they are intended for validation by an external application or
-  // service.
+  // |result| is SUCCESS then |signed_data| holds the data which was signed
+  // by the platform key (this is the original challenge appended with a random
+  // nonce) and |signature| holds the RSA-PKCS1-v1.5 signature.  The
+  // |platform_key_certificate| certifies the key used to generate the
+  // signature.  This key may be generated on demand and is not guaranteed to
+  // persist across multiple calls to this method.  The browser does not check
+  // the validity of |signature| or |platform_key_certificate|.
   typedef base::Callback<void(Result result,
-                              const std::string& challenge_response,
+                              const std::string& signed_data,
+                              const std::string& signature,
                               const std::string& platform_key_certificate)>
       ChallengeCallback;
 
@@ -185,11 +186,12 @@ class PlatformVerificationFlow {
 
   // A callback called when a challenge signing request has completed.  The
   // |certificate| is the platform certificate for the key which signed the
-  // challenge.  |callback| is the same as in ChallengePlatformKey.
+  // |challenge|.  |callback| is the same as in ChallengePlatformKey.
   // |operation_success| is true iff the challenge signing operation was
   // successful.  If it was successful, |response_data| holds the challenge
   // response and the method will invoke |callback|.
   void OnChallengeReady(const std::string& certificate,
+                        const std::string& challenge,
                         const ChallengeCallback& callback,
                         bool operation_success,
                         const std::string& response_data);
diff --git a/chrome/browser/chromeos/attestation/platform_verification_flow_unittest.cc b/chrome/browser/chromeos/attestation/platform_verification_flow_unittest.cc
index 4c324ba..b722bee 100644
--- a/chrome/browser/chromeos/attestation/platform_verification_flow_unittest.cc
+++ b/chrome/browser/chromeos/attestation/platform_verification_flow_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/testing_pref_service.h"
 #include "base/run_loop.h"
+#include "chrome/browser/chromeos/attestation/attestation_signed_data.pb.h"
 #include "chrome/browser/chromeos/attestation/platform_verification_flow.h"
 #include "chrome/browser/chromeos/login/mock_user_manager.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
@@ -38,7 +39,8 @@ namespace {
 
 const char kTestID[] = "test_id";
 const char kTestChallenge[] = "test_challenge";
-const char kTestResponse[] = "test_challenge_response";
+const char kTestSignedData[] = "test_challenge_with_salt";
+const char kTestSignature[] = "test_signature";
 const char kTestCertificate[] = "test_certificate";
 const char kTestEmail[] = "test_email@chromium.org";
 const char kTestURL[] = "http://mytestdomain/test";
@@ -211,17 +213,20 @@ class PlatformVerificationFlowTest : public ::testing::Test {
 
   void FakeSignChallenge(
       const cryptohome::AsyncMethodCaller::DataCallback& callback) {
-    base::MessageLoop::current()->PostTask(FROM_HERE,
-                                           base::Bind(callback,
-                                                      sign_challenge_success_,
-                                                      kTestResponse));
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(callback,
+                   sign_challenge_success_,
+                   CreateFakeResponseProto()));
   }
 
   void FakeChallengeCallback(PlatformVerificationFlow::Result result,
-                             const std::string& response,
+                             const std::string& salt,
+                             const std::string& signature,
                              const std::string& certificate) {
     result_ = result;
-    challenge_response_ = response;
+    challenge_salt_ = salt;
+    challenge_signature_ = signature;
     certificate_ = certificate;
   }
 
@@ -229,6 +234,15 @@ class PlatformVerificationFlowTest : public ::testing::Test {
     check_state_result_ = result;
   }
 
+  std::string CreateFakeResponseProto() {
+    SignedData pb;
+    pb.set_data(kTestSignedData);
+    pb.set_signature(kTestSignature);
+    std::string serial;
+    CHECK(pb.SerializeToString(&serial));
+    return serial;
+  }
+
  protected:
   base::MessageLoop message_loop_;
   content::TestBrowserThread ui_thread_;
@@ -254,7 +268,8 @@ class PlatformVerificationFlowTest : public ::testing::Test {
   // Callback functions and data.
   PlatformVerificationFlow::ChallengeCallback callback_;
   PlatformVerificationFlow::Result result_;
-  std::string challenge_response_;
+  std::string challenge_salt_;
+  std::string challenge_signature_;
   std::string certificate_;
   base::Callback<void(bool result)> check_state_callback_;
   bool check_state_result_;
@@ -267,7 +282,8 @@ TEST_F(PlatformVerificationFlowTest, SuccessNoConsent) {
   verifier_->ChallengePlatformKey(NULL, kTestID, kTestChallenge, callback_);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(PlatformVerificationFlow::SUCCESS, result_);
-  EXPECT_EQ(kTestResponse, challenge_response_);
+  EXPECT_EQ(kTestSignedData, challenge_salt_);
+  EXPECT_EQ(kTestSignature, challenge_signature_);
   EXPECT_EQ(kTestCertificate, certificate_);
   EXPECT_EQ(0, fake_delegate_.num_consent_calls());
 }
@@ -279,7 +295,8 @@ TEST_F(PlatformVerificationFlowTest, SuccessWithAlwaysAskConsent) {
   verifier_->ChallengePlatformKey(NULL, kTestID, kTestChallenge, callback_);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(PlatformVerificationFlow::SUCCESS, result_);
-  EXPECT_EQ(kTestResponse, challenge_response_);
+  EXPECT_EQ(kTestSignedData, challenge_salt_);
+  EXPECT_EQ(kTestSignature, challenge_signature_);
   EXPECT_EQ(kTestCertificate, certificate_);
   EXPECT_EQ(1, fake_delegate_.num_consent_calls());
 }
@@ -290,7 +307,8 @@ TEST_F(PlatformVerificationFlowTest, SuccessWithAttestationConsent) {
   verifier_->ChallengePlatformKey(NULL, kTestID, kTestChallenge, callback_);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(PlatformVerificationFlow::SUCCESS, result_);
-  EXPECT_EQ(kTestResponse, challenge_response_);
+  EXPECT_EQ(kTestSignedData, challenge_salt_);
+  EXPECT_EQ(kTestSignature, challenge_signature_);
   EXPECT_EQ(kTestCertificate, certificate_);
   EXPECT_EQ(1, fake_delegate_.num_consent_calls());
 }
@@ -302,7 +320,8 @@ TEST_F(PlatformVerificationFlowTest, SuccessWithFirstTimeConsent) {
   verifier_->ChallengePlatformKey(NULL, kTestID, kTestChallenge, callback_);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(PlatformVerificationFlow::SUCCESS, result_);
-  EXPECT_EQ(kTestResponse, challenge_response_);
+  EXPECT_EQ(kTestSignedData, challenge_salt_);
+  EXPECT_EQ(kTestSignature, challenge_signature_);
   EXPECT_EQ(kTestCertificate, certificate_);
   EXPECT_EQ(1, fake_delegate_.num_consent_calls());
 }
diff --git a/chrome/chrome_browser_chromeos.gypi b/chrome/chrome_browser_chromeos.gypi
index f1886d4..1fa0277 100644
--- a/chrome/chrome_browser_chromeos.gypi
+++ b/chrome/chrome_browser_chromeos.gypi
@@ -955,6 +955,7 @@
       'type': 'static_library',
       'sources': [
         'browser/chromeos/attestation/attestation_key_payload.proto',
+        'browser/chromeos/attestation/attestation_signed_data.proto',
       ],
       'variables': {
         'proto_in_dir': 'browser/chromeos/attestation',