Skip interstitials and don't block requests for localhost SSL errors

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 6827da719b1ee4b5c11f8ec5de0bd8b35b360fa0..33953bf66458dd7eec4f299c1ac065b1045cb267 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -106,7 +106,9 @@
 #include "media/filters/gpu_video_accelerator_factories.h"
 #include "net/base/data_url.h"
 #include "net/base/net_errors.h"
+#include "net/base/net_util.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "net/cert/cert_status_flags.h"
 #include "net/http/http_util.h"
 #include "third_party/WebKit/public/platform/WebStorageQuotaCallbacks.h"
 #include "third_party/WebKit/public/platform/WebString.h"
@@ -115,6 +117,7 @@
 #include "third_party/WebKit/public/platform/WebURLResponse.h"
 #include "third_party/WebKit/public/platform/WebVector.h"
 #include "third_party/WebKit/public/web/WebColorSuggestion.h"
+#include "third_party/WebKit/public/web/WebConsoleMessage.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebFrameWidget.h"
 #include "third_party/WebKit/public/web/WebGlyphCache.h"
@@ -2736,6 +2739,31 @@ void RenderFrameImpl::didFinishLoad(blink::WebLocalFrame* frame) {
                     DidFinishLoad(frame));
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_, DidFinishLoad());
 
+  // If the navigation is to a localhost URL (and the flag is set to
+  // allow localhost SSL misconfigurations), print a warning to the
+  // console telling the developer to check their SSL configuration
+  // before going to production.

[davidben, 2015/02/11 04:13:14]

This only pays attention to the navigation request and not subresources, right?
(Since it's just looking at ds->request().url().) It might be better to key this
on the commit point (where we switch documents), so didCommitProvisionalLoad.
didFinishLoad is when the load event fires which depends on subresources and
recursively on the load events of any child frames.

I'd also suggest putting the logic in //chrome in a
RenderFrameObserver::DidCommitProvisionalLoad rather than //content. The logic
to let the certificate error through happens in //chrome, so it makes sense to
put the corresponding warning there too. (ChromeRenderFrameObserver? Probably no
need to attach a whole new one for a one-off thing like this.)

[estark, 2015/02/11 18:54:42]

Done -- mostly. DidCommitProvisionalLoad doesn't seem to work so well because
the console gets cleared after that, so you only see the message if you have
"Preserve log" checked. So I put it in DidFinishDocumentLoad, which seems to not
have to wait for all subresources at least. What do you think?

[davidben, 2015/02/11 19:41:35]

Huh. I guess some things are ordered funny. I wonder if we have other places
where console messages key on the document's response info (some header or text
encoding maybe), maybe within Blink itself, and when those trigger.

*shrug* This seems fine too.

+  bool allow_localhost = base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kAllowInsecureLocalhost);
+
+  if (allow_localhost) {
+    SSLStatus ssl_status = render_view_->GetSSLStatusOfFrame(frame_);
+    bool is_cert_error = net::IsCertStatusError(ssl_status.cert_status) &&
+                         !net::IsCertStatusMinorError(ssl_status.cert_status);
+    bool is_localhost = net::IsLocalhost(GURL(ds->request().url()).host());
+
+    if (is_cert_error && is_localhost) {
+      frame_->addMessageToConsole(blink::WebConsoleMessage(
+          blink::WebConsoleMessage::LevelWarning,
+          base::ASCIIToUTF16(
+              "This site does not have a valid SSL "
+              "certificate! Without SSL, your site's and "
+              "visitors' data is vulnerable to theft and "
+              "tampering. Get a valid SSL certificate before"
+              " releasing your website to the public.")));
+    }
+  }
+
   // Don't send this message while the frame is swapped out.
   if (is_swapped_out())
     return;