Quellcode durchsuchen

Warn when the info API's windows pipe is outside the protected namespace

A program reading the info API opens the pipe by name and has no owner
check of its own, so for a name any local user could create first,
whoever did would be answering its queries. nebula refuses to serve a
held name and logs it, but the program cannot tell. The daemon now warns
at bind when the configured name is not under
\\.\pipe\ProtectedPrefix\Administrators\, and the example config and
CHANGELOG say to put it there.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Matt Richardson vor 11 Stunden
Ursprung
Commit
3acce67984

+ 2 - 1
examples/config.yml

@@ -272,7 +272,8 @@ punchy:
   #
   #
   #info:
   #info:
     #enabled: false
     #enabled: false
-    # The absolute path to listen on, or a full pipe name on Windows.
+    # The absolute path to listen on, or a full pipe name on Windows. Name a pipe under
+    # \\.\pipe\ProtectedPrefix\Administrators\.
     #socket: /var/run/nebula/info.sock
     #socket: /var/run/nebula/info.sock
     # A group (or account/group/SID on Windows) that should be allowed to access the
     # A group (or account/group/SID on Windows) that should be allowed to access the
     # socket. On Windows the group may read and write the pipe but not create an
     # socket. On Windows the group may read and write the pipe but not create an

+ 8 - 0
localapi/access_windows_test.go

@@ -42,3 +42,11 @@ func TestSDDLAdmitsTheGroup(t *testing.T) {
 	assert.Zero(t, pipeClientAccess&windows.FILE_APPEND_DATA, "FILE_APPEND_DATA is FILE_CREATE_PIPE_INSTANCE on a pipe")
 	assert.Zero(t, pipeClientAccess&windows.FILE_APPEND_DATA, "FILE_APPEND_DATA is FILE_CREATE_PIPE_INSTANCE on a pipe")
 	assert.Zero(t, pipeClientAccess&(windows.GENERIC_READ|windows.GENERIC_WRITE|windows.GENERIC_ALL))
 	assert.Zero(t, pipeClientAccess&(windows.GENERIC_READ|windows.GENERIC_WRITE|windows.GENERIC_ALL))
 }
 }
+
+// TestUnprotectedNameWarning: a pipe outside the protected namespace is warned about,
+// since anyone could have created the name first; one under it, however spelled, is not.
+func TestUnprotectedNameWarning(t *testing.T) {
+	assert.Empty(t, unprotectedNameWarning(DefaultSocketPath))
+	assert.Empty(t, unprotectedNameWarning(`\\.\PIPE\protectedprefix\administrators\nebula-info`))
+	assert.Contains(t, unprotectedNameWarning(`\\.\pipe\nebula-info`), protectedPrefix)
+}

+ 3 - 0
localapi/info.go

@@ -143,6 +143,9 @@ func (s *InfoServer) Rebind(ctx context.Context, path string, acc Access) error
 		}
 		}
 
 
 		s.l.Info("Local info API listening", "path", path, "access", acc.String())
 		s.l.Info("Local info API listening", "path", path, "access", acc.String())
+		if w := unprotectedNameWarning(path); w != "" {
+			s.l.Warn("Local info API is served on a pipe name that is not protected", "path", path, "risk", w)
+		}
 		go s.serve(srv, ln)
 		go s.serve(srv, ln)
 
 
 		// Closing the server closes the listener Serve was handed, and a Serve that has
 		// Closing the server closes the listener Serve was handed, and a Serve that has

+ 4 - 0
localapi/platform_unix.go

@@ -337,6 +337,10 @@ func pathTooLong(path string) bool { return len(path) > maxSocketPath }
 // of its own with CtlSocketKey.
 // of its own with CtlSocketKey.
 const DefaultSocketPath = "/var/run/" + DirName + "/ctl.sock"
 const DefaultSocketPath = "/var/run/" + DirName + "/ctl.sock"
 
 
+// unprotectedNameWarning is for windows pipe names; a unix socket's directory is
+// checked instead.
+func unprotectedNameWarning(string) string { return "" }
+
 // checkSocketPath refuses a configured socket path that is relative, or too long to
 // checkSocketPath refuses a configured socket path that is relative, or too long to
 // bind. A relative path would mean a different socket to the daemon and to nebula ctl,
 // bind. A relative path would mean a different socket to the daemon and to nebula ctl,
 // each resolving it against the directory it happened to be started from.
 // each resolving it against the directory it happened to be started from.

+ 13 - 0
localapi/platform_windows.go

@@ -251,6 +251,19 @@ const DefaultSocketPath = protectedPrefix + "nebula-ctl"
 // pipePrefix is used as given. An override outside protectedPrefix is a name any
 // pipePrefix is used as given. An override outside protectedPrefix is a name any
 // account could have created first; the owner check in dial is what protects a client
 // account could have created first; the owner check in dial is what protects a client
 // of one.
 // of one.
+// unprotectedNameWarning says what a pipe name outside protectedPrefix costs, or "" for
+// one under it. A program reading the info API opens the pipe by name and has no owner
+// check of its own, so for a name any local user could create first, whoever did
+// would be answering its queries; the daemon refuses to serve a held name and logs it,
+// but the program cannot tell. The default control pipe is under the prefix; an info
+// pipe is wherever the operator put it.
+func unprotectedNameWarning(path string) string {
+	if len(path) >= len(protectedPrefix) && strings.EqualFold(path[:len(protectedPrefix)], protectedPrefix) {
+		return ""
+	}
+	return "any local user can create a pipe by this name ahead of the daemon and answer in its place; name it under " + protectedPrefix
+}
+
 func checkSocketPath(key, value string) error {
 func checkSocketPath(key, value string) error {
 	if len(value) <= len(pipePrefix) || !strings.EqualFold(value[:len(pipePrefix)], pipePrefix) {
 	if len(value) <= len(pipePrefix) || !strings.EqualFold(value[:len(pipePrefix)], pipePrefix) {
 		return fmt.Errorf(`%s must be a full pipe name under %s, got %q`, key, pipePrefix, value)
 		return fmt.Errorf(`%s must be a full pipe name under %s, got %q`, key, pipePrefix, value)