Skip to content

Commit

Permalink
core: Refactor and improve listener logic (#5089)
Browse files Browse the repository at this point in the history
* core: Refactor, improve listener logic

Deprecate:
- caddy.Listen
- caddy.ListenTimeout
- caddy.ListenPacket

Prefer caddy.NetworkAddress.Listen() instead.

Change:
- caddy.ListenQUIC (hopefully to remove later)
- caddy.ListenerFunc signature (add context and ListenConfig)

- Don't emit Alt-Svc header advertising h3 over HTTP/3

- Use quic.ListenEarly instead of quic.ListenEarlyAddr; this gives us
more flexibility (e.g. possibility of HTTP/3 over UDS) but also
introduces a new issue:
quic-go/quic-go#3560 (comment)

- Unlink unix socket before and after use

* Appease the linter

* Keep ListenAll
  • Loading branch information
mholt committed Sep 28, 2022
1 parent d055692 commit e3e8aab
Show file tree
Hide file tree
Showing 7 changed files with 562 additions and 292 deletions.
7 changes: 4 additions & 3 deletions admin.go
Expand Up @@ -382,7 +382,7 @@ func replaceLocalAdminServer(cfg *Config) error {

handler := cfg.Admin.newAdminHandler(addr, false)

ln, err := Listen(addr.Network, addr.JoinHostPort(0))
ln, err := addr.Listen(context.TODO(), 0, net.ListenConfig{})
if err != nil {
return err
}
Expand All @@ -403,7 +403,7 @@ func replaceLocalAdminServer(cfg *Config) error {
serverMu.Lock()
server := localAdminServer
serverMu.Unlock()
if err := server.Serve(ln); !errors.Is(err, http.ErrServerClosed) {
if err := server.Serve(ln.(net.Listener)); !errors.Is(err, http.ErrServerClosed) {
adminLogger.Error("admin server shutdown for unknown reason", zap.Error(err))
}
}()
Expand Down Expand Up @@ -549,10 +549,11 @@ func replaceRemoteAdminServer(ctx Context, cfg *Config) error {
serverMu.Unlock()

// start listener
ln, err := Listen(addr.Network, addr.JoinHostPort(0))
lnAny, err := addr.Listen(ctx, 0, net.ListenConfig{})
if err != nil {
return err
}
ln := lnAny.(net.Listener)
ln = tls.NewListener(ln, tlsConfig)

go func() {
Expand Down
26 changes: 7 additions & 19 deletions listen.go
Expand Up @@ -20,7 +20,7 @@
package caddy

import (
"fmt"
"context"
"net"
"sync"
"sync/atomic"
Expand All @@ -29,30 +29,22 @@ import (
"go.uber.org/zap"
)

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
}

lnKey := listenerKey(network, addr)
func reuseUnixSocket(network, addr string) (any, error) {
return nil, nil
}

func listenTCPOrUnix(ctx context.Context, lnKey string, network, address string, config net.ListenConfig) (net.Listener, error) {
sharedLn, _, err := listenerPool.LoadOrNew(lnKey, func() (Destructor, error) {
ln, err := net.Listen(network, addr)
ln, err := config.Listen(ctx, network, address)
if err != nil {
// https://github.com/caddyserver/caddy/pull/4534
if isUnixNetwork(network) && isListenBindAddressAlreadyInUseError(err) {
return nil, fmt.Errorf("%w: this can happen if Caddy was forcefully killed", err)
}
return nil, err
}
return &sharedListener{Listener: ln, key: lnKey}, nil
})
if err != nil {
return nil, err
}

return &fakeCloseListener{sharedListener: sharedLn.(*sharedListener), keepAlivePeriod: keepAlivePeriod}, nil
return &fakeCloseListener{sharedListener: sharedLn.(*sharedListener), keepAlivePeriod: config.KeepAlive}, nil
}

// fakeCloseListener is a private wrapper over a listener that
Expand Down Expand Up @@ -156,8 +148,6 @@ func (sl *sharedListener) clearDeadline() error {
switch ln := sl.Listener.(type) {
case *net.TCPListener:
err = ln.SetDeadline(time.Time{})
case *net.UnixListener:
err = ln.SetDeadline(time.Time{})
}
sl.deadline = false
}
Expand All @@ -173,8 +163,6 @@ func (sl *sharedListener) setDeadline() error {
switch ln := sl.Listener.(type) {
case *net.TCPListener:
err = ln.SetDeadline(timeInPast)
case *net.UnixListener:
err = ln.SetDeadline(timeInPast)
}
sl.deadline = true
}
Expand Down
103 changes: 53 additions & 50 deletions listen_unix.go
Expand Up @@ -24,78 +24,88 @@ import (
"errors"
"io/fs"
"net"
"sync"
"sync/atomic"
"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
// reuseUnixSocket copies and reuses the unix domain socket (UDS) if we already
// have it open; if not, unlink it so we can have it. No-op if not a unix network.
func reuseUnixSocket(network, addr string) (any, error) {
if !isUnixNetwork(network) {
return nil, nil
}

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
socket, exists := unixSockets[socketKey]
if exists {
// make copy of file descriptor
socketFile, err := socket.File() // does dup() deep down
if err != nil {
return nil, err
}

// use copied fd to make new Listener or PacketConn, then replace
// it in the map so that future copies always come from the most
// recent fd (as the previous ones will be closed, and we'd get
// "use of closed network connection" errors) -- note that we
// preserve the *pointer* to the counter (not just the value) so
// that all socket wrappers will refer to the same value
switch unixSocket := socket.(type) {
case *unixListener:
ln, err := net.FileListener(socketFile)
if err != nil {
return nil, err
}
atomic.AddInt32(unixSocket.count, 1)
unixSockets[socketKey] = &unixListener{ln.(*net.UnixListener), socketKey, unixSocket.count}

// use copy to make new listener
ln, err := net.FileListener(socketFile)
case *unixConn:
pc, err := net.FilePacketConn(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
atomic.AddInt32(unixSocket.count, 1)
unixSockets[socketKey] = &unixConn{pc.(*net.UnixConn), addr, socketKey, unixSocket.count}
}

// 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
}
return unixSockets[socketKey], nil
}

config := &net.ListenConfig{Control: reusePort, KeepAlive: keepalivePeriod}

ln, err := config.Listen(context.Background(), network, addr)
if err != 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, I guess?
if err := syscall.Unlink(addr); err != nil && !errors.Is(err, fs.ErrNotExist) {
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 nil, nil
}

return ln, nil
func listenTCPOrUnix(ctx context.Context, lnKey string, network, address string, config net.ListenConfig) (net.Listener, error) {
// wrap any Control function set by the user so we can also add our reusePort control without clobbering theirs
oldControl := config.Control
config.Control = func(network, address string, c syscall.RawConn) error {
if oldControl != nil {
if err := oldControl(network, address, c); err != nil {
return err
}
}
return reusePort(network, address, c)
}
return config.Listen(ctx, network, address)
}

// reusePort sets SO_REUSEPORT. Ineffective for unix sockets.
func reusePort(network, address string, conn syscall.RawConn) error {
if isUnixNetwork(network) {
return nil
}
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",
Expand All @@ -106,10 +116,3 @@ func reusePort(network, address string, conn syscall.RawConn) error {
}
})
}

// 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
)

8 comments on commit e3e8aab

@lxhao61
Copy link

@lxhao61 lxhao61 commented on e3e8aab Sep 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a bug in this update, local loopback listen conflicts with enabling HTTP/3, and the two cannot coexist.

@mholt
Copy link
Member Author

@mholt mholt commented on e3e8aab Sep 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lxhao61 Can you elaborate?

@lxhao61
Copy link

@lxhao61 lxhao61 commented on e3e8aab Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example configuration:

{
#.......
	servers 127.0.0.1:8443 {
		listener_wrappers {
			proxy_protocol
			tls
		}
		protocols h1 h2 h3
	}
}
#.......

This configuration worked fine before, but after compiling the file with this updated source code, an error occurred, as shown below:
root@yisu-6310a6cff2a61:# journalctl -u caddy --no-pager
-- Logs begin at Thu 2022-09-29 07:59:41 CST, end at Thu 2022-09-29 08:00:29 CST. --
Sep 29 07:59:43 yisu-6310a6cff2a61 systemd[1]: Starting Caddy...
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: caddy.HomeDir=/nonexistent
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: caddy.AppDataDir=/nonexistent/.local/share/caddy
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: caddy.AppConfigDir=/nonexistent/.config/caddy
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: caddy.ConfigAutosavePath=/nonexistent/.config/caddy/autosave.json
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: caddy.Version=v2.6.2-0.20220928212245-897a38958cb7 h1:unYePvmZmnnvfzT/KRNbJz4djx6N4w67vCxLtSvl5vM=
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: runtime.GOOS=linux
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: runtime.GOARCH=amd64
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: runtime.Compiler=gc
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: runtime.NumCPU=1
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: runtime.GOMAXPROCS=1
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: runtime.Version=go1.19.1
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: os.Getwd=/
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: LANG=en_US.UTF-8
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: LANGUAGE=en_US:en
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: NOTIFY_SOCKET=/run/systemd/notify
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: HOME=/nonexistent
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: LOGNAME=nobody
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: USER=nobody
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: INVOCATION_ID=6bfdcd3f5af84f598d1f724d06d6cac3
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: JOURNAL_STREAM=8:15590
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: {"level":"info","ts":1664409583.9663053,"msg":"using provided configuration","config_file":"/usr/local/etc/caddy/Caddyfile","config_adapter":""}
Sep 29 07:59:43 yisu-6310a6cff2a61 caddy[696]: {"level":"info","ts":1664409583.9736228,"msg":"redirected default logger","from":"stderr","to":"/var/log/caddy/access.log"}
Sep 29 07:59:44 yisu-6310a6cff2a61 systemd[1]: Started Caddy.
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: panic: interface conversion: interface {} is *caddy.sharedPacketConn, not *caddy.sharedQuicListener
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: goroutine 1 [running]:
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2.ListenQUIC({0x7f8e9095a6e8, 0xc0005d5e80}, 0xc000345980, 0xc00015ec80)
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2@v2.6.2-0.20220928212245-897a38958cb7/listeners.go:474 +0x2ae
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2/modules/caddyhttp.(*Server).serveHTTP3(0xc00015ec80, {{0x1b2a9f9, 0x3}, {0xc000653b00, 0x9}, 0x20fb, 0x20fb}, 0xc000345980)
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2@v2.6.2-0.20220928212245-897a38958cb7/modules/caddyhttp/server.go:519 +0x1ca
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2/modules/caddyhttp.(*App).Start(0xc000162500)
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2@v2.6.2-0.20220928212245-897a38958cb7/modules/caddyhttp/app.go:432 +0xfc5
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2.run.func4(0xc00003ea50)
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2@v2.6.2-0.20220928212245-897a38958cb7/caddy.go:502 +0x122
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2.run(0xc0006538b0?, 0x1)
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2@v2.6.2-0.20220928212245-897a38958cb7/caddy.go:518 +0x5e5
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2.unsyncedDecodeAndRun({0xc000661000, 0xdd2, 0x1000}, 0x1)
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2@v2.6.2-0.20220928212245-897a38958cb7/caddy.go:337 +0x145
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2.changeConfig({0x1b2c7e3, 0x4}, {0x1b39886, 0x7}, {0xc000660000, 0xdd2, 0x1000}, {0x0, 0x0}, 0x1)
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2@v2.6.2-0.20220928212245-897a38958cb7/caddy.go:228 +0x79a
Sep 29 07:59:44 yisu-6310a6cff2a61 caddy[696]: github.com/caddyserver/caddy/v2.Load({0xc000660000, 0xdd2, 0x1000}, 0x0?)
Sep 29 07:59:44 yisu-6310a6cff2a61 systemd[1]: caddy.service: Main process exited, code=exited, status=2/INVALIDARGUMENT
Sep 29 07:59:44 yisu-6310a6cff2a61 systemd[1]: caddy.service: Unit entered failed state.
Sep 29 07:59:44 yisu-6310a6cff2a61 systemd[1]: caddy.service: Failed with result 'exit-code'.
root@yisu-6310a6cff2a61:
#

It works fine after turning off HTTP/3 or canceling the local listen.

@mholt
Copy link
Member Author

@mholt mholt commented on e3e8aab Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the full config though?

Also you don't need to specify protocols h1 h2 h3 -- that's the default.

PS. Thank you for testing it ❤️ (professional heart)

@lxhao61
Copy link

@lxhao61 lxhao61 commented on e3e8aab Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the full config though?

Also you don't need to specify protocols h1 h2 h3 -- that's the default.

PS. Thank you for testing it heart (professional heart)

full config:

{
	order forward_proxy before file_server
	admin off
	log {
		output file /var/log/caddy/access.log
		level ERROR
	}
	storage file_system {
		root /home/tls
	}

	auto_https off
	default_sni *.xx.yy
	cert_issuer zerossl
	acme_dns duckdns b8a33cc1-1111-2222-3333-b049bf07988f {
		override_domain zz.duckdns.org
	}

	servers unix//dev/shm/h1h2c.sock {
		listener_wrappers {
			proxy_protocol
		}
		protocols h1 h2c
	}

	servers 127.0.0.1:8443 {
		listener_wrappers {
			proxy_protocol
			tls
		}
	}
}

:80 {
	redir https://{host}{uri} permanent
}

:88 {
	bind unix//dev/shm/h1h2c.sock

	file_server {
		root /var/www/html
	}
}

:8443, *.xx.yy:8443 {
	bind 127.0.0.1

	forward_proxy {
		basic_auth user pass
		hide_ip
		hide_via
		probe_resistance
	}

	@host2 {
		host wh.xx.yy ch.xx.yy
	}
	file_server @host2 {
		root /var/www/html
	}
}

You're welcome, and thank you for your dedication.

@mholt
Copy link
Member Author

@mholt mholt commented on e3e8aab Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm able to reproduce it, thanks. Working on a fix.

@mholt
Copy link
Member Author

@mholt mholt commented on e3e8aab Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lxhao61 Should be fixed with ab720fb -- please give it another try!

@lxhao61
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lxhao61 Should be fixed with ab720fb -- please give it another try!

Tested, problem solved. Thanks!

Please sign in to comment.