Skip to content

Commit

Permalink
Implement SSL peer verification in HttpSocket
Browse files Browse the repository at this point in the history 
Use the stream API and enable SSL certificate validation. This
increases the security features of HttpSocket, and provides easy access
to the stream context options.  SSL related configuration should be
prefixed with 'ssl_'.

Refs #2270
  • Loading branch information
@markstory
markstory committed Nov 11, 2012
1 parent 7a5dfdf commit 240c871
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 9 deletions.
42 changes: 37 additions & 5 deletions lib/Cake/Network/CakeSocket.php
View file
Expand Up @@ -101,6 +101,14 @@ class CakeSocket {
// @codingStandardsIgnoreEnd
);

/**
* Used to capture connection warnings which can happen when there are
* SSL errors for example.
*
* @var array
*/
protected $_connectionErrors = array();

/**
* Constructor.
*
Expand Down Expand Up @@ -130,43 +138,67 @@ public function connect() {
$scheme = 'ssl://';
}

if (!empty($this->config['request']['context'])) {
$context = stream_context_create($this->config['request']['context']);
if (!empty($this->config['context'])) {
$context = stream_context_create($this->config['context']);
} else {
$context = stream_context_create();
}

$connectAs = STREAM_CLIENT_CONNECT;
if ($this->config['persistent']) {
$connectAs = STREAM_CLIENT_PERSISTENT;
$connectAs |= STREAM_CLIENT_PERSISTENT;
}
$this->connection = @stream_socket_client(

set_error_handler(array($this, '_connectionErrorHandler'));
$this->connection = stream_socket_client(
$scheme . $this->config['host'] . ':' . $this->config['port'],
$errNum,
$errStr,
$this->config['timeout'],
$connectAs,
$context
);
restore_error_handler();

if (!empty($errNum) || !empty($errStr)) {
$this->setLastError($errNum, $errStr);
throw new SocketException($errStr, $errNum);
}

if (!$this->connection && $this->_connectionErrors) {
$message = implode("\n", $this->_connectionErrors);
throw new SocketException($message, E_WARNING);
}

$this->connected = is_resource($this->connection);
if ($this->connected) {
stream_set_timeout($this->connection, $this->config['timeout']);
}
return $this->connected;
}

/**
* socket_stream_client() does not populate errNum, or $errStr when there are
* connection errors, as in the case of SSL verification failure.
*
* Instead we need to handle those errors manually.
*
* @param int $code
* @param string $message
*/
protected function _connectionErrorHandler($code, $message) {
$this->_connectionErrors[] = $message;
}

/**
* Get the connection context.
*
* @return array
* @return null|array Null when there is no connnection, an array when there is.
*/
public function context() {
if (!$this->connection) {
return;
}
return stream_context_get_options($this->connection);
}

Expand Down
38 changes: 36 additions & 2 deletions lib/Cake/Network/Http/HttpSocket.php
View file
Expand Up @@ -18,6 +18,7 @@
*/
App::uses('CakeSocket', 'Network');
App::uses('Router', 'Routing');
App::uses('Hash', 'Utility');

/**
* Cake network socket connection class.
Expand Down Expand Up @@ -64,7 +65,7 @@ class HttpSocket extends CakeSocket {
),
'raw' => null,
'redirect' => false,
'cookies' => array()
'cookies' => array(),
);

/**
Expand Down Expand Up @@ -92,14 +93,17 @@ class HttpSocket extends CakeSocket {
'protocol' => 'tcp',
'port' => 80,
'timeout' => 30,
'ssl_verify_peer' => true,
'ssl_verify_depth' => 5,
'ssl_verify_host' => true,
'request' => array(
'uri' => array(
'scheme' => array('http', 'https'),
'host' => 'localhost',
'port' => array(80, 443)
),
'redirect' => false,
'cookies' => array()
'cookies' => array(),
)
);

Expand Down Expand Up @@ -348,6 +352,8 @@ public function request($request = array()) {
return false;
}

$this->_configContext($this->request['uri']['host']);

$this->request['raw'] = '';
if ($this->request['line'] !== false) {
$this->request['raw'] = $this->request['line'];
Expand Down Expand Up @@ -395,6 +401,7 @@ public function request($request = array()) {
throw new SocketException(__d('cake_dev', 'Class %s not found.', $this->responseClass));
}
$this->response = new $responseClass($response);

if (!empty($this->response->cookies)) {
if (!isset($this->config['request']['cookies'][$Host])) {
$this->config['request']['cookies'][$Host] = array();
Expand Down Expand Up @@ -643,6 +650,33 @@ protected function _configUri($uri = null) {
return true;
}

/**
* Configure the socket's context. Adds in configuration
* that can not be declared in the class definition.
*
* @param string $host The host you're connecting to.
* @return void
*/
protected function _configContext($host) {
foreach ($this->config as $key => $value) {
if (substr($key, 0, 4) !== 'ssl_') {
continue;
}
$contextKey = substr($key, 4);
if (empty($this->config['context']['ssl'][$contextKey])) {
$this->config['context']['ssl'][$contextKey] = $value;
}
unset($this->config[$key]);
}
if (empty($this->_context['ssl']['cafile'])) {
$this->config['context']['ssl']['cafile'] = CAKE . 'Config' . DS . 'cacert.pem';
}
if (!empty($this->config['context']['ssl']['verify_host'])) {
$this->config['context']['ssl']['CN_match'] = $host;
unset($this->config['context']['ssl']['verify_host']);
}
}

