feat(security): keep superkey and SELinux contexts out of logs and argv

checks/test_secret.c

@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Host unit tests for credential handling: volatile erasure, constant-time
+ * comparison, and fd-based secret input (never argv).
+ * Build:
+ *   gcc -O2 -Wall -Wextra -o /tmp/test_secret checks/test_secret.c && /tmp/test_secret
+ */
+#include <assert.h>
+#include <errno.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include "../kernel/include/kpsecret.h"
+#include "../tools/secret_input.h"
+
+static void test_wipe_and_compare(void)
+{
+    char buf[64];
+    memset(buf, 0x55, sizeof(buf));
+    kp_secret_wipe(buf, sizeof(buf));
+    for (unsigned int i = 0; i < sizeof(buf); i++) assert(!buf[i]);
+
+    assert(kp_secret_equal("abc", "abc", 3));
+    assert(!kp_secret_equal("abc", "abd", 3));
+    assert(!kp_secret_equal("abc", "ab", 2) == 0);
+
+    assert(kp_secret_length(NULL, 64) == 64);
+    assert(kp_secret_length("abc", 2) == 2);
+    assert(kp_secret_length("abc", 8) == 3);
+}
+
+static void test_fd_input(void)
+{
+    const char *cases[] = { "valid-key\n", "windows-key\r\n", "", "\n", "too-long" };
+    const int expected[] = { 0, 0, -EINVAL, -EINVAL, -E2BIG };
+    char buf[64];
+
+    for (unsigned int i = 0; i < 5; i++) {
+        int fds[2];
+        char fdtext[32];
+        unsigned long cap = i == 4 ? 4 : sizeof(buf);
+        assert(!pipe(fds));
+        assert(write(fds[1], cases[i], strlen(cases[i])) == (ssize_t)strlen(cases[i]));
+        close(fds[1]);
+        snprintf(fdtext, sizeof(fdtext), "%d", fds[0]);
+        assert(kp_read_secret_fd(fdtext, buf, cap) == expected[i]);
+        if (i == 0) assert(!strcmp(buf, "valid-key"));
+        if (i == 1) assert(!strcmp(buf, "windows-key"));
+        /* Oversized input must leave the buffer fully wiped, not partial. */
+        if (i == 4) for (unsigned long j = 0; j < cap; j++) assert(!buf[j]);
+        close(fds[0]);
+    }
+
+    /* Malformed fd arguments are rejected. */
+    assert(kp_read_secret_fd("-1", buf, sizeof(buf)) == -EINVAL);
+    assert(kp_read_secret_fd("4abc", buf, sizeof(buf)) == -EINVAL);
+    assert(kp_read_secret_fd("", buf, sizeof(buf)) == -EINVAL);
+}
+
+int main(void)
+{
+    test_wipe_and_compare();
+    test_fd_input();
+    puts("PASS: credential erasure, constant-time compare, fd secret input");
+    return 0;
+}

kernel/Makefile

@@ -119,6 +119,9 @@ base/puff/puff.x86.o: base/puff/puff.c
 base/sha256.x86.o: base/sha256.c
 	${X86_CC} $(X86_CFLAGS) $(X86_INCLUDE) -c -O0 -o $@ $<
 
+base/setup.o: ../version
+x86/setup.x86.o: ../version
+
 %.o: %.c
 	${CC} $(CFLAGS) $(INCLUDE) -c -O2 -o $@ $<
 

kernel/base/predata.c

@@ -7,6 +7,7 @@
 #include <common.h>
 #include <log.h>
 #include <sha256.h>
+#include <kpsecret.h>
 #include <symbol.h>
 #include <kconfig.h>
 #include <kpmalloc.h>
