Guard log FIFO with SELinux

native/src/core/lib.rs

@@ -4,7 +4,9 @@
 use base::Utf8CStr;
 use cert::read_certificate;
 use daemon::{daemon_entry, find_apk_path, get_magiskd, MagiskD};
-use logging::{android_logging, magisk_logging, zygisk_logging};
+use logging::{
+    android_logging, magisk_logging, zygisk_close_logd, zygisk_get_logd, zygisk_logging,
+};
 use resetprop::{persist_delete_prop, persist_get_prop, persist_get_props, persist_set_prop};
 
 mod cert;
@@ -30,6 +32,8 @@ pub mod ffi {
         fn android_logging();
         fn magisk_logging();
         fn zygisk_logging();
+        fn zygisk_close_logd();
+        fn zygisk_get_logd() -> i32;
         fn find_apk_path(pkg: &[u8], data: &mut [u8]) -> usize;
         fn read_certificate(fd: i32, version: i32) -> Vec<u8>;
         unsafe fn persist_get_prop(name: *const c_char, prop_cb: Pin<&mut PropCb>);

native/src/core/logging.rs

@@ -165,19 +165,13 @@ fn magisk_log_to_pipe(prio: i32, msg: &Utf8CStr) {
 
 static ZYGISK_LOGD: AtomicI32 = AtomicI32::new(-1);
 
-#[no_mangle]
-extern "C" fn zygisk_close_logd() {
+pub fn zygisk_close_logd() {
     unsafe {
         libc::close(ZYGISK_LOGD.swap(-1, Ordering::Relaxed));
     }
 }
 
-#[no_mangle]
-extern "C" fn zygisk_get_logd() -> i32 {
-    ZYGISK_LOGD.load(Ordering::Relaxed)
-}
-
-fn zygisk_log_to_pipe(prio: i32, msg: &Utf8CStr) {
+pub fn zygisk_get_logd() -> i32 {
     // If we don't have the log pipe set, open the log pipe FIFO. This could actually happen
     // multiple times in the zygote daemon (parent process) because we had to close this
     // file descriptor to prevent crashing.
@@ -207,10 +201,18 @@ fn zygisk_log_to_pipe(prio: i32, msg: &Utf8CStr) {
                 libc::close(ZYGISK_LOGD.swap(fd, Ordering::Relaxed));
             }
         } else {
+            return -1;
+        }
+    }
+    fd
+}
+
+fn zygisk_log_to_pipe(prio: i32, msg: &Utf8CStr) {
+    let fd = zygisk_get_logd();
+    if fd < 0 {
         // Cannot talk to pipe, abort
         return;
     }
-    }
 
     // Block SIGPIPE
     let mut mask: sigset_t;

native/src/core/selinux.cpp

@@ -135,5 +135,5 @@ void restore_tmpcon() {
         setfilecon_at(dfd, entry->d_name, SYSTEM_CON);
 
     string logd = tmp + "/"s LOG_PIPE;
-    setfilecon(logd.data(), MAGISK_FILE_CON);
+    setfilecon(logd.data(), MAGISK_LOG_CON);
 }

native/src/include/magisk.hpp

@@ -38,6 +38,9 @@ constexpr const char *applet_names[] = { "su", "resetprop", nullptr };
 // Unconstrained file type that anyone can access
 #define SEPOL_FILE_TYPE     "magisk_file"
 #define MAGISK_FILE_CON     "u:object_r:" SEPOL_FILE_TYPE ":s0"
+// Log pipe that only root and zygote can open
+#define SEPOL_LOG_TYPE      "magisk_log_file"
+#define MAGISK_LOG_CON      "u:object_r:" SEPOL_LOG_TYPE ":s0"
 
 extern int SDK_INT;
 #define APP_DATA_DIR (SDK_INT >= 24 ? "/data/user_de" : "/data/user")

native/src/sepolicy/rules.cpp

@@ -19,6 +19,8 @@ void sepolicy::magisk_rules() {
     typeattribute(SEPOL_PROC_DOMAIN, "bluetoothdomain");
     type(SEPOL_FILE_TYPE, "file_type");
     typeattribute(SEPOL_FILE_TYPE, "mlstrustedobject");
+    type(SEPOL_LOG_TYPE, "file_type");
+    typeattribute(SEPOL_LOG_TYPE, "mlstrustedobject");
 
     // Make our root domain unconstrained
     allow(SEPOL_PROC_DOMAIN, ALL, ALL, ALL);
@@ -37,10 +39,17 @@ void sepolicy::magisk_rules() {
     allow(ALL, SEPOL_FILE_TYPE, "lnk_file", ALL);
     allow(ALL, SEPOL_FILE_TYPE, "sock_file", ALL);
 
-    // Allow these processes to access MagiskSU
-    const char *clients[]{"zygote", "shell",
-                          "system_app", "platform_app", "priv_app",
-                          "untrusted_app", "untrusted_app_all"};
+    // Only allow zygote to open log pipe
+    allow("zygote", SEPOL_LOG_TYPE, "fifo_file", "open");
+    allow("zygote", SEPOL_LOG_TYPE, "fifo_file", "read");
+    // Allow all processes to output logs
+    allow("domain", SEPOL_LOG_TYPE, "fifo_file", "write");
+
+    // Allow these processes to access MagiskSU and output logs
+    const char *clients[] {
+        "zygote", "shell", "system_app", "platform_app",
+        "priv_app", "untrusted_app", "untrusted_app_all"
+    };
     for (auto type: clients) {
         if (!exists(type))
             continue;

native/src/zygisk/hook.cpp

@@ -167,6 +167,14 @@ DCL_HOOK_FUNC(int, unshare, int flags) {
     return res;
 }
 
+// This is the last moment before the secontext of the process changes
+DCL_HOOK_FUNC(int, selinux_android_setcontext,
+              uid_t uid, bool isSystemServer, const char *seinfo, const char *pkgname) {
+    // Pre-fetch logd before secontext transition
+    zygisk_get_logd();
+    return old_selinux_android_setcontext(uid, isSystemServer, seinfo, pkgname);
+}
+
 // Close file descriptors to prevent crashing
 DCL_HOOK_FUNC(void, android_log_close) {
     if (g_ctx == nullptr || !g_ctx->flags[SKIP_CLOSE_LOG_PIPE]) {
@@ -745,6 +753,7 @@ void hook_functions() {
     PLT_HOOK_REGISTER(android_runtime_dev, android_runtime_inode, fork);
     PLT_HOOK_REGISTER(android_runtime_dev, android_runtime_inode, unshare);
     PLT_HOOK_REGISTER(android_runtime_dev, android_runtime_inode, androidSetCreateThreadFunc);
+    PLT_HOOK_REGISTER(android_runtime_dev, android_runtime_inode, selinux_android_setcontext);
     PLT_HOOK_REGISTER_SYM(android_runtime_dev, android_runtime_inode, "__android_log_close", android_log_close);
     hook_commit();
 

native/src/zygisk/zygisk.hpp

@@ -36,9 +36,6 @@ enum : int {
 #define HIJACK_BIN HIJACK_BIN32
 #endif
 
-extern "C" int zygisk_get_logd();
-extern "C" void zygisk_close_logd();
-
 // Unmap all pages matching the name
 void unmap_all(const char *name);