From 80b2f75a47754b3bd207685abb3693254ac5b378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 10 Apr 2024 22:43:42 +0200 Subject: [PATCH 1/8] support passing ssl context for RedisCluster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: robin-brabants Co-authored-by: robin-brabants Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- src/RedisClusterOptions.php | 12 ++++++++++++ src/RedisClusterResourceManager.php | 12 ++++++++---- test/unit/RedisClusterOptionsTest.php | 4 ++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/RedisClusterOptions.php b/src/RedisClusterOptions.php index d403cc4..51ad85f 100644 --- a/src/RedisClusterOptions.php +++ b/src/RedisClusterOptions.php @@ -53,6 +53,8 @@ final class RedisClusterOptions extends AdapterOptions private string $password = ''; + private array $sslContext = []; + /** * @param iterable|null|AdapterOptions $options * @psalm-param iterable|null|AdapterOptions $options @@ -222,4 +224,14 @@ public function setPassword(string $password): void { $this->password = $password; } + + public function getSslContext(): array + { + return $this->sslContext; + } + + public function setSslContext(array $sslContext): void + { + $this->sslContext = $sslContext; + } } diff --git a/src/RedisClusterResourceManager.php b/src/RedisClusterResourceManager.php index 7ae5fd2..b169d32 100644 --- a/src/RedisClusterResourceManager.php +++ b/src/RedisClusterResourceManager.php @@ -81,7 +81,8 @@ private function createRedisResource(RedisClusterOptions $options): RedisCluster $options->getTimeout(), $options->getReadTimeout(), $options->isPersistent(), - $options->getPassword() + $options->getPassword(), + $options->getSslContext() ); } @@ -96,7 +97,8 @@ private function createRedisResource(RedisClusterOptions $options): RedisCluster $options->getTimeout(), $options->getReadTimeout(), $options->isPersistent(), - $password + $password, + $options->getSslContext() ); } @@ -108,7 +110,8 @@ private function createRedisResourceFromName( float $fallbackTimeout, float $fallbackReadTimeout, bool $persistent, - string $fallbackPassword + string $fallbackPassword, + array $sslContext ): RedisClusterFromExtension { $options = new RedisClusterOptionsFromIni(); $seeds = $options->getSeeds($name); @@ -122,7 +125,8 @@ private function createRedisResourceFromName( $timeout, $readTimeout, $persistent, - $password + $password, + $sslContext ); } diff --git a/test/unit/RedisClusterOptionsTest.php b/test/unit/RedisClusterOptionsTest.php index 18f7d3b..9a5c879 100644 --- a/test/unit/RedisClusterOptionsTest.php +++ b/test/unit/RedisClusterOptionsTest.php @@ -38,6 +38,7 @@ public function testCanHandleOptionsWithNodename(): void 'persistent' => false, 'redis_version' => '1.0', 'password' => 'secret', + 'ssl_context' => ['verify_peer' => false], ]); $this->assertEquals('foo', $options->getName()); @@ -46,6 +47,7 @@ public function testCanHandleOptionsWithNodename(): void $this->assertEquals(false, $options->isPersistent()); $this->assertEquals('1.0', $options->getRedisVersion()); $this->assertEquals('secret', $options->getPassword()); + $this->assertEquals(['verify_peer' => false], $options->getSslContext()); } public function testCanHandleOptionsWithSeeds(): void @@ -57,6 +59,7 @@ public function testCanHandleOptionsWithSeeds(): void 'persistent' => false, 'redis_version' => '1.0', 'password' => 'secret', + 'ssl_context' => ['verify_peer' => false], ]); $this->assertEquals(['localhost:1234'], $options->getSeeds()); @@ -65,6 +68,7 @@ public function testCanHandleOptionsWithSeeds(): void $this->assertEquals(false, $options->isPersistent()); $this->assertEquals('1.0', $options->getRedisVersion()); $this->assertEquals('secret', $options->getPassword()); + $this->assertEquals(['verify_peer' => false], $options->getSslContext()); } public function testWillDetectSeedsAndNodenameConfiguration(): void From c72f32d906520e2e348c4b670634f5e888732c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 10 Apr 2024 22:44:00 +0200 Subject: [PATCH 2/8] raise minimum required version for the phpredis extension since the ssl/tls context parameter was introduced in version 5.3.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: robin-brabants Co-authored-by: robin-brabants Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- composer.json | 2 +- composer.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 9844849..d6da292 100644 --- a/composer.json +++ b/composer.json @@ -8,7 +8,7 @@ "license": "BSD-3-Clause", "require": { "php": "~8.1.0 || ~8.2.0 || ~8.3.0", - "ext-redis": "^5.0.2 || ^6.0", + "ext-redis": "^5.3.2 || ^6.0", "laminas/laminas-cache": "^3.10" }, "provide": { diff --git a/composer.lock b/composer.lock index dcfae77..c3afaab 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "18eabbd1db8bc1ba904fad53c3ef3071", + "content-hash": "2bad8f0b0911d3df5ac703cd0701dd45", "packages": [ { "name": "laminas/laminas-cache", @@ -5554,7 +5554,7 @@ "prefer-lowest": false, "platform": { "php": "~8.1.0 || ~8.2.0 || ~8.3.0", - "ext-redis": "^5.0.2 || ^6.0" + "ext-redis": "^5.3.2 || ^6.0" }, "platform-dev": [], "platform-overrides": { From 8151e2afa47bfb800ff65b961b843045b40f9585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 10 Apr 2024 22:44:03 +0200 Subject: [PATCH 3/8] improve type documentation of the redis cluster ssl context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: robin-brabants Co-authored-by: robin-brabants Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- src/RedisClusterOptions.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/RedisClusterOptions.php b/src/RedisClusterOptions.php index 51ad85f..335efd4 100644 --- a/src/RedisClusterOptions.php +++ b/src/RedisClusterOptions.php @@ -53,6 +53,7 @@ final class RedisClusterOptions extends AdapterOptions private string $password = ''; + /** @psalm-var array */ private array $sslContext = []; /** @@ -225,6 +226,9 @@ public function setPassword(string $password): void $this->password = $password; } + /** + * @psalm-return array + */ public function getSslContext(): array { return $this->sslContext; From 40aa037af3f55eba2346b76c13032ac9a1e92be6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 10 Apr 2024 22:44:07 +0200 Subject: [PATCH 4/8] set the redis cluster ssl context option default to null, otherwise a TLS connection is attempted by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: robin-brabants Co-authored-by: robin-brabants Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- src/RedisClusterOptions.php | 13 ++++++++----- src/RedisClusterResourceManager.php | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/RedisClusterOptions.php b/src/RedisClusterOptions.php index 335efd4..ce6ef66 100644 --- a/src/RedisClusterOptions.php +++ b/src/RedisClusterOptions.php @@ -53,8 +53,8 @@ final class RedisClusterOptions extends AdapterOptions private string $password = ''; - /** @psalm-var array */ - private array $sslContext = []; + /** @psalm-var array|null */ + private ?array $sslContext = null; /** * @param iterable|null|AdapterOptions $options @@ -227,14 +227,17 @@ public function setPassword(string $password): void } /** - * @psalm-return array + * @psalm-return array|null */ - public function getSslContext(): array + public function getSslContext(): ?array { return $this->sslContext; } - public function setSslContext(array $sslContext): void + /** + * @psalm-param array|null $sslContext + */ + public function setSslContext(?array $sslContext): void { $this->sslContext = $sslContext; } diff --git a/src/RedisClusterResourceManager.php b/src/RedisClusterResourceManager.php index b169d32..ef7ca50 100644 --- a/src/RedisClusterResourceManager.php +++ b/src/RedisClusterResourceManager.php @@ -111,7 +111,7 @@ private function createRedisResourceFromName( float $fallbackReadTimeout, bool $persistent, string $fallbackPassword, - array $sslContext + ?array $sslContext ): RedisClusterFromExtension { $options = new RedisClusterOptionsFromIni(); $seeds = $options->getSeeds($name); From 468a6b592dd223ff6c4cb1f776adac638416f52c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 10 Apr 2024 22:44:10 +0200 Subject: [PATCH 5/8] suppress psalm validation errors for the setter setSslContext and the phpredis RedisCluster class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: robin-brabants Co-authored-by: robin-brabants Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- psalm-baseline.xml | 1 + psalm.xml | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index e9fb47a..346dd6c 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -139,6 +139,7 @@ setReadTimeout setSeeds setTimeout + setSslContext diff --git a/psalm.xml b/psalm.xml index dfc0477..d34a358 100644 --- a/psalm.xml +++ b/psalm.xml @@ -50,6 +50,15 @@ + + + + + + From a195a52ebb0df3d4073fbe86b291a07538b71e9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 10 Apr 2024 22:44:12 +0200 Subject: [PATCH 6/8] implement an sslContext class which can be used for passing the sslContext to the RedisCluster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: robin-brabants Co-authored-by: robin-brabants Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- src/RedisClusterOptions.php | 19 ++- src/RedisClusterResourceManager.php | 6 +- src/SslContext.php | 200 ++++++++++++++++++++++++++ test/unit/RedisClusterOptionsTest.php | 27 +++- test/unit/SslContextTest.php | 83 +++++++++++ 5 files changed, 322 insertions(+), 13 deletions(-) create mode 100644 src/SslContext.php create mode 100644 test/unit/SslContextTest.php diff --git a/src/RedisClusterOptions.php b/src/RedisClusterOptions.php index ce6ef66..66e9c5c 100644 --- a/src/RedisClusterOptions.php +++ b/src/RedisClusterOptions.php @@ -7,6 +7,8 @@ use Laminas\Cache\Exception\RuntimeException; use Laminas\Cache\Storage\Adapter\Exception\InvalidRedisClusterConfigurationException; +use function is_array; + final class RedisClusterOptions extends AdapterOptions { public const LIBRARY_OPTIONS = [ @@ -53,8 +55,8 @@ final class RedisClusterOptions extends AdapterOptions private string $password = ''; - /** @psalm-var array|null */ - private ?array $sslContext = null; + /** @psalm-var SslContext|null */ + private ?SslContext $sslContext = null; /** * @param iterable|null|AdapterOptions $options @@ -227,18 +229,23 @@ public function setPassword(string $password): void } /** - * @psalm-return array|null + * @psalm-return SslContext|null */ - public function getSslContext(): ?array + public function getSslContext(): ?SslContext { return $this->sslContext; } /** - * @psalm-param array|null $sslContext + * @psalm-param array|SslContext|null $sslContext */ - public function setSslContext(?array $sslContext): void + public function setSslContext(array|SslContext|null $sslContext): void { + if (is_array($sslContext)) { + $data = $sslContext; + $sslContext = new SslContext(); + $sslContext->exchangeArray($data); + } $this->sslContext = $sslContext; } } diff --git a/src/RedisClusterResourceManager.php b/src/RedisClusterResourceManager.php index ef7ca50..6691cae 100644 --- a/src/RedisClusterResourceManager.php +++ b/src/RedisClusterResourceManager.php @@ -98,7 +98,7 @@ private function createRedisResource(RedisClusterOptions $options): RedisCluster $options->getReadTimeout(), $options->isPersistent(), $password, - $options->getSslContext() + $options->getSslContext()?->getArrayCopy() ); } @@ -111,7 +111,7 @@ private function createRedisResourceFromName( float $fallbackReadTimeout, bool $persistent, string $fallbackPassword, - ?array $sslContext + ?SslContext $sslContext ): RedisClusterFromExtension { $options = new RedisClusterOptionsFromIni(); $seeds = $options->getSeeds($name); @@ -126,7 +126,7 @@ private function createRedisResourceFromName( $readTimeout, $persistent, $password, - $sslContext + $sslContext?->getArrayCopy() ); } diff --git a/src/SslContext.php b/src/SslContext.php new file mode 100644 index 0000000..059e15c --- /dev/null +++ b/src/SslContext.php @@ -0,0 +1,200 @@ +peerName = $peerName; + $this->verifyPeer = $verifyPeer; + $this->verifyPeerName = $verifyPeerName; + $this->allowSelfSigned = $allowSelfSigned; + $this->cafile = $cafile; + $this->capath = $capath; + $this->localCert = $localCert; + $this->localPk = $localPk; + $this->passphrase = $passphrase; + $this->verifyDepth = $verifyDepth; + $this->ciphers = $ciphers; + $this->sniEnabled = $sniEnabled; + $this->disableCompression = $disableCompression; + $this->peerFingerprint = $peerFingerprint; + $this->securityLevel = $securityLevel; + if ($sniEnabled === null) { + $this->sniEnabled = boolval(OPENSSL_TLSEXT_SERVER_NAME); + } + } + + public function exchangeArray(array $array): void + { + foreach ($array as $key => $value) { + $property = $this->mapArrayKeyToPropertyName($key); + if (! property_exists($this, $property)) { + throw new InvalidArgumentException( + sprintf( + '%s does not contain the property "%s" corresponding to the array key "%s"', + self::class, + $property, + $key + ) + ); + } + $this->$property = $value; + } + } + + public function getArrayCopy(): array + { + $array = []; + foreach (get_object_vars($this) as $property => $value) { + if ($value !== null) { + $key = $this->mapPropertyNameToArrayKey($property); + $array[$key] = $value; + } + } + return $array; + } + + private function mapArrayKeyToPropertyName(string $key): string + { + if ($key === 'SNI_enabled') { + return 'sniEnabled'; + } + return (new CamelCaseToSnakeCaseNameConverter())->denormalize($key); + } + + private function mapPropertyNameToArrayKey(string $property): string + { + if ($property === 'sniEnabled') { + return 'SNI_enabled'; + } + return (new CamelCaseToSnakeCaseNameConverter())->normalize($property); + } +} diff --git a/test/unit/RedisClusterOptionsTest.php b/test/unit/RedisClusterOptionsTest.php index 9a5c879..fb8c2a7 100644 --- a/test/unit/RedisClusterOptionsTest.php +++ b/test/unit/RedisClusterOptionsTest.php @@ -9,6 +9,7 @@ use Laminas\Cache\Storage\Adapter\AdapterOptions; use Laminas\Cache\Storage\Adapter\Exception\InvalidRedisClusterConfigurationException; use Laminas\Cache\Storage\Adapter\RedisClusterOptions; +use Laminas\Cache\Storage\Adapter\SslContext; use Redis as RedisFromExtension; use ReflectionClass; @@ -29,6 +30,28 @@ protected function createAdapterOptions(): AdapterOptions return new RedisClusterOptions(['seeds' => ['localhost']]); } + public function testCanHandleOptionsWithSslContextObject(): void + { + $options = new RedisClusterOptions([ + 'name' => 'foo', + 'ssl_context' => new SslContext(localCert: '/path/to/localcert'), + ]); + + $this->assertEquals('foo', $options->getName()); + $this->assertEquals(new SslContext(localCert: '/path/to/localcert'), $options->getSslContext()); + } + + public function testCanHandleOptionsWithSslContextArray(): void + { + $options = new RedisClusterOptions([ + 'name' => 'foo', + 'ssl_context' => ['local_cert' => '/path/to/localcert'], + ]); + + $this->assertEquals('foo', $options->getName()); + $this->assertEquals(new SslContext(localCert: '/path/to/localcert'), $options->getSslContext()); + } + public function testCanHandleOptionsWithNodename(): void { $options = new RedisClusterOptions([ @@ -38,7 +61,6 @@ public function testCanHandleOptionsWithNodename(): void 'persistent' => false, 'redis_version' => '1.0', 'password' => 'secret', - 'ssl_context' => ['verify_peer' => false], ]); $this->assertEquals('foo', $options->getName()); @@ -47,7 +69,6 @@ public function testCanHandleOptionsWithNodename(): void $this->assertEquals(false, $options->isPersistent()); $this->assertEquals('1.0', $options->getRedisVersion()); $this->assertEquals('secret', $options->getPassword()); - $this->assertEquals(['verify_peer' => false], $options->getSslContext()); } public function testCanHandleOptionsWithSeeds(): void @@ -59,7 +80,6 @@ public function testCanHandleOptionsWithSeeds(): void 'persistent' => false, 'redis_version' => '1.0', 'password' => 'secret', - 'ssl_context' => ['verify_peer' => false], ]); $this->assertEquals(['localhost:1234'], $options->getSeeds()); @@ -68,7 +88,6 @@ public function testCanHandleOptionsWithSeeds(): void $this->assertEquals(false, $options->isPersistent()); $this->assertEquals('1.0', $options->getRedisVersion()); $this->assertEquals('secret', $options->getPassword()); - $this->assertEquals(['verify_peer' => false], $options->getSslContext()); } public function testWillDetectSeedsAndNodenameConfiguration(): void diff --git a/test/unit/SslContextTest.php b/test/unit/SslContextTest.php new file mode 100644 index 0000000..c75f7a0 --- /dev/null +++ b/test/unit/SslContextTest.php @@ -0,0 +1,83 @@ +correspondingSslContextObject = new SslContext( + peerName: 'some peer name', + allowSelfSigned: true, + verifyDepth: 10, + peerFingerprint: ['md5' => 'some fingerprint'] + ); + + $this->correspondingSslContextArray = [ + 'peer_name' => 'some peer name', + 'verify_peer' => true, + 'verify_peer_name' => true, + 'allow_self_signed' => true, + 'verify_depth' => 10, + 'ciphers' => OPENSSL_DEFAULT_STREAM_CIPHERS, + 'SNI_enabled' => boolval(OPENSSL_TLSEXT_SERVER_NAME), + 'disable_compression' => true, + 'peer_fingerprint' => ['md5' => 'some fingerprint'], + ]; + } + + public function testExchangeArraySetsPropertiesCorrectly(): void + { + $sslContextObject = new SslContext(); + $sslContextObject->exchangeArray($this->correspondingSslContextArray); + + $this->assertEquals( + $this->correspondingSslContextObject, + $sslContextObject + ); + } + + public function testGetArrayCopyReturnsAnArrayWithPropertyValues() + { + $sslContextArray = $this->correspondingSslContextObject->getArrayCopy(); + + $this->assertEquals($this->correspondingSslContextArray, $sslContextArray); + } + + public function testExchangeArrayThrowsExceptionWhenProvidingInvalidKeyName(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessageMatches( + '/does not contain the property "someInvalidKey" corresponding to the array key "some_invalid_key"/' + ); + + $sslContextObject = new SslContext(); + $sslContextObject->exchangeArray(['some_invalid_key' => true]); + } + + public function testExchangeArrayThrowsExceptionWhenProvidingInvalidValueType(): void + { + $this->expectException(TypeError::class); + $this->expectExceptionMessageMatches( + '/\$verifyPeer of type bool/' + ); + + $sslContextObject = new SslContext(); + $sslContextObject->exchangeArray(['verify_peer' => 'invalid type']); + } +} From da3bd318ff30a1d5476890688209455f7280952a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 10 Apr 2024 22:44:16 +0200 Subject: [PATCH 7/8] move psalm validation error suppression for the RedisCluster constructor to the code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: robin-brabants Co-authored-by: robin-brabants Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- psalm.xml | 9 --------- src/RedisClusterResourceManager.php | 12 ++++++++++++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/psalm.xml b/psalm.xml index d34a358..dfc0477 100644 --- a/psalm.xml +++ b/psalm.xml @@ -50,15 +50,6 @@ - - - - - - diff --git a/src/RedisClusterResourceManager.php b/src/RedisClusterResourceManager.php index 6691cae..382a498 100644 --- a/src/RedisClusterResourceManager.php +++ b/src/RedisClusterResourceManager.php @@ -91,6 +91,12 @@ private function createRedisResource(RedisClusterOptions $options): RedisCluster $password = null; } + /** + * Psalm currently (<= 5.23.1) uses an outdated (phpredis < 5.3.2) constructor signature for the RedisCluster + * class in the phpredis extension. + * + * @psalm-suppress TooManyArguments https://github.com/vimeo/psalm/pull/10862 + */ return new RedisClusterFromExtension( null, $options->getSeeds(), @@ -119,6 +125,12 @@ private function createRedisResourceFromName( $readTimeout = $options->getReadTimeout($name, $fallbackReadTimeout); $password = $options->getPasswordByName($name, $fallbackPassword); + /** + * Psalm currently (<= 5.23.1) uses an outdated (phpredis < 5.3.2) constructor signature for the RedisCluster + * class in the phpredis extension. + * + * @psalm-suppress TooManyArguments https://github.com/vimeo/psalm/pull/10862 + */ return new RedisClusterFromExtension( null, $seeds, From d387d4224419c668cdceeffaf4a198b3d298abd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20B=C3=B6sing?= <2189546+boesing@users.noreply.github.com> Date: Wed, 10 Apr 2024 22:44:19 +0200 Subject: [PATCH 8/8] refactor: `SslContext` and its instantiation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This refactors some of the `SslContext` implementation to have better readability, less complexity (regarding camel case handling, etc.) and explicit context serialization. Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com> --- src/RedisClusterOptions.php | 42 +++- src/RedisClusterResourceManager.php | 4 +- src/SslContext.php | 336 ++++++++++++++------------ test/unit/RedisClusterOptionsTest.php | 14 +- test/unit/SslContextTest.php | 109 ++++----- 5 files changed, 267 insertions(+), 238 deletions(-) diff --git a/src/RedisClusterOptions.php b/src/RedisClusterOptions.php index 66e9c5c..79bbaee 100644 --- a/src/RedisClusterOptions.php +++ b/src/RedisClusterOptions.php @@ -6,8 +6,11 @@ use Laminas\Cache\Exception\RuntimeException; use Laminas\Cache\Storage\Adapter\Exception\InvalidRedisClusterConfigurationException; +use Laminas\Stdlib\AbstractOptions; +use Traversable; use function is_array; +use function iterator_to_array; final class RedisClusterOptions extends AdapterOptions { @@ -55,7 +58,6 @@ final class RedisClusterOptions extends AdapterOptions private string $password = ''; - /** @psalm-var SslContext|null */ private ?SslContext $sslContext = null; /** @@ -81,6 +83,31 @@ public function __construct($options = null) } } + /** + * {@inheritDoc} + */ + public function setFromArray($options) + { + if ($options instanceof AbstractOptions) { + $options = $options->toArray(); + } elseif ($options instanceof Traversable) { + $options = iterator_to_array($options); + } + + $sslContext = $options['sslContext'] ?? $options['ssl_context'] ?? null; + unset($options['sslContext'], $options['ssl_context']); + if (is_array($sslContext)) { + /** @psalm-suppress MixedArgumentTypeCoercion Trust upstream that they verify the array beforehand. */ + $sslContext = SslContext::fromSslContextArray($sslContext); + } + + if ($sslContext instanceof SslContext) { + $options['ssl_context'] = $sslContext; + } + + return parent::setFromArray($options); + } + public function setTimeout(float $timeout): void { $this->timeout = $timeout; @@ -228,24 +255,13 @@ public function setPassword(string $password): void $this->password = $password; } - /** - * @psalm-return SslContext|null - */ public function getSslContext(): ?SslContext { return $this->sslContext; } - /** - * @psalm-param array|SslContext|null $sslContext - */ - public function setSslContext(array|SslContext|null $sslContext): void + public function setSslContext(SslContext|null $sslContext): void { - if (is_array($sslContext)) { - $data = $sslContext; - $sslContext = new SslContext(); - $sslContext->exchangeArray($data); - } $this->sslContext = $sslContext; } } diff --git a/src/RedisClusterResourceManager.php b/src/RedisClusterResourceManager.php index 382a498..a9609f4 100644 --- a/src/RedisClusterResourceManager.php +++ b/src/RedisClusterResourceManager.php @@ -104,7 +104,7 @@ private function createRedisResource(RedisClusterOptions $options): RedisCluster $options->getReadTimeout(), $options->isPersistent(), $password, - $options->getSslContext()?->getArrayCopy() + $options->getSslContext()?->toSslContextArray() ); } @@ -138,7 +138,7 @@ private function createRedisResourceFromName( $readTimeout, $persistent, $password, - $sslContext?->getArrayCopy() + $sslContext?->toSslContextArray() ); } diff --git a/src/SslContext.php b/src/SslContext.php index 059e15c..d9209a2 100644 --- a/src/SslContext.php +++ b/src/SslContext.php @@ -4,197 +4,217 @@ namespace Laminas\Cache\Storage\Adapter; -use InvalidArgumentException; -use Laminas\Stdlib\ArraySerializableInterface; -use Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter; - -use function boolval; -use function get_object_vars; -use function property_exists; -use function sprintf; - -use const OPENSSL_DEFAULT_STREAM_CIPHERS; -use const OPENSSL_TLSEXT_SERVER_NAME; +use SensitiveParameter; /** * Class containing the SSL context options in its fields. * * @link https://www.php.net/manual/en/context.ssl.php + * + * @psalm-type SSLContextArrayShape = array{ + * peer_name?: non-empty-string, + * verify_peer?: bool, + * verify_peer_name?: bool, + * allow_self_signed?: bool, + * cafile?: non-empty-string, + * capath?: non-empty-string, + * local_cert?: non-empty-string, + * local_pk?: non-empty-string, + * passphrase?: non-empty-string, + * verify_depth?: non-negative-int, + * ciphers?: non-empty-string, + * SNI_enabled?: bool, + * disable_compression?: bool, + * peer_fingerprint?: non-empty-string|array, + * security_level?: non-negative-int + * } */ -final class SslContext implements ArraySerializableInterface +final class SslContext { /** - * Peer name to be used. - * If this value is not set, then the name is guessed based on the hostname used when opening the stream. - */ - private ?string $peerName; - - /** - * Require verification of SSL certificate used. + * @param non-empty-string|null $expectedPeerName + * @param non-empty-string|null $certificateAuthorityFile + * @param non-empty-string|null $certificateAuthorityPath + * @param non-empty-string|null $localCertificatePath + * @param non-empty-string|null $localPrivateKeyPath + * @param non-empty-string|null $passphrase + * @param non-negative-int|null $verifyDepth + * @param non-empty-string|null $ciphers + * @param non-empty-string|array|null $peerFingerprint + * @param non-negative-int|null $securityLevel */ - private bool $verifyPeer; - - /** - * Require verification of peer name. - */ - private bool $verifyPeerName; - - /** - * Allow self-signed certificates. Requires verifyPeer. - */ - private bool $allowSelfSigned; + public function __construct( + /** + * Peer name to be used. + * If this value is not set, then the name is guessed based on the hostname used when opening the stream. + */ + public readonly ?string $expectedPeerName = null, + /** + * Require verification of SSL certificate used. + */ + public readonly ?bool $verifyPeer = null, + /** + * Require verification of peer name. + */ + public readonly ?bool $verifyPeerName = null, + /** + * Allow self-signed certificates. Requires verifyPeer. + */ + public readonly ?bool $allowSelfSignedCertificates = null, + /** + * Location of Certificate Authority file on local filesystem which should be used with the verifyPeer + * context option to authenticate the identity of the remote peer. + */ + public readonly ?string $certificateAuthorityFile = null, + /** + * If cafile is not specified or if the certificate is not found there, the directory pointed to by capath is + * searched for a suitable certificate. capath must be a correctly hashed certificate directory. + */ + public readonly ?string $certificateAuthorityPath = null, + /** + * Path to local certificate file on filesystem. It must be a PEM encoded file which contains your certificate + * and private key. It can optionally contain the certificate chain of issuers. + * The private key also may be contained in a separate file specified by localPk. + */ + public readonly ?string $localCertificatePath = null, + /** + * Path to local private key file on filesystem in case of separate files for certificate (localCert) + * and private key. + */ + public readonly ?string $localPrivateKeyPath = null, + /** + * Passphrase with which your localCert file was encoded. + */ + #[SensitiveParameter] + public readonly ?string $passphrase = null, + /** + * Abort if the certificate chain is too deep. + * If not set, defaults to no verification. + */ + public readonly ?int $verifyDepth = null, + /** + * Sets the list of available ciphers. The format of the string is described in + * https://www.openssl.org/docs/manmaster/man1/ciphers.html#CIPHER-LIST-FORMAT + */ + public readonly ?string $ciphers = null, + /** + * If set to true server name indication will be enabled. Enabling SNI allows multiple certificates on the same + * IP address. + * If not set, will automatically be enabled if SNI support is available. + */ + public readonly ?bool $serverNameIndicationEnabled = null, + /** + * If set, disable TLS compression. This can help mitigate the CRIME attack vector. + */ + public readonly ?bool $disableCompression = null, + /** + * Aborts when the remote certificate digest doesn't match the specified hash. + * + * When a string is used, the length will determine which hashing algorithm is applied, + * either "md5" (32) or "sha1" (40). + * + * When an array is used, the keys indicate the hashing algorithm name and each corresponding + * value is the expected digest. + */ + public readonly array|string|null $peerFingerprint = null, + /** + * Sets the security level. If not specified the library default security level is used. The security levels are + * described in https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_get_security_level.html. + */ + public readonly ?int $securityLevel = null, + ) { + } /** - * Location of Certificate Authority file on local filesystem which should be used with the verifyPeer - * context option to authenticate the identity of the remote peer. + * @param SSLContextArrayShape $context */ - private ?string $cafile; + public static function fromSslContextArray(array $context): self + { + return new self( + $context['peer_name'] ?? null, + $context['verify_peer'] ?? null, + $context['verify_peer_name'] ?? null, + $context['allow_self_signed'] ?? null, + $context['cafile'] ?? null, + $context['capath'] ?? null, + $context['local_cert'] ?? null, + $context['local_pk'] ?? null, + $context['passphrase'] ?? null, + $context['verify_depth'] ?? null, + $context['ciphers'] ?? null, + $context['SNI_enabled'] ?? null, + $context['disable_compression'] ?? null, + $context['peer_fingerprint'] ?? null, + $context['security_level'] ?? null, + ); + } /** - * If cafile is not specified or if the certificate is not found there, the directory pointed to by capath is - * searched for a suitable certificate. capath must be a correctly hashed certificate directory. + * @return SSLContextArrayShape */ - private ?string $capath; + public function toSslContextArray(): array + { + $context = []; + if ($this->expectedPeerName !== null) { + $context['peer_name'] = $this->expectedPeerName; + } - /** - * Path to local certificate file on filesystem. It must be a PEM encoded file which contains your certificate and - * private key. It can optionally contain the certificate chain of issuers. - * The private key also may be contained in a separate file specified by localPk. - */ - private ?string $localCert; + if ($this->verifyPeer !== null) { + $context['verify_peer'] = $this->verifyPeer; + } - /** - * Path to local private key file on filesystem in case of separate files for certificate (localCert) - * and private key. - */ - private ?string $localPk; + if ($this->verifyPeerName !== null) { + $context['verify_peer_name'] = $this->verifyPeerName; + } - /** - * Passphrase with which your localCert file was encoded. - */ - private ?string $passphrase; + if ($this->allowSelfSignedCertificates !== null) { + $context['allow_self_signed'] = $this->allowSelfSignedCertificates; + } - /** - * Abort if the certificate chain is too deep. - * If not set, defaults to no verification. - */ - private ?int $verifyDepth; + if ($this->certificateAuthorityFile !== null) { + $context['cafile'] = $this->certificateAuthorityFile; + } - /** - * Sets the list of available ciphers. The format of the string is described in - * https://www.openssl.org/docs/manmaster/man1/ciphers.html#CIPHER-LIST-FORMAT - */ - private string $ciphers; + if ($this->certificateAuthorityPath !== null) { + $context['capath'] = $this->certificateAuthorityPath; + } - /** - * If set to true server name indication will be enabled. Enabling SNI allows multiple certificates on the same - * IP address. - * If not set, will automatically be enabled if SNI support is available. - */ - private ?bool $sniEnabled; + if ($this->localCertificatePath !== null) { + $context['local_cert'] = $this->localCertificatePath; + } - /** - * If set, disable TLS compression. This can help mitigate the CRIME attack vector. - */ - private bool $disableCompression; + if ($this->localPrivateKeyPath !== null) { + $context['local_pk'] = $this->localPrivateKeyPath; + } - /** - * Aborts when the remote certificate digest doesn't match the specified hash. - * - * When a string is used, the length will determine which hashing algorithm is applied, - * either "md5" (32) or "sha1" (40). - * - * When an array is used, the keys indicate the hashing algorithm name and each corresponding - * value is the expected digest. - */ - private string|array|null $peerFingerprint; + if ($this->passphrase !== null) { + $context['passphrase'] = $this->passphrase; + } - /** - * Sets the security level. If not specified the library default security level is used. The security levels are - * described in https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_get_security_level.html. - */ - private ?int $securityLevel; + if ($this->verifyDepth !== null) { + $context['verify_depth'] = $this->verifyDepth; + } - public function __construct( - ?string $peerName = null, - bool $verifyPeer = true, - bool $verifyPeerName = true, - bool $allowSelfSigned = false, - ?string $cafile = null, - ?string $capath = null, - ?string $localCert = null, - ?string $localPk = null, - ?string $passphrase = null, - ?int $verifyDepth = null, - string $ciphers = OPENSSL_DEFAULT_STREAM_CIPHERS, - ?bool $sniEnabled = null, - bool $disableCompression = true, - array|string|null $peerFingerprint = null, - ?int $securityLevel = null - ) { - $this->peerName = $peerName; - $this->verifyPeer = $verifyPeer; - $this->verifyPeerName = $verifyPeerName; - $this->allowSelfSigned = $allowSelfSigned; - $this->cafile = $cafile; - $this->capath = $capath; - $this->localCert = $localCert; - $this->localPk = $localPk; - $this->passphrase = $passphrase; - $this->verifyDepth = $verifyDepth; - $this->ciphers = $ciphers; - $this->sniEnabled = $sniEnabled; - $this->disableCompression = $disableCompression; - $this->peerFingerprint = $peerFingerprint; - $this->securityLevel = $securityLevel; - if ($sniEnabled === null) { - $this->sniEnabled = boolval(OPENSSL_TLSEXT_SERVER_NAME); + if ($this->ciphers !== null) { + $context['ciphers'] = $this->ciphers; } - } - public function exchangeArray(array $array): void - { - foreach ($array as $key => $value) { - $property = $this->mapArrayKeyToPropertyName($key); - if (! property_exists($this, $property)) { - throw new InvalidArgumentException( - sprintf( - '%s does not contain the property "%s" corresponding to the array key "%s"', - self::class, - $property, - $key - ) - ); - } - $this->$property = $value; + if ($this->serverNameIndicationEnabled !== null) { + $context['SNI_enabled'] = $this->serverNameIndicationEnabled; } - } - public function getArrayCopy(): array - { - $array = []; - foreach (get_object_vars($this) as $property => $value) { - if ($value !== null) { - $key = $this->mapPropertyNameToArrayKey($property); - $array[$key] = $value; - } + if ($this->disableCompression !== null) { + $context['disable_compression'] = $this->disableCompression; } - return $array; - } - private function mapArrayKeyToPropertyName(string $key): string - { - if ($key === 'SNI_enabled') { - return 'sniEnabled'; + if ($this->peerFingerprint !== null) { + $context['peer_fingerprint'] = $this->peerFingerprint; } - return (new CamelCaseToSnakeCaseNameConverter())->denormalize($key); - } - private function mapPropertyNameToArrayKey(string $property): string - { - if ($property === 'sniEnabled') { - return 'SNI_enabled'; + if ($this->securityLevel !== null) { + $context['security_level'] = $this->securityLevel; } - return (new CamelCaseToSnakeCaseNameConverter())->normalize($property); + + return $context; } } diff --git a/test/unit/RedisClusterOptionsTest.php b/test/unit/RedisClusterOptionsTest.php index fb8c2a7..93ae56e 100644 --- a/test/unit/RedisClusterOptionsTest.php +++ b/test/unit/RedisClusterOptionsTest.php @@ -34,11 +34,13 @@ public function testCanHandleOptionsWithSslContextObject(): void { $options = new RedisClusterOptions([ 'name' => 'foo', - 'ssl_context' => new SslContext(localCert: '/path/to/localcert'), + 'ssl_context' => new SslContext(localCertificatePath: '/path/to/localcert'), ]); - $this->assertEquals('foo', $options->getName()); - $this->assertEquals(new SslContext(localCert: '/path/to/localcert'), $options->getSslContext()); + self::assertEquals('foo', $options->getName()); + $sslContext = $options->getSslContext(); + self::assertNotNull($sslContext); + self::assertSame('/path/to/localcert', $sslContext->localCertificatePath); } public function testCanHandleOptionsWithSslContextArray(): void @@ -48,8 +50,10 @@ public function testCanHandleOptionsWithSslContextArray(): void 'ssl_context' => ['local_cert' => '/path/to/localcert'], ]); - $this->assertEquals('foo', $options->getName()); - $this->assertEquals(new SslContext(localCert: '/path/to/localcert'), $options->getSslContext()); + self::assertEquals('foo', $options->getName()); + $sslContext = $options->getSslContext(); + self::assertNotNull($sslContext); + self::assertSame('/path/to/localcert', $sslContext->localCertificatePath); } public function testCanHandleOptionsWithNodename(): void diff --git a/test/unit/SslContextTest.php b/test/unit/SslContextTest.php index c75f7a0..bd6ed42 100644 --- a/test/unit/SslContextTest.php +++ b/test/unit/SslContextTest.php @@ -4,80 +4,69 @@ namespace LaminasTest\Cache\Storage\Adapter; -use InvalidArgumentException; use Laminas\Cache\Storage\Adapter\SslContext; use PHPUnit\Framework\TestCase; use TypeError; -use function boolval; - -use const OPENSSL_DEFAULT_STREAM_CIPHERS; -use const OPENSSL_TLSEXT_SERVER_NAME; - final class SslContextTest extends TestCase { - private SslContext $correspondingSslContextObject; - private array $correspondingSslContextArray; - - protected function setUp(): void + private const SSL_CONTEXT = [ + 'peer_name' => 'some peer name', + 'verify_peer' => true, + 'verify_peer_name' => true, + 'allow_self_signed' => true, + 'cafile' => '/some/path/to/cafile.pem', + 'capath' => '/some/path/to/ca', + 'local_cert' => '/some/path/to/local.certificate.pem', + 'local_pk' => '/some/path/to/local.key', + 'passphrase' => 'secret', + 'verify_depth' => 10, + 'ciphers' => 'ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256:" . +"ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:" . +"DHE-DSS-AES128-GCM-SHA256:kEDH+AESGCM:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:" . +"ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384:" . +"ECDHE-RSA-AES256-SHA:ECDHE-ECDSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:" . +"DHE-DSS-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA:DHE-RSA-AES256-SHA:AES128-GCM-SHA256:" . +"AES256-GCM-SHA384:AES128:AES256:HIGH:!SSLv2:!aNULL:!eNULL:!EXPORT:!DES:!MD5:!RC4:!ADH', + 'SNI_enabled' => true, + 'disable_compression' => true, + 'peer_fingerprint' => ['md5' => 'some fingerprint'], + 'security_level' => 5, + ]; + + public function testWillNotGenerateContextIfNoneProvided(): void { - $this->correspondingSslContextObject = new SslContext( - peerName: 'some peer name', - allowSelfSigned: true, - verifyDepth: 10, - peerFingerprint: ['md5' => 'some fingerprint'] - ); - - $this->correspondingSslContextArray = [ - 'peer_name' => 'some peer name', - 'verify_peer' => true, - 'verify_peer_name' => true, - 'allow_self_signed' => true, - 'verify_depth' => 10, - 'ciphers' => OPENSSL_DEFAULT_STREAM_CIPHERS, - 'SNI_enabled' => boolval(OPENSSL_TLSEXT_SERVER_NAME), - 'disable_compression' => true, - 'peer_fingerprint' => ['md5' => 'some fingerprint'], - ]; + $context = new SslContext(); + self::assertSame([], $context->toSslContextArray()); } - public function testExchangeArraySetsPropertiesCorrectly(): void + public function testSetFromArraySetsPropertiesCorrectly(): void { - $sslContextObject = new SslContext(); - $sslContextObject->exchangeArray($this->correspondingSslContextArray); - - $this->assertEquals( - $this->correspondingSslContextObject, - $sslContextObject - ); - } - - public function testGetArrayCopyReturnsAnArrayWithPropertyValues() - { - $sslContextArray = $this->correspondingSslContextObject->getArrayCopy(); - - $this->assertEquals($this->correspondingSslContextArray, $sslContextArray); - } - - public function testExchangeArrayThrowsExceptionWhenProvidingInvalidKeyName(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessageMatches( - '/does not contain the property "someInvalidKey" corresponding to the array key "some_invalid_key"/' - ); - - $sslContextObject = new SslContext(); - $sslContextObject->exchangeArray(['some_invalid_key' => true]); + $context = SslContext::fromSslContextArray(self::SSL_CONTEXT); + + self::assertSame(self::SSL_CONTEXT['peer_name'], $context->expectedPeerName); + self::assertSame(self::SSL_CONTEXT['verify_peer'], $context->verifyPeer); + self::assertSame(self::SSL_CONTEXT['verify_peer_name'], $context->verifyPeerName); + self::assertSame(self::SSL_CONTEXT['allow_self_signed'], $context->allowSelfSignedCertificates); + self::assertSame(self::SSL_CONTEXT['cafile'], $context->certificateAuthorityFile); + self::assertSame(self::SSL_CONTEXT['capath'], $context->certificateAuthorityPath); + self::assertSame(self::SSL_CONTEXT['local_cert'], $context->localCertificatePath); + self::assertSame(self::SSL_CONTEXT['local_pk'], $context->localPrivateKeyPath); + self::assertSame(self::SSL_CONTEXT['passphrase'], $context->passphrase); + self::assertSame(self::SSL_CONTEXT['verify_depth'], $context->verifyDepth); + self::assertSame(self::SSL_CONTEXT['ciphers'], $context->ciphers); + self::assertSame(self::SSL_CONTEXT['SNI_enabled'], $context->serverNameIndicationEnabled); + self::assertSame(self::SSL_CONTEXT['disable_compression'], $context->disableCompression); + self::assertSame(self::SSL_CONTEXT['peer_fingerprint'], $context->peerFingerprint); + self::assertSame(self::SSL_CONTEXT['security_level'], $context->securityLevel); } - public function testExchangeArrayThrowsExceptionWhenProvidingInvalidValueType(): void + public function testSetFromArrayThrowsTypeErrorWhenProvidingInvalidValueType(): void { $this->expectException(TypeError::class); - $this->expectExceptionMessageMatches( - '/\$verifyPeer of type bool/' - ); + $this->expectExceptionMessageMatches('/\(\$verifyPeer\) must be of type \?bool, string given/'); - $sslContextObject = new SslContext(); - $sslContextObject->exchangeArray(['verify_peer' => 'invalid type']); + /** @psalm-suppress InvalidArgument We do want to verify what happens when invalid types are passed. */ + SslContext::fromSslContextArray(['verify_peer' => 'invalid type']); } }