diff --git a/e2e/info_api_test.go b/e2e/info_api_test.go index c02035b6..f1136bb0 100644 --- a/e2e/info_api_test.go +++ b/e2e/info_api_test.go @@ -27,7 +27,7 @@ import ( // established with. func TestIdentityOverTheInfoAPI(t *testing.T) { if runtime.GOOS == "windows" { - t.Skip("the info API is not served on windows") + t.Skip("the test's HTTP helpers dial unix sockets; on windows the info API is a named pipe") } ca, _, caKey, _ := cert_test.NewTestCaCert(cert.Version2, cert.Curve_CURVE25519, time.Now(), time.Now().Add(10*time.Minute), nil, nil, []string{}) diff --git a/examples/config.yml b/examples/config.yml index 643f61ec..f53b4cdf 100644 --- a/examples/config.yml +++ b/examples/config.yml @@ -255,7 +255,7 @@ punchy: #group: nebula # The info socket is not made available by default. If you enable it, it makes some - # internal state available over HTTP. It's not available on Windows. + # internal state available over HTTP. # # The currently supported HTTP requests are: # GET /v1/identity?address=[:port] the host owning the address; the port @@ -269,10 +269,17 @@ punchy: # #info: #enabled: false - # The absolute path to listen on. + # The absolute path to listen on, or a full pipe name on Windows. #socket: /var/run/nebula/info.sock - # Octal mode for the socket, as a quoted string. Defaults to "0600". - #socket_mode: "0600" + # 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 + # instance of it, so a client in the group must open the pipe asking for read and + # write data rather than generic read and write. + #group: nebula-info + # Octal mode for the socket, as a quoted string. Defaults to "0600" unless a + # group is set, in which case it defaults to "0660". On Windows a pipe has no + # mode and this is ignored; the group alone decides who may connect. + #socket_mode: "0660" # EXPERIMENTAL: relay support for networks that can't establish direct connections. relay: diff --git a/localapi/access_unix_test.go b/localapi/access_unix_test.go index ed9d7575..98a8aa62 100644 --- a/localapi/access_unix_test.go +++ b/localapi/access_unix_test.go @@ -4,6 +4,7 @@ package localapi import ( "context" + "net/http" "os" "os/user" "strconv" @@ -142,6 +143,66 @@ func TestControlSocketWithAGroupTrustsTheKernel(t *testing.T) { require.ErrorContains(t, err, "not permitted") } +// TestInfoAccessFromConfig: a group with no mode is 0660; a mode that gives the group +// nothing is refused, with the mode that would; an explicit mode with group bits is +// kept as given. +func TestInfoAccessFromConfig(t *testing.T) { + name, gid := ownGroup(t) + load := func(body string) (infoConfig, error) { + c := config.NewC(test.NewLogger()) + require.NoError(t, c.LoadString(body)) + return infoConfigFrom(c) + } + section := func(extra string) string { + return "local_api:\n info:\n enabled: true\n socket: /run/nebula/info.sock\n" + extra + } + + cfg, err := load(section(" group: " + name + "\n")) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0660), cfg.Access.Mode) + require.NotNil(t, cfg.Access.Group) + assert.Equal(t, gid, cfg.Access.Group.ID) + + cfg, err = load(section(" group: " + name + "\n socket_mode: \"0664\"\n")) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0664), cfg.Access.Mode) + + _, err = load(section(" group: " + name + "\n socket_mode: \"0600\"\n")) + require.ErrorContains(t, err, "gives group "+name+" no access") + require.ErrorContains(t, err, `"0660"`) + + cfg, err = load(section("")) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0600), cfg.Access.Mode) + assert.Nil(t, cfg.Access.Group) +} + +// TestSocketIsHandedToTheGroup: the socket is created with the group's gid and the +// mode, and a changed group or mode on a reload is applied to the socket in place. +func TestSocketIsHandedToTheGroup(t *testing.T) { + name, gid := ownGroup(t) + group := &Group{Name: name, ID: gid} + want, err := strconv.ParseUint(gid, 10, 32) + require.NoError(t, err) + + s, path := serveInfo(t, infoRegistry(), 0600) + require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: 0660, Group: group})) + fi, err := os.Lstat(path) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0660), fi.Mode().Perm()) + assert.Equal(t, uint32(want), socketGroup(t, path)) + + status, _ := test.HTTPGet(t, path, "/v1/whoami") + assert.Equal(t, http.StatusOK, status, "the same listener is still serving") + + // The mode is the operator's to widen further. + require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: 0664, Group: group})) + fi, err = os.Lstat(path) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0664), fi.Mode().Perm()) + assert.Equal(t, path, s.Path()) +} + // TestRemovingTheGroupHandsTheSocketBack: a reload that drops the group key leaves the // mode the operator set but returns the socket to the daemon's own group, so the old // group's members are no longer admitted. The socket is first handed to a group this diff --git a/localapi/info.go b/localapi/info.go index ab186b39..600bc661 100644 --- a/localapi/info.go +++ b/localapi/info.go @@ -14,6 +14,7 @@ import ( "net/url" "os" "path" + "runtime" "runtime/debug" "slices" "strconv" @@ -31,12 +32,13 @@ const ( InfoEnabledKey = "local_api.info.enabled" InfoSocketKey = "local_api.info.socket" InfoModeKey = "local_api.info.socket_mode" + InfoGroupKey = "local_api.info.group" ) const ( - // defaultInfoMode is the socket's mode when the config names none: the daemon's - // user alone, so that enabling the API does not open it to anyone until the operator - // says who. + // defaultInfoMode is the socket's mode when the config names neither a mode nor a + // group: the daemon's user alone, so that enabling the API does not open it to + // anyone until the operator says who. Naming a group makes it groupSocketMode. defaultInfoMode os.FileMode = 0600 // infoMaxConns bounds the connections held open at once. Beyond it the kernel @@ -394,7 +396,10 @@ type infoConfig struct { } // infoConfigFrom reads the info API section. A disabled section needs nothing else to be -// right; an enabled one needs a socket path and, if it names a mode, one that parses. +// right; an enabled one needs a socket path, a group that exists if it names one, and +// a mode that parses if it names one. A group with no mode means groupSocketMode, and a +// mode that gives a named group nothing is refused: it would be a group that cannot +// connect, which is not what naming one asks for. func infoConfigFrom(c *config.C) (infoConfig, error) { cfg := infoConfig{Enabled: c.GetBool(InfoEnabledKey, false)} if !cfg.Enabled { @@ -410,11 +415,26 @@ func infoConfigFrom(c *config.C) (infoConfig, error) { } cfg.Path = p - mode, err := parseSocketMode(c.Get(InfoModeKey)) + group, err := groupFromConfig(c, InfoGroupKey) + if err != nil { + return cfg, err + } + def := defaultInfoMode + if group != nil { + def = groupSocketMode + } + + mode, err := parseSocketMode(c.Get(InfoModeKey), def) if err != nil { return cfg, fmt.Errorf("%s: %w", InfoModeKey, err) } - cfg.Access = Access{Mode: mode} + // On unix the mode is what admits the group, so one that shuts it out is a config + // error; on windows the group is an entry in the pipe's security descriptor and the + // mode means nothing to a pipe, so it is not held against the group there. + if runtime.GOOS != "windows" && group != nil && mode&0070 == 0 { + return cfg, fmt.Errorf("%s %s gives group %s no access to the socket; use \"%04o\" or leave the mode unset", InfoModeKey, modeString(mode), group.Name, mode|0060) + } + cfg.Access = Access{Mode: mode, Group: group} return cfg, nil } @@ -423,10 +443,10 @@ func infoConfigFrom(c *config.C) (infoConfig, error) { // integer, and which integer depends on the parser (octal 432 under YAML 1.1, decimal // 660 under 1.2), so a number is refused with the fix rather than quietly applied as // whichever mode its digits happen to spell in octal. -func parseSocketMode(raw any) (os.FileMode, error) { +func parseSocketMode(raw any, def os.FileMode) (os.FileMode, error) { switch v := raw.(type) { case nil: - return defaultInfoMode, nil + return def, nil case string: mode, err := strconv.ParseUint(v, 8, 32) if err != nil || mode&^0777 != 0 { diff --git a/localapi/info_windows_test.go b/localapi/info_windows_test.go index c9aa1d06..e214168f 100644 --- a/localapi/info_windows_test.go +++ b/localapi/info_windows_test.go @@ -4,20 +4,55 @@ package localapi import ( "context" + "io" + "net" + "net/http" "testing" + "time" + "github.com/Microsoft/go-winio" "github.com/slackhq/nebula/commands" "github.com/slackhq/nebula/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/sys/windows" ) -// TestInfoIsNotServedOnWindows: a pipe has no mode to open to other accounts, so Rebind -// says so and leaves nothing listening, whatever the config asked for. -func TestInfoIsNotServedOnWindows(t *testing.T) { +// TestInfoIsServedOnAPipe: the info API serves HTTP on a named pipe, whose security +// descriptor is its access control the way a socket's mode is on unix; a group named +// in the config is admitted by it. +func TestInfoIsServedOnAPipe(t *testing.T) { + users, err := windows.CreateWellKnownSid(windows.WinBuiltinUsersSid) + require.NoError(t, err) + group := &Group{Name: "Users", ID: users.String()} + s := NewInfoServer(test.NewLogger(), commands.New()) - err := s.Rebind(context.Background(), pipePrefix+"nebula-info-test", Access{Mode: 0600}) - require.ErrorIs(t, err, errInfoUnsupported) - assert.Empty(t, s.Path()) - s.Stop() + path := pipePrefix + "nebula-info-test" + require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: groupSocketMode, Group: group})) + t.Cleanup(s.Stop) + assert.Equal(t, path, s.Path()) + + client := &http.Client{Transport: &http.Transport{ + // What a program on the host has to do: ask for the data rights alone, since the + // group is granted no more; see pipeClientAccess. + DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { + ctx, cancel := context.WithTimeout(ctx, 2*time.Second) + defer cancel() + return winio.DialPipeAccess(ctx, path, pipeClientAccess) + }, + }} + res, err := client.Get("http://nebula/v1") + require.NoError(t, err) + defer res.Body.Close() + body, err := io.ReadAll(res.Body) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Contains(t, string(body), `"help"`) + + // A change of group means a new pipe under the same name, still serving. + require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: groupSocketMode})) + res, err = client.Get("http://nebula/v1") + require.NoError(t, err) + res.Body.Close() + assert.Equal(t, http.StatusOK, res.StatusCode) } diff --git a/localapi/platform_windows.go b/localapi/platform_windows.go index f20f534f..eea82077 100644 --- a/localapi/platform_windows.go +++ b/localapi/platform_windows.go @@ -33,11 +33,10 @@ const network = "pipe" // exists so a Server is built the same way on every platform. var currentUID = func() uint32 { return 0 } -// infoSupported is false here. A pipe has no mode to open to other accounts, and the -// security descriptor listen builds admits the daemon's account alone, which is the -// opposite of what the info API is for. Serving it on windows means designing which -// accounts a pipe admits, and that has not been done. -const infoSupported = false +// infoSupported is true here: the pipe's security descriptor is what a mode is on unix, +// and the group an operator names in the config is the way to open it to other +// accounts, as it is for the control pipe. The mode itself means nothing to a pipe. +const infoSupported = true // listen creates the named pipe at path with a security descriptor that admits the user // nebula runs as, SYSTEM and Administrators, and the group acc names, if any, for diff --git a/localapi/wire.go b/localapi/wire.go index d05c8cd0..026e0701 100644 --- a/localapi/wire.go +++ b/localapi/wire.go @@ -253,8 +253,9 @@ func wireInfo(ctx context.Context, l *slog.Logger, c *config.C, reg *commands.Re // errEndpointHeld is returned by listen when the endpoint's name is already held by // some other process, which nebula will not serve alongside: whoever holds it would be -// answering nebula ctl. It is only raised on windows, where the default name can be -// held by another administrator's process and an override outside the protected -// namespace by any account's; on unix a path that is still being served is an earlier -// nebula, which clearStale reports as in use. -var errEndpointHeld = errors.New("the local control socket's name is held by another process") +// answering the endpoint's clients. It is only raised on windows, where the default +// name can be held by another administrator's process and an override outside the +// protected namespace by any account's; on unix a path that is still being served is an +// earlier nebula, which clearStale reports as in use. The log line that carries it names +// the endpoint, so the sentence does not. +var errEndpointHeld = errors.New("the pipe's name is held by another process")