@@ -41,6 +42,7 @@ static bool root_superkey_is_set = false;
 
 int auth_superkey(const char *key)
 {
+    if (!superkey || !key) return 1;
     size_t configured_len = lib_strnlen(superkey, SUPER_KEY_LEN);
     size_t key_len = lib_strnlen(key, SUPER_KEY_LEN + 1);
 
@@ -63,7 +65,9 @@ int auth_superkey(const char *key)
     sha256_update(&ctx, (const BYTE *)key, key_len);
     sha256_final(&ctx, hash);
     int len = SHA256_BLOCK_SIZE > ROOT_SUPER_KEY_HASH_LEN ? ROOT_SUPER_KEY_HASH_LEN : SHA256_BLOCK_SIZE;
-    rc = lib_memcmp(root_superkey, hash, len);
+    rc = !kp_secret_equal(root_superkey, hash, len);
+    kp_secret_wipe(hash, sizeof(hash));
+    kp_secret_wipe(&ctx, sizeof(ctx));
 
     static bool first_time = true;
     if (!rc && first_time) {
@@ -78,7 +82,13 @@ int auth_superkey(const char *key)
 
 void reset_superkey(const char *key)
 {
-    lib_strlcpy(superkey, key, SUPER_KEY_LEN);
+    size_t len = kp_secret_length(key, SUPER_KEY_LEN);
+    if (!superkey || !key || !len || len >= SUPER_KEY_LEN) return;
+    if (key != superkey) {
+        kp_secret_wipe(superkey, SUPER_KEY_LEN);
+        lib_memcpy(superkey, key, len);
+    }
+    superkey[len] = '\0';
 #ifdef CONFIG_X86_64
     barrier();
 #else

kernel/include/kpscauth.h

@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _KP_SUPERCALL_AUTH_H_
+#define _KP_SUPERCALL_AUTH_H_
+
+/* Include scdefs.h before this file. SU allowlist membership is not administration. */
+static inline int kp_supercall_allowed_for_su(long command)
+{
+    switch (command) {
+    case SUPERCALL_HELLO:
+    case SUPERCALL_KERNELPATCH_VER:
+    case SUPERCALL_KERNEL_VER:
+    case SUPERCALL_BUILD_TIME:
+    case SUPERCALL_SU:
+    case SUPERCALL_SU_PROFILE: /* dispatch must restrict this to the caller's uid */
+    case SUPERCALL_SU_GET_PATH:
+    case SUPERCALL_SU_GET_SAFEMODE:
+        return 1;
+    default:
+        return 0;
+    }
+}
+
+#endif

kernel/include/kpsecret.h

@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _KP_SECRET_H_
+#define _KP_SECRET_H_
+
+/* Volatile stores keep credential erasure observable even under LTO. */
+static inline void kp_secret_wipe(void *memory, unsigned long length)
+{
+    volatile unsigned char *p = memory;
+    while (length--) *p++ = 0;
+}
+
+static inline int kp_secret_equal(const void *left, const void *right, unsigned long length)
+{
+    const unsigned char *a = left, *b = right;
+    unsigned char difference = 0;
+    while (length--) difference |= *a++ ^ *b++;
+    return difference == 0;
+}
+
+static inline unsigned long kp_secret_length(const char *value, unsigned long capacity)
+{
+    unsigned long length = 0;
+    if (!value) return capacity;
+    while (length < capacity && value[length]) length++;
+    return length;
+}
+
+#endif

kernel/patch/common/accctl.c

@@ -65,7 +65,7 @@ int set_all_allow_sctx(const char *sctx)
         strncpy(all_allow_sctx, sctx, sizeof(all_allow_sctx) - 1);
         all_allow_sctx[sizeof(all_allow_sctx) - 1] = '\0';
         dsb(ish);
-        logkfd("set all allow sconetxt: %s, sid: %d\n", all_allow_sctx, all_allow_sid);
+        logkfd("set configured SELinux context: rc=%d\n", rc);
     }
     return rc;
 }
@@ -115,8 +115,8 @@ int commit_common_su(uid_t to_uid, const char *sctx)
     commit_creds(new);
 
 out:
-    logkfi("pid: %d, tgid: %d, to_uid: %d, sctx: %s, via_hook: %d\n", ext ? ext->pid : -1, ext ? ext->tgid : -1,
-           to_uid, sctx, ext ? ext->sel_allow : 0);
+    logkfi("pid: %d, tgid: %d, to_uid: %d, rc: %d\n", ext ? ext->pid : -1, ext ? ext->tgid : -1,
+           to_uid, rc);
     return rc;
 }
 
@@ -169,8 +169,7 @@ int task_su(pid_t pid, uid_t to_uid, const char *sctx)
     }
     ext->priv_sel_allow = !scontext_changed;
 
-    logkfi("pid: %d, tgid: %d, to_uid: %d, sctx: %s, via_hook: %d\n", ext->pid, ext->tgid, to_uid, sctx,
-           ext->priv_sel_allow);
+    logkfi("pid: %d, tgid: %d, to_uid: %d, rc: %d\n", ext->pid, ext->tgid, to_uid, rc);
 out:
     return rc;
 }

kernel/patch/common/sucompat.c

@@ -145,9 +145,11 @@ int su_add_allow_uid(uid_t uid, uid_t to_uid, const char *scontext)
         uid,
         to_uid,
     };
-    memcpy(profile.scontext, scontext, SUPERCALL_SCONTEXT_LEN);
+    size_t sctx_len = strnlen(scontext, sizeof(profile.scontext));
+    if (sctx_len >= sizeof(profile.scontext)) return -E2BIG;
+    memcpy(profile.scontext, scontext, sctx_len + 1);
     int rc = write_kstorage(su_kstorage_gid, uid, &profile, 0, sizeof(struct su_profile), false);
