Limit lifetime of self-signed certificate used for TLS on Cast channel

extensions/browser/api/cast_channel/cast_socket.cc

Index: extensions/browser/api/cast_channel/cast_socket.cc
diff --git a/extensions/browser/api/cast_channel/cast_socket.cc b/extensions/browser/api/cast_channel/cast_socket.cc
index 83994df6b1c413a28d43401689a4db0c8b973224..3f654e8c70da41b1d1e163d3efd2ef497ba8daba 100644
--- a/extensions/browser/api/cast_channel/cast_socket.cc
+++ b/extensions/browser/api/cast_channel/cast_socket.cc
@@ -163,6 +163,26 @@ bool CastSocket::ExtractPeerCert(std::string* cert) {
 
   logger_->LogSocketEvent(channel_id_, proto::SSL_INFO_OBTAINED);
 
+  // we need to check that the peer cert (which is self-signed) doesn't have an

[mark a. foltz, 2014/11/01 20:17:34]

Please write a function in either cast_auth_util.cc (if the same code is
applicable to NSS and openssl) or NSS and openssl implementations written in
cast_auth_util_{nss,openssl}.cc

[dougsteed, 2014/11/03 21:06:11]

I set out to do this as you suggested, but it seems to require a
disproportionate amount of new code to verify a simple time comparison check. I
got so far as to add a function to cast_auth_util, but the body of the function
is literally two lines long (I realized that the code had been more complicated
than was necessary, and is simpler in the second draft). In order to add a unit
test for this simple function, I would need to create and import one or two
specially constructed X.509 certificates so that I can extract their expiry
times and do a comparison with a known time. Note also that there are already
unit tests for extracting the expiry time of a certificate in
net/cert/x509_certificate_unittest.cc. I will do this if you still think it
would be worthwhile.

+  // excessive life-time (i.e. no more than 48 hrs).
+  net::X509Certificate::OSCertHandles no_intermediates;
+  scoped_refptr<net::X509Certificate> peer_cert =
+      net::X509Certificate::CreateFromHandle(ssl_info.cert->os_cert_handle(),
+                                             no_intermediates);
+
+  base::Time expiry = peer_cert->valid_expiry();
+  if (expiry.is_null()) {
+    return false;
+  }
+
+  base::Time should_have_expired_by =
+      base::Time::Now() + base::TimeDelta::FromDays(2);

[mark a. foltz, 2014/11/01 20:17:34]

This is not really a security feature as the system clock can be trivially set
to bypass this check.

[dougsteed, 2014/11/03 21:06:11]

Yes, we know this is not a strong security check, and can be overridden by the
user. However it is a simple change which should, in conjunction with some
receiver changes, help mitigate a particular exploit, so we are making this
change on all senders. There will likely be other changes.

+
+  if (expiry > should_have_expired_by) {
+    LOG(ERROR) << "Peer cert has excessive lifetime. notAfter=" << expiry;
+    return false;
+  }
+
   bool result = net::X509Certificate::GetDEREncoded(
      ssl_info.cert->os_cert_handle(), cert);
   if (result) {