Send certificates to devtools when it's open instead of using certId

content/child/web_url_loader_impl.cc

Index: content/child/web_url_loader_impl.cc
diff --git a/content/child/web_url_loader_impl.cc b/content/child/web_url_loader_impl.cc
index 17da0e103519d484dec9df5c4376e17e589aa2f0..462f9b17243305ae05bd57772e6c4a06059b2416 100644
--- a/content/child/web_url_loader_impl.cc
+++ b/content/child/web_url_loader_impl.cc
@@ -33,8 +33,8 @@
 #include "content/child/weburlresponse_extradata_impl.h"
 #include "content/common/resource_messages.h"
 #include "content/common/resource_request_body_impl.h"
+#include "content/common/security_style_util.h"
 #include "content/common/service_worker/service_worker_types.h"
-#include "content/common/ssl_status_serialization.h"
 #include "content/common/url_loader.mojom.h"
 #include "content/public/child/fixed_received_data.h"
 #include "content/public/child/request_peer.h"
@@ -45,6 +45,9 @@
 #include "net/base/net_errors.h"
 #include "net/cert/cert_status_flags.h"
 #include "net/cert/ct_sct_to_string.h"
+#include "net/cert/internal/name_constraints.h"
+#include "net/cert/internal/parse_certificate.h"
+#include "net/cert/internal/parse_name.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_util.h"
 #include "net/ssl/ssl_cipher_suite_names.h"
@@ -209,6 +212,33 @@ blink::WebURLResponse::SignedCertificateTimestamp NetSCTToBlinkSCT(
           sct_and_status.sct->signature.signature_data.length())));
 }
 
