Implement ALPN for SSLClientSocketOpenSSL.

Also remove server_protos from the SSLClientSocket interface. NSS wasn't
filling it in and NPN is going away eventually. With ALPN, this is less useful.

BUG=388429

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286405 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 83 insertions, 82 deletions

diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
index eefb09c..82d7ed3 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -57,15 +57,12 @@ base::Value* NetLogHttpStreamJobCallback(const GURL* original_url,
 base::Value* NetLogHttpStreamProtoCallback(
     const SSLClientSocket::NextProtoStatus status,
     const std::string* proto,
-    const std::string* server_protos,
     NetLog::LogLevel /* log_level */) {
   base::DictionaryValue* dict = new base::DictionaryValue();
 
   dict->SetString("next_proto_status",
                   SSLClientSocket::NextProtoStatusToString(status));
   dict->SetString("proto", *proto);
-  dict->SetString("server_protos",
-                  SSLClientSocket::ServerProtosToString(*server_protos));
   return dict;
 }
 
@@ -923,16 +920,15 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
       if (ssl_socket->WasNpnNegotiated()) {
         was_npn_negotiated_ = true;
         std::string proto;
-        std::string server_protos;
         SSLClientSocket::NextProtoStatus status =
-            ssl_socket->GetNextProto(&proto, &server_protos);
+            ssl_socket->GetNextProto(&proto);
         NextProto protocol_negotiated =
             SSLClientSocket::NextProtoFromString(proto);
         protocol_negotiated_ = protocol_negotiated;
         net_log_.AddEvent(
             NetLog::TYPE_HTTP_STREAM_REQUEST_PROTO,
             base::Bind(&NetLogHttpStreamProtoCallback,
-                       status, &proto, &server_protos));
+                       status, &proto));
         if (ssl_socket->was_spdy_negotiated())
           SwitchToSpdyMode();
       }
diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc
index 4f671c6..287bb53 100644
--- a/net/socket/socket_test_util.cc
+++ b/net/socket/socket_test_util.cc
@@ -782,9 +782,8 @@ ChannelIDService* MockClientSocket::GetChannelIDService() const {
 }
 
 SSLClientSocket::NextProtoStatus
