🐛 Fix SNS signature verification field sets per AWS spec

V1 and V2 use the same field sets (only hash algorithm differs).
The AWS documentation explicitly shows that SigningCertURL and
SignatureVersion are metadata fields, not part of the signed content.

Changes:
- Remove V2-specific field lists (SigningCertURL, SignatureVersion)
- Simplify build-string-to-sign to use unified field sets
- Fix resource leak in fetch-certificate (close stream on non-200)
- Fix missing Message field returning 200 instead of 400
- Stub fetch-certificate in network-dependent test
- Update V2 test to reflect correct field sets

Reference: https://docs.aws.amazon.com/sns/latest/dg/sns-verify-signature-of-message-verify-message-signature.html

AI-assisted-by: qwen3.7-plus

backend/src/app/http/awsns.clj

@@ -51,51 +51,29 @@
         false))))
 
 ;; AWS SNS Signature Verification Field Sets
-;; See: https://docs.aws.amazon.com/sns/latest/dg/sns-verify-signature-of-message.html
+;; See: https://docs.aws.amazon.com/sns/latest/dg/sns-verify-signature-of-message-verify-message-signature.html
 ;;
-;; Signature Version 1 signs only specific fields (excludes SigningCertURL, SignatureVersion, Signature)
+;; V1 and V2 use the SAME field sets (only the hash algorithm differs: SHA1 vs SHA256)
-;; Signature Version 2 signs all fields except Signature
+;; The "Signature" field is NEVER part of the string-to-sign (it's the output, not input)
-;;
+;; "SigningCertURL" and "SignatureVersion" are metadata, not signed
-;; IMPORTANT: The "Signature" field is NEVER part of the string-to-sign (it's the output, not input)
 
-;; V1 Notification: Message, MessageId, Subject (if present), Timestamp, TopicArn, Type
+;; Notification: Message, MessageId, Subject (if present), Timestamp, TopicArn, Type
-(def ^:private v1-notification-fields
+(def ^:private notification-fields
   ["Message" "MessageId" "Subject" "Timestamp" "TopicArn" "Type"])
 
-;; V1 SubscriptionConfirmation: Message, MessageId, SubscribeURL, Timestamp, Token, TopicArn, Type
+;; SubscriptionConfirmation: Message, MessageId, SubscribeURL, Timestamp, Token, TopicArn, Type
-(def ^:private v1-subscription-fields
+(def ^:private subscription-fields
   ["Message" "MessageId" "SubscribeURL" "Timestamp" "Token" "TopicArn" "Type"])
 
-;; V2 Notification: All fields except Signature
-(def ^:private v2-notification-fields
-  ["Message" "MessageId" "Subject" "Timestamp" "TopicArn" "Type"
-   "SigningCertURL" "SignatureVersion"])
-
-;; V2 SubscriptionConfirmation: All fields except Signature (includes Token)
-(def ^:private v2-subscription-fields
-  ["Message" "MessageId" "SubscribeURL" "Timestamp" "Token" "TopicArn" "Type"
-   "SigningCertURL" "SignatureVersion"])
-
 (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"
+   V1 and V2 use the same field sets (only hash algorithm differs: SHA1 vs SHA256).
+   See: https://docs.aws.amazon.com/sns/latest/dg/sns-verify-signature-of-message-verify-message-signature.html"
   [body]
-  (let [sig-version (get body "SignatureVersion")
+  (let [msg-type (get body "Type")
-        msg-type    (get body "Type")
+        fields   (if (= "SubscriptionConfirmation" msg-type)
-        fields      (cond
+                   subscription-fields
-                      (= "1" sig-version)
+                   notification-fields)]
-                      (if (= "SubscriptionConfirmation" msg-type)
-                        v1-subscription-fields
-                        v1-notification-fields)
-
-                      (= "2" sig-version)
-                      (if (= "SubscriptionConfirmation" msg-type)
-                        v2-subscription-fields
-                        v2-notification-fields)
-
-                      :else
-                      (throw (ex-info "Unsupported SNS signature version"
-                                      {:version sig-version})))]
     (->> fields
          (filter #(contains? body %))
          (map #(str % "\n" (get body %) "\n"))
@@ -109,6 +87,8 @@
   (let [response (http/req cfg {:uri cert-url :method :get :timeout 10000}
                            {:sync? true :response-type :input-stream})]
     (when-not (= 200 (:status response))
+      (when-let [body (:body response)]
+        (.close ^java.io.Closeable body))
       (l/wrn :hint "failed to fetch SNS signing certificate"
              :action "sns-cert-fetch-failed"
              :status (:status response)
@@ -226,10 +206,15 @@
           {:status 200})
 
         (= mtype "Notification")
-        (when-let [message (parse-json (get body "Message"))]
+        (if-let [message (parse-json (get body "Message"))]
-          (let [notification (parse-notification cfg message)]
+          (do
-            (process-report cfg notification))
+            (let [notification (parse-notification cfg message)]
-          {:status 200})
+              (process-report cfg notification))
+            {:status 200})
+          (do
+            (l/wrn :hint "notification with missing or unparseable Message field"
+                   :action "sns-missing-message")
+            {:status 400}))
 
         :else
         (do

backend/test/backend_tests/bounce_handling_test.clj

@@ -480,11 +480,11 @@
     (t/is (string? result))
     (t/is (.contains result "MessageId"))
     (t/is (.contains result "TopicArn"))
-    ;; V2 includes SigningCertURL and SignatureVersion but NOT Signature
+    ;; V2 uses the same fields as V1 (only hash algorithm differs: SHA1 vs SHA256)
-    (t/is (.contains result "SigningCertURL"))
+    ;; SigningCertURL and SignatureVersion are metadata, not part of the signed content
-    (t/is (.contains result "SignatureVersion"))
+    (t/is (not (.contains result "SigningCertURL")))
-    ;; Check that "Signature\n" (the field name) is NOT in the result
+    (t/is (not (.contains result "SignatureVersion")))
-    ;; Note: "SignatureVersion" contains "Signature" as substring, so we check for the exact field pattern
+    ;; Signature is never part of the string-to-sign
     (t/is (not (.contains result "Signature\n")))))
 
 (t/deftest test-build-string-to-sign-subscription-confirmation
@@ -507,7 +507,8 @@
     (t/is (.contains result "test-token-123"))))
 
 (t/deftest test-handle-request-returns-4xx-for-invalid-signature
-  (let [body (j/write-str
+  (let [{:keys [cert-bytes]} (load-test-cert-and-key)
+        body (j/write-str
               {"Type"             "Notification"
                "MessageId"        "msg-123"
                "TopicArn"         "arn:aws:sns:eu-central-1:123:topic"
@@ -516,7 +517,9 @@
                "SigningCertURL"   "https://sns.eu-central-1.amazonaws.com/cert.pem"
                "SignatureVersion" "1"
                "Signature"        "invalid-signature=="})
-        result (#'awsns/handle-request th/*system* body)]
+        result (with-redefs [awsns/fetch-certificate (fn [_ _]
+                                                       (java.io.ByteArrayInputStream. cert-bytes))]
+                 (#'awsns/handle-request th/*system* body))]
     (t/is (= 400 (:status result)))))
 
 (t/deftest test-handle-request-returns-4xx-for-invalid-url