/**
* Takes a $uri array and turns it into a fully qualified URL string
*
Expand Down
53 changes: 51 additions & 2 deletions lib/Cake/Test/Case/Network/Http/HttpSocketTest.php
View file
Expand Up @@ -253,14 +253,17 @@ public function testConfigUri() {
'protocol' => 'tcp',
'port' => 23,
'timeout' => 30,
'ssl_verify_peer' => true,
'ssl_verify_depth' => 5,
'ssl_verify_host' => true,
'request' => array(
'uri' => array(
'scheme' => 'https',
'host' => 'www.cakephp.org',
'port' => 23
),
'redirect' => false,
'cookies' => array()
'cookies' => array(),
)
);
$this->assertEquals($expected, $this->Socket->config);
Expand All @@ -278,14 +281,17 @@ public function testConfigUri() {
'protocol' => 'tcp',
'port' => 80,
'timeout' => 30,
'ssl_verify_peer' => true,
'ssl_verify_depth' => 5,
'ssl_verify_host' => true,
'request' => array(
'uri' => array(
'scheme' => 'http',
'host' => 'www.foo.com',
'port' => 80
),
'redirect' => false,
'cookies' => array()
'cookies' => array(),
)
);
$this->assertEquals($expected, $this->Socket->config);
Expand All @@ -311,6 +317,15 @@ public function testRequest() {
$response = $this->Socket->request(true);
$this->assertFalse($response);

$context = array(
'ssl' => array(
'verify_peer' => true,
'verify_depth' => 5,
'CN_match' => 'www.cakephp.org',
'cafile' => CAKE . 'Config' . DS . 'cacert.pem'
)
);

$tests = array(
array(
'request' => 'http://www.cakephp.org/?foo=bar',
Expand All @@ -321,6 +336,7 @@ public function testRequest() {
'protocol' => 'tcp',
'port' => 80,
'timeout' => 30,
'context' => $context,
'request' => array(
'uri' => array(
'scheme' => 'http',
Expand Down Expand Up @@ -1668,4 +1684,37 @@ public function testPartialReset() {
}
$this->assertEquals(true, $return);
}

/**
* test configuring the context from the flat keys.
*
* @return void
*/
public function testConfigContext() {
$this->Socket->reset();
$this->Socket->request('http://example.com');
$this->assertTrue($this->Socket->config['context']['ssl']['verify_peer']);
$this->assertEquals(5, $this->Socket->config['context']['ssl']['verify_depth']);
$this->assertEquals('example.com', $this->Socket->config['context']['ssl']['CN_match']);
$this->assertArrayNotHasKey('ssl_verify_peer', $this->Socket->config);
$this->assertArrayNotHasKey('ssl_verify_host', $this->Socket->config);
$this->assertArrayNotHasKey('ssl_verify_depth', $this->Socket->config);
}

/**
* Test that requests fail when peer verification fails.
*
* @return void
*/
public function testVerifyPeer() {
$socket = new HttpSocket();
try {
$result = $socket->get('https://typography.com');
$this->markTestSkipped('Found valid certificate, was expecting invalid certificate.');
} catch (SocketException $e) {
$message = $e->getMessage();
$this->assertContains('Peer certificate CN', $message);
$this->assertContains('Failed to enable crypto', $message);
}
}
}

7 comments on commit 240c871

@ryantology
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this change is throwing new warnings. I am still trying to track down a proper abstracted test case but it looks like the change is not compatible with ElasticSource https://github.com/dkullmann/CakePHP-Elastic-Search-DataSource The new 'context' parameter is throwing a notice. I am going to look into how the Elastic Datasource is generating URLs and see if this can be updated.

This is the debug output that I have so far.


