From 6b4eca065416994ab3238f6e5d5c089d03213c8e Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sat, 31 Oct 2026 00:37:42 +0000 Subject: [PATCH] Warn if the admin socket directory lets other users use or replace the socket --- docs/config.md | 4 +++- server/server_admin.go | 4 ++++ server/server_admin_test.go | 31 +++++++++++++++++++++++++++++ server/server_admin_unix.go | 36 ++++++++++++++++++++++++++++++++++ server/server_admin_windows.go | 6 ++++++ 5 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 server/server_admin_unix.go create mode 100644 server/server_admin_windows.go diff --git a/docs/config.md b/docs/config.md index e6e4fce3..d562b1cb 100644 --- a/docs/config.md +++ b/docs/config.md @@ -934,7 +934,9 @@ warning: ntfy server not reachable at /run/ntfy/admin.sock, writing to the datab ``` To change the path, set `listen-admin-unix` in the `server.yml` file; the commands read it from there, too. To disable the -socket, set `listen-admin-unix: "-"`. The admin socket is not available on Windows. +socket, set `listen-admin-unix: "-"`. The admin socket is not available on Windows. If you change the path, pick a +directory that only the ntfy user can write to; otherwise the server logs a warning at startup, since other users may be +able to use or replace the socket. ### Example: Private instance The easiest way to configure a private instance is to set `auth-default-access` to `deny-all` in the `server.yml`, diff --git a/server/server_admin.go b/server/server_admin.go index 4dce5841..051122a7 100644 --- a/server/server_admin.go +++ b/server/server_admin.go @@ -503,6 +503,10 @@ func (s *Server) listenAdminSocket() (net.Listener, error) { if err := os.MkdirAll(filepath.Dir(path), adminSocketDirMode); err != nil { return nil, err } + if reason := insecureAdminSocketDir(filepath.Dir(path)); reason != "" { + // Not fatal: the path is the admin's choice + log.Tag(tagStartup).Warn("Admin socket directory %s %s; other users may be able to use or replace the admin socket", filepath.Dir(path), reason) + } if conn, err := net.DialTimeout("unix", path, adminSocketDialTimeout); err == nil { conn.Close() return nil, fmt.Errorf("another server is listening on %s", path) diff --git a/server/server_admin_test.go b/server/server_admin_test.go index 5fae555f..cf8c7352 100644 --- a/server/server_admin_test.go +++ b/server/server_admin_test.go @@ -7,12 +7,14 @@ import ( "errors" "fmt" "io" + "io/fs" "net" "net/http" "net/http/httptest" "net/netip" "os" "path/filepath" + "runtime" "strings" "sync" "sync/atomic" @@ -1389,6 +1391,35 @@ func TestAdminSocket_UnwritableDirectory(t *testing.T) { require.False(t, adminSocketListening(s)) } +func TestAdminSocket_InsecureDirectoryWarning(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("unix file modes") + } + for _, tt := range []struct { + mode fs.FileMode + warn bool + }{ + {0750, false}, + {0777, true}, // Anyone can replace the socket + {0777 | fs.ModeSticky, false}, // Like /tmp: only the owner can remove or replace it + } { + dir := shortTempDir(t) + require.Nil(t, os.Chmod(dir, tt.mode)) + conf := newTestConfigWithAuthFile(t, "") + conf.ListenAdminUnix = filepath.Join(dir, "admin.sock") + s := newTestServer(t, conf) + output := captureLog(t, func() { + runTestServer(t, s) + waitFor(t, func() bool { return adminSocketListening(s) }) // Warned before it listens + }) + if tt.warn { + require.Contains(t, output, "Admin socket directory "+dir+" is writable by all users", tt.mode) + } else { + require.NotContains(t, output, "Admin socket directory", tt.mode) + } + } +} + func TestAdminSocket_DisabledWithoutPathOrAuth(t *testing.T) { // No path: no listener, no warning s := newTestServer(t, newTestConfigWithAuthFile(t, "")) diff --git a/server/server_admin_unix.go b/server/server_admin_unix.go new file mode 100644 index 00000000..5710fb12 --- /dev/null +++ b/server/server_admin_unix.go @@ -0,0 +1,36 @@ +//go:build !windows + +package server + +import ( + "io/fs" + "os" + "syscall" +) + +// insecureAdminSocketDir returns why other users could use or replace an admin socket in dir, or +// "" if they cannot. A directory like /tmp (world-writable, but sticky) is fine: only the owner can +// remove or rename the socket there. +func insecureAdminSocketDir(dir string) string { + info, err := os.Stat(dir) + if err != nil { + return "" + } + st, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return "" + } + mode, sticky := info.Mode(), info.Mode()&fs.ModeSticky != 0 + ownGroup := int(st.Gid) == os.Getgid() + switch { + case int(st.Uid) != os.Getuid() && st.Uid != 0: + return "is owned by another user" + case mode&0002 != 0 && !sticky: + return "is writable by all users" + case mode&0020 != 0 && !ownGroup && !sticky: + return "is writable by another group" + case mode&fs.ModeSetgid != 0 && !ownGroup: + return "has the setgid bit for another group, which the socket inherits" + } + return "" +} diff --git a/server/server_admin_windows.go b/server/server_admin_windows.go new file mode 100644 index 00000000..dba7ccbe --- /dev/null +++ b/server/server_admin_windows.go @@ -0,0 +1,6 @@ +package server + +// insecureAdminSocketDir is not implemented on Windows, where file modes do not apply +func insecureAdminSocketDir(dir string) string { + return "" +}