[Remoting Host] Select Latest Valid Cert

remoting/host/token_validator_base.cc

Index: remoting/host/token_validator_base.cc
diff --git a/remoting/host/token_validator_base.cc b/remoting/host/token_validator_base.cc
index 0160dd4e6892f8148e6df156e4ef020b9950db05..da94cbc930d01c647d79717b834d4bea3e49a758 100644
--- a/remoting/host/token_validator_base.cc
+++ b/remoting/host/token_validator_base.cc
@@ -43,6 +43,48 @@ namespace {
 const int kBufferSize = 4096;
 const char kCertIssuerWildCard[] = "*";
 
+// The certificate is valid if:
+// * The issuer matches exactly the expected issuer or the expected issuer is
+//   a wildcard.
+// * |now| is within [valid_start, valid_expiry], or [valid_start, +inf)
+//   if |valid_expiry| is null.
+bool IsCertificateValid(const std::string& issuer,
+                        const base::Time& now,
+                        const scoped_refptr<net::X509Certificate>& cert) {
+  return (issuer == kCertIssuerWildCard ||
+      issuer == cert->issuer().common_name) &&
+      cert->valid_start() <= now &&
+      (cert->valid_expiry() > now || cert->valid_expiry().is_null());
+}
+
+// Returns true if |c1| is a worse certificate to use than |c2|.
+//
+// Criteria:
+// 1. An invalid certificate is always worse than a valid certificate.

[Yuwei, 2016/09/27 21:37:42]

Ideally if there is something like a filtering iterator adapter then WorseThan
won't need to care whether a certificate is valid. Doesn't look like STL has
such thing...

+// 2. Invalid certificates are equally worse, in which case false will be
+//    returned.
+// 3. A certificate with earlier |valid_start| time is worse.
+// 4. When |valid_start| are the same, the certificate with earlier
+//    |valid_expiry| is worse.
+bool WorseThan(const std::string& issuer,
+               const base::Time& now,
+               const scoped_refptr<net::X509Certificate>& c1,
+               const scoped_refptr<net::X509Certificate>& c2) {
+  bool c1_valid = IsCertificateValid(issuer, now, c1);
+  bool c2_valid = IsCertificateValid(issuer, now, c2);
+
+  if (!c1_valid && c2_valid)
+    return true;
+
+  if (!c2_valid)
+    return false;
+
+  if (c1->valid_start() != c2->valid_start())
+    return c1->valid_start() < c2->valid_start();
+
+  return c1->valid_expiry() < c2->valid_expiry();
+}
+
 }  // namespace
 
 namespace remoting {
@@ -172,19 +214,26 @@ void TokenValidatorBase::OnCertificateRequested(
 void TokenValidatorBase::OnCertificatesSelected(
     net::CertificateList* selected_certs,
     net::ClientCertStore* unused) {
+  if (!request_)
+    return;
+
   const std::string& issuer =
       third_party_auth_config_.token_validation_cert_issuer;
-  if (request_) {
-    for (size_t i = 0; i < selected_certs->size(); ++i) {
-      net::X509Certificate* cert = (*selected_certs)[i].get();
-      if (issuer == kCertIssuerWildCard ||
-          issuer == cert->issuer().common_name) {
-        request_->ContinueWithCertificate(
-            cert, net::FetchClientCertPrivateKey(cert).get());
-        return;
-      }
-    }
+
+  base::Time now = base::Time::Now();
+
+  auto best_match_position =
+      std::max_element(selected_certs->begin(), selected_certs->end(),
+                       std::bind(&WorseThan, issuer, now, std::placeholders::_1,
+                                 std::placeholders::_2));
+
+  if (best_match_position == selected_certs->end() ||
+      !IsCertificateValid(issuer, now, *best_match_position)) {
     request_->ContinueWithCertificate(nullptr, nullptr);
+  } else {
+    request_->ContinueWithCertificate(
+        best_match_position->get(),
+        net::FetchClientCertPrivateKey(best_match_position->get()).get());
   }
 }