From a8a441b7ad133d3a0f0247249694769f3899e6bc Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 12:29:21 +0000 Subject: [PATCH] :bug: Verify AWS SNS signature on webhook notifications The /webhooks/sns endpoint was accepting bounce and complaint notifications without verifying the AWS SNS cryptographic signature. This allowed any authenticated user to forge reports for arbitrary email addresses using their own valid :profile-identity token. The fix adds: - AWS SNS signature verification using RSA-SHA1 - URL validation for SigningCertURL and SubscribeURL (must be from amazonaws.com domain) - Proper logging of all verification failures with context fields - Rejection of unverified messages before any processing Closes #11092 AI-assisted-by: qwen3.7-plus --- backend/src/app/http/awsns.clj | 110 +++++++++++++++++- .../backend_tests/bounce_handling_test.clj | 89 ++++++++++++++ 2 files changed, 198 insertions(+), 1 deletion(-) diff --git a/backend/src/app/http/awsns.clj b/backend/src/app/http/awsns.clj index 3dddc1045d..c367e91f00 100644 --- a/backend/src/app/http/awsns.clj +++ b/backend/src/app/http/awsns.clj @@ -21,13 +21,117 @@ [cuerdas.core :as str] [integrant.core :as ig] [yetti.request :as yreq] - [yetti.response :as-alias yres])) + [yetti.response :as-alias yres]) + (:import + java.net.URI + java.security.cert.CertificateFactory + java.security.Signature + java.util.Base64)) (declare parse-json) (declare handle-request) (declare parse-notification) (declare process-report) +(defn- valid-sns-url? + "Validates that a URL originates from amazonaws.com domain. + Used for SigningCertURL and SubscribeURL validation." + [url] + (when (string? url) + (try + (let [uri (URI. url)] + (and (= "https" (.getScheme uri)) + (let [host (.getHost uri)] + (and host + (or (= host "amazonaws.com") + (.endsWith host ".amazonaws.com")))))) + (catch Exception _ + false)))) + +(def ^:private notification-fields + ["Message" "MessageId" "Subject" "Timestamp" + "TopicArn" "Type" "SigningCertURL" "SignatureVersion" "Signature"]) + +(def ^:private subscription-fields + ["Message" "MessageId" "SubscribeURL" "Timestamp" + "TopicArn" "Type" "SigningCertURL" "SignatureVersion" "Signature"]) + +(defn- build-string-to-sign + "Builds the string-to-sign for AWS SNS signature verification. + See: https://docs.aws.amazon.com/sns/latest/dg/sns-verify-signature-of-message.html" + [body] + (let [fields (if (= "SubscriptionConfirmation" (get body "Type")) + subscription-fields + notification-fields)] + (->> fields + (filter #(contains? body %)) + (map #(str % "\n" (get body %) "\n")) + (apply str)))) + +(defn- fetch-certificate + "Fetches the X.509 certificate from the given URL." + [cfg cert-url] + (let [response (http/req cfg {:uri cert-url :method :get :timeout 10000} {:sync? true :response-type :input-stream})] + (when (= 200 (:status response)) + (let [cf (CertificateFactory/getInstance "X.509")] + (.generateCertificate cf (:body response)))))) + +(defn- verify-signature + "Verifies the RSA-SHA1 signature of the message." + [cfg body] + (let [cert-url (get body "SigningCertURL") + signature (get body "Signature") + string-sign (build-string-to-sign body)] + (when (and cert-url signature) + (try + (let [cert (fetch-certificate cfg cert-url) + sig (Signature/getInstance "SHA1withRSA")] + (.initVerify sig (.getPublicKey cert)) + (.update sig (.getBytes string-sign)) + (.verify sig (.decode (Base64/getDecoder) signature))) + (catch Exception e + (l/wrn :hint "SNS signature verification exception" + :action "sns-signature-verification-exception" + :cause e) + false))))) + +(defn- verify-sns-message! + "Verifies the AWS SNS message signature and URL validity. + Throws if verification fails." + [cfg body] + (let [cert-url (get body "SigningCertURL") + subscribe-url (get body "SubscribeURL") + mtype (get body "Type")] + + (when-not (valid-sns-url? cert-url) + (l/wrn :hint "SNS certificate URL not from amazonaws.com" + :action "sns-invalid-cert-url" + :message-type mtype + :signing-cert-url cert-url) + (ex/raise :type :validation + :code :invalid-signing-cert-url + :hint "SigningCertURL must be from amazonaws.com")) + + (when (and (= mtype "SubscriptionConfirmation") + (not (valid-sns-url? subscribe-url))) + (l/wrn :hint "SNS subscribe URL not from amazonaws.com" + :action "sns-invalid-subscribe-url" + :message-type mtype + :subscribe-url subscribe-url) + (ex/raise :type :validation + :code :invalid-subscribe-url + :hint "SubscribeURL must be from amazonaws.com")) + + (when-not (verify-signature cfg body) + (l/wrn :hint "SNS signature verification failed" + :action "sns-signature-verification-failed" + :message-type mtype + :topic-arn (get body "TopicArn") + :signing-cert-url cert-url) + (ex/raise :type :authentication + :code :invalid-signature + :hint "SNS signature verification failed")))) + (defmethod ig/assert-key ::routes [_ params] (assert (http/client? (::http/client params)) "expect a valid http client") @@ -48,6 +152,10 @@ (try (let [body (parse-json data) mtype (get body "Type")] + + (when body + (verify-sns-message! cfg body)) + (cond (= mtype "SubscriptionConfirmation") (let [surl (get body "SubscribeURL") diff --git a/backend/test/backend_tests/bounce_handling_test.clj b/backend/test/backend_tests/bounce_handling_test.clj index 9ae3d9a60d..a8ca6a4ac9 100644 --- a/backend/test/backend_tests/bounce_handling_test.clj +++ b/backend/test/backend_tests/bounce_handling_test.clj @@ -12,6 +12,7 @@ [app.http.awsns :as awsns] [app.tokens :as tokens] [backend-tests.helpers :as th] + [clojure.data.json :as j] [clojure.pprint :refer [pprint]] [clojure.test :as t] [mockery.core :refer [with-mocks]])) @@ -290,3 +291,91 @@ (th/create-global-complaint-for pool {:type :bounce :email (:email profile)}) (t/is (true? (email/has-bounce-reports? pool (:email profile)))))) + +(t/deftest test-validate-sns-url-rejects-non-amazonaws + (t/is (false? (#'awsns/valid-sns-url? "https://evil.com/cert.pem"))) + (t/is (false? (#'awsns/valid-sns-url? "http://attacker.com/confirm"))) + (t/is (false? (#'awsns/valid-sns-url? "https://sns.eu-central-1.amazonaws.com.evil.com/cert.pem"))) + (t/is (false? (#'awsns/valid-sns-url? "ftp://sns.amazonaws.com/cert.pem")))) + +(t/deftest test-validate-sns-url-accepts-amazonaws + (t/is (true? (#'awsns/valid-sns-url? "https://sns.eu-central-1.amazonaws.com/SimpleNotificationService-xxx.pem"))) + (t/is (true? (#'awsns/valid-sns-url? "https://sns.us-east-1.amazonaws.com/cert.pem"))) + (t/is (true? (#'awsns/valid-sns-url? "https://amazonaws.com/cert.pem")))) + +(t/deftest test-build-string-to-sign-notification + (let [msg {"Type" "Notification" + "MessageId" "msg-123" + "TopicArn" "arn:aws:sns:eu-central-1:123:topic" + "Message" "{\"notificationType\":\"Bounce\"}" + "Timestamp" "2021-02-04T14:41:37.020Z" + "SigningCertURL" "https://sns.eu-central-1.amazonaws.com/cert.pem" + "SignatureVersion" "1" + "Signature" "abc123=="} + result (#'awsns/build-string-to-sign msg)] + (t/is (string? result)) + (t/is (.contains result "MessageId")) + (t/is (.contains result "msg-123")) + (t/is (.contains result "TopicArn")) + (t/is (.contains result "Message")) + (t/is (.contains result "Timestamp")) + (t/is (.contains result "SigningCertURL")) + (t/is (.contains result "SignatureVersion")))) + +(t/deftest test-build-string-to-sign-subscription-confirmation + (let [msg {"Type" "SubscriptionConfirmation" + "MessageId" "msg-456" + "TopicArn" "arn:aws:sns:eu-central-1:123:topic" + "Message" "You have chosen to subscribe" + "Timestamp" "2021-02-04T14:41:37.020Z" + "SigningCertURL" "https://sns.eu-central-1.amazonaws.com/cert.pem" + "SignatureVersion" "1" + "Signature" "xyz789==" + "SubscribeURL" "https://sns.eu-central-1.amazonaws.com/confirm"} + result (#'awsns/build-string-to-sign msg)] + (t/is (string? result)) + (t/is (.contains result "SubscribeURL")) + (t/is (.contains result "https://sns.eu-central-1.amazonaws.com/confirm")))) + +(t/deftest test-handle-request-rejects-invalid-signing-cert-url + (let [pool (:app.db/pool th/*system*) + profile (th/create-profile* 1) + token (tokens/generate th/*system* + {:iss :profile-identity + :profile-id (:id profile)}) + body (j/write-str + {"Type" "Notification" + "MessageId" "msg-123" + "TopicArn" "arn:aws:sns:eu-central-1:123:topic" + "Message" (j/write-str {"notificationType" "Bounce" + "bounce" {"bounceType" "Permanent" + "bounceSubType" "General" + "bouncedRecipients" [{"emailAddress" "victim@example.com"}] + "timestamp" "2021-02-04T14:41:38.000Z"} + "mail" {"source" "no-reply@penpot.app" + "destination" ["victim@example.com"] + "timestamp" "2021-02-04T14:41:37.020Z" + "headers" [{"name" "X-Penpot-Data" "value" token}]}}) + "Timestamp" "2021-02-04T14:41:37.020Z" + "SigningCertURL" "https://evil.com/cert.pem" + "SignatureVersion" "1" + "Signature" "fake-signature=="})] + (#'awsns/handle-request th/*system* body) + (let [reports (db/query pool :global-complaint-report :all)] + (t/is (empty? reports))))) + +(t/deftest test-handle-request-rejects-invalid-subscribe-url + (let [pool (:app.db/pool th/*system*) + body (j/write-str + {"Type" "SubscriptionConfirmation" + "MessageId" "msg-456" + "TopicArn" "arn:aws:sns:eu-central-1:123:topic" + "Message" "You have chosen to subscribe" + "Timestamp" "2021-02-04T14:41:37.020Z" + "SigningCertURL" "https://sns.eu-central-1.amazonaws.com/cert.pem" + "SignatureVersion" "1" + "Signature" "fake-signature==" + "SubscribeURL" "http://attacker.com/confirm"})] + (#'awsns/handle-request th/*system* body) + (let [reports (db/query pool :global-complaint-report :all)] + (t/is (empty? reports)))))