From adc5fead65f666f8723afc124773d3f79cf16b4d Mon Sep 17 00:00:00 2001
From: Michael Dowling <mtdowling@gmail.com>
Date: Wed, 30 Jul 2014 17:14:34 -0700
Subject: [PATCH] Adding stricter validation of SNS SigningCertURL

Updating the SNS MessageValidator to ensure that the SigningCertURL
is from a trusted source: the host is a known AWS host, the scheme
is http or https, and the file extension is ".pem".
---
 .../Sns/MessageValidator/MessageValidator.php | 17 +++++--
 .../MessageValidator/MessageValidatorTest.php | 47 ++++++++++++++++---
 2 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/src/Aws/Sns/MessageValidator/MessageValidator.php b/src/Aws/Sns/MessageValidator/MessageValidator.php
index 3db051eae0..113453e28a 100644
--- a/src/Aws/Sns/MessageValidator/MessageValidator.php
+++ b/src/Aws/Sns/MessageValidator/MessageValidator.php
@@ -64,9 +64,7 @@ public function validate(Message $message)
     {
         // Get the cert's URL and ensure it is from AWS
         $certUrl = Url::factory($message->get('SigningCertURL'));
-        if ('.amazonaws.com' != substr($certUrl->getHost(), -14)) {
-            throw new CertificateFromUnrecognizedSourceException();
-        }
+        $this->validateUrl($certUrl);
 
         // Get the cert itself and extract the public key
         $certificate = $this->client->get((string) $certUrl)->send()->getBody();
@@ -83,6 +81,19 @@ public function validate(Message $message)
         }
     }
 
+    private function validateUrl(Url $url)
+    {
+        // The host must match the following pattern
+        $hostPattern = '/^sns\.[a-zA-Z0-9\-]{3,}\.amazonaws\.com(\.cn)?$/';
+
+        if ($url->getScheme() !== 'https' ||
+            substr($url, -4) !== '.pem' ||
+            !preg_match($hostPattern, $url->getHost())
+        ) {
+            throw new CertificateFromUnrecognizedSourceException();
+        }
+    }
+
     /**
      * 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.
diff --git a/tests/Aws/Tests/Sns/MessageValidator/MessageValidatorTest.php b/tests/Aws/Tests/Sns/MessageValidator/MessageValidatorTest.php
index 8152e76a91..e235bec3f2 100644
--- a/tests/Aws/Tests/Sns/MessageValidator/MessageValidatorTest.php
+++ b/tests/Aws/Tests/Sns/MessageValidator/MessageValidatorTest.php
@@ -16,9 +16,11 @@
 
 namespace Aws\Tests\Sns\MessageValidator;
 
+use Aws\Sns\MessageValidator\Exception\CertificateFromUnrecognizedSourceException;
 use Aws\Sns\MessageValidator\Message;
 use Aws\Sns\MessageValidator\MessageValidator;
 use Guzzle\Common\Collection;
+use Guzzle\Http\Url;
 use Guzzle\Plugin\Mock\MockPlugin;
 use Guzzle\Http\Message\Response;
 use Guzzle\Http\Client;
@@ -42,13 +44,44 @@ public function testIsValidReturnsFalseOnFailedValidation()
         $this->assertFalse($validator->isValid($message));
     }
 
-    public function testValidateFailsWhenCertUrlDoesNotMatchAws()
+    public function urlProvider()
     {
-        $this->setExpectedException('Aws\Sns\MessageValidator\Exception\CertificateFromUnrecognizedSourceException');
+        return array(
+            array('https://sns.us-east-1.amazonaws.com/key.pem', false),
+            array('https://sns.us-west-2.amazonaws.com/key.pem', false),
+            array('https://sns.us-west-2.amazonaws.com/key.pem', false),
+            array('https://sns.cn-north-1.amazonaws.com.cn/SimpleNotificationService-foo.pem', false),
+            array('https://sns.us-gov-west-1.amazonaws.com/abc.pem', false),
+            // Failure cases
+            array('http://sns.us-gov-west-1.amazonaws.com/abc.pem', true),
+            array('https://sns.us-gov-west-1.amazonaws.com/abc', true),
+            array('https://sns.amazonaws.com/abc', true),
+            array('https://s3.sns.amazonaws.com/abc.pem', true),
+            array('https://s3.amazonaws.com/sns/abc.pem', true),
+            array('https://sns.us-west-2.amazon.com/key.pem', true),
+            array('https://sns.s3.amazonaws.com/key.pem', true),
+        );
+    }
 
+    /**
+     * @dataProvider urlProvider
+     */
+    public function testValidateFailsWhenCertUrlDoesNotMatchAws($url, $throws)
+    {
         $validator = new MessageValidator();
-        $message = new Message(new Collection());
-        $validator->validate($message);
+        $m = new \ReflectionMethod($validator, 'validateUrl');
+        $m->setAccessible(true);
+
+        try {
+            $m->invoke($validator, Url::factory($url));
+            if ($throws) {
+                $this->fail('URL was invalid but did not fail the test');
+            }
+        } catch (CertificateFromUnrecognizedSourceException $e) {
+            if (!$throws) {
+                $this->fail('URL was supposed to be valid, but failed test');
+            }
+        }
     }
 
     public function testValidateFailsWhenCannotDeterminePublicKey()
@@ -59,7 +92,7 @@ public function testValidateFailsWhenCannotDeterminePublicKey()
         $client = $this->getMockClient();
         $validator = new MessageValidator($client);
 
-        $message = new Message(new Collection(array('SigningCertURL' => 'https://foo.amazonaws.com/bar')));
+        $message = new Message(new Collection(array('SigningCertURL' => 'https://sns.foo.amazonaws.com/bar.pem')));
         $validator->validate($message);
     }
 
@@ -75,7 +108,7 @@ public function testValidateFailsWhenMessageIsInvalid()
         $validator = new MessageValidator($client);
 
         $message = new Message(new Collection(array(
-            'SigningCertURL' => 'https://foo.amazonaws.com/bar',
+            'SigningCertURL' => 'https://sns.foo.amazonaws.com/bar.pem',
             'Signature'      => $signature,
         )));
         $validator->validate($message);
@@ -90,7 +123,7 @@ public function testValidateSucceedsWhenMessageIsValid()
             'Timestamp'      => time(),
             'TopicArn'       => 'baz',
             'Type'           => 'Notification',
-            'SigningCertURL' => 'https://foo.amazonaws.com/bar',
+            'SigningCertURL' => 'https://sns.us-west-2.amazonaws.com/bar.pem',
             'Signature'      => '',
         ));