Repository: caddyserver/caddy (language: go)

Issue:
Config reload fails when admin endpoint is a socket
I've been using this Caddyfile global configuration, which was working at e289ba6187ddd50afc86feb85246a14298673a69 but is no longer working in 2.6.0 (Caddy's official package for Debian).
```
{
	admin unix//run/caddy/admin.sock
}
```
The goal is to support zero downtime config reloads without exposing Caddy's config to any process on the system that might send some json to 127.0.0.1.  Partly in case some other service gets compromised, and partly because we have human SSH user accounts on the system which don't have sysadmin (sudo) access and should not be able to reconfigure the main web server.

`/run/caddy` has mode `0750` and `caddy:caddy` ownership.  

Log:
```
systemd[1]: Reloading Caddy.
caddy[3372028]: {"level":"info","ts":1663700361.6707227,"msg":"using provided configuration","config_file":"/etc/caddy/Caddyfile","config_adapter":""}
caddy[3371872]: {"level":"info","ts":1663700361.6808317,"logger":"admin.api","msg":"received request","method":"POST","host":"","uri":"/load","remote_ip":"@","remote_port":"","headers":{"Accept-Encoding":["gzip"],"Cache-Control":["must-revalidate"],"Content-Length":["20327"],"Content-Type":["application/json"],"User-Agent":["Go-http-client/1.1"]}}
caddy[3371872]: {"level":"error","ts":1663700361.6862683,"logger":"admin.api","msg":"request error","error":"loading config: loading new config: starting caddy administration endpoint: listen unix /run/caddy/admin.sock: bind: address already in use","status_code":400}
caddy[3372028]: Error: sending configuration to instance: caddy responded with error: HTTP 400: {"error":"loading config: loading new config: starting caddy administration endpoint: listen unix /run/caddy/admin.sock: bind: address already in use"}
systemd[1]: caddy.service: Control process exited, code=exited, status=1/FAILURE
```

Relatedly I also needed this addition to the packaged `/lib/systemd/system/caddy.service` to make Caddy start reliably with the socket admin endpoint (loaded via systemd drop-in config):
```
[Service]
# bones_was_here: 2.5.2 fails to start if the admin endpoint socket still exists!
ExecStartPre=rm -f /run/caddy/admin.sock
```


Discussion / hints:
If it's hard to cleanly re-use the same file, the socket path could point to a directory instead of a file.  This would allow each process to create its own socket file in that directory so multiple can exist during a handover.
Thanks for the report.

How can I reproduce it?

I cannot with this Caddyfile:

```
{
	debug
	admin unix//home/matt/Desktop/admin.sock
}
```

and then running Caddy with it, and then running `caddy reload`, works just fine.

Still, now that you mention it, I somehow missed that SO_REUSEPORT does not work for unix sockets (even though it's an SO, "socket option")... (That's kind of ironic that a unix socket option does not work for unix sockets.)

But it works for me? How can I see what you're seeing?
Hmm... I tried it again to see if it was a one-off but it failed again the same way immediately, on two different servers.  Are you using your Debian package?  Possibly it only happens with the included systemd configuration?
There's certainly a few quirks in the naming of some network stuff in go, when was the last time you connected to a system by _dialling_ it :)
Are you reloading an unchanged config?  If i remove `--force` from the `ExecReload=/usr/bin/caddy reload --config /etc/caddy/Caddyfile --force` line then it succeeds, but if I actually change the config then it fails again.
@bones-was-here Ah, okay it happens only as a systemd service.

If you follow my steps above (without systemd, just a plain config file like that and run `caddy run` then `caddy reload`) does it work for you?

Trying to narrow down if systemd is the culprit here.

(Edit: The `--force` behavior is expected, as the config won't be changed at all if it hasn't changed.)
I can reproduce it without systemd, compiled with go 1.19.1 on my PC (debian testing).  Caddyfile:
```
{
        debug
        admin unix//home/bones/tmp/admin.sock
}
```
`./caddy run --config Caddyfile`  
Startup looks OK, socket file has been created.  
`./caddy reload --config Caddyfile`  
Succeeds, logging that config is unchanged.  
`./caddy reload --config Caddyfile --force`  
`ERROR   admin.api       request error   {"error": "loading config: loading new config: starting caddy administration endpoint: listen unix /home/bones/tmp/admin.sock: bind: address already in use", "status_code": 400}`

