Reject renegotiations in SSLClientSocket by default.

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 bb923431f94a9c8d463302ac29f0ed035617a729..c9cbf36512498479038faf892f1733821b37b1c3 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -524,6 +524,9 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe<Core> {
   // verified, and may not be called within an NSS callback.
   void CacheSessionIfNecessary();
 
+  // Called on the network task runner.
+  void SetRenegotiationsAllowed(bool allowed);
+
  private:
   friend class base::RefCountedThreadSafe<Core>;
   ~Core();
@@ -1110,6 +1113,24 @@ void SSLClientSocketNSS::Core::CacheSessionIfNecessary() {
     SSL_CacheSession(nss_fd_);
 }
 
+void SSLClientSocketNSS::Core::SetRenegotiationsAllowed(bool allowed) {
+  if (!OnNSSTaskRunner()) {
+    DCHECK(!detached_);
+    nss_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&Core::SetRenegotiationsAllowed, this, allowed));
+    return;
+  }
+
+  // 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.

[Ryan Sleevi, 2015/05/07 01:41:26]

Word it w/o pronouns :)

Also, this comment doesn't make sense with lines 1128 - 1130. You set
SSL_RENEGOTIATE_NEVER but then you say in the comment prohibiting it is a waste
of time?

[davidben, 2015/05/07 19:12:34]

I just copied it from the other one. I think it's explaining why we do
transition vs requiring renegotiation info? I've just removed it because the
link is dead and I'm not sure exactly what it's saying.

+  // http://extendedsubset.com/?p=8

[Ryan Sleevi, 2015/05/07 01:41:26]

Bad link? Doesn't load here

[davidben, 2015/05/07 19:12:34]

Removed.

+  SECStatus rv = SSL_OptionSet(
+      nss_fd_, SSL_ENABLE_RENEGOTIATION,
+      allowed ? SSL_RENEGOTIATE_TRANSITIONAL : SSL_RENEGOTIATE_NEVER);
+  DCHECK_EQ(SECSuccess, rv);
+}
+
 bool SSLClientSocketNSS::Core::OnNSSTaskRunner() const {
   return nss_task_runner_->RunsTasksOnCurrentThread();
 }
@@ -2460,6 +2481,10 @@ bool SSLClientSocketNSS::GetSSLInfo(SSLInfo* ssl_info) {
   return true;
 }
 
+void SSLClientSocketNSS::SetRenegotiationsAllowed(bool allowed) {
+  core_->SetRenegotiationsAllowed(allowed);
+}
+
 void SSLClientSocketNSS::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
   EnterFunction("");
@@ -2792,13 +2817,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);
+  // Forbid renegotiation by default. Callers must opt into requesting
+  // renegotiation.
+  rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_RENEGOTIATION, SSL_RENEGOTIATE_NEVER);
   if (rv != SECSuccess) {
     LogFailedNSSFunction(
         net_log_, "SSL_OptionSet", "SSL_ENABLE_RENEGOTIATION");