From 9565dde138f6b1182f97e309a877323b02f24feb Mon Sep 17 00:00:00 2001 From: Mike Nomitch Date: Thu, 7 Mar 2024 11:23:33 -0800 Subject: [PATCH] Only parsing id ranges once --- drivers/exec/driver.go | 51 ++++-- drivers/exec/driver_test.go | 69 ++++---- drivers/rawexec/driver.go | 15 +- drivers/rawexec/driver_test.go | 19 +-- drivers/rawexec/driver_unix.go | 2 +- drivers/rawexec/driver_unix_test.go | 6 +- drivers/rawexec/driver_windows.go | 2 +- drivers/shared/validators/validators.go | 164 +++++++++---------- drivers/shared/validators/validators_test.go | 56 +++---- website/content/docs/drivers/exec.mdx | 14 +- website/content/docs/drivers/raw_exec.mdx | 4 +- 11 files changed, 200 insertions(+), 202 deletions(-) diff --git a/drivers/exec/driver.go b/drivers/exec/driver.go index 7ec930810..c61a780ae 100644 --- a/drivers/exec/driver.go +++ b/drivers/exec/driver.go @@ -164,11 +164,17 @@ type Config struct { // running on this node. AllowCaps []string `codec:"allow_caps"` + // TODO: Move this out of validators into structs + // if you are to use it here + + DeniedHostUidsStr string `codec:"denied_host_uids"` + DeniedHostGidsStr string `codec:"denied_host_gids"` + // DeniedHostUids configures which host uids are disallowed - DeniedHostUids string `codec:"denied_host_uids"` + DeniedHostUids []validators.IDRange `codec:"-"` // DeniedHostGids configures which host gids are disallowed - DeniedHostGids string `codec:"denied_host_gids"` + DeniedHostGids []validators.IDRange `codec:"-"` } func (c *Config) validate() error { @@ -189,14 +195,6 @@ func (c *Config) validate() error { return fmt.Errorf("allow_caps configured with capabilities not supported by system: %s", badCaps) } - if err := validators.IDRangeValid("denied_host_uids", c.DeniedHostUids); err != nil { - return err - } - - if err := validators.IDRangeValid("denied_host_gids", c.DeniedHostGids); err != nil { - return err - } - return nil } @@ -223,7 +221,7 @@ type TaskConfig struct { CapDrop []string `codec:"cap_drop"` } -func (tc *TaskConfig) validate(cfg *drivers.TaskConfig, driverConfig *Config) error { +func (tc *TaskConfig) validate() error { switch tc.ModePID { case "", executor.IsolationModePrivate, executor.IsolationModeHost: default: @@ -241,20 +239,20 @@ func (tc *TaskConfig) validate(cfg *drivers.TaskConfig, driverConfig *Config) er if !badAdds.Empty() { return fmt.Errorf("cap_add configured with capabilities not supported by system: %s", badAdds) } + badDrops := supported.Difference(capabilities.New(tc.CapDrop)) if !badDrops.Empty() { return fmt.Errorf("cap_drop configured with capabilities not supported by system: %s", badDrops) } - usernameToLookup := getUsername(cfg) - - if err := validators.UserInRange(users.Lookup, usernameToLookup, driverConfig.DeniedHostUids, driverConfig.DeniedHostGids); err != nil { - return err - } - return nil } +func (tc *TaskConfig) validateUserIds(cfg *drivers.TaskConfig, driverConfig *Config) error { + usernameToLookup := getUsername(cfg) + return validators.HasValidIds(users.Lookup, usernameToLookup, driverConfig.DeniedHostUids, driverConfig.DeniedHostGids) +} + // TaskState is the state which is encoded in the handle returned in // StartTask. This information is needed to rebuild the task state and handler // during recovery. @@ -319,6 +317,19 @@ func (d *Driver) SetConfig(cfg *base.Config) error { } d.config = config + deniedUidRanges, err := validators.ParseIdRange("denied_host_uids", config.DeniedHostUidsStr) + if err != nil { + return err + } + + deniedGidRanges, err := validators.ParseIdRange("denied_host_gids", config.DeniedHostGidsStr) + if err != nil { + return err + } + + d.config.DeniedHostUids = deniedUidRanges + d.config.DeniedHostGids = deniedGidRanges + if cfg != nil && cfg.AgentConfig != nil { d.nomadConfig = cfg.AgentConfig.Driver d.compute = cfg.AgentConfig.Compute() @@ -457,10 +468,14 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive return nil, nil, fmt.Errorf("failed to decode driver config: %v", err) } - if err := driverConfig.validate(cfg, &d.config); err != nil { + if err := driverConfig.validate(); err != nil { return nil, nil, fmt.Errorf("failed driver config validation: %v", err) } + if err := driverConfig.validateUserIds(cfg, &d.config); err != nil { + return nil, nil, fmt.Errorf("failed host user validation: %v", err) + } + d.logger.Info("starting task", "driver_cfg", hclog.Fmt("%+v", driverConfig)) handle := drivers.NewTaskHandle(taskHandleVersion) handle.Config = cfg diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go index 3c7e539cb..dfaa9a4d4 100644 --- a/drivers/exec/driver_test.go +++ b/drivers/exec/driver_test.go @@ -887,7 +887,7 @@ func TestDriver_Config_validate(t *testing.T) { {uidRanges: "", gidRanges: "10-1", errorStr: &invalidGidRange}, } { - validationErr := (&Config{ + err := (&Config{ DefaultModePID: "private", DefaultModeIPC: "private", DeniedHostUids: tc.uidRanges, @@ -895,24 +895,21 @@ func TestDriver_Config_validate(t *testing.T) { }).validate() if tc.errorStr == nil { - require.Nil(t, validationErr) + must.NoError(t, err) } else { - require.Contains(t, validationErr.Error(), *tc.errorStr) + must.ErrorContains(t, err, *tc.errorStr) } } }) } -func TestDriver_TaskConfig_validate(t *testing.T) { +func TestDriver_TaskConfig_validateUserIds(t *testing.T) { ci.Parallel(t) current, err := users.Current() require.NoError(t, err) - currentUid, err := strconv.ParseUint(current.Uid, 10, 32) - require.NoError(t, err) - nobody, err := users.Lookup("nobody") - require.NoError(t, err) - nobodyUid, err := strconv.ParseUint(nobody.Uid, 10, 32) + currentUid := os.Getuid() + nobodyUid, _, _, err := users.LookupUnix("nobody") require.NoError(t, err) allowAll := "" @@ -923,6 +920,30 @@ func TestDriver_TaskConfig_validate(t *testing.T) { configDenyAnonymous := Config{DeniedHostUids: denyNobody} driverConfigNoUserSpecified := drivers.TaskConfig{} driverConfigSpecifyCurrent := drivers.TaskConfig{User: current.Name} + currentUserErrStr := fmt.Sprintf("running as uid %d is disallowed", currentUid) + anonUserErrStr := fmt.Sprintf("running as uid %d is disallowed", nobodyUid) + + for _, tc := range []struct { + config Config + driverConfig drivers.TaskConfig + expectedErr string + }{ + {config: configAllowCurrent, driverConfig: driverConfigSpecifyCurrent, expectedErr: ""}, + {config: configDenyCurrent, driverConfig: driverConfigNoUserSpecified, expectedErr: ""}, + {config: configDenyCurrent, driverConfig: driverConfigSpecifyCurrent, expectedErr: currentUserErrStr}, + {config: configDenyAnonymous, driverConfig: driverConfigNoUserSpecified, expectedErr: anonUserErrStr}, + } { + err := (&TaskConfig{}).validateUserIds(&tc.driverConfig, &tc.config) + if tc.expectedErr == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, tc.expectedErr) + } + } +} + +func TestDriver_TaskConfig_validate(t *testing.T) { + ci.Parallel(t) t.Run("pid/ipc", func(t *testing.T) { for _, tc := range []struct { @@ -939,10 +960,10 @@ func TestDriver_TaskConfig_validate(t *testing.T) { {pidMode: "", ipcMode: "host", exp: nil}, {pidMode: "other", ipcMode: "host", exp: errors.New(`pid_mode must be "private" or "host", got "other"`)}, } { - require.Equal(t, tc.exp, (&TaskConfig{ + must.Eq(t, tc.exp, (&TaskConfig{ ModePID: tc.pidMode, ModeIPC: tc.ipcMode, - }).validate(&driverConfigNoUserSpecified, &configAllowCurrent)) + }).validate()) } }) @@ -957,9 +978,9 @@ func TestDriver_TaskConfig_validate(t *testing.T) { {adds: []string{"chown", "sys_time"}, exp: nil}, {adds: []string{"chown", "not_valid", "sys_time"}, exp: errors.New("cap_add configured with capabilities not supported by system: not_valid")}, } { - require.Equal(t, tc.exp, (&TaskConfig{ + must.Eq(t, tc.exp, (&TaskConfig{ CapAdd: tc.adds, - }).validate(&driverConfigNoUserSpecified, &configAllowCurrent)) + }).validate()) } }) @@ -974,27 +995,9 @@ func TestDriver_TaskConfig_validate(t *testing.T) { {drops: []string{"chown", "sys_time"}, exp: nil}, {drops: []string{"chown", "not_valid", "sys_time"}, exp: errors.New("cap_drop configured with capabilities not supported by system: not_valid")}, } { - require.Equal(t, tc.exp, (&TaskConfig{ + must.Eq(t, tc.exp, (&TaskConfig{ CapDrop: tc.drops, - }).validate(&driverConfigNoUserSpecified, &configAllowCurrent)) - } - }) - - t.Run("uid_restriction", func(t *testing.T) { - currentUserErrStr := fmt.Sprintf("running as uid %d is disallowed", currentUid) - anonUserErrStr := fmt.Sprintf("running as uid %d is disallowed", nobodyUid) - - for _, tc := range []struct { - config Config - driverConfig drivers.TaskConfig - exp error - }{ - {config: configAllowCurrent, driverConfig: driverConfigSpecifyCurrent, exp: nil}, - {config: configDenyCurrent, driverConfig: driverConfigNoUserSpecified, exp: nil}, - {config: configDenyCurrent, driverConfig: driverConfigSpecifyCurrent, exp: errors.New(currentUserErrStr)}, - {config: configDenyAnonymous, driverConfig: driverConfigNoUserSpecified, exp: errors.New(anonUserErrStr)}, - } { - require.Equal(t, tc.exp, (&TaskConfig{}).validate(&tc.driverConfig, &tc.config)) + }).validate()) } }) } diff --git a/drivers/rawexec/driver.go b/drivers/rawexec/driver.go index e6c7b8c24..5348d1fd7 100644 --- a/drivers/rawexec/driver.go +++ b/drivers/rawexec/driver.go @@ -143,11 +143,14 @@ type Config struct { // Enabled is set to true to enable the raw_exec driver Enabled bool `codec:"enabled"` + DeniedHostUidsStr string `codec:"denied_host_uids"` + DeniedHostGidsStr string `codec:"denied_host_gids"` + // DeniedHostUids configures which host uids are disallowed - DeniedHostUids string `codec:"denied_host_uids"` + DeniedHostUids []validators.IDRange // DeniedHostGids configures which host gids are disallowed - DeniedHostGids string `codec:"denied_host_gids"` + DeniedHostGids []validators.IDRange } // TaskConfig is the driver configuration of a task within a job @@ -210,15 +213,19 @@ func (d *Driver) SetConfig(cfg *base.Config) error { } } - if err := validators.IDRangeValid("denied_host_uids", config.DeniedHostUids); err != nil { + deniedUidRanges, err := validators.ParseIdRange("denied_host_uids", config.DeniedHostUidsStr) + if err != nil { return err } - if err := validators.IDRangeValid("denied_host_gids", config.DeniedHostGids); err != nil { + deniedGidRanges, err := validators.ParseIdRange("denied_host_gids", config.DeniedHostGidsStr) + if err != nil { return err } d.config = &config + d.config.DeniedHostUids = deniedUidRanges + d.config.DeniedHostGids = deniedGidRanges if cfg.AgentConfig != nil { d.nomadConfig = cfg.AgentConfig.Driver diff --git a/drivers/rawexec/driver_test.go b/drivers/rawexec/driver_test.go index 56defca3e..5a0c23bb8 100644 --- a/drivers/rawexec/driver_test.go +++ b/drivers/rawexec/driver_test.go @@ -92,7 +92,6 @@ func newEnabledRawExecDriver(t *testing.T) *Driver { func TestRawExecDriver_SetConfig(t *testing.T) { ci.Parallel(t) - require := require.New(t) ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -110,27 +109,27 @@ func TestRawExecDriver_SetConfig(t *testing.T) { ) // Default is raw_exec is disabled. - require.NoError(basePlug.MsgPackEncode(&data, config)) + must.NoError(t, basePlug.MsgPackEncode(&data, config)) bconfig.PluginConfig = data - require.NoError(harness.SetConfig(bconfig)) - require.Exactly(config, d.(*Driver).config) + must.NoError(t, harness.SetConfig(bconfig)) + must.Eq(t, config, d.(*Driver).config) // Enable raw_exec, but disable cgroups. config.Enabled = true data = []byte{} - require.NoError(basePlug.MsgPackEncode(&data, config)) + must.NoError(t, basePlug.MsgPackEncode(&data, config)) bconfig.PluginConfig = data - require.NoError(harness.SetConfig(bconfig)) - require.Exactly(config, d.(*Driver).config) + must.NoError(t, harness.SetConfig(bconfig)) + must.Eq(t, config, d.(*Driver).config) // Turns on uid/gid restrictions, and sets the range to a bad value config.DeniedHostUids = "10-0" data = []byte{} - require.NoError(basePlug.MsgPackEncode(&data, config)) + must.NoError(t, basePlug.MsgPackEncode(&data, config)) bconfig.PluginConfig = data err := harness.SetConfig(bconfig) - require.Error(err) - require.Contains(err.Error(), "invalid denied_host_uids value") + must.Error(t, err) + must.ErrorContains(t, err, "invalid denied_host_uids value") } func TestRawExecDriver_Fingerprint(t *testing.T) { diff --git a/drivers/rawexec/driver_unix.go b/drivers/rawexec/driver_unix.go index 399c8806b..406616165 100644 --- a/drivers/rawexec/driver_unix.go +++ b/drivers/rawexec/driver_unix.go @@ -25,5 +25,5 @@ func (tc *TaskConfig) Validate(driverCofig Config, cfg drivers.TaskConfig) error usernameToLookup = current.Name } - return validators.UserInRange(users.Lookup, usernameToLookup, driverCofig.DeniedHostUids, driverCofig.DeniedHostGids) + return validators.HasValidIds(users.Lookup, usernameToLookup, driverCofig.DeniedHostUids, driverCofig.DeniedHostGids) } diff --git a/drivers/rawexec/driver_unix_test.go b/drivers/rawexec/driver_unix_test.go index 3575c4a8a..d76c74c11 100644 --- a/drivers/rawexec/driver_unix_test.go +++ b/drivers/rawexec/driver_unix_test.go @@ -551,9 +551,9 @@ func TestRawExec_Validate(t *testing.T) { ci.Parallel(t) current, err := users.Current() - require.NoError(t, err) + must.NoError(t, err) currentUid, err := strconv.ParseUint(current.Uid, 10, 32) - require.NoError(t, err) + must.NoError(t, err) currentUserErrStr := fmt.Sprintf("running as uid %d is disallowed", currentUid) @@ -573,6 +573,6 @@ func TestRawExec_Validate(t *testing.T) { {config: configDenyCurrent, driverConfig: driverConfigNoUserSpecified, exp: errors.New(currentUserErrStr)}, {config: configDenyCurrent, driverConfig: driverConfigSpecifyCurrent, exp: errors.New(currentUserErrStr)}, } { - require.Equal(t, tc.exp, (&TaskConfig{}).Validate(tc.config, tc.driverConfig)) + must.Eq(t, tc.exp, (&TaskConfig{}).Validate(tc.config, tc.driverConfig)) } } diff --git a/drivers/rawexec/driver_windows.go b/drivers/rawexec/driver_windows.go index ab2425978..f7c4aef41 100644 --- a/drivers/rawexec/driver_windows.go +++ b/drivers/rawexec/driver_windows.go @@ -12,7 +12,7 @@ import ( func (tc *TaskConfig) Validate(driverCofig Config, cfg drivers.TaskConfig) error { // This is a noop on windows since the uid and gid cannot be checked against a range easily // We could eventually extend this functionality to check for individual users IDs strings - // but that is not currently supported. See driverValidators.UserInRange for + // but that is not currently supported. See driverValidators.HasValidIds for // unix logic return nil } diff --git a/drivers/shared/validators/validators.go b/drivers/shared/validators/validators.go index 2ba6f7197..6898ace87 100644 --- a/drivers/shared/validators/validators.go +++ b/drivers/shared/validators/validators.go @@ -1,6 +1,8 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: MPL-2.0 +//go:build !windows + package validators import ( @@ -16,83 +18,10 @@ type IDRange struct { Upper uint64 `codec:"to"` } -// IDRangeValid is used to ensure that the configuration for ID ranges is valid. -func IDRangeValid(rangeType string, deniedRanges string) error { - _, err := parseRanges(deniedRanges) - if err != nil { - return fmt.Errorf("invalid %s value %q: %v", rangeType, deniedRanges, err) - } - - return nil -} - -type userLookupFn func(string) (*user.User, error) - -// UserInRange is used when running a task to ensure the -// given user is in the ID range defined in the task config -func UserInRange(userLookupFn userLookupFn, usernameToLookup string, deniedHostUIDs, deniedHostGIDs string) error { - - // look up user on host given username - - u, err := userLookupFn(usernameToLookup) - if err != nil { - return fmt.Errorf("failed to identify user %q: %v", usernameToLookup, err) - } - uid, err := strconv.ParseUint(u.Uid, 10, 32) - if err != nil { - return fmt.Errorf("unable to convert userid %s to integer", u.Uid) - } - - // check uids - - uidRanges, err := parseRanges(deniedHostUIDs) - if err != nil { - return fmt.Errorf("invalid denied_host_uids value %q: %v", deniedHostUIDs, err) - } - - for _, uidRange := range uidRanges { - if uid >= uidRange.Lower && uid <= uidRange.Upper { - return fmt.Errorf("running as uid %d is disallowed", uid) - } - } - - // check gids - - gidStrings, err := u.GroupIds() - if err != nil { - return fmt.Errorf("unable to lookup user's group membership: %v", err) - } - gids := make([]uint64, len(gidStrings)) - - for _, gidString := range gidStrings { - u, err := strconv.ParseUint(gidString, 10, 32) - if err != nil { - return fmt.Errorf("unable to convert user's group %q to integer", gidString) - } - - gids = append(gids, u) - } - - gidRanges, err := parseRanges(deniedHostGIDs) - if err != nil { - return fmt.Errorf("invalid denied_host_gids value %q: %v", deniedHostGIDs, err) - } - - for _, gidRange := range gidRanges { - for _, gid := range gids { - if gid >= gidRange.Lower && gid <= gidRange.Upper { - return fmt.Errorf("running as gid %d is disallowed", gid) - } - } - } - - return nil -} - -func parseRanges(ranges string) ([]IDRange, error) { +// ParseIdRange is used to ensure that the configuration for ID ranges is valid. +func ParseIdRange(rangeType string, deniedRanges string) ([]IDRange, error) { var idRanges []IDRange - - parts := strings.Split(ranges, ",") + parts := strings.Split(deniedRanges, ",") // exit early if empty string if len(parts) == 1 && parts[0] == "" { @@ -111,6 +40,59 @@ func parseRanges(ranges string) ([]IDRange, error) { return idRanges, nil } +type userLookupFn func(string) (*user.User, error) + +// HasValidIds is used when running a task to ensure the +// given user is in the ID range defined in the task config +func HasValidIds(userLookupFn userLookupFn, usernameToLookup string, deniedHostUIDs, deniedHostGIDs []IDRange) error { + + // look up user on host given username + + u, err := userLookupFn(usernameToLookup) + if err != nil { + return fmt.Errorf("failed to identify user %q: %w", usernameToLookup, err) + } + uid, err := strconv.ParseUint(u.Uid, 10, 32) + if err != nil { + return fmt.Errorf("unable to convert userid %s to integer", u.Uid) + } + + // check uids + + for _, uidRange := range deniedHostUIDs { + if uid >= uidRange.Lower && uid <= uidRange.Upper { + return fmt.Errorf("running as uid %d is disallowed", uid) + } + } + + // check gids + + gidStrings, err := u.GroupIds() + if err != nil { + return fmt.Errorf("unable to lookup user's group membership: %w", err) + } + gids := make([]uint64, len(gidStrings)) + + for _, gidString := range gidStrings { + u, err := strconv.ParseUint(gidString, 10, 32) + if err != nil { + return fmt.Errorf("unable to convert user's group %q to integer: %w", gidString, err) + } + + gids = append(gids, u) + } + + for _, gidRange := range deniedHostGIDs { + for _, gid := range gids { + if gid >= gidRange.Lower && gid <= gidRange.Upper { + return fmt.Errorf("running as gid %d is disallowed", gid) + } + } + } + + return nil +} + func parseRangeString(boundsString string) (*IDRange, error) { uidDenyRangeParts := strings.Split(boundsString, "-") @@ -118,36 +100,36 @@ func parseRangeString(boundsString string) (*IDRange, error) { switch len(uidDenyRangeParts) { case 0: - return nil, fmt.Errorf("range cannot be empty, invalid range: \"%q\" ", boundsString) + return nil, fmt.Errorf("range value cannot be empty") case 1: - singleBound := uidDenyRangeParts[0] - singleBoundInt, err := strconv.ParseUint(singleBound, 10, 64) + disallowedIdStr := uidDenyRangeParts[0] + disallowedIdInt, err := strconv.ParseUint(disallowedIdStr, 10, 32) if err != nil { - return nil, fmt.Errorf("range bound not valid, invalid bound: \"%q\" ", singleBoundInt) + return nil, fmt.Errorf("range bound not valid, invalid bound: %q ", disallowedIdInt) } - idRange.Lower = singleBoundInt - idRange.Upper = singleBoundInt + idRange.Lower = disallowedIdInt + idRange.Upper = disallowedIdInt case 2: - boundAStr := uidDenyRangeParts[0] - boundBStr := uidDenyRangeParts[1] + lowerBoundStr := uidDenyRangeParts[0] + upperBoundStr := uidDenyRangeParts[1] - boundAInt, err := strconv.ParseUint(boundAStr, 10, 64) + lowerBoundInt, err := strconv.ParseUint(lowerBoundStr, 10, 32) if err != nil { - return nil, fmt.Errorf("invalid range %q, invalid bound: \"%q\" ", boundsString, boundAStr) + return nil, fmt.Errorf("invalid bound: %q", lowerBoundStr) } - boundBInt, err := strconv.ParseUint(boundBStr, 10, 64) + upperBoundInt, err := strconv.ParseUint(upperBoundStr, 10, 32) if err != nil { - return nil, fmt.Errorf("invalid range %q, invalid bound: \"%q\" ", boundsString, boundBStr) + return nil, fmt.Errorf("invalid bound: %q", upperBoundStr) } - if boundAInt > boundBInt { + if lowerBoundInt > upperBoundInt { return nil, fmt.Errorf("invalid range %q, lower bound cannot be greater than upper bound", boundsString) } - idRange.Lower = boundAInt - idRange.Upper = boundBInt + idRange.Lower = lowerBoundInt + idRange.Upper = upperBoundInt } return &idRange, nil diff --git a/drivers/shared/validators/validators_test.go b/drivers/shared/validators/validators_test.go index e9cc7d55a..beb34c2a8 100644 --- a/drivers/shared/validators/validators_test.go +++ b/drivers/shared/validators/validators_test.go @@ -23,36 +23,32 @@ var invalidRangeEmpty = "1-100,,200-300" func Test_IDRangeValid(t *testing.T) { testCases := []struct { - name string - idRange string - expectedPass bool - expectedErr string + name string + idRange string + expectedErr string }{ - {name: "standard-range-is-valid", idRange: validRange, expectedPass: true}, - {name: "same-number-for-both-bounds-is-valid", idRange: validRangeSingle, expectedPass: true}, - {name: "lower-higher-than-upper-is-invalid", idRange: invalidRangeFlipped, expectedPass: false, expectedErr: flippedBoundsMessage}, - {name: "missing-lower-is-invalid", idRange: invalidRangeSubstring, expectedPass: false, expectedErr: invalidBound}, - {name: "missing-higher-is-invalid", idRange: invalidRangeEmpty, expectedPass: false, expectedErr: invalidBound}, + {name: "standard-range-is-valid", idRange: validRange}, + {name: "same-number-for-both-bounds-is-valid", idRange: validRangeSingle}, + {name: "lower-higher-than-upper-is-invalid", idRange: invalidRangeFlipped, expectedErr: flippedBoundsMessage}, + {name: "missing-lower-is-invalid", idRange: invalidRangeSubstring, expectedErr: invalidBound}, + {name: "missing-higher-is-invalid", idRange: invalidRangeEmpty, expectedErr: invalidBound}, } for _, tc := range testCases { tc := tc t.Run(tc.name, func(t *testing.T) { - err := IDRangeValid("uid", tc.idRange) - if tc.expectedPass { + err := ParseIdRange("uid", tc.idRange) + if tc.expectedErr == "" { must.NoError(t, err) } else { - if err == nil { - t.Errorf("expected error, got nil") - } else { - must.StrContains(t, err.Error(), tc.expectedErr) - } + must.Error(t, err) + must.ErrorContains(t, err, tc.expectedErr) } }) } } -func Test_UserInRange(t *testing.T) { +func Test_HasValidIds(t *testing.T) { emptyRange := "" invalidRange := "foo" @@ -62,17 +58,16 @@ func Test_UserInRange(t *testing.T) { gidRanges string uid string gid string - expectedPass bool expectedErr string userLookupFunc userLookupFn }{ - {name: "no-ranges-are-valid", uidRanges: emptyRange, gidRanges: emptyRange, expectedPass: true}, - {name: "uid-and-gid-outside-of-ranges-valid", uidRanges: validRange, gidRanges: validRange, expectedPass: true}, - {name: "uid-in-one-of-ranges-is-invalid", uidRanges: validRange, gidRanges: validRange, uid: "50", expectedPass: false, expectedErr: "running as uid 50 is disallowed"}, - {name: "gid-in-one-of-ranges-is-invalid", uidRanges: validRange, gidRanges: validRange, gid: "50", expectedPass: false, expectedErr: "running as gid 50 is disallowed"}, - {name: "invalid-uid-range-throws-error", uidRanges: invalidRange, gidRanges: validRange, expectedPass: false, expectedErr: "invalid denied_host_uids value"}, - {name: "invalid-gid-range-throws-error", uidRanges: validRange, gidRanges: invalidRange, expectedPass: false, expectedErr: "invalid denied_host_gids value"}, - {name: "string-uid-throws-error", uid: "banana", expectedPass: false, expectedErr: "unable to convert userid banana to integer"}, + {name: "no-ranges-are-valid", uidRanges: emptyRange, gidRanges: emptyRange}, + {name: "uid-and-gid-outside-of-ranges-valid", uidRanges: validRange, gidRanges: validRange}, + {name: "uid-in-one-of-ranges-is-invalid", uidRanges: validRange, gidRanges: validRange, uid: "50", expectedErr: "running as uid 50 is disallowed"}, + {name: "gid-in-one-of-ranges-is-invalid", uidRanges: validRange, gidRanges: validRange, gid: "50", expectedErr: "running as gid 50 is disallowed"}, + {name: "invalid-uid-range-throws-error", uidRanges: invalidRange, gidRanges: validRange, expectedErr: "invalid denied_host_uids value"}, + {name: "invalid-gid-range-throws-error", uidRanges: validRange, gidRanges: invalidRange, expectedErr: "invalid denied_host_gids value"}, + {name: "string-uid-throws-error", uid: "banana", expectedErr: "unable to convert userid banana to integer"}, } for _, tc := range testCases { @@ -95,15 +90,12 @@ func Test_UserInRange(t *testing.T) { return defaultUserToReturn, nil } - err := UserInRange(getUserFn, "username", tc.uidRanges, tc.gidRanges) - if tc.expectedPass { + err := HasValidIds(getUserFn, "username", tc.uidRanges, tc.gidRanges) + if tc.expectedErr == "" { must.NoError(t, err) } else { - if err == nil { - t.Errorf("expected error, got nil") - } else { - must.StrContains(t, err.Error(), tc.expectedErr) - } + must.Error(t, err) + must.ErrorContains(t, err, tc.expectedErr) } }) } diff --git a/website/content/docs/drivers/exec.mdx b/website/content/docs/drivers/exec.mdx index e1be18a65..5bcd3df08 100644 --- a/website/content/docs/drivers/exec.mdx +++ b/website/content/docs/drivers/exec.mdx @@ -178,11 +178,11 @@ able to make use of IPC features, like sending unexpected POSIX signals. "net_bind_service", "setfcap", "setgid", "setpcap", "setuid", "sys_chroot"] ``` -which is modeled after the capabilities allowed by [docker by default][docker_caps] -(without [`NET_RAW`][no_net_raw]). Allows the operator to control which capabilities -can be obtained by tasks using [`cap_add`][cap_add] and [`cap_drop`][cap_drop] options. -Supports the value `"all"` as a shortcut for allow-listing all capabilities supported -by the operating system. + which is modeled after the capabilities allowed by [docker by default][docker_caps] + (without [`NET_RAW`][no_net_raw]). Allows the operator to control which capabilities + can be obtained by tasks using [`cap_add`][cap_add] and [`cap_drop`][cap_drop] options. + Supports the value `"all"` as a shortcut for allow-listing all capabilities supported + by the operating system. !> **Warning:** Allowing more capabilities beyond the default may lead to undesirable consequences, including untrusted tasks being able to compromise the @@ -191,7 +191,7 @@ host system. - `denied_host_uids` - (Optional) Specifies a comma-separated list of host uids to deny. Ranges can be specified by using a hyphen separating the two inclusive ends. If a "user" value is specified in task configuration and that user has a user id in - the given ranges, the task will error before starting. This will not be run on Windows + the given ranges, the task will error before starting. This will not be checked on Windows clients. ```hcl @@ -204,7 +204,7 @@ config { deny. Ranges can be specified by using a hyphen separating the two inclusive ends. If a "user" value is specified in task configuration and that user has is part of any groups with gid's in the specified ranges, the task will error before - starting. This will not be run on Windows clients. + starting. This will not be checked on Windows clients. ```hcl config { diff --git a/website/content/docs/drivers/raw_exec.mdx b/website/content/docs/drivers/raw_exec.mdx index d289fac22..364d50ba1 100644 --- a/website/content/docs/drivers/raw_exec.mdx +++ b/website/content/docs/drivers/raw_exec.mdx @@ -133,7 +133,7 @@ client { - `denied_host_uids` - (Optional) Specifies a comma-separated list of host uids to deny. Ranges can be specified by using a hyphen separating the two inclusive ends. If a "user" value is specified in task configuration and that user has a user id in - the given ranges, the task will error before starting. This will not be run on Windows + the given ranges, the task will error before starting. This will not be checked on Windows clients. ```hcl @@ -146,7 +146,7 @@ config { deny. Ranges can be specified by using a hyphen separating the two inclusive ends. If a "user" value is specified in task configuration and that user has is part of any groups with gid's in the specified ranges, the task will error before - starting. This will not be run on Windows clients. + starting. This will not be checked on Windows clients. ```hcl config {