Skip to content

Commit

Permalink
Remove dependency on laminas-crypt
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
gsteel committed Nov 1, 2023
1 parent 152e2ca commit f9ada6f
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 174 deletions.
10 changes: 4 additions & 6 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
143 changes: 6 additions & 137 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 11 additions & 8 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
@@ -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>
Expand Down Expand Up @@ -507,7 +507,6 @@
<MixedArgumentTypeCoercion>
<code>$from</code>
<code>$result</code>
<code>$string</code>
</MixedArgumentTypeCoercion>
<MixedArrayAccess>
<code>$ids[0]</code>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -764,6 +762,10 @@
</RedundantCastGivenDocblockType>
</file>
<file src="src/Protocol/Smtp/Auth/Crammd5.php">
<DocblockTypeContradiction>
<code>! is_string($key)</code>
<code><![CDATA[$key === '']]></code>
</DocblockTypeContradiction>
<InvalidArgument>
<code>235</code>
<code>334</code>
Expand All @@ -775,9 +777,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>
Expand Down Expand Up @@ -1746,9 +1752,6 @@
<MixedOperand>
<code>$param</code>
</MixedOperand>
<MixedReturnStatement>
<code><![CDATA[$header->getFieldValue(HeaderInterface::FORMAT_ENCODED)]]></code>
</MixedReturnStatement>
<PossiblyFalseReference>
<code>getFieldValue</code>
</PossiblyFalseReference>
Expand Down
51 changes: 28 additions & 23 deletions src/Protocol/Smtp/Auth/Crammd5.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -87,7 +84,7 @@ public function setUsername($username)
/**
* Get username
*
* @return string
* @return non-empty-string|null
*/
public function getUsername()
{
Expand All @@ -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)
Expand All @@ -109,7 +106,7 @@ public function setPassword($password)
/**
* Get password
*
* @return string
* @return non-empty-string|null
*/
public function getPassword()
{
Expand All @@ -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 === '') {

Check failure on line 130 in src/Protocol/Smtp/Auth/Crammd5.php

View workflow job for this annotation

GitHub Actions / ci / QA Checks (Psalm [8.1, locked], ubuntu-latest, laminas/laminas-continuous-integration-action@v1, ...

DocblockTypeContradiction

src/Protocol/Smtp/Auth/Crammd5.php:130:13: DocblockTypeContradiction: Docblock-defined type non-empty-string for $data is always string (see https://psalm.dev/155)

Check failure on line 130 in src/Protocol/Smtp/Auth/Crammd5.php

View workflow job for this annotation

GitHub Actions / ci / QA Checks (Psalm [8.1, locked], ubuntu-latest, laminas/laminas-continuous-integration-action@v1, ...

DocblockTypeContradiction

src/Protocol/Smtp/Auth/Crammd5.php:130:35: DocblockTypeContradiction: '' does not contain non-empty-string (see https://psalm.dev/155)
throw new InvalidArgumentException('CramMD5 authentication requires a non-empty challenge');
}

return hash_hmac('md5', $data, $key, false);
}
}
Loading

0 comments on commit f9ada6f

Please sign in to comment.