diff --git a/examples/config.yml b/examples/config.yml index 008b0651..643fee3b 100644 --- a/examples/config.yml +++ b/examples/config.yml @@ -272,7 +272,8 @@ punchy: # #info: #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 # 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 diff --git a/localapi/access_windows_test.go b/localapi/access_windows_test.go index 5b8ce14d..e33a1cd9 100644 --- a/localapi/access_windows_test.go +++ b/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.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) +} diff --git a/localapi/info.go b/localapi/info.go index e27b3cf9..8713ae93 100644 --- a/localapi/info.go +++ b/localapi/info.go @@ -127,6 +127,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()) + 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) // Closing the server closes the listener Serve was handed, and a Serve that has diff --git a/localapi/platform_unix.go b/localapi/platform_unix.go index 81e6e972..89e2df67 100644 --- a/localapi/platform_unix.go +++ b/localapi/platform_unix.go @@ -337,6 +337,10 @@ func pathTooLong(path string) bool { return len(path) > maxSocketPath } // of its own with CtlSocketKey. 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 // 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. diff --git a/localapi/platform_windows.go b/localapi/platform_windows.go index a634ea6d..b5c5b8c7 100644 --- a/localapi/platform_windows.go +++ b/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 // account could have created first; the owner check in dial is what protects a client // 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 { 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)