From 76e54914f64594262216948163fbc2fb02cc8d82 Mon Sep 17 00:00:00 2001 From: Sean O'Brien <60306702+stobrien89@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:03:11 -0400 Subject: [PATCH] reject conflicting SNS URL key aliases --- README.md | 8 ++ src/Message.php | 27 ++++++ src/MessageValidator.php | 48 +++++++---- tests/MessageTest.php | 47 +++++++++++ tests/MessageValidatorTest.php | 146 ++++++++++++++++++++++++++++++++- tests/MockPhpStream.php | 2 + 6 files changed, 261 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 9ba8632..5f6c56d 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,14 @@ if ($message['Type'] === 'SubscriptionConfirmation') { } ``` +After `validate()` succeeds, Lambda-style URL keys such as `SubscribeUrl` are +normalized to their canonical SNS spelling, such as `SubscribeURL`, on the +`Message` object. + +* Before requesting `SubscribeURL`, verify that it uses HTTPS and that its host + matches the expected regional SNS endpoint, such as + `sns..amazonaws.com`. + ### Receiving a Notification To receive a notification, use the same code as the preceding example, but diff --git a/src/Message.php b/src/Message.php index 04a71dd..94b38f6 100644 --- a/src/Message.php +++ b/src/Message.php @@ -24,6 +24,12 @@ class Message implements \ArrayAccess, \IteratorAggregate 'Token' ]; + private static $keyAliases = [ + ['SigningCertURL', 'SigningCertUrl'], + ['SubscribeURL', 'SubscribeUrl'], + ['UnsubscribeURL', 'UnsubscribeUrl'], + ]; + /** @var array The message data */ private $data; @@ -81,6 +87,8 @@ public static function fromJsonString($requestBody) */ public function __construct(array $data) { + $this->validateKeyAliases($data); + // Ensure that all the required keys for the message's type are present. $this->validateRequiredKeys($data, self::$requiredKeys); if ($data['Type'] === 'SubscriptionConfirmation' @@ -132,6 +140,25 @@ public function toArray() return $this->data; } + private function validateKeyAliases(array $data) + { + foreach (self::$keyAliases as $keyAliases) { + $present = []; + foreach ($keyAliases as $keyAlias) { + if (array_key_exists($keyAlias, $data)) { + $present[] = $keyAlias; + } + } + + if (count($present) > 1) { + throw new \InvalidArgumentException(sprintf( + 'The SNS message contains multiple spellings of the same field: %s.', + implode(', ', $present) + )); + } + } + } + private function validateRequiredKeys(array $data, array $keys) { foreach ($keys as $key) { diff --git a/src/MessageValidator.php b/src/MessageValidator.php index 4697a31..b8e9f73 100644 --- a/src/MessageValidator.php +++ b/src/MessageValidator.php @@ -30,26 +30,28 @@ class MessageValidator private static function isLambdaStyle(Message $message) { - return isset($message['SigningCertUrl']); + $messageData = $message->toArray(); + + return array_key_exists('SigningCertUrl', $messageData) + || array_key_exists('SubscribeUrl', $messageData) + || array_key_exists('UnsubscribeUrl', $messageData); } - private static function convertLambdaMessage(Message $lambdaMessage) + private static function convertLambdaMessage(Message $message) { + $messageData = $message->toArray(); $keyReplacements = [ 'SigningCertUrl' => 'SigningCertURL', 'SubscribeUrl' => 'SubscribeURL', 'UnsubscribeUrl' => 'UnsubscribeURL', ]; - $message = clone $lambdaMessage; foreach ($keyReplacements as $lambdaKey => $canonicalKey) { - if (isset($message[$lambdaKey])) { + if (array_key_exists($lambdaKey, $messageData)) { $message[$canonicalKey] = $message[$lambdaKey]; unset($message[$lambdaKey]); } } - - return $message; } /** @@ -74,6 +76,9 @@ public function __construct( /** * Validates a message from SNS to ensure that it was delivered by AWS. * + * Lambda-style URL keys are canonicalized on the provided message after + * successful validation. + * * @param Message $message Message to validate. * * @throws InvalidSnsMessageException If the cert cannot be retrieved or its @@ -82,16 +87,22 @@ public function __construct( */ public function validate(Message $message) { + $messageToValidate = $message; if (self::isLambdaStyle($message)) { - $message = self::convertLambdaMessage($message); + $messageToValidate = clone $message; + self::convertLambdaMessage($messageToValidate); } // Get the certificate. - $this->validateUrl($message['SigningCertURL']); - $certificate = call_user_func($this->certClient, $message['SigningCertURL']); + $certUrl = $messageToValidate['SigningCertURL']; + $this->validateUrl($certUrl); + $certificate = call_user_func( + $this->certClient, + $certUrl + ); if ($certificate === false) { throw new InvalidSnsMessageException( - "Cannot get the certificate from \"{$message['SigningCertURL']}\"." + "Cannot get the certificate from \"{$certUrl}\"." ); } @@ -104,20 +115,29 @@ public function validate(Message $message) } // Verify the signature of the message. - $content = $this->getStringToSign($message); - $signature = base64_decode($message['Signature']); - $algo = ($message['SignatureVersion'] === self::SIGNATURE_VERSION_1 ? OPENSSL_ALGO_SHA1 : OPENSSL_ALGO_SHA256); + $content = $this->getStringToSign($messageToValidate); + $signature = base64_decode($messageToValidate['Signature']); + $algo = $messageToValidate['SignatureVersion'] + === self::SIGNATURE_VERSION_1 + ? OPENSSL_ALGO_SHA1 + : OPENSSL_ALGO_SHA256; if (openssl_verify($content, $signature, $key, $algo) !== 1) { throw new InvalidSnsMessageException( 'The message signature is invalid.' ); } + + if ($messageToValidate !== $message) { + self::convertLambdaMessage($message); + } } /** * Determines if a message is valid and that is was delivered by AWS. This * method does not throw exceptions and returns a simple boolean value. * + * The provided message is not modified. + * * @param Message $message The message to validate * * @return bool @@ -125,7 +145,7 @@ public function validate(Message $message) public function isValid(Message $message) { try { - $this->validate($message); + $this->validate(clone $message); return true; } catch (InvalidSnsMessageException $e) { return false; diff --git a/tests/MessageTest.php b/tests/MessageTest.php index 91ed755..26084ca 100644 --- a/tests/MessageTest.php +++ b/tests/MessageTest.php @@ -102,6 +102,39 @@ public function testRequiresTokenAndSubscribeUrlForUnsubscribeMessage() ); } + public function testRejectsMessageWithBothSigningCertKeySpellings() + { + $this->assertDuplicateAliasesRejected( + ['SigningCertUrl' => 'alternate'] + $this->messageData, + 'SigningCertURL', + 'SigningCertUrl' + ); + } + + public function testRejectsMessageWithBothSubscribeKeySpellings() + { + $this->assertDuplicateAliasesRejected( + [ + 'Type' => 'SubscriptionConfirmation', + 'SubscribeUrl' => 'alternate', + ] + $this->messageData, + 'SubscribeURL', + 'SubscribeUrl' + ); + } + + public function testRejectsMessageWithBothUnsubscribeKeySpellings() + { + $this->assertDuplicateAliasesRejected( + [ + 'UnsubscribeURL' => 'canonical', + 'UnsubscribeUrl' => 'alternate', + ] + $this->messageData, + 'UnsubscribeURL', + 'UnsubscribeUrl' + ); + } + public function testCanCreateFromRawPost() { $_SERVER['HTTP_X_AMZ_SNS_MESSAGE_TYPE'] = 'Notification'; @@ -167,4 +200,18 @@ public function testArrayAccess() unset($message['foo']); $this->assertFalse(isset($message['foo'])); } + + private function assertDuplicateAliasesRejected( + array $messageData, + $canonicalKey, + $alternateKey + ) { + try { + new Message($messageData); + $this->fail('Expected duplicate field spellings to be rejected.'); + } catch (\InvalidArgumentException $e) { + $this->assertNotFalse(strpos($e->getMessage(), $canonicalKey)); + $this->assertNotFalse(strpos($e->getMessage(), $alternateKey)); + } + } } diff --git a/tests/MessageValidatorTest.php b/tests/MessageValidatorTest.php index 75501b1..7597aa2 100644 --- a/tests/MessageValidatorTest.php +++ b/tests/MessageValidatorTest.php @@ -20,12 +20,11 @@ public static function set_up_before_class() $csr = openssl_csr_new([], self::$pKey); $x509 = openssl_csr_sign($csr, null, self::$pKey, 1); openssl_x509_export($x509, self::$certificate); - openssl_x509_free($x509); } public static function tear_down_after_class() { - openssl_pkey_free(self::$pKey); + self::$pKey = null; } public function testIsValidReturnsFalseOnFailedValidation() @@ -88,7 +87,9 @@ function () { public function testValidateFailsWhenCannotGetCertificate() { $this->expectException(InvalidSnsMessageException::class); - $this->expectDeprecationMessageMatches('/Cannot get the certificate from ".+"./'); + $this->expectExceptionMessage( + 'Cannot get the certificate from "' . self::VALID_CERT_URL . '".' + ); $validator = new MessageValidator($this->getMockHttpClient(false)); $message = $this->getTestMessage(); $validator->validate($message); @@ -153,6 +154,120 @@ public function testValidateSucceedsWhenSha256MessageIsValid() $this->assertTrue($validator->isValid($message)); } + public function testLambdaStyleDetectionTriggersOnSubscribeUrlAlone() + { + $validator = new MessageValidator($this->getMockCertServerClient()); + $message = $this->getSignedTestMessage($validator, [ + 'Type' => 'SubscriptionConfirmation', + 'SubscribeURL' => 'https://sns.foo.amazonaws.com/subscribe', + 'Token' => 'token', + ]); + $message = $this->renameMessageKey( + $message, + 'SubscribeURL', + 'SubscribeUrl' + ); + + $validator->validate($message); + + $this->assertSame( + 'https://sns.foo.amazonaws.com/subscribe', + $message['SubscribeURL'] + ); + $this->assertFalse(isset($message['SubscribeUrl'])); + } + + public function testLambdaStyleDetectionTriggersOnUnsubscribeUrlAlone() + { + $validator = new MessageValidator($this->getMockCertServerClient()); + $message = $this->getSignedTestMessage($validator, [ + 'UnsubscribeURL' => 'https://sns.foo.amazonaws.com/unsubscribe', + ]); + $message = $this->renameMessageKey( + $message, + 'UnsubscribeURL', + 'UnsubscribeUrl' + ); + + $validator->validate($message); + + $this->assertSame( + 'https://sns.foo.amazonaws.com/unsubscribe', + $message['UnsubscribeURL'] + ); + $this->assertFalse(isset($message['UnsubscribeUrl'])); + } + + public function testValidateWritesCanonicalKeysBackToCallerMessage() + { + $validator = new MessageValidator($this->getMockCertServerClient()); + $message = $this->getSignedTestMessage($validator); + $message = $this->renameMessageKey( + $message, + 'SigningCertURL', + 'SigningCertUrl' + ); + + $validator->validate($message); + + $this->assertSame(self::VALID_CERT_URL, $message['SigningCertURL']); + $this->assertFalse(isset($message['SigningCertUrl'])); + } + + public function testValidatePreservesCanonicalOnlyPayload() + { + $validator = new MessageValidator($this->getMockCertServerClient()); + $message = $this->getSignedTestMessage($validator); + $messageData = $message->toArray(); + + $validator->validate($message); + + $this->assertSame($messageData, $message->toArray()); + } + + public function testIsValidDoesNotMutateLambdaStyleMessage() + { + $validator = new MessageValidator($this->getMockCertServerClient()); + $message = $this->getSignedTestMessage($validator, [ + 'UnsubscribeURL' => 'https://sns.foo.amazonaws.com/unsubscribe', + ]); + $message = $this->renameMessageKey( + $message, + 'SigningCertURL', + 'SigningCertUrl' + ); + $message = $this->renameMessageKey( + $message, + 'UnsubscribeURL', + 'UnsubscribeUrl' + ); + $messageData = $message->toArray(); + + $this->assertTrue($validator->isValid($message)); + $this->assertSame($messageData, $message->toArray()); + } + + public function testFailedValidateDoesNotCanonicalizeCallerMessage() + { + $validator = new MessageValidator($this->getMockCertServerClient()); + $message = $this->getTestMessage([ + 'Signature' => $this->getSignature('invalid'), + ]); + $message = $this->renameMessageKey( + $message, + 'SigningCertURL', + 'SigningCertUrl' + ); + + try { + $validator->validate($message); + $this->fail('Expected validation to fail.'); + } catch (InvalidSnsMessageException $e) { + $this->assertSame(self::VALID_CERT_URL, $message['SigningCertUrl']); + $this->assertFalse(isset($message['SigningCertURL'])); + } + } + public function testBuildsStringToSignCorrectly() { $validator = new MessageValidator(); @@ -195,6 +310,31 @@ private function getTestMessage(array $customData = []) ]); } + private function getSignedTestMessage( + MessageValidator $validator, + array $customData = [] + ) { + $message = $this->getTestMessage($customData); + $message['Signature'] = $this->getSignature( + $validator->getStringToSign($message), + $message['SignatureVersion'] + ); + + return $message; + } + + private function renameMessageKey( + Message $message, + $canonicalKey, + $alternateKey + ) { + $messageData = $message->toArray(); + $messageData[$alternateKey] = $messageData[$canonicalKey]; + unset($messageData[$canonicalKey]); + + return new Message($messageData); + } + private function getMockHttpClient($responseBody = '') { return function () use ($responseBody) { diff --git a/tests/MockPhpStream.php b/tests/MockPhpStream.php index 445be93..92ee44f 100644 --- a/tests/MockPhpStream.php +++ b/tests/MockPhpStream.php @@ -3,6 +3,8 @@ class MockPhpStream { + public $context; + private static $startingData = ''; private $index; private $length;