From e2c303b2b92129d3761ba6d2853bab13b0f35b9d Mon Sep 17 00:00:00 2001 From: Ian den Hartog Date: Thu, 1 Oct 2015 21:47:30 +0200 Subject: [PATCH 1/6] Add support for Self Signed certificates with smtp --- lib/Cake/Network/CakeSocket.php | 14 ++++++++++++++ lib/Cake/Network/Email/SmtpTransport.php | 6 +++++- lib/Cake/Test/Case/Network/Email/CakeEmailTest.php | 3 ++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Network/CakeSocket.php b/lib/Cake/Network/CakeSocket.php index aebdee191..13ce21be0 100644 --- a/lib/Cake/Network/CakeSocket.php +++ b/lib/Cake/Network/CakeSocket.php @@ -406,5 +406,19 @@ class CakeSocket { throw new SocketException($errorMessage); } +/** + * Accept Self-signed certificate on current stream socket + * + * @return bool True on success + * @see stream_context_set_option + */ + public function enableSelfSigned() { + $options['ssl'] = array( + 'allow_self_signed' => true, + 'verify_peer' => false, + 'verify_peer_name' => false + ); + return stream_context_set_option($this->connection, $options); + } } diff --git a/lib/Cake/Network/Email/SmtpTransport.php b/lib/Cake/Network/Email/SmtpTransport.php index 73af32b46..ab9a00755 100644 --- a/lib/Cake/Network/Email/SmtpTransport.php +++ b/lib/Cake/Network/Email/SmtpTransport.php @@ -118,7 +118,8 @@ class SmtpTransport extends AbstractTransport { 'username' => null, 'password' => null, 'client' => null, - 'tls' => false + 'tls' => false, + 'selfSigned' => false ); $this->_config = array_merge($default, $this->_config, $config); return $this->_config; @@ -168,6 +169,9 @@ class SmtpTransport extends AbstractTransport { $this->_smtpSend("EHLO {$host}", '250'); if ($this->_config['tls']) { $this->_smtpSend("STARTTLS", '220'); + if ($this->_config['selfSigned']) { + $this->_socket->enableSelfSigned(); + } $this->_socket->enableCrypto('tls'); $this->_smtpSend("EHLO {$host}", '250'); } diff --git a/lib/Cake/Test/Case/Network/Email/CakeEmailTest.php b/lib/Cake/Test/Case/Network/Email/CakeEmailTest.php index a86412ce6..0243db242 100644 --- a/lib/Cake/Test/Case/Network/Email/CakeEmailTest.php +++ b/lib/Cake/Test/Case/Network/Email/CakeEmailTest.php @@ -946,7 +946,8 @@ class CakeEmailTest extends CakeTestCase { 'username' => null, 'password' => null, 'client' => null, - 'tls' => false + 'tls' => false, + 'selfSigned' => false ); $this->assertEquals($expected, $this->CakeEmail->transportClass()->config()); From 5c722c666578df3e9710ae60372837c597b88099 Mon Sep 17 00:00:00 2001 From: Ian den Hartog Date: Fri, 2 Oct 2015 10:04:17 +0200 Subject: [PATCH 2/6] Fix peer verification --- lib/Cake/Network/CakeSocket.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/Cake/Network/CakeSocket.php b/lib/Cake/Network/CakeSocket.php index 13ce21be0..56c370494 100644 --- a/lib/Cake/Network/CakeSocket.php +++ b/lib/Cake/Network/CakeSocket.php @@ -413,12 +413,7 @@ class CakeSocket { * @see stream_context_set_option */ public function enableSelfSigned() { - $options['ssl'] = array( - 'allow_self_signed' => true, - 'verify_peer' => false, - 'verify_peer_name' => false - ); - return stream_context_set_option($this->connection, $options); + return stream_context_set_option($this->connection, 'ssl', 'allow_self_signed', true); } } From bb7e7850ae6205c2d474cfc534efc1ec8e10cfe3 Mon Sep 17 00:00:00 2001 From: Ian den Hartog Date: Fri, 2 Oct 2015 16:17:26 +0200 Subject: [PATCH 3/6] Add test for Self-signed certificates --- lib/Cake/Network/CakeSocket.php | 1 + lib/Cake/Test/Case/Network/CakeSocketTest.php | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/lib/Cake/Network/CakeSocket.php b/lib/Cake/Network/CakeSocket.php index 56c370494..7adff35bb 100644 --- a/lib/Cake/Network/CakeSocket.php +++ b/lib/Cake/Network/CakeSocket.php @@ -410,6 +410,7 @@ class CakeSocket { * Accept Self-signed certificate on current stream socket * * @return bool True on success + * @link http://php.net/manual/en/context.ssl.php About the 'allow_self_signed' option. * @see stream_context_set_option */ public function enableSelfSigned() { diff --git a/lib/Cake/Test/Case/Network/CakeSocketTest.php b/lib/Cake/Test/Case/Network/CakeSocketTest.php index 1d095e8da..91cc1f4d4 100644 --- a/lib/Cake/Test/Case/Network/CakeSocketTest.php +++ b/lib/Cake/Test/Case/Network/CakeSocketTest.php @@ -349,6 +349,18 @@ class CakeSocketTest extends CakeTestCase { $this->assertTrue($this->Socket->encrypted); } +/** + * testEnableCryptoSelfSigned + * + * @return void + */ + public function testEnableCryptoSelfSigned() { + $this->_connectSocketToSslTls(); + $this->assertTrue($this->Socket->enableSelfSigned()); + $this->assertTrue($this->Socket->enableCrypto('tls', 'client')); + $this->Socket->disconnect(); + } + /** * test getting the context for a socket. * From 5dfb780970867407e6e1b9dd0b829d23cd3c259d Mon Sep 17 00:00:00 2001 From: Ian den Hartog Date: Tue, 6 Oct 2015 10:10:34 +0200 Subject: [PATCH 4/6] Change names --- lib/Cake/Network/CakeSocket.php | 2 +- lib/Cake/Network/Email/SmtpTransport.php | 6 +++--- lib/Cake/Test/Case/Network/CakeSocketTest.php | 2 +- lib/Cake/Test/Case/Network/Email/CakeEmailTest.php | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Cake/Network/CakeSocket.php b/lib/Cake/Network/CakeSocket.php index 7adff35bb..2ad6832dc 100644 --- a/lib/Cake/Network/CakeSocket.php +++ b/lib/Cake/Network/CakeSocket.php @@ -413,7 +413,7 @@ class CakeSocket { * @link http://php.net/manual/en/context.ssl.php About the 'allow_self_signed' option. * @see stream_context_set_option */ - public function enableSelfSigned() { + public function allowSelfSigned() { return stream_context_set_option($this->connection, 'ssl', 'allow_self_signed', true); } } diff --git a/lib/Cake/Network/Email/SmtpTransport.php b/lib/Cake/Network/Email/SmtpTransport.php index ab9a00755..3f4144a65 100644 --- a/lib/Cake/Network/Email/SmtpTransport.php +++ b/lib/Cake/Network/Email/SmtpTransport.php @@ -119,7 +119,7 @@ class SmtpTransport extends AbstractTransport { 'password' => null, 'client' => null, 'tls' => false, - 'selfSigned' => false + 'ssl_allow_self_signed' => false ); $this->_config = array_merge($default, $this->_config, $config); return $this->_config; @@ -169,8 +169,8 @@ class SmtpTransport extends AbstractTransport { $this->_smtpSend("EHLO {$host}", '250'); if ($this->_config['tls']) { $this->_smtpSend("STARTTLS", '220'); - if ($this->_config['selfSigned']) { - $this->_socket->enableSelfSigned(); + if ($this->_config['ssl_allow_self_signed']) { + $this->_socket->allowSelfSigned(); } $this->_socket->enableCrypto('tls'); $this->_smtpSend("EHLO {$host}", '250'); diff --git a/lib/Cake/Test/Case/Network/CakeSocketTest.php b/lib/Cake/Test/Case/Network/CakeSocketTest.php index 91cc1f4d4..ff975564d 100644 --- a/lib/Cake/Test/Case/Network/CakeSocketTest.php +++ b/lib/Cake/Test/Case/Network/CakeSocketTest.php @@ -356,7 +356,7 @@ class CakeSocketTest extends CakeTestCase { */ public function testEnableCryptoSelfSigned() { $this->_connectSocketToSslTls(); - $this->assertTrue($this->Socket->enableSelfSigned()); + $this->assertTrue($this->Socket->allowSelfSigned()); $this->assertTrue($this->Socket->enableCrypto('tls', 'client')); $this->Socket->disconnect(); } diff --git a/lib/Cake/Test/Case/Network/Email/CakeEmailTest.php b/lib/Cake/Test/Case/Network/Email/CakeEmailTest.php index 0243db242..d27f2f709 100644 --- a/lib/Cake/Test/Case/Network/Email/CakeEmailTest.php +++ b/lib/Cake/Test/Case/Network/Email/CakeEmailTest.php @@ -947,7 +947,7 @@ class CakeEmailTest extends CakeTestCase { 'password' => null, 'client' => null, 'tls' => false, - 'selfSigned' => false + 'ssl_allow_self_signed' => false ); $this->assertEquals($expected, $this->CakeEmail->transportClass()->config()); From cc3531d2886c96afb9c53e363cd4eaa04f92b2f3 Mon Sep 17 00:00:00 2001 From: mark_story Date: Mon, 12 Oct 2015 21:56:20 -0400 Subject: [PATCH 5/6] Move SSL context options into CakeSocket. Having all the options consolidated in one places enables all the SSL context options to be used in the SmtpTransport instead of just allowing self_signed as proposed in #7496 --- lib/Cake/Network/CakeSocket.php | 41 ++++++++++++++++ lib/Cake/Network/Http/HttpSocket.php | 47 ++----------------- lib/Cake/Test/Case/Network/CakeSocketTest.php | 32 ++++++++++++- .../Test/Case/Network/Http/HttpSocketTest.php | 43 ++--------------- 4 files changed, 79 insertions(+), 84 deletions(-) diff --git a/lib/Cake/Network/CakeSocket.php b/lib/Cake/Network/CakeSocket.php index 2ad6832dc..396dfe93a 100644 --- a/lib/Cake/Network/CakeSocket.php +++ b/lib/Cake/Network/CakeSocket.php @@ -134,6 +134,7 @@ class CakeSocket { $scheme = $this->config['protocol'] . '://'; } + $this->_setSslContext($this->config['host']); if (!empty($this->config['context'])) { $context = stream_context_create($this->config['context']); } else { @@ -195,6 +196,46 @@ class CakeSocket { return $this->connected; } +/** + * Configure the SSL context options. + * + * @param string $host The host name being connected to. + */ + protected function _setSslContext($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 (version_compare(PHP_VERSION, '5.3.2', '>=')) { + if (!isset($this->config['context']['ssl']['SNI_enabled'])) { + $this->config['context']['ssl']['SNI_enabled'] = true; + } + if (version_compare(PHP_VERSION, '5.6.0', '>=')) { + if (empty($this->config['context']['ssl']['peer_name'])) { + $this->config['context']['ssl']['peer_name'] = $host; + } + } else { + if (empty($this->config['context']['ssl']['SNI_server_name'])) { + $this->config['context']['ssl']['SNI_server_name'] = $host; + } + } + } + if (empty($this->config['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']); + } + /** * socket_stream_client() does not populate errNum, or $errStr when there are * connection errors, as in the case of SSL verification failure. diff --git a/lib/Cake/Network/Http/HttpSocket.php b/lib/Cake/Network/Http/HttpSocket.php index 0fdf4a12d..4c6c25675 100644 --- a/lib/Cake/Network/Http/HttpSocket.php +++ b/lib/Cake/Network/Http/HttpSocket.php @@ -72,7 +72,7 @@ class HttpSocket extends CakeSocket { * Contain information about the last response (read only) * * @var array - */ +*/ public $response = null; /** @@ -361,8 +361,6 @@ class HttpSocket extends CakeSocket { return false; } - $this->_configContext($this->request['uri']['host']); - $this->request['raw'] = ''; if ($this->request['line'] !== false) { $this->request['raw'] = $this->request['line']; @@ -374,6 +372,8 @@ class HttpSocket extends CakeSocket { $this->request['raw'] .= "\r\n"; $this->request['raw'] .= $this->request['body']; + + // SSL context is set during the connect() method. $this->write($this->request['raw']); $response = null; @@ -700,47 +700,6 @@ class HttpSocket extends CakeSocket { 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 (version_compare(PHP_VERSION, '5.3.2', '>=')) { - if (!isset($this->config['context']['ssl']['SNI_enabled'])) { - $this->config['context']['ssl']['SNI_enabled'] = true; - } - if (version_compare(PHP_VERSION, '5.6.0', '>=')) { - if (empty($this->config['context']['ssl']['peer_name'])) { - $this->config['context']['ssl']['peer_name'] = $host; - } - } else { - if (empty($this->config['context']['ssl']['SNI_server_name'])) { - $this->config['context']['ssl']['SNI_server_name'] = $host; - } - } - } - if (empty($this->config['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 * diff --git a/lib/Cake/Test/Case/Network/CakeSocketTest.php b/lib/Cake/Test/Case/Network/CakeSocketTest.php index ff975564d..027f6a5ad 100644 --- a/lib/Cake/Test/Case/Network/CakeSocketTest.php +++ b/lib/Cake/Test/Case/Network/CakeSocketTest.php @@ -379,7 +379,37 @@ class CakeSocketTest extends CakeTestCase { $this->Socket = new CakeSocket($config); $this->Socket->connect(); $result = $this->Socket->context(); - $this->assertEquals($config['context'], $result); + $this->assertSame($config['context']['ssl']['capture_peer'], $result['ssl']['capture_peer']); } +/** + * test configuring the context from the flat keys. + * + * @return void + */ + public function testConfigContext() { + $this->skipIf(!extension_loaded('openssl'), 'OpenSSL is not enabled cannot test SSL.'); + $config = array( + 'host' => 'smtp.gmail.com', + 'port' => 465, + 'timeout' => 5, + 'ssl_verify_peer' => true, + 'ssl_allow_self_signed' => false, + 'ssl_verify_depth' => 5, + 'ssl_verify_host' => true, + ); + $this->Socket = new CakeSocket($config); + + $this->Socket->connect(); + $result = $this->Socket->context(); + + $this->assertTrue($result['ssl']['verify_peer']); + $this->assertFalse($result['ssl']['allow_self_signed']); + $this->assertEquals(5, $result['ssl']['verify_depth']); + $this->assertEquals('smtp.gmail.com', $result['ssl']['CN_match']); + $this->assertArrayNotHasKey('ssl_verify_peer', $this->Socket->config); + $this->assertArrayNotHasKey('ssl_allow_self_signed', $this->Socket->config); + $this->assertArrayNotHasKey('ssl_verify_host', $this->Socket->config); + $this->assertArrayNotHasKey('ssl_verify_depth', $this->Socket->config); + } } diff --git a/lib/Cake/Test/Case/Network/Http/HttpSocketTest.php b/lib/Cake/Test/Case/Network/Http/HttpSocketTest.php index 318e43401..559980494 100644 --- a/lib/Cake/Test/Case/Network/Http/HttpSocketTest.php +++ b/lib/Cake/Test/Case/Network/Http/HttpSocketTest.php @@ -314,23 +314,6 @@ class HttpSocketTest extends CakeTestCase { $response = $this->Socket->request(true); $this->assertFalse($response); - $context = array( - 'ssl' => array( - 'verify_peer' => true, - 'allow_self_signed' => false, - 'verify_depth' => 5, - 'SNI_enabled' => true, - 'CN_match' => 'www.cakephp.org', - 'cafile' => CAKE . 'Config' . DS . 'cacert.pem' - ) - ); - - if (version_compare(PHP_VERSION, '5.6.0', '>=')) { - $context['ssl']['peer_name'] = 'www.cakephp.org'; - } else { - $context['ssl']['SNI_server_name'] = 'www.cakephp.org'; - } - $tests = array( array( 'request' => 'http://www.cakephp.org/?foo=bar', @@ -341,7 +324,10 @@ class HttpSocketTest extends CakeTestCase { 'protocol' => 'tcp', 'port' => 80, 'timeout' => 30, - 'context' => $context, + 'ssl_verify_peer' => true, + 'ssl_allow_self_signed' => false, + 'ssl_verify_depth' => 5, + 'ssl_verify_host' => true, 'request' => array( 'uri' => array( 'scheme' => 'http', @@ -1843,27 +1829,6 @@ class HttpSocketTest extends CakeTestCase { $this->assertEquals(true, $return); } -/** - * test configuring the context from the flat keys. - * - * @return void - */ - public function testConfigContext() { - $this->Socket->expects($this->any()) - ->method('read')->will($this->returnValue(false)); - - $this->Socket->reset(); - $this->Socket->request('http://example.com'); - $this->assertTrue($this->Socket->config['context']['ssl']['verify_peer']); - $this->assertFalse($this->Socket->config['context']['ssl']['allow_self_signed']); - $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_allow_self_signed', $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. * From 3a4facbf8d8ef211f876ab5261737c47a182e8aa Mon Sep 17 00:00:00 2001 From: mark_story Date: Mon, 12 Oct 2015 21:58:24 -0400 Subject: [PATCH 6/6] Remove allowSelfSigned() method. This method is no longer needed as the low level socket understands the `ssl_*` options now. Refs #7496 --- lib/Cake/Network/CakeSocket.php | 12 ------------ lib/Cake/Network/Email/SmtpTransport.php | 3 --- lib/Cake/Test/Case/Network/CakeSocketTest.php | 12 ------------ 3 files changed, 27 deletions(-) diff --git a/lib/Cake/Network/CakeSocket.php b/lib/Cake/Network/CakeSocket.php index 396dfe93a..b08d60b0a 100644 --- a/lib/Cake/Network/CakeSocket.php +++ b/lib/Cake/Network/CakeSocket.php @@ -446,16 +446,4 @@ class CakeSocket { $this->setLastError(null, $errorMessage); throw new SocketException($errorMessage); } - -/** - * Accept Self-signed certificate on current stream socket - * - * @return bool True on success - * @link http://php.net/manual/en/context.ssl.php About the 'allow_self_signed' option. - * @see stream_context_set_option - */ - public function allowSelfSigned() { - return stream_context_set_option($this->connection, 'ssl', 'allow_self_signed', true); - } } - diff --git a/lib/Cake/Network/Email/SmtpTransport.php b/lib/Cake/Network/Email/SmtpTransport.php index 3f4144a65..f37dadf99 100644 --- a/lib/Cake/Network/Email/SmtpTransport.php +++ b/lib/Cake/Network/Email/SmtpTransport.php @@ -169,9 +169,6 @@ class SmtpTransport extends AbstractTransport { $this->_smtpSend("EHLO {$host}", '250'); if ($this->_config['tls']) { $this->_smtpSend("STARTTLS", '220'); - if ($this->_config['ssl_allow_self_signed']) { - $this->_socket->allowSelfSigned(); - } $this->_socket->enableCrypto('tls'); $this->_smtpSend("EHLO {$host}", '250'); } diff --git a/lib/Cake/Test/Case/Network/CakeSocketTest.php b/lib/Cake/Test/Case/Network/CakeSocketTest.php index 027f6a5ad..d68774fe0 100644 --- a/lib/Cake/Test/Case/Network/CakeSocketTest.php +++ b/lib/Cake/Test/Case/Network/CakeSocketTest.php @@ -349,18 +349,6 @@ class CakeSocketTest extends CakeTestCase { $this->assertTrue($this->Socket->encrypted); } -/** - * testEnableCryptoSelfSigned - * - * @return void - */ - public function testEnableCryptoSelfSigned() { - $this->_connectSocketToSslTls(); - $this->assertTrue($this->Socket->allowSelfSigned()); - $this->assertTrue($this->Socket->enableCrypto('tls', 'client')); - $this->Socket->disconnect(); - } - /** * test getting the context for a socket. *