New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for delayed secure connections (STARTTLS) #69
Comments
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 $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');
}); |
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.
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). |
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
On a side note, have you checked out https://github.com/clue/php-http-proxy-react? :) |
Closed in favor of reactphp/socket#89 (Migrating over to Socket now that #86 is in) |
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:
I'm going to implement this, however I would like your opinion on these solutions:
Add a DelayedSecureConnector class. Its create() method would result in a DelayedSecureStream instance, with a connect() method
Add a createDelayed() method on SecureConnector, returning the stream and a callback that would allow to enable TLS. Also add a SecureConnectorInterface.
Add a "prologue" callback argument to create(), allowing the callback to manipulate the stream before encryption is enabled
WDYT ?
The text was updated successfully, but these errors were encountered: