From fa0746f2e21e8263630d20e5007d3be9673800f3 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 29 Dec 2023 13:01:01 -0800 Subject: [PATCH 1/5] parser: add special casing for detached move mounts upsteam move_mount mediation now allows for a detached (disconnected) mount to be move mounted into a namespace. Add support for this by detecting 'detached' as a keyword for the source/device and using it to create a null match. Because existing mount encoding using a null separator between the mount terms null match followed by the null seperator will separate detached mounts within the existing encoding. Eg. mount detached -> /destination, mount options=(ro) fstype=ext4 detached -> /destination, This is functionally equivalent to using mount "" -> /destination, However using "" does not provide any context that about what the rule is allowing or why so the 'detached' form is preferred. This is not a perfect solution, but is what can be currently supported by the kernel without more LSM hooks. On kernels that don't support detached mount detection, rules using the detached souce conditional will be ignored (never matched). This encoding also allows the existing mount, mount options=(move), mount options=(move) -> /destination, to continue to work with both detached and regular mounts on kernels that support the move_mount() syscall. Signed-off-by: John Johansen --- parser/mount.cc | 29 +++++++++++++++++++++++++---- parser/parser.h | 1 + parser/parser_common.c | 1 + parser/parser_main.c | 8 ++++++++ 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/parser/mount.cc b/parser/mount.cc index c7130786f..c349e5ca6 100644 --- a/parser/mount.cc +++ b/parser/mount.cc @@ -772,8 +772,17 @@ int mnt_rule::gen_policy_remount(Profile &prof, int &count, goto fail; vec[0] = mntbuf.c_str(); } else { - if (!convert_entry(mntbuf, device)) + if (device && strcmp(device, "detached") == 0) { + /* if (features_supports_detached_mount) ... + * not needed because this is equiv to "" + * which was preivously supported + * + * match nothing + */ + devbuf.clear(); + } else if (!clear_and_convert_entry(devbuf, device)) { goto fail; + } vec[0] = mntbuf.c_str(); } /* skip device */ @@ -844,8 +853,12 @@ int mnt_rule::gen_policy_bind_mount(Profile &prof, int &count, if (!convert_entry(mntbuf, mnt_point)) goto fail; vec[0] = mntbuf.c_str(); - if (!clear_and_convert_entry(devbuf, device)) + if (device && strcmp(device, "detached") == 0) { + /* see note in move_mount. match nothing */ + devbuf.clear(); + } else if (!clear_and_convert_entry(devbuf, device)) { goto fail; + } vec[1] = devbuf.c_str(); /* skip type */ vec[2] = default_match_pattern; @@ -946,8 +959,12 @@ int mnt_rule::gen_policy_move_mount(Profile &prof, int &count, if (!convert_entry(mntbuf, mnt_point)) goto fail; vec[0] = mntbuf.c_str(); - if (!clear_and_convert_entry(devbuf, device)) + if (device && strcmp(device, "detached") == 0) { + /* see note in move_mount. match nothing */ + devbuf.clear(); + } else if (!clear_and_convert_entry(devbuf, device)) { goto fail; + } vec[1] = devbuf.c_str(); /* skip type */ vec[2] = default_match_pattern; @@ -987,8 +1004,12 @@ int mnt_rule::gen_policy_new_mount(Profile &prof, int &count, if (!convert_entry(mntbuf, mnt_point)) goto fail; vec[0] = mntbuf.c_str(); - if (!clear_and_convert_entry(devbuf, device)) + if (device && strcmp(device, "detached") == 0) { + /* see note in move mount. match nothing */ + devbuf.clear(); + } else if (!clear_and_convert_entry(devbuf, device)) { goto fail; + } vec[1] = devbuf.c_str(); typebuf.clear(); if (!build_list_val_expr(typebuf, dev_type)) diff --git a/parser/parser.h b/parser/parser.h index 2f63e65cc..3e0eb5ee9 100644 --- a/parser/parser.h +++ b/parser/parser.h @@ -321,6 +321,7 @@ extern int features_supports_inet; extern int kernel_supports_policydb; extern int kernel_supports_diff_encode; extern int features_supports_mount; +extern bool features_supports_detached_mount; extern int features_supports_dbus; extern int features_supports_signal; extern int features_supports_ptrace; diff --git a/parser/parser_common.c b/parser/parser_common.c index 7da975991..ca9457c94 100644 --- a/parser/parser_common.c +++ b/parser/parser_common.c @@ -73,6 +73,7 @@ int features_supports_inet = 0; /* kernel supports inet network rules */ int features_supports_unix = 0; /* kernel supports unix socket rules */ int kernel_supports_policydb = 0; /* kernel supports new policydb */ int features_supports_mount = 0; /* kernel supports mount rules */ +bool features_supports_detached_mount = false; int features_supports_dbus = 0; /* kernel supports dbus rules */ int kernel_supports_diff_encode = 0; /* kernel supports diff_encode */ int features_supports_signal = 0; /* kernel supports signal rules */ diff --git a/parser/parser_main.c b/parser/parser_main.c index 8234ec9c3..802217dd1 100644 --- a/parser/parser_main.c +++ b/parser/parser_main.c @@ -956,6 +956,14 @@ void set_supported_features() features_supports_mount = features_intersect(kernel_features, policy_features, "mount"); + /* + * note: detached mounts are just a null condition, so previous + * mount rule encoding supports it, if the kernel supports + * it. So support for detached depends on mount intersect and + * kernel detached. + */ + features_supports_detached_mount = aa_features_supports(kernel_features, + "mount/move_mount/detached"); features_supports_dbus = features_intersect(kernel_features, policy_features, "dbus"); features_supports_signal = features_intersect(kernel_features, From d4f75cec2beb66b4985e7c3b452301713de01253 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Tue, 1 Apr 2025 09:55:40 -0700 Subject: [PATCH 2/5] regression: uncomment the detached keyword mount tests Signed-off-by: Ryan Lee --- tests/regression/apparmor/mount.sh | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/regression/apparmor/mount.sh b/tests/regression/apparmor/mount.sh index 58d667ffd..cc859aebf 100755 --- a/tests/regression/apparmor/mount.sh +++ b/tests/regression/apparmor/mount.sh @@ -278,15 +278,15 @@ open_tree_test() { runchecktest "MOVE_MOUNT (confined${desc}: mount options=(move) -> ${mnt_target}/,)" ${result} open_tree ${mnt_source} ${mnt_target} ${fsname} remove_mnt - # genprofile cap:sys_admin "${qualifier}mount: detached -> ${mnt_target}/" ${additional_perms} - # mount ${loop_device} ${mnt_source} - # runchecktest "MOVE_MOUNT (confined${desc}: mount detached -> ${mnt_target}/,)" ${result} open_tree ${mnt_source} ${mnt_target} ${fsname} - # remove_mnt + genprofile cap:sys_admin "${qualifier}mount: detached -> ${mnt_target}/" ${additional_perms} + mount ${loop_device} ${mnt_source} + runchecktest "MOVE_MOUNT (confined${desc}: mount detached -> ${mnt_target}/,)" ${result} open_tree ${mnt_source} ${mnt_target} ${fsname} + remove_mnt - # genprofile cap:sys_admin "${qualifier}mount: options=(move) detached -> ${mnt_target}/" ${additional_perms} - # mount ${loop_device} ${mnt_source} - # runchecktest "MOVE_MOUNT (confined${desc}: mount options=(move) detached -> ${mnt_target}/,)" ${result} open_tree ${mnt_source} ${mnt_target} ${fsname} - # remove_mnt + genprofile cap:sys_admin "${qualifier}mount: options=(move) detached -> ${mnt_target}/" ${additional_perms} + mount ${loop_device} ${mnt_source} + runchecktest "MOVE_MOUNT (confined${desc}: mount options=(move) detached -> ${mnt_target}/,)" ${result} open_tree ${mnt_source} ${mnt_target} ${fsname} + remove_mnt genprofile cap:sys_admin "${qualifier}mount: \"\" -> ${mnt_target}/" ${additional_perms} mount ${loop_device} ${mnt_source} @@ -359,13 +359,13 @@ fsmount_test() { runchecktest "MOVE_MOUNT (confined${desc}: mount options=(move) -> ${mnt_target}/,)" ${result} fsmount ${mnt_source} ${mnt_target} ${fsname} remove_mnt - # genprofile cap:sys_admin "${qualifier}mount: detached -> ${mnt_target}/" ${additional_perms} - # runchecktest "MOVE_MOUNT (confined${desc}: mount detached -> ${mnt_target}/,)" ${result} fsmount ${mnt_source} ${mnt_target} ${fsname} - # remove_mnt + genprofile cap:sys_admin "${qualifier}mount: detached -> ${mnt_target}/" ${additional_perms} + runchecktest "MOVE_MOUNT (confined${desc}: mount detached -> ${mnt_target}/,)" ${result} fsmount ${mnt_source} ${mnt_target} ${fsname} + remove_mnt - # genprofile cap:sys_admin "${qualifier}mount: options=(move) detached -> ${mnt_target}/" ${additional_perms} - # runchecktest "MOVE_MOUNT (confined${desc}: mount options=(move) detached -> ${mnt_target}/,)" ${result} fsmount ${mnt_source} ${mnt_target} ${fsname} - # remove_mnt + genprofile cap:sys_admin "${qualifier}mount: options=(move) detached -> ${mnt_target}/" ${additional_perms} + runchecktest "MOVE_MOUNT (confined${desc}: mount options=(move) detached -> ${mnt_target}/,)" ${result} fsmount ${mnt_source} ${mnt_target} ${fsname} + remove_mnt genprofile cap:sys_admin "${qualifier}mount: \"\" -> ${mnt_target}/" ${additional_perms} runchecktest "MOVE_MOUNT (confined${desc}: mount \"\" -> ${mnt_target}/,)" ${result} fsmount ${mnt_source} ${mnt_target} ${fsname} From 315d999013911e8af7be8b9028d1da9d02edb630 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Tue, 1 Apr 2025 10:10:53 -0700 Subject: [PATCH 3/5] parser: add detached move mounts to simple tests Signed-off-by: Ryan Lee --- parser/tst/simple_tests/mount/ok_24.sd | 7 +++++++ parser/tst/simple_tests/mount/ok_opt_86.sd | 7 +++++++ 2 files changed, 14 insertions(+) create mode 100644 parser/tst/simple_tests/mount/ok_24.sd create mode 100644 parser/tst/simple_tests/mount/ok_opt_86.sd diff --git a/parser/tst/simple_tests/mount/ok_24.sd b/parser/tst/simple_tests/mount/ok_24.sd new file mode 100644 index 000000000..7753c8762 --- /dev/null +++ b/parser/tst/simple_tests/mount/ok_24.sd @@ -0,0 +1,7 @@ +# +#=Description basic detached mount rule +#=EXRESULT PASS +# +/usr/bin/foo { + mount detached -> /foo, +} diff --git a/parser/tst/simple_tests/mount/ok_opt_86.sd b/parser/tst/simple_tests/mount/ok_opt_86.sd new file mode 100644 index 000000000..3835ff882 --- /dev/null +++ b/parser/tst/simple_tests/mount/ok_opt_86.sd @@ -0,0 +1,7 @@ +# +#=Description detached mount rule with options +#=EXRESULT PASS +# +/usr/bin/foo { + mount options=(ro) fstype=ext4 detached -> /destination, +} From 63857a79727cc0f6fd2d39d260685f645570af26 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Tue, 1 Apr 2025 10:31:17 -0700 Subject: [PATCH 4/5] parser: add a detached mount equality test Signed-off-by: Ryan Lee --- parser/tst/equality.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/parser/tst/equality.sh b/parser/tst/equality.sh index abce2e920..6f4ee6082 100755 --- a/parser/tst/equality.sh +++ b/parser/tst/equality.sh @@ -879,6 +879,11 @@ verify_binary_equality "'$p1'x'$p2' link rules slash filtering" \ @{BAR}=/mnt/ /t { $p2 link @{FOO}/foo -> @{BAR}/bar, }" +# Verify equality with mount detached source + +verify_binary_equality "'$p1'x'$p2' mount detached vs empty source" \ + "/t { $p1 mount \"\" -> /destination, }" \ + "/t { $p2 mount detached -> /destination, }" # This can potentially fail as ideally it requires a better dfa comparison # routine as it can generates hormomorphic dfas. The enumeration of the From 2b9f2d2cb702b16d91160a59a3c64d41b5257203 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 4 Apr 2025 16:44:11 -0700 Subject: [PATCH 5/5] utils: tests: mark detached mount as tools wrong The tools are wrong in parsing the detached mount test. Until that can be fixed, mark the tools as wrong. Signed-off-by: John Johansen --- utils/test/test-parser-simple-tests.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/utils/test/test-parser-simple-tests.py b/utils/test/test-parser-simple-tests.py index 42f483390..0695b9bbd 100644 --- a/utils/test/test-parser-simple-tests.py +++ b/utils/test/test-parser-simple-tests.py @@ -441,6 +441,9 @@ syntax_failure = ( 'network/network_ok_17.sd', 'network/network_ok_45.sd', 'network/network_ok_46.sd', + + # detached mount + 'mount/ok_opt_86.sd', )