Suggest upgrading to SP3 or later for invalid certificate errors.

chrome/browser/ssl/ssl_blocking_page.cc

 // Copyright (c) 2012 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/build_time.h"
 #include "base/command_line.h"
 #include "base/i18n/rtl.h"
 #include "base/i18n/time_formatting.h"
@@ skipping 524 matching lines @@
     load_time_data.SetString(
         "primaryParagraph",
         l10n_util::GetStringFUTF16(IDS_SSL_V2_PRIMARY_PARAGRAPH, url));
   }
   load_time_data.SetString(
      "openDetails",
      l10n_util::GetStringUTF16(IDS_SSL_V2_OPEN_DETAILS_BUTTON));
   load_time_data.SetString(
      "closeDetails",
      l10n_util::GetStringUTF16(IDS_SSL_V2_CLOSE_DETAILS_BUTTON));
-

[felt, 2014/06/30 18:16:07]

any particular reason why you're deleting this line?

[radhikabhar, 2014/07/02 18:51:08]

Done.

   if (overridable_ && !strict_enforcement_) {  // Overridable.
     SSLErrorInfo error_info =
         SSLErrorInfo::CreateError(
             SSLErrorInfo::NetErrorToErrorType(cert_error_),
             ssl_info_.cert.get(),
             request_url_);
     load_time_data.SetString(
         "explanationParagraph", error_info.details());
     load_time_data.SetString(
         "primaryButtonText",
         l10n_util::GetStringUTF16(IDS_SSL_OVERRIDABLE_SAFETY_BUTTON));
     load_time_data.SetString(
         "finalParagraph",
         l10n_util::GetStringFUTF16(IDS_SSL_OVERRIDABLE_PROCEED_PARAGRAPH, url));
   } else {  // Non-overridable.
     load_time_data.SetBoolean("overridable", false);
-    load_time_data.SetString(
-        "explanationParagraph",
-        l10n_util::GetStringFUTF16(IDS_SSL_NONOVERRIDABLE_MORE, url));
+    SSLErrorInfo::ErrorType type =
+        SSLErrorInfo::NetErrorToErrorType(cert_error_);
+    if (type == SSLErrorInfo::CERT_INVALID) {

[felt, 2014/06/30 18:16:07]

it seems like this would be simpler as:

if (type == SSLErrorInfo::CERT_INVALID && WindowsVersionSP3Lower()) {
  ...
} else {
  ...
}

instead of doing two separate comparisons

[radhikabhar, 2014/07/01 23:56:49]

Done.

+      load_time_data.SetString(
+          "explanationParagraph",
+          WindowsVersionSP3Lower() ?
+          l10n_util::GetStringFUTF16(
+              IDS_SSL_NONOVERRIDABLE_MORE_INVALID_SP3,url) :

[felt, 2014/06/30 18:16:07]

nit: you need a space here

IDS_SSL_NONOVERRIDABLE_MORE_INVALID_SP3, url

[radhikabhar, 2014/07/01 23:56:49]

Done.

+          l10n_util::GetStringFUTF16(
+              IDS_SSL_NONOVERRIDABLE_MORE, url));
+    } else {
+      load_time_data.SetString("explanationParagraph",
+                               l10n_util::GetStringFUTF16(
+                                   IDS_SSL_NONOVERRIDABLE_MORE, url));
+    }
     load_time_data.SetString(
         "primaryButtonText",
         l10n_util::GetStringUTF16(IDS_SSL_NONOVERRIDABLE_RELOAD_BUTTON));
     // Customize the help link depending on the specific error type.
     // Only mark as HSTS if none of the more specific error types apply, and use
     // INVALID as a fallback if no other string is appropriate.
-    SSLErrorInfo::ErrorType type =
-        SSLErrorInfo::NetErrorToErrorType(cert_error_);
     load_time_data.SetInteger("errorType", type);
     int help_string = IDS_SSL_NONOVERRIDABLE_INVALID;
     switch (type) {
       case SSLErrorInfo::CERT_REVOKED:
         help_string = IDS_SSL_NONOVERRIDABLE_REVOKED;
         break;
       case SSLErrorInfo::CERT_PINNED_KEY_MISSING:
         help_string = IDS_SSL_NONOVERRIDABLE_PINNED;
         break;
       case SSLErrorInfo::CERT_INVALID:
@@ skipping 170 matching lines @@
     // sure we don't clear the captive portal flag, since the interstitial was
     // potentially caused by the captive portal.
     captive_portal_detected_ = captive_portal_detected_ ||
         (results->result == captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL);
     // Also keep track of non-HTTP portals and error cases.
     captive_portal_no_response_ = captive_portal_no_response_ ||
         (results->result == captive_portal::RESULT_NO_RESPONSE);
   }
 #endif
 }
+
+bool SSLBlockingPage::WindowsVersionSP3Lower() {
+#if defined(OS_WIN)
+  bool on_windows_xp = base::win::GetVersion() < base::win::VERSION_VISTA;

[felt, 2014/06/30 18:16:07]

is it correct that this includes VERSION_SERVER_2003?

[radhikabhar, 2014/07/01 23:56:49]

Changed it so that it directly compares with the version

[felt, 2014/07/02 18:11:57]

I can see it going either way. I don't know anything about VERSION_SERVER_2003,
but there is something in the enum that makes it sound like maybe it *should*
apply to VERSION_SERVER_2003 too. Can you ask wfh? I suspect he would know.

[radhikabhar, 2014/07/02 18:51:08]

Windows Server 2003 does not have support for SHA2 but it can be downloaded.
This link gives the details
http://blogs.technet.com/b/pki/archive/2010/09/30/sha2-and-windows.aspx. 
I have asked @wfh also and waiting for his reply. 

+  if (on_windows_xp) {
+    OSVERSIONINFOEX version_info = { sizeof version_info };
+    GetVersionEx(reinterpret_cast<OSVERSIONINFO*>(&version_info));

[felt, 2014/06/30 18:36:15]

I don't think you should be directly invoking ::GetVersionEx. Instead, it would
be better for you to use the OSInfo class:
https://code.google.com/p/chromium/codesearch#chromium/src/base/win/windows_v...

[radhikabhar, 2014/07/01 23:56:49]

Done.

+    if (version_info.wServicePackMajor < 3)
+      return true;
+  }
+  return false;

[felt, 2014/06/30 18:16:07]

I don't think you need "return false" twice

[radhikabhar, 2014/07/01 23:56:49]

Done.

+#endif
+  return false;
+}