linux-kernel - Re: [GIT PULL] execve updates for v6.13-rc1 (take 2)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87mshhn8cg.fsf@email.froward.int.ebiederm.org>
Date: Fri, 29 Nov 2024 13:33:19 -0600
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: Casey Schaufler <casey@...aufler-ca.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,  Kees Cook
 <kees@...nel.org>,  linux-kernel@...r.kernel.org,  Christophe JAILLET
 <christophe.jaillet@...adoo.fr>,  Nir Lichtman <nir@...htman.org>,  Tycho
 Andersen <tandersen@...flix.com>,  Vegard Nossum
 <vegard.nossum@...cle.com>,  linux-security-module@...r.kernel.org,  Al
 Viro <viro@...iv.linux.org.uk>
Subject: Re: [GIT PULL] execve updates for v6.13-rc1 (take 2)

Casey and the smack folks my apologies for copying you in.

I just read the code below a little more carefully and it is definitely
a systemd bug.

mac_smack_read_fd reads the xattr that smack will apply as a label if it
is present.  So there is no reason for systemd to apply the label
itself.  Worse smack_bprm_creds_for_exec applies checks before
applying the label (aka is the superblock trusted) that systemd doesn't.

Which means systemd might apply a label from a smack xattr when
smack wouldn't.

> static int setup_smack(
>                 const ExecParameters *params,
>                 const ExecContext *context,
>                 int executable_fd) {
>         int r;
>
>         assert(params);
>         assert(executable_fd >= 0);
>
>         if (context->smack_process_label) {
>                 r = mac_smack_apply_pid(0, context->smack_process_label);
>                 if (r < 0)
>                         return r;
>         } else if (params->fallback_smack_process_label) {
>                 _cleanup_free_ char *exec_label = NULL;
>
>                 r = mac_smack_read_fd(executable_fd, SMACK_ATTR_EXEC, &exec_label);
>                 if (r < 0 && !ERRNO_IS_XATTR_ABSENT(r))
>                         return r;
>
>                 r = mac_smack_apply_pid(0, exec_label ?: params->fallback_smack_process_label);
>                 if (r < 0)
>                         return r;
>         }
>
>         return 0;
> }

Which means the systemd code should really be:

> static int setup_smack(
>                 const ExecParameters *params,
>                 const ExecContext *context) {
>         int r;
>
>         assert(params);
>         if (context->smack_process_label) {
>                 r = mac_smack_apply_pid(0, context->smack_process_label);
>                 if (r < 0)
>                         return r;
>         } else if (params->fallback_smack_process_label) {
>                 r = mac_smack_apply_pid(0, params->fallback_smack_process_label);
>                 if (r < 0)
>                         return r;
>         }
>
>         return 0;
> }

At which point systemd has no need to open the executable file
descriptor and thus no need to play with fexecve.

Eric