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..0c97442c1a754f056338d4276e1856d875f36dbe 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -972,9 +972,17 @@ bool SSLClientSocketNSS::Core::Init(PRFileDesc* socket,
 
   SECStatus rv = SECSuccess;
 
+#if !defined(CKM_AES_GCM)
+#define CKM_AES_GCM 0x00001087
+#endif

[Ryan Sleevi, 2014/12/12 21:33:28]

Don't mix #defines in bodies like this. This block should go on line 151 -
immediately after the headers, before any code - because this is effectively a
"control for older headers" check.

[Bence, 2014/12/12 22:08:43]

Done.

+
   if (!ssl_config_.next_protos.empty()) {
-    std::vector<uint8_t> wire_protos =
-        SerializeNextProtos(ssl_config_.next_protos);
+    // On platforms using NSS, AES-GCM is the only mechanism that satisfies the
+    // security requirements of HTTP/2.
+    // TODO(bnc): Check if ssl_config_.disabled_cipher_suites contains all
+    // AES-GCM ciphersuites.
+    std::vector<uint8_t> wire_protos = SerializeNextProtos(
+        ssl_config_.next_protos, PK11_TokenExists(CKM_AES_GCM));

[Ryan Sleevi, 2014/12/12 21:33:28]

I'm fine with the simplified form - the death of NSS will make this easier.

However, I think we can make this a little more robust for the edge cases:

For bulk encryption:
(PK11_TokenExists(CKM_AES_GCM) || PK11_TokenExists(CKM_NSS_CHACHA20_POLY1305))

Will need to ifdef dance to CKM_NSS_CHACHA20_POLY1305 to (CKM_NSS + 26)

For key agreement:
(PK11_TokenExists(CKM_DH_PKCS_DERIVE) || PK11_TokenExists(CKM_ECDH1_DERIVE))

Shouldn't need to #ifdef dance this

The reason for also checking key agreement is that some distros (Fedora, Red
Hat, and, IIRC, Debian) also disable ECC. If that's the case, then they're
leaving the key agreement alg to DH, which some disable by policy.

The word you're looking for is "yay"

Put together:

((PK11_TokenExists(CKM_AES_GCM) || PK11_TokenExists(CKM_NSS_CHACHA20_POLY1305))
&& (PK11_TokenExists(CKM_DH_PKCS_DERIVE) || PK11_TokenExists(CKM_ECDH1_DERIVE)))

Finally, it doesn't seem like you're checking for TLS1.2 support here. 
IsSecurityAdequateForHTTP2 does, but you don't call that here, so you should
also be testing that (in the ssl_config_, presumably)

[davidben, 2014/12/12 21:56:22]

Wait, seriously, we can't do ECDH on Fedora, Red Hat, and Debian?
Note that this still doesn't absolve us of assuming the server implements 9.2.2.
Google servers will speak RSA and ECDHE_RSA, but not DHE_RSA.

[Bence, 2014/12/12 22:08:43]

Done.

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