attestation: Fix policy observer expiry check.

chrome/browser/chromeos/attestation/attestation_policy_observer.cc

 // Copyright (c) 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.
 
 #include "chrome/browser/chromeos/attestation/attestation_policy_observer.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/location.h"
 #include "base/time/time.h"
 #include "chrome/browser/chrome_notification_types.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 "chromeos/attestation/attestation_flow.h"
 #include "chromeos/cryptohome/async_method_caller.h"
 #include "chromeos/dbus/cryptohome_client.h"
 #include "chromeos/dbus/dbus_method_call_status.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "components/policy/core/common/cloud/cloud_policy_client.h"
 #include "components/policy/core/common/cloud/cloud_policy_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
+#include "net/cert/pem_tokenizer.h"
 #include "net/cert/x509_certificate.h"
 
 namespace {
 
 // The number of days before a certificate expires during which it is
 // considered 'expiring soon' and replacement is initiated.  The Chrome OS CA
 // issues certificates with an expiry of at least two years.  This value has
 // been set large enough so that the majority of users will have gone through
 // a full sign-in during the period.
 const int kExpiryThresholdInDays = 30;
@@ skipping 168 matching lines @@
       base::Bind(DBusStringCallback,
                  base::Bind(&AttestationPolicyObserver::CheckCertificateExpiry,
                             weak_factory_.GetWeakPtr()),
                  base::Bind(&AttestationPolicyObserver::Reschedule,
                             weak_factory_.GetWeakPtr()),
                  FROM_HERE));
 }
 
 void AttestationPolicyObserver::CheckCertificateExpiry(
     const std::string& 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 {
+  int num_certificates = 0;
+  net::PEMTokenizer pem_tokenizer(certificate, {"CERTIFICATE"});
+  while (pem_tokenizer.GetNext()) {
+    ++num_certificates;
+    scoped_refptr<net::X509Certificate> x509 =
+        net::X509Certificate::CreateFromBytes(pem_tokenizer.data().data(),
+                                              pem_tokenizer.data().length());
+    if (!x509.get() || x509->valid_expiry().is_null()) {
+      // This logic intentionally fails open. In theory this should not happen
+      // but in practice parsing X.509 can be brittle and there are a lot of
+      // factors including which underlying module is parsing the certificate,
+      // whether that module performs more checks than just ASN.1/DER format,
+      // and the server module that generated the certificate(s). Renewal is
+      // expensive so we only renew certificates with good evidence that they
+      // have expired or will soon expire; if we don't know, we don't renew.
+      LOG(WARNING) << "Failed to parse certificate, cannot check expiry.";
+      continue;
+    }
     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;
     }
   }
-
+  if (num_certificates == 0) {
+    LOG(WARNING) << "Failed to parse certificate chain, cannot check expiry.";
+  }
   // 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) {
   policy_client_->UploadCertificate(
       certificate,
@@ skipping 64 matching lines @@
         base::Bind(&AttestationPolicyObserver::Start,
                    weak_factory_.GetWeakPtr()),
         base::TimeDelta::FromSeconds(retry_delay_));
   } else {
     LOG(WARNING) << "AttestationPolicyObserver: Retry limit exceeded.";
   }
 }
 
 }  // namespace attestation
 }  // namespace chromeos