AWS: S3SignerServlet should strip out more request headers for caching#15428
Draft
steveloughran wants to merge 3 commits intoapache:mainfrom
Draft
AWS: S3SignerServlet should strip out more request headers for caching#15428steveloughran wants to merge 3 commits intoapache:mainfrom
steveloughran wants to merge 3 commits intoapache:mainfrom
Conversation
Contributor
Author
|
I don't like that the server client needs to guess which headers in a new request are excluded from the sign and hence safe to cache. Change encryption for example and everything blows up with a signing failure. Either the signer should parse and cache the header list from the signature, or (slightly better) the rest servlet should return that list independently. The signer can then use that header list in the cache information to decide whether to reuse. Simplest strategy: include the list of headers alongside the signature, and if changed, don't use the cache value. |
steveloughran
changed the title
A bit concerned that the cache is static so spans all signers; for s3a I'd like a prefix in the cache with every fs instance having a different prefix. This allows for different encryption settings per instance
steveloughran
force-pushed
the
pr/15417-S3SignerServlet
branch
from
c56182e to
7f8cbfb
Compare
Contributor
Author
|
If the signing string is parsed then the cache should already have everything needed to identify conflicting s3 requests. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#15417
A bit concerned that the cache is static so spans all signers; for s3a I'd like a prefix in the cache with every fs instance having a different prefix.
This allows for different encryption settings per instance
The test does show that unsigned headers (here: ifMatch()) can be changed without the signature becoming invalid