Add IMDSv1 fallback (#2609) #2610
Conversation
| @@ -583,4 +595,84 @@ mod tests { | |||
| assert!(!secret.is_empty()); | |||
| assert!(!token.is_empty()) | |||
| } | |||
|
|
|||
| #[tokio::test] | |||
| async fn test_mock() { | |||
There was a problem hiding this comment.
There wasn't an obvious way to use the ec2-metadata-mock to test this fallback, so I opted to reuse the plumbing from the retry tests. More test coverage can't hurt 😄
tustvold
force-pushed
the
add-imdsv1-fallback
branch
from
d485141 to
fb7c505
Compare
object_store/src/aws/credential.rs
Outdated
|
|
||
| let token = match token_result { | ||
| Ok(t) => Some(t.text().await?), | ||
| Err(e) if matches!(e.status(), Some(StatusCode::FORBIDDEN)) => { |
There was a problem hiding this comment.
If I understand this correctly, this means a sligthly lower security level. Maybe allowing this should be a client option?
There was a problem hiding this comment.
Tbh the security issue is the fact the endpoint has IMDSv1 enabled, not whether the application is actually using it. However, adding an option so you can be confident you aren't using IMDSv1 seems sensible
| /// This option has no effect if not using instance credentials | ||
| /// | ||
| /// [IMDSv2]: [https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/configuring-instance-metadata-service.html] | ||
| /// [SSRF attack]: [https://aws.amazon.com/blogs/security/defense-in-depth-open-firewalls-reverse-proxies-ssrf-vulnerabilities-ec2-instance-metadata-service/] |
There was a problem hiding this comment.
I really like that you describe the implications here and include some really helpful links to official docs 👍
|
Benchmark runs are scheduled for baseline = 171f80b and contender = 62eeaa5. 62eeaa5 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2609
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?