Pass the operating system's error through when nebula ctl cannot reach the daemon

A failed connect was followed by advice that guessed at the operator's
setup: sudo, a stale path, a different config. The guess could be wrong,
and when the config itself could not be read the guess never came, so
the common case, a root daemon with a 0600 config, got the least help.
The operating system's error says what happened: permission denied, no
such file. It is printed as it came, in both places, and nothing more.

- the dial failure prints the path and the error; the four advice
  branches and the config-name plumbing that fed one of them go
- a config that could not be read wraps the error rather than reprinting
  it, so a caller can still see which error it was
- the usage line says who may run it: the daemon's user, or a member of
  the configured group

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

cmd/internal/ctl/ctl.go

@@ -89,7 +89,7 @@ func Run(args []string) int {
 	fs.Usage = func() {
 		fmt.Fprintf(os.Stderr, "Usage: nebula ctl [flags] <command> [command flags]\n\n")
 		fmt.Fprintf(os.Stderr, "Inspects and controls a nebula already running on this machine.\n")
-		fmt.Fprintf(os.Stderr, "Run it as the user who ran nebula; nobody else can connect.\n\n")
+		fmt.Fprintf(os.Stderr, "Run it as the user who ran nebula, or as a member of the group %s names.\n\n", localapi.CtlGroupKey)
 		fmt.Fprintf(os.Stderr, "Flags, which may come before or after the command:\n")
 		fs.PrintDefaults()
 		fmt.Fprintf(os.Stderr, "\nRun `nebula ctl help` for the available commands.\n")
@@ -109,7 +109,7 @@ func Run(args []string) int {
 		cmdArgs = []string{"help"}
 	}
 
-	path, _, overrideIn, err := resolveSocket(*socketPath, *configPath)
+	path, err := resolveSocket(*socketPath, *configPath)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "%s\n", err)
 		return 1
@@ -122,9 +122,8 @@ func Run(args []string) int {
 	// while the wire protocol stays a named-parameter API rather than a shipped argv.
 	schema, err := client.Describe()
 	if err != nil {
-		// A daemon that answered was reached; what it said, or that it could not be read,
-		// is the whole message, and the advice below, which is about finding the daemon,
-		// would send the operator after the wrong fix.
+		// A daemon that answered was reached, and what it said, or that it could not be
+		// read, is the whole message.
 		var refused *localapi.DaemonError
 		var unread *localapi.ReplyError
 		if errors.As(err, &refused) || errors.As(err, &unread) {
@@ -132,24 +131,10 @@ func Run(args []string) int {
 			return 1
 		}
 
+		// Otherwise the operating system said why, and it is passed on as it came: a
+		// permission denied or a missing socket says more about what to do than a
+		// guess at the operator's setup would.
 		fmt.Fprintf(os.Stderr, "nebula: could not reach the control socket at %s: %s\n", path, err)
-		switch {
-		case errors.Is(err, os.ErrPermission):
-			// The kernel refused the connect: the socket is another user's. Nebula runs as
-			// root for the tun device and this did not, as a rule.
-			fmt.Fprintf(os.Stderr, "The socket is not yours to connect to. Run nebula ctl as the user who ran nebula, or with sudo when nebula runs as root.\n")
-		case *socketPath != "":
-			// The operator named the path; there is nothing to add.
-		case overrideIn != "":
-			// When the config names the path, the usual way the two disagree is time: the
-			// key was edited and the daemon has not reloaded yet, which is exactly what
-			// `nebula ctl reload` against that config runs into.
-			fmt.Fprintf(os.Stderr, "The path is %s in %s. A daemon that has not reloaded since that key changed is still serving the old path: pass it with -socket <old path>, or reload the daemon another way, such as SIGHUP.\n", localapi.CtlSocketKey, overrideIn)
-		default:
-			// The path was computed, and the daemon computes its own from its environment.
-			// Failing all else, the daemon's log says where its socket is.
-			fmt.Fprintf(os.Stderr, "If nebula runs as root, run nebula ctl with sudo. If it runs with another config, pass -config <path>. The daemon logs its socket path at startup; pass it with -socket <path>.\n")
-		}
 		return 1
 	}
 
@@ -370,12 +355,9 @@ func splitLine(fs *flag.FlagSet, args []string) (own, cmd []string) {
 // An explicit flag wins. Otherwise the config is read, the one the operator named or the
 // one found next to the executable, and the path is the socket that config implies: its
 // local_api.control.socket if set, else the same derivation the daemon applies.
-// configPath is the config that was read, "" when none was; overrideIn is that config
-// when its key set the path, and "" when the path is the default or was named on the command
-// line.
-func resolveSocket(socketFlag, configFlag string) (path, configPath, overrideIn string, err error) {
+func resolveSocket(socketFlag, configFlag string) (string, error) {
 	if socketFlag != "" {
-		return socketFlag, "", "", nil
+		return socketFlag, nil
 	}
 
 	return pathFromConfig(configFlag)
@@ -383,26 +365,24 @@ func resolveSocket(socketFlag, configFlag string) (path, configPath, overrideIn
 
 // pathFromConfig returns the socket path the daemon would be serving: the one
 // local_api.control.socket names in the config, so an operator who moved the socket does
-// not have to repeat themselves on every invocation, else the default. configPath is the
-// config that was read, "" when none was, and overrideIn is the same path when the key
-// is what answered.
+// not have to repeat themselves on every invocation, else the default.
 //
 // A config the operator did not name is a guess: config.DefaultPath looks next to the
 // executable, and a package that installs a default config there while systemd starts
 // the daemon with another will make it the wrong guess. It is still the only answer
 // that involves no searching. A guess that cannot be read is not an error, it is no
-// guess: a user the daemon's socket admits need not be able to read the daemon's
-// config, and is answered by the default path, where the daemon serves unless that
-// config moved it. A config the operator named is theirs to make readable.
-func pathFromConfig(configFlag string) (path, configPath, overrideIn string, err error) {
+// guess: a member of the socket's group cannot read a root daemon's config, and is
+// answered by the default path, where the daemon serves unless that config moved it.
+// A config the operator named is theirs to make readable.
+func pathFromConfig(configFlag string) (string, error) {
 	return socketFromConfigFile(configFlag, configFlag != "")
 }
 
-func socketFromConfigFile(configPath string, named bool) (path, readConfig, overrideIn string, err error) {
+func socketFromConfigFile(configPath string, named bool) (string, error) {
 	if !named {
 		p, err := config.DefaultPath()
 		if err != nil {
-			return localapi.DefaultSocketPath(), "", "", nil
+			return localapi.DefaultSocketPath(), nil
 		}
 		configPath = p
 	}
@@ -413,26 +393,25 @@ func socketFromConfigFile(configPath string, named bool) (path, readConfig, over
 	c := config.NewC(slog.New(slog.DiscardHandler))
 	if err := c.Load(configPath); err != nil {
 		if !named {
-			return localapi.DefaultSocketPath(), "", "", nil
+			return localapi.DefaultSocketPath(), nil
 		}
-		return "", "", "", fmt.Errorf("nebula: could not read %s: %s", configPath, err)
+		// The operating system's error, as it came: permission denied on a root daemon's
+		// config says what to do.
+		return "", fmt.Errorf("nebula: could not read %s: %w", configPath, err)
 	}
 
 	// A daemon running this config serves no socket; dialing the path would only say
 	// that nothing is there.
 	if !localapi.ControlEnabled(c) {
-		return "", "", "", fmt.Errorf("nebula: the control socket is disabled in %s by %s", configPath, localapi.CtlEnabledKey)
+		return "", fmt.Errorf("nebula: the control socket is disabled in %s by %s", configPath, localapi.CtlEnabledKey)
 	}
 
 	// The same call the daemon makes, so the two agree on the path and on what makes a
 	// path unusable.
 	p, err := localapi.SocketPathFromConfig(c)
 	if err != nil {
-		return "", "", "", fmt.Errorf("nebula: %s", err)
+		return "", fmt.Errorf("nebula: %w", err)
 	}
 
-	if c.GetString(localapi.CtlSocketKey, "") != "" {
-		overrideIn = configPath
-	}
-	return p, configPath, overrideIn, nil
+	return p, nil
 }

cmd/internal/ctl/ctl_test.go

@@ -20,22 +20,22 @@ import (
 func TestResolveSocketReportsAnUnreadableConfig(t *testing.T) {
 	missing := filepath.Join(t.TempDir(), "typo.yml")
 
-	_, _, _, err := resolveSocket("", missing)
+	_, err := resolveSocket("", missing)
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), missing)
 }
 
 // TestResolveSocketGuessedConfigNeedNotBeReadable: a config nobody named is a guess, and
-// one that cannot be read, a root daemon's config read by another user the socket
-// admits, means the default path rather than an error.
+// one that cannot be read, a root daemon's config read by a member of the socket's
+// group, means the default path rather than an error.
 func TestResolveSocketGuessedConfigNeedNotBeReadable(t *testing.T) {
 	missing := filepath.Join(t.TempDir(), "config.yml")
 
-	p, _, _, err := socketFromConfigFile(missing, false)
+	p, err := socketFromConfigFile(missing, false)
 	require.NoError(t, err)
 	assert.Equal(t, localapi.DefaultSocketPath(), p)
 
-	_, _, _, err = socketFromConfigFile(missing, true)
+	_, err = socketFromConfigFile(missing, true)
 	require.ErrorContains(t, err, missing)
 }
 
@@ -53,7 +53,7 @@ func TestResolveSocketTrustsANamedConfig(t *testing.T) {
 	cfg := filepath.Join(t.TempDir(), "config.yml")
 	require.NoError(t, os.WriteFile(cfg, []byte(namedSocketConfig), 0600))
 
-	p, _, _, err := resolveSocket("", cfg)
+	p, err := resolveSocket("", cfg)
 	require.NoError(t, err)
 	assert.Equal(t, nowhereSocket, p)
 }
@@ -64,10 +64,10 @@ func TestResolveSocketSaysTheSocketIsDisabled(t *testing.T) {
 	cfg := filepath.Join(t.TempDir(), "config.yml")
 	require.NoError(t, os.WriteFile(cfg, []byte("local_api:\n  control:\n    enabled: false\n"), 0600))
 
-	_, _, _, err := resolveSocket("", cfg)
+	_, err := resolveSocket("", cfg)
 	require.ErrorContains(t, err, "the control socket is disabled in "+cfg+" by local_api.control.enabled")
 
-	p, _, _, err := resolveSocket("/explicit.sock", cfg)
+	p, err := resolveSocket("/explicit.sock", cfg)
 	require.NoError(t, err)
 	assert.Equal(t, "/explicit.sock", p)
 }
@@ -80,45 +80,20 @@ func TestResolveSocketUsesTheDefaultWithoutAnOverride(t *testing.T) {
 	cfg := filepath.Join(t.TempDir(), "config.yml")
 	require.NoError(t, os.WriteFile(cfg, []byte("static_host_map: {}\n"), 0600))
 
-	p, readConfig, overrideIn, err := resolveSocket("", cfg)
+	p, err := resolveSocket("", cfg)
 	require.NoError(t, err)
 	assert.Equal(t, localapi.DefaultSocketPath(), p)
-	assert.Equal(t, cfg, readConfig)
-	assert.Empty(t, overrideIn)
 
 	// The test binary has no config next to it.
-	p, readConfig, overrideIn, err = resolveSocket("", "")
+	p, err = resolveSocket("", "")
 	require.NoError(t, err)
 	assert.Equal(t, localapi.DefaultSocketPath(), p)
-	assert.Empty(t, readConfig)
-	assert.Empty(t, overrideIn)
 }
 
 func TestResolveSocketFlagWins(t *testing.T) {
-	p, _, overrideIn, err := resolveSocket("/explicit.sock", "/does/not/matter.yml")
+	p, err := resolveSocket("/explicit.sock", "/does/not/matter.yml")
 	require.NoError(t, err)
 	assert.Equal(t, "/explicit.sock", p)
-	assert.Empty(t, overrideIn)
-}
-
-// TestResolveSocketSaysWhenTheConfigNamedThePath: a path read from the override key is
-// reported with the file it came from, so a dial failure can say that a daemon which
-// has not reloaded since the key changed is still on the old path. A derived path is
-// not an override.
-func TestResolveSocketSaysWhenTheConfigNamedThePath(t *testing.T) {
-	base := t.TempDir()
-
-	named := filepath.Join(base, "named.yml")
-	require.NoError(t, os.WriteFile(named, []byte(namedSocketConfig), 0600))
-	_, _, overrideIn, err := resolveSocket("", named)
-	require.NoError(t, err)
-	assert.Equal(t, named, overrideIn)
-
-	derived := filepath.Join(base, "derived.yml")
-	require.NoError(t, os.WriteFile(derived, []byte("static_host_map: {}\n"), 0600))
-	_, _, overrideIn, err = resolveSocket("", derived)
-	require.NoError(t, err)
-	assert.Empty(t, overrideIn)
 }
 
 // TestParseCommandReportsWhatFlagSaid covers the three ways a flag line goes wrong. They

cmd/internal/ctl/ctl_unix_test.go

@@ -22,7 +22,7 @@ func TestResolveSocketDiagnosesAnOverlongOverride(t *testing.T) {
 	long := "/" + strings.Repeat("x", 120)
 	require.NoError(t, os.WriteFile(cfg, []byte("local_api:\n  control:\n    socket: "+long+"\n"), 0600))
 
-	_, _, _, err := resolveSocket("", cfg)
+	_, err := resolveSocket("", cfg)
 	require.Error(t, err)
 	require.ErrorContains(t, err, "too long for this platform")
 	assert.ErrorContains(t, err, localapi.CtlSocketKey)
@@ -35,7 +35,7 @@ func TestResolveSocketRefusesARelativeOverride(t *testing.T) {
 	cfg := filepath.Join(t.TempDir(), "config.yml")
 	require.NoError(t, os.WriteFile(cfg, []byte("local_api:\n  control:\n    socket: run/ctl.sock\n"), 0600))
 
-	_, _, _, err := resolveSocket("", cfg)
+	_, err := resolveSocket("", cfg)
 	require.Error(t, err)
 	require.ErrorContains(t, err, "must be an absolute path")
 	assert.ErrorContains(t, err, localapi.CtlSocketKey)

localapi/platform_unix.go

@@ -59,8 +59,9 @@ const infoSupported = true
 // chmoded: a directory a service manager made for us is served from at whatever mode it
 // came with (systemd's RuntimeDirectoryMode= is 0755 by default), and one an operator
 // pointed us into keeps whatever else it holds readable. The one we make ourselves is
-// 0755, searchable by everyone, so that an unprivileged nebula ctl can see a root
-// daemon's socket and say so; see ServedByRootFor.
+// 0755, searchable by everyone, so that a nebula ctl run as another user reaches the
+// socket and gets the kernel's answer, which for a member of the configured group is
+// a connection and for anyone else is permission denied.
 func listen(path string, acc Access) (net.Listener, error) {
 	// The kernel silently truncates an over long sun_path and bind then fails with
 	// "invalid argument", which tells an operator nothing. Say what is actually wrong.