interfaces/utils: allow commas in filepaths

interfaces/builtin/custom_device.go

@@ -77,7 +77,8 @@ func (iface *customDeviceInterface) validateFilePath(path string, attrName strin
 		return fmt.Errorf(`custom-device %q path is not clean: %q`, attrName, path)
 	}
 
-	if _, err := utils.NewPathPattern(path); err != nil {
+	const allowCommas = true
+	if _, err := utils.NewPathPattern(path, allowCommas); err != nil {
 		return fmt.Errorf(`custom-device %q path cannot be used: %v`, attrName, err)
 	}
 

interfaces/builtin/custom_device_test.go

@@ -409,6 +409,7 @@ slots:
   devices:
     - /dev/input/event[0-9]
     - /dev/input/mice
+    - /dev/dma_heap/qcom,qseecom
   read-devices:
     - /dev/js*
   %s
@@ -428,6 +429,7 @@ apps:
 				{`KERNEL`: `"input/event[0-9]"`},
 				{`KERNEL`: `"input/mice"`},
 				{`KERNEL`: `"js*"`},
+				{`KERNEL`: `"dma_heap/qcom,qseecom"`},
 			},
 		},
 		{
@@ -436,6 +438,7 @@ apps:
 				{`KERNEL`: `"input/event[0-9]"`},
 				{`KERNEL`: `"input/mice"`, `SUBSYSTEM`: `"input"`},
 				{`KERNEL`: `"js*"`},
+				{`KERNEL`: `"dma_heap/qcom,qseecom"`},
 			},
 		},
 		{
@@ -450,6 +453,7 @@ apps:
 				{`KERNEL`: `"input/event[0-9]"`},
 				{`KERNEL`: `"input/mice"`, `SUBSYSTEM`: `"input"`},
 				{`KERNEL`: `"js*"`, `ATTR{attr1}`: `"one"`, `ATTR{attr2}`: `"two"`},
+				{`KERNEL`: `"dma_heap/qcom,qseecom"`},
 			},
 		},
 		{
@@ -471,27 +475,25 @@ apps:
 				},
 				{`KERNEL`: `"input/mice"`, `ATTR{wheel}`: `"true"`},
 				{`KERNEL`: `"js*"`},
+				{`KERNEL`: `"dma_heap/qcom,qseecom"`},
 			},
 		},
 		{
-			`udev-tagging:
-   - kernel: mice
-     attributes:
-      wheel: "true"
-   - kernel: event[0-9]
-     subsystem: input
-     environment:
-      env1: first
-      env2: second|other`,
+			"udev-tagging:\n   - kernel: dma_heap/qcom,qseecom",
 			[]map[string]string{
-				{
-					`KERNEL`:    `"event[0-9]"`,
-					`SUBSYSTEM`: `"input"`,
-					`ENV{env1}`: `"first"`,
-					`ENV{env2}`: `"second|other"`,
-				},
-				{`KERNEL`: `"mice"`, `ATTR{wheel}`: `"true"`},
+				{`KERNEL`: `"input/event[0-9]"`},
+				{`KERNEL`: `"input/mice"`},
+				{`KERNEL`: `"js*"`},
+				{`KERNEL`: `"dma_heap/qcom,qseecom"`},
+			},
+		},
+		{
+			"udev-tagging:\n   - kernel: qcom,qseecom",
+			[]map[string]string{
+				{`KERNEL`: `"input/event[0-9]"`},
+				{`KERNEL`: `"input/mice"`},
 				{`KERNEL`: `"js*"`},
+				{`KERNEL`: `"qcom,qseecom"`},
 			},
 		},
 	}

interfaces/builtin/mount_control.go

@@ -311,7 +311,8 @@ func validateWhatAttr(mountInfo *MountInfo) error {
 		return fmt.Errorf(`mount-control "what" pattern is not clean: %q`, what)
 	}
 
