Add secure sockets to dart:io

sdk/lib/io/tls_socket.dart

Index: sdk/lib/io/tls_socket.dart
diff --git a/sdk/lib/io/tls_socket.dart b/sdk/lib/io/tls_socket.dart
new file mode 100644
index 0000000000000000000000000000000000000000..06ddb602e6b19d79e53cb20c56fddb005c0bf3b1
--- /dev/null
+++ b/sdk/lib/io/tls_socket.dart
@@ -0,0 +1,309 @@
+// Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+abstract class TlsSocket implements Socket {

[Mads Ager (google), 2012/11/12 11:39:08]

Could you add a class comment for DartDoc please?

[Bill Hesse, 2012/11/13 20:11:08]

Done.

+  /**
+   * Constructs a new secure socket and connect it to the given
+   * host on the given port. The returned socket is not yet connected
+   * but ready for registration of callbacks.
+   */
+  factory TlsSocket(String host, int port) => new _TlsSocket(host, port);
+
+  /**
+   * Initializes the TLS library with the path to a certificate database
+   * containing root certificates for verifying certificate paths on
+   * client connections, and server certificates to provide on server
+   * connections.
+   */
+  external static void setCertificateDatabase(String pkcertDirectory);
+}
+
+
+class _TlsSocket implements TlsSocket {
+  // Status states
+  static final int NOT_CONNECTED = 200;
+  static final int HANDSHAKE = 201;
+  static final int CONNECTED = 202;
+  static final int CLOSED = 203;
+
+  // Buffer identifiers.
+  static final int kReadPlaintext = 0;

[Mads Ager (google), 2012/11/12 11:39:08]

Style update. These needs to be in sync with some C++ constants, right? Maybe
add that to the comment so the connection is clearer?

[Bill Hesse, 2012/11/13 20:11:08]

Done.

+  static final int kWritePlaintext = 1;
+  static final int kReadEncrypted = 2;
+  static final int kWriteEncrypted = 3;
+  static final int kNumBuffers = 4;
+
+  // Constructs a new secure client socket.
+  _TlsSocket(String host, int port)
+      : _host = host,
+        _socket = new Socket(host, port),
+        _tlsFilter = new TlsFilter() {
+    _socket.onConnect = _tlsConnectHandler;
+    _socket.onWrite = _tlsWriteHandler;

[Mads Ager (google), 2012/11/12 11:39:08]

Can't we simplify this. It seems to me that the _tlsWriteHandler is always used.
Can we just set it here and always set it again in _tlsWriteHandler itself. Then
there is no need to set it in so many places?

[Bill Hesse, 2012/11/13 20:11:08]

We actually don't want to set it in _tlsWriteHandler itself - it is a one-shot
handler, and we don't usually want it to be enabled.  Only when we are
handshaking or when we have pending data in the write pipeline, or when the
write handler is set on the TlsSocket.

But a few of the sets are redundant, and have been removed.
There also seems to be a missing call to it in writeEncrypted, if we are blocked
on the socket write - added there.

On 2012/11/12 11:39:08, Mads Ager wrote:

+    _socket.onData = _tlsDataHandler;
+    _socket.onClosed = _tlsCloseHandler;
+    _tlsFilter.init();
+    _tlsFilter.registerHandshakeCallbacks(_tlsHandshakeStartHandler,
+                                          _tlsHandshakeFinishHandler);
+  }
+
+  void set onConnect(void callback()) {
+    _socketConnectHandler = callback;
+  }
+
+  void set onWrite(void callback()) {
+    _socketWriteHandler = callback;
+    // Reset the one-shot onWrite handler.
+    _socket.onWrite = _tlsWriteHandler;
+  }
+
+  void set onData(void callback()) {
+    _socketDataHandler = callback;
+  }
+
+  void set onClosed(void callback()) {
+    _socketCloseHandler = callback;
+  }
+
+  void _tlsConnectHandler() {
+    _connectPending = true;
+    _tlsFilter.connect(_host);

[Søren Gjesse, 2012/11/12 12:04:57]

How about the port - is that not needed here?

[Bill Hesse, 2012/11/13 20:11:08]

Of course it is.  Added.

On 2012/11/12 12:04:57, Søren Gjesse wrote:

+    _tlsFilter.handshake();
+    _socket.onWrite = _tlsWriteHandler;
+  }
+
+  void _tlsWriteHandler() {
+    if (_status == HANDSHAKE) {
+      _tlsHandshake();
+    } else if (_status == CONNECTED) {
+      if (_socketWriteHandler != null) {
+        _socketWriteHandler();
+      }
+    }
+  }
+
+  void _tlsDataHandler() {
+    if (_status == HANDSHAKE) {
+      _tlsHandshake();
+    } else {
+      if (scheduledDataEvent != null) {
+        scheduledDataEvent.cancel();
+        scheduledDataEvent = null;
+      }
+      if (_socketDataHandler != null) {
+        _readEncryptedData();

[Mads Ager (google), 2012/11/12 11:39:08]

Does this mean that you are reading out the data before calling the data
callback? Currently, the model is that we don't read anything out from the
system before a data callback. The read method does the reading of the data from
the system.

[Søren Gjesse, 2012/11/12 12:04:57]

Does encrypted data on the socket always mean that there is unencrypted data
available? We probably can call onData even if read will not read anything, but
I think that's not a good idea.

[Bill Hesse, 2012/11/13 20:11:08]

We need to read data before the user-visible data callback, for the SSL
handshake.  We can create two data handlers, so that we only do this in the
handshake phase.

But we also need to read encrypted data to find out whether we will have any
plaintext data available.  I think it is more important not to send a data event
to the user with no actual data available, than to try and be totally lazy.



On 2012/11/12 11:39:08, Mads Ager wrote:

+        _socketDataHandler();
+      }
+    }
+  }
+
+  void _tlsCloseHandler() {
+    _socketClosed = true;
+    _status = CLOSED;
+    _socket.close();

[Søren Gjesse, 2012/11/12 12:04:57]

I don't think we should close the socket here. If the socket is closed for
reading it should still be possible to write more data. I think that it should
only be called in close().

[Bill Hesse, 2012/11/13 20:11:08]

Sounds right.  We will need to test that this works, once we control both ends
of the socket.  Since this also affects fireCloseEvent, and the destruction of
the filter, I'd like to make it a separate CL.

On 2012/11/12 12:04:57, Søren Gjesse wrote:

+    if (_filterEmpty) {
+      _fireCloseEvent();
+    } else {
+      _fireCloseEventPending = true;
+    }
+  }
+
+  void _tlsHandshake() {
+      _writeEncryptedData();
+      _readEncryptedData();
+      _tlsFilter.handshake();
+      // TODO(whesse): Set the write handler only when there is filter data
+      // to be written to the socket.  Currently Windows stalls if we do this.
+      // The condition should be_tlsFilter.buffers[kWriteEncrypted].length > 0.
+      _socket.onWrite = _tlsWriteHandler;
+  }
+
+  void _tlsHandshakeStartHandler() {
+    _status = HANDSHAKE;
+    _socket.onWrite = _tlsWriteHandler;
+  }
+
+  void _tlsHandshakeFinishHandler() {
+    _status = CONNECTED;
+    if (_connectPending && _socketConnectHandler != null) {
+      _connectPending = false;
+      _socketConnectHandler();
+    }
+  }
+
+  void _fireCloseEvent() {
+    _fireCloseEventPending = false;
+    _tlsFilter.destroy();
+    _tlsFilter = null;
+    if (scheduledDataEvent != null) {
+      scheduledDataEvent.cancel();
+    }
+    if (_socketCloseHandler != null) {
+      _socketCloseHandler();
+    }
+  }
+
+  void close([bool halfClose]) {
+    _socket.close(halfClose);
+  }
+
+  int readList(List<int> data, int offset, int bytes) {

[Mads Ager (google), 2012/11/12 11:39:08]

We are missing the read method on sockets that Soren added.

[Bill Hesse, 2012/11/13 20:11:08]

Added, along with all other methods on the socket interface.
Bug filed for the unimplemented SocketStream methods.

On 2012/11/12 11:39:08, Mads Ager wrote:

+    _readEncryptedData();
+    if (offset < 0 || bytes < 0 || offset + bytes > data.length) {
+      throw new IllegalArgumentException(
+          "Invalid offset or bytes in TlsSocket.readList");
+    }
+    int bytesRead = 0;
+    var buffer = _tlsFilter.buffers[kReadPlaintext];
+    if (buffer.length > 0) {
+      int toRead = min(bytes, buffer.length);
+      data.setRange(offset, toRead, buffer.data, buffer.start);
+      buffer.advanceStart(toRead);
+      bytesRead += toRead;
+    }
+    int newBytes = _tlsFilter.processBuffer(kReadPlaintext);
+    if (newBytes > 0) {
+      buffer.length += newBytes;
+    }
+    if (bytes - bytesRead > 0 && buffer.length > 0) {
+      int toRead = min(bytes - bytesRead, buffer.length);
+      data.setRange(offset + bytesRead, toRead, buffer.data, buffer.start);
+      buffer.advanceStart(toRead);
+      bytesRead += toRead;
+    }
+
+    // If bytesRead is 0, then something is blocked or empty, and
+    // we are guaranteed an event when it becomes unblocked.
+    // Otherwise, give an event if there is data available, and
+    // there has been a read call since the last data event.
+    // This gives the invariant that:
+    // If there is data available, and there has been a read after the
+    // last data event (or no previous one fired), then we are guaranteed
+    // to get a data event.
+    _filterEmpty = (bytesRead == 0);
+    if (bytesRead > 0 && scheduledDataEvent == null) {
+      scheduledDataEvent = new Timer(0, (_) => _tlsDataHandler());
+    } else if (bytesRead == 0) {
+      if (_fireCloseEventPending) {
+        _fireCloseEvent();
+      } else if (scheduledDataEvent != null) {
+        scheduledDataEvent.cancel();
+        scheduledDataEvent = null;
+      }
+    }
+    return bytesRead;
+  }
+
+
+  // Write the data to the socket, and flush it as much as possible
+  // without blocking.  If not all the data is written, enable the

[Mads Ager (google), 2012/11/12 11:39:08]

This comment mentions more than the method is doing.

[Bill Hesse, 2012/11/13 20:11:08]

Yes, and it is an error that this does not do all of this.
We want to write more than the buffer size, with a loop: Adding a TODO.

Also added to _writeEncrypted the code to flush the WRITE_PLAINTEXT buffer.

On 2012/11/12 11:39:08, Mads Ager wrote:

+  // onWrite event.  If data is not all flushed, add handlers to all
+  // relevant events.
+  int writeList(List<int> data, int offset, int bytes) {
+    _writeEncryptedData();  // Tries to flush all post-filter stages.
+    var buffer = _tlsFilter.buffers[kWritePlaintext];
+    if (bytes > buffer.free) {
+      bytes = buffer.free;
+    }
+    if (bytes > 0) {
+      buffer.data.setRange(buffer.start + buffer.length, bytes, data, offset);
+      buffer.length += bytes;
+    }
+    int bytesWritten = _tlsFilter.processBuffer(kWritePlaintext);
+    buffer.advanceStart(bytesWritten);
+    _readEncryptedData();

[Søren Gjesse, 2012/11/12 12:04:57]

Please provide a comment on why you are also calling _readEncryptedData here.

[Bill Hesse, 2012/11/13 20:11:08]

Removed.

On 2012/11/12 12:04:57, Søren Gjesse wrote:

+    _writeEncryptedData();
+    return bytes;
+  }
+
+  void _readEncryptedData() {
+    // Read from the socket and write to the filter.
+    var buffer = _tlsFilter.buffers[kReadEncrypted];
+    while (true) {
+      if (buffer.length > 0) {
+        int bytes = _tlsFilter.processBuffer(kReadEncrypted);
+        if (bytes > 0) {
+          buffer.advanceStart(bytes);
+        } else {
+          break;
+        }
+      } else if (!_socketClosed) {
+        int bytes = _socket.readList(buffer.data,
+                                     buffer.start + buffer.length,
+                                     buffer.free);
+        if (bytes <= 0) break;
+        buffer.length += bytes;
+      } else {
+        break;  // Socket is closed and read buffer is empty.
+      }
+    }
+  }
+
+  void _writeEncryptedData() {
+    // Write from the filter to the socket.
+    var buffer = _tlsFilter.buffers[kWriteEncrypted];
+    while (true) {
+      if (buffer.length > 0) {
+        int bytes = _socket.writeList(buffer.data, buffer.start, buffer.length);
+        if (bytes <= 0) break;
+        buffer.advanceStart(bytes);
+      } else {
+        int bytes = _tlsFilter.processBuffer(kWriteEncrypted);
+        if (bytes <= 0) break;
+        buffer.length += bytes;
+      }
+    }
+  }
+
+  // _TlsSocket cannot extend _Socket and use _Socket's factory constructor.
+  Socket _socket;
+  String _host;
+
+  var _status = NOT_CONNECTED;
+  bool _socketClosed = false;
+  bool _filterEmpty = false;
+  bool _connectPending = false;
+  bool _fireCloseEventPending = false;
+  Function _socketConnectHandler;
+  Function _socketWriteHandler;
+  Function _socketDataHandler;
+  Function _socketCloseHandler;
+  Timer scheduledDataEvent;
+
+  // TODO(whesse): Specify an abstract class for TlsFilter

[Mads Ager (google), 2012/11/12 11:39:08]

This TODO is now done?

[Bill Hesse, 2012/11/13 20:11:08]

Done.

+  TlsFilter _tlsFilter;
+}
+
+
+class _TlsExternalBuffer {
+  static final int kSize = 8 * 1024;

[Mads Ager (google), 2012/11/12 11:39:08]

Change to Dart constant style.

[Søren Gjesse, 2012/11/12 12:04:57]

We should consider using a larger buffer size.

[Bill Hesse, 2012/11/13 20:11:08]

Done.

[Bill Hesse, 2012/11/13 20:11:08]

The TLS specification does specify that packet size can be set (connected to MTU
size) to powers of 2 between 2^9 and 2^14.  So 2^14 is twice as big.  But we do
have 4 of these for each socket.

+  _TlsExternalBuffer() : start = 0, length = 0;
+
+  void advanceStart(int numBytes) {
+    start += numBytes;
+    length -= numBytes;
+    if (length == 0) {
+      start = 0;
+    }
+  }
+
+  int get free => kSize - (start + length);
+
+  List data;  // This will be a ExternalByteArray, backed by C allocated data.
+  int start;
+  int length;
+}
+
+
+abstract class TlsFilter {

[Mads Ager (google), 2012/11/12 11:39:08]

This is public and needs doc comments. Alternatively, keep it private until you
are ready to document and expose it to users.

[Bill Hesse, 2012/11/13 20:11:08]

I'm not convinced that we ever want to expose it.  Making it private.

+  external factory TlsFilter();
+
+  void connect(String hostName);
+  void destroy();
+  void handshake();
+  void init();
+  int processBuffer(int bufferIndex);
+  void registerHandshakeCallbacks(Function startHandshakeHandler,
+                                  Function finishHandshakeHandler);
+}