diff --git a/runsc/config/config.go b/runsc/config/config.go index 1b403277f..3f34d5639 100644 --- a/runsc/config/config.go +++ b/runsc/config/config.go @@ -31,9 +31,11 @@ import ( // Follow these steps to add a new flag: // 1. Create a new field in Config. // 2. Add a field tag with the flag name -// 3. Register a new flag in flags.go, with name and description +// 3. Register a new flag in flags.go, with same name and add a description // 4. Add any necessary validation into validate() // 5. If adding an enum, follow the same pattern as FileAccessType +// 6. Evaluate if the flag can be changed with OCI annotations. See +// overrideAllowlist for more details // type Config struct { // RootDir is the runtime root directory. diff --git a/runsc/config/config_test.go b/runsc/config/config_test.go index 57c241c86..420df1868 100644 --- a/runsc/config/config_test.go +++ b/runsc/config/config_test.go @@ -305,3 +305,60 @@ func TestOverrideError(t *testing.T) { }) } } + +func TestOverrideAllowlist(t *testing.T) { + c, err := NewFromFlags() + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + flag string + value string + error string + }{ + { + flag: "debug", + value: "true", + }, + { + flag: "debug", + value: "123", + error: "error setting flag", + }, + { + flag: "oci-seccomp", + value: "true", + }, + { + flag: "oci-seccomp", + value: "false", + error: `disabling "oci-seccomp" requires flag`, + }, + { + flag: "oci-seccomp", + value: "123", + error: "invalid syntax", + }, + { + flag: "profile", + value: "true", + error: "flag override disabled", + }, + { + flag: "profile", + value: "123", + error: "flag override disabled", + }, + } { + t.Run(tc.flag, func(t *testing.T) { + err := c.Override(tc.flag, tc.value) + if len(tc.error) == 0 { + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + } else if err == nil || !strings.Contains(err.Error(), tc.error) { + t.Errorf("Override(%q, %q) wrong error: %v", tc.flag, tc.value, err) + } + }) + } +} diff --git a/runsc/config/flags.go b/runsc/config/flags.go index 5678cd6b6..f5ca6b868 100644 --- a/runsc/config/flags.go +++ b/runsc/config/flags.go @@ -81,8 +81,8 @@ func RegisterFlags() { flag.Bool("verity", false, "specifies whether a verity file system will be mounted.") flag.Bool("fsgofer-host-uds", false, "allow the gofer to mount Unix Domain Sockets.") flag.Bool("vfs2", true, "enables VFSv2. This uses the new VFS layer that is faster than the previous one.") - flag.Bool("fuse", false, "TEST ONLY; use while FUSE in VFSv2 is landing. This allows the use of the new experimental FUSE filesystem.") - flag.Bool("lisafs", false, "Enables lisafs protocol instead of 9P. This is only effective with VFS2.") + flag.Bool("fuse", false, "TEST ONLY; This allows the use of the new experimental FUSE filesystem. Only works with VFS2.") + flag.Bool("lisafs", false, "Enables lisafs protocol instead of 9P. Only works with VFS2.") flag.Bool("cgroupfs", false, "Automatically mount cgroupfs.") flag.Bool("ignore-cgroups", false, "don't configure cgroups.") @@ -103,6 +103,33 @@ func RegisterFlags() { }) } +// overrideAllowlist lists all flags that can be changed using OCI +// annotations without an administrator setting `--allow-flag-override` on the +// runtime. Flags in this list can be set by container authors and should not +// make the sandbox less secure. +var overrideAllowlist = map[string]struct { + check func(name string, value string) error +}{ + "debug": {}, + "strace": {}, + "strace-syscalls": {}, + "strace-log-size": {}, + + "oci-seccomp": {check: checkOciSeccomp}, +} + +// checkOciSeccomp ensures that seccomp can be enabled but not disabled. +func checkOciSeccomp(name string, value string) error { + enable, err := strconv.ParseBool(value) + if err != nil { + return err + } + if !enable { + return fmt.Errorf("disabling %q requires flag %q to be enabled", name, "allow-flag-override") + } + return nil +} + // NewFromFlags creates a new Config with values coming from command line flags. func NewFromFlags() (*Config, error) { conf := &Config{} @@ -168,10 +195,6 @@ func (c *Config) ToFlags() []string { // Override writes a new value to a flag. func (c *Config) Override(name string, value string) error { - if !c.AllowFlagOverride { - return fmt.Errorf("flag override disabled, use --allow-flag-override to enable it") - } - obj := reflect.ValueOf(c).Elem() st := obj.Type() for i := 0; i < st.NumField(); i++ { @@ -186,6 +209,9 @@ func (c *Config) Override(name string, value string) error { // Flag must exist if there is a field match above. panic(fmt.Sprintf("Flag %q not found", name)) } + if err := c.isOverrideAllowed(name, value); err != nil { + return fmt.Errorf("error setting flag %s=%q: %w", name, value, err) + } // Use flag to convert the string value to the underlying flag type, using // the same rules as the command-line for consistency. @@ -201,6 +227,23 @@ func (c *Config) Override(name string, value string) error { return fmt.Errorf("flag %q not found. Cannot set it to %q", name, value) } +func (c *Config) isOverrideAllowed(name string, value string) error { + if c.AllowFlagOverride { + return nil + } + // If the global override flag is not enabled, check if individual flag is + // safe to apply. + if allow, ok := overrideAllowlist[name]; ok { + if allow.check != nil { + if err := allow.check(name, value); err != nil { + return err + } + } + return nil + } + return fmt.Errorf("flag override disabled, use --allow-flag-override to enable it") +} + func getVal(field reflect.Value) string { if str, ok := field.Addr().Interface().(fmt.Stringer); ok { return str.String()