i/builtin: check mount flags for conflicts

The mount-control interface used to allow requesting contradicting mount options, such as nodev and dev and would silently pass this down to apparmor_parser. The parser would detect this and cause the failure of the security setup. With this code we now detect this much earlier, at interface validation stage. Fixes: https://warthogs.atlassian.net/browse/SNAPDENG-32984 Signed-off-by: Zygmunt Krynicki <[email protected]>
canonical · Dec 10, 2024 · f5aaf1d · f5aaf1d
1 parent fa1c20b
commit f5aaf1d
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 0 deletions.
diff --git a/interfaces/builtin/mount_control.go b/interfaces/builtin/mount_control.go
@@ -29,6 +29,7 @@ import (
 	"github.com/snapcore/snapd/interfaces"
 	"github.com/snapcore/snapd/interfaces/apparmor"
 	"github.com/snapcore/snapd/interfaces/utils"
+	"github.com/snapcore/snapd/osutil/mount/libmount"
 	apparmor_sandbox "github.com/snapcore/snapd/sandbox/apparmor"
 	"github.com/snapcore/snapd/snap"
 	"github.com/snapcore/snapd/strutil"
@@ -482,8 +483,11 @@ func validateMountOptions(mountInfo *MountInfo) error {
 	} else {
 		types = defaultFSTypes
 	}
+
+	var optsToCheck []string // Kernel options to check for consistency.
 	for _, o := range mountInfo.options {
 		if strutil.ListContains(allowedKernelMountOptions, o) {
+			optsToCheck = append(optsToCheck, o)
 			continue
 		}
 		optionName := strings.SplitAfter(o, "=")[0] // for options with arguments, validate only option
@@ -495,6 +499,11 @@ func validateMountOptions(mountInfo *MountInfo) error {
 		}
 		return fmt.Errorf(`mount-control option unrecognized or forbidden: %q`, o)
 	}
+
+	if err := libmount.ValidateMountOptions(optsToCheck...); err != nil {
+		return fmt.Errorf("mount-control options are inconsistent: %w", err)
+	}
+
 	return nil
 }
 

diff --git a/interfaces/builtin/mount_control_test.go b/interfaces/builtin/mount_control_test.go
@@ -459,6 +459,20 @@ func (s *MountControlInterfaceSuite) TestMountDevicePathWithCommas(c *C) {
 	c.Check(err, IsNil)
 }
 
+func (s *MountControlInterfaceSuite) TestConflictingMountOptions(c *C) {
+	plugYaml := `
+  mount:
+  - persistent: true
+    what: /dev/foo
+    where: /mnt/foo
+    options: [rw, ro]
+`
+	snapYaml := fmt.Sprintf(mountControlYaml, plugYaml)
+	plug, _ := MockConnectedPlug(c, snapYaml, nil, "mntctl")
+	err := interfaces.BeforeConnectPlug(s.iface, plug)
+	c.Check(err, ErrorMatches, "mount-control options are inconsistent: option ro conflicts with rw")
+}
+
 func (s *MountControlInterfaceSuite) TestMountOptionsAreValid(c *C) {
 	// All the options we claim to support are also allowed by the validator.
 	for _, opt := range builtin.AllowedKernelMountOptions() {