Huh, I'm stumped then. For me it only happens as a systemd service.

I mean, I presume this has to do with our recent use of SO_REUSEPORT, but I'm surprised I can't reproduce it except with systemd.
It logs `INFO    admin.api       received request` before hitting the error, so I guess at that point you would know if the config has been parsed and is OK to run?  In which case it _should_ be safe to just delete the socket file before starting the new listener on it?  That should also work for when the system hard reboots and at next startup the socket file still exists.
That would disconnect the current API request. We need to close the old listener while the new one is already running on the same socket...

https://copyconstruct.medium.com/file-descriptor-transfer-over-unix-domain-sockets-dcbbf5b3b6ec
Currently working on a fix. It involves copying the file descriptor and closing the old one. I *think* it works in some testing I've done, but still doing more testing and polishing of the implementation.

Fix patch:
diff --git a/listen.go b/listen.go
index 2c4a0b28877..268785a3d57 100644
--- a/listen.go
+++ b/listen.go
@@ -1,4 +1,21 @@
-//go:build !linux
+// Copyright 2015 Matthew Holt and The Caddy Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// TODO: Go 1.19 introduced the "unix" build tag. We have to support Go 1.18 until Go 1.20 is released.
+// When Go 1.19 is our minimum, change this build tag to simply "!unix".
+// (see similar change needed in listen_unix.go)
+//go:build !(aix || android || darwin || dragonfly || freebsd || hurd || illumos || ios || linux || netbsd || openbsd || solaris)
 
 package caddy
 
