Add support for 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?

[sergeyfrolov]

Hey @mholt!
Sorry I haven't paid much attention to my open-source projects lately. Between academia and new full-time job I don't have much extra energy.

I took a look at the code and left some comments.

I specifically looked through the new ServeHTTP function to see if correct probe-resistance behavior is preserved, and it appears to be. Haven't yet checked why probe-resistance tests fail, but we can fix them after ensuring that it does actually work correctly. The most bulletproof way to do that would be to spin up 2 Caddy servers: one without forwardproxy, and another with forwardproxy and probe_resistance and see if there's a difference in their responses to requests. How about we try to do that today or tomorrow evening (or some other evening)?

[sergeyfrolov]

Why do we add those 2 fields, instead of using the existing authCredentials slice? How does this temporary solution help tests and why can't we re-use existing credentials checking code?

[mholt]

These fields are here so users can access them via configuration. Otherwise the authCredentials field is unexported so the JSON unmarshaler can't reach it.

I am hoping to remove auth from this module entirely, and let Caddy's existing authentication handler deal with it. Do you think that's possible?

[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.

[klzgrad]

All tests are green. Any remaining issues?

[WeidiDeng]

There are some uses of http.Flusher which won't work if logging is enabled. These two parts need to be changed 361 and 500. This diff only covers part of the issue.

[johnzhanghua]

There are some uses of http.Flusher which won't work if logging is enabled. These two parts need to be changed 361 and 500. This diff only covers part of the issue.

Is that related to this PR ? @gaby what is blocking this merge ?

[WeidiDeng]

There are some uses of http.Flusher which won't work if logging is enabled. These two parts need to be changed 361 and 500. This diff only covers part of the issue.

Is that related to this PR ? @gaby what is blocking this merge ?

It's related in that if it's not fixed, this plugin won't work if you need access logs.

[gaby]

@WeidiDeng I will fix it thus weekend. Thanks!

@johnzhanghua Only a few things left, cleaning up the readme, updating the docker file + workflow,

We could probably merge after that and do a release candidate.

[gedw99]

ready to dog food this. woof !!!!

[FrostKiwi]

So happy to see the commit yesterday. Looking forward to a potential merge.

[gaby]

Tests are now run in:

• Linux
• Windows
• MacOS This one gets stuck

TODO:

• Add coverage report
• Update dependencies
• Migrate default branch to main
• Add Benchmarks
• Migrate tests to testify
• Update/Cleanup README
• Docker

[fbuetler]

Just stumbled over this PR as I am looking for a way to use caddy as a forward proxy. Nice work!

I reviewed the caddy2 branch and would like to note some points:

• The module handles proxy authentication itself. But I'd argue this functionality can be removed in favor of http.authentication.providers like http_basic. A comment even mentioned this should only be temporary.
• It would be nice to have the same plugin functionality for the transport as there is in the reverse proxy module, but I don't know how this is gonna work with the custom dial functionality (this is more of a nice-to-have feature for the future).

[klzgrad]

Please reduce scope and get this merged first after 3 years and we can talk about more improvements.

Including

Add coverage report
Add Benchmarks
Migrate tests to testify
Update/Cleanup README
Docker

which I argue are non-essential work before the merge.

[gaby]

@mholt This should be good to merge now. Pending issues and documentation can be handle in a separate PR.

[mholt]

Acknowledged. Thank you for your help with it!