Notice (8): Array to string conversion [CORE/Cake/Network/Http/HttpSocket.php, line 718]
Code Context
$uri = array(
    'host' => '*****',
    'port' => '*****',
    'scheme' => 'http',
    'user' => null,
    'pass' => null,
    'path' => 'content/item/_search',
    'query' => '',
    'fragment' => null,
    'persistent' => false,
    'protocol' => (int) 6,
    'timeout' => (int) 30,
    'context' => array(
        'ssl' => array(
            'verify_peer' => true,
            'verify_depth' => (int) 5,
            'cafile' => '/Users/ryanw/Documents/Projects/PROJECT/lib/Cake/Config/cacert.pem',
            'CN_match' => '127.0.0.1'
        )
    )
)
$uriTemplate = 'http://127.0.0.1:9200/content/item/_search'
$stripIfEmpty = array(
    'host' => '*****',
    'query' => '?%query',
    'fragment' => '#%fragment',
    'user' => '%user:%pass@'
)
$key = 'host'
$strip = '%host:%port/'
$defaultPorts = array(
    'http' => (int) 80,
    'https' => (int) 443
)
$property = 'context'
$value = array(
    'ssl' => array(
        'verify_peer' => true,
        'verify_depth' => (int) 5,
        'cafile' => '/Users/ryanw/Documents/Projects/PROJECT/lib/Cake/Config/cacert.pem',
        'CN_match' => '127.0.0.1'
    )
)
str_replace - [internal], line ??
HttpSocket::_buildUri() - CORE/Cake/Network/Http/HttpSocket.php, line 718
HttpSocket::url() - CORE/Cake/Network/Http/HttpSocket.php, line 567
ElasticSource::logQuery() - APP/Plugin/Elastic/Model/Datasource/ElasticSource.php, line 1607
ElasticSource::execute() - APP/Plugin/Elastic/Model/Datasource/ElasticSource.php, line 1417
ElasticSource::__call() - APP/Plugin/Elastic/Model/Datasource/ElasticSource.php, line 1378
ElasticSource::get() - APP/Plugin/Elastic/Model/Datasource/ElasticSource.php, line 244
ElasticSource::read() - APP/Plugin/Elastic/Model/Datasource/ElasticSource.php, line 244
Model::find() - CORE/Cake/Model/Model.php, line 2674
ContentitemElastic::getRecent() - APP/Model/ContentitemElastic.php, line 54
ReflectionMethod::invokeArgs() - [internal], line ??
Controller::invokeAction() - CORE/Cake/Controller/Controller.php, line 485
Dispatcher::_invoke() - CORE/Cake/Routing/Dispatcher.php, line 186
Dispatcher::dispatch() - CORE/Cake/Routing/Dispatcher.php, line 161
[main] - APP/webroot/index.php, line 92

@ryantology
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue has to do with the persistent HttpSocket instance. It looks like the SSL peer verification does not clean up the 'context' value between requests. Here is an example.

    public function example() {
        App::uses('HttpSocket', 'Network/Http');
        $config = array(
            'datasource' => 'Example',
            'host' => '127.0.0.1',
            'port' => 80,
            'format' => 'application/json',
            'encoding' => 'utf-8',
        );
        extract($config);
        $scheme = 'http';
        $request = array('uri' => compact('host', 'port', 'scheme'), 'header' => array('Accept' => $config['format'], 'Content-Type' => $config['format']));
        $httpConfig = compact('host', 'port', 'request');
        $this->Http = new HttpSocket($httpConfig);

        // string query     
        $path = '/content/';
        $query  = null;
        $uri = $this->_uri(compact('path', 'query'));
        $body = null;
        debug($uri);
        $response = $this->Http->get($uri, array(), compact('body'));
        $uri = $this->_uri(compact('path', 'query'));
        debug($uri);
        $response = $this->Http->get($uri, array(), compact('body'));
        debug($response);

        $this->autoRender = false;
    }

    protected function _uri($config) {
        $config = Set::merge($this->Http->config, $config);
        unset($config['request']);
        return $config;
    }

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused as I don't use this specific datasource. Is it just merging all the config options into the request array? I don't see where in HttpSocket the context is merged into the request data. Also your example doesn't use persistent connections, is it still related to that?

@ryantology
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, the point about persistence wasn't very clear. What I meant was that the instance of Httpsocket is re-used rather than re-instantiated between requests.

As you pointed out the issue has to do with the reuse of $this->Http->config as the merged array of the uri value for Httpsocket::get The change in SSL peer verification commit pollutes the Httpsocket::config array and does not clean up the context value between requests.

It could be argued that ElasticSource is not implementing Httpsocket properly by merging the config parameters. As a patch to Elastic source I will remove the context parameter from the uri generator but it is possible that other people are implementing this the same way.

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll see if I can reproduce outside of the ElasticSearch source. Merging the entire config array into the url is dangerous and shouldn't be done.

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running the following from a shell method doesn't trigger any errors or warnings:

$http = new HttpSocket();
$res = $http->get('https://github.com/markstory');
debug($res->isOk());

$res2 = $http->get('https://github.com/lorenzo');
debug($res2->isOk());

Additionally $http->request does not contain the context options. I'm inclined to believe that the ElasticSearch datasource is doing something silly here.

@ryantology
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it has only to do with the http->settings being re-used for the computation of the URI. It's not really a bug in Httpsocket as much as an unexpected change. This was the fix that I pushed to ElasticSource https://github.com/IdeaCouture/CakePHP-Elastic-Search-DataSource/commit/4c7e669e4e33fb35e24e5b2b00e7a117fe07feb0 As you can see there is already a edge case for the 'request' parameter being an array. Sorry for the witch hunt.

I do think it is a bit strange to change the settings array after the construct is called but it's not 'wrong'. It just makes it strange that the settings for one call are different if the same call is made twice.

Please sign in to comment.