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 <george@net-glue.co.uk>
laminas · Nov 1, 2023 · 37edbe8 · 37edbe8
1 parent 152e2ca
commit 37edbe8
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 174 deletions.
diff --git 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
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.11.0@c9b192ab8400fdaf04b2b13d110575adc879aa90">
+<files psalm-version="5.15.0@5c774aca4746caf3d239d9c8cadb9f882ca29352">
   <file src="src/Address.php">
     <DocblockTypeContradiction>
       <code>! is_string($email)</code>
@@ -507,7 +507,6 @@
     <MixedArgumentTypeCoercion>
       <code>$from</code>
       <code>$result</code>
-      <code>$string</code>
     </MixedArgumentTypeCoercion>
     <MixedArrayAccess>
       <code>$ids[0]</code>
@@ -609,7 +608,6 @@
       <code>$endPos</code>
     </PossiblyFalseOperand>
     <PossiblyInvalidArgument>
-      <code>$string</code>
       <code><![CDATA[$this->escapeString($old, $new)]]></code>
       <code><![CDATA[$this->escapeString($reference, $mailbox)]]></code>
       <code><![CDATA[$this->escapeString($user, $password)]]></code>
@@ -764,6 +762,15 @@
     </RedundantCastGivenDocblockType>
   </file>
   <file src="src/Protocol/Smtp/Auth/Crammd5.php">
+    <ArgumentTypeCoercion>
+      <code>$challenge</code>
+    </ArgumentTypeCoercion>
+    <DocblockTypeContradiction>
+      <code>! is_string($data)</code>
+      <code>! is_string($key)</code>
+      <code><![CDATA[$data === '']]></code>
+      <code><![CDATA[$key === '']]></code>
+    </DocblockTypeContradiction>
     <InvalidArgument>
       <code>235</code>
       <code>334</code>
@@ -775,9 +782,13 @@
       <code><![CDATA[$config['password']]]></code>
       <code><![CDATA[$config['username']]]></code>
     </MixedArgument>
+    <PossiblyNullArgument>
+      <code><![CDATA[$this->getPassword()]]></code>
+    </PossiblyNullArgument>
+    <PossiblyNullOperand>
+      <code><![CDATA[$this->getUsername()]]></code>
+    </PossiblyNullOperand>
     <PropertyNotSetInConstructor>
-      <code>$password</code>
-      <code>$username</code>
       <code>Crammd5</code>
       <code>Crammd5</code>
       <code>Crammd5</code>
@@ -1746,9 +1757,6 @@
     <MixedOperand>
       <code>$param</code>
     </MixedOperand>
-    <MixedReturnStatement>
-      <code><![CDATA[$header->getFieldValue(HeaderInterface::FORMAT_ENCODED)]]></code>
-    </MixedReturnStatement>
     <PossiblyFalseReference>
       <code>getFieldValue</code>
     </PossiblyFalseReference>

diff --git 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);
     }
 }