Skip to content

Commit

Permalink
Simplify code after introducing landlock.NewConfig().
Browse files Browse the repository at this point in the history
In particular, we remove the Config validity check at the top of
RestrictPaths(), as it's now impossible to contruct invalid Config
values.
  • Loading branch information
gnoack committed Aug 28, 2021
1 parent 4e4877f commit ec6c6b8
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 49 deletions.
2 changes: 0 additions & 2 deletions landlock/abi_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions landlock/abi_versions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package landlock

import (
"testing"

ll "github.com/landlock-lsm/go-landlock/landlock/syscall"
)

func TestAbiVersionsIncrementing(t *testing.T) {
Expand All @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion landlock/accessfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 "∅"
Expand Down Expand Up @@ -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)
}
18 changes: 4 additions & 14 deletions landlock/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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
Expand Down
37 changes: 12 additions & 25 deletions landlock/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
}
Expand Down
4 changes: 0 additions & 4 deletions landlock/restrict.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit ec6c6b8

Please sign in to comment.