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

Kees Cook
2024-03-07T17:54:23Z

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@kernel.org>
> Cc: Günther Noack <gnoack@google.com>
> Cc: Jann Horn <jannh@google.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20240307095203.1467189-1-mic@digikod.net

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

Reviewed-by: Kees Cook <keescook@chromium.org>

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

--
Kees Cook