Do not use HTTP/2 without adequate security.

net/socket/ssl_client_socket_nss.cc

Index: net/socket/ssl_client_socket_nss.cc
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 3651e8d62466696169433993af5f4cd92d62b67b..39a5c2a4edbaf7481f909ea3fd0af6dc151283d7 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -973,8 +973,23 @@ bool SSLClientSocketNSS::Core::Init(PRFileDesc* socket,
   SECStatus rv = SECSuccess;
 
   if (!ssl_config_.next_protos.empty()) {
-    std::vector<uint8_t> wire_protos =
-        SerializeNextProtos(ssl_config_.next_protos);
+    //EnsureNSSSSLInit();
+    DCHECK(NSS_IsInitialized());
+    const std::vector<uint16> cipher_suites = GetNSSEnabledCipherSuites();

[davidben, 2014/12/10 21:29:57]

I don't think this excludes AES-GCM when unimplemented.

[Ryan Sleevi, 2014/12/10 22:53:06]

Correct. NumImplemented returns what NSS is aware of, but it doesn't do the run
time detection until later. This is just a static list of all it's theoretically
capable of.

[Bence, 2014/12/11 16:50:49]

Done.

+    // We still have to apply the disabled_cipher_suites list to these cipher
+    // suites, because NSS does not know about them quite yet.

[davidben, 2014/12/10 21:29:57]

Is this true? Core::Init is called from line 3304, after disabled_cipher_suites
is processed. There's SSL_CipherPrefGet, though that does a linear search. I
wonder if we want to optimize that or expose another function from libssl.
(That's bundled so we can patch it freely.)

For more fun, there's apparently a separate "policy" flag to each cipher suite
which is ANOTHER way to enable/disable them. But we never set it, so let's
ignore it.
https://code.google.com/p/chromium/codesearch#chromium/src/net/third_party/ns...

[Bence, 2014/12/11 16:50:49]

Done.

+    std::vector<uint16> enabled_cipher_suites;
+    for (std::vector<uint16>::const_iterator it = cipher_suites.begin();
+         it != cipher_suites.end(); ++it) {

[Ryan Hamilton, 2014/12/10 21:11:41]

for (uint16 cipher : cipher_suites) {
}

[Bence, 2014/12/11 16:50:49]

Moot.

+      if (std::find(ssl_config_.disabled_cipher_suites.begin(),
+                    ssl_config_.disabled_cipher_suites.end(),
+                    *it) == ssl_config_.disabled_cipher_suites.end()) {

[Ryan Hamilton, 2014/12/10 21:11:41]

Hm, I wonder if this logic would be better done down in
SSLClientSocketNSS::InitializeSSLOptions where we currently handle
disabled_cipher_suites. (I guess we can't do that because we need to serialize
next protos already?)

In that case, I wonder if we could move that code up here? perhaps the SSL folks
can comment.

[Bence, 2014/12/11 16:50:49]

For the time being, I decided to use SSL_CipherPrefGet, and I think this is a
correct solution.  Yes, I agree that moving lines 3227--3233 here would make
sense.  Can we do it on a separate CL?

[Ryan Hamilton, 2014/12/11 20:06:09]

Sure, just add a TODO.

[Bence, 2014/12/12 15:49:24]

Done.

+        enabled_cipher_suites.push_back(*it);
+      }
+    }
+    std::vector<uint8_t> wire_protos = SerializeNextProtos(
+        ssl_config_.next_protos,
+        IsSecurityAdequateForHTTP2(ssl_config_, enabled_cipher_suites));

[davidben, 2014/12/10 21:29:57]

There's actually yet another case, but I don't think we can do much about it.

IF the server have previously negotiated TLS 1.1, say, because of the fallback.
Then we'll have a session cached for that server. NSS then clamps the
client_version to the session's version when offering a resumption.

(BoringSSL doesn't do this. It's dumb. NSS cites this recommendation in the spec
which I think was flat-out wrong.)

[Ryan Sleevi, 2014/12/10 22:53:06]

File an NSS bug?

Then again, when BoringSSL tries TLS 1.3, where the master secret negotiation
has changed, BoringSSL will be the wrong one ;)

[Bence, 2014/12/11 16:50:49]

Acknowledged.

     rv = SSL_SetNextProtoNego(
         nss_fd_, wire_protos.empty() ? NULL : &wire_protos[0],
         wire_protos.size());