net: disable SSLv3.

chrome/common/localized_error.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/common/localized_error.h"
 
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 296 matching lines @@
    IDS_ERRORPAGES_DETAILS_BLOCKED_ENROLLMENT_CHECK_PENDING,
    SUGGEST_CHECK_CONNECTION,
   },
   {net::ERR_SSL_FALLBACK_BEYOND_MINIMUM_VERSION,
    IDS_ERRORPAGES_TITLE_LOAD_FAILED,
    IDS_ERRORPAGES_HEADING_SSL_FALLBACK_BEYOND_MINIMUM_VERSION,
    IDS_ERRORPAGES_SUMMARY_SSL_FALLBACK_BEYOND_MINIMUM_VERSION,
    IDS_ERRORPAGES_DETAILS_SSL_FALLBACK_BEYOND_MINIMUM_VERSION,
    SUGGEST_LEARNMORE,
   },
+  {net::ERR_SSL_VERSION_OR_CIPHER_MISMATCH,

[davidben, 2014/10/29 00:32:05]

Is it worth separating out the version and cipher suite error codes from each
other. On the OpenSSL side, it also triggers on
SSL_R_TLSV1_ALERT_INSUFFICIENT_SECURITY while we probably just want to show this
message on local errors.

[agl, 2014/10/29 01:01:18]

My feeling here is to keep this as is, although only weakly.

NSS actually returns SSL_ERROR_NO_CYPHER_OVERLAP for a bad server version, so we
would need to patch that too. Additionally, this message is reasonably generic
and will probably serve if/when we want to do things like disable RC4 etc.

If someone does want this split, do we narrow the existing error to just cipher
mismatches and create a second, or deprecate the existing one and create two new
ones? (These error values end up in histograms so there are external
dependencies.)

[davidben, 2014/10/29 19:19:33]

Alright. I'll defer to what felt and co come with re messaging. For something
like disabling RC4, it would be the server reporting it, not us, so splitting
there wouldn't let us reuse the message.

(Mostly I'm thinking about the case where the server has stricter requirements
than our ClientHello advertises. Though, realistically, it's probably always
going to be the inverse.)

If we were to split, I don't have particular opinions as to whether we'd reuse
the old one or deprecate it.

[felt, 2014/10/29 19:28:18]

is it worth having & maintaining very slightly different neterror strings? seems
better to have a single neterror that is generic enough.

[davidben, 2014/10/29 19:34:32]

Generic SGTM. I guess it depends on how specific the messaging ends up being. If
it's specifically talking about SSLv3 or dropping a version of SSL, we'll
probably want a dedicated error code. If it's a generic "we decided to drop
support for something", the single net error would be fine.

The case I was thinking about was if, say, the server rejected the ClientHello
because it only supports stronger ciphers. But that's not distinguishable from
it rejecting because it only supports weaker ciphers. And realistically, the
latter's much much more likely than the former, so it's probably not worth
worrying about.

+   IDS_ERRORPAGES_TITLE_LOAD_FAILED,
+   IDS_ERRORPAGES_HEADING_SSL_VERSION_OR_CIPHER_MISMATCH,
+   IDS_ERRORPAGES_SUMMARY_SSL_VERSION_OR_CIPHER_MISMATCH,
+   IDS_ERRORPAGES_DETAILS_SSL_VERSION_OR_CIPHER_MISMATCH,
+   SUGGEST_NONE,

[davidben, 2014/10/29 00:32:05]

Do we want to link this one to a bug as well?

[agl, 2014/10/29 01:01:18]

I don't think so. The existing learn-more link is just for trunk -- it's not in
Chrome 39. I'll remove it before branching. If we want a learn-more link for
this on trunk, it should be a different CL, after the branch point.

[davidben, 2014/10/29 19:19:33]

Acknowledged.

+  },
 };
 
 // Special error page to be used in the case of navigating back to a page
 // generated by a POST.  LocalizedError::HasStrings expects this net error code
 // to also appear in the array above.
 const LocalizedErrorMap repost_error = {
   net::ERR_CACHE_MISS,
   IDS_ERRORPAGES_TITLE_NOT_AVAILABLE,
   IDS_HTTP_POST_WARNING_TITLE,
   IDS_ERRORPAGES_HTTP_POST_WARNING,
@@ skipping 557 matching lines @@
   GURL learn_more_url(kAppWarningLearnMoreUrl);
   base::DictionaryValue* suggest_learn_more = new base::DictionaryValue();
   suggest_learn_more->SetString("msg",
                                 l10n_util::GetStringUTF16(
                                     IDS_ERRORPAGES_SUGGESTION_LEARNMORE_BODY));
   suggest_learn_more->SetString("learnMoreUrl", learn_more_url.spec());
   error_strings->Set("suggestionsLearnMore", suggest_learn_more);
 #endif  // defined(OS_CHROMEOS)
 }
 #endif