Finished implementing AttestationPolicyObserver.

chrome/browser/chromeos/attestation/attestation_policy_observer.cc

Index: chrome/browser/chromeos/attestation/attestation_policy_observer.cc
diff --git a/chrome/browser/chromeos/attestation/attestation_policy_observer.cc b/chrome/browser/chromeos/attestation/attestation_policy_observer.cc
index 21664cde31c1188311062f51478bc9a4f81ce67d..b274060faefdec296a9718c72fb7653c12af02b7 100644
--- a/chrome/browser/chromeos/attestation/attestation_policy_observer.cc
+++ b/chrome/browser/chromeos/attestation/attestation_policy_observer.cc
@@ -7,7 +7,10 @@
 #include <string>
 
 #include "base/bind.h"
+#include "base/callback.h"
+#include "base/time.h"
 #include "chrome/browser/chromeos/attestation/attestation_ca_client.h"
+#include "chrome/browser/chromeos/attestation/attestation_key_payload.pb.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/policy/cloud/cloud_policy_client.h"
 #include "chrome/browser/policy/cloud/cloud_policy_manager.h"
@@ -19,6 +22,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
+#include "net/cert/x509_certificate.h"
 
 namespace {
 
@@ -31,10 +35,13 @@ namespace {
 //   value - The value returned by the dbus operation.
 void DBusBoolRedirectCallback(const base::Closure& on_true,
                               const base::Closure& on_false,
+                              const std::string& method_name,
                               chromeos::DBusMethodCallStatus status,
                               bool value) {
-  if (status != chromeos::DBUS_METHOD_CALL_SUCCESS)
+  if (status != chromeos::DBUS_METHOD_CALL_SUCCESS) {
+    LOG(WARNING) << "Cryptohome DBus method failed: " << method_name;

[Mattias Nissler (ping if slow), 2013/04/23 10:53:01]

You probably want to log |status| as well.

[dkrahn, 2013/04/23 21:17:09]

Done.

     return;
+  }
   const base::Closure& task = value ? on_true : on_false;
   if (!task.is_null())
     task.Run();
@@ -49,11 +56,14 @@ void DBusBoolRedirectCallback(const base::Closure& on_true,
 //   data - The data returned by the dbus operation.
 void DBusStringCallback(
     const base::Callback<void(const std::string&)> on_success,
+    const std::string& method_name,
     chromeos::DBusMethodCallStatus status,
     bool result,
     const std::string& data) {
-  if (status != chromeos::DBUS_METHOD_CALL_SUCCESS || !result)
+  if (status != chromeos::DBUS_METHOD_CALL_SUCCESS || !result) {
+    LOG(WARNING) << "Cryptohome DBus method failed: " << method_name;
     return;
+  }
   on_success.Run(data);
 }
 
@@ -65,6 +75,8 @@ namespace attestation {
 const char AttestationPolicyObserver::kEnterpriseMachineKey[] =
     "attest-ent-machine";
 
+const int AttestationPolicyObserver::kExpiryThresholdInDays = 30;
+
 AttestationPolicyObserver::AttestationPolicyObserver(
     policy::CloudPolicyClient* policy_client)
     : cros_settings_(CrosSettings::Get()),
@@ -145,7 +157,10 @@ void AttestationPolicyObserver::Start() {
   cryptohome_client_->TpmAttestationDoesKeyExist(
       CryptohomeClient::DEVICE_KEY,
       kEnterpriseMachineKey,
-      base::Bind(DBusBoolRedirectCallback, on_does_exist, on_does_not_exist));
+      base::Bind(DBusBoolRedirectCallback,
+                 on_does_exist,
+                 on_does_not_exist,
+                 "TpmAttestationDoesKeyExist"));
 }
 
 void AttestationPolicyObserver::GetNewCertificate() {
@@ -155,7 +170,7 @@ void AttestationPolicyObserver::GetNewCertificate() {
       base::Bind(DBusStringCallback,
                  base::Bind(&AttestationPolicyObserver::UploadCertificate,
                             weak_factory_.GetWeakPtr()),
-                 DBUS_METHOD_CALL_SUCCESS));
+                 "", DBUS_METHOD_CALL_SUCCESS));

[Mattias Nissler (ping if slow), 2013/04/23 10:53:01]

Should have a proper identifying name? FWIW, you could also use FROM_HERE.

[dkrahn, 2013/04/23 21:17:09]

Good idea, changed to use FROM_HERE.

 }
 
 void AttestationPolicyObserver::GetExistingCertificate() {
@@ -164,23 +179,87 @@ void AttestationPolicyObserver::GetExistingCertificate() {
       kEnterpriseMachineKey,
       base::Bind(DBusStringCallback,
                  base::Bind(&AttestationPolicyObserver::CheckCertificateExpiry,
-                            weak_factory_.GetWeakPtr())));
+                            weak_factory_.GetWeakPtr()),
+                 "TpmAttestationGetCertificate"));
 }
 
 void AttestationPolicyObserver::CheckCertificateExpiry(
     const std::string& certificate) {
-  // TODO(dkrahn): Check if the certificate will expire soon, for now assume no.
-  CheckIfUploaded(certificate);
+  scoped_refptr<net::X509Certificate> x509(
+      net::X509Certificate::CreateFromBytes(certificate.data(),
+                                            certificate.length()));
+  if (!x509.get() || x509->valid_expiry().is_null()) {
+    LOG(WARNING) << "Failed to parse certificate, cannot check expiry.";
+  } else {
+    const base::TimeDelta threshold =
+        base::TimeDelta::FromDays(kExpiryThresholdInDays);
+    if ((base::Time::Now() + threshold) > x509->valid_expiry()) {
+      // The certificate has expired or will soon, replace it.
+      GetNewCertificate();
+      return;
+    }
+  }
+
+  // Get the payload and check if the certificate has already been uploaded.
+  GetKeyPayload(base::Bind(&AttestationPolicyObserver::CheckIfUploaded,
+                           weak_factory_.GetWeakPtr(),
+                           certificate));
 }
 
 void AttestationPolicyObserver::UploadCertificate(
     const std::string& certificate) {
-  // TODO(dkrahn): Upload the certificate.
+  policy_client_->UploadCertificate(
+      certificate,
+      base::Bind(&AttestationPolicyObserver::OnUploadComplete,
+                 weak_factory_.GetWeakPtr()));
 }
 
 void AttestationPolicyObserver::CheckIfUploaded(
-    const std::string& certificate) {
-  // TODO(dkrahn): Check if we've already uploaded the certificate.
+    const std::string& certificate,
+    const std::string& key_payload) {
+  AttestationKeyPayload payload_pb;
+  if (!key_payload.empty() &&
+      payload_pb.ParseFromString(key_payload) &&
+      payload_pb.is_certificate_uploaded()) {
+    // Already uploaded... nothing more to do.
+    return;
+  }
+  UploadCertificate(certificate);
+}
+
+void AttestationPolicyObserver::GetKeyPayload(
+    base::Callback<void(const std::string&)> callback) {
+  cryptohome_client_->TpmAttestationGetKeyPayload(
+      CryptohomeClient::DEVICE_KEY,
+      kEnterpriseMachineKey,
+      base::Bind(DBusStringCallback, callback, "TpmAttestationGetKeyPayload"));
+}
+
+void AttestationPolicyObserver::OnUploadComplete(bool status) {
+  if (!status)
+    return;
+  GetKeyPayload(base::Bind(&AttestationPolicyObserver::MarkAsUploaded,
+                           weak_factory_.GetWeakPtr()));
+}
+
+void AttestationPolicyObserver::MarkAsUploaded(const std::string& key_payload) {
+  AttestationKeyPayload payload_pb;
+  if (!key_payload.empty())
+    payload_pb.ParseFromString(key_payload);
+  payload_pb.set_is_certificate_uploaded(true);
+  std::string new_payload;
+  if (!payload_pb.SerializeToString(&new_payload)) {
+    LOG(WARNING) << "Failed to serialize key payload.";
+    return;
+  }
+  cryptohome_client_->TpmAttestationSetKeyPayload(
+      CryptohomeClient::DEVICE_KEY,
+      kEnterpriseMachineKey,
+      new_payload,
+      base::Bind(DBusBoolRedirectCallback,
+                 base::Closure(),
+                 base::Closure(),
+                 "TpmAttestationSetKeyPayload"));
 }
 
 }  // namespace attestation