Reject renegotiations in SSLClientSocket by default.

Only HTTP/1.1 (and below) sockets may renegotiate. This fix a
crash because SpdyHttpStream didn't account for this properly.
(And can't as the renego + client auth hack is inherently
incompatible with multiplexing.)

Tested manually against hacked up Go servers:

- HTTP/1.1 server which renegotiates with client auth before
  sending a response on a fresh socket.

- Same as above but with a reused socket (the server only
  requests renego when fetching /auth).

- HTTP/2 which incorrectly renegotiates with client auth upon
  requesting /auth. Verified that we get ERR_SSL_PROTOCOL_ERROR
  and not crash.

- HTTP/1.1 server which does two handshakes in a row with Finished
  and HelloRequest in the same record. NSS and BoringSSL differ in
  their behavior here, but in neither port should we miss the
  renego.

BUG=484543,462283

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

Cr-Commit-Position: refs/heads/master@{#329466}

net/http/http_stream_factory_impl_job.cc

@@ -1266,15 +1266,29 @@ bool HttpStreamFactoryImpl::Job::IsSpdyAlternate() const {
 void HttpStreamFactoryImpl::Job::InitSSLConfig(const HostPortPair& server,
                                                SSLConfig* ssl_config,
                                                bool is_proxy) const {
+  if (!is_proxy) {
+    // Prior to HTTP/2 and SPDY, some servers use TLS renegotiation to request
+    // TLS client authentication after the HTTP request was sent. Allow
+    // renegotiation for only those connections.
+    //
+    // Note that this does NOT implement the provision in
+    // https://http2.github.io/http2-spec/#rfc.section.9.2.1 which allows the
+    // server to request a renegotiation immediately before sending the
+    // connection preface as waiting for the preface would cost the round trip
+    // that False Start otherwise saves.
+    ssl_config->renego_allowed_default = true;
+    ssl_config->renego_allowed_for_protos.push_back(kProtoHTTP11);
+  }
+
   if (proxy_info_.is_https() && ssl_config->send_client_cert) {
     // When connecting through an HTTPS proxy, disable TLS False Start so
     // that client authentication errors can be distinguished between those
     // originating from the proxy server (ERR_PROXY_CONNECTION_FAILED) and
     // those originating from the endpoint (ERR_SSL_PROTOCOL_ERROR /
     // ERR_BAD_SSL_CLIENT_AUTH_CERT).
-    // TODO(rch): This assumes that the HTTPS proxy will only request a
-    // client certificate during the initial handshake.
-    // http://crbug.com/59292
+    //
+    // This assumes the proxy will only request certificates on the initial
+    // handshake; renegotiation on the proxy connection is unsupported.
     ssl_config->false_start_enabled = false;
   }
 

net/socket/ssl_client_socket_nss.cc

@@ -569,12 +569,14 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe<Core> {
                                          void* arg,
                                          PRBool* can_false_start);
 
-  // Called by NSS once the handshake has completed.
+  // Called by NSS each time a handshake completely finishes.
   // |arg| contains a pointer to the current SSLClientSocketNSS::Core.
   static void HandshakeCallback(PRFileDesc* socket, void* arg);
 
-  // Called once the handshake has succeeded.
-  void HandshakeSucceeded();
+  // Called once for each successful handshake. If the initial handshake false
+  // starts, it is called when it false starts and not when it completely
+  // finishes. is_initial is true if this is the initial handshake.
+  void HandshakeSucceeded(bool is_initial);
 
   // Handles an NSS error generated while handshaking or performing IO.
   // Returns a network error code mapped from the original NSS error.
@@ -636,6 +638,9 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe<Core> {
   // Record TLS extension used for protocol negotiation (NPN or ALPN).
   void UpdateExtensionUsed();
 
+  // Returns true if renegotiations are allowed.
+  bool IsRenegotiationAllowed() const;
+
   ////////////////////////////////////////////////////////////////////////////
   // Methods that are ONLY called on the network task runner:
   ////////////////////////////////////////////////////////////////////////////
@@ -740,7 +745,8 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe<Core> {
   bool channel_id_needed_;
   // True if the handshake state machine was interrupted for client auth.
   bool client_auth_cert_needed_;
-  // True if NSS has False Started.
+  // True if NSS has False Started in the initial handshake, but the initial
+  // handshake has not yet completely finished..
   bool false_started_;
   // True if NSS has called HandshakeCallback.
   bool handshake_callback_called_;
@@ -1279,6 +1285,7 @@ void SSLClientSocketNSS::Core::HandshakeCallback(
   Core* core = reinterpret_cast<Core*>(arg);
   DCHECK(core->OnNSSTaskRunner());
 
+  bool is_initial = !core->handshake_callback_called_;
   core->handshake_callback_called_ = true;
   if (core->false_started_) {
     core->false_started_ = false;
@@ -1295,10 +1302,10 @@ void SSLClientSocketNSS::Core::HandshakeCallback(
     // called HandshakeSucceeded(), so return now.
     return;
   }
-  core->HandshakeSucceeded();
+  core->HandshakeSucceeded(is_initial);
 }
 
-void SSLClientSocketNSS::Core::HandshakeSucceeded() {
+void SSLClientSocketNSS::Core::HandshakeSucceeded(bool is_initial) {
   DCHECK(OnNSSTaskRunner());
 
   PRBool last_handshake_resumed;
@@ -1317,6 +1324,22 @@ void SSLClientSocketNSS::Core::HandshakeSucceeded() {
   UpdateNextProto();
   UpdateExtensionUsed();
 
+  if (is_initial && IsRenegotiationAllowed()) {
+    // For compatibility, do not enforce RFC 5746 support. Per section 4.1,
+    // enforcement falls largely on the server.
+    //
+    // This is done in a callback rather than after SSL_ForceHandshake returns
+    // because SSL_ForceHandshake will otherwise greedly consume renegotiations
+    // before returning if Finished and HelloRequest are in the same
+    // record.
+    //
+    // Note that SSL_OptionSet should only be called for an initial
+    // handshake. See https://crbug.com/125299.
+    SECStatus rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_RENEGOTIATION,
+                                 SSL_RENEGOTIATE_TRANSITIONAL);
+    DCHECK_EQ(SECSuccess, rv);
+  }
+
   // Update the network task runners view of the handshake state whenever
   // a handshake has completed.
   PostOrRunCallback(
@@ -1447,7 +1470,7 @@ int SSLClientSocketNSS::Core::DoHandshake() {
   } else if (rv == SECSuccess) {
     if (!handshake_callback_called_) {
       false_started_ = true;
-      HandshakeSucceeded();
+      HandshakeSucceeded(true);
     }
   } else {
     PRErrorCode prerr = PR_GetError();
@@ -2120,6 +2143,20 @@ void SSLClientSocketNSS::Core::UpdateExtensionUsed() {
   }
 }
 
+bool SSLClientSocketNSS::Core::IsRenegotiationAllowed() const {
+  DCHECK(OnNSSTaskRunner());
+
+  if (nss_handshake_state_.next_proto_status == kNextProtoUnsupported)
+    return ssl_config_.renego_allowed_default;
+
+  NextProto next_proto = NextProtoFromString(nss_handshake_state_.next_proto);
+  for (NextProto allowed : ssl_config_.renego_allowed_for_protos) {
+    if (next_proto == allowed)
+      return true;
+  }
+  return false;
+}
+
 void SSLClientSocketNSS::Core::RecordChannelIDSupportOnNSSTaskRunner() {
   DCHECK(OnNSSTaskRunner());
   if (nss_handshake_state_.resumed_handshake)
@@ -2791,13 +2828,9 @@ int SSLClientSocketNSS::InitializeSSLOptions() {
   if (rv != SECSuccess)
     LogFailedNSSFunction(net_log_, "SSL_OptionSet", "SSL_ENABLE_FALSE_START");
 
-  // We allow servers to request renegotiation. Since we're a client,
-  // prohibiting this is rather a waste of time. Only servers are in a
-  // position to prevent renegotiation attacks.
-  // http://extendedsubset.com/?p=8
-
-  rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_RENEGOTIATION,
-                     SSL_RENEGOTIATE_TRANSITIONAL);
+  // By default, renegotiations are rejected. After the initial handshake
+  // completes, some application protocols may re-enable it.
+  rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_RENEGOTIATION, SSL_RENEGOTIATE_NEVER);
   if (rv != SECSuccess) {
     LogFailedNSSFunction(
         net_log_, "SSL_OptionSet", "SSL_ENABLE_RENEGOTIATION");

net/socket/ssl_client_socket_openssl.cc

@@ -845,6 +845,10 @@ int SSLClientSocketOpenSSL::Init() {
                                ssl_config_.fastradio_padding_enabled &&
                                    ssl_config_.fastradio_padding_eligible);
 
+  // By default, renegotiations are rejected. After the initial handshake
+  // completes, some application protocols may re-enable it.
+  SSL_set_reject_peer_renegotiations(ssl_, 1);
+
   return OK;
 }
 
@@ -951,6 +955,9 @@ int SSLClientSocketOpenSSL::DoHandshake() {
     SSL_get0_signed_cert_timestamp_list(ssl_, &sct_list, &sct_list_len);
     set_signed_cert_timestamps_received(sct_list_len != 0);
 
+    if (IsRenegotiationAllowed())
+      SSL_set_reject_peer_renegotiations(ssl_, 0);
+
     // Verify the certificate.
     UpdateServerCert();
     GotoState(STATE_VERIFY_CERT);
@@ -1887,6 +1894,18 @@ std::string SSLClientSocketOpenSSL::GetSessionCacheKey() const {
   return result;
 }
 
+bool SSLClientSocketOpenSSL::IsRenegotiationAllowed() const {
+  if (npn_status_ == kNextProtoUnsupported)
+    return ssl_config_.renego_allowed_default;
+
+  NextProto next_proto = NextProtoFromString(npn_proto_);
+  for (NextProto allowed : ssl_config_.renego_allowed_for_protos) {
+    if (next_proto == allowed)
+      return true;
+  }
+  return false;
+}
+
 scoped_refptr<X509Certificate>
 SSLClientSocketOpenSSL::GetUnverifiedServerCertificateChain() const {
   return server_cert_;

net/socket/ssl_client_socket_openssl.h

@@ -189,6 +189,9 @@ class SSLClientSocketOpenSSL : public SSLClientSocket {
   // this socket.
   std::string GetSessionCacheKey() const;
 
+  // Returns true if renegotiations are allowed.
+  bool IsRenegotiationAllowed() const;
+
   bool transport_send_busy_;
   bool transport_recv_busy_;
 

net/ssl/ssl_config.cc

@@ -31,6 +31,7 @@ SSLConfig::SSLConfig()
       verify_ev_cert(false),
       version_fallback(false),
       cert_io_enabled(true),
+      renego_allowed_default(false),
       fastradio_padding_enabled(false),
       fastradio_padding_eligible(false) {
 }

net/ssl/ssl_config.h

@@ -161,6 +161,13 @@ struct NET_EXPORT SSLConfig {
   // requested by the client.
   NextProtoVector next_protos;
 
+  // True if renegotiation should be allowed for the default application-level
+  // protocol when the peer negotiates neither ALPN nor NPN.
+  bool renego_allowed_default;
+
+  // The list of application-level protocols to enable renegotiation for.
+  NextProtoVector renego_allowed_for_protos;
+
   scoped_refptr<X509Certificate> client_cert;
 
   // Information about how to proceed with fastradio padding.