Small updates throughout based on feedback

Signed-off-by: Matthew Heon <[email protected]>

configs/config.go

@@ -102,7 +102,7 @@ type Config struct {
 	ReadonlyPaths []string `json:"readonly_paths"`
 
 	// SeccompConfig holds information on system calls to be restricted in the container
-	SeccompConfig seccomp.SeccompConfig `json:"seccomp_config,omitempty"`
+	SeccompConfig seccomp.Config `json:"seccomp_config,omitempty"`
 }
 
 // Gets the root uid for the process on host which could be non-zero

security/seccomp/seccomp.go → security/seccomp/seccomp_linux.go

@@ -12,22 +12,33 @@ import (
 )
 
 var (
-	actAllow libseccomp.ScmpAction = libseccomp.ActAllow
-	actDeny  libseccomp.ScmpAction = libseccomp.ActErrno.SetReturnCode(int16(syscall.EPERM))
+	actAllow = libseccomp.ActAllow
+	actDeny  = libseccomp.ActErrno.SetReturnCode(int16(syscall.EPERM))
 )
 
 // Filters given syscalls in a container, preventing them from being used
 // Started in the container init process, and carried over to all child processes
-func InitSeccomp(config SeccompConfig) error {
+func InitSeccomp(config *Config) error {
+	if config == nil {
+		return fmt.Errorf("Cannot initialize Seccomp - null config passed!")
+	}
+
 	if !config.Enable {
 		return nil
 	}
 
-	var defaultAction libseccomp.ScmpAction
+	// For a whitelist, deny by default, allow matching calls
+	// For a blacklist, the reverse
+	var (
+		defaultAction libseccomp.ScmpAction
+		matchAction   libseccomp.ScmpAction
+	)
 	if config.WhitelistToggle {
 		defaultAction = actDeny
+		matchAction = actAllow
 	} else {
 		defaultAction = actAllow
+		matchAction = actDeny
 	}
 
 	filter, err := libseccomp.NewFilter(defaultAction)
@@ -36,7 +47,7 @@ func InitSeccomp(config SeccompConfig) error {
 	}
 
 	// Unset no new privs bit
-	if err = filter.SetNoNewPrivsBit(false); err != nil {
+	if err := filter.SetNoNewPrivsBit(false); err != nil {
 		return fmt.Errorf("Error setting no new privileges: %s", err)
 	}
 
@@ -54,7 +65,11 @@ func InitSeccomp(config SeccompConfig) error {
 
 	// Add a rule for each syscall
 	for _, call := range config.Syscalls {
-		if err = blockCall(config.WhitelistToggle, filter, call); err != nil {
+		if call == nil {
+			return fmt.Errorf("Encountered nil syscall while initializing Seccomp!")
+		}
+
+		if err = matchCall(matchAction, filter, call); err != nil {
 			return err
 		}
 	}
@@ -92,7 +107,12 @@ func compareOpFromString(op string) (libseccomp.ScmpCompareOp, error) {
 	}
 }
 
-func blockCall(isWhitelist bool, filter *libseccomp.ScmpFilter, call BlockedSyscall) error {
+// Add a rule to match a single syscall
+func matchCall(action libseccomp.ScmpAction, filter *libseccomp.ScmpFilter, call *BlockedSyscall) error {
+	if call == nil || filter == nil {
+		return fmt.Errorf("Nil passed to matchCall!")
+	}
+
 	if len(call.Name) == 0 {
 		return fmt.Errorf("Empty string is not a valid syscall!")
 	}
@@ -103,19 +123,13 @@ func blockCall(isWhitelist bool, filter *libseccomp.ScmpFilter, call BlockedSysc
 		return nil
 	}
 
-	var action libseccomp.ScmpAction
-
-	if isWhitelist {
-		action = actAllow
-	} else {
-		action = actDeny
-	}
-
+	// Unconditional match - just add the rule
 	if len(call.Conditions) == 0 {
 		if err = filter.AddRule(callNum, action); err != nil {
 			return err
 		}
 	} else {
+		// Conditional match - convert the matches into library format
 		conditions := []libseccomp.ScmpCondition{}
 
 		for _, cond := range call.Conditions {

security/seccomp/seccomp_unsupported.go

@@ -0,0 +1,8 @@
+// +build !linux !cgo !seccomp
+
+package seccomp
+
+// Seccomp not supported, do nothing
+func InitSeccomp(config *Config) error {
+	return nil
+}

security/seccomp/types.go

@@ -1,20 +1,48 @@
 package seccomp
 
+// A condition on which to match a syscall
+// The condition is considered matched if the following boolean expression
+// is true: (Value of Syscall Argument) Operator ValueOne
+// As an example, using an operator of > and value of 2 would compare
+// whether the value of a syscall argument was greater than 2
 type SyscallCondition struct {
-	Argument uint   `json:"argument"`
+	// Which argument of the syscall to inspect. Valid values are 0-6
+	Argument uint `json:"argument"`
+
+	// Operator to compare with
+	// Valid values are <, <=, ==, >=, >, and |= (masked equality)
 	Operator string `json:"operator"`
+
+	// Value to compare given argument against
 	ValueOne uint64 `json:"value_one"`
+
+	// Presently only used in masked equality - mask of bits to compare
 	ValueTwo uint64 `json:"value_two,omitempty"`
 }
 
+// An individual syscall to be blocked by Libseccomp
 type BlockedSyscall struct {
-	Name       string             `json:"name,"`
+	// Name of the syscall
+	Name string `json:"name"`
+
+	// Conditions on which to match the syscall.
+	// Can be omitted for an unconditional match.
 	Conditions []SyscallCondition `json:"conditions,omitempty"`
 }
 
-type SeccompConfig struct {
-	Enable          bool             `json:"enable"`
-	WhitelistToggle bool             `json:"whitelist_toggle"`
-	Architectures   []string         `json:"architectures,omitempty"`
-	Syscalls        []BlockedSyscall `json:"syscalls"`
+// Overall configuration for Seccomp support
+type Config struct {
+	// Enable/disable toggle for Libseccomp
+	Enable bool `json:"enable"`
+
+	// Toggle whitelisting on. Default is blacklisting - deny given syscalls.
+	// if set to true, this reverses this behavior - permit only the given syscalls
+	WhitelistToggle bool `json:"whitelist_toggle"`
+
+	// Additional architectures to support in the container.
+	// The installed kernel's default architecture is always supported
+	Architectures []string `json:"architectures,omitempty"`
+
+	// A list of syscalls to deny (or permit, if WhitelistToggle is set)
+	Syscalls []*BlockedSyscall `json:"syscalls"`
 }

setns_init_linux.go

@@ -21,8 +21,10 @@ func (l *linuxSetnsInit) Init() error {
 	if err := setupRlimits(l.config.Config); err != nil {
 		return err
 	}
-	if err := seccomp.InitSeccomp(l.config.Config.SeccompConfig); err != nil {
-		return err
+	if l.config.Config.SeccompConfig.Enable {
+		if err := seccomp.InitSeccomp(&l.config.Config.SeccompConfig); err != nil {
+			return err
+		}
 	}
 	if err := finalizeNamespace(l.config); err != nil {
 		return err

standard_init_linux.go

@@ -78,8 +78,10 @@ func (l *linuxStandardInit) Init() error {
 	if err != nil {
 		return err
 	}
-	if err := seccomp.InitSeccomp(l.config.Config.SeccompConfig); err != nil {
-		return err
+	if l.config.Config.SeccompConfig.Enable {
+		if err := seccomp.InitSeccomp(&l.config.Config.SeccompConfig); err != nil {
+			return err
+		}
 	}
 	if err := finalizeNamespace(l.config); err != nil {
 		return err