Support for server session cache.

remoting/protocol/ssl_hmac_channel_authenticator.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/protocol/ssl_hmac_channel_authenticator.h"
 
 #include <stdint.h>
 
 #include <utility>
 
@@ skipping 199 matching lines @@
 
   int result;
   if (is_ssl_server()) {
 #if defined(OS_NACL)
     // Client plugin doesn't use server SSL sockets, and so SSLServerSocket
     // implementation is not compiled for NaCl as part of net_nacl.
     NOTREACHED();
     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);
       return;
     }
 
     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(
         base::Bind(&SslHmacChannelAuthenticator::OnConnected,
                    base::Unretained(this)));
 #endif
   } else {
     transport_security_state_.reset(new net::TransportSecurityState);
     cert_verifier_.reset(new FailingCertVerifier);
 
@@ skipping 190 matching lines @@
              make_scoped_ptr(new P2PStreamSocketAdapter(std::move(socket_))));
   }
 }
 
 void SslHmacChannelAuthenticator::NotifyError(int error) {
   base::ResetAndReturn(&done_callback_).Run(error, nullptr);
 }
 
 }  // namespace protocol
 }  // namespace remoting