pluto: handle auth info in https_proxy #3639
Conversation
etungsten
force-pushed
the
old-dog-new-tricks
branch
from
32b7fc7 to
f5a8c9f
Compare
etungsten
force-pushed
the
old-dog-new-tricks
branch
from
f5a8c9f to
285bcca
Compare
|
Push above addresses @bcressey's comments. Retested, same behavior as observed in cloudtrail: |
etungsten
force-pushed
the
old-dog-new-tricks
branch
from
285bcca to
68622c6
Compare
sources/api/pluto/src/proxy.rs
Outdated
| if !proxy_url.username().is_empty() { | ||
| proxy.set_authorization(Authorization::basic( | ||
| proxy_url.username(), | ||
| proxy_url.password().unwrap_or_default(), |
There was a problem hiding this comment.
I haven't done much with Proxy-Authorization but my read of the header documentation and my understanding of URL username/password encoding makes me think that it's valid to provide a passphrase without a username.
Should we be inserting the header if a password is given without a username?
There was a problem hiding this comment.
I believe so. Good catch
etungsten
force-pushed
the
old-dog-new-tricks
branch
from
68622c6 to
6821716
Compare
|
Push above addresses @cbgbt 's comment |
etungsten
force-pushed
the
old-dog-new-tricks
branch
from
6821716 to
0b57801
Compare
Need to parse out auth info in `https_proxy` and set to proxy header.
etungsten
force-pushed
the
old-dog-new-tricks
branch
from
0b57801 to
6a66851
Compare
| /// Set `Proxy` authorization | ||
| pub fn set_authorization<C: Credentials + Clone>(&mut self, credentials: Authorization<C>) { | ||
| match self.intercept { | ||
| Intercept::Https => { | ||
| self.headers.typed_insert(ProxyAuthorization(credentials.0)); | ||
| } | ||
| _ => { | ||
| self.headers | ||
| .typed_insert(Authorization(credentials.0.clone())); | ||
| self.headers.typed_insert(ProxyAuthorization(credentials.0)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Why do we need to set the authorization headers in the _ (specifically, None) case where connections aren't going through the proxy?
There was a problem hiding this comment.
This was lifted from hyper_proxy source and it's not super clear to me either. In any case, we shouldn't be reaching this path since we're only proxying HTTPS traffic.
There was a problem hiding this comment.
After some digging, we've determined that the Auth header here will never get sent if the proxy doesn't "match" with the destination URL. We'll go ahead and simplify this logic here so it only contains the case we care about, i.e. proxying HTTPS traffic.
Issue number:
Resolves #3525
Description of changes:
Testing done:
Unit tests pass.
Set up my own tiny proxy host and launched new Bottlerocket host with the following settings:
The boot succeeds, pluto successfully fetches instance information through AWS EC2 API.
In cloudtrail, i can see the source of the DescribeInstance event as being my proxy:
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.