Support for delayed secure connections (STARTTLS)

[arnaud-lb]

Hi!

I would like to exchange some data on a stream before enabling TLS on it. Currently, the SecureConnector doesn't allow this, and there is no clean way of enabling TLS on Streams after the connection.

Use cases:

• Supporting HTTP proxy CONNECT (required to implement HTTPS proxy support in http-client)
• Supporting SMTP STARTTLS

I'm going to implement this, however I would like your opinion on these solutions:

1. Add a DelayedSecureConnector class. Its create() method would result in a DelayedSecureStream instance, with a connect() method

2. Add a createDelayed() method on SecureConnector, returning the stream and a callback that would allow to enable TLS. Also add a SecureConnectorInterface.

   list ($stream, $enableTLS) = $connector->connectDelayed();
   // Do something with the stream
   // Then, enable TLS:
   $enableTLS()->then(...);
   

3. Add a "prologue" callback argument to create(), allowing the callback to manipulate the stream before encryption is enabled

WDYT ?

[clue]

I would like to exchange some data on a stream before enabling TLS on it. […] Use cases: […]

Thanks for opening this, I agree that we should add support for this 👍

However, I'm not sold on the APIs you've suggested because I suppose STARTTLS would be highly protocol specific and I'm not sure what our API could look like here.

As an alternative, I would suggest making the StreamEncryption class public (i.e. document this for outside use) and provide some examples how this could be used to implement a custom STARTTLS logic, somewhat like this:

$tcpConnector->create('example.com', 9000)->then(function (DuplexStreamInterface $stream) {
    $parser = new CustomProtocolParser();
    $stream->pipe($parser);

    $parser->on('data', function ($data) use ($stream) {
        if ($data === 'OK') {
            $streamEncryption->enable($stream);
        }
    });

    $stream->write('STARTTLS');
});

[arnaud-lb]

This is one possibility I though about, but then I noticed that StreamEncryption is a bit coupled with SecureConnector. For instance, SecureConnector defines some ssl-related stream context options before opening the socket.

It is possible to make StreamEncryption public if all ConnectorInterface implementations set the stream context options.

I suppose STARTTLS would be highly protocol specific and I'm not sure what our API could look like here

CONNECT looks like the STARTTLS protocol: You first have to send some clear data, and receive a confirmation, before enabling TLS.

With API proposition 1, STARTTLS it would look like this:

$delayedSecureConnector->create('example.com', 9000)->then(function (DelayedSecureStreamInterface $stream) {
    $parser = new CustomProtocolParser();
    $stream->pipe($parser);

    $parser->on('data', function ($data) use ($stream) {
        if ($data === 'OK') {
            $stream->enableEncryption();
        }
    });

    $stream->write('STARTTLS');
});

Here, the promise returned by ->create() resolves to a DelayedSecureStreamInterface. This interface has a enableEncryption() method, allowing to enable encryption at anytime.

With API proposition 2:

$delayedSecureConnector->create('example.com', 9000)->then(function (DuplexStreamInterface $stream, callable $enableEncryption) {
    $parser = new CustomProtocolParser();
    $stream->pipe($parser);

    $parser->on('data', function ($data) use ($enableEncryption) {
        if ($data === 'OK') {
            $enableEncryption();
        }
    });

    $stream->write('STARTTLS');
});

Here the promise returned by ->create() resolves to a DuplexStreamInterface and a $enableEncryption callable. Calling this enables encryption on the stream.

With API proposition 3:

$prologue = new Promise(function ($resolve, $reject) {

    $parser = new CustomProtocolParser();
    $stream->pipe($parser);

    $parser->on('data', function ($data) use ($resolve) {
        if ($data === 'OK') {
            $resolve();
        }
    });

    $stream->write('STARTTLS');
});

$secureConnector->createWithPrologue('example.com', 9000, $prologue)->then(...);

I like propositions 1 and 2 better, because a client would need a single ConnectorInterface implementation, for all his connection needs, and it wouldn't have to choose between calling create() or createWithPrologue(). This avoids having to use the same "do I need encryption" logic at two different places in a client (choosing the right connector when creating the client or the request, then choosing whether to enable encryption).

[clue]

This is one possibility I though about, but then I noticed that StreamEncryption is a bit coupled with SecureConnector. For instance, SecureConnector defines some ssl-related stream context options before opening the socket.

I agree that this is less than ideal and we should look into resolving this 👍

I currently don't have a strong opinion on either of your suggestions, so I'd suggest making small steps first: We should check out the above issue and look into making StreamEncryption non-internal (exposing its API + documentation).

Use cases:

• Supporting HTTP proxy CONNECT (required to implement HTTPS proxy support in http-client)

On a side note, have you checked out https://github.com/clue/php-http-proxy-react? :)

[clue]

Closed in favor of reactphp/socket#89 (Migrating over to Socket now that #86 is in)