Skip to content

Improve SnsMessageManager message parsing, certificate handling and signature validation - #7495

Queued
alextwoods wants to merge 8 commits into
masterfrom
alexwoo/limit-certificate-fetches
Queued

alextwoods wants to merge 8 commits into
masterfrom
alexwoo/limit-certificate-fetches

Conversation

@alextwoods

@alextwoods alextwoods commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Improve SnsMessageManager parsing, certificate handling and validation

Motivation and Context

Follow-up hardening of the sns-message-manager module. Message parsing, certificate retrieval and field validation in SnsMessageManager were reworked to be more robust and to do less work for messages that cannot validate.

Modifications

  • Certificate retrieval. Signing certificates are cached by certificate content rather than by SigningCertURL. Before a certificate is fetched, the message signature is checked against the certificates already cached; a fetch only happens when none of them verifies. Concurrent downloads are bounded, concurrent requests for the same URL are collapsed into one, and response bodies are closed on error responses.
  • URL validation. The HTTPS / regional-host rules for SigningCertURL are shared through a new internal SnsEndpointUrlValidator and also applied to SubscribeURL and UnsubscribeURL. SigningCertURL path and query remain unconstrained.
  • Field validation. Signature, SignatureVersion, Timestamp and SigningCertURL must be present and well formed before the certificate is retrieved. Signed fields other than Message must be present and single-line. Unsupported signature versions and missing fields now fail with SdkClientException instead of IllegalArgumentException / NullPointerException.
  • Parsing. SnsMessageUnmarshaller reads the message with the streaming JSON parser and copies only the fields the message manager uses; other fields are skipped. Malformed JSON and non-object bodies fail with SdkClientException.

Public API is unchanged. Messages delivered by Amazon SNS are unaffected; Message may still contain line breaks.

Testing

New and existing unit tests.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

License

  • I confirm that this pull request can be released under the Apache 2 license

@alextwoods
alextwoods requested a review from a team as a code owner October 29, 2026 15:04
Limit concurrent certificate downloads, close the response body on error responses, and validate additional SigningCertURL components.
Cache signing certificates by content rather than URL, share the endpoint URL validation rules, and validate message fields before retrieving the certificate.
Read SNS messages with the streaming parser and skip fields the message manager does not use. Depend on third-party-jackson-core directly instead of json-utils.
Require the signed fields to be present and single-line, and validate SubscribeURL and UnsubscribeURL against the regional SNS endpoint.
@alextwoods
alextwoods force-pushed the alexwoo/limit-certificate-fetches branch from e4100f1 to ff6fa22 Compare October 29, 2026 21:58
@alextwoods alextwoods changed the title Limit concurrent SNS certificate fetches Improve SnsMessageManager message parsing, certificate handling and signature validation Oct 29, 2026
@alextwoods
alextwoods requested review from dagnir and zoewangg October 30, 2026 14:54
Validate SigningCertURL through the shared endpoint validator and remove the SubscribeURL and UnsubscribeURL host checks, verify a signature prepared once against each cached certificate, bound the decoded signature length, and consolidate the test fixtures and HTTP client stubs.
@alextwoods
alextwoods enabled auto-merge November 2, 2026 22:04
@alextwoods
alextwoods added this pull request to the merge queue Nov 2, 2026
Any commits made after this event will not be merged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants