From 788b1f66ccb0a98bc7da22d73fae45e2ad485458 Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Fri, 2 Dec 2011 23:58:08 -0500 Subject: [PATCH] RFC handshake verification + unit tests Went through section 4 of RFC 6455 making sure incoming handshake was valid with accompanying unit tests --- lib/Ratchet/Application/WebSocket/App.php | 4 +- .../Application/WebSocket/Util/HTTP.php | 14 +- .../Application/WebSocket/Version/HyBi10.php | 3 + .../Application/WebSocket/Version/RFC6455.php | 14 +- .../Version/RFC6455/HandshakeVerifier.php | 138 +++++++++++++++ .../WebSocket/Version/VersionInterface.php | 2 + .../Version/RFC6455/HandshakeVerifierTest.php | 166 ++++++++++++++++++ .../WebSocket/Version/RFC6455Test.php | 126 +++++++++++++ 8 files changed, 464 insertions(+), 3 deletions(-) create mode 100644 lib/Ratchet/Application/WebSocket/Version/RFC6455/HandshakeVerifier.php create mode 100644 tests/Ratchet/Tests/Application/WebSocket/Version/RFC6455/HandshakeVerifierTest.php create mode 100644 tests/Ratchet/Tests/Application/WebSocket/Version/RFC6455Test.php diff --git a/lib/Ratchet/Application/WebSocket/App.php b/lib/Ratchet/Application/WebSocket/App.php index 4600473..362039c 100644 --- a/lib/Ratchet/Application/WebSocket/App.php +++ b/lib/Ratchet/Application/WebSocket/App.php @@ -7,7 +7,6 @@ use Ratchet\Resource\Command\Factory; use Ratchet\Resource\Command\CommandInterface; use Ratchet\Resource\Command\Action\SendMessage; use Ratchet\Application\WebSocket\Util\HTTP; -use Ratchet\Application\WebSocket\Version; /** * The adapter to handle WebSocket requests/responses @@ -37,6 +36,7 @@ class App implements ApplicationInterface, ConfiguratorInterface { protected $_versions = array( 'HyBi10' => null , 'Hixie76' => null + , 'RFC6455' => null ); protected $_mask_payload = false; @@ -75,6 +75,7 @@ class App implements ApplicationInterface, ConfiguratorInterface { /** * Do handshake, frame/unframe messages coming/going in stack * @todo This needs some major refactoring + * @todo "Once the client's opening handshake has been sent, the client MUST wait for a response from the server before sending any further data." */ public function onMessage(Connection $from, $msg) { if (true !== $from->WebSocket->handshake) { @@ -209,6 +210,7 @@ class App implements ApplicationInterface, ConfiguratorInterface { * @return Version\VersionInterface * @throws UnderFlowException If we think the entire header message hasn't been buffered yet * @throws InvalidArgumentException If we can't understand protocol version request + * @todo Verify the first line of the HTTP header as per page 16 of RFC 6455 */ protected function getVersion($message) { if (false === strstr($message, "\r\n\r\n")) { // This CAN fail with Hixie, depending on the TCP buffer in between diff --git a/lib/Ratchet/Application/WebSocket/Util/HTTP.php b/lib/Ratchet/Application/WebSocket/Util/HTTP.php index 57dcc55..c8f711f 100644 --- a/lib/Ratchet/Application/WebSocket/Util/HTTP.php +++ b/lib/Ratchet/Application/WebSocket/Util/HTTP.php @@ -4,6 +4,7 @@ namespace Ratchet\Application\WebSocket\Util; /** * A helper class for handling HTTP requests * @todo Needs re-write...http_parse_headers is a PECL extension that changes the case to unexpected values + * @todo Again, RE-WRITE - I want all the expected headers to at least be set in the returned, even if not there, set as null - having to do too much work in HandshaekVerifier */ class HTTP { /** @@ -12,7 +13,18 @@ class HTTP { * @return array */ public static function getHeaders($http_message) { - return function_exists('http_parse_headers') ? http_parse_headers($http_message) : self::http_parse_headers($http_message); + $header_array = function_exists('http_parse_headers') ? http_parse_headers($http_message) : self::http_parse_headers($http_message); + + return $header_array + array( + 'Host' => null + , 'Upgrade' => null + , 'Connection' => null + , 'Sec-Websocket-Key' => null + , 'Origin' => null + , 'Sec-Websocket-Protocol' => null + , 'Sec-Websocket-Version' => null + , 'Sec-Websocket-Origin' => null + ); } /** diff --git a/lib/Ratchet/Application/WebSocket/Version/HyBi10.php b/lib/Ratchet/Application/WebSocket/Version/HyBi10.php index 5d90b66..39f20ea 100644 --- a/lib/Ratchet/Application/WebSocket/Version/HyBi10.php +++ b/lib/Ratchet/Application/WebSocket/Version/HyBi10.php @@ -1,6 +1,9 @@ = 6 to 6 through 10 (or w/e #) + * @var RFC6455\HandshakeVerifier */ + protected $_verifier; + + public function __construct() { + $this->_verifier = new HandshakeVerifier; + } + public static function isProtocol(array $headers) { if (isset($headers['Sec-Websocket-Version'])) { if ((int)$headers['Sec-Websocket-Version'] == 13) { @@ -24,11 +31,16 @@ class RFC6455 implements VersionInterface { /** * @return array * I kept this as an array and combined in App for future considerations...easier to add a subprotol as a key value than edit a string + * @todo Decide what to do on failure...currently throwing an exception and I think socket connection is closed. Should be sending 40x error - but from where? */ public function handshake($message) { $headers = HTTP::getHeaders($message); $key = $this->sign($headers['Sec-Websocket-Key']); + if (true !== $this->_verifier->verifyAll($headers)) { + throw new \InvalidArgumentException('Invalid HTTP header'); + } + return array( '' => 'HTTP/1.1 101 Switching Protocols' , 'Upgrade' => 'websocket' diff --git a/lib/Ratchet/Application/WebSocket/Version/RFC6455/HandshakeVerifier.php b/lib/Ratchet/Application/WebSocket/Version/RFC6455/HandshakeVerifier.php new file mode 100644 index 0000000..13e8c59 --- /dev/null +++ b/lib/Ratchet/Application/WebSocket/Version/RFC6455/HandshakeVerifier.php @@ -0,0 +1,138 @@ +verifyMethod($headers['Request Method']); + //$passes += (int)$this->verifyHTTPVersion($headers['???']); // This isn't in the array! + $passes += (int)$this->verifyRequestURI($headers['Request Url']); + $passes += (int)$this->verifyHost($headers['Host']); + $passes += (int)$this->verifyUpgradeRequest($headers['Upgrade']); + $passes += (int)$this->verifyConnection($headers['Connection']); + $passes += (int)$this->verifyKey($headers['Sec-Websocket-Key']); + //$passes += (int)$this->verifyVersion($headers['Sec-Websocket-Version']); // Temporarily breaking functionality + + return (6 === $passes); + } + + /** + * Test the HTTP method. MUST be "GET" + * @param string + * @return bool + * @todo Look into STD if "get" is valid (am I supposed to do case conversion?) + */ + public function verifyMethod($val) { + return ('GET' === $val); + } + + /** + * Test the HTTP version passed. MUST be 1.1 or greater + * @param string|int + * @return bool + */ + public function verifyHTTPVersion($val) { + return (1.1 <= (double)$val); + } + + /** + * @param string + * @return bool + * @todo Implement this functionality + */ + public function verifyRequestURI($val) { + return true; + } + + /** + * @param string|null + * @return bool + * @todo Find out if I can find the master socket, ensure the port is attached to header if not 80 or 443 - not sure if this is possible, as I tried to hide it + * @todo Once I fix HTTP::getHeaders just verify this isn't NULL or empty...or manybe need to verify it's a valid domin??? Or should it equal $_SERVER['HOST'] ? + */ + public function verifyHost($val) { + return (null !== $val); + } + + /** + * Verify the Upgrade request to WebSockets. + * @param string MUST equal "websocket" + * @return bool + */ + public function verifyUpgradeRequest($val) { + return ('websocket' === $val); + } + + /** + * Verify the Connection header + * @param string MUST equal "Upgrade" + * @return bool + */ + public function verifyConnection($val) { + if ('Upgrade' === $val) { + return true; + } + + $vals = explode(',', str_replace(', ', ',', $val)); + return (false !== array_search('Upgrade', $vals)); + } + + /** + * This function verifyies the nonce is valid (64 big encoded, 16 bytes random string) + * @param string|null + * @return bool + * @todo The spec says we don't need to base64_decode - can I just check if the length is 24 and not decode? + */ + public function verifyKey($val) { + return (16 === strlen(base64_decode((string)$val))); + } + + /** + * Verify Origin matches RFC6454 IF it is set + * Origin is an optional field + * @param string|null + * @return bool + * @todo Implement verification functality - see section 4.2.1.7 + */ + public function verifyOrigin($val) { + if (null === $val) { + return true; + } + + // logic here + return true; + } + + /** + * Verify the version passed matches this RFC + * @param string|int MUST equal 13|"13" + * @return bool + * @todo Ran in to a problem here...I'm having HyBi use the RFC files, this breaks it! oops + */ + public function verifyVersion($val) { + return (13 === (int)$val); + } + + /** + * @todo Write logic for this method. See section 4.2.1.8 + */ + public function verifyProtocol($val) { + } + + /** + * @todo Write logic for this method. See section 4.2.1.9 + */ + public function verifyExtensions($val) { + } +} \ No newline at end of file diff --git a/lib/Ratchet/Application/WebSocket/Version/VersionInterface.php b/lib/Ratchet/Application/WebSocket/Version/VersionInterface.php index 461e149..53ce979 100644 --- a/lib/Ratchet/Application/WebSocket/Version/VersionInterface.php +++ b/lib/Ratchet/Application/WebSocket/Version/VersionInterface.php @@ -20,6 +20,8 @@ interface VersionInterface { * Perform the handshake and return the response headers * @param string * @return array|string + * @throws InvalidArgumentException If the HTTP handshake is mal-formed + * @throws UnderflowException If the message hasn't finished buffering (not yet implemented, theoretically will only happen with Hixie version) */ function handshake($message); diff --git a/tests/Ratchet/Tests/Application/WebSocket/Version/RFC6455/HandshakeVerifierTest.php b/tests/Ratchet/Tests/Application/WebSocket/Version/RFC6455/HandshakeVerifierTest.php new file mode 100644 index 0000000..217e984 --- /dev/null +++ b/tests/Ratchet/Tests/Application/WebSocket/Version/RFC6455/HandshakeVerifierTest.php @@ -0,0 +1,166 @@ +_v = new HandshakeVerifier; + } + + public static function methodProvider() { + return array( + array(true, 'GET') + , array(false, 'get') // I'm not sure if this is valid or not, need to check standard + , array(false, 'POST') + , array(false, 'DELETE') + , array(false, 'PUT') + , array(false, 'PATCH') + ); + } + + /** + * @dataProvider methodProvider + */ + public function testMethodMustBeGet($result, $in) { + $this->assertEquals($result, $this->_v->verifyMethod($in)); + } + + public static function httpVersionProvider() { + return array( + array(true, 1.1) + , array(true, '1.1') + , array(true, 1.2) + , array(true, '1.2') + , array(true, 2) + , array(true, '2') + , array(true, '2.0') + , array(false, '1.0') + , array(false, 1) + , array(false, '0.9') + , array(false, '') + , array(false, 'hello') + ); + } + + /** + * @dataProvider httpVersionProvider + */ + public function testHttpVersionIsAtLeast1Point1($expected, $in) { + $this->assertEquals($expected, $this->_v->verifyHTTPVersion($in)); + } + + /** + * @todo Add failing values in here + */ + public static function uRIProvider() { + return array( + array(true, '/chat') + ); + } + + /** + * @dataProvider URIProvider + */ + public function testRequestUri($expected, $in) { + return $this->markTestIncomplete('Method this test is testing is incomplete'); + + $this->assertEquals($expected, $this->_v->verifyRequestURI($in)); + } + + public static function hostProvider() { + return array( + array(true, 'server.example.com') + , array(false, null) + ); + } + + /** + * @dataProvider HostProvider + */ + public function testVerifyHostIsSet($expected, $in) { + $this->assertEquals($expected, $this->_v->verifyHost($in)); + } + + public static function upgradeProvider() { + return array( + array(true, 'websocket') + , array(false, 'Websocket') + , array(false, null) + , array(false, '') + ); + } + + /** + * @dataProvider upgradeProvider + */ + public function testVerifyUpgradeIsWebSocket($expected, $val) { + $this->assertEquals($expected, $this->_v->verifyUpgradeRequest($val)); + } + + public static function connectionProvider() { + return array( + array(true, 'Upgrade') + , array(false, 'upgrade') + , array(true, 'keep-alive, Upgrade') + , array(true, 'Upgrade, keep-alive') + , array(true, 'keep-alive, Upgrade, something') + , array(false, '') + , array(false, null) + ); + } + + /** + * @dataProvider connectionProvider + */ + public function testConnectionHeaderVerification($expected, $val) { + $this->assertEquals($expected, $this->_v->verifyConnection($val)); + } + + public static function keyProvider() { + return array( + array(true, 'hkfa1L7uwN6DCo4IS3iWAw==') + , array(true, '765vVoQpKSGJwPzJIMM2GA==') + , array(true, 'AQIDBAUGBwgJCgsMDQ4PEC==') + , array(true, 'axa2B/Yz2CdpfQAY2Q5P7w==') + , array(false, 0) + , array(false, 'Hello World') + , array(false, '1234567890123456') + , array(false, '123456789012345678901234') + ); + } + + /** + * @dataProvider keyProvider + */ + public function testKeyIsBase64Encoded16BitNonce($expected, $val) { + $this->assertEquals($expected, $this->_v->verifyKey($val)); + } + + public static function versionProvider() { + return array( + array(true, 13) + , array(true, '13') + , array(false, 12) + , array(false, 14) + , array(false, '14') + , array(false, 'hi') + , array(false, '') + , array(false, null) + ); + } + + /** + * @dataProvider versionProvider + */ + public function testVersionEquals13($expected, $in) { + $this->assertEquals($expected, $this->_v->verifyVersion($in)); + } +} \ No newline at end of file diff --git a/tests/Ratchet/Tests/Application/WebSocket/Version/RFC6455Test.php b/tests/Ratchet/Tests/Application/WebSocket/Version/RFC6455Test.php new file mode 100644 index 0000000..d34b6c7 --- /dev/null +++ b/tests/Ratchet/Tests/Application/WebSocket/Version/RFC6455Test.php @@ -0,0 +1,126 @@ +_version = new RFC6455(); + } + + /** + * Is this useful? + */ + public function testClassImplementsVersionInterface() { + $constraint = $this->isInstanceOf('\\Ratchet\\Application\\WebSocket\\Version\\VersionInterface'); + $this->assertThat($this->_version, $constraint); + } + + /** + * @dataProvider HandshakeProvider + */ + public function testKeySigningForHandshake($key, $accept) { + $this->assertEquals($accept, $this->_version->sign($key)); + } + + public static function HandshakeProvider() { + return array( + array('x3JJHMbDL1EzLkh9GBhXDw==', 'HSmrc0sMlYUkAGmm5OPpG2HaGWk=') + , array('dGhlIHNhbXBsZSBub25jZQ==', 's3pPLMBiTxaQ9kYGzzhZRbK+xOo=') + ); + } + + /** + * @dataProvider UnframeMessageProvider + */ + public function testUnframeMessage($message, $framed) { + $frame = new Frame; + $frame->addBuffer(base64_decode($framed)); + + $this->assertEquals($message, $frame->getPayload()); + } + + public static function UnframeMessageProvider() { + return array( + array('Hello World!', 'gYydAIfa1WXrtvIg0LXvbOP7') + , array('!@#$%^&*()-=_+[]{}\|/.,<>`~', 'gZv+h96r38f9j9vZ+IHWrvOWoayF9oX6gtfRqfKXwOeg') + , array('ಠ_ಠ', 'gYfnSpu5B/g75gf4Ow==') + , array("The quick brown fox jumps over the lazy dog. All work and no play makes Chris a dull boy. I'm trying to get past 128 characters for a unit test here...", 'gf4Amahb14P8M7Kj2S6+4MN7tfHHLLmjzjSvo8IuuvPbe7j1zSn398A+9+/JIa6jzDSwrYh7lu/Ee6Ds2jD34sY/9+3He6fvySL37skwsvCIGL/xwSj34og/ou/Ee7Xs0XX3o+F8uqPcKa7qxjz398d7sObce6fi2y/3sppj9+DAOqXiyy+y8dt7sezae7aj3TW+94gvsvDce7/m2j75rYY=') + ); + } + + public function testUnframeMatchesPreFraming() { + $string = 'Hello World!'; + $framed = $this->_version->frame($string); + + $frame = new Frame; + $frame->addBuffer($framed); + + $this->assertEquals($string, $frame->getPayload()); + } + + public static $good_rest = 'GET /chat HTTP/1.1'; + + public static $good_header = array( + 'Host' => 'server.example.com' + , 'Upgrade' => 'websocket' + , 'Connection' => 'Upgrade' + , 'Sec-WebSocket-Key' => 'dGhlIHNhbXBsZSBub25jZQ==' + , 'Origin' => 'http://example.com' + , 'Sec-WebSocket-Protocol' => 'chat, superchat' + , 'Sec-WebSocket-Version' => 13 + ); + + /** + * A helper function to try and quickly put together a valid WebSocket HTTP handshake + * but optionally replace a piece to an invalid value for failure testing + */ + public static function getAndSpliceHeader($key = null, $val = null) { + $headers = static::$good_header; + + if (null !== $key && null !== $val) { + $headers[$key] = $val; + } + + $header = ''; + foreach ($headers as $key => $val) { + if (!empty($key)) { + $header .= "{$key}: "; + } + + $header .= "{$val}\r\n"; + } + $header .= "\r\n"; + + return $header; + } + + public static function headerHandshakeProvider() { + return array( + array(false, "GET /test HTTP/1.0\r\n" . static::getAndSpliceHeader()) + ); + } + +/* RFC example of a good header + GET /chat HTTP/1.1 + Host: server.example.com + Upgrade: websocket + Connection: Upgrade + Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ== + Origin: http://example.com + Sec-WebSocket-Protocol: chat, superchat + Sec-WebSocket-Version: 13 +*/ + + /** + * @dataProvider headerHandshakeProvider + */ + public function testVariousHeadersToCheckHandshakeTolerance($pass, $header) { + $this->markTestIncomplete(); + } +} \ No newline at end of file