mastercactapus / caddy-proxyprotocol Goto Github PK

Hi there! I'm writing in because we know of a Caddy instance that has several thousand (tens of thousands) of sites configured to use the proxyprotocol plugin.

It appears that this double for loop is causing unacceptably high latency during new connections, because it must iterate up to tens of thousands of sites before finding the right one, and then through each subnet (in this case, 2 per site).

Do you have any ideas about how to improve performance here? Is it possible to replace the slice with a map, or do a binary search?

Not sure if the problem comes from caddy or the proxyprotocol plugin.

If I setup caddy as a reverse proxy (with haproxy in front of it) and define the proxyprotocol directive, as soon as I kill -USR caddy, I end-up in an unusable state with an SSL error in the browser and the following error in the caddy log:

caddy_1 | 2018/06/16 21:25:06 http: TLS handshake error from 172.18.0.4:51960: tls: oversized record received with length 22617

Hi! I just spent some time trying set up this software behind an sniproxy-based reverse protocol with enabled “proxy_protocol” support, but I couldn't get it to work. When using the ALPN challenge I get:

test_caddy | Activating privacy features... 2019/01/26 20:44:19 [INFO] acme: Registering account for [email protected]
test_caddy | 2019/01/26 20:44:19 [INFO] [test.ninetailed.ninja] acme: Obtaining bundled SAN certificate
test_caddy | 2019/01/26 20:44:19 [INFO] [test.ninetailed.ninja] AuthURL: https://acme-staging-v02.api.letsencrypt.org/acme/authz/cFVMAoxTSk4JwTN2D0-MVhYXB0sVmOQdg6LSUNsgz78
test_caddy | 2019/01/26 20:44:19 [INFO] [test.ninetailed.ninja] acme: use tls-alpn-01 solver
test_caddy | 2019/01/26 20:44:19 [INFO] [test.ninetailed.ninja] acme: Trying to solve TLS-ALPN-01
test_caddy | 2019/01/26 20:44:21 http: TLS handshake error from 172.18.0.5:44350: tls: oversized record received with length 22617
test_caddy | 2019/01/26 20:44:21 http: TLS handshake error from 172.18.0.5:44354: tls: oversized record received with length 22617
test_caddy | 2019/01/26 20:44:21 http: TLS handshake error from 172.18.0.5:44358: tls: oversized record received with length 22617
test_caddy | 2019/01/26 20:44:21 http: TLS handshake error from 172.18.0.5:44362: tls: oversized record received with length 22617
test_caddy | 2019/01/26 20:44:21 http: TLS handshake error from 172.18.0.5:44364: tls: oversized record received with length 22617
test_caddy | 2019/01/26 20:44:21 http: TLS handshake error from 172.18.0.5:44366: tls: oversized record received with length 22617
test_caddy | 2019/01/26 20:44:21 http: TLS handshake error from 172.18.0.5:44368: tls: oversized record received with length 22617
test_caddy | 2019/01/26 20:44:21 http: TLS handshake error from 172.18.0.5:44370: tls: oversized record received with length 22617
test_caddy | 2019/01/26 20:44:26 [test.ninetailed.ninja] failed to obtain certificate: acme: Error -> One or more domains had a problem:
test_caddy | [test.ninetailed.ninja] acme: error: 400 :: urn:ietf:params:acme:error:tls :: remote error: tls: record overflow, url: 
test_caddy | exit status 1
test_caddy exited with code 1

When disabling TLS-ALPN-01 and using HTTP-01 instead, I get:

test_caddy | Activating privacy features... 2019/01/26 20:47:30 [INFO] [test.ninetailed.ninja] acme: Obtaining bundled SAN certificate
test_caddy | 2019/01/26 20:47:31 [INFO] [test.ninetailed.ninja] AuthURL: https://acme-staging-v02.api.letsencrypt.org/acme/authz/pSEq4OKVS8Rq_4Qvr_2DBRWW8izqUI0wkM1OXIkVhXU
test_caddy | 2019/01/26 20:47:31 [INFO] [test.ninetailed.ninja] acme: Could not find solver for: tls-alpn-01
test_caddy | 2019/01/26 20:47:31 [INFO] [test.ninetailed.ninja] acme: use http-01 solver
test_caddy | 2019/01/26 20:47:31 [INFO] [test.ninetailed.ninja] acme: Trying to solve HTTP-01
test_caddy | 2019/01/26 20:47:37 [test.ninetailed.ninja] failed to obtain certificate: acme: Error -> One or more domains had a problem:
test_caddy | [test.ninetailed.ninja] acme: error: 403 :: urn:ietf:params:acme:error:unauthorized :: Invalid response from http://test.ninetailed.ninja/.well-known/acme-challenge/XyPxxbLYHFOe8R_FySSwLJ2daCY1sa6epb71Yvdf39w [2001:1608:39::a]: 400, url: 
test_caddy | exit status 1
test_caddy exited with code 1

