Only parsing id ranges once

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

drivers/exec/driver_test.go

@@ -887,32 +887,29 @@ func TestDriver_Config_validate(t *testing.T) {
 			{uidRanges: "", gidRanges: "10-1", errorStr: &invalidGidRange},
 		} {
 
-			validationErr := (&Config{
+			err := (&Config{
 				DefaultModePID: "private",
 				DefaultModeIPC: "private",
 				DeniedHostUids: tc.uidRanges,
 				DeniedHostGids: tc.gidRanges,
 			}).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())
 		}
 	})
 }

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

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) {

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)
 }

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))
 	}
 }

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
 }