Implement issue #6296 passing FDs / socket activation

[MayCXC]

Implements #6296 without the changes to existing config fields made by #6507. Example Caddyfile:

{
	sockets :80 {env.CADDY_HTTP_FD}
	sockets :443 {env.CADDY_HTTPS_FD}
	sockets udp/:443 {env.CADDY_HTTP3_FD}
	auto_https disable_redirects
	admin off
}

http://localhost {
	bind tcp/ {
		protocols h1
	}
	log
	respond "Hello, HTTP!"
}

https://localhost {
	bind tcp/ {
		protocols h1 h2
	}
	bind udp/ {
		protocols h3
	}
	log
	respond "Hello, HTTPS!"
}

the sockets global option is now used instead of servers, to differentiate that while its first argument is a listen address, it applies to all the servers that bind to it, not just the ones that bind to it first. the bind directive can now be followed with an optional block and a protocols subdirective, which specifies which protocols to serve that listen address with.

this can be used to separate the tcp and udp listeners for a h1+h2+h3 server, which is needed for socket activation. it also allows for other configurations that previously resulted in errors, like serving h1 and h2 on a unix socket and h3 on a unixgram socket with the same server.

[MayCXC]

I tested the above config with a systemd socket unit. systemd-socket-activate did not work with both tcp and udp sockets, but this systemd.socket and systemd.service did:

caddy.socket:

[Socket]
ListenStream=80
ListenStream=443
ListenDatagram=443

[Install]
WantedBy = sockets.target

caddy.service:

[Unit]
Description=Caddy
Documentation=https://caddyserver.com/docs/
After=network.target network-online.target
Requires=network-online.target

[Service]
Type=notify
User=caddy
Group=caddy
Environment=CADDY_HTTP_FD=3 CADDY_HTTPS_FD=4 CADDY_HTTP3_FD=5
ExecStart=/path/to/caddy run --environ --config /path/to/Caddyfile
ExecReload=/path/to/caddy reload --config /path/to/Caddyfile --force
TimeoutStopSec=5s
LimitNOFILE=1048576
PrivateTmp=true
ProtectSystem=full
AmbientCapabilities=CAP_NET_ADMIN CAP_NET_BIND_SERVICE

[Install]
WantedBy=multi-user.target

[mohammed90]

Keep in mind, those gosec workarounds won't be needed once their bug is fixed.

securego/gosec#1187

[MayCXC]

Keep in mind, those gosec workarounds won't be needed once their bug is fixed.

securego/gosec#1187

yeah, it might be worth adding a TODO to remove most of them. for now i just wanted the tests to turn green.

[MayCXC]

I'll take a stab at windows support, but now I'm not sure if it's worth nolinting all the G115 errors :[

[MayCXC]

support for non unix builds was simple enough, but I don't want to nolint all those integer conversions. I feel it'd be better to remove the ones I added, and let G115 stay red / disable it. otherwise, this where I want it to be.

[mholt]

Thanks for working on this. It's quite a big change, and I'm not sure I follow every aspect of it. If we merge this in are you able to commit to helping fix any bugs that are reported related to this? Especially since the need seems to be fairly niche.

But all things considered this looks doable... just complicated 😵

[mholt]

Update the godoc to describe the socket parameter

[MayCXC]

📝

[mholt]

Let's put a godoc comment on this; you could even do something like "LisenQUIC is the same as ListenQUICWithSocket but without a special socket."

[MayCXC]

both funcs have doc comments now 👍

[mholt]

Godoc; describe the socket parameter. (Do this for all new/changed methods.)

[MayCXC]

all the socket and socketFile parameters have been documented

[MayCXC]

Thanks for working on this. It's quite a big change, and I'm not sure I follow every aspect of it. If we merge this in are you able to commit to helping fix any bugs that are reported related to this? Especially since the need seems to be fairly niche.

But all things considered this looks doable... just complicated 😵

yep, and I'll need those bugs fixed as much as anyone who reports them. a big part of the diff is the optional block for listen, which could have been a separate contribution. but it is needed for socket activation, and it addresses the case already mentioned in server.go: TODO: Maybe a better default is to not enable HTTP/3 if we do not know the network?

to decide which addresses to serve h3 with, protocols are mapped the way that listener addresses already were, where each address is 1:N with protocols, and each protocol is 1:N with server blocks. then all three are reduced to an N:N:N association, "which listen addresses serve which protocols from which servers".

so we can still assume that when h3 is enabled on a tcp/ address it is intentional, "i want to serve h3 on this host and port with udp", but now if h3 is enabled on a unix/ address, that is an error even when h3 is the only protocol enabled. h3 can be disabled for each address individually, so it can be served from any udp or unixgram address you want instead.

I'll document those socketFile args, right now they are just the sockets passed to caddy via socket activation. But they could also come from a listener map passed to child caddy with caddy upgrade ;]

[mholt]

Thank you so much. This is probably a high-quality change, from the looks of it, so I'd say we can give this a shot. I am sorry it has taken me so long to get to this point. I don't fully grok all the changes here, but I like seeing test cases and the ongoing maintenance of this PR. Thanks again, and please stick around and contribute again :)

[MayCXC]

of course, I really think the changes are simpler than the diff makes them seem. if you are available for a voice call tomorrow, i can probably talk through them better than i can write them out here. thinking from the angle that every listener needs to read at most one unique file descriptor number from the config, and then make a listener from it, each chunk is just the next most necessary change.

[MayCXC]

@mholt today i thought more about the approach in https://github.com/WeidiDeng/caddy-socket-activation , and considered that we could also register two networks, with addresses like fd/3 and fdgram/{env.SOCKET_FD_HTTP3}. i wanted listeners.go behaviors, like the reuse pool and uds permissions setting, to work the exact same for configs that added socket activation to their existing listeners. for a custom network, that at least means a listenerPool arg needs to be added to ListenerFunc, and a lot of pool logic gets duplicated, quite icky.

but i think socket fds really have the same requirements as unix sockets in the pool, just without needing being unlinked. so instead, we could treat fd and fdgram as built in networks, i.e panic both as reserved in RegisterNetwork, add both to IsUnixNetwork, add fdgram to the the listenReusable datagram check, etc. then the global sockets option, ListenSockets config array, and socketFile arguments can all be removed.

this interface feels much better in my head, so if it is ok to superset the net.go known networks I'll make a third branch for it. then as a bonus i can make a really funny plugin that uses RegisterNetwork and go-systemd to create a fd or fdgram NetworkAddress for each systemd "named" socket, and just calls .Listen() on them. this can be a separate contribution.

[mholt]

@MayCXC Actually I really like the sound of that. If it's not too much trouble I would like to see it!

I was about to merge this -- but again, its complexity caused hesitation. Would definitely be curious about this other approach :)

[MayCXC]

@MayCXC Actually I really like the sound of that. If it's not too much trouble I would like to see it!

I was about to merge this -- but again, its complexity caused hesitation. Would definitely be curious about this other approach :)

just in time for me to talk you out of it 😆
all of the changes in this branch save the sockets option are needed for fd/ and fdgram/, so it will be easy to add from here.

[MayCXC]

closing in favor of #6573
this PR was for master...MayCXC:caddy:socket-activation-with-global-sockets-option