From cad352da7d47245629d95e5a2c32b600cdf75a24 Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 3 May 2026 22:04:36 -0400 Subject: [PATCH 1/3] Use options DTO classes instead of arrays. --- CHANGELOG.md | 1 + UPGRADE-1.0.md | 146 +++++++++++ .../Sasl/Challenge/AnonymousChallenge.php | 19 +- .../Sasl/Challenge/ChallengeInterface.php | 12 +- .../Sasl/Challenge/CramMD5Challenge.php | 46 ++-- .../Sasl/Challenge/DigestMD5Challenge.php | 85 +++---- src/FreeDSx/Sasl/Challenge/PlainChallenge.php | 40 +-- .../Sasl/Challenge/ResolvesOptionsTrait.php | 44 ++++ src/FreeDSx/Sasl/Challenge/ScramChallenge.php | 56 ++--- src/FreeDSx/Sasl/MechanismSelector.php | 14 +- src/FreeDSx/Sasl/Options/AnonymousOptions.php | 36 +++ .../Options/ChallengeOptionsInterface.php | 23 ++ src/FreeDSx/Sasl/Options/CramMD5Options.php | 94 +++++++ src/FreeDSx/Sasl/Options/DigestMD5Options.php | 190 ++++++++++++++ src/FreeDSx/Sasl/Options/PlainOptions.php | 78 ++++++ src/FreeDSx/Sasl/Options/SaslOptions.php | 49 ++++ src/FreeDSx/Sasl/Options/ScramOptions.php | 139 ++++++++++ src/FreeDSx/Sasl/Options/SelectOptions.php | 50 ++++ src/FreeDSx/Sasl/Sasl.php | 22 +- .../unit/Challenge/AnonymousChallengeTest.php | 5 +- tests/unit/Challenge/CramMD5ChallengeTest.php | 25 +- .../unit/Challenge/DigestMD5ChallengeTest.php | 37 +-- tests/unit/Challenge/PlainChallengeTest.php | 17 +- tests/unit/Challenge/ScramChallengeTest.php | 237 +++++++++++------- tests/unit/MechanismSelectorTest.php | 11 +- tests/unit/SaslTest.php | 3 +- 26 files changed, 1202 insertions(+), 277 deletions(-) create mode 100644 src/FreeDSx/Sasl/Challenge/ResolvesOptionsTrait.php create mode 100644 src/FreeDSx/Sasl/Options/AnonymousOptions.php create mode 100644 src/FreeDSx/Sasl/Options/ChallengeOptionsInterface.php create mode 100644 src/FreeDSx/Sasl/Options/CramMD5Options.php create mode 100644 src/FreeDSx/Sasl/Options/DigestMD5Options.php create mode 100644 src/FreeDSx/Sasl/Options/PlainOptions.php create mode 100644 src/FreeDSx/Sasl/Options/SaslOptions.php create mode 100644 src/FreeDSx/Sasl/Options/ScramOptions.php create mode 100644 src/FreeDSx/Sasl/Options/SelectOptions.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aee3a9..2c87929 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ CHANGELOG * Replace SCRAM hash-algorithm string constants with the `HashAlgorithm` enum. * Replace DIGEST-MD5 message-type integer constants with the `DigestMD5MessageType` enum. * Replace DIGEST-MD5 cipher string keys with the `DigestMD5Cipher` enum. +* Replace all `array $options` parameters with typed options DTOs (see UPGRADE-1.0.md). 0.2.1 (2026-03-22) ------------------ diff --git a/UPGRADE-1.0.md b/UPGRADE-1.0.md index b6a16c5..f9b64ea 100644 --- a/UPGRADE-1.0.md +++ b/UPGRADE-1.0.md @@ -37,3 +37,149 @@ new Sasl(['supported' => [MechanismName::DIGEST_MD5]]); ## Concrete classes are `final` All concrete classes in the library are now marked `final`. If you were extending any of these, switch to composition instead. + +## Options arrays replaced by typed DTOs + +All `array $options` parameters have been replaced by typed DTO objects from the `FreeDSx\Sasl\Options` namespace. + +### `Sasl` constructor + +```php +// before +new Sasl(['supported' => [MechanismName::DIGEST_MD5]]); + +// after +use FreeDSx\Sasl\Options\SaslOptions; + +new Sasl(new SaslOptions(supported: [MechanismName::DIGEST_MD5])); +``` + +### `Sasl::select()` / `MechanismSelector::select()` + +```php +// before +$sasl->select($choices, ['use_integrity' => true]); +$sasl->select($choices, ['use_privacy' => true]); + +// after +use FreeDSx\Sasl\Options\SelectOptions; + +$sasl->select($choices, (new SelectOptions())->setUseIntegrity(true)); +$sasl->select($choices, (new SelectOptions())->setUsePrivacy(true)); +``` + +### `ChallengeInterface::challenge()` + +The second parameter changed from `array $options = []` to `?ChallengeOptionsInterface $options = null`. Pass the mechanism-specific DTO, or omit the argument entirely when no options are needed. + +Passing a DTO of the wrong type throws a `SaslException`. + +#### ANONYMOUS + +```php +// before +$challenge->challenge(null, ['trace' => 'user@example.com']); + +// after +use FreeDSx\Sasl\Options\AnonymousOptions; + +$challenge->challenge(null, (new AnonymousOptions())->setTrace('user@example.com')); +``` + +The `username` key alias accepted by the old array has been removed. Use `setTrace()` instead. + +#### PLAIN + +```php +// before — client +$challenge->challenge(null, ['username' => 'alice', 'password' => 'secret']); + +// before — server +$challenge->challenge($received, ['validate' => $fn]); + +// after — client +use FreeDSx\Sasl\Options\PlainOptions; + +$challenge->challenge(null, (new PlainOptions())->setUsername('alice')->setPassword('secret')); + +// after — server +$challenge->challenge($received, (new PlainOptions())->setValidate($fn)); +``` + +#### CRAM-MD5 + +The server-side callable was previously passed under the `password` key (conflicting with the client's string password). It is now `setPasswordCallback()`. + +```php +// before — client +$challenge->challenge($received, ['username' => 'alice', 'password' => 'secret']); + +// before — server (generate challenge with fixed nonce) +$challenge->challenge(null, ['challenge' => 'mynonce']); + +// before — server (validate) +$challenge->challenge($received, ['password' => $callable]); + +// after — client +use FreeDSx\Sasl\Options\CramMD5Options; + +$challenge->challenge($received, (new CramMD5Options())->setUsername('alice')->setPassword('secret')); + +// after — server (generate challenge with fixed nonce) +$challenge->challenge(null, (new CramMD5Options())->setChallenge('mynonce')); + +// after — server (validate) +$challenge->challenge($received, (new CramMD5Options())->setPasswordCallback($callable)); +``` + +#### SCRAM + +```php +// before — client-first +$challenge->challenge(null, ['username' => 'alice', 'cnonce' => $cnonce]); + +// before — client-final +$challenge->challenge($serverFirst, ['password' => 'secret']); + +// before — server-first +$challenge->challenge($clientFirst, ['nonce' => $snonce, 'salt' => $salt, 'iterations' => 4096]); + +// before — server-final +$challenge->challenge($clientFinal, ['password' => 'secret']); + +// after +use FreeDSx\Sasl\Options\ScramOptions; + +$challenge->challenge(null, (new ScramOptions())->setUsername('alice')->setCnonce($cnonce)); +$challenge->challenge($serverFirst, (new ScramOptions())->setPassword('secret')); +$challenge->challenge($clientFirst, (new ScramOptions())->setNonce($snonce)->setSalt($salt)->setIterations(4096)); +$challenge->challenge($clientFinal, (new ScramOptions())->setPassword('secret')); +``` + +`cbind_type` → `setCbindType()`, `cbind_data` → `setCbindData()`. + +#### DIGEST-MD5 + +```php +// before +$challenge->challenge($received, [ + 'use_privacy' => true, + 'use_integrity' => false, + 'username' => 'alice', + 'password' => 'secret', + 'host' => 'ldap.example.com', + 'nonce_size' => 32, +]); + +// after +use FreeDSx\Sasl\Options\DigestMD5Options; + +$challenge->challenge($received, (new DigestMD5Options()) + ->setUsePrivacy(true) + ->setUsername('alice') + ->setPassword('secret') + ->setHost('ldap.example.com') + ->setNonceSize(32)); +``` + +Key renames: `use_integrity` → `setUseIntegrity()`, `use_privacy` → `setUsePrivacy()`, `nonce_size` → `setNonceSize()`. diff --git a/src/FreeDSx/Sasl/Challenge/AnonymousChallenge.php b/src/FreeDSx/Sasl/Challenge/AnonymousChallenge.php index eaf1414..20827a6 100644 --- a/src/FreeDSx/Sasl/Challenge/AnonymousChallenge.php +++ b/src/FreeDSx/Sasl/Challenge/AnonymousChallenge.php @@ -15,6 +15,8 @@ use FreeDSx\Sasl\Encoder\AnonymousEncoder; use FreeDSx\Sasl\Message; +use FreeDSx\Sasl\Options\AnonymousOptions; +use FreeDSx\Sasl\Options\ChallengeOptionsInterface; use FreeDSx\Sasl\SaslContext; /** @@ -24,6 +26,8 @@ */ final readonly class AnonymousChallenge implements ChallengeInterface { + use ResolvesOptionsTrait; + private readonly SaslContext $context; private readonly AnonymousEncoder $encoder; @@ -37,12 +41,14 @@ public function __construct(bool $isServerMode = false) public function challenge( ?string $received = null, - array $options = [], + ?ChallengeOptionsInterface $options = null, ): SaslContext { + $resolved = $this->resolveOptions($options, AnonymousOptions::class); + if ($this->context->isServerMode()) { $this->processServer($received); } else { - $this->processClient($options); + $this->processClient($resolved); } return $this->context; @@ -63,14 +69,11 @@ private function processServer(?string $received): void } } - /** - * @param array $options - */ - private function processClient(array $options): void + private function processClient(AnonymousOptions $options): void { $data = []; - if (isset($options['username']) || isset($options['trace'])) { - $data['trace'] = $options['trace'] ?? $options['username']; + if ($options->getTrace() !== null) { + $data['trace'] = $options->getTrace(); } $this->context->setResponse($this->encoder->encode(new Message($data), $this->context)); diff --git a/src/FreeDSx/Sasl/Challenge/ChallengeInterface.php b/src/FreeDSx/Sasl/Challenge/ChallengeInterface.php index 6082479..6adb6fd 100644 --- a/src/FreeDSx/Sasl/Challenge/ChallengeInterface.php +++ b/src/FreeDSx/Sasl/Challenge/ChallengeInterface.php @@ -14,6 +14,7 @@ namespace FreeDSx\Sasl\Challenge; use FreeDSx\Sasl\Exception\SaslException; +use FreeDSx\Sasl\Options\ChallengeOptionsInterface; use FreeDSx\Sasl\SaslContext; /** @@ -24,20 +25,15 @@ interface ChallengeInterface { /** - * Generate the next response to send in the challenge. It takes two optional parameters: - * - * - The last message received. Null if no message has been received yet. - * - An array of options used for generating the next message. - * - * The SaslContext returned indicates various aspects of the state of the challenge, including the response. + * Generate the next response to send in the challenge. * * @param string|null $received the last message received, or null if none - * @param array $options options for generating the next message + * @param ?ChallengeOptionsInterface $options mechanism-specific options DTO * * @throws SaslException */ public function challenge( ?string $received = null, - array $options = [], + ?ChallengeOptionsInterface $options = null, ): SaslContext; } diff --git a/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php b/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php index 4d9ddc8..74d9fd2 100644 --- a/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php +++ b/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php @@ -17,6 +17,8 @@ use FreeDSx\Sasl\Exception\SaslException; use FreeDSx\Sasl\Factory\NonceTrait; use FreeDSx\Sasl\Message; +use FreeDSx\Sasl\Options\ChallengeOptionsInterface; +use FreeDSx\Sasl\Options\CramMD5Options; use FreeDSx\Sasl\SaslContext; /** @@ -27,6 +29,7 @@ final readonly class CramMD5Challenge implements ChallengeInterface { use NonceTrait; + use ResolvesOptionsTrait; private readonly SaslContext $context; @@ -41,33 +44,34 @@ public function __construct(bool $isServerMode = false) public function challenge( ?string $received = null, - array $options = [], + ?ChallengeOptionsInterface $options = null, ): SaslContext { + $resolved = $this->resolveOptions( + $options, + CramMD5Options::class, + ); $message = $received === null ? null : $this->encoder->decode($received, $this->context); if ($message === null) { if ($this->context->isServerMode()) { - $this->generateServerChallenge($options); + $this->generateServerChallenge($resolved); } return $this->context; } if ($this->context->isServerMode()) { - $this->validateClientResponse($message, $options); + $this->validateClientResponse($message, $resolved); } else { - $this->generateClientResponse($message, $options); + $this->generateClientResponse($message, $resolved); } return $this->context; } - /** - * @param array $options - */ - private function generateServerChallenge(array $options): void + private function generateServerChallenge(CramMD5Options $options): void { - $nonce = (string) ($options['challenge'] ?? $this->generateNonce(32)); + $nonce = $options->getChallenge() ?? $this->generateNonce(32); $challenge = new Message(['challenge' => $nonce]); $encoded = $this->encoder->encode($challenge, $this->context); $this->context->setResponse($encoded); @@ -77,36 +81,32 @@ private function generateServerChallenge(array $options): void } /** - * @param array $options - * * @throws SaslException */ private function generateClientResponse( Message $received, - array $options, + CramMD5Options $options, ): void { if (!$received->has('challenge')) { throw new SaslException('Expected a server challenge to generate a client response.'); } - if (!isset($options['username'], $options['password'])) { + if ($options->getUsername() === null || $options->getPassword() === null) { throw new SaslException('A username and password is required for a client response.'); } $response = new Message([ - 'username' => $options['username'], - 'digest' => $this->generateDigest((string) $received->get('challenge'), (string) $options['password']), + 'username' => $options->getUsername(), + 'digest' => $this->generateDigest((string) $received->get('challenge'), $options->getPassword()), ]); $this->context->setResponse($this->encoder->encode($response, $this->context)); $this->context->setIsComplete(true); } /** - * @param array $options - * * @throws SaslException */ private function validateClientResponse( Message $received, - array $options, + CramMD5Options $options, ): void { if (!$received->has('username')) { throw new SaslException('The client response must have a username.'); @@ -114,17 +114,13 @@ private function validateClientResponse( if (!$received->has('digest')) { throw new SaslException('The client response must have a digest.'); } - if (!isset($options['password'])) { - throw new SaslException('To validate the client response you must supply the password option.'); + if ($options->getPasswordCallback() === null) { + throw new SaslException('To validate the client response you must supply the passwordCallback option.'); } $username = $received->get('username'); $digest = $received->get('digest'); - $password = $options['password']; - if (!is_callable($password)) { - throw new SaslException('The password option must be callable. It will be passed the username and challenge'); - } - $expectedDigest = $password($username, $this->context->get('challenge')); + $expectedDigest = ($options->getPasswordCallback())($username, $this->context->get('challenge')); $this->context->setIsAuthenticated($expectedDigest === $digest); $this->context->setIsComplete(true); diff --git a/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php b/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php index 14e1250..1c67831 100644 --- a/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php +++ b/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php @@ -19,6 +19,8 @@ use FreeDSx\Sasl\Factory\DigestMD5MessageType; use FreeDSx\Sasl\Mechanism\DigestMD5Mechanism; use FreeDSx\Sasl\Message; +use FreeDSx\Sasl\Options\ChallengeOptionsInterface; +use FreeDSx\Sasl\Options\DigestMD5Options; use FreeDSx\Sasl\SaslContext; /** @@ -28,15 +30,7 @@ */ final class DigestMD5Challenge implements ChallengeInterface { - /** - * @var array - */ - private const DEFAULTS = [ - 'use_integrity' => false, - 'use_privacy' => false, - 'service' => 'ldap', - 'nonce_size' => null, - ]; + use ResolvesOptionsTrait; private readonly SaslContext $context; @@ -56,27 +50,28 @@ public function __construct(bool $isServerMode = false) public function challenge( ?string $received = null, - array $options = [], + ?ChallengeOptionsInterface $options = null, ): SaslContext { - $options = $options + self::DEFAULTS; + $resolved = $this->resolveOptions( + $options, + DigestMD5Options::class, + ); $message = $received === null ? null : $this->encoder->decode($received, $this->context); $response = $this->context->isServerMode() - ? $this->generateServerResponse($message, $options) - : $this->generateClientResponse($message, $options); + ? $this->generateServerResponse($message, $resolved) + : $this->generateClientResponse($message, $resolved); $this->context->setResponse($response); return $this->context; } /** - * @param array $options - * * @throws SaslException */ private function generateClientResponse( ?Message $message, - array $options, + DigestMD5Options $options, ): ?string { if ($message === null) { return null; @@ -90,7 +85,7 @@ private function generateClientResponse( } if ($message->has('rspauth') && $message->get('rspauth') === $this->context->get('verification')) { $this->context->setIsAuthenticated(true); - $this->context->setHasSecurityLayer($options['use_integrity'] || $options['use_privacy']); + $this->context->setHasSecurityLayer($options->isUseIntegrity() || $options->isUsePrivacy()); } if ($this->context->hasSecurityLayer()) { $this->context->set('seqnumsnt', 0); @@ -102,13 +97,11 @@ private function generateClientResponse( } /** - * @param array $options - * * @throws SaslException */ private function generateServerResponse( ?Message $received, - array $options, + DigestMD5Options $options, ): ?string { $response = $received === null ? $this->generateServerChallenge($options) @@ -127,38 +120,36 @@ private function isClientChallengeNeeded(Message $message): bool } /** - * @param array $options - * * @throws SaslException */ private function createClientResponse( Message $message, - array $options, + DigestMD5Options $options, ): string { - $password = (string) ($options['password'] ?? ''); + $password = $options->getPassword() ?? ''; $qop = match (true) { - (bool) $options['use_privacy'] => 'auth-conf', - (bool) $options['use_integrity'] => 'auth-int', + $options->isUsePrivacy() => 'auth-conf', + $options->isUseIntegrity() => 'auth-int', default => 'auth', }; $this->context->set('qop', $qop); $messageOpts = [ - 'username' => $options['username'] ?? null, - 'digest-uri' => isset($options['host']) ? ($options['service'] . '/' . $options['host']) : null, + 'username' => $options->getUsername(), + 'digest-uri' => $options->getHost() !== null ? ($options->getService() . '/' . $options->getHost()) : null, 'qop' => $qop, - 'nonce_size' => $options['nonce_size'], - 'service' => $options['service'], + 'nonce_size' => $options->getNonceSize(), + 'service' => $options->getService(), ]; - if (isset($options['cnonce'])) { - $messageOpts['cnonce'] = $options['cnonce']; + if ($options->getCnonce() !== null) { + $messageOpts['cnonce'] = $options->getCnonce(); } - if (isset($options['nonce'])) { - $messageOpts['nonce'] = $options['nonce']; + if ($options->getNonce() !== null) { + $messageOpts['nonce'] = $options->getNonce(); } - if (isset($options['cipher'])) { - $messageOpts['cipher'] = $options['cipher']; + if ($options->getCipher() !== null) { + $messageOpts['cipher'] = $options->getCipher(); } $response = $this->factory->create( DigestMD5MessageType::CLIENT_RESPONSE, @@ -184,7 +175,7 @@ private function createClientResponse( ); # The A1 / cipher value is used in the security layer. - if ($options['use_integrity'] || $options['use_privacy']) { + if ($options->isUseIntegrity() || $options->isUsePrivacy()) { $this->context->set('a1', hex2bin(DigestMD5Mechanism::computeA1($password, $message, $response))); $this->context->set('cipher', $response->get('cipher')); } @@ -193,13 +184,11 @@ private function createClientResponse( } /** - * @param array $options - * * @throws SaslException */ private function generateServerVerification( Message $received, - array $options, + DigestMD5Options $options, ): ?Message { $this->context->setIsComplete(true); # The client sent a response without us sending a challenge... @@ -209,7 +198,7 @@ private function generateServerVerification( # @todo This should accept some kind of computed value, like the a1. Then it could generate the other values # using that. - $password = $options['password'] ?? null; + $password = $options->getPassword(); $qop = $received->get('qop'); $cipher = $received->get('cipher'); if ($password === null) { @@ -256,15 +245,21 @@ private function generateServerVerification( } /** - * @param array $options - * * @throws SaslException */ - private function generateServerChallenge(array $options): Message + private function generateServerChallenge(DigestMD5Options $options): Message { $this->challenge = $this->factory->create( DigestMD5MessageType::SERVER_CHALLENGE, - $options, + [ + 'use_integrity' => $options->isUseIntegrity(), + 'use_privacy' => $options->isUsePrivacy(), + 'nonce' => $options->getNonce(), + 'nonce_size' => $options->getNonceSize(), + 'realm' => $options->getRealm(), + 'maxbuf' => $options->getMaxbuf(), + 'cipher' => $options->getCipher(), + ], ); return $this->challenge; diff --git a/src/FreeDSx/Sasl/Challenge/PlainChallenge.php b/src/FreeDSx/Sasl/Challenge/PlainChallenge.php index 47ab7bb..c21ebea 100644 --- a/src/FreeDSx/Sasl/Challenge/PlainChallenge.php +++ b/src/FreeDSx/Sasl/Challenge/PlainChallenge.php @@ -16,6 +16,8 @@ use FreeDSx\Sasl\Encoder\PlainEncoder; use FreeDSx\Sasl\Exception\SaslException; use FreeDSx\Sasl\Message; +use FreeDSx\Sasl\Options\ChallengeOptionsInterface; +use FreeDSx\Sasl\Options\PlainOptions; use FreeDSx\Sasl\SaslContext; /** @@ -25,9 +27,11 @@ */ final readonly class PlainChallenge implements ChallengeInterface { - private readonly PlainEncoder $encoder; + use ResolvesOptionsTrait; - private readonly SaslContext $context; + private PlainEncoder $encoder; + + private SaslContext $context; public function __construct(bool $isServerMode = false) { @@ -38,28 +42,30 @@ public function __construct(bool $isServerMode = false) public function challenge( ?string $received = null, - array $options = [], + ?ChallengeOptionsInterface $options = null, ): SaslContext { + $resolved = $this->resolveOptions( + $options, + PlainOptions::class, + ); $message = $received === null ? null : $this->encoder->decode($received, $this->context); return $this->context->isServerMode() - ? $this->serverProcess($message, $options) - : $this->clientProcess($options); + ? $this->serverProcess($message, $resolved) + : $this->clientProcess($resolved); } /** - * @param array $options - * * @throws SaslException */ private function serverProcess( ?Message $message, - array $options, + PlainOptions $options, ): SaslContext { if ($message === null) { return $this->context; } - if (!isset($options['validate']) || !is_callable($options['validate'])) { + if ($options->getValidate() === null) { throw new SaslException('You must pass a callable validate option to the plain mechanism in server mode.'); } $authzId = $message->get('authzid'); @@ -67,28 +73,26 @@ private function serverProcess( $password = $message->get('password'); $this->context->setIsComplete(true); - $this->context->setIsAuthenticated((bool) $options['validate']($authzId, $authcId, $password)); + $this->context->setIsAuthenticated((bool) ($options->getValidate())($authzId, $authcId, $password)); return $this->context; } /** - * @param array $options - * * @throws SaslException */ - private function clientProcess(array $options): SaslContext + private function clientProcess(PlainOptions $options): SaslContext { - if (!isset($options['username'])) { + if ($options->getUsername() === null) { throw new SaslException('You must supply a username for the PLAIN mechanism.'); } - if (!isset($options['password'])) { + if ($options->getPassword() === null) { throw new SaslException('You must supply a password for the PLAIN mechanism.'); } $message = new Message([ - 'authzid' => $options['username'], - 'authcid' => $options['username'], - 'password' => $options['password'], + 'authzid' => $options->getUsername(), + 'authcid' => $options->getUsername(), + 'password' => $options->getPassword(), ]); $this->context->setResponse($this->encoder->encode($message, $this->context)); $this->context->setIsComplete(true); diff --git a/src/FreeDSx/Sasl/Challenge/ResolvesOptionsTrait.php b/src/FreeDSx/Sasl/Challenge/ResolvesOptionsTrait.php new file mode 100644 index 0000000..2feb211 --- /dev/null +++ b/src/FreeDSx/Sasl/Challenge/ResolvesOptionsTrait.php @@ -0,0 +1,44 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Sasl\Challenge; + +use FreeDSx\Sasl\Exception\SaslException; +use FreeDSx\Sasl\Options\ChallengeOptionsInterface; + +trait ResolvesOptionsTrait +{ + /** + * @template T of ChallengeOptionsInterface + * + * @param class-string $expectedClass + * + * @return T + * + * @throws SaslException + */ + private function resolveOptions( + ?ChallengeOptionsInterface $options, + string $expectedClass, + ): ChallengeOptionsInterface { + if ($options !== null && !$options instanceof $expectedClass) { + throw new SaslException(sprintf( + 'Expected %s, got %s.', + $expectedClass, + $options::class, + )); + } + + return $options; + } +} diff --git a/src/FreeDSx/Sasl/Challenge/ScramChallenge.php b/src/FreeDSx/Sasl/Challenge/ScramChallenge.php index a6ed865..2da5441 100644 --- a/src/FreeDSx/Sasl/Challenge/ScramChallenge.php +++ b/src/FreeDSx/Sasl/Challenge/ScramChallenge.php @@ -18,6 +18,8 @@ use FreeDSx\Sasl\Factory\NonceTrait; use FreeDSx\Sasl\Mechanism\HashAlgorithm; use FreeDSx\Sasl\Message; +use FreeDSx\Sasl\Options\ChallengeOptionsInterface; +use FreeDSx\Sasl\Options\ScramOptions; use FreeDSx\Sasl\SaslContext; use FreeDSx\Sasl\SaslPrep; @@ -41,6 +43,7 @@ final readonly class ScramChallenge implements ChallengeInterface { use NonceTrait; + use ResolvesOptionsTrait; /** * Default nonce size in bytes. Produces ~32 base64 characters; well above the RFC 5802 minimum. @@ -121,15 +124,20 @@ public function __construct( public function challenge( ?string $received = null, - array $options = [], + ?ChallengeOptionsInterface $options = null, ): SaslContext { + $resolved = $this->resolveOptions( + $options, + ScramOptions::class, + ); + # Keep the raw received string for auth-message construction before decoding. $rawReceived = $received; $message = $received !== null ? $this->encoder->decode($received, $this->context) : null; $response = $this->context->isServerMode() - ? $this->generateServerResponse($message, $rawReceived, $options) - : $this->generateClientResponse($message, $rawReceived, $options); + ? $this->generateServerResponse($message, $rawReceived, $resolved) + : $this->generateClientResponse($message, $rawReceived, $resolved); $this->context->setResponse($response); @@ -137,14 +145,12 @@ public function challenge( } /** - * @param array $options - * * @throws SaslException */ private function generateClientResponse( ?Message $received, ?string $rawReceived, - array $options, + ScramOptions $options, ): ?string { # Step 1: No message yet — generate client-first-message. if ($received === null) { @@ -167,14 +173,12 @@ private function generateClientResponse( } /** - * @param array $options - * * @throws SaslException */ private function generateServerResponse( ?Message $received, ?string $rawReceived, - array $options, + ScramOptions $options, ): ?string { # SCRAM is client-initiated — the server has no opening message. if ($received === null) { @@ -195,24 +199,22 @@ private function generateServerResponse( } /** - * @param array{username?: string, cnonce?: string, cbind_type?: string} $options - * * @throws SaslException */ - private function generateClientFirst(array $options): string + private function generateClientFirst(ScramOptions $options): string { - $username = $options['username'] ?? null; + $username = $options->getUsername(); if ($username === null) { throw new SaslException('A username is required to initiate SCRAM authentication.'); } - $cnonce = $options['cnonce'] ?? $this->generateNonce(self::NONCE_SIZE); + $cnonce = $options->getCnonce() ?? $this->generateNonce(self::NONCE_SIZE); if ($cnonce === '') { throw new SaslException('The client nonce must not be empty.'); } if ($this->isChannelBinding) { - $cbindType = $options['cbind_type'] ?? 'tls-unique'; + $cbindType = $options->getCbindType() ?? 'tls-unique'; $this->validateCbindType($cbindType); $gs2Header = 'p=' . $cbindType . ',,'; } else { @@ -235,16 +237,14 @@ private function generateClientFirst(array $options): string } /** - * @param array{password?: string, cbind_data?: string} $options - * * @throws SaslException */ private function generateClientFinal( Message $serverFirst, string $rawServerFirst, - array $options, + ScramOptions $options, ): string { - $password = $options['password'] ?? null; + $password = $options->getPassword(); if ($password === null) { throw new SaslException('A password is required to complete SCRAM authentication.'); } @@ -263,7 +263,7 @@ private function generateClientFinal( # Channel binding value: base64(gs2-header + cbind-data). $gs2Header = (string) $this->context->get(self::CTX_GS2_HEADER); - $cbindData = $options['cbind_data'] ?? ''; + $cbindData = $options->getCbindData() ?? ''; $channelBinding = base64_encode($gs2Header . $cbindData); $clientFinalWithoutProof = 'c=' . $channelBinding . ',r=' . $fullNonce; @@ -312,21 +312,19 @@ private function verifyServerFinal(Message $serverFinal): void } /** - * @param array{nonce?: string, salt?: string, iterations?: int} $options - * * @throws SaslException */ private function generateServerFirst( Message $clientFirst, string $rawClientFirst, - array $options, + ScramOptions $options, ): string { $cnonce = (string) $clientFirst->get('r'); - $snonce = $options['nonce'] ?? $this->generateNonce(self::NONCE_SIZE); + $snonce = $options->getNonce() ?? $this->generateNonce(self::NONCE_SIZE); $fullNonce = $cnonce . $snonce; - $salt = $options['salt'] ?? random_bytes(16); - $iterations = (int) ($options['iterations'] ?? self::DEFAULT_ITERATIONS[$this->hashAlgorithm->value]); + $salt = $options->getSalt() ?? random_bytes(16); + $iterations = $options->getIterations() ?? self::DEFAULT_ITERATIONS[$this->hashAlgorithm->value]; if ($iterations < 1) { throw new SaslException('The iteration count must be greater than zero.'); } @@ -353,17 +351,15 @@ private function generateServerFirst( /** * Returns 'v=' on success, or 'e=' on failure. * - * @param array{password?: string} $options - * * @throws SaslException */ private function generateServerFinal( Message $clientFinal, - array $options, + ScramOptions $options, ): string { $this->context->setIsComplete(true); - $password = $options['password'] ?? null; + $password = $options->getPassword(); if ($password === null) { return self::INVALID_PROOF; } diff --git a/src/FreeDSx/Sasl/MechanismSelector.php b/src/FreeDSx/Sasl/MechanismSelector.php index f2c6f6c..682b3b2 100644 --- a/src/FreeDSx/Sasl/MechanismSelector.php +++ b/src/FreeDSx/Sasl/MechanismSelector.php @@ -16,6 +16,7 @@ use FreeDSx\Sasl\Exception\SaslException; use FreeDSx\Sasl\Mechanism\MechanismInterface; use FreeDSx\Sasl\Mechanism\MechanismName; +use FreeDSx\Sasl\Options\SelectOptions; /** * Given an array of mechanism names, choose the best one available. @@ -33,13 +34,12 @@ public function __construct(private readonly array $mechanisms) /** * @param MechanismName[] $choices - * @param array $options * * @throws SaslException */ public function select( array $choices = [], - array $options = [], + ?SelectOptions $options = null, ): MechanismInterface { $available = $this->getAvailableMechsFromChoices($choices, $options); @@ -97,7 +97,6 @@ static function (MechanismInterface $a, MechanismInterface $b): int { /** * @param MechanismName[] $choices - * @param array $options * * @return MechanismInterface[] * @@ -105,7 +104,7 @@ static function (MechanismInterface $a, MechanismInterface $b): int { */ private function getAvailableMechsFromChoices( array $choices, - array $options, + ?SelectOptions $options, ): array { $available = $this->filterFromChoices($choices); if (count($available) === 0) { @@ -143,16 +142,15 @@ private function filterFromChoices(array $choices): array /** * @param MechanismInterface[] $available - * @param array $options * * @return MechanismInterface[] */ private function filterOptions( array $available, - array $options, + ?SelectOptions $options, ): array { - $useIntegrity = (bool) ($options['use_integrity'] ?? false); - $usePrivacy = (bool) ($options['use_privacy'] ?? false); + $useIntegrity = $options?->isUseIntegrity() ?? false; + $usePrivacy = $options?->isUsePrivacy() ?? false; # Don't need to worry whether it supports integrity or privacy... if (!$useIntegrity && !$usePrivacy) { diff --git a/src/FreeDSx/Sasl/Options/AnonymousOptions.php b/src/FreeDSx/Sasl/Options/AnonymousOptions.php new file mode 100644 index 0000000..8298605 --- /dev/null +++ b/src/FreeDSx/Sasl/Options/AnonymousOptions.php @@ -0,0 +1,36 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Sasl\Options; + +/** + * Options for the ANONYMOUS challenge. + * + * @author Chad Sikorra + */ +final class AnonymousOptions implements ChallengeOptionsInterface +{ + private ?string $trace = null; + + public function getTrace(): ?string + { + return $this->trace; + } + + public function setTrace(?string $trace): self + { + $this->trace = $trace; + + return $this; + } +} diff --git a/src/FreeDSx/Sasl/Options/ChallengeOptionsInterface.php b/src/FreeDSx/Sasl/Options/ChallengeOptionsInterface.php new file mode 100644 index 0000000..5397071 --- /dev/null +++ b/src/FreeDSx/Sasl/Options/ChallengeOptionsInterface.php @@ -0,0 +1,23 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Sasl\Options; + +/** + * Marker interface for challenge options DTOs. + * + * @author Chad Sikorra + */ +interface ChallengeOptionsInterface +{ +} diff --git a/src/FreeDSx/Sasl/Options/CramMD5Options.php b/src/FreeDSx/Sasl/Options/CramMD5Options.php new file mode 100644 index 0000000..49148ca --- /dev/null +++ b/src/FreeDSx/Sasl/Options/CramMD5Options.php @@ -0,0 +1,94 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Sasl\Options; + +use Closure; + +/** + * Options for the CRAM-MD5 challenge. + * + * Client mode: set username and password. + * + * Server mode (initial): set challenge to override the auto-generated nonce. + * Server mode (validation): set passwordCallback to verify the client digest. + * + * @author Chad Sikorra + */ +final class CramMD5Options implements ChallengeOptionsInterface +{ + private ?string $username = null; + + private ?string $password = null; + + private ?string $challenge = null; + + /** + * @var (Closure(string $username, string $challenge): string)|null + */ + private ?Closure $passwordCallback = null; + + public function getUsername(): ?string + { + return $this->username; + } + + public function setUsername(string $username): self + { + $this->username = $username; + + return $this; + } + + public function getPassword(): ?string + { + return $this->password; + } + + public function setPassword(string $password): self + { + $this->password = $password; + + return $this; + } + + public function getChallenge(): ?string + { + return $this->challenge; + } + + public function setChallenge(string $challenge): self + { + $this->challenge = $challenge; + + return $this; + } + + /** + * @return (Closure(string $username, string $challenge): string)|null + */ + public function getPasswordCallback(): ?Closure + { + return $this->passwordCallback; + } + + /** + * @param Closure(string $username, string $challenge): string $passwordCallback + */ + public function setPasswordCallback(Closure $passwordCallback): self + { + $this->passwordCallback = $passwordCallback; + + return $this; + } +} diff --git a/src/FreeDSx/Sasl/Options/DigestMD5Options.php b/src/FreeDSx/Sasl/Options/DigestMD5Options.php new file mode 100644 index 0000000..cc3ad2f --- /dev/null +++ b/src/FreeDSx/Sasl/Options/DigestMD5Options.php @@ -0,0 +1,190 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Sasl\Options; + +/** + * Options for the DIGEST-MD5 challenge. + * + * @author Chad Sikorra + */ +final class DigestMD5Options implements ChallengeOptionsInterface +{ + private bool $useIntegrity = false; + + private bool $usePrivacy = false; + + private string $service = 'ldap'; + + private ?int $nonceSize = null; + + private ?string $username = null; + + private ?string $password = null; + + private ?string $host = null; + + private ?string $realm = null; + + private ?string $cnonce = null; + + private ?string $nonce = null; + + private ?string $cipher = null; + + private ?string $maxbuf = null; + + public function isUseIntegrity(): bool + { + return $this->useIntegrity; + } + + public function setUseIntegrity(bool $useIntegrity): self + { + $this->useIntegrity = $useIntegrity; + + return $this; + } + + public function isUsePrivacy(): bool + { + return $this->usePrivacy; + } + + public function setUsePrivacy(bool $usePrivacy): self + { + $this->usePrivacy = $usePrivacy; + + return $this; + } + + public function getService(): string + { + return $this->service; + } + + public function setService(string $service): self + { + $this->service = $service; + + return $this; + } + + public function getNonceSize(): ?int + { + return $this->nonceSize; + } + + public function setNonceSize(int $nonceSize): self + { + $this->nonceSize = $nonceSize; + + return $this; + } + + public function getUsername(): ?string + { + return $this->username; + } + + public function setUsername(string $username): self + { + $this->username = $username; + + return $this; + } + + public function getPassword(): ?string + { + return $this->password; + } + + public function setPassword(string $password): self + { + $this->password = $password; + + return $this; + } + + public function getHost(): ?string + { + return $this->host; + } + + public function setHost(string $host): self + { + $this->host = $host; + + return $this; + } + + public function getRealm(): ?string + { + return $this->realm; + } + + public function setRealm(string $realm): self + { + $this->realm = $realm; + + return $this; + } + + public function getCnonce(): ?string + { + return $this->cnonce; + } + + public function setCnonce(string $cnonce): self + { + $this->cnonce = $cnonce; + + return $this; + } + + public function getNonce(): ?string + { + return $this->nonce; + } + + public function setNonce(string $nonce): self + { + $this->nonce = $nonce; + + return $this; + } + + public function getCipher(): ?string + { + return $this->cipher; + } + + public function setCipher(string $cipher): self + { + $this->cipher = $cipher; + + return $this; + } + + public function getMaxbuf(): ?string + { + return $this->maxbuf; + } + + public function setMaxbuf(string $maxbuf): self + { + $this->maxbuf = $maxbuf; + + return $this; + } +} diff --git a/src/FreeDSx/Sasl/Options/PlainOptions.php b/src/FreeDSx/Sasl/Options/PlainOptions.php new file mode 100644 index 0000000..77d23ef --- /dev/null +++ b/src/FreeDSx/Sasl/Options/PlainOptions.php @@ -0,0 +1,78 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Sasl\Options; + +use Closure; + +/** + * Options for the PLAIN challenge. + * + * Client mode: set username and password. + * Server mode: set the validate closure. + * + * @author Chad Sikorra + */ +final class PlainOptions implements ChallengeOptionsInterface +{ + private ?string $username = null; + + private ?string $password = null; + + /** + * @var (Closure(?string $authzId, string $authcId, string $password): bool)|null + */ + private ?Closure $validate = null; + + public function getUsername(): ?string + { + return $this->username; + } + + public function setUsername(string $username): self + { + $this->username = $username; + + return $this; + } + + public function getPassword(): ?string + { + return $this->password; + } + + public function setPassword(string $password): self + { + $this->password = $password; + + return $this; + } + + /** + * @return (Closure(?string $authzId, string $authcId, string $password): bool)|null + */ + public function getValidate(): ?Closure + { + return $this->validate; + } + + /** + * @param Closure(?string $authzId, string $authcId, string $password): bool $validate + */ + public function setValidate(Closure $validate): self + { + $this->validate = $validate; + + return $this; + } +} diff --git a/src/FreeDSx/Sasl/Options/SaslOptions.php b/src/FreeDSx/Sasl/Options/SaslOptions.php new file mode 100644 index 0000000..cbf48b7 --- /dev/null +++ b/src/FreeDSx/Sasl/Options/SaslOptions.php @@ -0,0 +1,49 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Sasl\Options; + +use FreeDSx\Sasl\Mechanism\MechanismName; + +/** + * Top-level options for the Sasl class. + * + * @author Chad Sikorra + */ +final class SaslOptions +{ + /** + * @param MechanismName[] $supported Limit available mechanisms to this set (empty = all supported). + */ + public function __construct(private array $supported = []) + { + } + + /** + * @return MechanismName[] + */ + public function getSupported(): array + { + return $this->supported; + } + + /** + * @param MechanismName[] $supported + */ + public function setSupported(array $supported): self + { + $this->supported = $supported; + + return $this; + } +} diff --git a/src/FreeDSx/Sasl/Options/ScramOptions.php b/src/FreeDSx/Sasl/Options/ScramOptions.php new file mode 100644 index 0000000..d30ca5e --- /dev/null +++ b/src/FreeDSx/Sasl/Options/ScramOptions.php @@ -0,0 +1,139 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Sasl\Options; + +/** + * Options for the SCRAM challenge. + * + * Client-first: set username (required), and optionally cnonce and cbindType. + * Client-final: set password (required), and optionally cbindData. + * Server-first: optionally set nonce, salt, and iterations. + * Server-final: set password (required for verification). + * + * @author Chad Sikorra + */ +final class ScramOptions implements ChallengeOptionsInterface +{ + private ?string $username = null; + + private ?string $cnonce = null; + + private ?string $cbindType = null; + + private ?string $password = null; + + private ?string $cbindData = null; + + private ?string $nonce = null; + + private ?string $salt = null; + + private ?int $iterations = null; + + public function getUsername(): ?string + { + return $this->username; + } + + public function setUsername(string $username): self + { + $this->username = $username; + + return $this; + } + + public function getCnonce(): ?string + { + return $this->cnonce; + } + + public function setCnonce(string $cnonce): self + { + $this->cnonce = $cnonce; + + return $this; + } + + public function getCbindType(): ?string + { + return $this->cbindType; + } + + public function setCbindType(string $cbindType): self + { + $this->cbindType = $cbindType; + + return $this; + } + + public function getPassword(): ?string + { + return $this->password; + } + + public function setPassword(string $password): self + { + $this->password = $password; + + return $this; + } + + public function getCbindData(): ?string + { + return $this->cbindData; + } + + public function setCbindData(string $cbindData): self + { + $this->cbindData = $cbindData; + + return $this; + } + + public function getNonce(): ?string + { + return $this->nonce; + } + + public function setNonce(string $nonce): self + { + $this->nonce = $nonce; + + return $this; + } + + public function getSalt(): ?string + { + return $this->salt; + } + + public function setSalt(string $salt): self + { + $this->salt = $salt; + + return $this; + } + + public function getIterations(): ?int + { + return $this->iterations; + } + + public function setIterations(int $iterations): self + { + $this->iterations = $iterations; + + return $this; + } +} diff --git a/src/FreeDSx/Sasl/Options/SelectOptions.php b/src/FreeDSx/Sasl/Options/SelectOptions.php new file mode 100644 index 0000000..0f7e258 --- /dev/null +++ b/src/FreeDSx/Sasl/Options/SelectOptions.php @@ -0,0 +1,50 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Sasl\Options; + +/** + * Options controlling which mechanism is selected. + * + * @author Chad Sikorra + */ +final class SelectOptions +{ + private bool $useIntegrity = false; + + private bool $usePrivacy = false; + + public function isUseIntegrity(): bool + { + return $this->useIntegrity; + } + + public function setUseIntegrity(bool $useIntegrity): self + { + $this->useIntegrity = $useIntegrity; + + return $this; + } + + public function isUsePrivacy(): bool + { + return $this->usePrivacy; + } + + public function setUsePrivacy(bool $usePrivacy): self + { + $this->usePrivacy = $usePrivacy; + + return $this; + } +} diff --git a/src/FreeDSx/Sasl/Sasl.php b/src/FreeDSx/Sasl/Sasl.php index 3e00014..167f7cd 100644 --- a/src/FreeDSx/Sasl/Sasl.php +++ b/src/FreeDSx/Sasl/Sasl.php @@ -21,6 +21,8 @@ use FreeDSx\Sasl\Mechanism\MechanismName; use FreeDSx\Sasl\Mechanism\PlainMechanism; use FreeDSx\Sasl\Mechanism\ScramMechanism; +use FreeDSx\Sasl\Options\SaslOptions; +use FreeDSx\Sasl\Options\SelectOptions; /** * The main SASL class. @@ -34,17 +36,8 @@ final class Sasl */ private array $mechanisms = []; - /** - * @var array{supported: MechanismName[]} - */ - private array $options; - - /** - * @param array{supported?: MechanismName[]} $options - */ - public function __construct(array $options = []) + public function __construct(private readonly SaslOptions $options = new SaslOptions()) { - $this->options = $options + ['supported' => []]; $this->initMechs(); } @@ -95,16 +88,15 @@ public function remove(MechanismName $mechanism): self } /** - * Given an array of mechanism names, and optional options, select the best supported mechanism available. + * Given an array of mechanism names, and optional selection options, select the best supported mechanism available. * * @param MechanismName[] $choices array of mechanisms by their name - * @param array $options array of options (ie. ['use_integrity' => true]) * * @throws SaslException */ public function select( array $choices = [], - array $options = [], + ?SelectOptions $options = null, ): MechanismInterface { return (new MechanismSelector($this->mechanisms)) ->select($choices, $options); @@ -133,13 +125,13 @@ private function initMechs(): void } } - if (count($this->options['supported']) === 0) { + if (count($this->options->getSupported()) === 0) { return; } $allowed = array_map( static fn (MechanismName $name): string => $name->value, - $this->options['supported'], + $this->options->getSupported(), ); foreach (array_keys($this->mechanisms) as $key) { if (!in_array($key, $allowed, true)) { diff --git a/tests/unit/Challenge/AnonymousChallengeTest.php b/tests/unit/Challenge/AnonymousChallengeTest.php index f0703e9..d9daa1f 100644 --- a/tests/unit/Challenge/AnonymousChallengeTest.php +++ b/tests/unit/Challenge/AnonymousChallengeTest.php @@ -14,6 +14,7 @@ namespace Tests\Unit\FreeDSx\Sasl\Challenge; use FreeDSx\Sasl\Challenge\AnonymousChallenge; +use FreeDSx\Sasl\Options\AnonymousOptions; use PHPUnit\Framework\TestCase; final class AnonymousChallengeTest extends TestCase @@ -27,7 +28,7 @@ protected function setUp(): void public function testTheClientChallenge(): void { - $context = $this->challenge->challenge(null, ['username' => 'foo', 'password' => 'bar']); + $context = $this->challenge->challenge(null, (new AnonymousOptions())->setTrace('foo')); self::assertSame('foo', $context->getResponse()); self::assertTrue($context->isComplete()); @@ -36,7 +37,7 @@ public function testTheClientChallenge(): void public function testTheClientChallengeWithSpecificTrace(): void { - $context = $this->challenge->challenge(null, ['trace' => 'foobar', 'password' => 'bar']); + $context = $this->challenge->challenge(null, (new AnonymousOptions())->setTrace('foobar')); self::assertSame('foobar', $context->getResponse()); self::assertTrue($context->isComplete()); diff --git a/tests/unit/Challenge/CramMD5ChallengeTest.php b/tests/unit/Challenge/CramMD5ChallengeTest.php index b94f51f..e5124f1 100644 --- a/tests/unit/Challenge/CramMD5ChallengeTest.php +++ b/tests/unit/Challenge/CramMD5ChallengeTest.php @@ -13,7 +13,9 @@ namespace Tests\Unit\FreeDSx\Sasl\Challenge; +use Closure; use FreeDSx\Sasl\Challenge\CramMD5Challenge; +use FreeDSx\Sasl\Options\CramMD5Options; use PHPUnit\Framework\TestCase; final class CramMD5ChallengeTest extends TestCase @@ -27,7 +29,10 @@ protected function setUp(): void public function testChallengeWithFromClientWithServerChallenge(): void { - $context = $this->challenge->challenge('', ['username' => 'foo', 'password' => 'bar']); + $context = $this->challenge->challenge( + '', + (new CramMD5Options())->setUsername('foo')->setPassword('bar'), + ); self::assertSame('foo e23c893e9de272d4a75e646265768a45', $context->getResponse()); self::assertTrue($context->isComplete()); @@ -38,7 +43,10 @@ public function testChallengeWithFromServerWithClientWrongResponse(): void $serverChallenge = new CramMD5Challenge(true); $validate = static fn (string $username, string $challenge): string => ''; $serverChallenge->challenge(); - $context = $serverChallenge->challenge('foo e23c893e9de272d4a75e646265768a45', ['password' => $validate]); + $context = $serverChallenge->challenge( + 'foo e23c893e9de272d4a75e646265768a45', + (new CramMD5Options())->setPasswordCallback(Closure::fromCallable($validate)), + ); self::assertFalse($context->isAuthenticated()); self::assertTrue($context->isComplete()); @@ -49,8 +57,11 @@ public function testChallengeWithFromServerWithClientCorrectResponse(): void $serverChallenge = new CramMD5Challenge(true); $validate = static fn (string $username, string $challenge): string => hash_hmac('md5', $challenge, 'bar'); // The challenge option is the raw nonce; the encoder wraps it as . - $serverChallenge->challenge(null, ['challenge' => 'foobar']); - $context = $serverChallenge->challenge('foo e23c893e9de272d4a75e646265768a45', ['password' => $validate]); + $serverChallenge->challenge(null, (new CramMD5Options())->setChallenge('foobar')); + $context = $serverChallenge->challenge( + 'foo e23c893e9de272d4a75e646265768a45', + (new CramMD5Options())->setPasswordCallback(Closure::fromCallable($validate)), + ); self::assertTrue($context->isAuthenticated()); self::assertTrue($context->isComplete()); @@ -68,7 +79,7 @@ public function testPasswordCallableReceivesEncodedChallengeMatchingWhatClientUs }; // Server generates challenge with raw nonce 'foobar'; encoder sends '' to client. - $serverContext = $serverChallenge->challenge(null, ['challenge' => 'foobar']); + $serverContext = $serverChallenge->challenge(null, (new CramMD5Options())->setChallenge('foobar')); $encodedChallenge = $serverContext->getResponse(); self::assertSame('', $encodedChallenge); @@ -76,13 +87,13 @@ public function testPasswordCallableReceivesEncodedChallengeMatchingWhatClientUs $clientChallenge = new CramMD5Challenge(false); $clientContext = $clientChallenge->challenge( $encodedChallenge, - ['username' => 'foo', 'password' => 'bar'], + (new CramMD5Options())->setUsername('foo')->setPassword('bar'), ); // Server validates the client response. $serverChallenge->challenge( $clientContext->getResponse(), - ['password' => $validate], + (new CramMD5Options())->setPasswordCallback(Closure::fromCallable($validate)), ); // The callable must receive the encoded form so it can compute the same digest as the client. diff --git a/tests/unit/Challenge/DigestMD5ChallengeTest.php b/tests/unit/Challenge/DigestMD5ChallengeTest.php index 335f9fa..51839e2 100644 --- a/tests/unit/Challenge/DigestMD5ChallengeTest.php +++ b/tests/unit/Challenge/DigestMD5ChallengeTest.php @@ -16,6 +16,7 @@ use FreeDSx\Sasl\Challenge\DigestMD5Challenge; use FreeDSx\Sasl\Encoder\DigestMD5Encoder; use FreeDSx\Sasl\Exception\SaslException; +use FreeDSx\Sasl\Options\DigestMD5Options; use FreeDSx\Sasl\SaslContext; use PHPUnit\Framework\TestCase; @@ -43,7 +44,10 @@ protected function setUp(): void public function testThatClientResponseIsGeneratedFromChallenge(): void { $response = $this->encoder->decode( - (string) $this->challenge->challenge($this->challengeData, ['use_privacy' => true])->getResponse(), + (string) $this->challenge->challenge( + $this->challengeData, + (new DigestMD5Options())->setUsePrivacy(true), + )->getResponse(), (new SaslContext())->setIsServerMode(true), ); @@ -56,7 +60,7 @@ public function testThatClientResponseUsesSpecificHostForDigestUriIfRequested(): $response = $this->encoder->decode( (string) $this->challenge->challenge( $this->challengeData, - ['use_privacy' => true, 'host' => 'foo.bar.local'], + (new DigestMD5Options())->setUsePrivacy(true)->setHost('foo.bar.local'), )->getResponse(), (new SaslContext())->setIsServerMode(true), ); @@ -66,20 +70,16 @@ public function testThatClientResponseUsesSpecificHostForDigestUriIfRequested(): public function testSecurityLayerIsInitializedProperlyInTheContext(): void { - $options = [ - 'use_privacy' => true, - 'cipher' => 'rc4', - 'username' => 'WillifoA', - 'password' => 'Password1', - ]; - $this->challenge->challenge( - $this->challengeData, - ['cnonce' => 'tQvBPuDOMTE=', 'nonce' => 'Zzk0ux7KgOVPmN7dLofGm9KqNesbnCXRcIAQSmxuQEk='] + $options, - ); - $context = $this->challenge->challenge( - $this->rspAuthSuccess, - ['cnonce' => 'tQvBPuDOMTE=', 'nonce' => 'Zzk0ux7KgOVPmN7dLofGm9KqNesbnCXRcIAQSmxuQEk='] + $options, - ); + $options = (new DigestMD5Options()) + ->setUsePrivacy(true) + ->setCipher('rc4') + ->setUsername('WillifoA') + ->setPassword('Password1') + ->setCnonce('tQvBPuDOMTE=') + ->setNonce('Zzk0ux7KgOVPmN7dLofGm9KqNesbnCXRcIAQSmxuQEk='); + + $this->challenge->challenge($this->challengeData, $options); + $context = $this->challenge->challenge($this->rspAuthSuccess, $options); self::assertTrue($context->isAuthenticated(), 'Context should be authenticated, but was not.'); self::assertTrue($context->hasSecurityLayer(), 'Context should have a security layer, but it does not.'); @@ -107,7 +107,10 @@ public function testGenerateServerChallengeForClientInServerMode(): void { $serverChallenge = new DigestMD5Challenge(true); $challenge = $this->encoder->decode( - (string) $serverChallenge->challenge(null, ['use_integrity' => true, 'use_privacy' => true])->getResponse(), + (string) $serverChallenge->challenge( + null, + (new DigestMD5Options())->setUseIntegrity(true)->setUsePrivacy(true), + )->getResponse(), new SaslContext(), ); diff --git a/tests/unit/Challenge/PlainChallengeTest.php b/tests/unit/Challenge/PlainChallengeTest.php index ce2dc08..485b698 100644 --- a/tests/unit/Challenge/PlainChallengeTest.php +++ b/tests/unit/Challenge/PlainChallengeTest.php @@ -13,7 +13,9 @@ namespace Tests\Unit\FreeDSx\Sasl\Challenge; +use Closure; use FreeDSx\Sasl\Challenge\PlainChallenge; +use FreeDSx\Sasl\Options\PlainOptions; use PHPUnit\Framework\TestCase; final class PlainChallengeTest extends TestCase @@ -27,7 +29,10 @@ protected function setUp(): void public function testTheClientChallenge(): void { - $context = $this->challenge->challenge(null, ['username' => 'foo', 'password' => 'bar']); + $context = $this->challenge->challenge( + null, + (new PlainOptions())->setUsername('foo')->setPassword('bar'), + ); self::assertSame("foo\x00foo\x00bar", $context->getResponse()); self::assertTrue($context->isComplete()); @@ -38,7 +43,10 @@ public function testTheServerResponseToTheClientWhenSuccessful(): void $serverChallenge = new PlainChallenge(true); $validate = static fn (string $authzid, string $authcid, string $password): bool => true; - $context = $serverChallenge->challenge("foo\x00foo\x00bar", ['validate' => $validate]); + $context = $serverChallenge->challenge( + "foo\x00foo\x00bar", + (new PlainOptions())->setValidate(Closure::fromCallable($validate)), + ); self::assertTrue($context->isComplete()); self::assertTrue($context->isAuthenticated()); @@ -50,7 +58,10 @@ public function testTheServerResponseToTheClientWhenNotSuccessful(): void $serverChallenge = new PlainChallenge(true); $validate = static fn (string $authzid, string $authcid, string $password): bool => false; - $context = $serverChallenge->challenge("foo\x00foo\x00bar", ['validate' => $validate]); + $context = $serverChallenge->challenge( + "foo\x00foo\x00bar", + (new PlainOptions())->setValidate(Closure::fromCallable($validate)), + ); self::assertTrue($context->isComplete()); self::assertFalse($context->isAuthenticated()); diff --git a/tests/unit/Challenge/ScramChallengeTest.php b/tests/unit/Challenge/ScramChallengeTest.php index ab03106..1933d24 100644 --- a/tests/unit/Challenge/ScramChallengeTest.php +++ b/tests/unit/Challenge/ScramChallengeTest.php @@ -16,6 +16,7 @@ use FreeDSx\Sasl\Challenge\ScramChallenge; use FreeDSx\Sasl\Exception\SaslException; use FreeDSx\Sasl\Mechanism\HashAlgorithm; +use FreeDSx\Sasl\Options\ScramOptions; use PHPUnit\Framework\TestCase; /** @@ -59,10 +60,10 @@ protected function setUp(): void public function testClientFirstMessageMatchesRfcVector(): void { - $context = $this->challenge->challenge(null, [ - 'username' => self::USERNAME, - 'cnonce' => self::CNONCE, - ]); + $context = $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE), + ); self::assertSame(self::CLIENT_FIRST, $context->getResponse()); self::assertFalse($context->isComplete()); @@ -70,12 +71,15 @@ public function testClientFirstMessageMatchesRfcVector(): void public function testClientFinalMessageMatchesRfcVector(): void { - $this->challenge->challenge(null, [ - 'username' => self::USERNAME, - 'cnonce' => self::CNONCE, - ]); + $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE), + ); - $context = $this->challenge->challenge(self::SERVER_FIRST, ['password' => self::PASSWORD]); + $context = $this->challenge->challenge( + self::SERVER_FIRST, + (new ScramOptions())->setPassword(self::PASSWORD), + ); self::assertSame(self::CLIENT_FINAL, $context->getResponse()); self::assertFalse($context->isComplete()); @@ -83,8 +87,14 @@ public function testClientFinalMessageMatchesRfcVector(): void public function testClientCompletesAndAuthenticatesOnValidServerFinal(): void { - $this->challenge->challenge(null, ['username' => self::USERNAME, 'cnonce' => self::CNONCE]); - $this->challenge->challenge(self::SERVER_FIRST, ['password' => self::PASSWORD]); + $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE), + ); + $this->challenge->challenge( + self::SERVER_FIRST, + (new ScramOptions())->setPassword(self::PASSWORD), + ); $context = $this->challenge->challenge(self::SERVER_FINAL); self::assertNull($context->getResponse()); @@ -96,8 +106,14 @@ public function testClientThrowsExceptionOnServerFinalWithError(): void { $this->expectException(SaslException::class); - $this->challenge->challenge(null, ['username' => self::USERNAME, 'cnonce' => self::CNONCE]); - $this->challenge->challenge(self::SERVER_FIRST, ['password' => self::PASSWORD]); + $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE), + ); + $this->challenge->challenge( + self::SERVER_FIRST, + (new ScramOptions())->setPassword(self::PASSWORD), + ); $this->challenge->challenge('e=unknown-user'); } @@ -105,8 +121,14 @@ public function testClientThrowsExceptionOnInvalidServerSignature(): void { $this->expectException(SaslException::class); - $this->challenge->challenge(null, ['username' => self::USERNAME, 'cnonce' => self::CNONCE]); - $this->challenge->challenge(self::SERVER_FIRST, ['password' => self::PASSWORD]); + $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE), + ); + $this->challenge->challenge( + self::SERVER_FIRST, + (new ScramOptions())->setPassword(self::PASSWORD), + ); $this->challenge->challenge('v=aW52YWxpZHNpZ25hdHVyZQ=='); } @@ -114,10 +136,13 @@ public function testClientThrowsExceptionWhenNonceDoesNotBeginWithClientNonce(): { $this->expectException(SaslException::class); - $this->challenge->challenge(null, ['username' => self::USERNAME, 'cnonce' => self::CNONCE]); + $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE), + ); $this->challenge->challenge( 'r=wrongnonce,s=' . self::SALT_B64 . ',i=' . self::ITERATIONS, - ['password' => self::PASSWORD], + (new ScramOptions())->setPassword(self::PASSWORD), ); } @@ -132,16 +157,19 @@ public function testClientThrowsExceptionWhenPasswordIsMissing(): void { $this->expectException(SaslException::class); - $this->challenge->challenge(null, ['username' => self::USERNAME, 'cnonce' => self::CNONCE]); + $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE), + ); $this->challenge->challenge(self::SERVER_FIRST); } public function testClientEncodesSpecialCharactersInUsername(): void { - $context = $this->challenge->challenge(null, [ - 'username' => 'user=name,test', - 'cnonce' => self::CNONCE, - ]); + $context = $this->challenge->challenge( + null, + (new ScramOptions())->setUsername('user=name,test')->setCnonce(self::CNONCE), + ); self::assertStringContainsString('n=user=3Dname=2Ctest', (string) $context->getResponse()); } @@ -149,11 +177,10 @@ public function testClientEncodesSpecialCharactersInUsername(): void public function testChannelBindingClientFirstUsesCorrectGs2Header(): void { $plusChallenge = new ScramChallenge(false, HashAlgorithm::SHA256, true); - $context = $plusChallenge->challenge(null, [ - 'username' => self::USERNAME, - 'cnonce' => self::CNONCE, - 'cbind_type' => 'tls-unique', - ]); + $context = $plusChallenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE)->setCbindType('tls-unique'), + ); self::assertStringStartsWith('p=tls-unique,,', (string) $context->getResponse()); } @@ -170,11 +197,13 @@ public function testServerReturnsNullResponseForInitialCall(): void public function testServerFirstMessageContainsClientNoncePrefix(): void { $server = new ScramChallenge(true, HashAlgorithm::SHA1); - $context = $server->challenge(self::CLIENT_FIRST, [ - 'nonce' => self::SNONCE, - 'salt' => base64_decode(self::SALT_B64), - 'iterations' => self::ITERATIONS, - ]); + $context = $server->challenge( + self::CLIENT_FIRST, + (new ScramOptions()) + ->setNonce(self::SNONCE) + ->setSalt((string) base64_decode(self::SALT_B64)) + ->setIterations(self::ITERATIONS), + ); self::assertSame(self::SERVER_FIRST, $context->getResponse()); self::assertFalse($context->isComplete()); @@ -183,12 +212,17 @@ public function testServerFirstMessageContainsClientNoncePrefix(): void public function testServerFinalMessageOnValidProof(): void { $server = new ScramChallenge(true, HashAlgorithm::SHA1); - $server->challenge(self::CLIENT_FIRST, [ - 'nonce' => self::SNONCE, - 'salt' => base64_decode(self::SALT_B64), - 'iterations' => self::ITERATIONS, - ]); - $context = $server->challenge(self::CLIENT_FINAL, ['password' => self::PASSWORD]); + $server->challenge( + self::CLIENT_FIRST, + (new ScramOptions()) + ->setNonce(self::SNONCE) + ->setSalt((string) base64_decode(self::SALT_B64)) + ->setIterations(self::ITERATIONS), + ); + $context = $server->challenge( + self::CLIENT_FINAL, + (new ScramOptions())->setPassword(self::PASSWORD), + ); self::assertSame(self::SERVER_FINAL, $context->getResponse()); self::assertTrue($context->isComplete()); @@ -198,12 +232,17 @@ public function testServerFinalMessageOnValidProof(): void public function testServerFinalMessageOnInvalidProof(): void { $server = new ScramChallenge(true, HashAlgorithm::SHA1); - $server->challenge(self::CLIENT_FIRST, [ - 'nonce' => self::SNONCE, - 'salt' => base64_decode(self::SALT_B64), - 'iterations' => self::ITERATIONS, - ]); - $context = $server->challenge(self::CLIENT_FINAL, ['password' => 'wrongpassword']); + $server->challenge( + self::CLIENT_FIRST, + (new ScramOptions()) + ->setNonce(self::SNONCE) + ->setSalt((string) base64_decode(self::SALT_B64)) + ->setIterations(self::ITERATIONS), + ); + $context = $server->challenge( + self::CLIENT_FINAL, + (new ScramOptions())->setPassword('wrongpassword'), + ); self::assertStringStartsWith('e=', (string) $context->getResponse()); self::assertTrue($context->isComplete()); @@ -213,12 +252,14 @@ public function testServerFinalMessageOnInvalidProof(): void public function testServerFinalMessageWhenPasswordIsMissing(): void { $server = new ScramChallenge(true, HashAlgorithm::SHA1); - $server->challenge(self::CLIENT_FIRST, [ - 'nonce' => self::SNONCE, - 'salt' => base64_decode(self::SALT_B64), - 'iterations' => self::ITERATIONS, - ]); - $context = $server->challenge(self::CLIENT_FINAL, []); + $server->challenge( + self::CLIENT_FIRST, + (new ScramOptions()) + ->setNonce(self::SNONCE) + ->setSalt((string) base64_decode(self::SALT_B64)) + ->setIterations(self::ITERATIONS), + ); + $context = $server->challenge(self::CLIENT_FINAL); self::assertSame('e=invalid-proof', $context->getResponse()); self::assertTrue($context->isComplete()); @@ -229,10 +270,13 @@ public function testClientThrowsExceptionWhenServerIterationCountIsBelowMinimum( { $this->expectException(SaslException::class); - $this->challenge->challenge(null, ['username' => self::USERNAME, 'cnonce' => self::CNONCE]); + $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE), + ); $this->challenge->challenge( 'r=' . self::CNONCE . self::SNONCE . ',s=' . self::SALT_B64 . ',i=1', - ['password' => self::PASSWORD], + (new ScramOptions())->setPassword(self::PASSWORD), ); } @@ -241,34 +285,41 @@ public function testServerThrowsExceptionWhenSuppliedIterationCountIsZero(): voi $this->expectException(SaslException::class); $server = new ScramChallenge(true, HashAlgorithm::SHA1); - $server->challenge(self::CLIENT_FIRST, [ - 'nonce' => self::SNONCE, - 'salt' => base64_decode(self::SALT_B64), - 'iterations' => 0, - ]); + $server->challenge( + self::CLIENT_FIRST, + (new ScramOptions()) + ->setNonce(self::SNONCE) + ->setSalt((string) base64_decode(self::SALT_B64)) + ->setIterations(0), + ); } public function testClientThrowsExceptionOnInvalidBase64Salt(): void { $this->expectException(SaslException::class); - $this->challenge->challenge(null, ['username' => self::USERNAME, 'cnonce' => self::CNONCE]); + $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE), + ); $this->challenge->challenge( 'r=' . self::CNONCE . self::SNONCE . ',s=not!!valid!!base64,i=' . self::ITERATIONS, - ['password' => self::PASSWORD], + (new ScramOptions())->setPassword(self::PASSWORD), ); } public function testServerReturnsInvalidProofOnMalformedBase64Proof(): void { $server = new ScramChallenge(true, HashAlgorithm::SHA1); - $server->challenge(self::CLIENT_FIRST, [ - 'nonce' => self::SNONCE, - 'salt' => base64_decode(self::SALT_B64), - 'iterations' => self::ITERATIONS, - ]); + $server->challenge( + self::CLIENT_FIRST, + (new ScramOptions()) + ->setNonce(self::SNONCE) + ->setSalt((string) base64_decode(self::SALT_B64)) + ->setIterations(self::ITERATIONS), + ); $tampered = (string) preg_replace('/,p=[^,]+$/', ',p=not!!valid', self::CLIENT_FINAL); - $context = $server->challenge($tampered, ['password' => self::PASSWORD]); + $context = $server->challenge($tampered, (new ScramOptions())->setPassword(self::PASSWORD)); self::assertSame('e=invalid-proof', $context->getResponse()); self::assertFalse($context->isAuthenticated()); @@ -278,7 +329,10 @@ public function testClientThrowsExceptionWhenClientFinalCalledWithoutClientFirst { $this->expectException(SaslException::class); - $this->challenge->challenge(self::SERVER_FIRST, ['password' => self::PASSWORD]); + $this->challenge->challenge( + self::SERVER_FIRST, + (new ScramOptions())->setPassword(self::PASSWORD), + ); } public function testClientThrowsExceptionOnInvalidChannelBindingType(): void @@ -286,21 +340,23 @@ public function testClientThrowsExceptionOnInvalidChannelBindingType(): void $this->expectException(SaslException::class); $plusChallenge = new ScramChallenge(false, HashAlgorithm::SHA256, true); - $plusChallenge->challenge(null, [ - 'username' => self::USERNAME, - 'cnonce' => self::CNONCE, - 'cbind_type' => 'invalid,,type', - ]); + $plusChallenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE)->setCbindType('invalid,,type'), + ); } public function testClientThrowsExceptionWhenServerIterationCountExceedsMaximum(): void { $this->expectException(SaslException::class); - $this->challenge->challenge(null, ['username' => self::USERNAME, 'cnonce' => self::CNONCE]); + $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(self::CNONCE), + ); $this->challenge->challenge( 'r=' . self::CNONCE . self::SNONCE . ',s=' . self::SALT_B64 . ',i=1000001', - ['password' => self::PASSWORD], + (new ScramOptions())->setPassword(self::PASSWORD), ); } @@ -309,21 +365,23 @@ public function testServerThrowsExceptionWhenSuppliedIterationCountExceedsMaximu $this->expectException(SaslException::class); $server = new ScramChallenge(true, HashAlgorithm::SHA1); - $server->challenge(self::CLIENT_FIRST, [ - 'nonce' => self::SNONCE, - 'salt' => base64_decode(self::SALT_B64), - 'iterations' => 1000001, - ]); + $server->challenge( + self::CLIENT_FIRST, + (new ScramOptions()) + ->setNonce(self::SNONCE) + ->setSalt((string) base64_decode(self::SALT_B64)) + ->setIterations(1000001), + ); } public function testClientThrowsExceptionOnEmptyCnonce(): void { $this->expectException(SaslException::class); - $this->challenge->challenge(null, [ - 'username' => self::USERNAME, - 'cnonce' => '', - ]); + $this->challenge->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME)->setCnonce(''), + ); } public function testFullClientServerRoundTrip(): void @@ -331,16 +389,25 @@ public function testFullClientServerRoundTrip(): void $client = new ScramChallenge(false, HashAlgorithm::SHA256); $server = new ScramChallenge(true, HashAlgorithm::SHA256); - $clientFirst = $client->challenge(null, ['username' => self::USERNAME])->getResponse(); + $clientFirst = $client->challenge( + null, + (new ScramOptions())->setUsername(self::USERNAME), + )->getResponse(); self::assertNotNull($clientFirst); $serverFirst = $server->challenge($clientFirst)->getResponse(); self::assertNotNull($serverFirst); - $clientFinal = $client->challenge($serverFirst, ['password' => self::PASSWORD])->getResponse(); + $clientFinal = $client->challenge( + $serverFirst, + (new ScramOptions())->setPassword(self::PASSWORD), + )->getResponse(); self::assertNotNull($clientFinal); - $serverContext = $server->challenge($clientFinal, ['password' => self::PASSWORD]); + $serverContext = $server->challenge( + $clientFinal, + (new ScramOptions())->setPassword(self::PASSWORD), + ); $serverFinal = $serverContext->getResponse(); self::assertStringStartsWith('v=', (string) $serverFinal); self::assertTrue($serverContext->isAuthenticated()); diff --git a/tests/unit/MechanismSelectorTest.php b/tests/unit/MechanismSelectorTest.php index 68b3ee8..5a1100f 100644 --- a/tests/unit/MechanismSelectorTest.php +++ b/tests/unit/MechanismSelectorTest.php @@ -20,6 +20,7 @@ use FreeDSx\Sasl\Mechanism\MechanismName; use FreeDSx\Sasl\Mechanism\PlainMechanism; use FreeDSx\Sasl\MechanismSelector; +use FreeDSx\Sasl\Options\SelectOptions; use PHPUnit\Framework\TestCase; final class MechanismSelectorTest extends TestCase @@ -52,21 +53,21 @@ public function testSelectWithAnonymousOrAuthOnly(): void public function testSelectWithUseIntegrity(): void { - $choice = $this->selector->select([], ['use_integrity' => true]); + $choice = $this->selector->select([], (new SelectOptions())->setUseIntegrity(true)); self::assertSame(MechanismName::DIGEST_MD5, $choice->getName()); } public function testSelectWithUsePrivacy(): void { - $choice = $this->selector->select([], ['use_privacy' => true]); + $choice = $this->selector->select([], (new SelectOptions())->setUsePrivacy(true)); self::assertSame(MechanismName::DIGEST_MD5, $choice->getName()); } public function testSelectWithoutPrivacyOrIntegrityStillSelectsTheBetterChoiceForAuth(): void { - $choice = $this->selector->select([], ['use_integrity' => false, 'use_privacy' => false]); + $choice = $this->selector->select([], null); self::assertSame(MechanismName::DIGEST_MD5, $choice->getName()); } @@ -91,7 +92,7 @@ public function testSelectThrowsAnExceptionIfNoMechsMeetsIntegrityRequirements() $this->selector->select( [MechanismName::ANONYMOUS, MechanismName::PLAIN, MechanismName::CRAM_MD5], - ['use_integrity' => true], + (new SelectOptions())->setUseIntegrity(true), ); } @@ -101,7 +102,7 @@ public function testSelectThrowsAnExceptionIfNoMechsMeetsPrivacyRequirements(): $this->selector->select( [MechanismName::ANONYMOUS, MechanismName::PLAIN, MechanismName::CRAM_MD5], - ['use_privacy' => true], + (new SelectOptions())->setUsePrivacy(true), ); } diff --git a/tests/unit/SaslTest.php b/tests/unit/SaslTest.php index 5150ee9..2b7853f 100644 --- a/tests/unit/SaslTest.php +++ b/tests/unit/SaslTest.php @@ -17,6 +17,7 @@ use FreeDSx\Sasl\Mechanism\DigestMD5Mechanism; use FreeDSx\Sasl\Mechanism\MechanismInterface; use FreeDSx\Sasl\Mechanism\MechanismName; +use FreeDSx\Sasl\Options\SaslOptions; use FreeDSx\Sasl\Sasl; use PHPUnit\Framework\TestCase; @@ -68,7 +69,7 @@ public function testGetCramMD5(): void public function testSupportedOption(): void { - $sasl = new Sasl(['supported' => [MechanismName::DIGEST_MD5]]); + $sasl = new Sasl(new SaslOptions(supported: [MechanismName::DIGEST_MD5])); self::assertCount(1, $sasl->mechanisms()); } From 147e15588802421d0d6c6f96482692b26421a810 Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 3 May 2026 22:58:42 -0400 Subject: [PATCH 2/3] Set PHPStan to max. Make Message getters type safe. --- phpstan.neon | 6 +- .../Sasl/Challenge/AnonymousChallenge.php | 5 +- .../Sasl/Challenge/CramMD5Challenge.php | 10 +-- .../Sasl/Challenge/DigestMD5Challenge.php | 12 ++-- src/FreeDSx/Sasl/Challenge/PlainChallenge.php | 10 +-- .../Sasl/Challenge/ResolvesOptionsTrait.php | 4 +- src/FreeDSx/Sasl/Challenge/ScramChallenge.php | 42 ++++++------ src/FreeDSx/Sasl/Encoder/AnonymousEncoder.php | 2 +- src/FreeDSx/Sasl/Encoder/CramMD5Encoder.php | 6 +- src/FreeDSx/Sasl/Encoder/DigestMD5Encoder.php | 20 +++--- src/FreeDSx/Sasl/Encoder/PlainEncoder.php | 8 +-- src/FreeDSx/Sasl/Encoder/ScramEncoder.php | 4 +- .../Sasl/Factory/DigestMD5MessageFactory.php | 33 ++++----- src/FreeDSx/Sasl/Factory/NonceTrait.php | 7 +- .../Sasl/Mechanism/DigestMD5Mechanism.php | 22 +++--- src/FreeDSx/Sasl/Message.php | 67 +++++++++++++++++++ src/FreeDSx/Sasl/SaslContext.php | 12 ++++ .../Sasl/Security/DigestMD5SecurityLayer.php | 52 +++++++------- tests/unit/Challenge/PlainChallengeTest.php | 4 +- 19 files changed, 206 insertions(+), 120 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 737524e..3be3afc 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,9 +1,7 @@ parameters: - # Level 6 for now. Bumping to 9 (matching LDAP-fork) is gated on the planned - # Options DTO refactor — most level 7-9 noise comes from the array - # accessors on Message and SaslContext. - level: 6 + level: max paths: - %currentWorkingDirectory%/src + - %currentWorkingDirectory%/tests treatPhpDocTypesAsCertain: false reportUnmatchedIgnoredErrors: false diff --git a/src/FreeDSx/Sasl/Challenge/AnonymousChallenge.php b/src/FreeDSx/Sasl/Challenge/AnonymousChallenge.php index 20827a6..728ea1b 100644 --- a/src/FreeDSx/Sasl/Challenge/AnonymousChallenge.php +++ b/src/FreeDSx/Sasl/Challenge/AnonymousChallenge.php @@ -43,7 +43,10 @@ public function challenge( ?string $received = null, ?ChallengeOptionsInterface $options = null, ): SaslContext { - $resolved = $this->resolveOptions($options, AnonymousOptions::class); + $resolved = $this->resolveOptions( + $options ?? new AnonymousOptions(), + AnonymousOptions::class, + ); if ($this->context->isServerMode()) { $this->processServer($received); diff --git a/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php b/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php index 74d9fd2..efce0e1 100644 --- a/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php +++ b/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php @@ -47,7 +47,7 @@ public function challenge( ?ChallengeOptionsInterface $options = null, ): SaslContext { $resolved = $this->resolveOptions( - $options, + $options ?? new CramMD5Options(), CramMD5Options::class, ); $message = $received === null ? null : $this->encoder->decode($received, $this->context); @@ -95,7 +95,7 @@ private function generateClientResponse( } $response = new Message([ 'username' => $options->getUsername(), - 'digest' => $this->generateDigest((string) $received->get('challenge'), $options->getPassword()), + 'digest' => $this->generateDigest($received->getString('challenge') ?? '', $options->getPassword()), ]); $this->context->setResponse($this->encoder->encode($response, $this->context)); $this->context->setIsComplete(true); @@ -117,10 +117,10 @@ private function validateClientResponse( if ($options->getPasswordCallback() === null) { throw new SaslException('To validate the client response you must supply the passwordCallback option.'); } - $username = $received->get('username'); - $digest = $received->get('digest'); + $username = $received->getString('username') ?? ''; + $digest = $received->getString('digest'); - $expectedDigest = ($options->getPasswordCallback())($username, $this->context->get('challenge')); + $expectedDigest = ($options->getPasswordCallback())($username, $this->context->getString('challenge') ?? ''); $this->context->setIsAuthenticated($expectedDigest === $digest); $this->context->setIsComplete(true); diff --git a/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php b/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php index 1c67831..b4d628b 100644 --- a/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php +++ b/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php @@ -53,7 +53,7 @@ public function challenge( ?ChallengeOptionsInterface $options = null, ): SaslContext { $resolved = $this->resolveOptions( - $options, + $options ?? new DigestMD5Options(), DigestMD5Options::class, ); @@ -199,21 +199,21 @@ private function generateServerVerification( # @todo This should accept some kind of computed value, like the a1. Then it could generate the other values # using that. $password = $options->getPassword(); - $qop = $received->get('qop'); - $cipher = $received->get('cipher'); + $qop = $received->getString('qop'); + $cipher = $received->getString('cipher'); if ($password === null) { return null; } # Client selected a qop we did not advertise... - if (!in_array($qop, $this->challenge->get('qop'), true)) { + if (!in_array($qop, $this->challenge->getStringArray('qop') ?? [], true)) { return null; } # Client selected a cipher we did not advertise... - if (!in_array($cipher, $this->challenge->get('cipher'), true)) { + if (!in_array($cipher, $this->challenge->getStringArray('cipher') ?? [], true)) { return null; } # The client sent a nonce without the minimum length from the RFC... - if (strlen((string) $received->get('cnonce')) < 12) { + if (strlen($received->getString('cnonce') ?? '') < 12) { return null; } # The client sent back a nonce different than what we sent them... diff --git a/src/FreeDSx/Sasl/Challenge/PlainChallenge.php b/src/FreeDSx/Sasl/Challenge/PlainChallenge.php index c21ebea..33ee263 100644 --- a/src/FreeDSx/Sasl/Challenge/PlainChallenge.php +++ b/src/FreeDSx/Sasl/Challenge/PlainChallenge.php @@ -45,7 +45,7 @@ public function challenge( ?ChallengeOptionsInterface $options = null, ): SaslContext { $resolved = $this->resolveOptions( - $options, + $options ?? new PlainOptions(), PlainOptions::class, ); $message = $received === null ? null : $this->encoder->decode($received, $this->context); @@ -68,12 +68,12 @@ private function serverProcess( if ($options->getValidate() === null) { throw new SaslException('You must pass a callable validate option to the plain mechanism in server mode.'); } - $authzId = $message->get('authzid'); - $authcId = $message->get('authcid'); - $password = $message->get('password'); + $authzId = $message->getString('authzid'); + $authcId = $message->getString('authcid') ?? ''; + $password = $message->getString('password') ?? ''; $this->context->setIsComplete(true); - $this->context->setIsAuthenticated((bool) ($options->getValidate())($authzId, $authcId, $password)); + $this->context->setIsAuthenticated(($options->getValidate())($authzId, $authcId, $password)); return $this->context; } diff --git a/src/FreeDSx/Sasl/Challenge/ResolvesOptionsTrait.php b/src/FreeDSx/Sasl/Challenge/ResolvesOptionsTrait.php index 2feb211..ba54bba 100644 --- a/src/FreeDSx/Sasl/Challenge/ResolvesOptionsTrait.php +++ b/src/FreeDSx/Sasl/Challenge/ResolvesOptionsTrait.php @@ -28,10 +28,10 @@ trait ResolvesOptionsTrait * @throws SaslException */ private function resolveOptions( - ?ChallengeOptionsInterface $options, + ChallengeOptionsInterface $options, string $expectedClass, ): ChallengeOptionsInterface { - if ($options !== null && !$options instanceof $expectedClass) { + if (!$options instanceof $expectedClass) { throw new SaslException(sprintf( 'Expected %s, got %s.', $expectedClass, diff --git a/src/FreeDSx/Sasl/Challenge/ScramChallenge.php b/src/FreeDSx/Sasl/Challenge/ScramChallenge.php index 2da5441..40a634b 100644 --- a/src/FreeDSx/Sasl/Challenge/ScramChallenge.php +++ b/src/FreeDSx/Sasl/Challenge/ScramChallenge.php @@ -127,7 +127,7 @@ public function challenge( ?ChallengeOptionsInterface $options = null, ): SaslContext { $resolved = $this->resolveOptions( - $options, + $options ?? new ScramOptions(), ScramOptions::class, ); @@ -250,26 +250,25 @@ private function generateClientFinal( } # Guard against out-of-order calls: client-first must have run first to populate the cnonce. - $cnonce = $this->context->get(self::CTX_CNONCE); + $cnonce = $this->context->getString(self::CTX_CNONCE); if ($cnonce === null) { throw new SaslException( 'client-final called before client-first: no client nonce found in context.', ); } - $cnonce = (string) $cnonce; # The full nonce must begin with the client nonce we sent — verify before processing anything else. [$fullNonce, $salt, $iterations] = $this->parseServerFirst($serverFirst, $cnonce); # Channel binding value: base64(gs2-header + cbind-data). - $gs2Header = (string) $this->context->get(self::CTX_GS2_HEADER); + $gs2Header = $this->context->getString(self::CTX_GS2_HEADER) ?? ''; $cbindData = $options->getCbindData() ?? ''; $channelBinding = base64_encode($gs2Header . $cbindData); $clientFinalWithoutProof = 'c=' . $channelBinding . ',r=' . $fullNonce; # RFC 5802 §3: AuthMessage = client-first-bare "," server-first "," client-final-without-proof - $authMessage = $this->context->get(self::CTX_CLIENT_FIRST_BARE) + $authMessage = ($this->context->getString(self::CTX_CLIENT_FIRST_BARE) ?? '') . ',' . $rawServerFirst . ',' . $clientFinalWithoutProof; @@ -297,12 +296,12 @@ private function verifyServerFinal(Message $serverFinal): void if ($serverFinal->has('e')) { throw new SaslException(sprintf( 'SCRAM authentication failed with server error: %s', - $serverFinal->get('e'), + $serverFinal->getString('e') ?? '', )); } - $expectedSig = (string) $this->context->get(self::CTX_SERVER_SIGNATURE); - $receivedSig = (string) $serverFinal->get('v'); + $expectedSig = $this->context->getString(self::CTX_SERVER_SIGNATURE) ?? ''; + $receivedSig = $serverFinal->getString('v') ?? ''; if (!hash_equals($expectedSig, $receivedSig)) { throw new SaslException('The server signature does not match the expected value.'); @@ -319,7 +318,7 @@ private function generateServerFirst( string $rawClientFirst, ScramOptions $options, ): string { - $cnonce = (string) $clientFirst->get('r'); + $cnonce = $clientFirst->getString('r') ?? ''; $snonce = $options->getNonce() ?? $this->generateNonce(self::NONCE_SIZE); $fullNonce = $cnonce . $snonce; @@ -364,18 +363,19 @@ private function generateServerFinal( return self::INVALID_PROOF; } - $fullNonce = (string) $this->context->get(self::CTX_NONCE); - if ($clientFinal->get('r') !== $fullNonce) { + $fullNonce = $this->context->getString(self::CTX_NONCE) ?? ''; + $clientNonce = $clientFinal->getString('r'); + if ($clientNonce !== $fullNonce) { return self::INVALID_PROOF; } - $salt = (string) $this->context->get(self::CTX_SALT); - $iterations = (int) $this->context->get(self::CTX_ITERATIONS); + $salt = $this->context->getString(self::CTX_SALT) ?? ''; + $iterations = $this->context->getInt(self::CTX_ITERATIONS) ?? 1; # RFC 5802: client-final-without-proof = c=...,r=...[,extensions] — fixed order, safely rebuildable. - $clientFinalWithoutProof = 'c=' . $clientFinal->get('c') . ',r=' . $clientFinal->get('r'); - $authMessage = $this->context->get(self::CTX_CLIENT_FIRST_BARE) - . ',' . $this->context->get(self::CTX_SERVER_FIRST) + $clientFinalWithoutProof = 'c=' . ($clientFinal->getString('c') ?? '') . ',r=' . $clientNonce; + $authMessage = ($this->context->getString(self::CTX_CLIENT_FIRST_BARE) ?? '') + . ',' . ($this->context->getString(self::CTX_SERVER_FIRST) ?? '') . ',' . $clientFinalWithoutProof; try { @@ -430,20 +430,20 @@ private function parseServerFirst( Message $serverFirst, string $cnonce, ): array { - $fullNonce = (string) $serverFirst->get('r'); + $fullNonce = $serverFirst->getString('r') ?? ''; if (strncmp($fullNonce, $cnonce, strlen($cnonce)) !== 0) { throw new SaslException('The server nonce does not begin with the client nonce.'); } $salt = base64_decode( - (string) $serverFirst->get('s'), + $serverFirst->getString('s') ?? '', true, ); if ($salt === false) { throw new SaslException('The server-provided salt is not valid base64.'); } - $iterations = (int) $serverFirst->get('i'); + $iterations = $serverFirst->getIntOrParse('i') ?? 0; $minIterations = self::MIN_ITERATIONS[$this->hashAlgorithm->value]; if ($iterations < $minIterations) { throw new SaslException(sprintf( @@ -500,7 +500,7 @@ private function isValidClientProof( $authMessage, ); $receivedProof = base64_decode( - (string) $clientFinal->get('p'), + $clientFinal->getString('p') ?? '', true, ); @@ -522,7 +522,7 @@ private function pbkdf2( $this->hashAlgorithm->value, $password, $salt, - $iterations, + max(1, $iterations), 0, true, ); diff --git a/src/FreeDSx/Sasl/Encoder/AnonymousEncoder.php b/src/FreeDSx/Sasl/Encoder/AnonymousEncoder.php index 9e4ac45..f8e32e3 100644 --- a/src/FreeDSx/Sasl/Encoder/AnonymousEncoder.php +++ b/src/FreeDSx/Sasl/Encoder/AnonymousEncoder.php @@ -28,7 +28,7 @@ public function encode( SaslContext $context, ): string { if ($message->has('trace')) { - return (string) $message->get('trace'); + return $message->getString('trace') ?? ''; } return ''; diff --git a/src/FreeDSx/Sasl/Encoder/CramMD5Encoder.php b/src/FreeDSx/Sasl/Encoder/CramMD5Encoder.php index 394ab2b..45631e9 100644 --- a/src/FreeDSx/Sasl/Encoder/CramMD5Encoder.php +++ b/src/FreeDSx/Sasl/Encoder/CramMD5Encoder.php @@ -51,7 +51,7 @@ private function encodeServerChallenge(Message $message): string throw new SaslEncodingException('The server challenge message must contain a "challenge".'); } - return '<' . $message->get('challenge') . '>'; + return '<' . ($message->getString('challenge') ?? '') . '>'; } /** @@ -65,8 +65,8 @@ private function encodeClientResponse(Message $message): string if (!$message->has('digest')) { throw new SaslEncodingException('The client response must contain a digest.'); } - $username = (string) $message->get('username'); - $digest = (string) $message->get('digest'); + $username = $message->getString('username') ?? ''; + $digest = $message->getString('digest') ?? ''; if (preg_match('/^[0-9a-f]{32}$/', $digest) !== 1) { throw new SaslEncodingException('The client digest must be a 16 octet, lower-case, hexadecimal value'); diff --git a/src/FreeDSx/Sasl/Encoder/DigestMD5Encoder.php b/src/FreeDSx/Sasl/Encoder/DigestMD5Encoder.php index d0591ef..da93a7f 100644 --- a/src/FreeDSx/Sasl/Encoder/DigestMD5Encoder.php +++ b/src/FreeDSx/Sasl/Encoder/DigestMD5Encoder.php @@ -71,13 +71,13 @@ public function encode( SaslContext $context, ): string { $response = ''; - foreach ($message->toArray() as $key => $value) { + foreach (array_keys($message->toArray()) as $key) { if ($response !== '') { $response .= ','; } $response .= $key . '=' . $this->encodeOptValue( - (string) $key, - $value, + $key, + $message, $context->isServerMode(), ); } @@ -176,20 +176,20 @@ private function parseOptValue( */ private function encodeOptValue( string $name, - mixed $value, + Message $message, bool $isServerMode, ): string { return match ($name) { 'realm', 'nonce', 'username', 'cnonce', 'authzid', 'digest-uri' - => '"' . str_replace(['\\', '"'], ['\\\\', '\"'], (string) $value) . '"', + => '"' . str_replace(['\\', '"'], ['\\\\', '\"'], $message->getString($name) ?? '') . '"', 'qop', 'cipher' => $isServerMode - ? '"' . implode(',', (array) $value) . '"' - : (string) $value, + ? '"' . implode(',', $message->getStringArray($name) ?? []) . '"' + : ($message->getString($name) ?? ''), 'stale' => 'true', - 'maxbuf', 'algorithm', 'charset' => (string) $value, - 'nc' => str_pad(dechex((int) $value), 8, '0', STR_PAD_LEFT), - 'response', 'rspauth' => $this->encodeLHexValue((string) $value, 32), + 'maxbuf', 'algorithm', 'charset' => $message->getString($name) ?? '', + 'nc' => str_pad(dechex($message->getInt($name) ?? 0), 8, '0', STR_PAD_LEFT), + 'response', 'rspauth' => $this->encodeLHexValue($message->getString($name) ?? '', 32), default => throw new SaslEncodingException(sprintf( 'Digest option %s is not supported.', $name, diff --git a/src/FreeDSx/Sasl/Encoder/PlainEncoder.php b/src/FreeDSx/Sasl/Encoder/PlainEncoder.php index 0f00dfd..ed9adea 100644 --- a/src/FreeDSx/Sasl/Encoder/PlainEncoder.php +++ b/src/FreeDSx/Sasl/Encoder/PlainEncoder.php @@ -37,9 +37,9 @@ public function encode( if (!$message->has('password')) { throw new SaslEncodingException('The PLAIN message must contain a password.'); } - $authzid = $this->validate((string) $message->get('authzid')); - $authcid = $this->validate((string) $message->get('authcid')); - $password = $this->validate((string) $message->get('password')); + $authzid = $this->validate($message->getString('authzid') ?? ''); + $authcid = $this->validate($message->getString('authcid') ?? ''); + $password = $this->validate($message->getString('password') ?? ''); return $authzid . "\x00" . $authcid . "\x00" . $password; } @@ -48,7 +48,7 @@ public function decode( string $data, SaslContext $context, ): Message { - if (preg_match('/^([^\x0]+)\x00([^\x0]+)\x00([^\x0]+)$/', $data, $matches) === 0) { + if (preg_match('/^([^\x0]+)\x00([^\x0]+)\x00([^\x0]+)$/', $data, $matches) !== 1) { throw new SaslEncodingException('The PLAIN message data is malformed.'); } diff --git a/src/FreeDSx/Sasl/Encoder/ScramEncoder.php b/src/FreeDSx/Sasl/Encoder/ScramEncoder.php index 5d5dba1..69e74a9 100644 --- a/src/FreeDSx/Sasl/Encoder/ScramEncoder.php +++ b/src/FreeDSx/Sasl/Encoder/ScramEncoder.php @@ -42,8 +42,8 @@ public function encode( SaslContext $context, ): string { $parts = []; - foreach ($message->toArray() as $attr => $value) { - $parts[] = $attr . '=' . $value; + foreach (array_keys($message->toArray()) as $attr) { + $parts[] = $attr . '=' . ($message->getString($attr) ?? ''); } return implode(',', $parts); diff --git a/src/FreeDSx/Sasl/Factory/DigestMD5MessageFactory.php b/src/FreeDSx/Sasl/Factory/DigestMD5MessageFactory.php index 27fa3d7..9224f4d 100644 --- a/src/FreeDSx/Sasl/Factory/DigestMD5MessageFactory.php +++ b/src/FreeDSx/Sasl/Factory/DigestMD5MessageFactory.php @@ -72,8 +72,9 @@ public function create( private function generateServerChallenge(array $options): Message { $challenge = new Message(); + $nonceSize = is_int($options['nonce_size'] ?? null) ? $options['nonce_size'] : self::NONCE_SIZE; $challenge->set('algorithm', 'md5-sess'); - $challenge->set('nonce', $options['nonce'] ?? $this->generateNonce((int) ($options['nonce_size'] ?? self::NONCE_SIZE))); + $challenge->set('nonce', $options['nonce'] ?? $this->generateNonce($nonceSize)); $challenge->set('qop', $this->generateAvailableQops($options)); $challenge->set('realm', $options['realm'] ?? $_SERVER['USERDOMAIN'] ?? gethostname()); $challenge->set('maxbuf', $options['maxbuf'] ?? '65536'); @@ -110,11 +111,12 @@ private function generateClientResponse( Message $challenge, ): Message { $response = new Message(); - $qop = isset($options['qop']) ? (string) $options['qop'] : null; + $qop = is_string($options['qop'] ?? null) ? $options['qop'] : null; $response->set('algorithm', 'md5-sess'); $response->set('nonce', $challenge->get('nonce')); - $response->set('cnonce', $options['cnonce'] ?? $this->generateNonce((int) ($options['nonce_size'] ?? self::NONCE_SIZE))); + $cnNonceSize = is_int($options['nonce_size'] ?? null) ? $options['nonce_size'] : self::NONCE_SIZE; + $response->set('cnonce', $options['cnonce'] ?? $this->generateNonce($cnNonceSize)); $response->set('nc', $options['nc'] ?? 1); $response->set('qop', $this->selectQopFromChallenge($challenge, $qop)); $response->set('username', $options['username'] ?? $this->getCurrentUser()); @@ -142,8 +144,8 @@ private function getDigestUri( return sprintf( '%s/%s', - $options['service'], - $response->get('realm'), + is_string($options['service'] ?? null) ? $options['service'] : '', + $response->getString('realm') ?? '', ); } @@ -173,7 +175,7 @@ private function selectQopFromChallenge( Message $challenge, ?string $qop, ): string { - $available = (array) ($challenge->get('qop') ?? []); + $available = $challenge->getStringArray('qop') ?? []; /* Per the RFC: This directive is optional; if not present it defaults to "auth". */ if (count($available) === 0) { return 'auth'; @@ -240,7 +242,7 @@ private function setCipherForChallenge( if (!$challenge->has('cipher')) { throw new SaslException('The client requested auth-conf, but the challenge contains no ciphers.'); } - $ciphers = (array) $challenge->get('cipher'); + $ciphers = $challenge->getStringArray('cipher') ?? []; # If we are requesting a specific cipher, then only check that one... $toCheck = isset($options['cipher']) ? (array) $options['cipher'] @@ -254,7 +256,7 @@ private function setCipherForChallenge( $selected = null; foreach ($toCheck as $selection) { - if (in_array($selection, $ciphers, true)) { + if (is_string($selection) && in_array($selection, $ciphers, true)) { $selected = $selection; break; } @@ -274,11 +276,11 @@ private function setCipherForChallenge( */ private function getCurrentUser(): string { - if (isset($_SERVER['USERNAME'])) { - return (string) $_SERVER['USERNAME']; + if (is_string($_SERVER['USERNAME'] ?? null)) { + return $_SERVER['USERNAME']; } - if (isset($_SERVER['USER'])) { - return (string) $_SERVER['USER']; + if (is_string($_SERVER['USER'] ?? null)) { + return $_SERVER['USER']; } throw new SaslException('Unable to determine a username for the response. You must supply a username.'); @@ -291,12 +293,11 @@ private function getCurrentUser(): string */ private function getRealmFromChallenge(Message $challenge): string { - if (!$challenge->has('realm')) { + $realm = $challenge->getString('realm'); + if ($realm === null) { throw new SaslException('Unable to determine a realm for the response.'); } - $realms = (array) $challenge->get('realm'); - $selected = array_pop($realms); - return (string) $selected; + return $realm; } } diff --git a/src/FreeDSx/Sasl/Factory/NonceTrait.php b/src/FreeDSx/Sasl/Factory/NonceTrait.php index 0d037a1..45f3d7b 100644 --- a/src/FreeDSx/Sasl/Factory/NonceTrait.php +++ b/src/FreeDSx/Sasl/Factory/NonceTrait.php @@ -35,7 +35,12 @@ trait NonceTrait protected function generateNonce(int $byteLength): string { try { - return base64_encode(random_bytes($byteLength)); + return base64_encode( + random_bytes(max( + 1, + $byteLength, + )) + ); } catch (Exception $e) { throw new SaslException( sprintf('Unable to generate the nonce: %s', $e->getMessage()), diff --git a/src/FreeDSx/Sasl/Mechanism/DigestMD5Mechanism.php b/src/FreeDSx/Sasl/Mechanism/DigestMD5Mechanism.php index e266946..ebda625 100644 --- a/src/FreeDSx/Sasl/Mechanism/DigestMD5Mechanism.php +++ b/src/FreeDSx/Sasl/Mechanism/DigestMD5Mechanism.php @@ -91,8 +91,8 @@ public static function computeResponse( ): string { $a1 = self::computeA1($password, $challenge, $response); - $qop = $response->get('qop'); - $digestUri = $response->get('digest-uri'); + $qop = $response->getString('qop'); + $digestUri = $response->getString('digest-uri') ?? ''; $a2 = $useServerMode ? self::A2_SERVER : self::A2_CLIENT; $a2 .= match ($qop) { @@ -105,10 +105,10 @@ public static function computeResponse( return hash('md5', sprintf( '%s:%s:%s:%s:%s:%s', $a1, - $challenge->get('nonce'), - str_pad(dechex((int) $response->get('nc')), 8, '0', STR_PAD_LEFT), - $response->get('cnonce'), - $response->get('qop'), + $challenge->getString('nonce') ?? '', + str_pad(dechex($response->getIntOrParse('nc') ?? 0), 8, '0', STR_PAD_LEFT), + $response->getString('cnonce') ?? '', + $response->getString('qop') ?? '', $a2, )); } @@ -131,18 +131,18 @@ public static function computeA1( ): string { $a1 = hash('md5', sprintf( '%s:%s:%s', - $response->get('username'), - $response->get('realm'), + $response->getString('username') ?? '', + $response->getString('realm') ?? '', $password, ), true); $a1 = sprintf( '%s:%s:%s', $a1, - $challenge->get('nonce'), - $response->get('cnonce'), + $challenge->getString('nonce') ?? '', + $response->getString('cnonce') ?? '', ); if ($response->has('authzid')) { - $a1 .= ':' . $response->get('authzid'); + $a1 .= ':' . ($response->getString('authzid') ?? ''); } return hash('md5', $a1); diff --git a/src/FreeDSx/Sasl/Message.php b/src/FreeDSx/Sasl/Message.php index 4793538..b9f8045 100644 --- a/src/FreeDSx/Sasl/Message.php +++ b/src/FreeDSx/Sasl/Message.php @@ -40,6 +40,73 @@ public function get(string $name): mixed return $this->data[$name] ?? null; } + public function getString(string $name): ?string + { + $value = $this->data[$name] ?? null; + + return is_string($value) + ? $value + : null; + } + + public function getInt(string $name): ?int + { + $value = $this->data[$name] ?? null; + + return is_int($value) + ? $value + : null; + } + + /** + * Returns the value as int whether it is stored as int or as a numeric string. + */ + public function getIntOrParse(string $name): ?int + { + $value = $this->data[$name] ?? null; + if (is_int($value)) { + return $value; + } + + if (is_string($value)) { + return (int) $value; + } + + return null; + } + + /** + * @return mixed[]|null + */ + public function getArray(string $name): ?array + { + $value = $this->data[$name] ?? null; + + return is_array($value) + ? $value + : null; + } + + /** + * @return string[]|null + */ + public function getStringArray(string $name): ?array + { + $value = $this->data[$name] ?? null; + if (!is_array($value)) { + return null; + } + $strings = []; + + foreach ($value as $v) { + if (is_string($v)) { + $strings[] = $v; + } + } + + return $strings; + } + public function has(string $name): bool { return array_key_exists($name, $this->data); diff --git a/src/FreeDSx/Sasl/SaslContext.php b/src/FreeDSx/Sasl/SaslContext.php index 99e5eb6..e3591e4 100644 --- a/src/FreeDSx/Sasl/SaslContext.php +++ b/src/FreeDSx/Sasl/SaslContext.php @@ -125,6 +125,18 @@ public function get(string $key): mixed return $this->data[$key] ?? null; } + public function getString(string $key): ?string + { + $value = $this->data[$key] ?? null; + return is_string($value) ? $value : null; + } + + public function getInt(string $key): ?int + { + $value = $this->data[$key] ?? null; + return is_int($value) ? $value : null; + } + public function set( string $key, mixed $value, diff --git a/src/FreeDSx/Sasl/Security/DigestMD5SecurityLayer.php b/src/FreeDSx/Sasl/Security/DigestMD5SecurityLayer.php index ee4d35b..bd692ca 100644 --- a/src/FreeDSx/Sasl/Security/DigestMD5SecurityLayer.php +++ b/src/FreeDSx/Sasl/Security/DigestMD5SecurityLayer.php @@ -42,18 +42,18 @@ public function wrap( string $data, SaslContext $context, ): string { - $qop = $context->get('qop'); + $qop = $context->getString('qop'); $wrapped = match ($qop) { 'auth-conf' => $this->encrypt($data, $context), 'auth-int' => $this->sign($data, $context), default => throw new SaslException(sprintf( 'The qop option "%s" is not recognized as a security layer.', - (string) $qop, + $qop ?? 'null', )), }; $this->validateBufferLength($wrapped, $context); - $context->set('seqnumsnt', ((int) $context->get('seqnumsnt')) + 1); + $context->set('seqnumsnt', ($context->getInt('seqnumsnt') ?? 0) + 1); return $wrapped; } @@ -62,7 +62,7 @@ public function unwrap( string $data, SaslContext $context, ): string { - $qop = $context->get('qop'); + $qop = $context->getString('qop'); $this->validateBufferLength($data, $context); $unwrapped = match ($qop) { @@ -70,10 +70,10 @@ public function unwrap( 'auth-int' => $this->verify($data, $context), default => throw new SaslException(sprintf( 'The qop option "%s" is not recognized as a security layer.', - (string) $qop, + $qop ?? 'null', )), }; - $context->set('seqnumrcv', ((int) $context->get('seqnumrcv')) + 1); + $context->set('seqnumrcv', ($context->getInt('seqnumrcv') ?? 0) + 1); return $unwrapped; } @@ -97,17 +97,17 @@ private function decrypt( $receivedMsgType, )); } - $seqnum = $context->get('seqnumrcv'); - if (!is_int($seqnum) || $seqnum !== $receivedSeqNum) { + $seqnum = $context->getInt('seqnumrcv'); + if ($seqnum === null || $seqnum !== $receivedSeqNum) { throw new SaslException(sprintf( 'The received sequence number was unexpected. Expected %s, but got %s.', - (string) $seqnum, + $seqnum ?? 'null', (string) $receivedSeqNum, )); } $cipher = $this->resolveCipher($context); - $a1 = (string) $context->get('a1'); + $a1 = $context->getString('a1') ?? ''; $isServerMode = $context->isServerMode(); $encrypted = substr($data, 0, -6); @@ -152,9 +152,9 @@ private function encrypt( SaslContext $context, ): string { $cipher = $this->resolveCipher($context); - $a1 = (string) $context->get('a1'); + $a1 = $context->getString('a1') ?? ''; $isServerMode = $context->isServerMode(); - $seqnum = (int) $context->get('seqnumsnt'); + $seqnum = $context->getInt('seqnumsnt') ?? 0; $mcKc = $isServerMode ? self::KCS_MC : self::KCC_MC; $kc = $this->generateKeyKc($a1, $cipher, $mcKc); @@ -232,9 +232,9 @@ private function sign( string $message, SaslContext $context, ): string { - $seqnum = (int) $context->get('seqnumsnt'); + $seqnum = $context->getInt('seqnumsnt') ?? 0; $mc = $context->isServerMode() ? self::KIS_MC : self::KIC_MC; - $ki = $this->generateKeyKi((string) $context->get('a1'), $mc); + $ki = $this->generateKeyKi($context->getString('a1') ?? '', $mc); $macBlock = $this->generateMACBlock($ki, $message, $seqnum); return $message . $macBlock; @@ -254,11 +254,11 @@ private function verify( throw new SaslException('Expected at least 16 bytes of data for the MAC.'); } - $seqnum = (int) $context->get('seqnumrcv'); + $seqnum = $context->getInt('seqnumrcv') ?? 0; $message = substr($data, 0, -16); # Inverted selection of constant here, as this would be the receiving end. $mc = $context->isServerMode() ? self::KIC_MC : self::KIS_MC; - $ki = $this->generateKeyKi((string) $context->get('a1'), $mc); + $ki = $this->generateKeyKi($context->getString('a1') ?? '', $mc); $expectedMac = $this->generateMACBlock($ki, $message, $seqnum); if ($receivedMac !== $expectedMac) { @@ -282,9 +282,9 @@ private function generatePadding( if ($blockSize === 1) { return ''; } - $pad = $blockSize - (strlen($data) + 10) & ($blockSize - 1); + $pad = ($blockSize - (strlen($data) + 10)) & ($blockSize - 1); - return str_repeat(chr($pad), $pad); + return str_repeat(chr($pad & 0xFF), $pad); } /** @@ -384,13 +384,13 @@ private function expandDesKey(string $key): string } return chr($bytes[0] & 0xfe) - . chr(($bytes[0] << 7) | ($bytes[1] >> 1)) - . chr(($bytes[1] << 6) | ($bytes[2] >> 2)) - . chr(($bytes[2] << 5) | ($bytes[3] >> 3)) - . chr(($bytes[3] << 4) | ($bytes[4] >> 4)) - . chr(($bytes[4] << 3) | ($bytes[5] >> 5)) - . chr(($bytes[5] << 2) | ($bytes[6] >> 6)) - . chr($bytes[6] << 1); + . chr((($bytes[0] << 7) | ($bytes[1] >> 1)) & 0xFF) + . chr((($bytes[1] << 6) | ($bytes[2] >> 2)) & 0xFF) + . chr((($bytes[2] << 5) | ($bytes[3] >> 3)) & 0xFF) + . chr((($bytes[3] << 4) | ($bytes[4] >> 4)) & 0xFF) + . chr((($bytes[4] << 3) | ($bytes[5] >> 5)) & 0xFF) + . chr((($bytes[5] << 2) | ($bytes[6] >> 6)) & 0xFF) + . chr(($bytes[6] << 1) & 0xFF); } /** @@ -400,7 +400,7 @@ private function validateBufferLength( string $data, SaslContext $context, ): void { - $maxbuf = $context->has('maxbuf') ? (int) $context->get('maxbuf') : self::MAXBUF; + $maxbuf = $context->has('maxbuf') ? (int) ($context->getString('maxbuf') ?? '') : self::MAXBUF; if (strlen($data) > $maxbuf) { throw new SaslException(sprintf( 'The wrapped buffer exceeds the maxbuf length of %s', diff --git a/tests/unit/Challenge/PlainChallengeTest.php b/tests/unit/Challenge/PlainChallengeTest.php index 485b698..7911a5c 100644 --- a/tests/unit/Challenge/PlainChallengeTest.php +++ b/tests/unit/Challenge/PlainChallengeTest.php @@ -41,7 +41,7 @@ public function testTheClientChallenge(): void public function testTheServerResponseToTheClientWhenSuccessful(): void { $serverChallenge = new PlainChallenge(true); - $validate = static fn (string $authzid, string $authcid, string $password): bool => true; + $validate = static fn (?string $authzid, string $authcid, string $password): bool => true; $context = $serverChallenge->challenge( "foo\x00foo\x00bar", @@ -56,7 +56,7 @@ public function testTheServerResponseToTheClientWhenSuccessful(): void public function testTheServerResponseToTheClientWhenNotSuccessful(): void { $serverChallenge = new PlainChallenge(true); - $validate = static fn (string $authzid, string $authcid, string $password): bool => false; + $validate = static fn (?string $authzid, string $authcid, string $password): bool => false; $context = $serverChallenge->challenge( "foo\x00foo\x00bar", From 6b73924822d267bb376d71b8797c0f58abb49305 Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Mon, 4 May 2026 18:01:41 -0400 Subject: [PATCH 3/3] Replace all the null coalescing. Makes it easier to read. Add a method that will throw. --- .../Sasl/Challenge/CramMD5Challenge.php | 6 +-- .../Sasl/Challenge/DigestMD5Challenge.php | 2 +- src/FreeDSx/Sasl/Challenge/PlainChallenge.php | 7 ++-- src/FreeDSx/Sasl/Challenge/ScramChallenge.php | 38 +++++++++---------- src/FreeDSx/Sasl/Encoder/AnonymousEncoder.php | 2 +- src/FreeDSx/Sasl/Encoder/CramMD5Encoder.php | 6 +-- src/FreeDSx/Sasl/Encoder/DigestMD5Encoder.php | 8 ++-- src/FreeDSx/Sasl/Encoder/PlainEncoder.php | 6 +-- src/FreeDSx/Sasl/Encoder/ScramEncoder.php | 2 +- .../Sasl/Factory/DigestMD5MessageFactory.php | 12 +++--- .../Sasl/Mechanism/DigestMD5Mechanism.php | 18 ++++----- src/FreeDSx/Sasl/Message.php | 25 +++++++++++- src/FreeDSx/Sasl/SaslContext.php | 24 +++++++++++- .../Sasl/Security/DigestMD5SecurityLayer.php | 14 +++---- 14 files changed, 104 insertions(+), 66 deletions(-) diff --git a/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php b/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php index efce0e1..17c8a23 100644 --- a/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php +++ b/src/FreeDSx/Sasl/Challenge/CramMD5Challenge.php @@ -95,7 +95,7 @@ private function generateClientResponse( } $response = new Message([ 'username' => $options->getUsername(), - 'digest' => $this->generateDigest($received->getString('challenge') ?? '', $options->getPassword()), + 'digest' => $this->generateDigest($received->getString('challenge'), $options->getPassword()), ]); $this->context->setResponse($this->encoder->encode($response, $this->context)); $this->context->setIsComplete(true); @@ -117,10 +117,10 @@ private function validateClientResponse( if ($options->getPasswordCallback() === null) { throw new SaslException('To validate the client response you must supply the passwordCallback option.'); } - $username = $received->getString('username') ?? ''; + $username = $received->getString('username'); $digest = $received->getString('digest'); - $expectedDigest = ($options->getPasswordCallback())($username, $this->context->getString('challenge') ?? ''); + $expectedDigest = ($options->getPasswordCallback())($username, $this->context->getString('challenge')); $this->context->setIsAuthenticated($expectedDigest === $digest); $this->context->setIsComplete(true); diff --git a/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php b/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php index b4d628b..7151a5f 100644 --- a/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php +++ b/src/FreeDSx/Sasl/Challenge/DigestMD5Challenge.php @@ -213,7 +213,7 @@ private function generateServerVerification( return null; } # The client sent a nonce without the minimum length from the RFC... - if (strlen($received->getString('cnonce') ?? '') < 12) { + if (strlen($received->getString('cnonce')) < 12) { return null; } # The client sent back a nonce different than what we sent them... diff --git a/src/FreeDSx/Sasl/Challenge/PlainChallenge.php b/src/FreeDSx/Sasl/Challenge/PlainChallenge.php index 33ee263..3465366 100644 --- a/src/FreeDSx/Sasl/Challenge/PlainChallenge.php +++ b/src/FreeDSx/Sasl/Challenge/PlainChallenge.php @@ -68,9 +68,10 @@ private function serverProcess( if ($options->getValidate() === null) { throw new SaslException('You must pass a callable validate option to the plain mechanism in server mode.'); } - $authzId = $message->getString('authzid'); - $authcId = $message->getString('authcid') ?? ''; - $password = $message->getString('password') ?? ''; + $rawAuthzId = $message->get('authzid'); + $authzId = is_string($rawAuthzId) ? $rawAuthzId : null; + $authcId = $message->getString('authcid'); + $password = $message->getString('password'); $this->context->setIsComplete(true); $this->context->setIsAuthenticated(($options->getValidate())($authzId, $authcId, $password)); diff --git a/src/FreeDSx/Sasl/Challenge/ScramChallenge.php b/src/FreeDSx/Sasl/Challenge/ScramChallenge.php index 40a634b..98fa947 100644 --- a/src/FreeDSx/Sasl/Challenge/ScramChallenge.php +++ b/src/FreeDSx/Sasl/Challenge/ScramChallenge.php @@ -250,25 +250,23 @@ private function generateClientFinal( } # Guard against out-of-order calls: client-first must have run first to populate the cnonce. - $cnonce = $this->context->getString(self::CTX_CNONCE); - if ($cnonce === null) { - throw new SaslException( - 'client-final called before client-first: no client nonce found in context.', - ); - } + $cnonce = $this->context->getStringOrFail( + self::CTX_CNONCE, + 'client-final called before client-first: no client nonce found in context.', + ); # The full nonce must begin with the client nonce we sent — verify before processing anything else. [$fullNonce, $salt, $iterations] = $this->parseServerFirst($serverFirst, $cnonce); # Channel binding value: base64(gs2-header + cbind-data). - $gs2Header = $this->context->getString(self::CTX_GS2_HEADER) ?? ''; + $gs2Header = $this->context->getString(self::CTX_GS2_HEADER); $cbindData = $options->getCbindData() ?? ''; $channelBinding = base64_encode($gs2Header . $cbindData); $clientFinalWithoutProof = 'c=' . $channelBinding . ',r=' . $fullNonce; # RFC 5802 §3: AuthMessage = client-first-bare "," server-first "," client-final-without-proof - $authMessage = ($this->context->getString(self::CTX_CLIENT_FIRST_BARE) ?? '') + $authMessage = $this->context->getString(self::CTX_CLIENT_FIRST_BARE) . ',' . $rawServerFirst . ',' . $clientFinalWithoutProof; @@ -296,12 +294,12 @@ private function verifyServerFinal(Message $serverFinal): void if ($serverFinal->has('e')) { throw new SaslException(sprintf( 'SCRAM authentication failed with server error: %s', - $serverFinal->getString('e') ?? '', + $serverFinal->getString('e'), )); } - $expectedSig = $this->context->getString(self::CTX_SERVER_SIGNATURE) ?? ''; - $receivedSig = $serverFinal->getString('v') ?? ''; + $expectedSig = $this->context->getString(self::CTX_SERVER_SIGNATURE); + $receivedSig = $serverFinal->getString('v'); if (!hash_equals($expectedSig, $receivedSig)) { throw new SaslException('The server signature does not match the expected value.'); @@ -318,7 +316,7 @@ private function generateServerFirst( string $rawClientFirst, ScramOptions $options, ): string { - $cnonce = $clientFirst->getString('r') ?? ''; + $cnonce = $clientFirst->getString('r'); $snonce = $options->getNonce() ?? $this->generateNonce(self::NONCE_SIZE); $fullNonce = $cnonce . $snonce; @@ -363,19 +361,19 @@ private function generateServerFinal( return self::INVALID_PROOF; } - $fullNonce = $this->context->getString(self::CTX_NONCE) ?? ''; + $fullNonce = $this->context->getString(self::CTX_NONCE); $clientNonce = $clientFinal->getString('r'); if ($clientNonce !== $fullNonce) { return self::INVALID_PROOF; } - $salt = $this->context->getString(self::CTX_SALT) ?? ''; + $salt = $this->context->getString(self::CTX_SALT); $iterations = $this->context->getInt(self::CTX_ITERATIONS) ?? 1; # RFC 5802: client-final-without-proof = c=...,r=...[,extensions] — fixed order, safely rebuildable. - $clientFinalWithoutProof = 'c=' . ($clientFinal->getString('c') ?? '') . ',r=' . $clientNonce; - $authMessage = ($this->context->getString(self::CTX_CLIENT_FIRST_BARE) ?? '') - . ',' . ($this->context->getString(self::CTX_SERVER_FIRST) ?? '') + $clientFinalWithoutProof = 'c=' . $clientFinal->getString('c') . ',r=' . $clientNonce; + $authMessage = $this->context->getString(self::CTX_CLIENT_FIRST_BARE) + . ',' . $this->context->getString(self::CTX_SERVER_FIRST) . ',' . $clientFinalWithoutProof; try { @@ -430,13 +428,13 @@ private function parseServerFirst( Message $serverFirst, string $cnonce, ): array { - $fullNonce = $serverFirst->getString('r') ?? ''; + $fullNonce = $serverFirst->getString('r'); if (strncmp($fullNonce, $cnonce, strlen($cnonce)) !== 0) { throw new SaslException('The server nonce does not begin with the client nonce.'); } $salt = base64_decode( - $serverFirst->getString('s') ?? '', + $serverFirst->getString('s'), true, ); if ($salt === false) { @@ -500,7 +498,7 @@ private function isValidClientProof( $authMessage, ); $receivedProof = base64_decode( - $clientFinal->getString('p') ?? '', + $clientFinal->getString('p'), true, ); diff --git a/src/FreeDSx/Sasl/Encoder/AnonymousEncoder.php b/src/FreeDSx/Sasl/Encoder/AnonymousEncoder.php index f8e32e3..7ea29c1 100644 --- a/src/FreeDSx/Sasl/Encoder/AnonymousEncoder.php +++ b/src/FreeDSx/Sasl/Encoder/AnonymousEncoder.php @@ -28,7 +28,7 @@ public function encode( SaslContext $context, ): string { if ($message->has('trace')) { - return $message->getString('trace') ?? ''; + return $message->getString('trace'); } return ''; diff --git a/src/FreeDSx/Sasl/Encoder/CramMD5Encoder.php b/src/FreeDSx/Sasl/Encoder/CramMD5Encoder.php index 45631e9..1c92c3a 100644 --- a/src/FreeDSx/Sasl/Encoder/CramMD5Encoder.php +++ b/src/FreeDSx/Sasl/Encoder/CramMD5Encoder.php @@ -51,7 +51,7 @@ private function encodeServerChallenge(Message $message): string throw new SaslEncodingException('The server challenge message must contain a "challenge".'); } - return '<' . ($message->getString('challenge') ?? '') . '>'; + return '<' . ($message->getString('challenge')) . '>'; } /** @@ -65,8 +65,8 @@ private function encodeClientResponse(Message $message): string if (!$message->has('digest')) { throw new SaslEncodingException('The client response must contain a digest.'); } - $username = $message->getString('username') ?? ''; - $digest = $message->getString('digest') ?? ''; + $username = $message->getString('username'); + $digest = $message->getString('digest'); if (preg_match('/^[0-9a-f]{32}$/', $digest) !== 1) { throw new SaslEncodingException('The client digest must be a 16 octet, lower-case, hexadecimal value'); diff --git a/src/FreeDSx/Sasl/Encoder/DigestMD5Encoder.php b/src/FreeDSx/Sasl/Encoder/DigestMD5Encoder.php index da93a7f..7eb7612 100644 --- a/src/FreeDSx/Sasl/Encoder/DigestMD5Encoder.php +++ b/src/FreeDSx/Sasl/Encoder/DigestMD5Encoder.php @@ -181,15 +181,15 @@ private function encodeOptValue( ): string { return match ($name) { 'realm', 'nonce', 'username', 'cnonce', 'authzid', 'digest-uri' - => '"' . str_replace(['\\', '"'], ['\\\\', '\"'], $message->getString($name) ?? '') . '"', + => '"' . str_replace(['\\', '"'], ['\\\\', '\"'], $message->getString($name)) . '"', 'qop', 'cipher' => $isServerMode ? '"' . implode(',', $message->getStringArray($name) ?? []) . '"' - : ($message->getString($name) ?? ''), + : $message->getString($name), 'stale' => 'true', - 'maxbuf', 'algorithm', 'charset' => $message->getString($name) ?? '', + 'maxbuf', 'algorithm', 'charset' => $message->getString($name), 'nc' => str_pad(dechex($message->getInt($name) ?? 0), 8, '0', STR_PAD_LEFT), - 'response', 'rspauth' => $this->encodeLHexValue($message->getString($name) ?? '', 32), + 'response', 'rspauth' => $this->encodeLHexValue($message->getString($name), 32), default => throw new SaslEncodingException(sprintf( 'Digest option %s is not supported.', $name, diff --git a/src/FreeDSx/Sasl/Encoder/PlainEncoder.php b/src/FreeDSx/Sasl/Encoder/PlainEncoder.php index ed9adea..f084cb9 100644 --- a/src/FreeDSx/Sasl/Encoder/PlainEncoder.php +++ b/src/FreeDSx/Sasl/Encoder/PlainEncoder.php @@ -37,9 +37,9 @@ public function encode( if (!$message->has('password')) { throw new SaslEncodingException('The PLAIN message must contain a password.'); } - $authzid = $this->validate($message->getString('authzid') ?? ''); - $authcid = $this->validate($message->getString('authcid') ?? ''); - $password = $this->validate($message->getString('password') ?? ''); + $authzid = $this->validate($message->getString('authzid')); + $authcid = $this->validate($message->getString('authcid')); + $password = $this->validate($message->getString('password')); return $authzid . "\x00" . $authcid . "\x00" . $password; } diff --git a/src/FreeDSx/Sasl/Encoder/ScramEncoder.php b/src/FreeDSx/Sasl/Encoder/ScramEncoder.php index 69e74a9..4157456 100644 --- a/src/FreeDSx/Sasl/Encoder/ScramEncoder.php +++ b/src/FreeDSx/Sasl/Encoder/ScramEncoder.php @@ -43,7 +43,7 @@ public function encode( ): string { $parts = []; foreach (array_keys($message->toArray()) as $attr) { - $parts[] = $attr . '=' . ($message->getString($attr) ?? ''); + $parts[] = $attr . '=' . ($message->getString($attr)); } return implode(',', $parts); diff --git a/src/FreeDSx/Sasl/Factory/DigestMD5MessageFactory.php b/src/FreeDSx/Sasl/Factory/DigestMD5MessageFactory.php index 9224f4d..ac4ba32 100644 --- a/src/FreeDSx/Sasl/Factory/DigestMD5MessageFactory.php +++ b/src/FreeDSx/Sasl/Factory/DigestMD5MessageFactory.php @@ -145,7 +145,7 @@ private function getDigestUri( return sprintf( '%s/%s', is_string($options['service'] ?? null) ? $options['service'] : '', - $response->getString('realm') ?? '', + $response->getString('realm'), ); } @@ -293,11 +293,9 @@ private function getCurrentUser(): string */ private function getRealmFromChallenge(Message $challenge): string { - $realm = $challenge->getString('realm'); - if ($realm === null) { - throw new SaslException('Unable to determine a realm for the response.'); - } - - return $realm; + return $challenge->getStringOrFail( + 'realm', + 'Unable to determine a realm for the response.', + ); } } diff --git a/src/FreeDSx/Sasl/Mechanism/DigestMD5Mechanism.php b/src/FreeDSx/Sasl/Mechanism/DigestMD5Mechanism.php index ebda625..68b2851 100644 --- a/src/FreeDSx/Sasl/Mechanism/DigestMD5Mechanism.php +++ b/src/FreeDSx/Sasl/Mechanism/DigestMD5Mechanism.php @@ -92,7 +92,7 @@ public static function computeResponse( $a1 = self::computeA1($password, $challenge, $response); $qop = $response->getString('qop'); - $digestUri = $response->getString('digest-uri') ?? ''; + $digestUri = $response->getString('digest-uri'); $a2 = $useServerMode ? self::A2_SERVER : self::A2_CLIENT; $a2 .= match ($qop) { @@ -105,10 +105,10 @@ public static function computeResponse( return hash('md5', sprintf( '%s:%s:%s:%s:%s:%s', $a1, - $challenge->getString('nonce') ?? '', + $challenge->getString('nonce'), str_pad(dechex($response->getIntOrParse('nc') ?? 0), 8, '0', STR_PAD_LEFT), - $response->getString('cnonce') ?? '', - $response->getString('qop') ?? '', + $response->getString('cnonce'), + $response->getString('qop'), $a2, )); } @@ -131,18 +131,18 @@ public static function computeA1( ): string { $a1 = hash('md5', sprintf( '%s:%s:%s', - $response->getString('username') ?? '', - $response->getString('realm') ?? '', + $response->getString('username'), + $response->getString('realm'), $password, ), true); $a1 = sprintf( '%s:%s:%s', $a1, - $challenge->getString('nonce') ?? '', - $response->getString('cnonce') ?? '', + $challenge->getString('nonce'), + $response->getString('cnonce'), ); if ($response->has('authzid')) { - $a1 .= ':' . ($response->getString('authzid') ?? ''); + $a1 .= ':' . $response->getString('authzid'); } return hash('md5', $a1); diff --git a/src/FreeDSx/Sasl/Message.php b/src/FreeDSx/Sasl/Message.php index b9f8045..6dc1bf2 100644 --- a/src/FreeDSx/Sasl/Message.php +++ b/src/FreeDSx/Sasl/Message.php @@ -15,6 +15,7 @@ use ArrayIterator; use Countable; +use FreeDSx\Sasl\Exception\SaslException; use IteratorAggregate; use Traversable; @@ -40,13 +41,33 @@ public function get(string $name): mixed return $this->data[$name] ?? null; } - public function getString(string $name): ?string + public function getString(string $name): string { $value = $this->data[$name] ?? null; return is_string($value) ? $value - : null; + : ''; + } + + /** + * @throws SaslException + */ + public function getStringOrFail( + string $name, + string $errorMessage = '', + ): string { + $value = $this->data[$name] ?? null; + + if (!is_string($value)) { + throw new SaslException( + $errorMessage !== '' + ? $errorMessage + : sprintf('The required field "%s" is missing.', $name), + ); + } + + return $value; } public function getInt(string $name): ?int diff --git a/src/FreeDSx/Sasl/SaslContext.php b/src/FreeDSx/Sasl/SaslContext.php index e3591e4..c6d2a89 100644 --- a/src/FreeDSx/Sasl/SaslContext.php +++ b/src/FreeDSx/Sasl/SaslContext.php @@ -13,6 +13,8 @@ namespace FreeDSx\Sasl; +use FreeDSx\Sasl\Exception\SaslException; + /** * Holds SASL context specific data related to a particular mechanism challenge / response. * @@ -125,10 +127,28 @@ public function get(string $key): mixed return $this->data[$key] ?? null; } - public function getString(string $key): ?string + public function getString(string $key): string { $value = $this->data[$key] ?? null; - return is_string($value) ? $value : null; + + return is_string($value) + ? $value + : ''; + } + + /** + * @throws SaslException + */ + public function getStringOrFail(string $key, string $errorMessage = ''): string + { + $value = $this->data[$key] ?? null; + if (!is_string($value)) { + throw new SaslException( + $errorMessage !== '' ? $errorMessage : sprintf('The required key "%s" is missing.', $key), + ); + } + + return $value; } public function getInt(string $key): ?int diff --git a/src/FreeDSx/Sasl/Security/DigestMD5SecurityLayer.php b/src/FreeDSx/Sasl/Security/DigestMD5SecurityLayer.php index bd692ca..763be71 100644 --- a/src/FreeDSx/Sasl/Security/DigestMD5SecurityLayer.php +++ b/src/FreeDSx/Sasl/Security/DigestMD5SecurityLayer.php @@ -49,7 +49,7 @@ public function wrap( 'auth-int' => $this->sign($data, $context), default => throw new SaslException(sprintf( 'The qop option "%s" is not recognized as a security layer.', - $qop ?? 'null', + $qop, )), }; $this->validateBufferLength($wrapped, $context); @@ -70,7 +70,7 @@ public function unwrap( 'auth-int' => $this->verify($data, $context), default => throw new SaslException(sprintf( 'The qop option "%s" is not recognized as a security layer.', - $qop ?? 'null', + $qop, )), }; $context->set('seqnumrcv', ($context->getInt('seqnumrcv') ?? 0) + 1); @@ -107,7 +107,7 @@ private function decrypt( } $cipher = $this->resolveCipher($context); - $a1 = $context->getString('a1') ?? ''; + $a1 = $context->getString('a1'); $isServerMode = $context->isServerMode(); $encrypted = substr($data, 0, -6); @@ -152,7 +152,7 @@ private function encrypt( SaslContext $context, ): string { $cipher = $this->resolveCipher($context); - $a1 = $context->getString('a1') ?? ''; + $a1 = $context->getString('a1'); $isServerMode = $context->isServerMode(); $seqnum = $context->getInt('seqnumsnt') ?? 0; @@ -234,7 +234,7 @@ private function sign( ): string { $seqnum = $context->getInt('seqnumsnt') ?? 0; $mc = $context->isServerMode() ? self::KIS_MC : self::KIC_MC; - $ki = $this->generateKeyKi($context->getString('a1') ?? '', $mc); + $ki = $this->generateKeyKi($context->getString('a1'), $mc); $macBlock = $this->generateMACBlock($ki, $message, $seqnum); return $message . $macBlock; @@ -258,7 +258,7 @@ private function verify( $message = substr($data, 0, -16); # Inverted selection of constant here, as this would be the receiving end. $mc = $context->isServerMode() ? self::KIC_MC : self::KIS_MC; - $ki = $this->generateKeyKi($context->getString('a1') ?? '', $mc); + $ki = $this->generateKeyKi($context->getString('a1'), $mc); $expectedMac = $this->generateMACBlock($ki, $message, $seqnum); if ($receivedMac !== $expectedMac) { @@ -400,7 +400,7 @@ private function validateBufferLength( string $data, SaslContext $context, ): void { - $maxbuf = $context->has('maxbuf') ? (int) ($context->getString('maxbuf') ?? '') : self::MAXBUF; + $maxbuf = $context->has('maxbuf') ? (int) $context->getString('maxbuf') : self::MAXBUF; if (strlen($data) > $maxbuf) { throw new SaslException(sprintf( 'The wrapped buffer exceeds the maxbuf length of %s',