Skip to content

Commit 78a2591

Browse files
jsorjsor
authored
Merge pull request #186 from clue-labs/tls1.3
Improve TLS 1.3 support
2 parents bfef257 + 4da6fda commit 78a2591

File tree

5 files changed

+128
-46
lines changed

5 files changed

+128
-46
lines changed

‎README.md‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,19 +1372,6 @@ This library does not take responsibility over these context options, so it's
13721372
up to consumers of this library to take care of setting appropriate context
13731373
options as described above.
13741374

1375-
All versions of PHP prior to 5.6.8 suffered from a buffering issue where reading
1376-
from a streaming TLS connection could be one `data` event behind.
1377-
This library implements a work-around to try to flush the complete incoming
1378-
data buffers on these legacy PHP versions, which has a penalty of around 10% of
1379-
throughput on all connections.
1380-
With this work-around, we have not been able to reproduce this issue anymore,
1381-
but we have seen reports of people saying this could still affect some of the
1382-
older PHP versions (`5.5.23`, `5.6.7`, and `5.6.8`).
1383-
Note that this only affects *some* higher-level streaming protocols, such as
1384-
IRC over TLS, but should not affect HTTP over TLS (HTTPS).
1385-
Further investigation of this issue is needed.
1386-
For more insights, this issue is also covered by our test suite.
1387-
13881375
PHP < 7.1.4 (and PHP < 7.0.18) suffers from a bug when writing big
13891376
chunks of data over TLS streams at once.
13901377
We try to work around this by limiting the write chunk size to 8192

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"evenement/evenement": "^3.0 || ^2.0 || ^1.0",
99
"react/dns": "^0.4.13",
1010
"react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5",
11-
"react/stream": "^1.0 || ^0.7.1",
11+
"react/stream": "^1.1",
1212
"react/promise": "^2.6.0 || ^1.2.1",
1313
"react/promise-timer": "^1.4.0"
1414
},

‎src/Connection.php‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,6 @@ class Connection extends EventEmitter implements ConnectionInterface
4343

