parser: fix rule flag generation change_mount type rules

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1048
made it so rules like

  mount slave /snap/bin/** -> /**,

  mount /snap/bin/** -> /**,

would get passed into change_mount_type rule generation when they
shouldn't have been. This would result in two different errors.

1. If kernel mount flags were present on the rule. The error would
   be caught causing an error to be returned, causing profile compilation
   to fail.

2. If the rule did not contain explicit flags then rule would generate
   change_mount_type permissions based on souly the mount point. And
   the implied set of flags. However this is incorrect as it should
   not generate change_mount permissions for this type of rule. Not
   only does it ignore the source/device type condition but it
   generates permissions that were never intended.

   When used in combination with a deny prefix this overly broad
   rule can result in almost all mount rules being denied, as the
   denial takes priority over the allow mount rules.

Fixes: https://bugs.launchpad.net/apparmor/+bug/2023814
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1211989
Fixes: 9d3f8c6cc ("parser: fix parsing of source as mount point for propagation type flags")
Fixes: MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1048

Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 86d193e183)
Signed-off-by: John Johansen <john.johansen@canonical.com>

parser/mount.cc

@@ -996,7 +996,7 @@ int mnt_rule::gen_flag_rules(Profile &prof, int &count, unsigned int flags,
 		if (!dev_type && !opts &&
 		    gen_policy_bind_mount(prof, count, flags, opt_flags) == RULE_ERROR)
 			return RULE_ERROR;
-		if (!dev_type && !opts &&
+		if ((!device || !mnt_point) && !dev_type && !opts &&
 		    gen_policy_change_mount_type(prof, count, flags, opt_flags) == RULE_ERROR)
 			return RULE_ERROR;
 		if (!dev_type && !opts &&
@@ -1012,7 +1012,7 @@ int mnt_rule::gen_flag_rules(Profile &prof, int &count, unsigned int flags,
 		return gen_policy_bind_mount(prof, count, flags, opt_flags);
 	} else if ((allow & AA_MAY_MOUNT) &&
 		   (flags & (MS_MAKE_CMDS))
-		   && !dev_type && !opts) {
+		   && (!device || !mnt_point) && !dev_type && !opts) {
 		return gen_policy_change_mount_type(prof, count, flags, opt_flags);
 	} else if ((allow & AA_MAY_MOUNT) && (flags & MS_MOVE)
 		   && !dev_type && !opts) {

parser/tst/equality.sh

@@ -563,6 +563,17 @@ verify_binary_equality "link rules slash filtering" \
                         @{BAR}=/mnt/
                            /t { link @{FOO}/foo -> @{BAR}/bar, }" \
 
+
+# This can potentially fail as ideally it requires a better dfa comparison
+# routine as it can generates hormomorphic dfas. The enumeration of the
+# dfas dumped will be different, even if the binary is the same
+# Note: this test in the future will require -O filter-deny and
+# -O minimize and -O remove-unreachable.
+verify_binary_equality "mount specific deny doesn't affect non-overlapping" \
+			"/t { mount options=bind /e/ -> /**, }" \
+			"/t { audit deny mount /s/** -> /**,
+			      mount options=bind /e/ -> /**, }"
+
 if [ $fails -ne 0 -o $errors -ne 0 ]
 then
 	printf "ERRORS: %d\nFAILS: %d\n" $errors $fails 2>&1

parser/tst/simple_tests/mount/bad_opt_32.sd

@@ -0,0 +1,6 @@
+#
+#=Description test we fail make rules with source and mntpnt associated with MR 1054
+#=EXRESULT FAIL
+/usr/bin/foo {
+    mount options=(slave) /snap/bin/** -> /**,
+}

parser/tst/simple_tests/mount/bad_opt_35.sd

@@ -0,0 +1,6 @@
+#
+#=Description test we fail make rules with source and mntpnt associated with MR 1054
+#=EXRESULT FAIL
+/usr/bin/foo {
+    mount options=(rslave) /snap/bin/** -> /**,
+}

parser/tst/simple_tests/mount/bad_opt_36.sd

@@ -0,0 +1,6 @@
+#
+#=Description test we fail make rules with source and mntpnt associated with MR 1054
+#=EXRESULT FAIL
+/usr/bin/foo {
+    mount options=(unbindable) /snap/bin/** -> /**,
+}

parser/tst/simple_tests/mount/bad_opt_37.sd

@@ -0,0 +1,6 @@
+#
+#=Description test we fail make rules with source and mntpnt associated with MR 1054
+#=EXRESULT FAIL
+/usr/bin/foo {
+    mount options=(runbindable) /snap/bin/** -> /**,
+}

parser/tst/simple_tests/mount/bad_opt_38.sd

@@ -0,0 +1,6 @@
+#
+#=Description test we fail make rules with source and mntpnt associated with MR 1054
+#=EXRESULT FAIL
+/usr/bin/foo {
+    mount options=(private) /snap/bin/** -> /**,
+}

parser/tst/simple_tests/mount/bad_opt_39.sd

@@ -0,0 +1,6 @@
+#
+#=Description test we fail make rules with source and mntpnt associated with MR 1054
+#=EXRESULT FAIL
+/usr/bin/foo {
+    mount options=(rprivate) /snap/bin/** -> /**,
+}

parser/tst/simple_tests/mount/bad_opt_40.sd

@@ -0,0 +1,6 @@
+#
+#=Description test we fail make rules with source and mntpnt associated with MR 1054
+#=EXRESULT FAIL
+/usr/bin/foo {
+    mount options=(shared) /snap/bin/** -> /**,
+}

parser/tst/simple_tests/mount/bad_opt_41.sd

@@ -0,0 +1,6 @@
+#
+#=Description test we fail make rules with source and mntpnt associated with MR 1054
+#=EXRESULT FAIL
+/usr/bin/foo {
+    mount options=(rshared) /snap/bin/** -> /**,
+}

parser/tst/simple_tests/mount/ok_opt_84.sd

@@ -0,0 +1,8 @@
+#
+#=Description test we can parse rules associated with MR 1054
+#=EXRESULT PASS
+/usr/bin/foo {
+    mount options=(slave) /**,
+    mount options=(slave) -> /**,
+    mount /snap/bin/** -> /**,
+}

tests/regression/apparmor/mount.sh

@@ -398,6 +398,16 @@ else
 	runchecktest "UMOUNT (confined cap umount:ALL)" pass umount ${loop_device} ${mount_point}
 	remove_mnt
 
+	# MR:https://gitlab.com/apparmor/apparmor/-/merge_requests/1054
+	# https://bugs.launchpad.net/apparmor/+bug/2023814
+	# https://bugzilla.opensuse.org/show_bug.cgi?id=1211989
+	# based on rules from profile in bug that triggered issue
+	genprofile cap:sys_admin "qual=deny:mount:/snap/bin/:-> /**" \
+				 "mount:options=(rw,bind):-> ${mount_point}/"
+
+	runchecktest "MOUNT (confined cap bind mount with deny mount that doesn't overlap)" pass mount ${mount_point2} ${mount_point} -o bind
+	remove_mnt
+
 	test_options
 fi
 

utils/test/test-parser-simple-tests.py

@@ -107,6 +107,14 @@ exception_not_raised = [
     'mount/bad_opt_29.sd',
     'mount/bad_opt_30.sd',
     'mount/bad_opt_31.sd',
+    'mount/bad_opt_32.sd',
+    'mount/bad_opt_35.sd',
+    'mount/bad_opt_36.sd',
+    'mount/bad_opt_37.sd',
+    'mount/bad_opt_38.sd',
+    'mount/bad_opt_39.sd',
+    'mount/bad_opt_40.sd',
+    'mount/bad_opt_41.sd',
     'profile/flags/flags_bad10.sd',
     'profile/flags/flags_bad11.sd',
     'profile/flags/flags_bad12.sd',