Both of these work however, when disabling the PROXY protocol in both Caddy (this plugin) and sniproxy (the proxy_protocol flag). I can also attest that sniproxy's PROXY protocol implementation works without issues when used with nginx.

When getting a cert without using the PROXY protocol (ie: disable this plugin), I'm also not able to view HTTPS content when enabling it afterwards (but HTTP work although it will receive the PROXY headers as well):

test_caddy | Activating privacy features... 2019/01/26 20:53:35 [INFO][test.ninetailed.ninja] Obtain: Certificate already exists in storage
test_caddy | done.
test_caddy | https://test.ninetailed.ninja
test_caddy | http://test.ninetailed.ninja
test_caddy | 2019/01/26 20:53:35 https://test.ninetailed.ninja
test_caddy | 2019/01/26 20:53:35 http://test.ninetailed.ninja
test_caddy | 2019/01/26 20:53:35 [NOTICE] Sending telemetry: we were too early; waiting 1h1m45.482846269s before trying again
test_caddy | 2019/01/26 20:53:58 http: TLS handshake error from 172.18.0.5:39472: invalid source address
test_caddy | 2019/01/26 20:54:01 http: TLS handshake error from 172.18.0.5:39498: invalid source address

In each case the Caddyfile was just:

test.ninetailed.ninja
proxyprotocol

And the sniproxy config was:

listener [::]:80 {
    protocol http
}

listener [::]:443 {
    protocol tls
}

table {
    # some other rules

    test.ninetailed.ninja test_caddy proxy_protocol

    # some other rules
}

(where test_caddy is the name of the Caddy server container).

Do maybe have some insights into this by any chance?

I opened an issue describing a DoS vulnerability in the github.com/mastercactapus/proxyprotocol package used by this plugin: mastercactapus/proxyprotocol#1

This code is the only consumer of the package I was able to find with light googling/github searching.

Since fixing the mastercactapus/proxyprotocol parsing bug will require an updated version of this plugin I wanted to file an issue here as well so it doesn't fall through the cracks.

Configuring the plugin with a source address filter is one potential mitigation in the short term.

The documentation would greatly benefit from strong language encouraging the use of a source address filter in all circumstances since above-and-beyond the current bug omitting such a filter will allow any client to spoof the IP metadata processed by the plugin. E.g. Apache's equivalent documentation says:

It is critical to only enable this behavior from intermediate hosts (proxies, etc) which are trusted by this server, since it is trivial for the remote useragent to impersonate another useragent.

Hi

Could you please explain how to install this plugin as the following command output an error:

docker plugin install mastercactapus/caddy-proxyprotocol
Error response from daemon: error resolving plugin reference: pull access denied, repository does not exist or may require authorization: server message: insufficient_scope: authorization failed

Thank you.

Hi!

Whenever you're ready, Caddy 2 (still in beta, at time of writing) is now able to accommodate wrapping listeners. The commit that adds this is caddyserver/caddy@f596fd7.

The docs aren't published yet, but basically, the servers struct in Caddy's JSON will have a new field called listener_wrappers which takes an array of structs. Each struct can configure a different listener wrapper. (Users can even specify which wrappers come before or after the TLS listener, so if a plugin needs to read before the TLS handshake, that's fine too! But by default they go after TLS.)

Listener wrapper modules are in the caddy.listeners namespace, so for example: caddy.listeners.proxy_protocol might be your module's ID.

The site has docs for extending Caddy 2 but I will be happy to help answer any questions you have in the process!

Hey Nathaniel, hope you're doing well!

Caddy's import path (and Go module name) has changed from

github.com/mholt/caddy

to

github.com/caddyserver/caddy

Unfortunately, Go modules are not yet mature enough to handle a change like this (see https://golang.org/issue/26904 - "haven't implemented that part yet" but high on priority list for Go 1.14) which caught me off-guard. Using Go module's replace feature didn't act the way I expected, either. Caddy now fails to build with plugins until they update their import paths.

I've hacked a fix into the build server, so downloading Caddy with your plugin from our website should continue working without any changes on your part, for now. However, please take a moment and update your import paths, and do a new deploy on the website, because the workaround involves ignoring module checksums and performing a delicate recursive search-and-replace.

I'm terribly sorry about this. I did a number of tests and dry-runs to ensure the change would be smooth, but apparently some unknown combination of GOPATH, Go modules' lack of maturity, and other hidden variables in the system or environment must have covered up something I missed.

This bash script should make it easy (run it from your project's top-level directory):

find . -name '*.go' | while read -r f; do
	sed -i.bak 's/\/mholt\/caddy/\/caddyserver\/caddy/g' $f && rm $f.bak
done

We use this script in the build server as part of the temporary workaround.

Let me know if you have any questions! Sorry again for the inconvenience.