parser: fix parsing of source as mount point for propagation type flags

Before 300889c3a, mount rules would compile policy when using source
as mount point for rules that contain propagation type flags, such as
unbindable, runbindable, private, rprivate, slave, rslave, shared, and
rshared. Even though it compiled, the rule generated would not work as
expected.

This commit fixes both issues. It allows the usage of source as mount
point for the specified flags, albeit with a deprecation warning, and
it correctly generates the mount rule.

The policy fails to load when both source and mount point are
specified, keeping the original behavior (reference
parser/tst/simple_tests/mount/bad_opt_10.sd for example).

Fixes: https://bugs.launchpad.net/bugs/1648245
Fixes: https://bugs.launchpad.net/bugs/2023025

Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>

parser/mount.cc

@@ -828,15 +828,30 @@ int mnt_rule::gen_policy_change_mount_type(Profile &prof, int &count,
 	std::string optsbuf;
 	char class_mount_hdr[64];
 	const char *vec[5];
+	char *mountpoint = mnt_point;
 
 	sprintf(class_mount_hdr, "\\x%02x", AA_CLASS_MOUNT);
 
-	/* change type base rules can not be conditional on device,
-	 * device type or data
+	/* change type base rules can specify the mount point by using
+	 * the parser token position reserved to device. that's why if
+	 * the mount point is not specified, we use device in its
+	 * place. this is a deprecated behavior.
+	 *
+	 * change type base rules can not be conditional on device
+	 * (source), device type or data
 	 */
 	/* rule class single byte header */
 	mntbuf.assign(class_mount_hdr);
-	if (!convert_entry(mntbuf, mnt_point))
+	if (flags && flags != MS_ALL_FLAGS && device && mnt_point) {
+		PERROR("source and mount point cannot be used at the "
+		       "same time for propagation type flags");
+		goto fail;
+	} else if (device && !mnt_point) {
+		pwarn(WARN_DEPRECATED, _("The use of source as mount point for "
+					 "propagation type flags is deprecated.\n"));
+		mountpoint = device;
+	}
+	if (!convert_entry(mntbuf, mountpoint))
 		goto fail;
 	vec[0] = mntbuf.c_str();
 	/* skip device and type */
@@ -981,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 (!device && !dev_type && !opts &&
+		if (!dev_type && !opts &&
 		    gen_policy_change_mount_type(prof, count, flags, opt_flags) == RULE_ERROR)
 			return RULE_ERROR;
 		if (!dev_type && !opts &&
@@ -997,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 ((perms & AA_MAY_MOUNT) &&
 		   (flags & (MS_MAKE_CMDS))
-		   && !device && !dev_type && !opts) {
+		   && !dev_type && !opts) {
 		return gen_policy_change_mount_type(prof, count, flags, opt_flags);
 	} else if ((perms & AA_MAY_MOUNT) && (flags & MS_MOVE)
 		   && !dev_type && !opts) {

parser/tst/simple_tests/mount/ok_opt_68.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "unbindable" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=unbindable /1,
+  mount options=(unbindable) /2,
+  mount options=(rw,unbindable) /3,
+  mount options in (unbindable) /4,
+  mount options in (ro,unbindable) /5,
+}

parser/tst/simple_tests/mount/ok_opt_69.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "runbindable" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=runbindable /1,
+  mount options=(runbindable) /2,
+  mount options=(rw,runbindable) /3,
+  mount options in (runbindable) /4,
+  mount options in (ro,runbindable) /5,
+}

parser/tst/simple_tests/mount/ok_opt_70.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "rprivate" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=rprivate /1,
+  mount options=(rprivate) /2,
+  mount options=(rw,rprivate) /3,
+  mount options in (rprivate) /4,
+  mount options in (ro,rprivate) /5,
+}

parser/tst/simple_tests/mount/ok_opt_71.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "private" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=private /1,
+  mount options=(private) /2,
+  mount options=(rw,private) /3,
+  mount options in (private) /4,
+  mount options in (ro,private) /5,
+}

parser/tst/simple_tests/mount/ok_opt_72.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "slave" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=slave /1,
+  mount options=(slave) /2,
+  mount options=(rw,slave) /3,
+  mount options in (slave) /4,
+  mount options in (ro,slave) /5,
+}