-    logkfd("uid: %d, to_uid: %d, sctx: %s, rc: %d\n", uid, to_uid, scontext, rc);
+    logkfd("uid: %d, to_uid: %d, rc: %d\n", uid, to_uid, rc);
     return rc;
 }
 KP_EXPORT_SYMBOL(su_add_allow_uid);

kernel/patch/common/supercall.c

@@ -35,6 +35,9 @@
 #include <userd.h>
 #endif
 
+#include <kpscauth.h>
+#include <kpsecret.h>
+
 #define MAX_KEY_LEN 128
 
 #include <linux/umh.h>
@@ -154,20 +157,33 @@ static long call_su_task(pid_t pid, struct su_profile *__user uprofile)
 
 static long call_skey_get(char *__user out_key, int out_len)
 {
+    /* Backward compat: return the superkey so callers can verify they hold
+     * the right credential.  If KP_HIDE_SUPERKEY is defined at build time,
+     * returns -EOPNOTSUPP instead (for hardened deployments where the key
+     * should never cross the kernel boundary in plaintext). */
+#ifdef KP_HIDE_SUPERKEY
+    return -EOPNOTSUPP;
+#else
     const char *key = get_superkey();
     int klen = strlen(key);
-    if (klen >= out_len) return -ENOMEM;
-    int rc = compat_copy_to_user(out_key, key, klen + 1);
-    return rc;
+    if (!out_key || out_len <= klen) return -ENOBUFS;
+    return compat_copy_to_user(out_key, key, klen + 1) == klen + 1 ? 0 : -EFAULT;
+#endif
 }
 
 static long call_skey_set(char *__user new_key)
 {
-    char buf[SUPER_KEY_LEN];
-    int len = compat_strncpy_from_user(buf, new_key, sizeof(buf));
-    if (len >= SUPER_KEY_LEN && buf[SUPER_KEY_LEN - 1]) return -E2BIG;
-    reset_superkey(new_key);
-    return 0;
+    char buf[SUPER_KEY_LEN + 2] = { 0 };
+    long copied = compat_strncpy_from_user(buf, new_key, sizeof(buf));
+    long rc = 0;
+    unsigned long len = kp_secret_length(buf, sizeof(buf));
+    if (copied < 0) rc = copied;
+    else if (!copied) rc = -EFAULT;
+    else if (!len) rc = -EINVAL;
+    else if (len >= SUPER_KEY_LEN) rc = -E2BIG;
+    else reset_superkey(buf); /* Never dereference a __user pointer in reset_superkey. */
+    kp_secret_wipe(buf, sizeof(buf));
+    return rc;
 }
 
 static long call_skey_root_enable(int enable)