+bool GetCommonName(const net::der::Input& tlv, std::string* common_name) {
+  net::RDNSequence rdn_sequence;
+  if (!net::ParseName(tlv, &rdn_sequence))
+    return false;
+
+  for (const net::RelativeDistinguishedName& rdn : rdn_sequence) {
+    for (const auto& atv : rdn) {
+      if (atv.type == net::TypeCommonNameOid()) {
+        return atv.ValueAsStringUnsafe(common_name);
+      }
+    }
+  }
+  return false;
+}
+
+bool DecodeTime(const net::der::GeneralizedTime& generalized_time,
+                base::Time* time) {
+  base::Time::Exploded exploded = {0};
+  exploded.year = generalized_time.year;
+  exploded.month = generalized_time.month;
+  exploded.day_of_month = generalized_time.day;
+  exploded.hour = generalized_time.hours;
+  exploded.minute = generalized_time.minutes;
+  exploded.second = generalized_time.seconds;
+  return base::Time::FromUTCExploded(exploded, time);
+}
+
 void SetSecurityStyleAndDetails(const GURL& url,
                                 const ResourceResponseInfo& info,
                                 WebURLResponse* response,
@@ -224,22 +254,13 @@ void SetSecurityStyleAndDetails(const GURL& url,
 
   // There are cases where an HTTPS request can come in without security
   // info attached (such as a redirect response).
-  const std::string& security_info = info.security_info;
-  if (security_info.empty()) {
+  if (info.certificate.empty()) {
     response->setSecurityStyle(WebURLResponse::SecurityStyleUnknown);
     return;
   }
 
-  SSLStatus ssl_status;
-  if (!DeserializeSecurityInfo(security_info, &ssl_status)) {
-    response->setSecurityStyle(WebURLResponse::SecurityStyleUnknown);
-    DLOG(ERROR)
-        << "DeserializeSecurityInfo() failed for an authenticated request.";
-    return;
-  }
-
   int ssl_version =
-      net::SSLConnectionStatusToVersion(ssl_status.connection_status);
+      net::SSLConnectionStatusToVersion(info.ssl_connection_status);
   const char* protocol;
   net::SSLVersionToString(&protocol, ssl_version);
 
@@ -248,7 +269,7 @@ void SetSecurityStyleAndDetails(const GURL& url,
   const char* mac;
   bool is_aead;
   uint16_t cipher_suite =
-      net::SSLConnectionStatusToCipherSuite(ssl_status.connection_status);
+      net::SSLConnectionStatusToCipherSuite(info.ssl_connection_status);
   net::SSLCipherSuiteToStrings(&key_exchange, &cipher, &mac, &is_aead,
                                cipher_suite);
   if (mac == NULL) {
@@ -256,27 +277,30 @@ void SetSecurityStyleAndDetails(const GURL& url,
     mac = "";
   }
 
-  blink::WebURLResponse::SecurityStyle securityStyle =
+  SecurityStyle security_style = GetSecurityStyleForResource(
+      url, true, info.cert_status);
+
+  blink::WebURLResponse::SecurityStyle security_style_blink =
       WebURLResponse::SecurityStyleUnknown;
-  switch (ssl_status.security_style) {
+  switch (security_style) {
     case SECURITY_STYLE_UNKNOWN:
-      securityStyle = WebURLResponse::SecurityStyleUnknown;
+      security_style_blink = WebURLResponse::SecurityStyleUnknown;
       break;
     case SECURITY_STYLE_UNAUTHENTICATED:
-      securityStyle = WebURLResponse::SecurityStyleUnauthenticated;
+      security_style_blink = WebURLResponse::SecurityStyleUnauthenticated;
       break;
     case SECURITY_STYLE_AUTHENTICATION_BROKEN:
-      securityStyle = WebURLResponse::SecurityStyleAuthenticationBroken;
+      security_style_blink = WebURLResponse::SecurityStyleAuthenticationBroken;
       break;
     case SECURITY_STYLE_WARNING:
-      securityStyle = WebURLResponse::SecurityStyleWarning;
+      security_style_blink = WebURLResponse::SecurityStyleWarning;
       break;
     case SECURITY_STYLE_AUTHENTICATED:
-      securityStyle = WebURLResponse::SecurityStyleAuthenticated;
+      security_style_blink = WebURLResponse::SecurityStyleAuthenticated;
       break;
   }
 
-  response->setSecurityStyle(securityStyle);
+  response->setSecurityStyle(security_style_blink);
 
   blink::WebURLResponse::SignedCertificateTimestampList sct_list(
       info.signed_certificate_timestamps.size());
@@ -284,9 +308,60 @@ void SetSecurityStyleAndDetails(const GURL& url,
   for (size_t i = 0; i < sct_list.size(); ++i)
     sct_list[i] = NetSCTToBlinkSCT(info.signed_certificate_timestamps[i]);
 
+  net::der::Input cert_data(&info.certificate[0]);
+  net::der::Input tbs_cert, signature_alg;
+  net::der::BitString signature_value;
+  bool rv = net::ParseCertificate(cert_data, &tbs_cert, &signature_alg,
+                                  &signature_value);
+  DCHECK(rv);

[Ryan Sleevi, 2016/09/03 00:02:51]

DESIGN: You've removed the code that sanity checked the input (233-240), and
this brings parsing into the renderer. Is it safe to assume this information is
trustworthy? I would think DevTools shouldn't be, but I'm curious your security
model.

On an implementation side, exposing //net/cert/internal is definitely not
something we want to do (hence the name internal). My understanding from
discussing with David is that you'd be perfectly happy with
net::X509Certificate, safe for the fact it hinges on/depends on OSCertHandle at
present, which is not sandbox friendly, and this will be sandboxed.

In trying to examine both your current and future needs, would they be met if
there was a hypothetical net::SandboxedX509Certificate that didn't have the
OSCertHandle connotations (something David and I have already been discussing
how to move away from for the general X509Certificate anyways), which returned
the fields of X509Certificate that were relevant?

I want to make sure I understand the needs correctly (and that seems to be
consistent with what this code is doing), so we can see about offering you a
better alternative than reaching into //net/cert/internal.

[jam, 2016/09/03 01:42:39]

This wasn't sanity check as much as deserialization. The other end is the
browser, so there's no security checking needed for data from it.
The certificate is sent to the renderer only to be displayed in devtools (i.e.
no security decisions are taken based upon it).

A malicious renderer can cause incorrect certificate information to be shown in
devtools' security panel. Note that this is still possible today, since a
renderer can prime the CertStore with whatever certificates it wants and then
give their IDs (which are just sequential numbers). So from that respect, things
shouldn't be worse than today.

As an aside, note that these serious of patches (culminating in removal of
CertStore) have stopped sending the certificate ID and security style (i.e. lock
icon) through the renderer to the browser which was what was used for the UI.
Yes definitely that would be preferable. I originally used X509Certificate in
earlier patchsets but then ran into it not working inside Linux & Mac sandbox.
The former was easy to fix by bringing code that David previously removed, but I
couldn't figure out Mac. I also didn't want future consumers of the network
service to have to change their sandbox just to get whatever data they want from
the certificate.

+  net::ParsedTbsCertificate parsed_tbs_cert;
+  rv = net::ParseTbsCertificate(tbs_cert, net::ParseCertificateOptions(),
+                                &parsed_tbs_cert);
+  DCHECK(rv);
+  std::string subject, issuer;
+
+  GetCommonName(parsed_tbs_cert.subject_tlv, &subject);
+  GetCommonName(parsed_tbs_cert.issuer_tlv, &issuer);
+  base::Time valid_start, valid_expiry;
+  DecodeTime(parsed_tbs_cert.validity_not_before, &valid_start);
+  DecodeTime(parsed_tbs_cert.validity_not_after, &valid_expiry);
+
+  std::map<net::der::Input, net::ParsedExtension> extensions;
+  rv = net::ParseExtensions(parsed_tbs_cert.extensions_tlv, &extensions);
+
+  std::vector<std::string> san;
+  if (extensions.find(net::SubjectAltNameOid()) != extensions.end()) {
+    std::unique_ptr<net::GeneralNames> subject_alt_names =
+        net::GeneralNames::CreateFromDer(
+            extensions[net::SubjectAltNameOid()].value);
+    if (subject_alt_names) {
+      san = subject_alt_names->dns_names;
+      for (const net::IPAddress& ip : subject_alt_names->ip_addresses)
+        san.push_back(ip.ToString());
+    }
+  }
+
+  blink::WebVector<blink::WebString> web_san(san.size());
+  std::transform(
+      san.begin(),
+      san.end(), web_san.begin(),
+      [](const std::string& h) { return blink::WebString::fromLatin1(h); });
+
+  blink::WebVector<blink::WebString> web_cert(info.certificate.size());
+  std::transform(
+      info.certificate.begin(),
+      info.certificate.end(), web_cert.begin(),
+      [](const std::string& h) { return blink::WebString::fromLatin1(h); });
+
   blink::WebURLResponse::WebSecurityDetails webSecurityDetails(
       WebString::fromUTF8(protocol), WebString::fromUTF8(key_exchange),
-      WebString::fromUTF8(cipher), WebString::fromUTF8(mac), ssl_status.cert_id,
+      WebString::fromUTF8(cipher), WebString::fromUTF8(mac),
+      WebString::fromUTF8(subject),
+      web_san,
+      WebString::fromUTF8(issuer),
+      valid_start.ToDoubleT(),
+      valid_expiry.ToDoubleT(),
+      web_cert,
       sct_list);
 
   response->setSecurityDetails(webSecurityDetails);