Skip to content
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

feat(transport): TLS StreamDialer & Dynamic HTTP CONNECT #117

Merged
merged 30 commits into from
Oct 30, 2023
Merged

Conversation

fortuna
Copy link
Contributor

@fortuna fortuna commented Oct 25, 2023

This PR introduces the TLS transport. It also adds a way to dynamically specify a transport in the local proxy HTTP CONNECT.

This will help us explore domain-fronting, DNS-over-TLS and SOCKS5-over-TLS, and address SNI-based censorship.

Protocols over TLS

This works for DNS-over-TLS:

go run github.com/Jigsaw-Code/outline-sdk/x/examples/resolve -tcp -transport "split:2|tls:sni=decoy.com" -type a -resolver dns.google:853 www.example.com

Given a SOCKS5 server [host:port], you can do SOCKS5-over-TLS with:

go run github.com/Jigsaw-Code/outline-sdk/x/examples/fetch  -transport "tls|socks5:[host:port]"  http://www.rferl.org:443/

Domain Fronting

This now works for domain-fronting:

go run github.com/Jigsaw-Code/outline-sdk/x/examples/fetch  -transport "tls:sni=www.example.com"  -v http://www.rferl.org:443/

The example below shows the the bypass censorship of RFE/RL in a network Russia.

Running a local proxy (KEY is a config for a server in Russia):

go run github.com/Jigsaw-Code/outline-sdk/x/examples/http2transport -transport "$KEY|tls:sni=www.example.com&certName=www.rferl.org" -localAddr localhost:8080

Fetching the page via the proxy in Russia and domain-fronting:

curl -p -x http://localhost:8080 --connect-to ::e4887.dscb.akamaiedge.net: http://www.rferl.org:443/ -D -

Note that I had to tell Curl to connect to e4887.dscb.akamaiedge.net in order to bypass DNS-based blocking. That forces the need to specify the certName=www.rferl.org in the transport, otherwise is tries to use the dialed address (e4887.dscb.akamaiedge.net).

This is not ideal. In the future I'd like to have a transport config that will map the domain to resolve and connect to. That way the user can keep dialing the intended destination. An example with a fictitious fixed dialer that always dials the same address could look like:

"fixed:e4887.dscb.akamaiedge.net|tls:sni=www.example.com"

We may want to introduce matchers based on host:port, do a allow a dialer to be used across endpoints. An example where we map www.rferl.org to e4887.dscb.akamaiedge.net, and only changes the SNI for TLS connection could look like:

"[www.rferl.org:*]fixed:e4887.dscb.akamaiedge.net|[www.rferl.org:443]tls:sni=www.example.com"

Dynamic HTTP CONNECT

With the HTTP CONNECT proxy, we can now use the Transport header to specify a transport config that wraps the base dialer. So a tunnel request could look like:

CONNECT www.rferl.org:443
Transport: tls:sni=example.com

or

CONNECT www.rferl.org:443
Transport: ss://[OUTLINE_KEY]

This allows the client to dynamically change the transport on a per-connection basis, so the transport selection logic can live in Flutter, for example. This also mitigates the issue of the transport config not working for every endpoint.

Below is the domain-fronting example above, modified to use dynamic transports.

Proxy (just connects to the Russian proxy, no bypass logic):

go run github.com/Jigsaw-Code/outline-sdk/x/examples/http2transport -transport "$KEY" -localAddr localhost:8080

Request with domain-fronting:

curl -p -x http://localhost:8080 --proxy-header 'Transport: tls:sni=www.example.com&certName=www.rferl.org' --connect-to ::e4887.dscb.akamaiedge.net: http://www.rferl.org:443/ -D -

@fortuna fortuna requested a review from jyyi1 October 25, 2023 17:57
Copy link
Contributor

@jyyi1 jyyi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Suggested PR title: "feat(transport): TLS StreamDialer"

transport/tls/tls.go Outdated Show resolved Hide resolved
transport/tls/stream_dialer.go Outdated Show resolved Hide resolved
transport/tls/stream_dialer.go Outdated Show resolved Hide resolved
transport/tls/stream_dialer.go Outdated Show resolved Hide resolved
@fortuna
Copy link
Contributor Author

fortuna commented Oct 25, 2023

To add to this PR: reuse https://pkg.go.dev/crypto/tls#ClientSessionCache

@fortuna fortuna changed the title Create TLS transport feat(transport): TLS StreamDialer Oct 26, 2023
@fortuna fortuna requested a review from jyyi1 October 26, 2023 17:02
@fortuna fortuna marked this pull request as draft October 26, 2023 17:28
@fortuna
Copy link
Contributor Author

fortuna commented Oct 26, 2023

Hold on a bit. I want to move TLS to x/ and experiment with some new approaches.
We probably need an api where we can configure on a per-connection basis.
The goal being so that we can pass configs to the mobileproxy and let the user pick the strategies that make sense.

@fortuna fortuna removed the request for review from jyyi1 October 26, 2023 17:30
@fortuna fortuna marked this pull request as ready for review October 27, 2023 17:03
@fortuna fortuna requested a review from jyyi1 October 27, 2023 17:07
Copy link
Contributor

@jyyi1 jyyi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! The code looks good, added some comments to double check.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
x/config/config.go Outdated Show resolved Hide resolved
x/config/config.go Outdated Show resolved Hide resolved

var _ transport.StreamConn = (*streamConn)(nil)

func (c streamConn) CloseRead() error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to override CloseWrite to close both connections as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe tls.Conn already does that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it won't call innerConn.CloseWrite(), just wanna make sure this is expected.

... and does not call CloseWrite on the underlying connection ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right. I added the call to close the inner conn

x/config/config.go Show resolved Hide resolved
x/config/config.go Show resolved Hide resolved
x/config/config.go Show resolved Hide resolved
x/httpproxy/connect_handler.go Show resolved Hide resolved
@@ -48,9 +75,16 @@ func (h *connectHandler) ServeHTTP(proxyResp http.ResponseWriter, proxyReq *http
}

// Dial the target.
targetConn, err := h.dialer.Dial(proxyReq.Context(), proxyReq.Host)
transportConfig := proxyReq.Header.Get("Transport")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we prefix the header name with sth like X-Outline-Transport?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That goes against the official current best practice: https://www.rfc-editor.org/rfc/rfc6648

We could consider Outline-Transport though. It's longer, more things to type when I write command lines, but it could make it clear it's our header, in case we add multiple ones.

@fortuna fortuna requested a review from jyyi1 October 30, 2023 17:41
@fortuna
Copy link
Contributor Author

fortuna commented Oct 30, 2023

FYI, I fixed the flaky test, because it was blocking my PR

Copy link
Contributor

@jyyi1 jyyi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I think we just need to double confirm the CloseWrite behavior is expected (replied in the comment above).

@fortuna fortuna requested a review from jyyi1 October 30, 2023 21:10
@fortuna fortuna changed the title feat(transport): TLS StreamDialer feat(transport): TLS StreamDialer & Dynamic HTTP CONNECT Oct 30, 2023
@fortuna fortuna merged commit c103980 into main Oct 30, 2023
6 checks passed
@fortuna fortuna deleted the fortuna/tls branch October 30, 2023 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants