Support Caddy 2

[mholt]

All the tests pass. I tried to keep as much as possible the same, but a few things don't translate well to Caddy 2, notably one test in probe_resist_test.go on L200, I had to change that test case since I didn't quite understand why it was the way it was before.

Have not vetted this for privacy or security guarantees. Do not rely on this code.

Example config:

{
	"apps": {
		"http": {
			"servers": {
				"srv0": {
					"listen": [
						":443"
					],
					"logs": {},
					"routes": [
						{
							"handle": [
								{
									"handler": "forward_proxy"
								}
							]
						},
						{
							"match": [
								{
									"host": ["foo.localhost"]
								}
							],
							"handle": [
								{
									"handler": "static_response",
									"body": "Yahaha! You found me!"
								}
							]
						}
					]
				}
			}
		}
	}
}

@sergeyfrolov Please give this a once-over if you have a chance. Or better yet, ping me in Slack so I can give you a brief walkthrough of the changes. It'd be good to have an extra set of eyeballs review any sensitive parts that I might have (probably) botched up.

I also wonder if we can remove basicauth logic out from this code and use Caddy 2's existing basicauth module instead. Let's chat when you have a chance. But, if I don't hear from you in the next couple weeks I might just go ahead anyway.

Thanks!

[mholt]

@sergeyfrolov Do you think you'd have a chance to look this over or try it out soon?

[mholt]

@sergeyfrolov Thanks for taking a look!

I guess the main thing to verify now is the probe resistance and authentication. I'm hoping we can remove the authentication logic entirely...

When do you have time today or this weekend? (Feel free to ping me in Slack at your convenience.)

[mholt]

Btw, I think this is probably good to go for most people, if we are OK with being complacent about the failing test case.

[mholt]

Would anyone who is interested in this like to take up development and maintenance of this repo? Preferably if you have experience with writing proxies and tunneling connections, OR if you are able to understand the code and tests. I am afraid I don't have the time or the special expertise required to ensure the desired security guarantees on my own.

[gaby]

@mholt I was really looking forward to using the forwardproxy until I realized it hasn't been updated in a longtime and doesn't have support for Caddy 2.

Would it make sense to create a new release under 1.0.2 with the 5 commits missing from 1.0.1, and then merging this branch towards releasing 2.0.0?

From looking at your changes besides the test failing, I don't see anything major concerns. There's plenty of updates needed on this repo. Adding proper CI with github actions, adding support for recent go versions. Updating and release a new Docker image, etc.

[mholt]

@gaby Yeah, that sounds like a good plan. Honestly I've been super swamped and haven't been able to use or work on this for a while. I'm sorry about that.

I'd be happy to open up maintainership to someone else who is confident in the concepts and working with this code. Would you be interested? 😁

[gaby]

@mholt I can definitely help with some of the maintenance tasks while getting up to speed with the code base.

I have mostly done backend services and proxing/forwarding between protocols. While I'm not up to speed in some of the concepts used here like "ProbeResistant", and "Server PAC", I do have extensive knowledge configuring, managing and deploying Squid Proxy for a large user base.

[mholt]

@gaby Your experience is relevant, and that's fine with me -- I'm sure many people watching this PR would appreciate it!

While I'm not up to speed in some of the concepts used here like "ProbeResistant", and "Server PAC",

Simple enough: probe resistance is the ability to not appear as a proxy server when "probed" remotely, and a PAC file is a Proxy Auto-Config that is usually just a 1-line string that defines which proxy to use and/or how to use it: https://en.wikipedia.org/wiki/Proxy_auto-config

I'll add invite you to the repo in a few minutes. Feel free to help out as much as you'd like, I just ask that code changes are tested before being merged in :) Thanks!

Anyone else who has relevant experience or know-how is also welcome to contribute, just let me know.

[gaby]

@mholt I just accepted the invite! Sounds good! Looking forward to getting this project back up and running.

[gaby]

I'm planning to add github-actions support to this branch tonight. Also updating dependencies/formatting/linting/etc.

[gaby]

Some news:

• The tests for the current master branch are failing with a panic.
• I'm getting the FAIL for the Probe test.
• I have updated all the dependencies to latest, including Caddy 2.6.x.
• The Dockerfile has to be rewritten completely, since getcaddy.com is not a thing anymore.
• Fixed several warnings raised by staticcheck.
• There's several minor warnings raised by gosec. Planing on fixing those too.

I'm holding up on the github-actions part until I can figure out why the probe test is failing.

[mholt]

@gaby

I'm planning to add github-actions support to this branch tonight. Also updating dependencies/formatting/linting/etc.

Awesome!! Thanks so much for doing that.

I'm getting the FAIL for the Probe test.

Yeah, that's where I got hung up too. I'm not sure if the test is right, wrong, or irrelevant/outdated now.

The tests for the current master branch are failing with a panic.

The master branch is for Caddy v1 and probably hasn't been updated in years even at the end of Caddy v1's lifetime.

Thank you thank you again for all you're doing :pray:

[gaby]

@mholt I have been looking at this test all weekend. One big difference I noticed is that v1 doesnt require basicauth while v2 does for Probe resistance.

I added basicauth to the server listening on :9999 and that caused the error to change. Now I get an error about response being http 403 instead of http 200.

[mholt]

@gaby Thank you for looking into it! :pray:

I have been looking at this test all weekend. One big difference I noticed is that v1 doesnt require basicauth while v2 does for Probe resistance.

So, I believe basicauth should be required to use the proxy, but with probe resistance enabled, the behavior should be pass-thru/ignore if basicauth is missing or incorrect. The idea with probe resistance enabled is to not act like a proxy (and hence not require auth), but if auth is present and correct, then transparently allow the use of the proxy.

It's quite possible I got some of this wrong in my v2 work. I trust the v1 implementation as basically correct, since it was done by brains smarter than my own. :innocent:

[gaby]

@mholt I have been doing a ton of debugging, even using GPT tools to try to get a better understanding of the issue. One thing I realized is that the header failing is related to HTTP/3

Could this be caused by https://github.com/caddyserver/caddy/issues/4996 ?

Failing test:

=== RUN   TestGETAuthWrongProbeResist
    probe_resist_test.go:52: header "Alt-Svc" is different: different string at position 0: h3=":8888"; ma=2592000 vs h3=":9999"; ma=2592000
--- FAIL: TestGETAuthWrongProbeResist (0.00s)

Using fmt.Printf:

=== RUN   TestGETAuthWrongProbeResist
responseProbeResist: &{200 OK 200 HTTP/2.0 2 0 map[Accept-Ranges:[bytes] Alt-Svc:[h3=":8888"; ma=2592000] Content-Length:[36] Content-Type:[text/html; charset=utf-8] Date:[Thu, 06 Apr 2023 02:40:24 GMT] Etag:["rrycf210"] Last-Modified:[Thu, 23 Mar 2023 02:33:02 GMT] Server:[Caddy]] {0xc0019f4780} 36 [] false false map[] 0xc0023c5800 0xc0021c7080}

responseReference: &{200 OK 200 HTTP/2.0 2 0 map[Accept-Ranges:[bytes] Alt-Svc:[h3=":9999"; ma=2592000] Content-Length:[36] Content-Type:[text/html; charset=utf-8] Date:[Thu, 06 Apr 2023 02:40:24 GMT] Etag:["rrycf210"] Last-Modified:[Thu, 23 Mar 2023 02:33:02 GMT] Server:[Caddy]] {0xc001f5fc80} 36 [] false false map[] 0xc0002cdc00 0xc0023e6580}

responseForwardProxy: &{407 Proxy Authentication Required 407 HTTP/2.0 2 0 map[Alt-Svc:[h3=":4891"; ma=2592000] Content-Length:[0] Date:[Thu, 06 Apr 2023 02:40:24 GMT] Proxy-Authenticate:[Basic realm="Caddy Secure Web Proxy"] Server:[Caddy]] {} 0 [] false false map[] 0xc0022d5000 0xc0021c76b0}

    probe_resist_test.go:57: header "Alt-Svc" is different: different string at position 0: h3=":8888"; ma=2592000 vs h3=":9999"; ma=2592000
--- FAIL: TestGETAuthWrongProbeResist (0.00s)

Relevant test setup servers:

caddyForwardProxyProbeResist = caddyTestServer{
	addr: "127.0.88.88:8888",
	root: "./test/forwardproxy",
	tls:  true,
	proxyHandler: &Handler{
		PACPath:         "/superhiddenfile.pac",
		ACL:             []ACLRule{{Subjects: []string{"all"}, Allow: true}},
		ProbeResistance: &ProbeResistance{Domain: "test.localhost"},
		BasicauthUser:   "test",
		BasicauthPass:   "pass",
	},
	httpRedirPort: "8880",
}

caddyDummyProbeResist = caddyTestServer{
	addr:          "127.0.99.99:9999",
	root:          "./test/forwardproxy",
	tls:           true,
	httpRedirPort: "9980",
}

[mholt]

Ah yeah, you may be onto something... the HTTP/3 server thinks its port is one thing but the outside value is another due to the tunneling, right?

[gaby]

@mholt Yes, Caddy 1 didnt have HTTP/3 and Caddy 2 defaults to enabling HTTP/3. The tests are testing all the headers and based on https://github.com/caddyserver/caddy/issues/4996 HTTP/3 will set the Alt-Svc header port to whatever the server is listening on.

