Merge pull request #69 from clue-labs/secure-connection

Documentation and tests for exposing secure context options

Author: WyriHaximus

README.md

@@ -263,6 +263,11 @@ $server = new SecureServer($server, $loop, array(
 ));
 ```
 
+> Note that available [TLS context options](http://php.net/manual/en/context.ssl.php),
+their defaults and effects of changing these may vary depending on your system
+and/or PHP version.
+Passing unknown context options has no effect.
+
 Whenever a client completes the TLS handshake, it will emit a `connection` event
 with a connection instance implementing [`ConnectionInterface`](#connectioninterface):
 
@@ -286,6 +291,19 @@ $server->on('error', function (Exception $e) {
 
 See also the [`ServerInterface`](#serverinterface) for more details.
 
+Note that the `SecureServer` class is a concrete implementation for TLS sockets.
+If you want to typehint in your higher-level protocol implementation, you SHOULD
+use the generic [`ServerInterface`](#serverinterface) instead.
+
+> Advanced usage: Internally, the `SecureServer` has to set the required
+context options on the underlying stream resources.
+It should therefor be used with an unmodified `Server` instance as first
+parameter so that it can allocate an empty context resource which this
+class uses to set required TLS context options.
+Failing to do so may result in some hard to trace race conditions,
+because all stream resources will use a single, shared default context
+resource otherwise.
+
 ### ConnectionInterface
 
 The `ConnectionInterface` is used to represent any incoming connection.

src/SecureServer.php

@@ -6,41 +6,110 @@
 use React\EventLoop\LoopInterface;
 use React\Socket\Server;
 use React\Socket\ConnectionInterface;
+use React\Stream\Stream;
 
 /**
  * The `SecureServer` class implements the `ServerInterface` and is responsible
  * for providing a secure TLS (formerly known as SSL) server.
  *
  * It does so by wrapping a `Server` instance which waits for plaintext
  * TCP/IP connections and then performs a TLS handshake for each connection.
- * It thus requires valid [TLS context options],
- * which in its most basic form may look something like this if you're using a
- * PEM encoded certificate file:
  *
+ * ```php
+ * $server = new Server(8000, $loop);
+ * $server = new SecureServer($server, $loop, array(
+ *     // tls context options here…
+ * ));
  * ```
- * $context = array(
- *     'local_cert' => __DIR__ . '/localhost.pem'
- * );
+ *
+ * Whenever a client completes the TLS handshake, it will emit a `connection` event
+ * with a connection instance implementing [`ConnectionInterface`](#connectioninterface):
+ *
+ * ```php
+ * $server->on('connection', function (ConnectionInterface $connection) {
+ *     echo 'Secure connection from' . $connection->getRemoteAddress() . PHP_EOL;
+ *
+ *     $connection->write('hello there!' . PHP_EOL);
+ *     …
+ * });
  * ```
  *
- * If your private key is encrypted with a passphrase, you have to specify it
- * like this:
+ * Whenever a client fails to perform a successful TLS handshake, it will emit an
+ * `error` event and then close the underlying TCP/IP connection:
  *
  * ```php
