Re: [PATCH] landlock: Use f_cred in security_file_open() hook

From: Kees Cook
Date: Thu Mar 07 2024 - 12:54:32 EST


On Thu, Mar 07, 2024 at 10:52:03AM +0100, Mickaël Salaün wrote:
> Use landlock_cred(file->f_cred)->domain instead of
> landlock_get_current_domain() in security_file_open() hook
> implementation.
>
> This should not change the current behavior but could avoid potential
> race conditions in case of current task's credentials change.
>
> This will also ensure consistency with upcoming audit support relying on
> file->f_cred.
>
> Add and use a new get_fs_domain() helper to mask non-filesystem domains.
>
> file->f_cred is set by path_openat()/alloc_empty_file()/init_file() just
> before calling security_file_alloc().
>
> Cc: Christian Brauner <brauner@xxxxxxxxxx>
> Cc: Günther Noack <gnoack@xxxxxxxxxx>
> Cc: Jann Horn <jannh@xxxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: Paul Moore <paul@xxxxxxxxxxxxxx>
> Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxx>
> Link: https://lore.kernel.org/r/20240307095203.1467189-1-mic@xxxxxxxxxxx

This looks sensible to me. It follows best practices[1] for avoiding
confused deputy attacks as well.

Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>

[1] https://docs.kernel.org/security/credentials.html?highlight=confused+deputy#open-file-credentials

--
Kees Cook