Refuse to serve from a directory somebody else could change

The socket's mode and group say who may connect; the directory is what
makes the path they connect to the daemon's. Whoever can write it can
put a socket of their own there and answer the clients in nebula's
place, and the clients, now including every member of the group, trust
the path. The directory was not checked at all.

- checkSocketDir, at bind: the socket's directory must be a directory,
  not a link, owned by the user nebula runs as or by root, and writable
  by its owner alone; refused with the owner or mode named and the fix
- catches a runtime directory a service manager made for another user
  and reused by a root daemon, and /var/run/nebula created ahead of a
  root daemon by a member of group daemon, which darwin lets write
  /var/run
- only the socket's own directory, not its parents: /var/run is group
  writable on darwin by design, and the parents are the operator's to
  trust, as for every file nebula reads

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

CHANGELOG.md

@@ -16,7 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   named pipe, and is limited to the user nebula runs as: the socket is 0600 and the daemon checks the peer's
   credentials on every connection, so `nebula ctl` is run as that user, or with sudo when nebula runs as root.
   The socket is `/var/run/nebula/ctl.sock` unless `local_api.control.socket` names another absolute path,
-  which a daemon that does not run as root, or a second daemon on one host, does. `nebula ctl` talks to the
+  which a daemon that does not run as root, or a second daemon on one host, does. The directory holding the
+  socket must be owned by nebula's user or by root and be writable by its owner alone; one that is not is
+  refused, with the owner or mode named, since whoever could write it could put their own socket at the path. `nebula ctl` talks to the
   socket named with `-socket`, or the one named in the config given with `-config` or found next to the
   executable, else the default; both flags may come before or after the command, and
   `nebula -config <path> ctl <command>` carries the daemon's config over. It never searches for a socket. On

examples/config.yml

@@ -245,7 +245,8 @@ punchy:
   # the BSDs; on Windows it is a named pipe under \\.\pipe\ProtectedPrefix\Administrators.
   #control:
     # The absolute path where the control socket should live, or a full pipe name on
-    # Windows. The last directory is created if missing.
+    # Windows. The last directory is created if missing, and must be owned by root or by
+    # the user running nebula. It must not be writable by anyone else.
     #socket: /var/run/nebula/ctl.sock
     # A group (or account/group/SID on Windows) that should be allowed to access the
     # socket.

localapi/platform_unix.go

