From c8d84da7e3814950fea2a865e4180d003cb87b9d Mon Sep 17 00:00:00 2001 From: Kevin Jansen Date: Thu, 1 Feb 2024 10:53:22 +0100 Subject: [PATCH 1/2] Revert "Add fallback verification method to the HMAC implementation" This reverts commit 2b6003b7112b06be86be8cca14868182ed5f619e. --- src/Methods/HmacSignature.php | 42 ++---------------------- tests/Unit/Methods/HmacSignatureTest.php | 15 --------- 2 files changed, 3 insertions(+), 54 deletions(-) diff --git a/src/Methods/HmacSignature.php b/src/Methods/HmacSignature.php index cd72779..d5d0b9d 100644 --- a/src/Methods/HmacSignature.php +++ b/src/Methods/HmacSignature.php @@ -37,7 +37,7 @@ public function __construct(HmacSignatureKeyRepositoryInterface $signatureKeyRep */ public function sign(RequestInterface $request, string $keyId, string $algorithm): RequestInterface { - $signatureData = $this->generateSignature((string)$request->getBody(), $keyId, $algorithm); + $signatureData = $this->generateSignature((string) $request->getBody(), $keyId, $algorithm); return $request ->withHeader(RequestSigningMethodInterface::SIGNATURE_HEADER, $signatureData->getSignature()) @@ -90,10 +90,10 @@ public function verify(RequestInterface $request): bool $algorithm = $request->getHeaderLine(RequestSigningMethodInterface::SIGNATURE_ALGORITHM_HEADER); $generatedSignature = $this - ->generateSignature((string)$request->getBody(), $keyId, $algorithm) + ->generateSignature((string) $request->getBody(), $keyId, $algorithm) ->getSignature(); - return hash_equals($signature, $generatedSignature) || $this->doFallbackVerification($request); + return hash_equals($signature, $generatedSignature); } catch (Throwable $throwable) { // We do nothing with the exception. The request stays marked as "invalid". } @@ -105,40 +105,4 @@ public function supports(string $method): bool { return strtolower(self::METHOD_NAME) === strtolower($method); } - - /** - * Since we've moved away from hash() signing in favor of hash_hmac we should (for now) support the - * hash verification method to keep older implementations valid - * - * @throws UnsupportedHashingAlgorithmException - * @throws SignatureKeyNotFoundException - */ - private function doFallbackVerification(RequestInterface $request): bool - { - $keyId = $request->getHeaderLine(RequestSigningMethodInterface::SIGNATURE_KEY_ID_HEADER); - $signature = $request->getHeaderLine(RequestSigningMethodInterface::SIGNATURE_HEADER); - $algorithm = $request->getHeaderLine(RequestSigningMethodInterface::SIGNATURE_ALGORITHM_HEADER); - - $key = $this->signatureKeyRepository->findOneById($keyId); - - $algorithm = strtolower($algorithm); - - if (in_array($algorithm, hash_algos()) === false) { - throw UnsupportedHashingAlgorithmException::forAlgorithm($algorithm); - } - - $generatedSignature = hash( - $algorithm, - implode( - ':', - [ - $key->getId(), - $key->getSecret(), - (string)$request->getBody(), - ] - ) - ); - - return hash_equals($signature, $generatedSignature); - } } diff --git a/tests/Unit/Methods/HmacSignatureTest.php b/tests/Unit/Methods/HmacSignatureTest.php index f3297b8..08d76ee 100644 --- a/tests/Unit/Methods/HmacSignatureTest.php +++ b/tests/Unit/Methods/HmacSignatureTest.php @@ -122,21 +122,6 @@ public function testItReturnsFalseForAnHashingAlgorithm(): void $this->assertFalse($hmacSignature->verify($request)); } - public function testItCanVerifyUsingTheFallbackMethod(): void - { - // First, we'll create a dummy request - $request = $this->getSignedDummyRequest()->withHeader( - RequestSigningMethodInterface::SIGNATURE_HEADER, - '6086a395c7fe9b47f47cee4623b76cc4cc79cb3f44e968091edc447cfe7e7533dc2564f49b22bc1f225c24f172be0d2a5b5893e0825c6fde782a63f3e43ce7d1' - ); - - // Next, we'll instantiate the HmacSignature class - $hmacSignature = new HmacSignature($this->getKeyRepository($this->getDummySignatureKey())); - - // Next, we'll verify that this request passes its verification - $this->assertTrue($hmacSignature->verify($request)); - } - private function getDummyRequest(): RequestInterface { return new Request('POST', 'https://pay.nl'); From b34c1824e1eb6f98e3335d5ffd3b7ab0b9b14a86 Mon Sep 17 00:00:00 2001 From: Kevin Jansen Date: Thu, 1 Feb 2024 11:58:21 +0100 Subject: [PATCH 2/2] Correct singing to signing typo --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index d41193e..c1d150c 100644 --- a/README.md +++ b/README.md @@ -23,12 +23,12 @@ composer require paynl/request-signing The `PayNL\RequestSigning\RequestSigningService` simplifies both signing and verifying requests, avoiding the need for manual class instantiation. -The constructor function of this service requires an array of _SingingMethods_ that you wish to support. Each signing method has different needs and functionality, and which one(s) you opt to support falls under your discretion. +The constructor function of this service requires an array of _SigningMethods_ that you wish to support. Each signing method has different needs and functionality, and which one(s) you opt to support falls under your discretion. By providing the service with the array of these chosen methods, the service can handle the configuration and functionality. This decouples the setup process from your main application logic, allowing a more streamlined integration. ### Signing -The below code snippet shows the basis of singing a request using this package: +The below code snippet shows the basis of signing a request using this package: ```php use PayNL\RequestSigning\RequestSigningService; @@ -73,7 +73,7 @@ $requestValid = $signingService->verify($request); The request signing service and the underlying signing / verifying methods may throw exceptions when unexpected values are encountered, these are: - SignatureKeyNotFound, this exception must be thrown when the implementation of the `PayNL\RequestSigning\Repository\SignatureKeyRepositoryInterface` can not find the key based on the provided id; -- UnknownSigningMethodException, this exception will be thrown by the singing / verifying methods when they are requested to sign / verify a request with an algorithm they do not support; +- UnknownSigningMethodException, this exception will be thrown by the signing / verifying methods when they are requested to sign / verify a request with an algorithm they do not support; - UnsupportedHashingAlgorithmException, this exception will be thrown by the `PayNL\RequestSigning\RequestSigningService` when it is requested to sign / verify a request with a method it doesn't support. ### Supported Signing / Verifying methods