From 56fd3714d82354a0df39889fb0f28a828b3d1067 Mon Sep 17 00:00:00 2001 From: Kailun Qin Date: Fri, 3 Sep 2021 07:28:48 -0400 Subject: [PATCH] Address comments * use landlock.AccessFSSet type directly * remove non-linux files * some minor updates Signed-off-by: Kailun Qin --- libcontainer/configs/config.go | 34 +++------------- libcontainer/landlock/config.go | 40 ++++++++++--------- .../{landlock_linux.go => landlock.go} | 23 ++++------- libcontainer/landlock/landlock_unsupported.go | 19 --------- libcontainer/specconv/spec_linux.go | 18 ++++----- libcontainer/standard_init_linux.go | 2 +- 6 files changed, 44 insertions(+), 92 deletions(-) rename libcontainer/landlock/{landlock_linux.go => landlock.go} (70%) delete mode 100644 libcontainer/landlock/landlock_unsupported.go diff --git a/libcontainer/configs/config.go b/libcontainer/configs/config.go index 6595fc7fa75..498bf840af2 100644 --- a/libcontainer/configs/config.go +++ b/libcontainer/configs/config.go @@ -9,6 +9,7 @@ import ( "github.com/sirupsen/logrus" + "github.com/landlock-lsm/go-landlock/landlock" "github.com/opencontainers/runc/libcontainer/devices" "github.com/opencontainers/runtime-spec/specs-go" ) @@ -87,7 +88,7 @@ type Landlock struct { // Ruleset identifies a set of rules (i.e., actions on objects) that need to be handled in Landlock. type Ruleset struct { - HandledAccessFS AccessFS `json:"handledAccessFS"` + HandledAccessFS landlock.AccessFSSet `json:"handledAccessFS"` } // Rules represents the security policies (i.e., actions allowed on objects) in Landlock. @@ -96,35 +97,12 @@ type Rules struct { } // RulePathBeneath defines the file-hierarchy typed rule that grants the access rights specified by -// `AllowedAccess` to the file hierarchies under the given `Paths` in Landlock. +// AllowedAccess to the file hierarchies under the given Paths in Landlock. type RulePathBeneath struct { - AllowedAccess AccessFS `json:"allowedAccess"` - Paths []string `json:"paths"` + AllowedAccess landlock.AccessFSSet `json:"allowedAccess"` + Paths []string `json:"paths"` } -// AccessFS is taken upon ruleset and rule setup in Landlock. -type AccessFS uint64 - -// Landlock access rights for FS. -// -// Please see the full documentation at -// https://www.kernel.org/doc/html/latest/userspace-api/landlock.html#access-rights. -const ( - Execute AccessFS = (1 << 0) - WriteFile AccessFS = (1 << 1) - ReadFile AccessFS = (1 << 2) - ReadDir AccessFS = (1 << 3) - RemoveDir AccessFS = (1 << 4) - RemoveFile AccessFS = (1 << 5) - MakeChar AccessFS = (1 << 6) - MakeDir AccessFS = (1 << 7) - MakeReg AccessFS = (1 << 8) - MakeSock AccessFS = (1 << 9) - MakeFifo AccessFS = (1 << 10) - MakeBlock AccessFS = (1 << 11) - MakeSym AccessFS = (1 << 12) -) - // TODO Windows. Many of these fields should be factored out into those parts // which are common across platforms, and those which are platform specific. @@ -255,7 +233,7 @@ type Config struct { RootlessCgroups bool `json:"rootless_cgroups,omitempty"` // Landlock specifies the Landlock unprivileged access control settings for the container process. - // `noNewPrivileges` must be enabled to use Landlock. + // NoNewPrivileges must be enabled to use Landlock. Landlock *Landlock `json:"landlock,omitempty"` } diff --git a/libcontainer/landlock/config.go b/libcontainer/landlock/config.go index c91ef4d3720..b2b4302b0ec 100644 --- a/libcontainer/landlock/config.go +++ b/libcontainer/landlock/config.go @@ -3,28 +3,32 @@ package landlock import ( "fmt" - "github.com/opencontainers/runc/libcontainer/configs" + "github.com/landlock-lsm/go-landlock/landlock" + ll "github.com/landlock-lsm/go-landlock/landlock/syscall" ) -var accessFSs = map[string]configs.AccessFS{ - "execute": configs.Execute, - "write_file": configs.WriteFile, - "read_file": configs.ReadFile, - "read_dir": configs.ReadDir, - "remove_dir": configs.RemoveDir, - "remove_file": configs.RemoveFile, - "make_char": configs.MakeChar, - "make_dir": configs.MakeDir, - "make_reg": configs.MakeReg, - "make_sock": configs.MakeSock, - "make_fifo": configs.MakeFifo, - "make_block": configs.MakeBlock, - "make_sym": configs.MakeSym, +var accessFSSets = map[string]landlock.AccessFSSet{ + "execute": ll.AccessFSExecute, + "write_file": ll.AccessFSWriteFile, + "read_file": ll.AccessFSReadFile, + "read_dir": ll.AccessFSReadDir, + "remove_dir": ll.AccessFSRemoveDir, + "remove_file": ll.AccessFSRemoveFile, + "make_char": ll.AccessFSMakeChar, + "make_dir": ll.AccessFSMakeDir, + "make_reg": ll.AccessFSMakeReg, + "make_sock": ll.AccessFSMakeSock, + "make_fifo": ll.AccessFSMakeFifo, + "make_block": ll.AccessFSMakeBlock, + "make_sym": ll.AccessFSMakeSym, } -// ConvertStringToAccessFS converts a string into a Landlock access right. -func ConvertStringToAccessFS(in string) (configs.AccessFS, error) { - if access, ok := accessFSs[in]; ok { +// ConvertStringToAccessFSSet converts a string into a go-landlock AccessFSSet +// access right. +// This gives more explicit control over the mapping between the permitted +// values in the spec and the ones supported in go-landlock library. +func ConvertStringToAccessFSSet(in string) (landlock.AccessFSSet, error) { + if access, ok := accessFSSets[in]; ok { return access, nil } return 0, fmt.Errorf("string %s is not a valid access right for landlock", in) diff --git a/libcontainer/landlock/landlock_linux.go b/libcontainer/landlock/landlock.go similarity index 70% rename from libcontainer/landlock/landlock_linux.go rename to libcontainer/landlock/landlock.go index 158f23cc126..c15ef8a4c2b 100644 --- a/libcontainer/landlock/landlock_linux.go +++ b/libcontainer/landlock/landlock.go @@ -1,5 +1,3 @@ -// +build linux - package landlock import ( @@ -25,7 +23,7 @@ func InitLandlock(config *configs.Landlock) error { var llConfig landlock.Config - ruleset := getAccess(config.Ruleset.HandledAccessFS) + ruleset := config.Ruleset.HandledAccessFS // Panic on error when constructing the Landlock configuration using invalid config values. if config.DisableBestEffort { llConfig = landlock.MustConfig(ruleset) @@ -34,32 +32,25 @@ func InitLandlock(config *configs.Landlock) error { } if err := llConfig.RestrictPaths( - getPathAccesses(config.Rules)..., + pathAccesses(config.Rules)..., ); err != nil { - return fmt.Errorf("Could not restrict paths: %v", err) + return fmt.Errorf("could not restrict paths: %w", err) } return nil } -// Convert Libcontainer AccessFS to go-landlock AccessFSSet. -func getAccess(access configs.AccessFS) landlock.AccessFSSet { - return landlock.AccessFSSet(access) -} - // Convert Libcontainer RulePathBeneath to go-landlock PathOpt. -func getPathAccess(rule *configs.RulePathBeneath) landlock.PathOpt { - return landlock.PathAccess( - getAccess(rule.AllowedAccess), - rule.Paths...) +func pathAccess(rule *configs.RulePathBeneath) landlock.PathOpt { + return landlock.PathAccess(rule.AllowedAccess, rule.Paths...) } // Convert Libcontainer Rules to an array of go-landlock PathOpt. -func getPathAccesses(rules *configs.Rules) []landlock.PathOpt { +func pathAccesses(rules *configs.Rules) []landlock.PathOpt { pathAccesses := []landlock.PathOpt{} for _, rule := range rules.PathBeneath { - opt := getPathAccess(rule) + opt := pathAccess(rule) pathAccesses = append(pathAccesses, opt) } diff --git a/libcontainer/landlock/landlock_unsupported.go b/libcontainer/landlock/landlock_unsupported.go deleted file mode 100644 index e2fe222ee99..00000000000 --- a/libcontainer/landlock/landlock_unsupported.go +++ /dev/null @@ -1,19 +0,0 @@ -// +build !linux - -package landlock - -import ( - "errors" - - "github.com/opencontainers/runc/libcontainer/configs" -) - -var ErrLandlockNotSupported = errors.New("land: config provided but Landlock not supported") - -// InitLandlock does nothing because Landlock is not supported. -func InitSLandlock(config *configs.Landlock) error { - if config != nil { - return ErrLandlockNotSupported - } - return nil -} diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index f3ae2aa815c..0223f85963e 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -321,14 +321,12 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) { Ambient: spec.Process.Capabilities.Ambient, } } - if spec.Process.Landlock != nil { - landlock, err := SetupLandlock(spec.Process.Landlock) - if err != nil { - return nil, err - } - config.Landlock = landlock - } + landlock, err := SetupLandlock(spec.Process.Landlock) + if err != nil { + return nil, err + } + config.Landlock = landlock } createHooks(spec, config) config.Version = specs.Version @@ -865,11 +863,11 @@ func SetupLandlock(ll *specs.Landlock) (*configs.Landlock, error) { } for _, access := range ll.Ruleset.HandledAccessFS { - newAccessFs, err := landlock.ConvertStringToAccessFS(string(access)) + newAccessFS, err := landlock.ConvertStringToAccessFSSet(string(access)) if err != nil { return nil, err } - newConfig.Ruleset.HandledAccessFS |= newAccessFs + newConfig.Ruleset.HandledAccessFS |= newAccessFS } // Loop through all Landlock path beneath rule blocks and convert them to libcontainer format. @@ -881,7 +879,7 @@ func SetupLandlock(ll *specs.Landlock) (*configs.Landlock, error) { } for _, access := range rulePath.AllowedAccess { - newAllowedAccess, err := landlock.ConvertStringToAccessFS(string(access)) + newAllowedAccess, err := landlock.ConvertStringToAccessFSSet(string(access)) if err != nil { return nil, err } diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index 6bceca0116b..5c8908a497e 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -213,7 +213,7 @@ func (l *linuxStandardInit) Init() error { // https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318 _ = unix.Close(l.fifoFd) - // `noNewPrivileges` must be enabled to use Landlock. + // NoNewPrivileges must be enabled to use Landlock. if l.config.Config.Landlock != nil && l.config.NoNewPrivileges { if err := landlock.InitLandlock(l.config.Config.Landlock); err != nil { return fmt.Errorf("unable to init Landlock: %w", err)