While the forwardproxy tests can be updated to be flexible around this, I think it exposes a security issue. If someone is running a forwardproxy with port forwarding the HTTP/3 headers will leak the internal proxy port.

[mholt]

Yeah, so we should probably figure out a way to fix that to avoid exposing the proxy. Thanks for figuring that out. I don't have the bandwidth to work on that right now but I can at least participate in discussions and proposals :D

[klzgrad]

running a forwardproxy with port forwarding

Do you mean running forwardproxy behind a load balancer front end?

In that case this may not be a security issue for forwardproxy per se but a whole system issue. The design and threat model of a probe resistant forwardproxy need to have it running as the front end, so for a probe, the server looks identical to a regular Caddy server. If the scenario has a different front end, then it is the job of that front end to ensure it looks identical to a regular server. So that load balancer has to rewrite the Alt-Svc header for this setup to work. And even without rewriting Alt-Svc, it would not reveal the existence of a forwarding proxy, aside from the existence of a regular load balancer and misconfigured backend with Alt-Svc coming out from behind.

[gaby]

@klzgrad Yes, that's what I was trying to say.😄 I don't see anything wrong implementation wise, so I'm leaning towards updating the tests. Currently two of the tests are failing with the exact same error.

[mholt]

Sounds good; as long as the probe resistance is preserved and the tunneling continues to work, the change should be acceptable!

[gaby]

I'm merging #99 into this branch today, and then rebasing with my changes to push them to github.

[gaby]

Since quic-go 0.36 release we can't build/test/run the forwardproxy. I'm not sure how the error doesn't show up in caddyserver Related to https://github.com/quic-go/quic-go/issues/3907

[gaby]

TODO:

• ~Fix all the gosec issues~
• ~Fix all the golanci-lint issues~
• ~Fix all the govulncheck issues~
• ~Add staticcheck and go-critic to CI process~
• Enable multi-platform CI tests once ubuntu-latest is stable
• Update README
• Update Dockerfile
• Add workflow for building/publishing the Docker Image
• ~Figured out why we can't build/test/check the code since quic-go newest release~

[mholt]

Oh, quic-go might have had some breaking changes... :thinking:

[gaby]

Oh, quic-go might have had some breaking changes... 🤔

Yeah... I'm running the latest, so is the CI.

$ go version
go version go1.20.5 linux/amd64

CaddyServer is also using the same version but the CI Tests are passing. See here

[gaby]

@mholt I was recommended to downgrade qtls-go to v0.2.2, so after doing that I get errors related to Caddy listeners in v2.6.4

go vet ./...
# github.com/caddyserver/caddy/v2
../../../../go/pkg/mod/github.com/caddyserver/caddy/[email protected]/listeners.go:455:45: cannot use earlyLn (variable of type *quic.EarlyListener) as quic.EarlyListener value in struct literal
../../../../go/pkg/mod/github.com/caddyserver/caddy/[email protected]/listeners.go:458:10: cannot use nil as quic.EarlyListener value in return statement
../../../../go/pkg/mod/github.com/caddyserver/caddy/[email protected]/listeners.go:473:9: cannot use &fakeCloseQuicListener{…} (value of type *fakeCloseQuicListener) as quic.EarlyListener value in return statement

The same happens with golanci-lint:

golangci-lint run ./... --tests=false
caddyfile.go:8:8: could not import github.com/caddyserver/caddy/v2 (-: # github.com/caddyserver/caddy/v2
../../../../go/pkg/mod/github.com/caddyserver/caddy/[email protected]/listeners.go:455:45: cannot use earlyLn (variable of type *quic.EarlyListener) as quic.EarlyListener value in struct literal
../../../../go/pkg/mod/github.com/caddyserver/caddy/[email protected]/listeners.go:458:10: cannot use nil as quic.EarlyListener value in return statement
../../../../go/pkg/mod/github.com/caddyserver/caddy/[email protected]/listeners.go:473:9: cannot use &fakeCloseQuicListener{…} (value of type *fakeCloseQuicListener) as quic.EarlyListener value in return statement) (typecheck)
	caddy "github.com/caddyserver/caddy/v2"

Let me delete all my /pkg and see if that helps

Edit: Same errors

[mholt]

Huh, that's weird. Maybe @WeidiDeng has an idea as to why it's not able to compile, even though Caddy is 🤔

[gaby]

I was able to figured it out.... So I went and removed ALL the dependencies from go.mod and only left the original 3-4 dependencies. Then ran go mod tidy and now i'm able to build/run tests :-)

Note: Never run go get -u -t -d ./... :-)

[mholt]

Oh! You know I've had that problem before. Not sure how it arises but resetting go.mod does help sometimes. Can't believe I forgot that.