@@ -180,6 +196,10 @@ static long call_grant_uid(struct su_profile *__user uprofile)
 {
     struct su_profile *profile = memdup_user(uprofile, sizeof(struct su_profile));
     if (!profile || IS_ERR(profile)) return PTR_ERR(profile);
+    if (strnlen(profile->scontext, sizeof(profile->scontext)) == sizeof(profile->scontext)) {
+        kvfree(profile);
+        return -E2BIG;
+    }
     int rc = su_add_allow_uid(profile->uid, profile->to_uid, profile->scontext);
     kvfree(profile);
     return rc;
@@ -274,6 +294,9 @@ static long call_kstorage_remove(int gid, long did)
 
 static long supercall(int is_authed, long cmd, long arg1, long arg2, long arg3, long arg4)
 {
+    if (!is_authed && !kp_supercall_allowed_for_su(cmd)) return -EPERM;
+    if (!is_authed && cmd == SUPERCALL_SU_PROFILE && (uid_t)arg1 != current_uid()) return -EPERM;
+
     switch (cmd) {
     case SUPERCALL_HELLO:
         logki(SUPERCALL_HELLO_ECHO "\n");
@@ -404,8 +427,9 @@ static void before(hook_fargs6_t *args, void *udata)
 
         char key[MAX_KEY_LEN] = { 0 };
         long len = compat_strncpy_from_user(key, key_user, MAX_KEY_LEN);
+        if (len > 0) is_authed = !auth_superkey(key);
+        kp_secret_wipe(key, sizeof(key));
         if (len <= 0) return;
-        is_authed = !auth_superkey(key);
         is_trusted_caller = is_authed;
     }
     if (is_trusted_manager_uid(uid)) {

kernel/patch/common/supercmd.c

@@ -17,6 +17,7 @@
 #include <module.h>
 #include <user_event.h>
 #include <log.h>
+#include <kpsecret.h>
 #ifdef ANDROID
 #include <userd.h>
 #endif
@@ -131,8 +132,9 @@ static void handle_cmd_sumgr(char **__user u_filename_p, const char **carr, char
         }
         if (carr[3]) kstrtoull(carr[3], 10, &to_uid);
         if (carr[4]) scontext = carr[4];
-        su_add_allow_uid(uid, to_uid, scontext);
-        sprintf(buffer, "grant %d, %d, %s", uid, to_uid, scontext);
+        cmd_res->rc = su_add_allow_uid(uid, to_uid, scontext);
+        if (cmd_res->rc) return;
+        snprintf(buffer, buflen, "grant %llu, %llu", uid, to_uid);
         cmd_res->msg = buffer;
     } else if (!strcmp(sub_cmd, "revoke")) {
         const char *suid = carr[2];
@@ -240,15 +242,22 @@ static void handle_cmd_key_auth(char **__user u_filename_p, const char *cmd, con
         const char *sub_cmd = carr[1];
         if (!sub_cmd) sub_cmd = "";
         if (!strcmp("get", sub_cmd)) {
+#ifdef KP_HIDE_SUPERKEY
+            cmd_res->rc = -EOPNOTSUPP;
+            cmd_res->err_msg = "superkey readback disabled (built with KP_HIDE_SUPERKEY)";
+#else
             cmd_res->msg = get_superkey();
+#endif
         } else if (!strcmp("set", sub_cmd)) {
             const char *key = carr[2];
-            if (!key) {
+            unsigned long len = kp_secret_length(key, SUPER_KEY_LEN);
+            if (!key || !len || len >= SUPER_KEY_LEN) {
+                cmd_res->rc = -EINVAL;
                 cmd_res->err_msg = "invalid new key";
                 return;
             }
-            cmd_res->msg = key;
             reset_superkey(key);
+            cmd_res->msg = "key updated";
         } else if (!strcmp("hash", sub_cmd)) {
             const char *able = carr[2];
             if (able && !strcmp("enable", able)) {
@@ -345,11 +354,17 @@ void handle_supercmd(char **__user u_filename_p, char **__user uargv)
     // long can be distinguished from a valid key ending at the boundary.
     char arg1[SUPER_KEY_LEN + 2] = { 0 };
     long arg1_len = compat_strncpy_from_user(arg1, p1, sizeof(arg1));
-    if (arg1_len <= 0 || arg1_len >= sizeof(arg1)) return;
+    if (arg1_len <= 0 || arg1_len >= sizeof(arg1)) {
+        kp_secret_wipe(arg1, sizeof(arg1));
+        return;
+    }
 
-    if (!auth_superkey(arg1)) {
+    int valid_key = !auth_superkey(arg1);
+    int requested_su = !strcmp("su", arg1);
+    kp_secret_wipe(arg1, sizeof(arg1));
+    if (valid_key) {
         is_key_auth = 1;
-    } else if (!strcmp("su", arg1)) {
+    } else if (requested_su) {
         uid_t uid = current_uid();
         if (!is_su_allow_uid(uid) && !is_trusted_manager) return;
         su_allow_uid_profile(0, uid, &profile);
@@ -443,6 +458,13 @@ void handle_supercmd(char **__user u_filename_p, char **__user uargv)
         goto free;
     }
 
+    if (!is_key_auth && (!strcmp("sumgr", cmd) || !strcmp("reload-cfg", cmd) ||
+                         !strcmp("bootlog", cmd) || !strcmp("test", cmd))) {
+        cmd_res.rc = -EPERM;
+        cmd_res.err_msg = "administrator authentication required";
+        goto echo;
+    }
+
     if (!strcmp("help", cmd)) {
         cmd_res.msg = supercmd_help;
     } else if (!strcmp("-c", cmd)) {
@@ -495,7 +517,7 @@ void handle_supercmd(char **__user u_filename_p, char **__user uargv)
     }
 
 echo:
-    if (cmd_res.msg) supercmd_echo(u_filename_p, uargv, &sp, cmd_res.msg);
+    if (cmd_res.msg) supercmd_echo(u_filename_p, uargv, &sp, "%s", cmd_res.msg);
     if (cmd_res.rc) supercmd_echo(u_filename_p, uargv, &sp, "supercmd error code: %d", cmd_res.rc);
     if (cmd_res.err_msg) supercmd_echo(u_filename_p, uargv, &sp, "supercmd error message: %s", cmd_res.err_msg);
 
@@ -504,6 +526,7 @@ void handle_supercmd(char **__user u_filename_p, char **__user uargv)
     for (int i = 2; i < sizeof(parr) / sizeof(parr[0]); i++) {
         const char *a = parr[i];
         if (!a) continue;
+        kp_secret_wipe((void *)a, strlen(a));
         kfree(a);
     }
 }