Secure server sockets for dart:io. Add TlsServerSocket class, providing SSL server sockets.

Working tls server sockets.


Add is_server parameter to _TlsSocket, pass it through to _TlsFilterImpl.connect.


Add TlsServerSocket class.


BUG=

Committed: https://code.google.com/p/dart/source/detail?r=15163

[Mads Ager (google), 2012-11-20 14:59:44]

LGTM with comments addressed.

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc
File runtime/bin/tls_socket.cc (right):

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc#...
runtime/bin/tls_socket.cc:185: password = NULL;
In that case this is a noop, right? So maybe leave that out and just check

} else if (!Dart_IsNull(password_object)) {
  throw exception
}

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc#...
runtime/bin/tls_socket.cc:263: ThrowPRException("Unsuccessful
SSL_CipherPrefSetDefault RC4 call.");
This text does not match the call you are making.

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc#...
runtime/bin/tls_socket.cc:301: 
Remove one of these empty lines? Or maybe both of them?

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc#...
runtime/bin/tls_socket.cc:308: ThrowPRException(
Indentation is off.

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc#...
runtime/bin/tls_socket.cc:311: ThrowPRException(
Indentation.

https://codereview.chromium.org/11414009/diff/9001/sdk/lib/io/tls_server_sock...
File sdk/lib/io/tls_server_socket.dart (right):

https://codereview.chromium.org/11414009/diff/9001/sdk/lib/io/tls_server_sock...
sdk/lib/io/tls_server_socket.dart:24: void setCertificate(String
certificate_name);
Since this is per instance, can we make it an argument to the constructor
instead? I would prefer to avoid the extra call to create a functioning tls
server socket.

https://codereview.chromium.org/11414009/diff/9001/sdk/lib/io/tls_socket.dart
File sdk/lib/io/tls_socket.dart (right):

https://codereview.chromium.org/11414009/diff/9001/sdk/lib/io/tls_socket.dart...
sdk/lib/io/tls_socket.dart:32: external static void
setCertificateDatabase(String certificateDatabase,
I think this is fine for now. However, it will be a little weird when you want
to do HTTPS connections that you have to have a call to
TlsSocket.setCertificateDatabase(...). Once we have the HTTPS client and server
we might want to revisit how to provide the database and the password.

https://codereview.chromium.org/11414009/diff/9001/tests/standalone/io/tls_se...
File tests/standalone/io/tls_server_test.dart (right):

https://codereview.chromium.org/11414009/diff/9001/tests/standalone/io/tls_se...
tests/standalone/io/tls_server_test.dart:31: Expect.equals(true, server !=
null);
Expect.isTrue(server != null);

https://codereview.chromium.org/11414009/diff/9001/tests/standalone/io/tls_se...
tests/standalone/io/tls_server_test.dart:35: print("port: ${server.port}");
remove debug printing.

https://codereview.chromium.org/11414009/diff/9001/tests/standalone/io/tls_se...
tests/standalone/io/tls_server_test.dart:106: print("Test PASSED");
remove print.

[Bill Hesse, 2012-11-20 17:46:55]

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc
File runtime/bin/tls_socket.cc (right):

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc#...
runtime/bin/tls_socket.cc:185: password = NULL;
On 2012/11/20 14:59:45, Mads Ager wrote:
> In that case this is a noop, right? So maybe leave that out and just check
> 
> } else if (!Dart_IsNull(password_object)) {
>   throw exception
> }

It is difficult to avoid null pointer uses later on if we do this, so I have
replaced this with the empty string: password = "";

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc#...
runtime/bin/tls_socket.cc:263: ThrowPRException("Unsuccessful
SSL_CipherPrefSetDefault RC4 call.");
On 2012/11/20 14:59:45, Mads Ager wrote:
> This text does not match the call you are making.

Done.

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc#...
runtime/bin/tls_socket.cc:301: 
On 2012/11/20 14:59:45, Mads Ager wrote:
> Remove one of these empty lines? Or maybe both of them?

Both removed, and SetPasswordFunc moved up to top of block.

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc#...
runtime/bin/tls_socket.cc:308: ThrowPRException(
On 2012/11/20 14:59:45, Mads Ager wrote:
> Indentation is off.

Done.

https://codereview.chromium.org/11414009/diff/9001/runtime/bin/tls_socket.cc#...
runtime/bin/tls_socket.cc:311: ThrowPRException(
On 2012/11/20 14:59:45, Mads Ager wrote:
> Indentation.

Done.

https://codereview.chromium.org/11414009/diff/9001/sdk/lib/io/tls_server_sock...
File sdk/lib/io/tls_server_socket.dart (right):

https://codereview.chromium.org/11414009/diff/9001/sdk/lib/io/tls_server_sock...
sdk/lib/io/tls_server_socket.dart:24: void setCertificate(String
certificate_name);
On 2012/11/20 14:59:45, Mads Ager wrote:
> Since this is per instance, can we make it an argument to the constructor
> instead? I would prefer to avoid the extra call to create a functioning tls
> server socket.

Done.

https://codereview.chromium.org/11414009/diff/9001/sdk/lib/io/tls_socket.dart
File sdk/lib/io/tls_socket.dart (right):

https://codereview.chromium.org/11414009/diff/9001/sdk/lib/io/tls_socket.dart...
sdk/lib/io/tls_socket.dart:32: external static void
setCertificateDatabase(String certificateDatabase,
On 2012/11/20 14:59:45, Mads Ager wrote:
> I think this is fine for now. However, it will be a little weird when you want
> to do HTTPS connections that you have to have a call to
> TlsSocket.setCertificateDatabase(...). Once we have the HTTPS client and
server
> we might want to revisit how to provide the database and the password.

Done.

https://codereview.chromium.org/11414009/diff/9001/tests/standalone/io/tls_se...
File tests/standalone/io/tls_server_test.dart (right):

https://codereview.chromium.org/11414009/diff/9001/tests/standalone/io/tls_se...
tests/standalone/io/tls_server_test.dart:31: Expect.equals(true, server !=
null);
On 2012/11/20 14:59:45, Mads Ager wrote:
> Expect.isTrue(server != null);

Replaced by Expect.isNotNull(server).
There are a huge number of "Expect.equals(true, "
in the standalone/io tests.  I wonder why?

https://codereview.chromium.org/11414009/diff/9001/tests/standalone/io/tls_se...
tests/standalone/io/tls_server_test.dart:35: print("port: ${server.port}");
On 2012/11/20 14:59:45, Mads Ager wrote:
> remove debug printing.

Done.

https://codereview.chromium.org/11414009/diff/9001/tests/standalone/io/tls_se...
tests/standalone/io/tls_server_test.dart:106: print("Test PASSED");
On 2012/11/20 14:59:45, Mads Ager wrote:
> remove print.

I thought this was a nice print, but removed.
If we don't reach here, then we should fail by timing out.

[Søren Gjesse, 2012-11-20 21:03:34]

https://codereview.chromium.org/11414009/diff/3011/tests/standalone/io/tls_se...
File tests/standalone/io/tls_server_test.dart (right):

https://codereview.chromium.org/11414009/diff/3011/tests/standalone/io/tls_se...
tests/standalone/io/tls_server_test.dart:16: var data = connection.read();
This read is not guaranteed to return a specific amount of data, so you should
wait with checking the expectation until the expected number of bytes has been
read.

https://codereview.chromium.org/11414009/diff/3011/tests/standalone/io/tls_se...
tests/standalone/io/tls_server_test.dart:21: connection.writeList(reply_bytes,
0, reply_bytes.length);
Likewise the writeList is not guaranteed to write all the bytes passed to it.

https://codereview.chromium.org/11414009/diff/3011/tests/standalone/io/tls_se...
tests/standalone/io/tls_server_test.dart:57: socket.writeList(request_bytes, 0,
request_bytes.length);
Ditto.

https://codereview.chromium.org/11414009/diff/3011/tests/standalone/io/tls_se...
tests/standalone/io/tls_server_test.dart:61: var data = socket.read();
Ditto.