-MockClientSocket::GetNextProto(std::string* proto, std::string* server_protos) {
+MockClientSocket::GetNextProto(std::string* proto) {
   proto->clear();
-  server_protos->clear();
   return SSLClientSocket::kNextProtoUnsupported;
 }
 
@@ -1400,9 +1399,8 @@ void MockSSLClientSocket::GetSSLCertRequestInfo(
 }
 
 SSLClientSocket::NextProtoStatus MockSSLClientSocket::GetNextProto(
-    std::string* proto, std::string* server_protos) {
+    std::string* proto) {
   *proto = data_->next_proto;
-  *server_protos = data_->server_protos;
   return data_->next_proto_status;
 }
 
diff --git a/net/socket/socket_test_util.h b/net/socket/socket_test_util.h
index 712cae3..678693e 100644
--- a/net/socket/socket_test_util.h
+++ b/net/socket/socket_test_util.h
@@ -326,7 +326,6 @@ struct SSLSocketDataProvider {
   MockConnect connect;
   SSLClientSocket::NextProtoStatus next_proto_status;
   std::string next_proto;
-  std::string server_protos;
   bool was_npn_negotiated;
   NextProto protocol_negotiated;
   bool client_cert_sent;
@@ -699,8 +698,7 @@ class MockClientSocket : public SSLClientSocket {
                                    unsigned char* out,
                                    unsigned int outlen) OVERRIDE;
   virtual int GetTLSUniqueChannelBinding(std::string* out) OVERRIDE;
-  virtual NextProtoStatus GetNextProto(std::string* proto,
-                                       std::string* server_protos) OVERRIDE;
+  virtual NextProtoStatus GetNextProto(std::string* proto) OVERRIDE;
   virtual ChannelIDService* GetChannelIDService() const OVERRIDE;
 
  protected:
@@ -952,8 +950,7 @@ class MockSSLClientSocket : public MockClientSocket, public AsyncSocket {
   // SSLClientSocket implementation.
   virtual void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info)
       OVERRIDE;
-  virtual NextProtoStatus GetNextProto(std::string* proto,
-                                       std::string* server_protos) OVERRIDE;
+  virtual NextProtoStatus GetNextProto(std::string* proto) OVERRIDE;
   virtual bool set_was_npn_negotiated(bool negotiated) OVERRIDE;
   virtual void set_protocol_negotiated(NextProto protocol_negotiated) OVERRIDE;
   virtual NextProto GetNegotiatedProtocol() const OVERRIDE;
diff --git a/net/socket/ssl_client_socket.cc b/net/socket/ssl_client_socket.cc
index f489ebf..45e3fa9 100644
--- a/net/socket/ssl_client_socket.cc
+++ b/net/socket/ssl_client_socket.cc
@@ -80,21 +80,6 @@ const char* SSLClientSocket::NextProtoStatusToString(
   return NULL;
 }
 
-// static
-std::string SSLClientSocket::ServerProtosToString(
-    const std::string& server_protos) {
-  const char* protos = server_protos.c_str();
-  size_t protos_len = server_protos.length();
-  std::vector<std::string> server_protos_with_commas;
-  for (size_t i = 0; i < protos_len; ) {
-    const size_t len = protos[i];
-    std::string proto_str(&protos[i + 1], len);
-    server_protos_with_commas.push_back(proto_str);
-    i += len + 1;
-  }
-  return JoinString(server_protos_with_commas, ',');
-}
-
 bool SSLClientSocket::WasNpnNegotiated() const {
   return was_npn_negotiated_;
 }
@@ -210,4 +195,40 @@ bool SSLClientSocket::IsChannelIDEnabled(
   return true;
 }
 
+// static
+std::vector<uint8_t> SSLClientSocket::SerializeNextProtos(
+    const std::vector<std::string>& next_protos) {
+  // Do a first pass to determine the total length.
+  size_t wire_length = 0;
+  for (std::vector<std::string>::const_iterator i = next_protos.begin();
+       i != next_protos.end(); ++i) {
+    if (i->size() > 255) {
+      LOG(WARNING) << "Ignoring overlong NPN/ALPN protocol: " << *i;
+      continue;
+    }
+    if (i->size() == 0) {
+      LOG(WARNING) << "Ignoring empty NPN/ALPN protocol";
+      continue;
+    }
+    wire_length += i->size();
+    wire_length++;
+  }
+
+  // Allocate memory for the result and fill it in.
+  std::vector<uint8_t> wire_protos;
+  wire_protos.reserve(wire_length);
+  for (std::vector<std::string>::const_iterator i = next_protos.begin();
+       i != next_protos.end(); i++) {
+    if (i->size() == 0 || i->size() > 255)
+      continue;
+    wire_protos.push_back(i->size());
+    wire_protos.resize(wire_protos.size() + i->size());
+    memcpy(&wire_protos[wire_protos.size() - i->size()],
+           i->data(), i->size());
+  }
+  DCHECK_EQ(wire_protos.size(), wire_length);
+
+  return wire_protos;
+}
+
 }  // namespace net
diff --git a/net/socket/ssl_client_socket.h b/net/socket/ssl_client_socket.h
index 99bdd41..9993bb6 100644
--- a/net/socket/ssl_client_socket.h
+++ b/net/socket/ssl_client_socket.h
@@ -93,9 +93,7 @@ class NET_EXPORT SSLClientSocket : public SSLSocket {
   //   kNextProtoNegotiated:  *proto is set to the negotiated protocol.
   //   kNextProtoNoOverlap:   *proto is set to the first protocol in the
   //                          supported list.
-  // *server_protos is set to the server advertised protocols.
-  virtual NextProtoStatus GetNextProto(std::string* proto,
-                                       std::string* server_protos) = 0;
+  virtual NextProtoStatus GetNextProto(std::string* proto) = 0;
 
   static NextProto NextProtoFromString(const std::string& proto_string);
 
@@ -103,10 +101,6 @@ class NET_EXPORT SSLClientSocket : public SSLSocket {
 
   static const char* NextProtoStatusToString(const NextProtoStatus status);
 
-  // Can be used with the second argument(|server_protos|) of |GetNextProto| to
-  // construct a comma separated string of server advertised protocols.
-  static std::string ServerProtosToString(const std::string& server_protos);
-
   static bool IgnoreCertError(int error, int load_flags);
 
   // ClearSessionCache clears the SSL session cache, used to resume SSL
@@ -155,6 +149,11 @@ class NET_EXPORT SSLClientSocket : public SSLSocket {
       const SSLConfig& ssl_config,
       ChannelIDService* channel_id_service);
 
+  // Serializes |next_protos| in the wire format for ALPN: protocols are listed
+  // in order, each prefixed by a one-byte length.
+  static std::vector<uint8_t> SerializeNextProtos(
+      const std::vector<std::string>& next_protos);
+
   // For unit testing only.
   // Returns the unverified certificate chain as presented by server.
   // Note that chain may be different than the verified chain returned by
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 007cc79..83a4d62 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -408,7 +408,6 @@ struct HandshakeState {
   void Reset() {
     next_proto_status = SSLClientSocket::kNextProtoUnsupported;
     next_proto.clear();
-    server_protos.clear();
     channel_id_sent = false;
     server_cert_chain.Reset(NULL);
     server_cert = NULL;
@@ -422,8 +421,6 @@ struct HandshakeState {
   // negotiated protocol stored in |next_proto|.
   SSLClientSocket::NextProtoStatus next_proto_status;
   std::string next_proto;
-  // If the server supports NPN, the protocols supported by the server.
-  std::string server_protos;
 
   // True if a channel ID was sent.
   bool channel_id_sent;
@@ -972,30 +969,11 @@ bool SSLClientSocketNSS::Core::Init(PRFileDesc* socket,
   SECStatus rv = SECSuccess;
 
   if (!ssl_config_.next_protos.empty()) {
-    size_t wire_length = 0;
-    for (std::vector<std::string>::const_iterator
-         i = ssl_config_.next_protos.begin();
-         i != ssl_config_.next_protos.end(); ++i) {
-      if (i->size() > 255) {
-        LOG(WARNING) << "Ignoring overlong NPN/ALPN protocol: " << *i;
-        continue;
-      }
-      wire_length += i->size();
-      wire_length++;
-    }
-    scoped_ptr<uint8[]> wire_protos(new uint8[wire_length]);
-    uint8* dst = wire_protos.get();
-    for (std::vector<std::string>::const_iterator
-         i = ssl_config_.next_protos.begin();
-         i != ssl_config_.next_protos.end(); i++) {
-      if (i->size() > 255)
-        continue;
-      *dst++ = i->size();
-      memcpy(dst, i->data(), i->size());
-      dst += i->size();
-    }
-    DCHECK_EQ(dst, wire_protos.get() + wire_length);
-    rv = SSL_SetNextProtoNego(nss_fd_, wire_protos.get(), wire_length);
+    std::vector<uint8_t> wire_protos =
+        SerializeNextProtos(ssl_config_.next_protos);
+    rv = SSL_SetNextProtoNego(
+        nss_fd_, wire_protos.empty() ? NULL : &wire_protos[0],
+        wire_protos.size());
     if (rv != SECSuccess)
       LogFailedNSSFunction(*weak_net_log_, "SSL_SetNextProtoNego", "");
     rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_ALPN, PR_TRUE);
@@ -2902,10 +2880,8 @@ int SSLClientSocketNSS::GetTLSUniqueChannelBinding(std::string* out) {
 }
 
 SSLClientSocket::NextProtoStatus
-SSLClientSocketNSS::GetNextProto(std::string* proto,
-                                 std::string* server_protos) {
+SSLClientSocketNSS::GetNextProto(std::string* proto) {
   *proto = core_->state().next_proto;
-  *server_protos = core_->state().server_protos;
   return core_->state().next_proto_status;
 }
 
diff --git a/net/socket/ssl_client_socket_nss.h b/net/socket/ssl_client_socket_nss.h
index 43e2039..e9b64b1 100644
--- a/net/socket/ssl_client_socket_nss.h
+++ b/net/socket/ssl_client_socket_nss.h
@@ -70,8 +70,7 @@ class SSLClientSocketNSS : public SSLClientSocket {
   // SSLClientSocket implementation.
   virtual void GetSSLCertRequestInfo(
       SSLCertRequestInfo* cert_request_info) OVERRIDE;
-  virtual NextProtoStatus GetNextProto(std::string* proto,
-                                       std::string* server_protos) OVERRIDE;
+  virtual NextProtoStatus GetNextProto(std::string* proto) OVERRIDE;
 
   // SSLSocket implementation.
   virtual int ExportKeyingMaterial(const base::StringPiece& label,
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index 0b4958d..7192d85 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -386,9 +386,8 @@ void SSLClientSocketOpenSSL::GetSSLCertRequestInfo(
 }
 
 SSLClientSocket::NextProtoStatus SSLClientSocketOpenSSL::GetNextProto(
-    std::string* proto, std::string* server_protos) {
+    std::string* proto) {
   *proto = npn_proto_;
-  *server_protos = server_protos_;
   return npn_status_;
 }
 
@@ -489,6 +488,9 @@ void SSLClientSocketOpenSSL::Disconnect() {
   cert_key_types_.clear();
   client_auth_cert_needed_ = false;
 
+  npn_status_ = kNextProtoUnsupported;
+  npn_proto_.clear();
+
   channel_id_xtn_negotiated_ = false;
   channel_id_request_handle_.Cancel();
 }
@@ -772,6 +774,13 @@ int SSLClientSocketOpenSSL::Init() {
     SSL_enable_tls_channel_id(ssl_);
   }
 
+  if (!ssl_config_.next_protos.empty()) {
+    std::vector<uint8_t> wire_protos =
+        SerializeNextProtos(ssl_config_.next_protos);
+    SSL_set_alpn_protos(ssl_, wire_protos.empty() ? NULL : &wire_protos[0],
+                        wire_protos.size());
+  }
+
   return OK;
 }
 
@@ -835,7 +844,19 @@ int SSLClientSocketOpenSSL::DoHandshake() {
       DVLOG(2) << "Result of session reuse for " << host_and_port_.ToString()
                << " is: " << (SSL_session_reused(ssl_) ? "Success" : "Fail");
     }
-    // SSL handshake is completed.  Let's verify the certificate.
+
+    // SSL handshake is completed. If NPN wasn't negotiated, see if ALPN was.
+    if (npn_status_ == kNextProtoUnsupported) {
+      const uint8_t* alpn_proto = NULL;
+      unsigned alpn_len = 0;
+      SSL_get0_alpn_selected(ssl_, &alpn_proto, &alpn_len);
+      if (alpn_len > 0) {
+        npn_proto_.assign(reinterpret_cast<const char*>(alpn_proto), alpn_len);
+        npn_status_ = kNextProtoNegotiated;
+      }
+    }
+
+    // Verify the certificate.
     const bool got_cert = !!UpdateServerCert();
     DCHECK(got_cert);
     net_log_.AddEvent(
@@ -1470,7 +1491,6 @@ int SSLClientSocketOpenSSL::SelectNextProtoCallback(unsigned char** out,
   }
 
   npn_proto_.assign(reinterpret_cast<const char*>(*out), *outlen);
-  server_protos_.assign(reinterpret_cast<const char*>(in), inlen);
   DVLOG(2) << "next protocol: '" << npn_proto_ << "' status: " << npn_status_;
   return SSL_TLSEXT_ERR_OK;
 }
diff --git a/net/socket/ssl_client_socket_openssl.h b/net/socket/ssl_client_socket_openssl.h
index 06b91db..d20d5ee 100644
--- a/net/socket/ssl_client_socket_openssl.h
+++ b/net/socket/ssl_client_socket_openssl.h
@@ -59,8 +59,7 @@ class SSLClientSocketOpenSSL : public SSLClientSocket {
   // SSLClientSocket implementation.
   virtual void GetSSLCertRequestInfo(
       SSLCertRequestInfo* cert_request_info) OVERRIDE;
-  virtual NextProtoStatus GetNextProto(std::string* proto,
-                                       std::string* server_protos) OVERRIDE;
+  virtual NextProtoStatus GetNextProto(std::string* proto) OVERRIDE;
   virtual ChannelIDService* GetChannelIDService() const OVERRIDE;
 
   // SSLSocket implementation.
@@ -251,7 +250,6 @@ class SSLClientSocketOpenSSL : public SSLClientSocket {
   State next_handshake_state_;
   NextProtoStatus npn_status_;
   std::string npn_proto_;
-  std::string server_protos_;
   // Written by the |channel_id_service_|.
   std::string channel_id_private_key_;
   std::string channel_id_cert_;
diff --git a/net/socket/ssl_client_socket_pool.cc b/net/socket/ssl_client_socket_pool.cc
index 2fd7779..0dd7cf2 100644
--- a/net/socket/ssl_client_socket_pool.cc
+++ b/net/socket/ssl_client_socket_pool.cc
@@ -330,12 +330,11 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
   SSLClientSocket::NextProtoStatus status =
       SSLClientSocket::kNextProtoUnsupported;
   std::string proto;
-  std::string server_protos;
   // GetNextProto will fail and and trigger a NOTREACHED if we pass in a socket
   // that hasn't had SSL_ImportFD called on it. If we get a certificate error
   // here, then we know that we called SSL_ImportFD.
   if (result == OK || IsCertificateError(result))
-    status = ssl_socket_->GetNextProto(&proto, &server_protos);
+    status = ssl_socket_->GetNextProto(&proto);
 
   // If we want spdy over npn, make sure it succeeded.
   if (status == SSLClientSocket::kNextProtoNegotiated) {
diff --git a/net/socket/ssl_client_socket_pool_unittest.cc b/net/socket/ssl_client_socket_pool_unittest.cc
index a443ff1..2bbbfaf 100644
--- a/net/socket/ssl_client_socket_pool_unittest.cc
+++ b/net/socket/ssl_client_socket_pool_unittest.cc
@@ -466,8 +466,7 @@ TEST_P(SSLClientSocketPoolTest, DirectGotSPDY) {
   SSLClientSocket* ssl_socket = static_cast<SSLClientSocket*>(handle.socket());
   EXPECT_TRUE(ssl_socket->WasNpnNegotiated());
   std::string proto;
-  std::string server_protos;
-  ssl_socket->GetNextProto(&proto, &server_protos);
+  ssl_socket->GetNextProto(&proto);
   EXPECT_EQ(GetParam(), SSLClientSocket::NextProtoFromString(proto));
 }
 
@@ -498,8 +497,7 @@ TEST_P(SSLClientSocketPoolTest, DirectGotBonusSPDY) {
   SSLClientSocket* ssl_socket = static_cast<SSLClientSocket*>(handle.socket());
   EXPECT_TRUE(ssl_socket->WasNpnNegotiated());
   std::string proto;
-  std::string server_protos;
-  ssl_socket->GetNextProto(&proto, &server_protos);
+  ssl_socket->GetNextProto(&proto);
   EXPECT_EQ(GetParam(), SSLClientSocket::NextProtoFromString(proto));
 }