[sestarnix] Add permissions checks to fs_node_setsecurity()

When a file node's "security.selinux" xattr is modified, the system
validates that the task has "relabelfrom" and "relabelto" rights to
the new and old Security Contexts, respectively.

The new Security Context should also be checked for the "associate"
permission to the file-system; this check is made, but the result
ignored, until correct file-system labels are implemented.

Bug: 351195217, 355180447
Change-Id: I9f80f3064da711117d639adfb63de9b6880013ef
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1112132
Commit-Queue: Auto-Submit <[email protected]>
Reviewed-by: Nelly Vouzoukidou <[email protected]>
Fuchsia-Auto-Submit: Wez <[email protected]>

Author: drwez

src/starnix/kernel/security/hooks.rs

@@ -1284,7 +1284,7 @@ mod tests {
     }
 
     #[fuchsia::test]
-    async fn fs_node_setsecurity_selinux_enforcing_invalid_context_succeeds() {
+    async fn fs_node_setsecurity_selinux_enforcing_invalid_context_fails() {
         spawn_kernel_with_selinux_hooks_test_policy_and_run(
             |locked, current_task, _security_server| {
                 let node = &testing::create_test_file(locked, current_task).entry.node;
@@ -1300,12 +1300,9 @@ mod tests {
                     "!".into(), // Note: Not a valid security context.
                     XattrOp::Set,
                 )
-                .is_ok());
+                .is_err());
 
-                assert_eq!(
-                    Some(SecurityId::initial(InitialSid::Unlabeled)),
-                    selinux_hooks::get_cached_sid(node)
-                );
+                assert_eq!(before_sid, selinux_hooks::get_cached_sid(node));
             },
         )
     }