@@ -71,10 +71,10 @@ func listen(path string, acc Access) (net.Listener, error) {
 	// mistake to report, not a tree to build. A directory made here is remembered, so
 	// that removeSocket can take it away again once nothing is left in it.
 	//
-	// 0755 gives nothing away: the socket's own mode is the access control. What it gives is a directory others
-	// can look into, which is what ServedByRootFor needs. Mkdir applies the umask, so
-	// the mode is pinned afterwards, as the socket's is; only on a directory this call
-	// created.
+	// 0755 gives nothing away: the socket's own mode is the access control. What it
+	// gives is a directory others can look into, which is what a client that is not the
+	// daemon's user needs to reach the socket. Mkdir applies the umask, so the mode is
+	// pinned afterwards, as the socket's is; only on a directory this call created.
 	switch err := os.Mkdir(dir, 0755); {
 	case err == nil:
 		if err := os.Chmod(dir, 0755); err != nil {
@@ -85,6 +85,12 @@ func listen(path string, acc Access) (net.Listener, error) {
 		return nil, fmt.Errorf("could not create %s: %w", dir, err)
 	}
 
+	// Whether it was made here or came with the environment, the directory has to be
+	// one nobody else could change; see checkSocketDir.
+	if err := checkSocketDir(dir); err != nil {
+		return nil, err
+	}
+
 	if err := clearStale(path); err != nil {
 		return nil, err
 	}
@@ -134,6 +140,58 @@ func applyAccess(path string, acc Access) error {
 	return os.Chmod(path, acc.Mode.Perm())
 }
 
+// checkSocketDir refuses to serve from a directory that somebody other than the user
+// nebula runs as, or root, could change. The socket's mode and group say who may connect
+// to it; what makes the path they connect to the daemon's is the directory, since
+// whoever can write it can put a socket of their own at the path and answer the clients
+// in the daemon's place. So the directory must be owned by the daemon's user or by root
+// and writable by its owner alone; a sticky bit is not an exception. A runtime directory
+// a service manager made for another user, or /var/run/nebula created ahead of a root
+// daemon by a member of group daemon, where darwin lets that group write /var/run, are
+// both refused with the owner named.
+//
+// A link where the directory should be is judged twice: whoever owns the link decides
+// where the path leads, so the link must be ours or root's too, and the directory it
+// resolves to is then held to the rule above. /var/run is such a link on linux, root's,
+// to /run, and a socket named directly under it is served; a link somebody else planted
+// is refused as theirs.
+//
+// Only this directory is checked, not the ones above it. /var/run itself is group
+// writable on darwin by design, and the parents are the operator's to trust, as they are
+// for every file nebula reads. The check is made at bind, so a directory loosened while
+// the daemon runs is noticed at the next reload.
+func checkSocketDir(dir string) error {
+	fi, err := os.Lstat(dir)
+	if err != nil {
+		return err
+	}
+	if fi.Mode()&os.ModeSymlink != 0 {
+		if uid, ok := owner(fi); ok && uid != currentUID() && uid != 0 {
+			return fmt.Errorf("%s is a symlink owned by %s, not by the user nebula runs as or by root, and its owner decides where the path leads; name a socket path that does not go through it", dir, describeUID(uid))
+		}
+		if fi, err = os.Stat(dir); err != nil {
+			return err
+		}
+	}
+	if !fi.IsDir() {
+		return fmt.Errorf("%s is not a directory", dir)
+	}
+
+	if uid, ok := owner(fi); ok && uid != currentUID() && uid != 0 {
+		return fmt.Errorf("%s is owned by %s, not by the user nebula runs as or by root, and its owner could replace the socket; make it ours or root's, or name a socket path in a directory that is", dir, describeUID(uid))
+	}
+
+	if perm := fi.Mode().Perm(); perm&0022 != 0 {
+		who := "its group"
+		if perm&0002 != 0 {
+			who = "everyone"
+		}
+		return fmt.Errorf("%s is writable by %s (mode %04o), who could replace the socket; make it writable by its owner alone, or name a socket path in a directory that is", dir, who, perm)
+	}
+
+	return nil
+}
+
 // owner returns the uid that owns the file fi describes.
 func owner(fi os.FileInfo) (uint32, bool) {
 	st, ok := fi.Sys().(*syscall.Stat_t)

localapi/platform_unix_test.go

@@ -313,3 +313,72 @@ func TestMadeDirStaysRememberedUntilItIsGone(t *testing.T) {
 	d.remove(gone)
 	assert.False(t, d.made[gone])
 }
+
+// TestListenRefusesADirectoryOthersCanWrite: whoever can write the socket's directory
+// can put a socket of their own at the path and answer the clients, who trust the path;
+// so a directory writable by its group or by everyone is refused, and nothing is bound.
+func TestListenRefusesADirectoryOthersCanWrite(t *testing.T) {
+	for _, mode := range []os.FileMode{0775, 0757, 0777, 01777} {
+		dir := filepath.Join(privateBase(t), "loose")
+		require.NoError(t, os.Mkdir(dir, mode))
+		require.NoError(t, os.Chmod(dir, mode))
+
+		_, err := listen(filepath.Join(dir, "s.sock"), ownerOnly())
+		require.ErrorContains(t, err, "writable by", "mode %04o", mode)
+		assert.NoFileExists(t, filepath.Join(dir, "s.sock"))
+	}
+}
+
+// TestListenRefusesADirectoryOwnedBySomebodyElse: a directory that is neither ours nor
+// root's is refused, since its owner could replace the socket. The test's own directory
+// stands in for somebody else's by the process pretending to be another user.
+func TestListenRefusesADirectoryOwnedBySomebodyElse(t *testing.T) {
+	me := currentUID()
+	if me == 0 {
+		t.Skip("root's directories are accepted whoever is asking")
+	}
+	dir := filepath.Join(privateBase(t), "theirs")
+	require.NoError(t, os.Mkdir(dir, 0755))
+
+	asUID(t, me+1)
+	_, err := listen(filepath.Join(dir, "s.sock"), ownerOnly())
+	require.ErrorContains(t, err, "is owned by")
+	assert.NoFileExists(t, filepath.Join(dir, "s.sock"))
+
+	asUID(t, me)
+	ln, err := listen(filepath.Join(dir, "s.sock"), ownerOnly())
+	require.NoError(t, err, "the same directory, seen as our own, serves")
+	release(ln, filepath.Join(dir, "s.sock"))
+}
+
+// TestListenThroughASymlinkedDirectory: a link where the directory should be is judged
+// by who owns it, since they decide where the path leads, and then by the directory it
+// resolves to. Our own link to a proper directory serves, as /var/run on linux is root's
+// link to /run; somebody else's link is refused, and so is a link of ours to a directory
+// others can write.
+func TestListenThroughASymlinkedDirectory(t *testing.T) {
+	me := currentUID()
+	if me == 0 {
+		t.Skip("root's links are accepted whoever is asking")
+	}
+	base := privateBase(t)
+	real := filepath.Join(base, "real")
+	require.NoError(t, os.Mkdir(real, 0755))
+	link := filepath.Join(base, "link")
+	require.NoError(t, os.Symlink(real, link))
+	path := filepath.Join(link, "s.sock")
+
+	ln, err := listen(path, ownerOnly())
+	require.NoError(t, err, "our link to our directory")
+	release(ln, path)
+	assert.NoFileExists(t, filepath.Join(real, "s.sock"))
+
+	asUID(t, me+1)
+	_, err = listen(path, ownerOnly())
+	require.ErrorContains(t, err, "is a symlink owned by")
+	asUID(t, me)
+
+	require.NoError(t, os.Chmod(real, 0777))
+	_, err = listen(path, ownerOnly())
+	require.ErrorContains(t, err, "writable by everyone")
+}