parser/tst/simple_tests/mount/ok_opt_73.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "rslave" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=rslave /1,
+  mount options=(rslave) /2,
+  mount options=(rw,rslave) /3,
+  mount options in (rslave) /4,
+  mount options in (ro,rslave) /5,
+}

parser/tst/simple_tests/mount/ok_opt_74.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "shared" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=shared /1,
+  mount options=(shared) /2,
+  mount options=(rw,shared) /3,
+  mount options in (shared) /4,
+  mount options in (ro,shared) /5,
+}

parser/tst/simple_tests/mount/ok_opt_75.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "rshared" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=rshared /1,
+  mount options=(rshared) /2,
+  mount options=(rw,rshared) /3,
+  mount options in (rshared) /4,
+  mount options in (ro,rshared) /5,
+}

parser/tst/simple_tests/mount/ok_opt_76.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "make-unbindable" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=make-unbindable /1,
+  mount options=(make-unbindable) /2,
+  mount options=(rw,make-unbindable) /3,
+  mount options in (make-unbindable) /4,
+  mount options in (ro,make-unbindable) /5,
+}

parser/tst/simple_tests/mount/ok_opt_77.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "make-runbindable" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=make-runbindable /1,
+  mount options=(make-runbindable) /2,
+  mount options=(rw,make-runbindable) /3,
+  mount options in (make-runbindable) /4,
+  mount options in (ro,make-runbindable) /5,
+}

parser/tst/simple_tests/mount/ok_opt_78.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "make-private" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=make-private /1,
+  mount options=(make-private) /2,
+  mount options=(rw,make-private) /3,
+  mount options in (make-private) /4,
+  mount options in (ro,make-private) /5,
+}

parser/tst/simple_tests/mount/ok_opt_79.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "make-rprivate" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=make-rprivate /1,
+  mount options=(make-rprivate) /2,
+  mount options=(rw,make-rprivate) /3,
+  mount options in (make-rprivate) /4,
+  mount options in (ro,make-rprivate) /5,
+}

parser/tst/simple_tests/mount/ok_opt_80.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "make-slave" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=make-slave /1,
+  mount options=(make-slave) /2,
+  mount options=(rw,make-slave) /3,
+  mount options in (make-slave) /4,
+  mount options in (ro,make-slave) /5,
+}

parser/tst/simple_tests/mount/ok_opt_81.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "make-shared" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=make-shared /1,
+  mount options=(make-shared) /2,
+  mount options=(rw,make-shared) /3,
+  mount options in (make-shared) /4,
+  mount options in (ro,make-shared) /5,
+}

parser/tst/simple_tests/mount/ok_opt_82.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "make-rslave" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=make-rslave /1,
+  mount options=(make-rslave) /2,
+  mount options=(rw,make-rslave) /3,
+  mount options in (make-rslave) /4,
+  mount options in (ro,make-rslave) /5,
+}

parser/tst/simple_tests/mount/ok_opt_83.sd

@@ -0,0 +1,10 @@
+#
+#=Description basic rules to test the "make-rshared" mount option passing mount point as source (should emit a deprecation warning)
+#=EXRESULT PASS
+/usr/bin/foo {
+  mount options=make-rshared /1,
+  mount options=(make-rshared) /2,
+  mount options=(rw,make-rshared) /3,
+  mount options in (make-rshared) /4,
+  mount options in (ro,make-rshared) /5,
+}

tests/regression/apparmor/mount.sh

@@ -218,6 +218,10 @@ test_propagation_options() {
 	runchecktest "MOUNT (confined cap mount propagation setup $1)" pass mount ${loop_device} ${mount_point}
 	genprofile cap:sys_admin "mount:options=($1)"
 	runchecktest "MOUNT (confined cap mount propagation $1)" pass mount none ${mount_point} -o $1
+	genprofile cap:sys_admin "mount:options=($1):-> ${mount_point}/"
+	runchecktest "MOUNT (confined cap mount propagation $1 mountpoint)" pass mount none ${mount_point} -o $1
+	genprofile cap:sys_admin "mount:options=($1):${mount_point}/"
+	runchecktest "MOUNT (confined cap mount propagation $1 source as mountpoint - deprecated)" pass mount none ${mount_point} -o $1
 	remove_mnt
 
 	genprofile cap:sys_admin "mount:ALL" "qual=deny:mount:options=($1)"