Skip to content

Commit

Permalink
Do not use HTTP/2 without adequate security.
Browse files Browse the repository at this point in the history
Stop using HTTP/2 in case TLS 1.2 is not supported, connection has been
downgraded to below TLS 1.2, or AES-GCM cipher required by HTTP/2 draft
specification is not supported.

BUG=436835

Review URL: https://codereview.chromium.org/757033004

Cr-Commit-Position: refs/heads/master@{#308226}
  • Loading branch information
bnc authored and Commit bot committed Dec 13, 2014
1 parent fc5224c commit 1e75750
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 10 deletions.
1 change: 1 addition & 0 deletions net/http/http_network_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "net/socket/client_socket_factory.h"
#include "net/socket/client_socket_pool_manager_impl.h"
#include "net/socket/next_proto.h"
#include "net/socket/ssl_client_socket.h"
#include "net/spdy/hpack_huffman_aggregator.h"
#include "net/spdy/spdy_session_pool.h"

Expand Down
4 changes: 2 additions & 2 deletions net/net.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@
'ssl/signed_certificate_timestamp_and_status.h',
'ssl/ssl_cert_request_info.cc',
'ssl/ssl_cert_request_info.h',
'ssl/ssl_cipher_suite_names.cc',
'ssl/ssl_cipher_suite_names.h',
'ssl/ssl_client_auth_cache.cc',
'ssl/ssl_client_auth_cache.h',
'ssl/ssl_client_cert_type.h',
Expand Down Expand Up @@ -1107,8 +1109,6 @@
'ssl/client_cert_store_nss.h',
'ssl/client_cert_store_win.cc',
'ssl/client_cert_store_win.h',
'ssl/ssl_cipher_suite_names.cc',
'ssl/ssl_cipher_suite_names.h',
'ssl/ssl_config_service_defaults.cc',
'ssl/ssl_config_service_defaults.h',
'third_party/mozilla_security_manager/nsKeygenHandler.cpp',
Expand Down
24 changes: 23 additions & 1 deletion net/socket/ssl_client_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "net/base/connection_type_histograms.h"
#include "net/base/host_port_pair.h"
#include "net/ssl/channel_id_service.h"
#include "net/ssl/ssl_cipher_suite_names.h"
#include "net/ssl/ssl_config_service.h"
#include "net/ssl/ssl_connection_status_flags.h"

Expand Down Expand Up @@ -233,11 +234,32 @@ bool SSLClientSocket::IsChannelIDEnabled(
return true;
}

// static
bool SSLClientSocket::HasCipherAdequateForHTTP2(
const std::vector<uint16>& cipher_suites) {
for (uint16 cipher : cipher_suites) {
if (IsSecureTLSCipherSuite(cipher))
return true;
}
return false;
}

// static
bool SSLClientSocket::IsTLSVersionAdequateForHTTP2(
const SSLConfig& ssl_config) {
return ssl_config.version_max >= SSL_PROTOCOL_VERSION_TLS1_2;
}

// static
std::vector<uint8_t> SSLClientSocket::SerializeNextProtos(
const NextProtoVector& next_protos) {
const NextProtoVector& next_protos,
bool can_advertise_http2) {
std::vector<uint8_t> wire_protos;
for (const NextProto next_proto : next_protos) {
if (!can_advertise_http2 && kProtoSPDY4MinimumVersion <= next_proto &&
next_proto <= kProtoSPDY4MaximumVersion) {
continue;
}
const std::string proto = NextProtoToString(next_proto);
if (proto.size() > 255) {
LOG(WARNING) << "Ignoring overlong NPN/ALPN protocol: " << proto;
Expand Down
17 changes: 15 additions & 2 deletions net/socket/ssl_client_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,23 @@ class NET_EXPORT SSLClientSocket : public SSLSocket {
const SSLConfig& ssl_config,
ChannelIDService* channel_id_service);

// Determine if there is at least one enabled cipher suite that satisfies
// Section 9.2 of the HTTP/2 specification. Note that the server might still
// pick an inadequate cipher suite.
static bool HasCipherAdequateForHTTP2(
const std::vector<uint16>& cipher_suites);

// Determine if the TLS version required by Section 9.2 of the HTTP/2
// specification is enabled. Note that the server might still pick an
// inadequate TLS version.
static bool IsTLSVersionAdequateForHTTP2(const SSLConfig& ssl_config);

// Serializes |next_protos| in the wire format for ALPN: protocols are listed
// in order, each prefixed by a one-byte length.
// in order, each prefixed by a one-byte length. Any HTTP/2 protocols in
// |next_protos| are ignored if |can_advertise_http2| is false.
static std::vector<uint8_t> SerializeNextProtos(
const NextProtoVector& next_protos);
const NextProtoVector& next_protos,
bool can_advertise_http2);

// For unit testing only.
// Returns the unverified certificate chain as presented by server.
Expand Down
18 changes: 17 additions & 1 deletion net/socket/ssl_client_socket_nss.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ namespace net {
} while (0)
#endif

#if !defined(CKM_AES_GCM)
#define CKM_AES_GCM 0x00001087
#endif

#if !defined(CKM_NSS_CHACHA20_POLY1305)
#define CKM_NSS_CHACHA20_POLY1305 (CKM_NSS + 26)
#endif

namespace {

// SSL plaintext fragments are shorter than 16KB. Although the record layer
Expand Down Expand Up @@ -973,8 +981,16 @@ bool SSLClientSocketNSS::Core::Init(PRFileDesc* socket,
SECStatus rv = SECSuccess;

if (!ssl_config_.next_protos.empty()) {
// TODO(bnc): Check ssl_config_.disabled_cipher_suites.
const bool adequate_encryption =
PK11_TokenExists(CKM_AES_GCM) ||
PK11_TokenExists(CKM_NSS_CHACHA20_POLY1305);
const bool adequate_key_agreement = PK11_TokenExists(CKM_DH_PKCS_DERIVE) ||
PK11_TokenExists(CKM_ECDH1_DERIVE);
std::vector<uint8_t> wire_protos =
SerializeNextProtos(ssl_config_.next_protos);
SerializeNextProtos(ssl_config_.next_protos,
adequate_encryption && adequate_key_agreement &&
IsTLSVersionAdequateForHTTP2(ssl_config_));
rv = SSL_SetNextProtoNego(
nss_fd_, wire_protos.empty() ? NULL : &wire_protos[0],
wire_protos.size());
Expand Down
18 changes: 15 additions & 3 deletions net/socket/ssl_client_socket_openssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -788,8 +788,8 @@ int SSLClientSocketOpenSSL::Init() {
// disabled by default. Note that !SHA256 and !SHA384 only remove HMAC-SHA256
// and HMAC-SHA384 cipher suites, not GCM cipher suites with SHA256 or SHA384
// as the handshake hash.
std::string command("DEFAULT:!NULL:!aNULL:!IDEA:!FZA:!SRP:!SHA256:!SHA384:"
"!aECDH:!AESGCM+AES256");
std::string command(
"DEFAULT:!NULL:!aNULL:!SHA256:!SHA384:!aECDH:!AESGCM+AES256:!aPSK");
// Walk through all the installed ciphers, seeing if any need to be
// appended to the cipher removal |command|.
for (size_t i = 0; i < sk_SSL_CIPHER_num(ciphers); ++i) {
Expand Down Expand Up @@ -838,8 +838,20 @@ int SSLClientSocketOpenSSL::Init() {
}

if (!ssl_config_.next_protos.empty()) {
// Get list of ciphers that are enabled.
STACK_OF(SSL_CIPHER)* enabled_ciphers = SSL_get_ciphers(ssl_);
DCHECK(enabled_ciphers);
std::vector<uint16> enabled_ciphers_vector;
for (size_t i = 0; i < sk_SSL_CIPHER_num(enabled_ciphers); ++i) {
const SSL_CIPHER* cipher = sk_SSL_CIPHER_value(enabled_ciphers, i);
const uint16 id = static_cast<uint16>(SSL_CIPHER_get_id(cipher));
enabled_ciphers_vector.push_back(id);
}

std::vector<uint8_t> wire_protos =
SerializeNextProtos(ssl_config_.next_protos);
SerializeNextProtos(ssl_config_.next_protos,
HasCipherAdequateForHTTP2(enabled_ciphers_vector) &&
IsTLSVersionAdequateForHTTP2(ssl_config_));
SSL_set_alpn_protos(ssl_, wire_protos.empty() ? NULL : &wire_protos[0],
wire_protos.size());
}
Expand Down
2 changes: 1 addition & 1 deletion net/socket/ssl_client_socket_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2356,7 +2356,7 @@ TEST(SSLClientSocket, SerializeNextProtos) {
next_protos.push_back(kProtoHTTP11);
next_protos.push_back(kProtoSPDY31);
static std::vector<uint8_t> serialized =
SSLClientSocket::SerializeNextProtos(next_protos);
SSLClientSocket::SerializeNextProtos(next_protos, true);
ASSERT_EQ(18u, serialized.size());
EXPECT_EQ(8, serialized[0]); // length("http/1.1")
EXPECT_EQ('h', serialized[1]);
Expand Down

0 comments on commit 1e75750

Please sign in to comment.