diff --git a/listen_linux.go b/listen_linux.go
deleted file mode 100644
index b1220ce4c60..00000000000
--- a/listen_linux.go
+++ /dev/null
@@ -1,34 +0,0 @@
-package caddy
-
-import (
-	"context"
-	"net"
-	"syscall"
-	"time"
-
-	"go.uber.org/zap"
-	"golang.org/x/sys/unix"
-)
-
-// ListenTimeout is the same as Listen, but with a configurable keep-alive timeout duration.
-func ListenTimeout(network, addr string, keepalivePeriod time.Duration) (net.Listener, error) {
-	// check to see if plugin provides listener
-	if ln, err := getListenerFromPlugin(network, addr); err != nil || ln != nil {
-		return ln, err
-	}
-
-	config := &net.ListenConfig{Control: reusePort, KeepAlive: keepalivePeriod}
-	return config.Listen(context.Background(), network, addr)
-}
-
-func reusePort(network, address string, conn syscall.RawConn) error {
-	return conn.Control(func(descriptor uintptr) {
-		if err := unix.SetsockoptInt(int(descriptor), unix.SOL_SOCKET, unix.SO_REUSEPORT, 1); err != nil {
-			Log().Error("setting SO_REUSEPORT",
-				zap.String("network", network),
-				zap.String("address", address),
-				zap.Uintptr("descriptor", descriptor),
-				zap.Error(err))
-		}
-	})
-}
diff --git a/listen_unix.go b/listen_unix.go
new file mode 100644
index 00000000000..f7b62794064
--- /dev/null
+++ b/listen_unix.go
@@ -0,0 +1,115 @@
+// Copyright 2015 Matthew Holt and The Caddy Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// TODO: Go 1.19 introduced the "unix" build tag. We have to support Go 1.18 until Go 1.20 is released.
+// When Go 1.19 is our minimum, remove this build tag, since "_unix" in the filename will do this.
+// (see also change needed in listen.go)
+//go:build aix || android || darwin || dragonfly || freebsd || hurd || illumos || ios || linux || netbsd || openbsd || solaris
+
+package caddy
+
+import (
+	"context"
+	"errors"
+	"io/fs"
+	"net"
+	"sync"
+	"syscall"
+	"time"
+
+	"go.uber.org/zap"
+	"golang.org/x/sys/unix"
+)
+
+// ListenTimeout is the same as Listen, but with a configurable keep-alive timeout duration.
+func ListenTimeout(network, addr string, keepalivePeriod time.Duration) (net.Listener, error) {
+	// check to see if plugin provides listener
+	if ln, err := getListenerFromPlugin(network, addr); err != nil || ln != nil {
+		return ln, err
+	}
+
+	socketKey := listenerKey(network, addr)
+	if isUnixNetwork(network) {
+		unixSocketsMu.Lock()
+		defer unixSocketsMu.Unlock()
+
+		socket, exists := unixSockets[socketKey]
+		if exists {
+			// make copy of file descriptor
+			socketFile, err := socket.File() // dup() deep down
+			if err != nil {
+				return nil, err
+			}
+
+			// use copy to make new listener
+			ln, err := net.FileListener(socketFile)
+			if err != nil {
+				return nil, err
+			}
+
+			// the old socket fd will likely be closed soon, so replace it in the map
+			unixSockets[socketKey] = ln.(*net.UnixListener)
+
+			return ln.(*net.UnixListener), nil
+		}
+
+		// from what I can tell after some quick research, it's quite common for programs to
+		// leave their socket file behind after they close, so the typical pattern is to
+		// unlink it before you bind to it -- this is often crucial if the last program using
+		// it was killed forcefully without a chance to clean up the socket, but there is a
+		// race, as the comment in net.UnixListener.close() explains... oh well?
+		if err := syscall.Unlink(addr); err != nil && !errors.Is(err, fs.ErrNotExist) {
+			return nil, err
+		}
+	}
+
+	config := &net.ListenConfig{Control: reusePort, KeepAlive: keepalivePeriod}
+
+	ln, err := config.Listen(context.Background(), network, addr)
+	if err != nil {
+		return nil, err
+	}
+
+	if uln, ok := ln.(*net.UnixListener); ok {
+		// TODO: ideally, we should unlink the socket once we know we're done using it
+		// (i.e. either on exit or a new config that doesn't use this socket; in UsagePool
+		// terms, when the reference count reaches 0), but given that we unlink existing
+		// socket before we create the new one anyway (see above), we don't necessarily
+		// need to clean up after ourselves; still, doing so would probably be more tidy
+		uln.SetUnlinkOnClose(false)
+		unixSockets[socketKey] = uln
+	}
+
+	return ln, nil
+}
+
+// reusePort sets SO_REUSEPORT. Ineffective for unix sockets.
+func reusePort(network, address string, conn syscall.RawConn) error {
+	return conn.Control(func(descriptor uintptr) {
+		if err := unix.SetsockoptInt(int(descriptor), unix.SOL_SOCKET, unix.SO_REUSEPORT, 1); err != nil {
+			Log().Error("setting SO_REUSEPORT",
+				zap.String("network", network),
+				zap.String("address", address),
+				zap.Uintptr("descriptor", descriptor),
+				zap.Error(err))
+		}
+	})
+}
+
+// unixSockets keeps track of the currently-active unix sockets
+// so we can transfer their FDs gracefully during reloads.
+var (
+	unixSockets   = make(map[string]*net.UnixListener)
+	unixSocketsMu sync.Mutex
+)
diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go
index 84b0b94165a..c9a55431493 100644
--- a/modules/caddyhttp/app.go
+++ b/modules/caddyhttp/app.go
@@ -409,7 +409,7 @@ func (app *App) Start() error {
 					ln = tls.NewListener(ln, tlsCfg)
 
 					// enable HTTP/3 if configured
-					if srv.protocol("h3") {
+					if srv.protocol("h3") && !listenAddr.IsUnixNetwork() {
 						app.logger.Info("enabling HTTP/3 listener", zap.String("addr", hostport))
 						if err := srv.serveHTTP3(hostport, tlsCfg); err != nil {
 							return err


Test patch: