diff --git a/go.mod b/go.mod index da451d1f9d5..937200d4387 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ go 1.18 replace maze.io/x/crypto => github.com/snapcore/maze.io-x-crypto v0.0.0-20190131090603-9b94c9afe066 require ( + github.com/bmatcuk/doublestar/v4 v4.6.1 github.com/canonical/go-efilib v0.4.0 github.com/canonical/go-sp800.90a-drbg v0.0.0-20210314144037-6eeb1040d6c3 // indirect github.com/canonical/go-tpm2 v0.0.0-20210827151749-f80ff5afff61 diff --git a/go.sum b/go.sum index 354cd5d564c..4a8a5129223 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/bmatcuk/doublestar/v4 v4.6.1 h1:FH9SifrbvJhnlQpztAx++wlkk70QBf0iBWDwNy7PA4I= +github.com/bmatcuk/doublestar/v4 v4.6.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc= github.com/canonical/go-efilib v0.4.0 h1:2ee5pvhIZ+g1EO4HxFE/owBgs5Up2g7dw1+Ls9/fiSs= github.com/canonical/go-efilib v0.4.0/go.mod h1:9b2PNAuPcZsB76x75/uwH99D8CyH/A2y4rq1/+bvplg= github.com/canonical/go-sp800.108-kdf v0.0.0-20210314145419-a3359f2d21b9 h1:USzKjrfWo/ESzozv2i3OMM7XDgxrZRvaHFrKkIKRtwU= diff --git a/interfaces/prompting/constraints.go b/interfaces/prompting/constraints.go index 8105b320a48..615d22409b7 100644 --- a/interfaces/prompting/constraints.go +++ b/interfaces/prompting/constraints.go @@ -41,10 +41,6 @@ type Constraints struct { // ValidateForInterface returns nil if the constraints are valid for the given // interface, otherwise returns an error. func (c *Constraints) ValidateForInterface(iface string) error { - // TODO: change to this once PR #13866 is merged: - // if err := ValidatePathPattern(c.PathPattern); err != nil { - // return err - // } return c.validatePermissions(iface) } @@ -81,9 +77,7 @@ func (c *Constraints) validatePermissions(iface string) error { // // If the constraints or path are invalid, returns an error. func (c *Constraints) Match(path string) (bool, error) { - // TODO: change to this once PR #13866 is merged: - // return PathPatternMatch(c.PathPattern, path) - return true, nil + return PathPatternMatch(c.PathPattern, path) } // RemovePermission removes every instance of the given permission from the diff --git a/interfaces/prompting/constraints_test.go b/interfaces/prompting/constraints_test.go index b7f261fc610..325d0da2c28 100644 --- a/interfaces/prompting/constraints_test.go +++ b/interfaces/prompting/constraints_test.go @@ -24,8 +24,7 @@ import ( . "gopkg.in/check.v1" - // TODO: add this once PR #13730 is merged: - // doublestar "github.com/bmatcuk/doublestar/v4" + doublestar "github.com/bmatcuk/doublestar/v4" "github.com/snapcore/snapd/interfaces/prompting" "github.com/snapcore/snapd/sandbox/apparmor/notify" @@ -44,17 +43,10 @@ func (s *constraintsSuite) TestConstraintsValidateForInterface(c *C) { }{ { "foo", - "invalid/path", + "/invalid/path", []string{"read"}, "unsupported interface.*", }, - // TODO: add this once PR #13730 is merged: - // { - // "home", - // "invalid/path", - // []string{"read"}, - // "invalid path pattern.*", - // }, { "home", "/valid/path", @@ -151,12 +143,11 @@ func (*constraintsSuite) TestConstraintsMatch(c *C) { "/home/test/Documents/foo.txt", true, }, - // TODO: add this once PR #13730 is merged: - // { - // "/home/test/Documents/foo", - // "/home/test/Documents/foo.txt", - // false, - // }, + { + "/home/test/Documents/foo", + "/home/test/Documents/foo.txt", + false, + }, } for _, testCase := range cases { constraints := &prompting.Constraints{ @@ -176,11 +167,8 @@ func (s *constraintsSuite) TestConstraintsMatchUnhappy(c *C) { Permissions: []string{"read"}, } matches, err := badConstraints.Match(badPath) - // TODO: change to this once PR #13730 is merged: - // c.Check(err, Equals, doublestar.ErrBadPattern) - // c.Check(matches, Equals, false) - c.Check(err, Equals, nil) - c.Check(matches, Equals, true) + c.Check(err, Equals, doublestar.ErrBadPattern) + c.Check(matches, Equals, false) } func (s *constraintsSuite) TestConstraintsRemovePermission(c *C) { diff --git a/interfaces/prompting/patterns.go b/interfaces/prompting/patterns.go new file mode 100644 index 00000000000..c49eafb8935 --- /dev/null +++ b/interfaces/prompting/patterns.go @@ -0,0 +1,66 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2024 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package prompting + +import ( + "strings" + + doublestar "github.com/bmatcuk/doublestar/v4" +) + +// PathPatternMatch returns true if the given pattern matches the given path. +// +// The pattern should not contain groups, and should likely have been an output +// of ExpandPathPattern. +// +// Paths to directories are received with trailing slashes, but we don't want +// to require the user to include a trailing '/' if they want to match +// directories (and end their pattern with `{,/}` if they want to match both +// directories and non-directories). Thus, we want to ensure that patterns +// without trailing slashes match paths with trailing slashes. However, +// patterns with trailing slashes should not match paths without trailing +// slashes. +// +// The doublestar package has special cases for patterns ending in `/**` and +// `/**/`: `/foo/**`, and `/foo/**/` both match `/foo` and `/foo/`. We want to +// override this behavior to make `/foo/**/` not match `/foo`. We also want to +// override doublestar to make `/foo` match `/foo/`. +func PathPatternMatch(pattern string, path string) (bool, error) { + // Check the usual doublestar match first, in case the pattern is malformed + // and causes an error, and return the error if so. + matched, err := doublestar.Match(pattern, path) + if err != nil { + return false, err + } + // No matter if doublestar matched, return false if pattern ends in '/' but + // path is not a directory. + if strings.HasSuffix(pattern, "/") && !strings.HasSuffix(path, "/") { + return false, nil + } + if matched { + return true, nil + } + if strings.HasSuffix(pattern, "/") { + return false, nil + } + // Try again with a '/' appended to the pattern, so patterns like `/foo` + // match paths like `/foo/`. + return doublestar.Match(pattern+"/", path) +} diff --git a/interfaces/prompting/patterns_test.go b/interfaces/prompting/patterns_test.go new file mode 100644 index 00000000000..2381080ed22 --- /dev/null +++ b/interfaces/prompting/patterns_test.go @@ -0,0 +1,377 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2024 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package prompting_test + +import ( + . "gopkg.in/check.v1" + + doublestar "github.com/bmatcuk/doublestar/v4" + + "github.com/snapcore/snapd/interfaces/prompting" +) + +type patternsSuite struct{} + +var _ = Suite(&patternsSuite{}) + +func (s *patternsSuite) TestPathPatternMatch(c *C) { + cases := []struct { + pattern string + path string + matches bool + }{ + { + "/home/test/Documents", + "/home/test/Documents", + true, + }, + { + "/home/test/Documents", + "/home/test/Documents/", + true, + }, + { + "/home/test/Documents/", + "/home/test/Documents", + false, + }, + { + "/home/test/Documents/", + "/home/test/Documents/", + true, + }, + { + "/home/test/Documents/*", + "/home/test/Documents", + false, + }, + { + "/home/test/Documents/*", + "/home/test/Documents/", + true, + }, + { + "/home/test/Documents/*", + "/home/test/Documents/foo", + true, + }, + { + "/home/test/Documents/*", + "/home/test/Documents/foo/", + true, + }, + { + "/home/test/Documents/*/", + "/home/test/Documents", + false, + }, + { + "/home/test/Documents/*/", + "/home/test/Documents/", + false, + }, + { + "/home/test/Documents/*/", + "/home/test/Documents/foo", + false, + }, + { + "/home/test/Documents/*/", + "/home/test/Documents/foo/", + true, + }, + { + "/home/test/Documents/**", + "/home/test/Documents", + true, + }, + { + "/home/test/Documents/**", + "/home/test/Documents/", + true, + }, + { + "/home/test/Documents/**", + "/home/test/Documents/foo", + true, + }, + { + "/home/test/Documents/**", + "/home/test/Documents/foo/", + true, + }, + { + // Even though doublestar lets /path/to/a/**/ match /path/to/a, we + // want the ability to match only directories, so we impose the + // additional constraint that patterns ending in /**/ only match + // paths which end in / + "/home/test/Documents/**/", + "/home/test/Documents", + false, + }, + { + "/home/test/Documents/**/", + "/home/test/Documents/", + true, + }, + { + "/home/test/Documents/**/", + "/home/test/Documents/foo", + false, + }, + { + "/home/test/Documents/**/", + "/home/test/Documents/foo/", + true, + }, + { + "/home/test/Documents/**/", + "/home/test/Documents/foo/bar", + false, + }, + { + "/home/test/Documents/**/", + "/home/test/Documents/foo/bar/", + true, + }, + { + "/home/test/Documents/**/*.txt", + "/home/test/Documents/foo.txt", + true, + }, + { + "/home/test/Documents/**/*.txt", + "/home/test/Documents/foo/bar.tar.gz", + false, + }, + { + "/home/test/Documents/**/*.gz", + "/home/test/Documents/foo/bar.tar.gz", + true, + }, + { + "/home/test/Documents/**/*.tar.gz", + "/home/test/Documents/foo/bar.tar.gz", + true, + }, + { + "/home/test/Documents/*.tar.gz", + "/home/test/Documents/foo/bar.tar.gz", + false, + }, + { + "/home/test/Documents/foo", + "/home/test/Documents/foo.txt", + false, + }, + { + "/home/test/Documents/foo*", + "/home/test/Documents/foo.txt", + true, + }, + { + "/home/test/Documents/foo?*", + "/home/test/Documents/foo.txt", + true, + }, + { + "/home/test/Documents/foo????", + "/home/test/Documents/foo.txt", + true, + }, + { + "/home/test/Documents/foo????*", + "/home/test/Documents/foo.txt", + true, + }, + { + "/home/test/Documents/foo?????*", + "/home/test/Documents/foo.txt", + false, + }, + { + "/home/test/Documents/*", + "/home/test/Documents/foo/bar.tar.gz", + false, + }, + { + "/home/test/**", + "/home/test/Documents/foo/bar.tar.gz", + true, + }, + { + "/home/test/**/*.tar.gz", + "/home/test/Documents/foo/bar.tar.gz", + true, + }, + { + "/home/test/**/*.gz", + "/home/test/Documents/foo/bar.tar.gz", + true, + }, + { + "/home/test/**/*.txt", + "/home/test/Documents/foo/bar.tar.gz", + false, + }, + { + "/foo/bar*", + "/hoo/bar/", + false, + }, + { + "/foo/bar?", + "/hoo/bar/", + false, + }, + { + "/foo/bar/**", + "/foo/bar/", + true, + }, + { + "/foo/*/bar/**/baz**/fi*z/**buzz", + "/foo/abc/bar/baznm/fizz/xyzbuzz", + true, + }, + { + "/foo/*/bar/**/baz**/fi*z/**buzz", + "/foo/abc/bar/baz/nm/fizz/xyzbuzz", + false, + }, + { + "/foo*bar", + "/foobar", + true, + }, + { + "/foo*bar", + "/fooxbar", + true, + }, + { + "/foo*bar", + "/foo/bar", + false, + }, + { + "/foo?bar", + "/foobar", + false, + }, + { + "/foo?bar", + "/fooxbar", + true, + }, + { + "/foo?bar", + "/foo/bar", + false, + }, + { + "/foo/*/bar", + "/foo/bar", + false, + }, + { + "/foo/**/bar", + "/foo/bar", + true, + }, + { + "/foo/**/bar", + "/foo/bar/", + true, + }, + { + "/foo/**/bar", + "/foo/fizz/buzz/bar/", + true, + }, + { + "/foo**/bar", + "/fooabc/bar", + true, + }, + { + "/foo**/bar", + "/foo/bar", + true, + }, + { + "/foo**/bar", + "/foo/fizz/bar", + false, + }, + { + "/foo/**bar", + "/foo/abcbar", + true, + }, + { + "/foo/**bar", + "/foo/bar", + true, + }, + { + "/foo/**bar", + "/foo/fizz/bar", + false, + }, + { + "/foo/*/bar/**/baz**/fi*z/**buzz", + "/foo/abc/bar/baz/fiz/buzz", + true, + }, + { + "/foo/*/bar/**/baz**/fi*z/**buzz", + "/foo/abc/bar/baz/abc/fiz/buzz", + false, + }, + { + "/foo/*/bar/**/baz**/fi*z/**buzz", + "/foo/bar/bazmn/fizz/xyzbuzz", + false, + }, + { + "/foo/bar/**/*/", + "/foo/bar/baz", + false, + }, + { + "/foo/bar/**/*/", + "/foo/bar/baz/", + true, + }, + } + for _, testCase := range cases { + matches, err := prompting.PathPatternMatch(testCase.pattern, testCase.path) + c.Check(err, IsNil, Commentf("test case: %+v", testCase)) + c.Check(matches, Equals, testCase.matches, Commentf("test case: %+v", testCase)) + } +} + +func (s *patternsSuite) TestPathPatternMatchErrors(c *C) { + badPattern := `badpattern\` + matches, err := prompting.PathPatternMatch(badPattern, "foo") + c.Check(err, Equals, doublestar.ErrBadPattern) + c.Check(matches, Equals, false) +} diff --git a/packaging/debian-sid/control b/packaging/debian-sid/control index ea200027c6a..aae3d42b252 100644 --- a/packaging/debian-sid/control +++ b/packaging/debian-sid/control @@ -22,6 +22,7 @@ Build-Depends: autoconf, gettext, gnupg2, golang-dbus-dev, + golang-github-bmatcuk-doublestar-dev, golang-github-coreos-bbolt-dev, golang-github-coreos-go-systemd-dev, golang-github-gorilla-mux-dev, diff --git a/packaging/fedora/snapd.spec b/packaging/fedora/snapd.spec index 24f4536adc4..76b47f0075b 100644 --- a/packaging/fedora/snapd.spec +++ b/packaging/fedora/snapd.spec @@ -171,6 +171,7 @@ Provides: %{name}-login-service%{?_isa} = 1.33 %if ! 0%{?with_bundled} BuildRequires: golang(go.etcd.io/bbolt) +BuildRequires: golang(github.com/bmatcuk/doublestar/v4) BuildRequires: golang(github.com/coreos/go-systemd/activation) BuildRequires: golang(github.com/godbus/dbus) BuildRequires: golang(github.com/godbus/dbus/introspect) @@ -268,6 +269,7 @@ BuildArch: noarch %if ! 0%{?with_bundled} Requires: golang(go.etcd.io/bbolt) +Requires: golang(github.com/bmatcuk/doublestar/v4) Requires: golang(github.com/coreos/go-systemd/activation) Requires: golang(github.com/godbus/dbus) Requires: golang(github.com/godbus/dbus/introspect) @@ -297,6 +299,7 @@ Requires: golang(gopkg.in/yaml.v3) # the bundled tarball are unversioned (they go by git commit) # *sigh*... I hate golang... Provides: bundled(golang(go.etcd.io/bbolt)) +Provides: bundled(golang(github.com/bmatcuk/doublestar/v4)) Provides: bundled(golang(github.com/coreos/go-systemd/activation)) Provides: bundled(golang(github.com/godbus/dbus)) Provides: bundled(golang(github.com/godbus/dbus/introspect))