Hoverfly hangs with Netty HttpProxyHandler

[JohnFDavenport]

Cross-referencing issue raised by @ttiurani here

[tommysitu]

@ttiurani, as I mentioned in your original issue, the Netty proxy always uses CONNECT method even it is an HTTP request, hence the error from Go:
2017/09/08 17:57:04 [001] WARN: Cannot handshake client time.jsontest.com:80 tls: first record does not look like a TLS handshake

[ttiurani]

If I understand correctly, it seems to me that Hoverfly (via Go) here is in the wrong and shouldn't mandate TLS after CONNECT? Is this fixable in Hoverfly?

[ttiurani]

The issue at:

reactor/reactor-netty#159

was closed with info about this. Netty's proxy handler does CONNECT always and is not wrong to do so.

I ran into this when using Spring Webflux, which is a big part of Spring 5.0.0, which is now at the last RC before release. So I would guess that a significant number of Spring users will run into this problem when using Hoverfly.

If Hoverfly wants to support Spring, I would guess this should be fixed here?

[tommysitu]

You are right, Netty proxy does the right thing according to this: https://tools.ietf.org/html/rfc7231#section-4.3.6. A proxy should be able to handle CONNECT regardless the HTTP scheme.

[JohnFDavenport]

The RFC also says:

There are significant risks in establishing a tunnel to arbitrary servers ... Proxies that support CONNECT SHOULD restrict its use to a limited set of known port or a configurable whitelist of safe request targets.

There are implications to changing this that we need to understand.

[ttiurani]

@JohnFDavenport At least for my use case this could be a configuration setting or startup parameter in Hoverfly, and the current behaviour the default? In fact, I could live with having a switch in Hoverfly to just turn off all TLS handling, as I can configure the endpoint addresses in my app.

[mogronalol]

@tommysitu How are you able to produce this error?:

2017/09/08 17:57:04 [001] WARN: Cannot handshake client time.jsontest.com:80 tls: first record does not look like a TLS handshake

I've tried this which reproduces the timeout (I think), but the logs are empty when I check:

