Repository navigation
fix: prefer exact key name match over canonical header form in condition lookup - #226
Merged
harshavardhana merged 1 commit intoApr 9, 2026
Conversation
…ion lookup getValuesByKey() currently tries http.CanonicalHeaderKey(name) before the raw name. Reverse the order so that exact name matches take precedence, falling back to canonical header form only when needed.
rraulinio
approved these changes
Mar 23, 2026
varun0630
approved these changes
Mar 23, 2026
klauspost
approved these changes
Apr 7, 2026
Vonng
added a commit
to pgsty/silo-pkg
that referenced
this pull request
Aug 2, 2026
Backport of minio#226 (cb1e01c), plus regression tests upstream did not carry. getValuesByKey() resolved a condition key by trying http.CanonicalHeaderKey(name) before the raw name. The map it reads mixes two sources: values the server computed for the request (SourceIp, SecureTransport, CurrentTime, username, ...) keyed exactly as the condition key names them, and every request header, keyed in canonical MIME form. Canonical-first therefore let a client shadow a server-computed value with a header of its own. Against minio/minio this is a policy bypass. A request carrying `Sourceip: 10.1.2.3` satisfies an IpAddress condition on aws:SourceIp regardless of where it came from, and a plaintext request carrying `Securetransport: true` walks past the standard "Deny unless aws:SecureTransport" statement. Thirteen keys are reachable this way: aws:SourceIp, aws:SecureTransport, aws:CurrentTime, aws:EpochTime, aws:username, aws:userid, aws:principaltype, aws:UserAgent, ldap:username, s3:versionid, s3:signatureversion, s3:authType and s3:LocationConstraint. Anonymous bucket policies are exposed directly, and SigV4 does not help: only the headers named in SignedHeaders are covered, so an extra unsigned header rides along. Reverse the order. The canonical form stays as a fallback, which is what keys that genuinely name a header (s3:x-amz-acl and friends) rely on. The second test pins the case that makes this a correctness question and not only a shadowing one. MinIO's retention path builds the condition map, which keeps the caller's X-Amz-Object-Lock-Mode under the canonical spelling, and then writes the mode it is actually about to apply under the policy key's own lower-case spelling. Resolving the canonical form first returns the header, so a condition written to constrain the retention being set reads a value the caller chose instead. That is the half the server-side reservation cannot reach, because the entry is written before the merge loops the reservation gates, so the lookup order here is what decides it. TAKE WITH THE SERVER CHANGE. One behaviour change beyond the fix: where a value arrives both as a query parameter and as a header, the query parameter now wins. Both are client controlled, so no boundary moves inside this package - but the minio-side reservation of the internal condition names in getConditionValues closes the other half, and neither is complete alone. (cherry picked from commit 7a6fe14ec01e14408d4a243017a2fb7d07156737)
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
4 participants
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.
Summary
getValuesByKey()to try exact key name match beforehttp.CanonicalHeaderKeyformTest plan
go test ./policy/condition/...passes (72 tests)