i/builtin: prohibit trailing '@' in common-files filepaths (#14696)

* i/builtin: prohibit trailing '@' in common-files filepaths

For the `common-files` interface and those which use
`commonFilesInterface` (such as `system-files` and `personal-files`),
these append a trailing `{,/,/**}` to the end of each path.

The `@` character is allowed in filepaths specified in these plugs,
since filepaths like `/dev/foo@bar` may be required. However, if an `@`
character occurs at the end of the filepath (e.g. `/foo/bar@`), then the
resulting filepath in the rule is `/foo/bar@{,/,/**}`. AppArmor treats
`@{FOO}` as a variable, so having `@{,/,/**}` in the final rule snippet
is problematic.

This commit adds a check to `common_files.go` to ensure that filepaths
do not have a trailing `@`.

Signed-off-by: Oliver Calder <[email protected]>

* i/builtin: add comment explaining why common-files paths cannot end with @

Signed-off-by: Oliver Calder <[email protected]>

---------

Signed-off-by: Oliver Calder <[email protected]>

interfaces/builtin/common_files.go

@@ -104,6 +104,13 @@ func (iface *commonFilesInterface) validateSinglePath(np string) error {
 	if strings.HasSuffix(np, "/") {
 		return fmt.Errorf(`%q cannot end with "/"`, np)
 	}
+	if strings.HasSuffix(np, "@") {
+		// Variables in AppArmor profiles have the form `@{FOO}`. Since we're
+		// going to add `{,/,/**}` to the end of the path, we cannot have a
+		// trailing '@', else we'll end up with a path which ends with
+		// `@{,/,/**}`, which looks problematically like an AppArmor variable.
+		return fmt.Errorf(`%q cannot end with "@"`, np)
+	}
 	p := filepath.Clean(np)
 	if p != np {
 		return fmt.Errorf("cannot use %q: try %q", np, filepath.Clean(np))

interfaces/builtin/personal_files_test.go

@@ -244,6 +244,7 @@ plugs:
 		{`read: [ "$HOME/home/$HOME/foo" ]`, `\$HOME must only be used at the start of the path of "\$HOME/home/\$HOME/foo"`},
 		{`read: [ "$HOME/sweet/$HOME" ]`, `\$HOME must only be used at the start of the path of "\$HOME/sweet/\$HOME"`},
 		{`read: [ "/@{FOO}" ]`, `"/@{FOO}" contains a reserved apparmor char from .*`},
+		{`read: [ "/foo/bar@" ]`, `"/foo/bar@" cannot end with "@"`},
 		{`read: [ "/home/@{HOME}/foo" ]`, `"/home/@{HOME}/foo" contains a reserved apparmor char from .*`},
 		{`read: [ "${HOME}/foo" ]`, `"\${HOME}/foo" contains a reserved apparmor char from .*`},
 		{`read: [ "$HOME" ]`, `"\$HOME" must start with "\$HOME/"`},

interfaces/builtin/system_files_test.go

@@ -50,7 +50,7 @@ version: 1.0
 plugs:
  system-files:
   read: [/etc/read-dir2, /etc/read-file2]
-  write:  [/etc/write-dir2, /etc/write-file2]
+  write:  [/etc/write-dir2, /etc/write-file2, /dev/foo@bar]
 apps:
  app:
   command: foo
@@ -83,6 +83,7 @@ func (s *systemFilesInterfaceSuite) TestConnectedPlugAppArmor(c *C) {
 "/etc/read-file2{,/,/**}" rk,
 "/etc/write-dir2{,/,/**}" rwkl,
 "/etc/write-file2{,/,/**}" rwkl,
+"/dev/foo@bar{,/,/**}" rwkl,
 `)
 }
 
@@ -133,6 +134,7 @@ plugs:
 		{`read: [ "$HOME/sweet/$HOME" ]`, `"\$HOME/sweet/\$HOME" must start with "/"`},
 		{`read: [ "/@{FOO}" ]`, `"/@{FOO}" contains a reserved apparmor char from .*`},
 		{`read: [ "/home/@{HOME}/foo" ]`, `"/home/@{HOME}/foo" contains a reserved apparmor char from .*`},
+		{`read: [ "/foo/bar@" ]`, `"/foo/bar@" cannot end with "@"`},
 	}
 
 	for _, t := range testCases {