url -v -X CONNECT http://www.test.com --proxy localhost:8500 -v
* Rebuilt URL to: http://www.test.com/
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8500 (#0)
> CONNECT http://www.test.com/ HTTP/1.1
> Host: www.test.com
> User-Agent: curl/7.54.0
> Accept: */*
> Proxy-Connection: Keep-Alive
>
* HTTP 1.0, assume close after body
< HTTP/1.0 200 OK
<

[tommysitu]

@mogronalol you can have a look at the original reactor/reactor-netty#159, and run the test from the reactor-netty project, but it is not straight forward. I will try sharing what I've got with you later.

[ttiurani]

Any progress to report? Would a better Hoverfly-specific test case speed this up? I'd be happy to try to hack some "skipSslForConnect=true" thingy together myself, as this is blocking quite a lot of things for me.

Spring 5.0.0 has been released and v0.7.0 of reactor-netty that comes with this, has this problem.

[mogronalol]

For some reason GitHub isn't showing the link to the other issue:

elazarl/goproxy#246

I've isolated the problem in an upstream library (GoProxy), but it has made me more skeptical about Netty's CONNECT only approach to using a proxy.

As a CONNECT request is the same regardless of HTTP or HTTPS there is no way of a proxy knowing what to do. The only way of figuring it out is to peek the first few bytes of the request and figure out whether they are encrypted or not.

Whilst this is possible I personally feel that this approach is very hacky. Until this every other client I have worked with used CONNECT for HTTPS only meaning it is simple for a Proxy to know whether the traffic is TLS or not. Curl is a good candidate for testing this.

The other option is for the client (Hoverctl) to instruct Hoverfly whether it should do HTTP or HTTPS with CONNECT on a specific port, again feeling brittle.

[mogronalol]

I think a byte peeking solution is probably the best route over changing the client, but this will need to be a contribution to the GoProxy library. I'm not sure when we will be able to implement this, but PR's will be welcome if you want to try and do it.

[ttiurani]

"The other option is for the client (Hoverctl) to instruct Hoverfly whether it should do HTTP or HTTPS with CONNECT on a specific port"

I would be delighted about something like this. Anything that allows me to use Hoverfly any which way is better than the current.

Could there be even in the simulation.json some sort of "disableTlsOnConnect" custom flag, next to Hoverfly version? Even just "disableTls" would work for me?

[ttiurani]

That is: the goproxy peek byte route is of course "better", but if it will be slow to get merged and still unsatisfactory even if it does work, having at least some way to get netty support, is better than nothing.

Also a "tlsOnlyForPort443" would be just fine for me.

[ttiurani]

Unfortunately I haven't had time to look at this further, any news?

It might be a good idea to change the heading of this to something like "Hoverfly hangs with Netty HttpProxyHandler", so that other Netty users find this issue.

[tommysitu]

@ttiurani I have added the -plain-http-tunneling flag in hoverfly that should get around the problem. When this flag is present, hoverfly will skip TLS handshake for any CONNECT request to non-443 port. you can try it out by using the stable version of docker image: docker run -d -p 8888:8888 -p 8500:8500 spectolabs/hoverfly:master -plain-http-tunneling

[ttiurani]

@tommysitu awesome, thank you! I'll try my best to find time to test this asap (also I can try to do a PR for hoverfly-java for this).

[ttiurani]

I can verify that using the -plain-http-tunneling patch I can, with the latest reactor-netty, get my test to work, whereas without it or with using an older hoverfly version, the test fails.

[tommysitu]

Nice, we will do a minor release at some point.

[ttiurani]

@tommysitu Unfortunately with better debugging I found out, that although I can get around the ssl problem with -plain-http-tunneling, hoverfly fails with:

Error dialing to host-to-simulate:80: dial tcp: lookup host-to-simulate: no such host

when host-to-simulate isn't available. My test at the referenced reactor-netty issue also fails, if I just turn off the network on my machine and time.jsontest.com isn't reachable.

Now for hoverfly to be useful, it of course shouldn't be mandatory that the host that is simulated, needs to actually resolve before the mocking? Am I missing something here?

[ttiurani]

I did some digging, and the problem is that the -plain-http-tunneling ends up here:

https://github.com/elazarl/goproxy/blob/master/https.go#L130

which then fails, because it does a normal go Transport.Dial. But it might be possible to override that functionality by implementing a custom ConnectDial function, as is specified here:

https://github.com/elazarl/goproxy/blob/master/proxy.go#L27

Would it be possible to create such a dummy ConnectDial override in hoverfly for this -plain-http-tunneling @tommysitu ?

[tommysitu]

@ttiurani When I ran your test with this new flag, it passed. So I don't quite understand what the new problem is. Could you let me know how to reproduce the error using this example: ttiurani/reactor-netty@790b82d

[ttiurani]

@tommysitu The problem is that my test works only if it can actually connect to time.jsontest.com, as hoverfly with -plain-http-tunneling does a normal "Transport.Dial" first. The test does not work if you disconnect your wifi or unplug your ethernet cable.

Now as I'm using Hoverfly as a mock server to debug Kubernetes paths, such as my-not-ready-host:8300 it fails because my-not-ready-host isn't actually resovable. As it's not ready.

If I understand the codebase correctly, this could possible be fixed by creating a new ifpath like these others here:

hoverfly/core/proxy.go

Line 42 in aa3655e

if hoverfly.Cfg.HttpsOnly {

for -plain-http-tunneling. Inside the if you could implement a custom ConnectDial function, that just returns success always, as indicated here https://github.com/elazarl/goproxy/blob/master/proxy.go#L27

Disclaimer: I've never written even one line of go so might be just plain wrong.

[tommysitu]

@ttiurani, this PR SpectoLabs/goproxy#1 should fix your problem. The ConnectDial will only get called if the proxy does not produce a response.

[ttiurani]

Nice, thanks! Is that change testable with some hoverfly @tommysitu ? Do you need to upstream the PR before you can create a new hoverfly version?

[tommysitu]

@ttiurani I've just update goproxy version in this PR #683, please try it out.

[ttiurani]

@tommysitu I can verify that now the gradle test works after building from the plain-http-tunneling branch – even without network connection. However I haven't yet tested my actual Spring based project that uses hoverfly-java. (I have code that is PR-readyish about the -plain-http-tunneling parameter, but it would require that the binaries in there are also updated.)

I'll try to get the Spring 5 Java test also verified soon. But looks good now!

[ttiurani]

@tommysitu I'm not completely sure that everything is working as it should, but I now got a

2018/01/05 10:16:44 http: panic serving 127.0.0.1:49459: runtime error: invalid memory address or nil pointer dereference
goroutine 22 [running]:
net/http.(*conn).serve.func1(0xc4202308c0)
	/usr/local/go/src/net/http/server.go:1697 +0xd0
panic(0x1520e40, 0x1a1b350)
	/usr/local/go/src/runtime/panic.go:491 +0x283
github.com/SpectoLabs/hoverfly/core.NewProxy.func4(0x15b913d, 0x3, 0xc42021ac67, 0x15, 0x0, 0x20, 0x13, 0x20)
	/Users/ttiurani/devel/go/src/github.com/SpectoLabs/hoverfly/core/proxy.go:74 +0x2b6
github.com/SpectoLabs/hoverfly/vendor/github.com/SpectoLabs/goproxy.(*ProxyHttpServer).connectDial(0xc420202000, 0x15b913d, 0x3, 0xc42021ac67, 0x15, 0x0, 0x0, 0x100ec5e, 0xc42003d8f0)
	/Users/ttiurani/devel/go/src/github.com/SpectoLabs/hoverfly/vendor/github.com/SpectoLabs/goproxy/https.go:64 +0x5b
github.com/SpectoLabs/hoverfly/vendor/github.com/SpectoLabs/goproxy.(*ProxyHttpServer).handleHttps(0xc420202000, 0x1961c20, 0xc42023e2a0, 0xc420238400)
	/Users/ttiurani/devel/go/src/github.com/SpectoLabs/hoverfly/vendor/github.com/SpectoLabs/goproxy/https.go:130 +0xc5f
github.com/SpectoLabs/hoverfly/vendor/github.com/SpectoLabs/goproxy.(*ProxyHttpServer).ServeHTTP(0xc420202000, 0x1961c20, 0xc42023e2a0, 0xc420238400)
	/Users/ttiurani/devel/go/src/github.com/SpectoLabs/hoverfly/vendor/github.com/SpectoLabs/goproxy/proxy.go:98 +0xb42
net/http.serverHandler.ServeHTTP(0xc420120680, 0x1961c20, 0xc42023e2a0, 0xc420238400)
	/usr/local/go/src/net/http/server.go:2619 +0xb4
net/http.(*conn).serve(0xc4202308c0, 0x1962560, 0xc420206e40)
	/usr/local/go/src/net/http/server.go:1801 +0x71d
created by net/http.(*Server).Serve
	/usr/local/go/src/net/http/server.go:2720 +0x288

when trying to use the same binary through Spring and hoverfly-java. Any ideas as to what might be wrong? This is a new error.

I'm running in simulation mode, and this only happens with -plain-http-tunneling set on. If I run the same simulation.json file with the jaxrs client, there is no problem.

[tommysitu]

It works for me when I have updated hoverfly-java locally and used it in your reactor-netty test. Are you sure you are using the correct binary? Once I do a release today, we can have a look at your PR.

[ttiurani]

You're right, it was the wrong binary, sorry for the noise.

My Spring 5 webflux test now passes, so a release is more than welcome. Thank you!

[tommysitu]

The fix is now released in v0.15.0. What a long thread! Thank you for your patience @ttiurani

[qcastel]

We hit that issue today and we struggled a little bit because of an issue on our side:

We did:

hoverfly start -plain-http-tunneling

which actually should be:

hoverfly -plain-http-tunneling start

Would be nice if the hoverfly command could return an error if it doesn't recognised a parameter or just say it is ignoring it.

Anyway, the main idea of my comment was if someone else is stuck and hit that thread, remember to verify you put '-plain-http-tunneling' after the 'hoverfly' command.