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

doesn't allow secure connection to the proxy itself #2

Open
eli-darkly opened this issue Jul 2, 2019 · 5 comments
Open

doesn't allow secure connection to the proxy itself #2

eli-darkly opened this issue Jul 2, 2019 · 5 comments

Comments

@eli-darkly
Copy link

It looks like this won't behave correctly if the connection between the client and the proxy server (as opposed to the request from the proxy server to the target URL) has to be secure. WrapDialContext takes a proxy address rather than a URL, so it can't know whether to use TLS (and even if it did know, the DialContext function that you pass into WrapDialContext provides no way to make a TLS connection).

@Codehardt
Copy link
Owner

Currently, I have no resources to continue working on this project. In addition, I no longer have a proxy that I could use for testing.

@Codehardt Codehardt pinned this issue Jul 3, 2019
@eli-darkly
Copy link
Author

@Codehardt - Thanks for the response. If anyone's interested, this is how I've addressed the issue in our fork of the project; it seems to work.

@jimbobmcgee
Copy link

@eli-darkly: Is there any particular reason why you altered the approach from wrapping a DialContext function to wrapping a net.Dialer? The former can be used in other implementations, such as https://github.com/gorilla/websocket, which implement their own Dialer, while the latter cannot.

I'm now fighting with both interfaces: websocket.Dialer, which exposes a NetDialContext property that expects a DialContext for overriding, and your own implementation, which expects a net.Dialer, and the two don't play together.

(Apologies for hijacking, @Codehardt, but the https://github.com/launchdarkly/go-ntlm-proxy-auth repo referenced by @eli-darkly does not have Issues enabled, so I can't raise there.)

@eli-darkly
Copy link
Author

@jimbobmcgee It's been quite a while so my memory of this is not great, but here's what I wrote on the PR if that helps. Sorry issues were not enabled, they are now.

@jimbobmcgee
Copy link

jimbobmcgee commented Nov 24, 2020

@eli-darkly: Thanks for coming back to me; looking back over the code+PR comments (at not-1AM), it does indeed make sense why you need the full net.Dialer, rather than just the DialContext – it is the common ancestor to reach both DialContext and tls.DialWithDialer.

In my case, I think I'm OK passing nil and letting you create/use the zero net.Dialer because—if I'm reading the flow of Gorilla's websocket code—they are also only using the zero net.Dialer internally, unless one passes their own DialContext-equivalent func (https://github.com/gorilla/websocket/blob/c3dd95aea9779669bb3daafbd84ee0530c8ce1c1/client.go#L240).

I suppose that, if I end up needing settings in a custom net.Dialer, I can refactor to either pass the whole net.Dialer to ntlm.NewNTLMProxyDialContext, or pass the dialer.DialContext to websocket.Dialer.NetDialContext and let it close over the net.Dialer itself.

websocketDialer := &websocket.Dialer{}
customDialer := &net.Dialer{}  //...
if useNTLMProxy {
  websocketDialer.NetDialContext = ntlm.NewNTLMProxyDialContext(customDialer, proxyURI, user, passwd, domain, nil)
  websocketDialer.Proxy = nil
} else {
  websocketDialer.NetDialContext = customDialer.DialContext 
}

(Again, apologies to @Codehardt for the hijack – if I have any more issues for @eli-darkly, I'll raise them over at https://github.com/launchdarkly/go-ntlm-proxy-auth. Thanks to both for making my life a little easier – I was not looking forward to trying to learn the intricacies of the NTLM handshake!)

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

No branches or pull requests

3 participants