@@ -1422,10 +1419,12 @@ mod tests {
     #[fuchsia::test]
     async fn fs_node_getsecurity_delegates_to_get_xattr() {
         spawn_kernel_with_selinux_hooks_test_policy_and_run(
-            |locked, current_task, _security_server| {
+            |locked, current_task, security_server| {
                 let node = &testing::create_test_file(locked, current_task).entry.node;
 
                 // Set an invalid value in `node`'s "security.selinux" attribute.
+                // This requires SELinux to be in permissive mode, otherwise the "relabelto" permission check will fail.
+                security_server.set_enforcing(false);
                 const TEST_VALUE: &str = "Something Random";
                 fs_node_setsecurity(
                     locked,
@@ -1436,6 +1435,7 @@ mod tests {
                     XattrOp::Set,
                 )
                 .expect("set_xattr(security.selinux) failed");
+                security_server.set_enforcing(true);
 
                 // Reading the security attribute should pass-through to read the value from the file system.
                 let result = fs_node_getsecurity(

src/starnix/kernel/security/selinux_hooks/mod.rs

@@ -273,9 +273,6 @@ pub(super) fn fs_node_init_on_create(
     // Compute both the SID to store on the in-memory node and the xattr to persist on-disk
     // (or None if this circumstance is such that there's no xattr to persist).
     let (sid, xattr) = match label.scheme {
-        FileSystemLabelingScheme::Mountpoint => {
-            return Ok(None);
-        }
         FileSystemLabelingScheme::FsUse { fs_use_type, .. } => {
             let current_task_sid = current_task.read().security_state.attrs.current_sid;
             if fs_use_type == FsUseType::Task {
@@ -299,7 +296,7 @@ pub(super) fn fs_node_init_on_create(
                 (sid, xattr)
             }
         }
-        FileSystemLabelingScheme::GenFsCon => {
+        FileSystemLabelingScheme::Mountpoint | FileSystemLabelingScheme::GenFsCon => {
             // The label in this case is decided in the `fs_node_init_with_dentry` hook.
             return Ok(None);
         }
@@ -639,23 +636,90 @@ pub(super) fn fs_node_setsecurity<L>(
 where
     L: LockEqualOrBefore<FileOpsCore>,
 {
-    fs_node.ops().set_xattr(
+    if name != FsStr::new(XATTR_NAME_SELINUX.to_bytes()) {
+        return fs_node.ops().set_xattr(
+            &mut locked.cast_locked::<FileOpsCore>(),
+            fs_node,
+            current_task,
+            name,
+            value,
+            op,
+        );
+    }
+
+    // If the "security.selinux" attribute is being modified then the result depends on the
+    // `FileSystem`'s labeling scheme.
+    let fs = fs_node.fs();
+    let fs_label = match &mut *fs.security_state.state.0.lock() {
+        FileSystemLabelState::Unlabeled { .. } => {
+            // If the `FileSystem` has not yet been labeled then store the xattr but leave the
+            // label on the in-memory `fs_node` to be set up when the `FileSystem`'s labeling state
+            // has been initialized, during load of the initial policy.
+            return fs_node.ops().set_xattr(
+                &mut locked.cast_locked::<FileOpsCore>(),
+                fs_node,
+                current_task,
+                name,
+                value,
+                op,
+            );
+        }
+        FileSystemLabelState::Labeled { label } => label.clone(),
+    };
+
+    // If the "mountpoint"-labeling is used by this filesystem then setting labels is not supported.
+    // TODO: - Is re-labeling of "genfscon" nodes allowed?
+    if fs_label.scheme == FileSystemLabelingScheme::Mountpoint {
+        return error!(ENOTSUP);
+    }
+
+    // TODO: - Lock the `fs_node` security label here, to ensure consistency.
+
+    // Verify that the requested modification is permitted by the loaded policy.
+    let new_sid = security_server.security_context_to_sid(value.into()).ok();
+    if security_server.is_enforcing() {
+        let new_sid = new_sid.ok_or_else(|| errno!(EINVAL))?;
+        let task_sid = current_task.read().security_state.attrs.current_sid;
+        let old_sid = fs_node_effective_sid(fs_node);
+        let file_class = file_class_from_file_mode(fs_node.info().mode)?;
+        let permission_check = security_server.as_permission_check();
+        check_permission(
+            &permission_check,
+            task_sid,
+            old_sid,
+            CommonFilePermission::RelabelFrom.for_class(file_class),
+        )?;
+        check_permission(
+            &permission_check,
+            task_sid,
+            new_sid,
+            CommonFilePermission::RelabelTo.for_class(file_class),
+        )?;
+        check_permission(
+            &permission_check,
+            new_sid,
+            fs_label.sid,
+            FileSystemPermission::Associate,
+        )?;
+    }
+
+    // Apply the change to the file node.
+    let result = fs_node.ops().set_xattr(
         &mut locked.cast_locked::<FileOpsCore>(),
         fs_node,
         current_task,
         name,
         value,
         op,
-    )?;
-    if name == FsStr::new(XATTR_NAME_SELINUX.to_bytes()) {
-        // If the new value is a valid Security Context then label the node with the corresponding
-        // SID, otherwise use the policy's "unlabeled" SID.
-        let sid = security_server
-            .security_context_to_sid(value.into())
-            .unwrap_or(SecurityId::initial(InitialSid::Unlabeled));
-        set_cached_sid(fs_node, sid)
+    );
+
+    // If the operation succeeded then update the label cached on the file node.
+    if result.is_ok() {
+        let effective_new_sid = new_sid.unwrap_or(SecurityId::initial(InitialSid::Unlabeled));
+        set_cached_sid(fs_node, effective_new_sid);
     }
-    Ok(())
+
+    result
 }
 
 /// Returns the `SecurityId` that should be used for SELinux access control checks against `fs_node`.
@@ -1127,8 +1191,10 @@ mod tests {
                 let dir_entry = &testing::create_test_file(locked, current_task).entry;
                 let node = &dir_entry.node;
 
-                // Store a valid Security Context in the attribute, and ensure any label cached on the `FsNode`
-                // is removed, to force the effective-SID query to resolve the label again.
+                // Store a valid Security Context in the attribute, then clear the cached label and
+                // re-resolve it. The hooks test policy defines that "tmpfs" use "fs_use_xattr"
+                // labeling, which should result in the (valid) label being read from the file, and
+                // the corresponding SID cached.
                 node.ops()
                     .set_xattr(
                         &mut locked.cast_locked::<FileOpsCore>(),
@@ -1139,8 +1205,6 @@ mod tests {
                         XattrOp::Set,
                     )
                     .expect("setxattr");
-
-                // Clear the cached SID and use `fs_node_init_with_dentry()` to re-resolve the label.
                 clear_cached_sid(node);
                 assert_eq!(None, get_cached_sid(node));
                 fs_node_init_with_dentry(locked, &security_server, &current_task, dir_entry)

src/starnix/lib/selinux/src/lib.rs

@@ -327,6 +327,10 @@ common_permission_enum! {
         Link("link"),
         /// Permission to open a file.
         Open("open"),
+        /// Permission checked against the existing label when updating a file's security label.
+        RelabelFrom("relabelfrom"),
+        /// Permission checked against the new label when updating a file's security label.
+        RelabelTo("relabelto"),
         /// Permission to modify attributes, including uid, gid and extended attributes.
         SetAttr("setattr"),
         /// Permission to delete a file or remove a hard link.

src/starnix/lib/selinux/testdata/micro_policies/hooks_tests_policy.conf

@@ -1,6 +1,7 @@
 # handle_unknown deny
 class process
 class file
+class filesystem
 class blk_file
 class chr_file
 class lnk_file
@@ -33,9 +34,10 @@ sid kmod
 sid policy
 sid scmp_packet
 sid devnull
-common file { create open }
+common file { create open relabelfrom relabelto }
 class process { fork transition getsched setsched getpgid setpgid sigchld sigkill sigstop signal ptrace getsession setexec setfscreate setrlimit getrlimit setcurrent dyntransition }
 class file inherits file { execute_no_trans entrypoint }
+class filesystem { associate }
 sensitivity s0;
 dominance { s0 }
 category c0;
@@ -54,6 +56,8 @@ type kernel_t;
 type security_t;
 type unlabeled_t;
 type unlabeled_file_t;
+type tmpfs_t;
+type tmpfs_file_t;
 type test_valid_t;
 type test_different_valid_t;
 type test_getpgid_no_t;
@@ -79,6 +83,11 @@ type test_setsched_yes_t;
 type test_getsid_target_t;
 type test_getsid_yes_t;
 type test_getsid_no_t;
+
+# When the kernel/init process creates a file in "tmpfs", transition to
+# a "tmpfs_file_t" domain.
+type_transition kernel_t tmpfs_t:file tmpfs_file_t;
+
 allow exec_no_trans_source_t executable_file_no_trans_t:file { execute_no_trans };
 allow exec_transition_denied_target_t executable_file_trans_t:file { entrypoint };
 allow exec_transition_source_t exec_transition_target_t:process { transition };
@@ -96,10 +105,28 @@ allow test_setsched_yes_t test_setsched_target_t:process { setsched };
 allow test_getsid_yes_t test_getsid_target_t:process { getsession };
 allow kernel_t self:process { setexec setfscreate setcurrent };
 allow kernel_t test_valid_t:process { dyntransition };
+
+# Allow the kernel/init task to relabel files from the initial "tmpfs_file_t"
+# to the valid test types.
+allow kernel_t tmpfs_file_t:file { relabelfrom };
+allow kernel_t test_valid_t:file { relabelto relabelfrom };
+allow kernel_t test_different_valid_t:file { relabelto };
+
+# Allow files in "tmpfs" to be labeled with the "test_*valid_t" types.
+allow test_valid_t tmpfs_t:filesystem { associate };
+allow test_different_valid_t tmpfs_t:filesystem { associate };
+
 allow test_valid_t self:process { setcurrent };
 allow test_valid_t test_different_valid_t:process { dyntransition };
+
 user u roles object_r level s0 range s0 - s0:c0;
 sid kernel u:object_r:kernel_t:s0 - s0
 sid security u:object_r:security_t:s0
 sid unlabeled u:object_r:unlabeled_t:s0
 sid file u:object_r:unlabeled_file_t:s0
+
+# Apply fs_use_xattr to the test filesystem, which is "tmpfs".
+# This allows some xattr hook tests to clear the label cached in-memory,
+# and re-initialize the node to refresh the cached label from the xattr.
+# Type transition will cause new files to be labeled as "tmpfs_file_t".
+fs_use_xattr tmpfs u:object_r:tmpfs_t:s0 - s0;

src/starnix/lib/selinux/testdata/micro_policies/hooks_tests_policy.pp

@@ -46,7 +46,6 @@
                 "*open*",
                 "*perf_event*",
                 "*readlink*",
-                "*relabel*",
                 "*rename*",
                 "*rxdir*",
                 "*sem*",