From e1c226e63347eaea8984f5e779d18e8a45683fcb Mon Sep 17 00:00:00 2001 From: Mike Nomitch Date: Tue, 12 Mar 2024 10:06:18 -0700 Subject: [PATCH] Restructuring IDRange --- drivers/exec/driver.go | 28 +++++++++----------- drivers/exec/driver_test.go | 13 ++++----- drivers/rawexec/driver.go | 5 ++-- drivers/rawexec/driver_unix_test.go | 6 ++--- drivers/shared/validators/validators.go | 18 ++++++++----- drivers/shared/validators/validators_test.go | 13 +++++---- nomad/structs/structs.go | 6 ----- 7 files changed, 41 insertions(+), 48 deletions(-) diff --git a/drivers/exec/driver.go b/drivers/exec/driver.go index 7f17ff04f..6ef33bf26 100644 --- a/drivers/exec/driver.go +++ b/drivers/exec/driver.go @@ -24,7 +24,6 @@ import ( "github.com/hashicorp/nomad/helper/pluginutils/loader" "github.com/hashicorp/nomad/helper/pointer" "github.com/hashicorp/nomad/helper/users" - "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/plugins/base" "github.com/hashicorp/nomad/plugins/drivers" "github.com/hashicorp/nomad/plugins/drivers/fsisolation" @@ -169,10 +168,10 @@ type Config struct { DeniedHostGidsStr string `codec:"denied_host_gids"` // DeniedHostUids configures which host uids are disallowed - DeniedHostUids []structs.IDRange `codec:"-"` + DeniedHostUids []validators.IDRange `codec:"-"` // DeniedHostGids configures which host gids are disallowed - DeniedHostGids []structs.IDRange `codec:"-"` + DeniedHostGids []validators.IDRange `codec:"-"` } func (c *Config) validate() error { @@ -264,10 +263,9 @@ func (tc *TaskConfig) validate() error { } func (tc *TaskConfig) validateUserIds(cfg *drivers.TaskConfig, driverConfig *Config) error { - usernameToLookup := getUsername(cfg) - user, err := users.Lookup(usernameToLookup) + user, err := users.Lookup(cfg.User) if err != nil { - return fmt.Errorf("failed to identify user %q: %w", usernameToLookup, err) + return fmt.Errorf("failed to identify user %q: %w", cfg.User, err) } return validators.HasValidIds(user, driverConfig.DeniedHostUids, driverConfig.DeniedHostGids) @@ -483,6 +481,13 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive return nil, nil, fmt.Errorf("failed driver config validation: %v", err) } + if cfg.User == "" { + fmt.Println("User is not empty, setting to nobody") + cfg.User = "nobody" + } else { + fmt.Printf("User is not empty, user is %s\n", cfg.User) + } + if err := driverConfig.validateUserIds(cfg, &d.config); err != nil { return nil, nil, fmt.Errorf("failed host user validation: %v", err) } @@ -506,7 +511,7 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive return nil, nil, fmt.Errorf("failed to create executor: %v", err) } - user := getUsername(cfg) + user := cfg.User if cfg.DNS != nil { dnsMount, err := resolvconf.GenerateDNSMount(cfg.TaskDir().Dir, cfg.DNS) @@ -736,12 +741,3 @@ func (d *Driver) ExecTaskStreamingRaw(ctx context.Context, return handle.exec.ExecStreaming(ctx, command, tty, stream) } - -func getUsername(cfg *drivers.TaskConfig) string { - username := "nobody" - if cfg.User != "" { - username = cfg.User - } - - return username -} diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go index 8906e58b2..6fe7c7bce 100644 --- a/drivers/exec/driver_test.go +++ b/drivers/exec/driver_test.go @@ -23,6 +23,7 @@ import ( "github.com/hashicorp/nomad/client/lib/numalib" ctestutils "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/drivers/shared/executor" + "github.com/hashicorp/nomad/drivers/shared/validators" "github.com/hashicorp/nomad/helper/pluginutils/hclutils" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/helper/testtask" @@ -916,13 +917,13 @@ func TestDriver_TaskConfig_validateUserIds(t *testing.T) { nobodyUid, _, _, err := users.LookupUnix("nobody") require.NoError(t, err) - allowAll := []structs.IDRange{} - denyCurrent := []structs.IDRange{{Lower: uint64(currentUid), Upper: uint64(currentUid)}} - denyNobody := []structs.IDRange{{Lower: uint64(nobodyUid), Upper: uint64(nobodyUid)}} + allowAll := []validators.IDRange{} + denyCurrent := []validators.IDRange{{Lower: uint64(currentUid), Upper: uint64(currentUid)}} + denyNobody := []validators.IDRange{{Lower: uint64(nobodyUid), Upper: uint64(nobodyUid)}} configAllowCurrent := Config{DeniedHostUids: allowAll} configDenyCurrent := Config{DeniedHostUids: denyCurrent} configDenyAnonymous := Config{DeniedHostUids: denyNobody} - driverConfigNoUserSpecified := drivers.TaskConfig{} + driverConfigNoUserSpecified := drivers.TaskConfig{User: "nobody"} 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) @@ -939,9 +940,9 @@ func TestDriver_TaskConfig_validateUserIds(t *testing.T) { } { err := (&TaskConfig{}).validateUserIds(&tc.driverConfig, &tc.config) if tc.expectedErr == "" { - require.NoError(t, err) + must.NoError(t, err) } else { - require.ErrorContains(t, err, tc.expectedErr) + must.ErrorContains(t, err, tc.expectedErr) } } } diff --git a/drivers/rawexec/driver.go b/drivers/rawexec/driver.go index cae724f42..5348d1fd7 100644 --- a/drivers/rawexec/driver.go +++ b/drivers/rawexec/driver.go @@ -21,7 +21,6 @@ import ( "github.com/hashicorp/nomad/helper/pluginutils/hclutils" "github.com/hashicorp/nomad/drivers/shared/validators" "github.com/hashicorp/nomad/helper/pluginutils/loader" - "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/plugins/base" "github.com/hashicorp/nomad/plugins/drivers" "github.com/hashicorp/nomad/plugins/drivers/fsisolation" @@ -148,10 +147,10 @@ type Config struct { DeniedHostGidsStr string `codec:"denied_host_gids"` // DeniedHostUids configures which host uids are disallowed - DeniedHostUids []structs.IDRange + DeniedHostUids []validators.IDRange // DeniedHostGids configures which host gids are disallowed - DeniedHostGids []structs.IDRange + DeniedHostGids []validators.IDRange } // TaskConfig is the driver configuration of a task within a job diff --git a/drivers/rawexec/driver_unix_test.go b/drivers/rawexec/driver_unix_test.go index e9e5dfc40..792e41db1 100644 --- a/drivers/rawexec/driver_unix_test.go +++ b/drivers/rawexec/driver_unix_test.go @@ -22,10 +22,10 @@ import ( "github.com/hashicorp/nomad/ci" clienttestutil "github.com/hashicorp/nomad/client/testutil" + "github.com/hashicorp/nomad/drivers/shared/validators" "github.com/hashicorp/nomad/helper/testtask" "github.com/hashicorp/nomad/helper/users" "github.com/hashicorp/nomad/helper/uuid" - "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/plugins/base" basePlug "github.com/hashicorp/nomad/plugins/base" "github.com/hashicorp/nomad/plugins/drivers" @@ -558,8 +558,8 @@ func TestRawExec_Validate(t *testing.T) { currentUserErrStr := fmt.Sprintf("running as uid %d is disallowed", currentUid) - allowAll := []structs.IDRange{} - denyCurrent := []structs.IDRange{{Lower: currentUid, Upper: currentUid}} + allowAll := []validators.IDRange{} + denyCurrent := []validators.IDRange{{Lower: currentUid, Upper: currentUid}} configAllowCurrent := Config{DeniedHostUids: allowAll} configDenyCurrent := Config{DeniedHostUids: denyCurrent} driverConfigNoUserSpecified := drivers.TaskConfig{} diff --git a/drivers/shared/validators/validators.go b/drivers/shared/validators/validators.go index f48ab759c..7cf0d6991 100644 --- a/drivers/shared/validators/validators.go +++ b/drivers/shared/validators/validators.go @@ -8,13 +8,17 @@ import ( "os/user" "strconv" "strings" - - "github.com/hashicorp/nomad/nomad/structs" ) +// IDRange defines a range of uids or gids (to eventually restrict) +type IDRange struct { + Lower uint64 `codec:"from"` + Upper uint64 `codec:"to"` +} + // ParseIdRange is used to ensure that the configuration for ID ranges is valid. -func ParseIdRange(rangeType string, deniedRanges string) ([]structs.IDRange, error) { - var idRanges []structs.IDRange +func ParseIdRange(rangeType string, deniedRanges string) ([]IDRange, error) { + var idRanges []IDRange parts := strings.Split(deniedRanges, ",") // exit early if empty string @@ -36,7 +40,7 @@ func ParseIdRange(rangeType string, deniedRanges string) ([]structs.IDRange, err // HasValidIds is used when running a task to ensure the // given user is in the ID range defined in the task config -func HasValidIds(user *user.User, deniedHostUIDs, deniedHostGIDs []structs.IDRange) error { +func HasValidIds(user *user.User, deniedHostUIDs, deniedHostGIDs []IDRange) error { uid, err := strconv.ParseUint(user.Uid, 10, 32) if err != nil { return fmt.Errorf("unable to convert userid %s to integer", user.Uid) @@ -78,10 +82,10 @@ func HasValidIds(user *user.User, deniedHostUIDs, deniedHostGIDs []structs.IDRan return nil } -func parseRangeString(boundsString string) (*structs.IDRange, error) { +func parseRangeString(boundsString string) (*IDRange, error) { uidDenyRangeParts := strings.Split(boundsString, "-") - var idRange structs.IDRange + var idRange IDRange switch len(uidDenyRangeParts) { case 0: diff --git a/drivers/shared/validators/validators_test.go b/drivers/shared/validators/validators_test.go index 2a5e4813b..9c1eb84b6 100644 --- a/drivers/shared/validators/validators_test.go +++ b/drivers/shared/validators/validators_test.go @@ -7,7 +7,6 @@ import ( "os/user" "testing" - "github.com/hashicorp/nomad/nomad/structs" "github.com/shoenig/test/must" ) @@ -47,23 +46,23 @@ func Test_IDRangeValid(t *testing.T) { } func Test_HasValidIds(t *testing.T) { - var validRange = structs.IDRange{ + var validRange = IDRange{ Lower: 1, Upper: 100, } - var validRangeSingle = structs.IDRange{ + var validRangeSingle = IDRange{ Lower: 1, Upper: 1, } - emptyRanges := []structs.IDRange{} - validRangesList := []structs.IDRange{validRange, validRangeSingle} + emptyRanges := []IDRange{} + validRangesList := []IDRange{validRange, validRangeSingle} testCases := []struct { name string - uidRanges []structs.IDRange - gidRanges []structs.IDRange + uidRanges []IDRange + gidRanges []IDRange uid string gid string expectedErr string diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index bbdbabd73..ee505b460 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -13756,9 +13756,3 @@ func NewRpcError(err error, code *int64) *RpcError { func (r *RpcError) Error() string { return r.Message } - -// IDRange defines a range of uids or gids (to eventually restrict) -type IDRange struct { - Lower uint64 `codec:"from"` - Upper uint64 `codec:"to"` -}