From 4e4877f69930828348e10f975c3c778cca318bf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Noack?= Date: Sat, 28 Aug 2021 15:13:00 +0200 Subject: [PATCH] Hide Config.handledAccessFS again, and make it settable through a constructor. The test in `tests/customconfig/config_test.go` has a usage example. Fixes #12. --- landlock/config.go | 49 ++++++++++++++++++++++++++----- landlock/config_test.go | 43 ++++++++++++++++++++++----- landlock/restrict.go | 4 +-- tests/customconfig/config_test.go | 43 +++++++++++++++++++++++++++ tests/failure/failure_test.go | 12 -------- tests/restrict/restrict_test.go | 7 ++--- 6 files changed, 125 insertions(+), 33 deletions(-) create mode 100644 tests/customconfig/config_test.go diff --git a/landlock/config.go b/landlock/config.go index 656166e..9f09dab 100644 --- a/landlock/config.go +++ b/landlock/config.go @@ -31,7 +31,7 @@ const ( var ( // Landlock V1 support (basic file operations). V1 = Config{ - HandledAccessFS: abiInfos[1].supportedAccessFS, + handledAccessFS: abiInfos[1].supportedAccessFS, } ) @@ -39,16 +39,49 @@ var ( // landlockable operations to be restricted and the constraints on it // (e.g. best effort mode). type Config struct { - // File system operations to restrict when enabling Landlock. - // Needs to stay within the bounds of what go-landlock supports. - HandledAccessFS AccessFSSet + handledAccessFS AccessFSSet bestEffort bool } +// NewConfig creates a new Landlock configuration with the given parameters. +// +// Passing an AccessFSSet will set that as the set of file system +// operations to restrict when enabling Landlock. The AccessFSSet +// needs to stay within the bounds of what go-landlock supports. +func NewConfig(args ...interface{}) (*Config, error) { + // Implementation note: This factory is written with future + // extensibility in mind. Only specific types are supported as + // input, but in the future more might be added. + var c Config + for _, arg := range args { + if afs, ok := arg.(AccessFSSet); ok { + if !c.handledAccessFS.isEmpty() { + return nil, errors.New("only one AccessFSSet may be provided") + } + if !afs.valid() { + return nil, errors.New("unsupported AccessFSSet value; upgrade go-landlock?") + } + c.handledAccessFS = afs + } else { + return nil, fmt.Errorf("unknown argument %v; only AccessFSSet-type argument is supported", arg) + } + } + return &c, nil +} + +// MustConfig is like NewConfig but panics on error. +func MustConfig(args ...interface{}) Config { + c, err := NewConfig(args...) + if err != nil { + panic(err) + } + return *c +} + // validate returns success when the given config is supported by // go-landlock. (It may still be unsupported by your kernel though.) func (c Config) validate() error { - if !c.HandledAccessFS.valid() { + if !c.handledAccessFS.valid() { return errors.New("unsupported HandledAccessFS value") } return nil @@ -58,13 +91,13 @@ func (c Config) validate() error { func (c Config) String() string { abi := abiInfo{version: -1} // invalid for _, a := range abiInfos { - if c.HandledAccessFS.isSubset(a.supportedAccessFS) { + if c.handledAccessFS.isSubset(a.supportedAccessFS) { abi = a } } - var desc = c.HandledAccessFS.String() - if abi.supportedAccessFS == c.HandledAccessFS { + var desc = c.handledAccessFS.String() + if abi.supportedAccessFS == c.handledAccessFS { desc = "all" } diff --git a/landlock/config_test.go b/landlock/config_test.go index 6cfb21d..0324452 100644 --- a/landlock/config_test.go +++ b/landlock/config_test.go @@ -13,11 +13,11 @@ func TestConfigString(t *testing.T) { want string }{ { - cfg: Config{HandledAccessFS: 0}, + cfg: Config{handledAccessFS: 0}, want: fmt.Sprintf("{Landlock V1; HandledAccessFS: %v}", AccessFSSet(0)), }, { - cfg: Config{HandledAccessFS: ll.AccessFSWriteFile}, + cfg: Config{handledAccessFS: ll.AccessFSWriteFile}, want: "{Landlock V1; HandledAccessFS: {WriteFile}}", }, { @@ -29,7 +29,7 @@ func TestConfigString(t *testing.T) { want: "{Landlock V1; HandledAccessFS: all (best effort)}", }, { - cfg: Config{HandledAccessFS: 1<<63}, + cfg: Config{handledAccessFS: 1 << 63}, want: "{Landlock V???; HandledAccessFS: {1<<63} (unsupported HandledAccessFS value)}", }, } { @@ -43,8 +43,8 @@ func TestConfigString(t *testing.T) { func TestValidateSuccess(t *testing.T) { for _, c := range []Config{ V1, V1.BestEffort(), - Config{HandledAccessFS: ll.AccessFSWriteFile}, - Config{HandledAccessFS: 0}, + Config{handledAccessFS: ll.AccessFSWriteFile}, + Config{handledAccessFS: 0}, } { err := c.validate() if err != nil { @@ -55,8 +55,8 @@ func TestValidateSuccess(t *testing.T) { func TestValidateFailure(t *testing.T) { for _, c := range []Config{ - Config{HandledAccessFS: 0xffffffffffffffff}, - Config{HandledAccessFS: highestKnownABIVersion.supportedAccessFS + 1}, + Config{handledAccessFS: 0xffffffffffffffff}, + Config{handledAccessFS: highestKnownABIVersion.supportedAccessFS + 1}, } { err := c.validate() if err == nil { @@ -64,3 +64,32 @@ func TestValidateFailure(t *testing.T) { } } } + +func TestNewConfig(t *testing.T) { + c, err := NewConfig(AccessFSSet(ll.AccessFSWriteFile)) + if err != nil { + t.Errorf("NewConfig(): expected success, got %v", err) + } + want := AccessFSSet(ll.AccessFSWriteFile) + if c.handledAccessFS != want { + t.Errorf("c.handledAccessFS = %v, want %v", c.handledAccessFS, want) + } +} + +func TestNewConfigFailures(t *testing.T) { + for _, args := range [][]interface{}{ + {ll.AccessFSWriteFile}, + {123}, + {"a string"}, + {"foo", 42}, + // May not specify two AccessFSSets + {AccessFSSet(ll.AccessFSWriteFile), AccessFSSet(ll.AccessFSReadFile)}, + // May not specify an unsupported AccessFSSet value + {AccessFSSet(1 << 63)}, + } { + _, err := NewConfig(args...) + if err == nil { + t.Errorf("NewConfig(%v) success, expected error", args) + } + } +} diff --git a/landlock/restrict.go b/landlock/restrict.go index d1591a2..e153a49 100644 --- a/landlock/restrict.go +++ b/landlock/restrict.go @@ -13,9 +13,9 @@ import ( func restrictPaths(c Config, opts ...PathOpt) error { err := c.validate() if err != nil { - return fmt.Errorf("unsupported Landlock config %v (upgrade go-landlock?): %v", c, err) + return bug(fmt.Errorf("unsupported Landlock config %v: %v", c, err)) } - handledAccessFS := c.HandledAccessFS + handledAccessFS := c.handledAccessFS abi := getSupportedABIVersion() if c.bestEffort { handledAccessFS = handledAccessFS.intersect(abi.supportedAccessFS) diff --git a/tests/customconfig/config_test.go b/tests/customconfig/config_test.go new file mode 100644 index 0000000..a55af58 --- /dev/null +++ b/tests/customconfig/config_test.go @@ -0,0 +1,43 @@ +package landlock_test + +import ( + "os" + "testing" + + "github.com/landlock-lsm/go-landlock/landlock" + ll "github.com/landlock-lsm/go-landlock/landlock/syscall" +) + +// True if the given path can be opened for reading. +func canAccess(path string) bool { + f, err := os.Open(path) + if err != nil { + return false + } + defer f.Close() + return true +} + +func TestCustomConfig(t *testing.T) { + if !canAccess("/etc/passwd") { + t.Skipf("expected normal accesses to /etc/passwd to work") + } + + if !canAccess("/etc/group") { + t.Skipf("expected normal accesses to /etc/group to work") + } + + readFile := landlock.AccessFSSet(ll.AccessFSReadFile) + if err := landlock.MustConfig(readFile).RestrictPaths( + landlock.PathAccess(readFile, "/etc/passwd"), + ); err != nil { + t.Fatalf("Could not restrict paths: %v", err) + } + + if !canAccess("/etc/passwd") { + t.Error("expected to have read access to /etc/passwd, but didn't") + } + if canAccess("/etc/group") { + t.Error("expected to have NO read access to /etc/group, but did") + } +} diff --git a/tests/failure/failure_test.go b/tests/failure/failure_test.go index 1ece352..249224d 100644 --- a/tests/failure/failure_test.go +++ b/tests/failure/failure_test.go @@ -47,15 +47,3 @@ func TestEmptyAccessRights(t *testing.T) { t.Errorf("expected error message with «empty access rights», got: %v", err) } } - -func TestSpecifiedTooManyFlags(t *testing.T) { - cfg := landlock.Config{HandledAccessFS: 1 << 13} - err := cfg.RestrictPaths() - if err == nil { - t.Errorf("%v.RestrictPaths(): expected error, got success", cfg) - } - want := "unsupported Landlock config" - if !strings.Contains(err.Error(), want) { - t.Errorf("%v.RestrictPaths(): expected error with %q, got %q", cfg, want, err.Error()) - } -} diff --git a/tests/restrict/restrict_test.go b/tests/restrict/restrict_test.go index 0efbf1d..60265b7 100644 --- a/tests/restrict/restrict_test.go +++ b/tests/restrict/restrict_test.go @@ -25,12 +25,11 @@ func canAccess(path string) bool { // been discussed in the context of seccomp at // https://github.com/golang/go/issues/3405. func TestRestrictInPresenceOfThreading(t *testing.T) { - _, err := os.ReadFile("/etc/passwd") - if err != nil { - t.Skipf("expected normal accesses to /etc/passwd to work, got error: %v", err) + if !canAccess("/etc/passwd") { + t.Skipf("expected normal accesses to /etc/passwd to work") } - err = landlock.V1.RestrictPaths() // No access permitted at all. + err := landlock.V1.RestrictPaths() // No access permitted at all. if err != nil { t.Skipf("kernel does not support Landlock v1; tests cannot be run.") }