Bool file symlinks #329

Closed
wants to merge 2 commits into
from
Jump to file or symbol
Failed to load files and symbols.
+75 −3
Split
View
@@ -21,8 +21,26 @@ package caps
import (
"fmt"
+ "path/filepath"
+
+ "github.com/ubuntu-core/snappy/testutil"
)
+type evalSymlinksFn func(string) (string, error)
+
+// evalSymlinks is either filepath.EvalSymlinks or a mocked function for
+// applicable for testing.
+var evalSymlinks = filepath.EvalSymlinks
+
+// MockEvalSymlinks replaces the path/filepath.EvalSymlinks function used inside the caps package.
+func MockEvalSymlinks(test *testutil.BaseTest, fn func(string) (string, error)) {
+ orig := evalSymlinks
+ evalSymlinks = fn
+ test.AddCleanup(func() {
+ evalSymlinks = orig
+ })
+}
+
// Type describes a group of interchangeable capabilities with common features.
// Types are managed centrally and act as a contract between system builders,
// application developers and end users.
@@ -53,6 +71,14 @@ func (t *BoolFileType) Name() string {
return "bool-file"
}
+// Error for reporting invalid/not-sanitized capabilities in places where we
+// expect everything to be valid.
+type notSanitizedError struct{}
+
+func (e *notSanitizedError) Error() string {
+ return "capability is not sanitized"
@niemeyer

niemeyer Jan 20, 2016

Contributor

I don't really understand what this is trying to say.. my guess is that other people likely won't as well.

It should also be a var:

var errNotSanitized = errors.New("foo bar")

Or even inline, since there's really a single occurrence right now, which is never used in an error check.

+}
+
// Sanitize checks and possibly modifies a capability.
// Valid "bool-file" capabilities must contain the attribute "path".
func (t *BoolFileType) Sanitize(c *Capability) error {
@@ -72,8 +98,10 @@ func (t *BoolFileType) Sanitize(c *Capability) error {
func (t *BoolFileType) SecuritySnippet(c *Capability, securitySystem SecuritySystem) ([]byte, error) {
switch securitySystem {
case SecurityApparmor:
- // TODO: switch to the real path later
- path := c.Attrs["path"]
+ path, err := t.dereferencedPath(c)
+ if err != nil {
+ return nil, err
+ }
// Allow read, write and lock on the file designated by the path.
return []byte(fmt.Sprintf("%s rwl,\n", path)), nil
case SecuritySeccomp:
@@ -85,6 +113,18 @@ func (t *BoolFileType) SecuritySnippet(c *Capability, securitySystem SecuritySys
}
}
+func (t *BoolFileType) dereferencedPath(c *Capability) (string, error) {
+ path := c.Attrs["path"]
+ if path == "" {
+ return "", &notSanitizedError{}
@niemeyer

niemeyer Jan 20, 2016

Contributor
return fmt.Errorf("bool-file capability is invalid: missing path attribute")

?

@niemeyer

niemeyer Jan 20, 2016

Contributor

Actually, perhaps even:

return fmt.Errorf("%q bool-file capability is invalid: missing path attribute", c.Name)

So we say what the actual capability is as well.

+ }
+ realPath, err := evalSymlinks(path)
+ if err != nil {
+ return "", fmt.Errorf("bool-file path is invalid: %s", err)
@niemeyer

niemeyer Jan 20, 2016

Contributor
return fmt.Errorf("%q bool-file capability has invalid path attribute: %s", c.Name, err)
+ }
+ return realPath, nil
+}
+
// TestType is a type for various kind of tests.
// It is public so that it can be consumed from other packages.
type TestType struct {
View
@@ -23,11 +23,14 @@ import (
"fmt"
. "gopkg.in/check.v1"
+
+ "github.com/ubuntu-core/snappy/testutil"
)
// BoolFileType
type BoolFileTypeSuite struct {
+ testutil.BaseTest
t Type
}
@@ -65,13 +68,16 @@ func (s *BoolFileTypeSuite) TestSanitizeMissingPath(c *C) {
}
func (s *BoolFileTypeSuite) TestSecuritySnippet(c *C) {
+ MockEvalSymlinks(&s.BaseTest, func(path string) (string, error) {
+ return "real-path", nil
+ })
cap := &Capability{
TypeName: "bool-file",
Attrs: map[string]string{"path": "path"},
}
snippet, err := s.t.SecuritySnippet(cap, SecurityApparmor)
c.Assert(err, IsNil)
- c.Assert(snippet, DeepEquals, []byte("path rwl,\n"))
+ c.Assert(snippet, DeepEquals, []byte("real-path rwl,\n"))
snippet, err = s.t.SecuritySnippet(cap, SecuritySeccomp)
c.Assert(err, IsNil)
c.Assert(snippet, IsNil)
@@ -83,6 +89,32 @@ func (s *BoolFileTypeSuite) TestSecuritySnippet(c *C) {
c.Assert(snippet, IsNil)
}
+func (s *BoolFileTypeSuite) TestDereferencePathSuccess(c *C) {
+ MockEvalSymlinks(&s.BaseTest, func(path string) (string, error) {
+ return "real-path", nil
+ })
+ cap := &Capability{
+ TypeName: "bool-file",
+ Attrs: map[string]string{"path": "symbolic-path"},
+ }
+ path, err := s.t.(*BoolFileType).dereferencedPath(cap)
+ c.Assert(err, IsNil)
+ c.Assert(path, Equals, "real-path")
+}
+
+func (s *BoolFileTypeSuite) TestDereferencePathError(c *C) {
+ MockEvalSymlinks(&s.BaseTest, func(path string) (string, error) {
+ return "", fmt.Errorf("broken symbolic link")
+ })
+ cap := &Capability{
+ TypeName: "bool-file",
+ Attrs: map[string]string{"path": "symbolic-path"},
+ }
+ path, err := s.t.(*BoolFileType).dereferencedPath(cap)
+ c.Assert(err, ErrorMatches, "bool-file path is invalid: broken symbolic link")
+ c.Assert(path, Equals, "")
+}
+
// TestType
type TestTypeSuite struct {