-
Notifications
You must be signed in to change notification settings - Fork 29
Add upstream proxy #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
eraow
wants to merge
17
commits into
coder:main
Choose a base branch
from
eraow:feature/upstream-proxy
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Add upstream proxy #104
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
4b3a9a6
Add upstream proxy feature
eraow a28754e
fix: redact upstream proxy credentials
eraow 1bd3203
fix: time out upstream proxy TCP connects
eraow d08e61c
fix: support IPv6 upstream proxy addresses
eraow 3d11be8
refactor: use libraries for proxy credential encoding
eraow 7301b5a
refactor: parse upstream proxy URLs with url crate
eraow 701d116
fix: use proxy environment for upstream egress
eraow 7180064
refactor: derive Debug for RunArgs
eraow 88c13dc
refactor: drop TLS connections to the upstream proxy
eraow fab4e09
refactor: own the upstream client per ProxyServer
eraow 7c34dc1
fix: send CONNECT to IPv6 destinations without doubled brackets
eraow 894f40d
feat: honor NO_PROXY for httpjail's own egress
eraow 0b3e7b6
fix: stop passing the parent's proxy environment to jailed processes
eraow 3cd13f3
fix: match a bare NO_PROXY address against host name destinations too
eraow d39cf7f
perf: read the CONNECT response in chunks instead of byte by byte
eraow e2d603d
test: cover an HTTPS destination through the upstream CONNECT tunnel
eraow 1dd901c
refactor: close the upstream connector's test-only constructors
eraow File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| # Upstream Proxy | ||
|
|
||
| By default httpjail contacts destination servers directly. When httpjail itself | ||
| runs in an environment that has no direct internet access — for example behind a | ||
| corporate proxy — you can route httpjail's own outbound requests through an | ||
| upstream proxy with `--upstream-proxy` (or the `HTTPJAIL_UPSTREAM_PROXY` | ||
| environment variable). | ||
|
|
||
| Rule evaluation still happens locally on the intercepted traffic. Only the | ||
| request that httpjail re-originates towards the real destination is forwarded | ||
| through the upstream proxy. | ||
|
|
||
| ```bash | ||
| # Route httpjail's egress through a corporate proxy | ||
| httpjail --upstream-proxy http://proxy.corp:3128 --js "true" -- curl https://api.github.com | ||
|
|
||
| # With Basic authentication | ||
| httpjail --upstream-proxy http://user:pass@proxy.corp:3128 --js "true" -- ./my-app | ||
|
|
||
| # Through an HTTPS proxy | ||
| httpjail --upstream-proxy https://proxy.corp:8443 --js "true" -- ./my-app | ||
|
|
||
| # Via the environment variable (equivalent to --upstream-proxy) | ||
| HTTPJAIL_UPSTREAM_PROXY=http://proxy.corp:3128 httpjail --js "true" -- ./my-app | ||
| ``` | ||
|
|
||
| ## Accepted formats | ||
|
|
||
| | Form | Example | Notes | | ||
| | --- | --- | --- | | ||
| | `http://host:port` | `http://proxy.corp:3128` | Plain HTTP proxy | | ||
| | `https://host:port` | `https://proxy.corp:8443` | Connection to the proxy is wrapped in TLS | | ||
| | `host:port` | `proxy.corp:3128` | Bare authority, `http` scheme assumed | | ||
| | With credentials | `http://user:pass@proxy.corp:3128` | Sends `Proxy-Authorization: Basic ...` | | ||
|
|
||
| The command-line flag takes precedence over the environment variable. Credentials | ||
| are never written to the logs. | ||
|
|
||
| ## How it works | ||
|
|
||
| - **HTTPS destinations** are reached by issuing a `CONNECT` to the upstream | ||
| proxy to obtain a raw TCP tunnel; httpjail then performs the destination TLS | ||
| handshake over that tunnel. TLS is validated against Mozilla's webpki roots | ||
| plus the httpjail CA, exactly as for a direct connection. | ||
| - **Plain HTTP destinations** are forwarded to the proxy in absolute-form, with | ||
| the `Proxy-Authorization` header attached when credentials are configured. | ||
| - Only connection setup (TCP connect, optional TLS to the proxy, and the | ||
| `CONNECT` exchange) is bounded by a timeout. The established tunnel carries no | ||
| timeout, so long-running connections such as WebSocket and gRPC keep working. | ||
|
|
||
| ## Relationship to `HTTP_PROXY` / `HTTPS_PROXY` | ||
|
|
||
| This feature is independent of the `HTTP_PROXY` and `HTTPS_PROXY` variables that | ||
| httpjail sets *inside* the jail to point sandboxed processes at httpjail itself. | ||
|
|
||
| ``` | ||
| [ jailed process ] --HTTP_PROXY/HTTPS_PROXY--> [ httpjail ] --upstream-proxy--> [ corporate proxy ] --> internet | ||
| ``` | ||
|
|
||
| The jailed process always talks to httpjail; `--upstream-proxy` only affects the | ||
| hop from httpjail to the outside world. | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,5 +9,6 @@ pub mod proxy_tls; | |
| pub mod rules; | ||
| pub mod sys_resource; | ||
| pub mod tls; | ||
| pub mod upstream; | ||
|
|
||
| pub mod test_utils; | ||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why shouldn't
httpjailitself respect the standard HTTP_PROXY variables? I think it's clear on its face it wouldn't pass that down to children (as that would invalidate the whole point of the jail).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. For the first release, I’ll remove
--upstream-proxyandHTTPJAIL_UPSTREAM_PROXYand use the standardHTTP_PROXY/HTTPS_PROXYenvironment variables for httpjail’s own egress.Adding a dedicated CLI option and httpjail-specific env var creates an extra configuration path before we have a concrete need for it. If we later need an explicit per-invocation override, we can add it in a follow-up change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 701d116.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ammario following up on 701d116, which drops
--upstream-proxy/HTTPJAIL_UPSTREAM_PROXYin favor of the standardHTTP_PROXY/HTTPS_PROXYfor httpjail's own egress.One question: did you intend for the dedicated flag and env var to be removed entirely, or kept as an explicit override alongside
HTTP_PROXY? I went with full removal, but it's easy to bring back either way. Let me know how this looks.