mirror of
https://github.com/penpot/penpot.git
synced 2026-08-07 05:18:36 +00:00
🐛 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
This commit is contained in:
parent
a4b9ccee8b
commit
64fe8e57e3
@ -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)
|
||||
;; Signature Version 2 signs all fields except Signature
|
||||
;;
|
||||
;; IMPORTANT: The "Signature" field is NEVER part of the string-to-sign (it's the output, not input)
|
||||
;; V1 and V2 use the SAME field sets (only the hash algorithm differs: SHA1 vs SHA256)
|
||||
;; The "Signature" field is NEVER part of the string-to-sign (it's the output, not input)
|
||||
;; "SigningCertURL" and "SignatureVersion" are metadata, not signed
|
||||
|
||||
;; V1 Notification: Message, MessageId, Subject (if present), Timestamp, TopicArn, Type
|
||||
(def ^:private v1-notification-fields
|
||||
;; Notification: Message, MessageId, Subject (if present), Timestamp, TopicArn, Type
|
||||
(def ^:private notification-fields
|
||||
["Message" "MessageId" "Subject" "Timestamp" "TopicArn" "Type"])
|
||||
|
||||
;; V1 SubscriptionConfirmation: Message, MessageId, SubscribeURL, Timestamp, Token, TopicArn, Type
|
||||
(def ^:private v1-subscription-fields
|
||||
;; SubscriptionConfirmation: Message, MessageId, SubscribeURL, Timestamp, Token, TopicArn, Type
|
||||
(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")
|
||||
msg-type (get body "Type")
|
||||
fields (cond
|
||||
(= "1" sig-version)
|
||||
(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})))]
|
||||
(let [msg-type (get body "Type")
|
||||
fields (if (= "SubscriptionConfirmation" msg-type)
|
||||
subscription-fields
|
||||
notification-fields)]
|
||||
(->> 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"))]
|
||||
(let [notification (parse-notification cfg message)]
|
||||
(process-report cfg notification))
|
||||
{:status 200})
|
||||
(if-let [message (parse-json (get body "Message"))]
|
||||
(do
|
||||
(let [notification (parse-notification cfg message)]
|
||||
(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
|
||||
|
||||
@ -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
|
||||
(t/is (.contains result "SigningCertURL"))
|
||||
(t/is (.contains result "SignatureVersion"))
|
||||
;; Check that "Signature\n" (the field name) is NOT in the result
|
||||
;; Note: "SignatureVersion" contains "Signature" as substring, so we check for the exact field pattern
|
||||
;; V2 uses the same fields as V1 (only hash algorithm differs: SHA1 vs SHA256)
|
||||
;; SigningCertURL and SignatureVersion are metadata, not part of the signed content
|
||||
(t/is (not (.contains result "SigningCertURL")))
|
||||
(t/is (not (.contains result "SignatureVersion")))
|
||||
;; 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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user