-	if _, err := utils.NewPathPattern(what); err != nil {
+	const allowCommas = true
+	if _, err := utils.NewPathPattern(what, allowCommas); err != nil {
 		return fmt.Errorf(`mount-control "what" setting cannot be used: %v`, err)
 	}
 
@@ -337,7 +338,8 @@ func validateWhereAttr(where string) error {
 		return fmt.Errorf(`mount-control "where" pattern is not clean: %q`, where)
 	}
 
-	if _, err := utils.NewPathPattern(where); err != nil {
+	const allowCommas = true
+	if _, err := utils.NewPathPattern(where, allowCommas); err != nil {
 		return fmt.Errorf(`mount-control "where" setting cannot be used: %v`, err)
 	}
 

interfaces/builtin/mount_control_test.go

@@ -353,3 +353,17 @@ func (s *MountControlInterfaceSuite) TestFunctionfsValidates(c *C) {
 	err := interfaces.BeforeConnectPlug(s.iface, plug)
 	c.Check(err, IsNil)
 }
+
+func (s *MountControlInterfaceSuite) TestMountDevicePathWithCommas(c *C) {
+	plugYaml := `
+  mount:
+  - persistent: true
+    what: /dev/dma_heap/qcom,qseecom
+    where: /mnt/foo,bar
+    options: [rw]
+`
+	snapYaml := fmt.Sprintf(mountControlYaml, plugYaml)
+	plug, _ := MockConnectedPlug(c, snapYaml, nil, "mntctl")
+	err := interfaces.BeforeConnectPlug(s.iface, plug)
+	c.Check(err, IsNil)
+}

interfaces/utils/path_patterns.go

@@ -41,7 +41,7 @@ const (
 // createRegex converts the apparmor-like glob sequence into a regex. Loosely
 // using this as reference:
 // https://gitlab.com/apparmor/apparmor/-/blob/master/parser/parser_regex.c#L107
-func createRegex(pattern string, glob GlobFlags) (string, error) {
+func createRegex(pattern string, glob GlobFlags, allowCommas bool) (string, error) {
 	regex := "^"
 
 	appendGlob := func(defaultGlob, nullGlob string) {
@@ -137,6 +137,9 @@ func createRegex(pattern string, glob GlobFlags) (string, error) {
 			if currentGroupLevel > 0 {
 				itemCountInGroup[currentGroupLevel]++
 				regex += "|"
+			} else if allowCommas {
+				// treat commas outside of groups as literal commas
+				regex += ","
 			} else {
 				return "", fmt.Errorf("cannot use ',' outside of group or character class")
 			}
@@ -160,8 +163,8 @@ func createRegex(pattern string, glob GlobFlags) (string, error) {
 	return regex, nil
 }
 
-func NewPathPattern(pattern string) (*PathPattern, error) {
-	regexPattern, err := createRegex(pattern, globDefault)
+func NewPathPattern(pattern string, allowCommas bool) (*PathPattern, error) {
+	regexPattern, err := createRegex(pattern, globDefault, allowCommas)
 	if err != nil {
 		return nil, err
 	}

interfaces/utils/path_patterns_test.go

@@ -57,14 +57,17 @@ func (s *pathPatternsSuite) TestRegexCreationHappy(c *C) {
 		{`/quoted/bracket/[ab\]c]`, d, `^/quoted/bracket/[ab\]c]$`},
 		{`{[,],}`, d, `^([,]|)$`},
 		{`/path/with/comma[,]`, d, `^/path/with/comma[,]$`},
+		{`/path/with/commas,\,,`, d, `^/path/with/commas,,,$`},
+		{`/path/with/comma,{and[,]group,with\,comma}`, d, `^/path/with/comma,(and[,]group|with,comma)$`},
 		{`/$pecial/c^aracters`, d, `^/\$pecial/c\^aracters$`},
 		{`/in/char/class[^$]`, d, `^/in/char/class[^$]$`},
 	}
 
 	for _, testData := range data {
 		pattern := testData.pattern
 		expectedRegex := testData.expectedRegex
-		regex, err := utils.CreateRegex(pattern, testData.glob)
+		const allowCommas = true
+		regex, err := utils.CreateRegex(pattern, testData.glob, allowCommas)
 		c.Assert(err, IsNil, Commentf("%s", pattern))
 		c.Assert(regex, Equals, expectedRegex, Commentf("%s", pattern))
 		// Also, make sure that the obtained regex is valid
@@ -92,12 +95,50 @@ func (s *pathPatternsSuite) TestRegexCreationUnhappy(c *C) {
 	for _, testData := range data {
 		pattern := testData.pattern
 		expectedError := testData.expectedError
-		pathPattern, err := utils.NewPathPattern(pattern)
+		const allowCommas = false
+		pathPattern, err := utils.NewPathPattern(pattern, allowCommas)
 		c.Assert(pathPattern, IsNil, Commentf("%s", pattern))
 		c.Assert(err, ErrorMatches, expectedError, Commentf("%s", pattern))
 	}
 }
 
+func (s *pathPatternsSuite) TestCreateRegexWithCommas(c *C) {
+	data := []struct {
+		pattern        string
+		allowCommas    bool
+		shouldError    bool
+		expectedString string
+	}{
+		{`/dev/sd{a,b,c}`, false, false, `^/dev/sd(a|b|c)$`},
+		{`/dev/sd{a,b,c}`, true, false, `^/dev/sd(a|b|c)$`},
+		{`/dev/dma_heap/qcom,qseecom`, false, true, `cannot use ',' outside of group or character class`},
+		{`/dev/dma_heap/qcom,qseecom`, true, false, `^/dev/dma_heap/qcom,qseecom$`},
+		{`/path/with/comma[,]`, false, false, `^/path/with/comma[,]$`},
+		{`/path/with/comma[,]`, true, false, `^/path/with/comma[,]$`},
+		{`/path/with/commas,\,,`, false, true, `cannot use ',' outside of group or character class`},
+		{`/path/with/commas,\,,`, true, false, `^/path/with/commas,,,$`},
+		{`/path/with/comma,{and[,]group,with\,comma}`, false, true, `cannot use ',' outside of group or character class`},
+		{`/path/with/comma,{and[,]group,with\,comma}`, true, false, `^/path/with/comma,(and[,]group|with,comma)$`},
+		{`/path/with/empty{}group`, false, true, `invalid number of items between {}:.*`},
+		{`/path/with/empty{}group`, true, true, `invalid number of items between {}:.*`},
+	}
+
+	for _, testData := range data {
+		pattern := testData.pattern
+		allowCommas := testData.allowCommas
+		shouldError := testData.shouldError
+		expectedString := testData.expectedString
+		regex, err := utils.CreateRegex(pattern, utils.GlobDefault, allowCommas)
+		if shouldError {
+			c.Assert(regex, Equals, "", Commentf("%s", pattern))
+			c.Assert(err, ErrorMatches, expectedString, Commentf("%s", pattern))
+		} else {
+			c.Assert(regex, Equals, expectedString, Commentf("%s", pattern))
+			c.Assert(err, IsNil, Commentf("%s", pattern))
+		}
+	}
+}
+
 func (s *pathPatternsSuite) TestPatternMatches(c *C) {
 	data := []struct {
 		pattern       string
@@ -126,7 +167,41 @@ func (s *pathPatternsSuite) TestPatternMatches(c *C) {
 		pattern := testData.pattern
 		testPath := testData.testPath
 		expectedMatch := testData.expectedMatch
-		pathPattern, err := utils.NewPathPattern(pattern)
+		const allowCommas = false
+		pathPattern, err := utils.NewPathPattern(pattern, allowCommas)
+		c.Assert(err, IsNil, Commentf("%s", pattern))
+		c.Assert(pathPattern.Matches(testPath), Equals, expectedMatch, Commentf("%s", pattern))
+	}
+}
+
+func (s *pathPatternsSuite) TestPatternWithCommasMatches(c *C) {
+	data := []struct {
+		pattern       string
+		testPath      string
+		expectedMatch bool
+	}{
+		{`/dev/dma_heap/qcom,qseecom`, `/dev/dma_heap/qcom,qseecom`, true},
+		{`/dev/dma_heap/qcom,[,]qseecom`, `/dev/dma_heap/qcom,qseecom`, false},
+		{`/dev/dma_heap/qcom,[,]qseecom`, `/dev/dma_heap/qcom,,qseecom`, true},
+		{`/dev/dma_heap/qcom,{,[,]}qseecom`, `/dev/dma_heap/qcom,qseecom`, true},
+		{`/dev/dma_heap/qcom,{,[,]}qseecom`, `/dev/dma_heap/qcom,,qseecom`, true},
+		{`/dev/dma_heap/qcom,{\,,}qseecom`, `/dev/dma_heap/qcom,qseecom`, true},
+		{`/dev/dma_heap/qcom,{\,,}qseecom`, `/dev/dma_heap/qcom,,qseecom`, true},
+		{`/dev/dma_heap/qcom,{\,,[,]}qseecom`, `/dev/dma_heap/qcom,,,qseecom`, false},
+	}
+
+	const commaError = `cannot use ',' outside of group or character class`
+
+	for _, testData := range data {
+		pattern := testData.pattern
+		testPath := testData.testPath
+		expectedMatch := testData.expectedMatch
+		const allowCommasFalse = false
+		pathPattern, err := utils.NewPathPattern(pattern, allowCommasFalse)
+		c.Assert(pathPattern, IsNil, Commentf("%s", pattern))
+		c.Assert(err, ErrorMatches, commaError, Commentf("%s", pattern))
+		const allowCommasTrue = true
+		pathPattern, err = utils.NewPathPattern(pattern, allowCommasTrue)
 		c.Assert(err, IsNil, Commentf("%s", pattern))
 		c.Assert(pathPattern.Matches(testPath), Equals, expectedMatch, Commentf("%s", pattern))
 	}

overlord/hookstate/ctlcmd/mount.go

@@ -68,7 +68,8 @@ func matchMountPathAttribute(path string, attribute interface{}, snapInfo *snap.
 
 	expandedPattern := snapInfo.ExpandSnapVariables(pattern)
 
-	pp, err := utils.NewPathPattern(expandedPattern)
+	const allowCommas = true
+	pp, err := utils.NewPathPattern(expandedPattern, allowCommas)
 	return err == nil && pp.Matches(path)
 }
 

overlord/hookstate/ctlcmd/mount_test.go

@@ -145,6 +145,12 @@ func (s *mountSuite) SetUpTest(c *C) {
 					"options":    []string{"bind", "ro"},
 					"persistent": false,
 				},
+				map[string]interface{}{
+					"what":       "/dev/dma_heap/qcom,qseecom",
+					"where":      "/dest,with,commas",
+					"options":    []string{"ro"},
+					"persistent": false,
+				},
 			},
 		},
 	}
@@ -369,3 +375,27 @@ func (s *mountSuite) TestHappyWithVariableExpansion(c *C) {
 		{"/path/unit.mount"},
 	})
 }
+
+func (s *mountSuite) TestHappyWithCommasInPath(c *C) {
+	s.injectSnapWithProperPlug(c)
+
+	s.sysd.EnsureMountUnitFileWithOptionsResult = ResultForEnsureMountUnitFileWithOptions{"/path/unit.mount", nil}
+
+	// Now try with commas in the paths
+	_, _, err := ctlcmd.Run(s.mockContext, []string{"mount", "-o", "ro", "/dev/dma_heap/qcom,qseecom", "/dest,with,commas"}, 0)
+	c.Check(err, IsNil)
+	c.Check(s.sysd.EnsureMountUnitFileWithOptionsCalls, DeepEquals, []*systemd.MountUnitOptions{
+		{
+			Lifetime: systemd.Transient,
+			SnapName: "snap1",
+			Revision: "1",
+			What:     "/dev/dma_heap/qcom,qseecom",
+			Where:    "/dest,with,commas",
+			Options:  []string{"ro"},
+			Origin:   "mount-control",
+		},
+	})
+	c.Check(s.sysd.StartCalls, DeepEquals, [][]string{
+		{"/path/unit.mount"},
+	})
+}