- * $context = array(
- *     'local_cert' => 'server.pem',
- *     'passphrase' => 'secret'
- * );
+ * $server->on('error', function (Exception $e) {
+ *     echo 'Error' . $e->getMessage() . PHP_EOL;
+ * });
  * ```
  *
- * @see Server
- * @link http://php.net/manual/en/context.ssl.php for TLS context options
+ * See also the `ServerInterface` for more details.
+ *
+ * Note that the `SecureServer` class is a concrete implementation for TLS sockets.
+ * If you want to typehint in your higher-level protocol implementation, you SHOULD
+ * use the generic `ServerInterface` instead.
+ *
+ * @see ServerInterface
+ * @see ConnectionInterface
  */
 class SecureServer extends EventEmitter implements ServerInterface
 {
     private $tcp;
     private $encryption;
 
+    /**
+     * Creates a secure TLS server and starts waiting for incoming connections
+     *
+     * It does so by wrapping a `Server` instance which waits for plaintext
+     * TCP/IP connections and then performs a TLS handshake for each connection.
+     * It thus requires valid [TLS context options],
+     * which in its most basic form may look something like this if you're using a
+     * PEM encoded certificate file:
+     *
+     * ```php
+     * $server = new Server(8000, $loop);
+     * $server = new SecureServer($server, $loop, array(
+     *     'local_cert' => 'server.pem'
+     * ));
+     * ```
+     *
+     * Note that the certificate file will not be loaded on instantiation but when an
+     * incoming connection initializes its TLS context.
+     * This implies that any invalid certificate file paths or contents will only cause
+     * an `error` event at a later time.
+     *
+     * If your private key is encrypted with a passphrase, you have to specify it
+     * like this:
+     *
+     * ```php
+     * $server = new Server(8000, $loop);
+     * $server = new SecureServer($server, $loop, array(
+     *     'local_cert' => 'server.pem',
+     *     'passphrase' => 'secret'
+     * ));
+     * ```
+     *
+     * Note that available [TLS context options],
+     * their defaults and effects of changing these may vary depending on your system
+     * and/or PHP version.
+     * Passing unknown context options has no effect.
+     *
+     * Advanced usage: Internally, the `SecureServer` has to set the required
+     * context options on the underlying stream resources.
+     * It should therefor be used with an unmodified `Server` instance as first
+     * parameter so that it can allocate an empty context resource which this
+     * class uses to set required TLS context options.
+     * Failing to do so may result in some hard to trace race conditions,
+     * because all stream resources will use a single, shared default context
+     * resource otherwise.
+     *
+     * @param Server $tcp
+     * @param LoopInterface $loop
+     * @param array $context
+     * @throws ConnectionException
+     * @see Server
+     * @link http://php.net/manual/en/context.ssl.php for TLS context options
+     */
     public function __construct(Server $tcp, LoopInterface $loop, array $context)
     {
         if (!is_resource($tcp->master)) {
@@ -81,6 +150,12 @@ public function close()
     /** @internal */
     public function handleConnection(ConnectionInterface $connection)
     {
+        if (!$connection instanceof Stream) {
+            $this->emit('error', array(new \UnexpectedValueException('Connection event MUST emit an instance extending Stream in order to access underlying stream resource')));
+            $connection->end();
+            return;
+        }
+
         $that = $this;
 
         $this->encryption->enable($connection)->then(

tests/SecureServerTest.php

@@ -38,4 +38,35 @@ public function testCloseWillBePassedThroughToTcpServer()
 
         $server->close();
     }
+
+    public function testConnectionWillBeEndedWithErrorIfItIsNotAStream()
+    {
+        $tcp = $this->getMockBuilder('React\Socket\Server')->disableOriginalConstructor()->setMethods(null)->getMock();
+        $tcp->master = stream_socket_server('tcp://localhost:0');
+
+        $loop = $this->getMock('React\EventLoop\LoopInterface');
+
+        $connection = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock();
+        $connection->expects($this->once())->method('end');
+
+        $server = new SecureServer($tcp, $loop, array());
+
+        $server->on('error', $this->expectCallableOnce());
+
+        $tcp->emit('connection', array($connection));
+    }
+
+    public function testSocketErrorWillBeForwarded()
+    {
+        $tcp = $this->getMockBuilder('React\Socket\Server')->disableOriginalConstructor()->setMethods(null)->getMock();
+        $tcp->master = stream_socket_server('tcp://localhost:0');
+
+        $loop = $this->getMock('React\EventLoop\LoopInterface');
+
+        $server = new SecureServer($tcp, $loop, array());
+
+        $server->on('error', $this->expectCallableOnce());
+
+        $tcp->emit('error', array(new \RuntimeException('test')));
+    }
 }