Message-ID: <3d89a03f31cacb53a2ed8017899f2dab10476b62.camel@huaweicloud.com>
Date: Mon, 01 Sep 2025 19:01:42 +0200
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: Andy Lutomirski <luto@...nel.org>
Cc: Aleksa Sarai <cyphar@...har.com>, Mickaël Salaün
 <mic@...ikod.net>, Christian Brauner <brauner@...nel.org>, Al Viro
 <viro@...iv.linux.org.uk>, Kees Cook <keescook@...omium.org>, Paul Moore
 <paul@...l-moore.com>, Serge Hallyn <serge@...lyn.com>, Arnd Bergmann
 <arnd@...db.de>, Christian Heimes <christian@...hon.org>, Dmitry Vyukov
 <dvyukov@...gle.com>, Elliott Hughes <enh@...gle.com>, Fan Wu
 <wufan@...ux.microsoft.com>, Florian Weimer <fweimer@...hat.com>, Jann Horn
 <jannh@...gle.com>, Jeff Xu <jeffxu@...gle.com>, Jonathan Corbet
 <corbet@....net>,  Jordan R Abrahams <ajordanr@...gle.com>, Lakshmi
 Ramasubramanian <nramas@...ux.microsoft.com>, Luca Boccassi
 <bluca@...ian.org>, Matt Bobrowski <mattbobrowski@...gle.com>, Miklos
 Szeredi <mszeredi@...hat.com>, Mimi Zohar <zohar@...ux.ibm.com>, Nicolas
 Bouchinet <nicolas.bouchinet@....cyber.gouv.fr>, Robert Waite
 <rowait@...rosoft.com>,  Roberto Sassu <roberto.sassu@...wei.com>, Scott
 Shell <scottsh@...rosoft.com>, Steve Dower <steve.dower@...hon.org>, Steve
 Grubb <sgrubb@...hat.com>,  kernel-hardening@...ts.openwall.com,
 linux-api@...r.kernel.org,  linux-fsdevel@...r.kernel.org,
 linux-integrity@...r.kernel.org,  linux-kernel@...r.kernel.org,
 linux-security-module@...r.kernel.org
Subject: Re: [RFC PATCH v1 0/2] Add O_DENY_WRITE (complement AT_EXECVE_CHECK)

On Mon, 2025-09-01 at 09:25 -0700, Andy Lutomirski wrote:
> Can you clarify this a bit for those of us who are not well-versed in
> exactly what "measurement" does?
> 
> On Mon, Sep 1, 2025 at 2:42 AM Roberto Sassu
> <roberto.sassu@...weicloud.com> wrote:
> > > Now, in cases where you have IMA or something and you only permit signed
> > > binaries to execute, you could argue there is a different race here (an
> > > attacker creates a malicious script, runs it, and then replaces it with
> > > a valid script's contents and metadata after the fact to get
> > > AT_EXECVE_CHECK to permit the execution). However, I'm not sure that
> > 
> > Uhm, let's consider measurement, I'm more familiar with.
> > 
> > I think the race you wanted to express was that the attacker replaces
> > the good script, verified with AT_EXECVE_CHECK, with the bad script
> > after the IMA verification but before the interpreter reads it.
> > 
> > Fortunately, IMA is able to cope with this situation, since this race
> > can happen for any file open, where of course a file can be not read-
> > locked.
> 
> I assume you mean that this has nothing specifically to do with
> scripts, as IMA tries to protect ordinary (non-"execute" file access)
> as well.  Am I right?

Yes, correct, violations are checked for all open() and mmap()
involving regular files. It would not be special to do it for scripts.

> > If the attacker tries to concurrently open the script for write in this
> > race window, IMA will report this event (called violation) in the
> > measurement list, and during remote attestation it will be clear that
> > the interpreter did not read what was measured.
> > 
> > We just need to run the violation check for the BPRM_CHECK hook too
> > (then, probably for us the O_DENY_WRITE flag or alternative solution
> > would not be needed, for measurement).
> 
> This seems consistent with my interpretation above, but ...

The comment here [1] seems to be clear on why the violation check it is
not done for execution (BPRM_CHECK hook). Since the OS read-locks the
files during execution, this implicitly guarantees that there will not
be concurrent writes, and thus no IMA violations.

However, recently, we took advantage of AT_EXECVE_CHECK to also
evaluate the integrity of scripts (when not executed via ./). Since we
are using the same hook for both executed files (read-locked) and
scripts (I guess non-read-locked), then we need to do a violation check
for BPRM_CHECK too, although it will be redundant for the first
category.

> > Please, let us know when you apply patches like 2a010c412853 ("fs:
> > don't block i_writecount during exec"). We had a discussion [1], but
> > probably I missed when it was decided to be applied (I saw now it was
> > in the same thread, but didn't get that at the time). We would have
> > needed to update our code accordingly. In the future, we will try to
> > clarify better our expectations from the VFS.
> 
> ... I didn't follow this.
> 
> Suppose there's some valid contents of /bin/sleep.  I execute
> /bin/sleep 1m.  While it's running, I modify /bin/sleep (by opening it
> for write, not by replacing it), and the kernel in question doesn't do
> ETXTBSY.  Then the sleep process reads (and executes) the modified
> contents.  Wouldn't a subsequent attestation fail?  Why is ETXTBSY
> needed?

Ok, this is actually a good opportunity to explain what it will be
missing. If you do the operations in the order you proposed, actually a
violation will be emitted, because the violating operation is an open()
and the check is done for this system call.

However, if you do the opposite, first open for write and then
execution, IMA will not be aware of that since it trusts the OS to not
make it happen and will not check for violations.

So yes, in your case the remote attestation will fail (actually it is
up to the remote verifier to decide...). But in the opposite case, the
writer could wait for IMA to measure the genuine content and then
modify the content conveniently. The remote attestation will succeed.

Adding the violation check on BPRM_CHECK should be sufficient to avoid
such situation, but I would try to think if there are other
implications for IMA of not read-locking the files on execution.

Roberto

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/security/integrity/ima/ima_main.c?h=v6.17-rc4#n565

Powered by blists - more mailing lists