search

Codehardt / go-ntlm-proxy-auth

Authorize to an NTLM Proxy for a HTTP(S) connection in Golang
MIT License
8 stars 6 forks source link

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

Open eli-darkly opened 5 years ago

eli-darkly commented 5 years ago

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 commented 5 years ago

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.

eli-darkly commented 5 years ago

@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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

@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!)