4444
public function __construct($resource, LoopInterface $loop)
4545
{
46-
// PHP < 5.6.8 suffers from a buffer indicator bug on secure TLS connections
47-
// as a work-around we always read the complete buffer until its end.
48-
// The buffer size is limited due to TCP/IP buffers anyway, so this
49-
// should not affect usage otherwise.
50-
// See https://bugs.php.net/bug.php?id=65137
51-
// https://bugs.php.net/bug.php?id=41631
52-
// https://github.com/reactphp/socket-client/issues/24
53-
$clearCompleteBuffer = \PHP_VERSION_ID < 50608;
54-
5546
// PHP < 7.1.4 (and PHP < 7.0.18) suffers from a bug when writing big
5647
// chunks of data over TLS streams at once.
5748
// We try to work around this by limiting the write chunk size to 8192
@@ -62,10 +53,14 @@ public function __construct($resource, LoopInterface $loop)
6253
// See https://github.com/reactphp/socket/issues/105
6354
$limitWriteChunks = (\PHP_VERSION_ID < 70018 || (\PHP_VERSION_ID >= 70100 && \PHP_VERSION_ID < 70104));
6455

56+
// Construct underlying stream to always consume complete receive buffer.
57+
// This avoids stale data in TLS buffers and also works around possible
58+
// buffering issues in legacy PHP versions. The buffer size is limited
59+
// due to TCP/IP buffers anyway, so this should not affect usage otherwise.
6560
$this->input = new DuplexResourceStream(
6661
$resource,
6762
$loop,
68-
$clearCompleteBuffer ? -1 : null,
63+
-1,
6964
new WritableResourceStream($resource, $loop, null, $limitWriteChunks ? 8192 : null)
7065
);
7166

‎src/StreamEncryption.php‎

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,35 +25,21 @@ public function __construct(LoopInterface $loop, $server = true)
2525
$this->server = $server;
2626

2727
// support TLSv1.0+ by default and exclude legacy SSLv2/SSLv3.
28-
// PHP 5.6+ supports bitmasks, legacy PHP only supports predefined
29-
// constants, so apply accordingly below.
30-
// Also, since PHP 5.6.7 up until before PHP 7.2.0 the main constant did
31-
// only support TLSv1.0, so we explicitly apply all versions.
32-
// @link http://php.net/manual/en/migration56.openssl.php#migration56.openssl.crypto-method
33-
// @link https://3v4l.org/plbFn
28+
// As of PHP 7.2+ the main crypto method constant includes all TLS versions.
29+
// As of PHP 5.6+ the crypto method is a bitmask, so we explicitly include all TLS versions.
30+
// For legacy PHP < 5.6 the crypto method is a single value only and this constant includes all TLS versions.
31+
// @link https://3v4l.org/9PSST
3432
if ($server) {
3533
$this->method = \STREAM_CRYPTO_METHOD_TLS_SERVER;
3634

37-
if (\defined('STREAM_CRYPTO_METHOD_TLSv1_0_SERVER')) {
38-
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_SERVER;
39-
}
40-
if (\defined('STREAM_CRYPTO_METHOD_TLSv1_1_SERVER')) {
41-
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_1_SERVER;
42-
}
43-
if (\defined('STREAM_CRYPTO_METHOD_TLSv1_2_SERVER')) {
44-
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_2_SERVER;
35+
if (\PHP_VERSION_ID < 70200 && \PHP_VERSION_ID >= 50600) {
36+
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_1_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_2_SERVER;
4537
}
4638
} else {
4739
$this->method = \STREAM_CRYPTO_METHOD_TLS_CLIENT;
4840

49-
if (\defined('STREAM_CRYPTO_METHOD_TLSv1_0_CLIENT')) {
50-
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_CLIENT;
51-
}
52-
if (\defined('STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT')) {
53-
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT;
54-
}
55-
if (\defined('STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT')) {
56-
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT;
41+
if (\PHP_VERSION_ID < 70200 && \PHP_VERSION_ID >= 50600) {
42+
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT;
5743
}
5844
}
5945
}

‎tests/FunctionalSecureServerTest.php‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,103 @@ public function testClientCanConnectToServer()
4848
$server->close();
4949
}
5050

51+
public function testClientUsesTls13ByDefaultWhenSupportedByOpenSSL()
52+
{
53+
if (PHP_VERSION_ID < 70000 || !$this->supportsTls13()) {
54+
$this->markTestSkipped('Test requires PHP 7+ for crypto meta data and OpenSSL 1.1.1+ for TLS 1.3');
55+
}
56+
57+
$loop = Factory::create();
58+
59+
$server = new TcpServer(0, $loop);
60+
$server = new SecureServer($server, $loop, array(
61+
'local_cert' => __DIR__ . '/../examples/localhost.pem'
62+
));
63+
64+
$connector = new SecureConnector(new TcpConnector($loop), $loop, array(
65+
'verify_peer' => false
66+
));
67+
$promise = $connector->connect($server->getAddress());
68+
69+
/* @var ConnectionInterface $client */
70+
$client = Block\await($promise, $loop, self::TIMEOUT);
71+
72+
$this->assertInstanceOf('React\Socket\Connection', $client);
73+
$this->assertTrue(isset($client->stream));
74+
75+
$meta = stream_get_meta_data($client->stream);
76+
$this->assertTrue(isset($meta['crypto']['protocol']));
77+
78+
if ($meta['crypto']['protocol'] === 'UNKNOWN') {
79+
// TLSv1.3 protocol will only be added via https://github.com/php/php-src/pull/3700
80+
// prior to merging that PR, this info is still available in the cipher version by OpenSSL
81+
$this->assertTrue(isset($meta['crypto']['cipher_version']));
82+
$this->assertEquals('TLSv1.3', $meta['crypto']['cipher_version']);
83+
} else {
84+
$this->assertEquals('TLSv1.3', $meta['crypto']['protocol']);
85+
}
86+
}
87+
88+
public function testClientUsesTls12WhenCryptoMethodIsExplicitlyConfiguredByClient()
89+
{
90+
if (PHP_VERSION_ID < 70000) {
91+
$this->markTestSkipped('Test requires PHP 7+ for crypto meta data');
92+
}
93+
94+
$loop = Factory::create();
95+
96+
$server = new TcpServer(0, $loop);
97+
$server = new SecureServer($server, $loop, array(
98+
'local_cert' => __DIR__ . '/../examples/localhost.pem'
99+
));
100+
101+
$connector = new SecureConnector(new TcpConnector($loop), $loop, array(
102+
'verify_peer' => false,
103+
'crypto_method' => STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT
104+
));
105+
$promise = $connector->connect($server->getAddress());
106+
107+
/* @var ConnectionInterface $client */
108+
$client = Block\await($promise, $loop, self::TIMEOUT);
109+
110+
$this->assertInstanceOf('React\Socket\Connection', $client);
111+
$this->assertTrue(isset($client->stream));
112+
113+
$meta = stream_get_meta_data($client->stream);
114+
$this->assertTrue(isset($meta['crypto']['protocol']));
115+
$this->assertEquals('TLSv1.2', $meta['crypto']['protocol']);
116+
}
117+
118+
public function testClientUsesTls12WhenCryptoMethodIsExplicitlyConfiguredByServer()
119+
{
120+
if (PHP_VERSION_ID < 70000) {
121+
$this->markTestSkipped('Test requires PHP 7+ for crypto meta data');
122+
}
123+
124+
$loop = Factory::create();
125+
126+
$server = new TcpServer(0, $loop);
127+
$server = new SecureServer($server, $loop, array(
128+
'local_cert' => __DIR__ . '/../examples/localhost.pem',
129+
'crypto_method' => STREAM_CRYPTO_METHOD_TLSv1_2_SERVER
130+
));
131+
132+
$connector = new SecureConnector(new TcpConnector($loop), $loop, array(
133+
'verify_peer' => false
134+
));
135+
$promise = $connector->connect($server->getAddress());
136+
137+
/* @var ConnectionInterface $client */
138+
$client = Block\await($promise, $loop, self::TIMEOUT);
139+
140+
$this->assertInstanceOf('React\Socket\Connection', $client);
141+
$this->assertTrue(isset($client->stream));
142+
143+
$meta = stream_get_meta_data($client->stream);
144+
$this->assertTrue(isset($meta['crypto']['protocol']));
145+
$this->assertEquals('TLSv1.2', $meta['crypto']['protocol']);
146+
}
147+
51148
public function testServerEmitsConnectionForClientConnection()
52149
{
53150
$loop = Factory::create();
@@ -621,4 +718,21 @@ private function createPromiseForEvent(EventEmitterInterface $emitter, $event, $
621718
});
622719
});
623720
}
721+
722+
private function supportsTls13()
723+
{
724+
// TLS 1.3 is supported as of OpenSSL 1.1.1 (https://www.openssl.org/blog/blog/2018/09/11/release111/)
725+
// The OpenSSL library version can only be obtained by parsing output from phpinfo().
726+
// OPENSSL_VERSION_TEXT refers to header version which does not necessarily match actual library version
727+
// see php -i | grep OpenSSL
728+
// OpenSSL Library Version => OpenSSL 1.1.1 11 Sep 2018
729+
ob_start();
730+
phpinfo(INFO_MODULES);
731+
$info = ob_get_clean();
732+
733+
if (preg_match('/OpenSSL Library Version => OpenSSL (\S+)/', $info, $match)) {
734+
return version_compare($match[1], '1.1.1', '>=');
735+
}
736+
return false;
737+
}
624738
}

0 commit comments

Comments
 (0)