ssh: leverage proxy from environment

[dweomer]

I was working on a little git-cli drop-in that creates an orphan branch for tracking (per-branch) versioning information when I realized that it did not work for clone/pull/push with SSH URLs in a proxied environment (good ole SOCKS5). I made a small change in ssh.command#connect() to replace the invocation of ssh.Dial() to a copy of that function that uses golang.org/x/net/proxy.FromEnvironment() to get a dialer and then hands off the dialed connection to ssh.NewClientConn. This breaks the connection timeout facility because proxy.FromEnvironment() returns a proxy.Dialer which is a single-method interface type (that doesn't support a timeout or context arg).

The cool thing about proxy.FromEnvironment() is that it returns a working Dialer (i.e. directly connecting) if no proxy details are found or the setup for recognized proxy details fails for any reason.

There are some existing issues tracking some sort of proxy.DialContext implementation:

• x/net/proxy: provide DialContext golang/go#17759
• proposal: x/net/internal/socks, socks_bundle.go in net/http: new package golang/go#19688

I am not confident that the Go folks will be tackling this lack of a timeout-capable dialer in x/net/proxy anytime soon so I understand if the behavioral breakage in this proposed change is not acceptable.

Signed-off-by: Jacob Blain Christen [email protected]

[mcuadros]

This looks awesome! Can you create test for this?

[dweomer]

@mcuadros, I added a ssh.ProxySuite that shims ssh.UploadPackSuite tests with a socks5 proxy, on localhost, in front of the fixture git server via an ALL_PROXY envvar.

[dweomer]

Please keep in mind, this change does not honor the config.Timeout that is passed into dial and so I will understand if this is considered a regression.

I thought of writing a simple select wrapper (with the timeout fallthrough) around the proxy.Dialer#Dial() invocation but that would leak go routines.

Let me know if you would prefer that the timeout still be honored and I will have another go at it.

[dweomer]

I am looking into submitting a couple of upstream fixes (x/net/proxy, and x/crypto/ssh) that would result in this change being simpler.

[dweomer]

@mcuadros I've updated to use my proxy.Dial change from golang/net#38.

I have added a test suite that is just a repeat of the plumbing/transport/ssh.UploadPackSuite but communicating to a SOCKSv5 server via github.com/armon/go-socks5.

[dweomer]

@mcuadros Additionally, I checked the coverage of the plumbing/transport/ssh package and it was at 74.2% before my change and at 74.3% after (according to go test -cover ./plumbing/transport/ssh/.)

[dweomer]

So:

But:

🤔

[dweomer]

@mcuadros thank you for the merge.

When should I look for a release/tag? I can add a replace directive to my downstream go.mod but then we will stop getting updates to the v4 stream.