From ec6c6b87a946a1502767a6850fb17c2e715d7a6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Noack?= Date: Sat, 28 Aug 2021 15:28:17 +0200 Subject: [PATCH] Simplify code after introducing landlock.NewConfig(). In particular, we remove the Config validity check at the top of RestrictPaths(), as it's now impossible to contruct invalid Config values. --- landlock/abi_versions.go | 2 -- landlock/abi_versions_test.go | 4 +--- landlock/accessfs.go | 4 +++- landlock/config.go | 18 ++++------------- landlock/config_test.go | 37 ++++++++++++----------------------- landlock/restrict.go | 4 ---- 6 files changed, 20 insertions(+), 49 deletions(-) diff --git a/landlock/abi_versions.go b/landlock/abi_versions.go index bf3bfc8..f53ad7d 100644 --- a/landlock/abi_versions.go +++ b/landlock/abi_versions.go @@ -18,8 +18,6 @@ var abiInfos = []abiInfo{ }, } -var highestKnownABIVersion = abiInfos[len(abiInfos)-1] - // getSupportedABIVersion returns the kernel-supported ABI version. // // If the ABI version supported by the kernel is higher than the diff --git a/landlock/abi_versions_test.go b/landlock/abi_versions_test.go index b313774..a8eeb4a 100644 --- a/landlock/abi_versions_test.go +++ b/landlock/abi_versions_test.go @@ -2,8 +2,6 @@ package landlock import ( "testing" - - ll "github.com/landlock-lsm/go-landlock/landlock/syscall" ) func TestAbiVersionsIncrementing(t *testing.T) { @@ -16,7 +14,7 @@ func TestAbiVersionsIncrementing(t *testing.T) { func TestSupportedAccessFS(t *testing.T) { got := abiInfos[1].supportedAccessFS - want := AccessFSSet(ll.AccessFSWriteFile | ll.AccessFSRemoveDir | ll.AccessFSRemoveFile | ll.AccessFSMakeChar | ll.AccessFSMakeDir | ll.AccessFSMakeReg | ll.AccessFSMakeSock | ll.AccessFSMakeFifo | ll.AccessFSMakeBlock | ll.AccessFSMakeSym | ll.AccessFSExecute | ll.AccessFSReadFile | ll.AccessFSReadDir) + want := supportedAccessFS if got != want { t.Errorf("V1 supported access FS: got %x, want %x", got, want) diff --git a/landlock/accessfs.go b/landlock/accessfs.go index 7b775b5..41f0802 100644 --- a/landlock/accessfs.go +++ b/landlock/accessfs.go @@ -24,6 +24,8 @@ var flagNames = []string{ // AccessFSSet is a set of Landlockable file system access operations. type AccessFSSet uint64 +var supportedAccessFS = AccessFSSet((1 << 13) - 1) + func (a AccessFSSet) String() string { if a.isEmpty() { return "∅" @@ -62,5 +64,5 @@ func (a AccessFSSet) isEmpty() bool { // valid returns true iff the given AccessFSSet is supported by this // version of go-landlock. func (a AccessFSSet) valid() bool { - return a.isSubset(highestKnownABIVersion.supportedAccessFS) + return a.isSubset(supportedAccessFS) } diff --git a/landlock/config.go b/landlock/config.go index 9f09dab..af4187d 100644 --- a/landlock/config.go +++ b/landlock/config.go @@ -52,6 +52,9 @@ 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. + // + // This constructor ensures that callers can't construct + // invalid Config values. var c Config for _, arg := range args { if afs, ok := arg.(AccessFSSet); ok { @@ -78,15 +81,6 @@ func MustConfig(args ...interface{}) Config { 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() { - return errors.New("unsupported HandledAccessFS value") - } - return nil -} - // String builds a human-readable representation of the Config. func (c Config) String() string { abi := abiInfo{version: -1} // invalid @@ -113,11 +107,7 @@ func (c Config) String() string { version = fmt.Sprintf("V%v", abi.version) } - errStr := "" - if err := c.validate(); err != nil { - errStr = fmt.Sprintf(" (%v)", err) - } - return fmt.Sprintf("{Landlock %v; HandledAccessFS: %v%v%v}", version, desc, bestEffort, errStr) + return fmt.Sprintf("{Landlock %v; HandledAccessFS: %v%v}", version, desc, bestEffort) } // BestEffort returns a config that will opportunistically enforce diff --git a/landlock/config_test.go b/landlock/config_test.go index 0324452..ff81484 100644 --- a/landlock/config_test.go +++ b/landlock/config_test.go @@ -30,7 +30,7 @@ func TestConfigString(t *testing.T) { }, { cfg: Config{handledAccessFS: 1 << 63}, - want: "{Landlock V???; HandledAccessFS: {1<<63} (unsupported HandledAccessFS value)}", + want: "{Landlock V???; HandledAccessFS: {1<<63}}", }, } { got := tc.cfg.String() @@ -40,37 +40,24 @@ func TestConfigString(t *testing.T) { } } -func TestValidateSuccess(t *testing.T) { - for _, c := range []Config{ - V1, V1.BestEffort(), - Config{handledAccessFS: ll.AccessFSWriteFile}, - Config{handledAccessFS: 0}, - } { - err := c.validate() - if err != nil { - t.Errorf("%v.validate(): expected success, got %v", c, err) - } +func TestNewConfig(t *testing.T) { + c, err := NewConfig(AccessFSSet(ll.AccessFSWriteFile)) + if err != nil { + t.Errorf("NewConfig(): expected success, got %v", err) } -} - -func TestValidateFailure(t *testing.T) { - for _, c := range []Config{ - Config{handledAccessFS: 0xffffffffffffffff}, - Config{handledAccessFS: highestKnownABIVersion.supportedAccessFS + 1}, - } { - err := c.validate() - if err == nil { - t.Errorf("%v.validate(): expected error, got success", c) - } + want := AccessFSSet(ll.AccessFSWriteFile) + if c.handledAccessFS != want { + t.Errorf("c.handledAccessFS = %v, want %v", c.handledAccessFS, want) } } -func TestNewConfig(t *testing.T) { - c, err := NewConfig(AccessFSSet(ll.AccessFSWriteFile)) +func TestNewConfigEmpty(t *testing.T) { + // Constructing an empty config is a bit pointless, but should work. + c, err := NewConfig() if err != nil { t.Errorf("NewConfig(): expected success, got %v", err) } - want := AccessFSSet(ll.AccessFSWriteFile) + want := AccessFSSet(0) if c.handledAccessFS != want { t.Errorf("c.handledAccessFS = %v, want %v", c.handledAccessFS, want) } diff --git a/landlock/restrict.go b/landlock/restrict.go index e153a49..247509a 100644 --- a/landlock/restrict.go +++ b/landlock/restrict.go @@ -11,10 +11,6 @@ import ( // The actual restrictPaths implementation. func restrictPaths(c Config, opts ...PathOpt) error { - err := c.validate() - if err != nil { - return bug(fmt.Errorf("unsupported Landlock config %v: %v", c, err)) - } handledAccessFS := c.handledAccessFS abi := getSupportedABIVersion() if c.bestEffort {