fix: correctly handle aws:SourceIp in policy evaluation (#1301) #1306
Conversation
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
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the aws:SourceIp condition key in IAM and Bucket policies was not being evaluated correctly during authorization. The fix ensures that the client's actual IP address is properly propagated through the request handling layers and used for policy evaluation.
Key Changes:
- Introduced
RemoteAddrwrapper type to carry the client's socket address through request extensions - Updated all
get_condition_valuesandvalidate_admin_requestcall sites to accept and pass the remote address - Modified IP extraction logic to use the actual socket address instead of only relying on headers
- Added comprehensive tests for IAM and Bucket policy evaluation with
aws:SourceIpconditions
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rustfs/src/server/mod.rs | Introduced RemoteAddr struct to wrap socket addresses |
| rustfs/src/server/http.rs | Added layer to capture and inject remote address into request extensions |
| rustfs/src/auth.rs | Updated get_condition_values to accept remote address parameter and use it for SourceIp; added tests |
| rustfs/src/storage/access.rs | Threaded remote address through authorization calls |
| rustfs/src/storage/ecfs.rs | Passed remote address to get_condition_values |
| rustfs/src/admin/auth.rs | Updated admin request validation to accept remote address |
| rustfs/src/admin/handlers/*.rs | Updated all admin handler call sites to extract and pass remote address |
| rustfs/Cargo.toml | Added add-extension feature to tower-http dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rustfs/src/auth.rs
Outdated
Comment on lines
302
to
304
| let remote_addr_s = remote_addr | ||
| .map(|a| a.ip().to_string()) | ||
| .unwrap_or_else(|| "127.0.0.1".to_string()); |
There was a problem hiding this comment.
Defaulting to '127.0.0.1' when remote_addr is None could bypass IP-based restrictions in policies. This represents a potential security issue where failed socket address extraction silently falls back to localhost, which may be implicitly trusted. Consider whether this default is appropriate or if the request should be rejected when the remote address cannot be determined.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: loverustfs <github@rustfs.com>
lgpseu
pushed a commit
to lgpseu/rustfs
that referenced
this pull request
Dec 31, 2025
…rustfs#1306) Signed-off-by: loverustfs <github@rustfs.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
houseme
added a commit
that referenced
this pull request
Jan 7, 2026
* 'main' of github.com:rustfs/rustfs: fix:correct RemoteAddr extension type to enable IP-based policy evaluation (#1356) Add workflow to mark stale issues automatically fix: remove nginx-ingress default body size limit (#1335) feat:Permission verification for deleting versions (#1341) chore: upgrade GitHub Actions artifact actions (#1339) chore: replace native-tls with pure rustls for FTPS/SFTP e2e tests (#1334) chore: upgrade dependencies and migrate to aws-lc-rs (#1333) fix: s3 list object versions next marker (#1328) fix(tagging): fix e2e test_object_tagging failure (#1327) Feat/ftps&sftp (#1308) fix(iam): preserve decrypt-failed credentials instead of deleting them (#1312) Restore globals and add unified TLS/mTLS loading from RUSTFS_TLS_PATH (#1309) fix: correctly handle aws:SourceIp in policy evaluation (#1301) (#1306) Add trendshift feat: add local s3-tests script with configurable options and improvements (#1300)
reatang
pushed a commit
that referenced
this pull request
Jan 11, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Type of Change
Related Issues
#1301
Summary of Changes
This PR fixes an issue where the
aws:SourceIpcondition key in IAM and Bucket policies was not being evaluated correctly.Key changes include:
SocketAddrthrough the request handling layers (server -> handlers -> auth) to ensure the actual remote address is available during authentication.auth.rsto userustfs_utils::http::ip::get_source_ip_raw. This ensures that we correctly identify the client IP, respecting standard headers likeX-Forwarded-Forwhen behind a proxy, aligning behavior with MinIO.get_condition_valuesto populate theSourceIpkey in the condition context, enabling the policy engine to validate IP address restrictions.rustfs/src/auth.rscovering:aws:SourceIp.aws:SourceIp.Checklist
make pre-commitImpact
Additional Notes
The fix ensures that security policies relying on IP restrictions function as expected. Code formatting has been applied to all modified files.
Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.