Support for server session cache.

remoting/protocol/ssl_hmac_channel_authenticator.cc

Index: remoting/protocol/ssl_hmac_channel_authenticator.cc
diff --git a/remoting/protocol/ssl_hmac_channel_authenticator.cc b/remoting/protocol/ssl_hmac_channel_authenticator.cc
index 808bd1b85a5ba0d032a7d1f4e9fbe51aca2d1c82..8fb2e5ea1c0ddc3a5b5fb32ff209de164107740a 100644
--- a/remoting/protocol/ssl_hmac_channel_authenticator.cc
+++ b/remoting/protocol/ssl_hmac_channel_authenticator.cc
@@ -217,8 +217,8 @@ void SslHmacChannelAuthenticator::SecureAndAuthenticate(
     result = net::ERR_FAILED;
 #else
     scoped_refptr<net::X509Certificate> cert =
-        net::X509Certificate::CreateFromBytes(
-            local_cert_.data(), local_cert_.length());
+        net::X509Certificate::CreateFromBytes(local_cert_.data(),
+                                              local_cert_.length());
     if (!cert.get()) {
       LOG(ERROR) << "Failed to parse X509Certificate";
       NotifyError(net::ERR_FAILED);
@@ -228,9 +228,12 @@ void SslHmacChannelAuthenticator::SecureAndAuthenticate(
     net::SSLServerConfig ssl_config;
     ssl_config.require_ecdhe = true;
 
-    scoped_ptr<net::SSLServerSocket> server_socket = net::CreateSSLServerSocket(
-        make_scoped_ptr(new NetStreamSocketAdapter(std::move(socket))),
+    server_context_ = net::CreateSSLServerContext(

[Sergey Ulanov, 2016/03/04 20:37:25]

Does server_context_ need to be a class member instead of just a local variable
here?

[davidben, 2016/03/04 20:56:55]

The socket shouldn't outlive the context. (It's not going to hold on to any
state that you didn't already hold on to before this change. You all don't
really use session caching, which is why it's a little degenerate on your end.)

[Sergey Ulanov, 2016/03/04 21:35:31]

So then this code will not work correctly. SslHmacChannelAuthenticator doesn't
outlive the socket it creates. It gives away ownership of the socket in
CheckDone() and is deleted shortly after that.

Maybe make SSLServerContext ref-counted so it's possible to share it between
multiple sockets?

[davidben, 2016/03/04 21:45:53]

Oh, that's annoying.

Reference-counting is viral because it makes the object have lifetime infinity,
which means lifetime contracts around non-owning pointers can't happen. And
there is a raw pointer in the SSLServerConfig (ClientCertVerifier), so we need
to have defined lifetimes.

I guess, for consumers that don't actually want session caching and want to be
able to carry server sockets standalone, we could make a StreamSocket
implementation which contains

  scoped_ptr<SSLServerContext> context_;
  scoped_ptr<SSLServerSocket> socket_;

and then forwards all methods to socket_. Then you can pass it around
standalone.

[davidben, 2016/03/04 21:48:04]

Oh, I probably should have elaborated here: you can already share it between
multiple sockets. That's the point. So, yeah, if you had a context struct that
SslHmacChannelAuthenticators shared, that would work too. But only if you were
actually okay with session resumption happening (which I suspect you aren't?).
So I think this is correct.

[Sergey Ulanov, 2016/03/04 22:34:07]

We have P2PStreamSocketAdapter that takes ownership of the socket. I think you
can add server_context_ there as well. For the client sockets it will be null.

[ryanchung, 2016/03/04 23:31:23]

Done. I've added server_context_ to P2PStreamSocketAdapter.
Thanks!

         cert.get(), *local_key_pair_->private_key(), ssl_config);
+
+    scoped_ptr<net::SSLServerSocket> server_socket =
+        server_context_->CreateSSLServerSocket(
+            make_scoped_ptr(new NetStreamSocketAdapter(std::move(socket))));
     net::SSLServerSocket* raw_server_socket = server_socket.get();
     socket_ = std::move(server_socket);
     result = raw_server_socket->Handshake(