Remove the wording that suggests it is possible to proceed after fatal SSL error.

chrome/browser/ssl/ssl_blocking_page.cc

 // Copyright (c) 2011 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/ssl/ssl_blocking_page.h"
 
 #include "base/i18n/rtl.h"
 #include "base/metrics/histogram.h"
+#include "base/string16.h"
 #include "base/string_piece.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/dom_operation_notification_details.h"
 #include "chrome/browser/ssl/ssl_error_info.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/common/jstemplate_builder.h"
 #include "content/browser/cert_store.h"
 #include "content/browser/renderer_host/render_view_host.h"
 #include "content/browser/ssl/ssl_cert_error_handler.h"
@@ skipping 52 matching lines @@
   DictionaryValue strings;
   SSLErrorInfo error_info = SSLErrorInfo::CreateError(
       SSLErrorInfo::NetErrorToErrorType(handler_->cert_error()),
       handler_->ssl_info().cert, handler_->request_url());
 
   strings.SetString("headLine", error_info.title());
   strings.SetString("description", error_info.details());
 
   strings.SetString("moreInfoTitle",
                     l10n_util::GetStringUTF16(IDS_CERT_ERROR_EXTRA_INFO_TITLE));
-  SetExtraInfo(&strings, error_info.extra_information());
 
   int resource_id;
   if (overridable_) {
     resource_id = IDR_SSL_ROAD_BLOCK_HTML;
     strings.SetString("title",
                       l10n_util::GetStringUTF16(IDS_SSL_BLOCKING_PAGE_TITLE));
     strings.SetString("proceed",
                       l10n_util::GetStringUTF16(IDS_SSL_BLOCKING_PAGE_PROCEED));
     strings.SetString("exit",
                       l10n_util::GetStringUTF16(IDS_SSL_BLOCKING_PAGE_EXIT));
+
+    // error_info.extra_information() returns const ref, but we need to add
+    // something. So, add to a copy.
+    std::vector<string16> extra_info = error_info.extra_information();
+    extra_info.push_back(
+        l10n_util::GetStringUTF16(IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED));

[wtc, 2011/12/08 00:35:37]

Many of the certificate error messages have some sort of
"you should not proceed" sentence at the end.  The
IDS_SSL_BLOCKING_PAGE_SHOULD_NOT_PROCEED message is specific
to the CERT_STATUS_AUTHORITY_INVALID error.  Ideally we should
check that the certificate error is CERT_STATUS_AUTHORITY_INVALID
right here.

Can we ever get here for a certificate error other than
CERT_STATUS_AUTHORITY_INVALID?  For example, if there is
a command-line option that forces all certificate errors
to be fatal, will we end up here for an expired certificate?
If so, then we will end up with two "you should not proceed"
sentences.

[Ryan Sleevi, 2011/12/08 00:45:11]

Should this logic be pushed into SSLErrorInfo::CreateError?

My concern with this approach (appending it to the bottom of |extra_info|) is
that it may cause a serious warning to appear as supplemental info. Also, I
can't recall, but doesn't |extra_info| show as a collapsible node (default
collapsed), meaning that users wouldn't see this warning?

It would seem that, to maintain parity with the current positioning of the
warning, it should be moved closer to headline/description (lines 77/78).
Perhaps one of the UI leads would have a better opinion / know if this is
actually a concern.

Also, like wtc said, a number of error messages have the proceed/don't proceed,
and I believe that many of them are reasonable/possible to encounter with HSTS
Strict/pinned certs. This CL may be fine for just that error, but the bug may
not be resolved until a more generic approach is used / the remaining strings
are updated.

In particular, see IDS_CERT_ERROR_COMMON_NAME_INVALID_EXTRA_INFO_2,
IDS_CERT_ERROR_EXPIRED_DETAILS, and IDS_CERT_ERROR_NOT_YET_VALID_DETAILS, which
seem the most likely errors to be encountered with pinned/HSTS. 

Either for this as a follow-up, you may also wish to double check the text about
proceeding with IDS_CERT_ERROR_CONTAINS_ERRORS_DETAILS - I don't believe you can
skip past it.

+    SetExtraInfo(&strings, extra_info);
   } else {
     resource_id = IDR_SSL_ERROR_HTML;
     strings.SetString("title",
                       l10n_util::GetStringUTF16(IDS_SSL_ERROR_PAGE_TITLE));
     strings.SetString("back",
                       l10n_util::GetStringUTF16(IDS_SSL_ERROR_PAGE_BACK));
+    SetExtraInfo(&strings, error_info.extra_information());
   }
 
   strings.SetString("textdirection", base::i18n::IsRTL() ? "rtl" : "ltr");
 
   base::StringPiece html(
       ResourceBundle::GetSharedInstance().GetRawDataResource(resource_id));
 
   return jstemplate_builder::GetI18nTemplateHtml(html, &strings);
 }
 
@@ skipping 53 matching lines @@
   DCHECK(!callback_.is_null());
 
   callback_.Run(handler_, true);
   callback_.Reset();
 }
 
 // static
 void SSLBlockingPage::SetExtraInfo(
     DictionaryValue* strings,
     const std::vector<string16>& extra_info) {
-  DCHECK(extra_info.size() < 5);  // We allow 5 paragraphs max.
+  DCHECK_LT(extra_info.size(), 5U);  // We allow 5 paragraphs max.
   const char* keys[5] = {
       "moreInfo1", "moreInfo2", "moreInfo3", "moreInfo4", "moreInfo5"
   };
   int i;
   for (i = 0; i < static_cast<int>(extra_info.size()); i++) {
     strings->SetString(keys[i], extra_info[i]);
   }
   for (; i < 5; i++) {
     strings->SetString(keys[i], "");
   }
 }