From 37edbe8cab395f21cf27df10d7e73f54f1f81aa4 Mon Sep 17 00:00:00 2001 From: George Steel Date: Wed, 1 Nov 2023 21:51:59 +0000 Subject: [PATCH] Remove dependency on laminas-crypt Instead of requiring laminas-crypt, call `hash_hmac` directly. A few psalm issues have been base-lined because we can't add native types without breaking BC. Also, some defensive conditions contradict doc-block types. Subtle potential BC break here is that an exception will now be thrown if the password is empty|null, whereas previously, it would likely have been a type error. Signed-off-by: George Steel --- composer.json | 10 +- composer.lock | 143 +----------------------- psalm-baseline.xml | 24 ++-- src/Protocol/Smtp/Auth/Crammd5.php | 51 +++++---- test/Protocol/Smtp/Auth/Crammd5Test.php | 27 +++++ 5 files changed, 81 insertions(+), 174 deletions(-) diff --git a/composer.json b/composer.json index e939b024..a811d30e 100644 --- a/composer.json +++ b/composer.json @@ -20,16 +20,14 @@ }, "require-dev": { "laminas/laminas-coding-standard": "~2.5.0", - "laminas/laminas-crypt": "^3.10.0", "laminas/laminas-db": "^2.18", - "laminas/laminas-servicemanager": "^3.21", - "phpunit/phpunit": "^10.1.3", + "laminas/laminas-servicemanager": "^3.22.1", + "phpunit/phpunit": "^10.4.2", "psalm/plugin-phpunit": "^0.18.4", - "symfony/process": "^6.2.10", - "vimeo/psalm": "^5.11" + "symfony/process": "^6.3.4", + "vimeo/psalm": "^5.15" }, "suggest": { - "laminas/laminas-crypt": "^3.10 Crammd5 support in SMTP Auth", "laminas/laminas-servicemanager": "^3.21 when using SMTP to deliver messages" }, "config": { diff --git a/composer.lock b/composer.lock index 7a9216e2..20fcb2f8 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": "ff7a7449f0dedf8f339662493072a3a5", + "content-hash": "9a47416ea44afe826a1255c43b30e03d", "packages": [ { "name": "laminas/laminas-loader", @@ -274,16 +274,16 @@ }, { "name": "laminas/laminas-validator", - "version": "2.40.0", + "version": "2.41.0", "source": { "type": "git", "url": "https://github.com/laminas/laminas-validator.git", - "reference": "0474aec33a802a8081ec2523ec0a51c1e3d64b08" + "reference": "ca27df621e12cbd4c138dab7abf3e7e5692c582c" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/laminas/laminas-validator/zipball/0474aec33a802a8081ec2523ec0a51c1e3d64b08", - "reference": "0474aec33a802a8081ec2523ec0a51c1e3d64b08", + "url": "https://api.github.com/repos/laminas/laminas-validator/zipball/ca27df621e12cbd4c138dab7abf3e7e5692c582c", + "reference": "ca27df621e12cbd4c138dab7abf3e7e5692c582c", "shasum": "" }, "require": { @@ -354,7 +354,7 @@ "type": "community_bridge" } ], - "time": "2023-10-23T09:49:44+00:00" + "time": "2023-10-30T08:22:19+00:00" }, { "name": "psr/container", @@ -1634,70 +1634,6 @@ ], "time": "2023-01-05T15:53:40+00:00" }, - { - "name": "laminas/laminas-crypt", - "version": "3.10.0", - "source": { - "type": "git", - "url": "https://github.com/laminas/laminas-crypt.git", - "reference": "588375caf4d505fee90d1449e9714c912ceb5051" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/laminas/laminas-crypt/zipball/588375caf4d505fee90d1449e9714c912ceb5051", - "reference": "588375caf4d505fee90d1449e9714c912ceb5051", - "shasum": "" - }, - "require": { - "ext-mbstring": "*", - "laminas/laminas-math": "^3.4", - "laminas/laminas-servicemanager": "^3.11.2", - "laminas/laminas-stdlib": "^3.6", - "php": "~8.0.0 || ~8.1.0 || ~8.2.0", - "psr/container": "^1.1" - }, - "conflict": { - "zendframework/zend-crypt": "*" - }, - "require-dev": { - "laminas/laminas-coding-standard": "~2.4.0", - "phpunit/phpunit": "^9.5.25" - }, - "suggest": { - "ext-openssl": "Required for most features of Laminas\\Crypt" - }, - "type": "library", - "autoload": { - "psr-4": { - "Laminas\\Crypt\\": "src/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "BSD-3-Clause" - ], - "description": "Strong cryptography tools and password hashing", - "homepage": "https://laminas.dev", - "keywords": [ - "crypt", - "laminas" - ], - "support": { - "chat": "https://laminas.dev/chat", - "docs": "https://docs.laminas.dev/laminas-crypt/", - "forum": "https://discourse.laminas.dev", - "issues": "https://github.com/laminas/laminas-crypt/issues", - "rss": "https://github.com/laminas/laminas-crypt/releases.atom", - "source": "https://github.com/laminas/laminas-crypt" - }, - "funding": [ - { - "url": "https://funding.communitybridge.org/projects/laminas-project", - "type": "community_bridge" - } - ], - "time": "2023-03-03T15:57:57+00:00" - }, { "name": "laminas/laminas-db", "version": "2.18.0", @@ -1769,73 +1705,6 @@ ], "time": "2023-05-05T16:22:28+00:00" }, - { - "name": "laminas/laminas-math", - "version": "3.7.0", - "source": { - "type": "git", - "url": "https://github.com/laminas/laminas-math.git", - "reference": "3e90445828fd64308de2a600b48c3df051b3b17a" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/laminas/laminas-math/zipball/3e90445828fd64308de2a600b48c3df051b3b17a", - "reference": "3e90445828fd64308de2a600b48c3df051b3b17a", - "shasum": "" - }, - "require": { - "ext-mbstring": "*", - "php": "~8.0.0 || ~8.1.0 || ~8.2.0 || ~8.3.0" - }, - "conflict": { - "zendframework/zend-math": "*" - }, - "require-dev": { - "laminas/laminas-coding-standard": "~2.4.0", - "phpunit/phpunit": "~9.5.25" - }, - "suggest": { - "ext-bcmath": "If using the bcmath functionality", - "ext-gmp": "If using the gmp functionality" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "3.2.x-dev", - "dev-develop": "3.3.x-dev" - } - }, - "autoload": { - "psr-4": { - "Laminas\\Math\\": "src/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "BSD-3-Clause" - ], - "description": "Create cryptographically secure pseudo-random numbers, and manage big integers", - "homepage": "https://laminas.dev", - "keywords": [ - "laminas", - "math" - ], - "support": { - "chat": "https://laminas.dev/chat", - "docs": "https://docs.laminas.dev/laminas-math/", - "forum": "https://discourse.laminas.dev", - "issues": "https://github.com/laminas/laminas-math/issues", - "rss": "https://github.com/laminas/laminas-math/releases.atom", - "source": "https://github.com/laminas/laminas-math" - }, - "funding": [ - { - "url": "https://funding.communitybridge.org/projects/laminas-project", - "type": "community_bridge" - } - ], - "time": "2023-10-18T09:53:37+00:00" - }, { "name": "myclabs/deep-copy", "version": "1.11.1", diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 0140e43e..81fdb8eb 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,5 @@ - + ! is_string($email) @@ -507,7 +507,6 @@ $from $result - $string $ids[0] @@ -609,7 +608,6 @@ $endPos - $string escapeString($old, $new)]]> escapeString($reference, $mailbox)]]> escapeString($user, $password)]]> @@ -764,6 +762,15 @@ + + $challenge + + + ! is_string($data) + ! is_string($key) + + + 235 334 @@ -775,9 +782,13 @@ + + getPassword()]]> + + + getUsername()]]> + - $password - $username Crammd5 Crammd5 Crammd5 @@ -1746,9 +1757,6 @@ $param - - getFieldValue(HeaderInterface::FORMAT_ENCODED)]]> - getFieldValue diff --git a/src/Protocol/Smtp/Auth/Crammd5.php b/src/Protocol/Smtp/Auth/Crammd5.php index 3435cf7c..571dd19e 100644 --- a/src/Protocol/Smtp/Auth/Crammd5.php +++ b/src/Protocol/Smtp/Auth/Crammd5.php @@ -2,23 +2,25 @@ namespace Laminas\Mail\Protocol\Smtp\Auth; -use Laminas\Crypt\Hmac; +use Laminas\Mail\Exception\InvalidArgumentException; use Laminas\Mail\Protocol\Smtp; use function array_replace_recursive; use function base64_decode; use function base64_encode; +use function hash_hmac; use function is_array; +use function is_string; /** * Performs CRAM-MD5 authentication */ class Crammd5 extends Smtp { - /** @var string */ + /** @var non-empty-string|null */ protected $username; - /** @var string */ + /** @var non-empty-string|null */ protected $password; /** @@ -32,23 +34,18 @@ class Crammd5 extends Smtp public function __construct($host = '127.0.0.1', $port = null, $config = null) { // Did we receive a configuration array? + $config = $config ?? []; $origConfig = $config; if (is_array($host)) { // Merge config array with principal array, if provided - if (is_array($config)) { - $config = array_replace_recursive($host, $config); - } else { - $config = $host; - } + $config = array_replace_recursive($host, $config); } - if (is_array($config)) { - if (isset($config['username'])) { - $this->setUsername($config['username']); - } - if (isset($config['password'])) { - $this->setPassword($config['password']); - } + if (isset($config['username'])) { + $this->setUsername($config['username']); + } + if (isset($config['password'])) { + $this->setPassword($config['password']); } // Call parent with original arguments @@ -75,7 +72,7 @@ public function auth() /** * Set value for username * - * @param string $username + * @param non-empty-string $username * @return Crammd5 */ public function setUsername($username) @@ -87,7 +84,7 @@ public function setUsername($username) /** * Get username * - * @return string + * @return non-empty-string|null */ public function getUsername() { @@ -97,7 +94,7 @@ public function getUsername() /** * Set value for password * - * @param string $password + * @param non-empty-string $password * @return Crammd5 */ public function setPassword($password) @@ -109,7 +106,7 @@ public function setPassword($password) /** * Get password * - * @return string + * @return non-empty-string|null */ public function getPassword() { @@ -119,13 +116,21 @@ public function getPassword() /** * Prepare CRAM-MD5 response to server's ticket * - * @param string $key Challenge key (usually password) - * @param string $data Challenge data + * @param non-empty-string $key Challenge key (usually password) + * @param non-empty-string $data Challenge data * @param int $block Length of blocks (deprecated; unused) * @return string */ - protected function hmacMd5($key, $data, $block = 64) + protected function hmacMd5($key, $data, /** @deprecated */ $block = 64) { - return Hmac::compute($key, 'md5', $data); + if (! is_string($key) || $key === '') { + throw new InvalidArgumentException('CramMD5 authentication requires a non-empty password'); + } + + if (! is_string($data) || $data === '') { + throw new InvalidArgumentException('CramMD5 authentication requires a non-empty challenge'); + } + + return hash_hmac('md5', $data, $key, false); } } diff --git a/test/Protocol/Smtp/Auth/Crammd5Test.php b/test/Protocol/Smtp/Auth/Crammd5Test.php index b9ed17d5..7fd570f9 100644 --- a/test/Protocol/Smtp/Auth/Crammd5Test.php +++ b/test/Protocol/Smtp/Auth/Crammd5Test.php @@ -2,6 +2,7 @@ namespace LaminasTest\Mail\Protocol\Smtp\Auth; +use Laminas\Mail\Exception\InvalidArgumentException; use Laminas\Mail\Protocol\Smtp\Auth\Crammd5; use PHPUnit\Framework\TestCase; use ReflectionClass; @@ -33,6 +34,32 @@ public function testHmacMd5ReturnsExpectedHash(): void $this->assertEquals('be56fa81a5671e0c62e00134180aae2c', $result); } + public function testAnExceptionIsThrownForEmptyPassword(): void + { + $class = new ReflectionClass(Crammd5::class); + $method = $class->getMethod('hmacMd5'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('CramMD5 authentication requires a non-empty password'); + $method->invokeArgs( + $this->auth, + ['', 'data'] + ); + } + + public function testAnExceptionIsThrownForEmptyChallenge(): void + { + $class = new ReflectionClass(Crammd5::class); + $method = $class->getMethod('hmacMd5'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('CramMD5 authentication requires a non-empty challenge'); + $method->invokeArgs( + $this->auth, + ['foo', ''] + ); + } + public function testUsernameAccessors(): void { $this->auth->setUsername('test');