Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Mickaël Salaün @ 2024-07-18 16:20 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: Kees Cook, Al Viro, Christian Brauner, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <ae769bbfe51a2c1c270739a91defc0dfbd5b8b5a.camel@huaweicloud.com>

On Thu, Jul 18, 2024 at 04:16:45PM +0200, Roberto Sassu wrote:
> On Sat, 2024-07-06 at 16:56 +0200, Mickaël Salaün wrote:
> > On Fri, Jul 05, 2024 at 02:44:03PM -0700, Kees Cook wrote:
> > > On Fri, Jul 05, 2024 at 07:54:16PM +0200, Mickaël Salaün wrote:
> > > > On Thu, Jul 04, 2024 at 05:18:04PM -0700, Kees Cook wrote:
> > > > > On Thu, Jul 04, 2024 at 09:01:34PM +0200, Mickaël Salaün wrote:
> > > > > > Such a secure environment can be achieved with an appropriate access
> > > > > > control policy (e.g. mount's noexec option, file access rights, LSM
> > > > > > configuration) and an enlighten ld.so checking that libraries are
> > > > > > allowed for execution e.g., to protect against illegitimate use of
> > > > > > LD_PRELOAD.
> > > > > > 
> > > > > > Scripts may need some changes to deal with untrusted data (e.g. stdin,
> > > > > > environment variables), but that is outside the scope of the kernel.
> > > > > 
> > > > > If the threat model includes an attacker sitting at a shell prompt, we
> > > > > need to be very careful about how process perform enforcement. E.g. even
> > > > > on a locked down system, if an attacker has access to LD_PRELOAD or a
> > > > 
> > > > LD_PRELOAD should be OK once ld.so will be patched to check the
> > > > libraries.  We can still imagine a debug library used to bypass security
> > > > checks, but in this case the issue would be that this library is
> > > > executable in the first place.
> > > 
> > > Ah yes, that's fair: the shell would discover the malicious library
> > > while using AT_CHECK during resolution of the LD_PRELOAD.
> > 
> > That's the idea, but it would be checked by ld.so, not the shell.
> > 
> > > 
> > > > > seccomp wrapper (which you both mention here), it would be possible to
> > > > > run commands where the resulting process is tricked into thinking it
> > > > > doesn't have the bits set.
> > > > 
> > > > As explained in the UAPI comments, all parent processes need to be
> > > > trusted.  This meeans that their code is trusted, their seccomp filters
> > > > are trusted, and that they are patched, if needed, to check file
> > > > executability.
> > > 
> > > But we have launchers that apply arbitrary seccomp policy, e.g. minijail
> > > on Chrome OS, or even systemd on regular distros. In theory, this should
> > > be handled via other ACLs.
> > 
> > Processes running with untrusted seccomp filter should be considered
> > untrusted.  It would then make sense for these seccomp filters/programs
> > to be considered executable code, and then for minijail and systemd to
> > check them with AT_CHECK (according to the securebits policy).
> > 
> > > 
> > > > > But this would be exactly true for calling execveat(): LD_PRELOAD or
> > > > > seccomp policy could have it just return 0.
> > > > 
> > > > If an attacker is allowed/able to load an arbitrary seccomp filter on a
> > > > process, we cannot trust this process.
> > > > 
> > > > > 
> > > > > While I like AT_CHECK, I do wonder if it's better to do the checks via
> > > > > open(), as was originally designed with O_MAYEXEC. Because then
> > > > > enforcement is gated by the kernel -- the process does not get a file
> > > > > descriptor _at all_, no matter what LD_PRELOAD or seccomp tricks it into
> > > > > doing.
> > > > 
> > > > Being able to check a path name or a file descriptor (with the same
> > > > syscall) is more flexible and cover more use cases.
> > > 
> > > If flexibility costs us reliability, I think that flexibility is not
> > > a benefit.
> > 
> > Well, it's a matter of letting user space do what they think is best,
> > and I think there are legitimate and safe uses of path names, even if I
> > agree that this should not be used in most use cases.  Would we want
> > faccessat2(2) to only take file descriptor as argument and not file
> > path? I don't think so but I'd defer to the VFS maintainers.
> > 
> > Christian, Al, Linus?
> > 
> > Steve, could you share a use case with file paths?
> > 
> > > 
> > > > The execveat(2)
> > > > interface, including current and future flags, is dedicated to file
> > > > execution.  I then think that using execveat(2) for this kind of check
> > > > makes more sense, and will easily evolve with this syscall.
> > > 
> > > Yeah, I do recognize that is feels much more natural, but I remain
> > > unhappy about how difficult it will become to audit a system for safety
> > > when the check is strictly per-process opt-in, and not enforced by the
> > > kernel for a given process tree. But, I think this may have always been
> > > a fiction in my mind. :)
> > 
> > Hmm, I'm not sure to follow. Securebits are inherited, so process tree.
> > And we need the parent processes to be trusted anyway.
> > 
> > > 
> > > > > And this thinking also applies to faccessat() too: if a process can be
> > > > > tricked into thinking the access check passed, it'll happily interpret
> > > > > whatever. :( But not being able to open the fd _at all_ when O_MAYEXEC
> > > > > is being checked seems substantially safer to me...
> > > > 
> > > > If attackers can filter execveat(2), they can also filter open(2) and
> > > > any other syscalls.  In all cases, that would mean an issue in the
> > > > security policy.
> > > 
> > > Hm, as in, make a separate call to open(2) without O_MAYEXEC, and pass
> > > that fd back to the filtered open(2) that did have O_MAYEXEC. Yes, true.
> > > 
> > > I guess it does become morally equivalent.
> > > 
> > > Okay. Well, let me ask about usability. Right now, a process will need
> > > to do:
> > > 
> > > - should I use AT_CHECK? (check secbit)
> > > - if yes: perform execveat(AT_CHECK)
> > > 
> > > Why not leave the secbit test up to the kernel, and then the program can
> > > just unconditionally call execveat(AT_CHECK)?
> > 
> > That was kind of the approach of the previous patch series and Linus
> > wanted the new interface to follow the kernel semantic.  Enforcing this
> > kind of restriction will always be the duty of user space anyway, so I
> > think it's simpler (i.e. no mix of policy definition, access check, and
> > policy enforcement, but a standalone execveat feature), more flexible,
> > and it fully delegates the policy enforcement to user space instead of
> > trying to enforce some part in the kernel which would only give the
> > illusion of security/policy enforcement.
> 
> A problem could be that from IMA perspective there is no indication on
> whether the interpreter executed or not execveat(). Sure, we can detect
> that the binary supports it, but if the enforcement was
> enabled/disabled that it is not recorded.

We should assume that if the interpreter call execveat+AT_CHECK, it will
enforce restrictions according to its securebits.

> 
> Maybe, setting the process flags should be influenced by the kernel,
> for example not allowing changes and enforcing when there is an IMA
> policy loaded requiring to measure/appraise scripts.

LSMs can set the required securebits per task/interpreter according to
their policies.

> 
> Roberto
> 
> > > 
> > > Though perhaps the issue here is that an execveat() EINVAL doesn't
> > > tell the program if AT_CHECK is unimplemented or if something else
> > > went wrong, and the secbit prctl() will give the correct signal about
> > > AT_CHECK availability?
> > 
> > This kind of check could indeed help to identify the issue.
> 
> 

^ permalink raw reply

* Re: [RFC PATCH v19 0/5] Script execution control (was O_MAYEXEC)
From: Mickaël Salaün @ 2024-07-18 16:21 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: James Bottomley, Mimi Zohar, Al Viro, Christian Brauner,
	Kees Cook, Linus Torvalds, Paul Moore, Theodore Ts'o,
	Alejandro Colomar, Aleksa Sarai, Andrew Morton, Andy Lutomirski,
	Arnd Bergmann, Casey Schaufler, Christian Heimes, Dmitry Vyukov,
	Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Nicolas Bouchinet, Scott Shell, Shuah Khan, Stephen Rothwell,
	Steve Dower, Steve Grubb, Thibaut Sautereau, Vincent Strubel,
	Xiaoming Ni, Yin Fengwei, kernel-hardening, linux-api,
	linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <20240716.leeV4ooveinu@digikod.net>

On Tue, Jul 16, 2024 at 07:31:45PM +0200, Mickaël Salaün wrote:
> On Tue, Jul 16, 2024 at 12:12:49PM -0400, James Bottomley wrote:
> > On Tue, 2024-07-16 at 17:57 +0200, Roberto Sassu wrote:
> > > But the Clip OS 4 patch does not cover the redirection case:
> > > 
> > > # ./bash < /root/test.sh
> > > Hello World
> > > 
> > > Do you have a more recent patch for that?
> 
> Bash was only partially restricted for CLIP OS because it was used for
> administrative tasks (interactive shell).
> 
> Python was also restricted for user commands though:
> https://github.com/clipos-archive/clipos4_portage-overlay/blob/master/dev-lang/python/files/python-2.7.9-clip-mayexec.patch
> 
> Steve and Christian could help with a better Python implementation.

I'll include a toy interpreter in the next patch series.  That should
help for experiments.

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Jeff Xu @ 2024-07-18 22:54 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Steve Dower, Al Viro, Christian Brauner, Kees Cook,
	Linus Torvalds, Paul Moore, Theodore Ts'o, Alejandro Colomar,
	Aleksa Sarai, Andrew Morton, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Christian Heimes, Dmitry Vyukov, Eric Biggers,
	Eric Chiang, Fan Wu, Florian Weimer, Geert Uytterhoeven,
	James Morris, Jan Kara, Jann Horn, Jonathan Corbet,
	Jordan R Abrahams, Lakshmi Ramasubramanian, Luca Boccassi,
	Luis Chamberlain, Madhavan T . Venkataraman, Matt Bobrowski,
	Matthew Garrett, Matthew Wilcox, Miklos Szeredi, Mimi Zohar,
	Nicolas Bouchinet, Scott Shell, Shuah Khan, Stephen Rothwell,
	Steve Grubb, Thibaut Sautereau, Vincent Strubel, Xiaoming Ni,
	Yin Fengwei, kernel-hardening, linux-api, linux-fsdevel,
	linux-integrity, linux-kernel, linux-security-module,
	Elliott Hughes
In-Reply-To: <20240718.ahph4che5Shi@digikod.net>

On Thu, Jul 18, 2024 at 5:23 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> On Wed, Jul 17, 2024 at 06:51:11PM -0700, Jeff Xu wrote:
> > On Wed, Jul 17, 2024 at 3:00 AM Mickaël Salaün <mic@digikod.net> wrote:
> > >
> > > On Wed, Jul 17, 2024 at 09:26:22AM +0100, Steve Dower wrote:
> > > > On 17/07/2024 07:33, Jeff Xu wrote:
> > > > > Consider those cases: I think:
> > > > > a> relying purely on userspace for enforcement does't seem to be
> > > > > effective,  e.g. it is trivial  to call open(), then mmap() it into
> > > > > executable memory.
> > > >
> > > > If there's a way to do this without running executable code that had to pass
> > > > a previous execveat() check, then yeah, it's not effective (e.g. a Python
> > > > interpreter that *doesn't* enforce execveat() is a trivial way to do it).
> > > >
> > > > Once arbitrary code is running, all bets are off. So long as all arbitrary
> > > > code is being checked itself, it's allowed to do things that would bypass
> > > > later checks (and it's up to whoever audited it in the first place to
> > > > prevent this by not giving it the special mark that allows it to pass the
> > > > check).
> > >
> > We will want to define what is considered as "arbitrary code is running"
> >
> > Using an example of ROP, attackers change the return address in stack,
> > e.g. direct the execution flow to a gauge to call "ld.so /tmp/a.out",
> > do you consider "arbitrary code is running" when stack is overwritten
> > ? or after execve() is called.
>
> Yes, ROP is arbitrary code execution (which can be mitigated with CFI).
> ROP could be enough to interpret custom commands and create a small
> interpreter/VM.
>
> > If it is later, this patch can prevent "ld.so /tmp/a.out".
> >
> > > Exactly.  As explained in the patches, one crucial prerequisite is that
> > > the executable code is trusted, and the system must provide integrity
> > > guarantees.  We cannot do anything without that.  This patches series is
> > > a building block to fix a blind spot on Linux systems to be able to
> > > fully control executability.
> >
> > Even trusted executable can have a bug.
>
> Definitely, but this patch series is dedicated to script execution
> control.
>
> >
> > I'm thinking in the context of ChromeOS, where all its system services
> > are from trusted partitions, and legit code won't load .so from a
> > non-exec mount.  But we want to sandbox those services, so even under
> > some kind of ROP attack, the service still won't be able to load .so
> > from /tmp. Of course, if an attacker can already write arbitrary
> > length of data into the stack, it is probably already a game over.
> >
>
> OK, you want to tie executable file permission to mmap.  That makes
> sense if you have a consistent execution model.  This can be enforced by
> LSMs.  Contrary to script interpretation which is a full user space
> implementation (and then controlled by user space), mmap restrictions
> should indeed be enforced by the kernel.
Ya, that is what I meant. it can be out of scope for this patch.
Indeed, as you point out, this patch is dedicated to script execution
control, and fixing ld.so /tmp/a.out is an extra bonus in addition to
script.

Thanks
-Jeff

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Jeff Xu @ 2024-07-19  1:29 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <20240718.kaePhei9Ahm9@digikod.net>

On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@digikod.net> wrote:
> > >
> > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > > >
> > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > executable code, scripts could also use this check [1].
> > > > >
> > > > > This is different than faccessat(2) which only checks file access
> > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > real execution, user space gets the same error codes.
> > > > >
> > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > exec, shared object, script and config file (such as seccomp config),
> > >
> > > "config file" that contains executable code.
> > >
> > Is seccomp config  considered as "contains executable code", seccomp
> > config is translated into bpf, so maybe yes ? but bpf is running in
> > the kernel.
>
> Because seccomp filters alter syscalls, they are similar to code
> injection.
>
that makes sense.

> >
> > > > I'm still thinking  execveat(AT_CHECK) vs faccessat(AT_CHECK) in
> > > > different use cases:
> > > >
> > > > execveat clearly has less code change, but that also means: we can't
> > > > add logic specific to exec (i.e. logic that can't be applied to
> > > > config) for this part (from do_execveat_common to
> > > > security_bprm_creds_for_exec) in future.  This would require some
> > > > agreement/sign-off, I'm not sure from whom.
> > >
> > > I'm not sure to follow. We could still add new flags, but for now I
> > > don't see use cases.  This patch series is not meant to handle all
> > > possible "trust checks", only executable code, which makes sense for the
> > > kernel.
> > >
> > I guess the "configfile" discussion is where I get confused, at one
> > point, I think this would become a generic "trust checks" api for
> > everything related to "generating executable code", e.g. javascript,
> > java code, and more.
> > We will want to clearly define the scope of execveat(AT_CHECK)
>
> The line between data and code is blurry.  For instance, a configuration
> file can impact the execution flow of a program.  So, where to draw the
> line?
>
> It might makes sense to follow the kernel and interpreter semantic: if a
> file can be executed by the kernel (e.g. ELF binary, file containing a
> shebang, or just configured with binfmt_misc), then this should be
> considered as executable code.  This applies to Bash, Python,
> Javascript, NodeJS, PE, PHP...  However, we can also make a picture
> executable with binfmt_misc.  So, again, where to draw the line?
>
> I'd recommend to think about interaction with the outside, through
> function calls, IPCs, syscalls...  For instance, "running" an image
> should not lead to reading or writing to arbitrary files, or accessing
> the network, but in practice it is legitimate for some file formats...
> PostScript is a programming language, but mostly used to draw pictures.
> So, again, where to draw the line?
>
> We should follow the principle of least astonishment.  What most users
> would expect?  This should follow the *common usage* of executable
> files.  At the end, the script interpreters will be patched by security
> folks for security reasons.  I think the right question to ask should
> be: could this file format be (ab)used to leak or modify arbitrary
> files, or to perform arbitrary syscalls?  If the answer is yes, then it
> should be checked for executability.  Of course, this excludes bugs
> exploited in the file format parser.
>
> I'll extend the next patch series with this rationale.
>
> >
> > > If we want other checks, we'll need to clearly define their semantic and
> > > align with the kernel.  faccessat2(2) might be used to check other file
> > > properties, but the executable property is not only defined by the file
> > > attributes.
> > >
> > Agreed.
> >
> > > >
> > > > --------------------------
> > > > now looked at user cases (focus on elf for now)
> > > >
> > > > 1> ld.so /tmp/a.out, /tmp/a.out is on non-exec mount
> > > > dynamic linker will first call execveat(fd, AT_CHECK) then execveat(fd)
> > > >
> > > > 2> execve(/usr/bin/some.out) and some.out has dependency on /tmp/a.so
> > > > /usr/bin/some.out will pass AT_CHECK
> > > >
> > > > 3> execve(usr/bin/some.out) and some.out uses custom /tmp/ld.so
> > > > /usr/bin/some.out will pass AT_CHECK, however, it uses a custom
> > > > /tmp/ld.so (I assume this is possible  for elf header will set the
> > > > path for ld.so because kernel has no knowledge of that, and
> > > > binfmt_elf.c allocate memory for ld.so during execveat call)
> > > >
> > > > 4> dlopen(/tmp/a.so)
> > > > I assume dynamic linker will call execveat(AT_CHECK), before map a.so
> > > > into memory.
> > > >
> > > > For case 1>
> > > > Alternative solution: Because AT_CHECK is always called, I think we
> > > > can avoid the first AT_CHECK call, and check during execveat(fd),
> > >
> > > There is no need to use AT_CHECK if we're going to call execveat(2) on
> > > the same file descriptor.  By design, AT_CHECK is implicit for any
> > > execve(2).
> > >
> > Yes. I realized I was wrong to say that ld.so will call execve() for
> > /tmp/a.out, there is no execve() call, otherwise it would have been
> > blocked already today.
> > The ld.so will  mmap the /tmp/a.out directly.  So case 1 is no
> > different than case 2 and 4.  ( the elf objects are mapped to memory
> > by dynamic linker.)
> > I'm not familiar with dynamic linker, Florian is on this thread, and
> > can help to correct me if my guess is wrong.
> >
> > > > this means the kernel will enforce SECBIT_EXEC_RESTRICT_FILE = 1, the
> > > > benefit is that there is no TOCTOU and save one round trip of syscall
> > > > for a succesful execveat() case.
> > >
> > > As long as user space uses the same file descriptor, there is no TOCTOU.
> > >
> > > SECBIT_EXEC_RESTRICT_FILE only makes sense for user space: it defines
> > > the user space security policy.  The kernel already enforces the same
> > > security policy for any execve(2), whatever are the calling process's
> > > securebits.
> > >
> > > >
> > > > For case 2>
> > > > dynamic linker will call execve(AT_CHECK), then mmap(fd) into memory.
> > > > However,  the process can all open then mmap() directly, it seems
> > > > minimal effort for an attacker to walk around such a defence from
> > > > dynamic linker.
> > >
> > > Which process?  What do you mean by "can all open then mmap() directly"?
> > >
> > > In this context the dynamic linker (like its parent processes) is
> > > trusted (guaranteed by the system).
> > >
> > > For case 2, the dynamic linker must check with AT_CHECK all files that
> > > will be mapped, which include /usr/bin/some.out and /tmp/a.so
> > >
> > My point is that the process can work around this by mmap() the file directly.
>
> Yes, see my answer in the other email. The process is trusted.
>
OK. Let's agree that this is out of scope for this patch series.

> >
> > > >
> > > > Alternative solution:
> > > > dynamic linker call AT_CHECK for each .so, kernel will save the state
> > > > (associated with fd)
> > > > kernel will check fd state at the time of mmap(fd, executable memory)
> > > > and enforce SECBIT_EXEC_RESTRICT_FILE = 1
> > >
> > > The idea with AT_CHECK is that there is no kernel side effect, no extra
> > > kernel state, and the semantic is the same as with execve(2).
> > >
> > > This also enables us to check file's executable permission and ignore
> > > it, which is useful in a "permissive mode" when preparing for a
> > > migration without breaking a system, or to do extra integrity checks.
> > For preparing a migration (detect all violations), this is useful.
> > But as a defense mechanism (SECBIT_EXEC_RESTRICT_FILE = 1) , this
> > seems to be weak, at least for elf loading case.
>
> We could add more restrictions, but that is outside the scope of this
> patch series.
>
Agreed.

> >
> > > BTW, this use case would also be more complex with a new openat2(2) flag
> > > like the original O_MAYEXEC.
> > >
> > > >
> > > > Alternative solution 2:
> > > > a new syscall to load the .so and enforce the AT_CHECK in kernel
> > >
> > > A new syscall would be overkill for this feature.  Please see Linus's
> > > comment.
> > >
> > maybe, I was thinking on how to prevent "/tmp/a.o" from getting mmap()
> > to executable memory.
>
> OK, this is another story.
>
> >
> > > >
> > > > This also means, for the solution to be complete, we might want to
> > > > block creation of executable anonymous memory (e.g. by seccomp, ),
> > >
> > > How seccomp could create anonymous memory in user space?
> > > seccomp filters should be treated (and checked with AT_CHECK) as
> > > executable code anyway.
> > >
> > > > unless the user space can harden the creation of  executable anonymous
> > > > memory in some way.
> > >
> > > User space is already in charge of mmapping its own memory.  I don't see
> > > what is missing.
> > >
> > > >
> > > > For case 3>
> > > > I think binfmt_elf.c in the kernel needs to check the ld.so to make
> > > > sure it passes AT_CHECK, before loading it into memory.
> > >
> > > All ELF dependencies are opened and checked with open_exec(), which
> > > perform the main executability checks (with the __FMODE_EXEC flag).
> > > Did I miss something?
> > >
> > I mean the ld-linux-x86-64.so.2 which is loaded by binfmt in the kernel.
> > The app can choose its own dynamic linker path during build, (maybe
> > even statically link one ?)  This is another reason that relying on a
> > userspace only is not enough.
>
> The kernel calls open_exec() on all dependencies, including
> ld-linux-x86-64.so.2, so these files are checked for executability too.
>
This might not be entirely true. iiuc, kernel  calls open_exec for
open_exec for interpreter, but not all its dependency (e.g. libc.so.6)
load_elf_binary() {
   interpreter = open_exec(elf_interpreter);
}

libc.so.6 is opened and mapped by dynamic linker.
so the call sequence is:
 execve(a.out)
  - open exec(a.out)
  - security_bprm_creds(a.out)
  - open the exec(ld.so)
  - call open_exec() for interruptor (ld.so)
  - call execveat(AT_CHECK, ld.so) <-- do we want ld.so going through
the same check and code path as libc.so below ?
  - transfer the control to ld.so)
  - ld.so open (libc.so)
  - ld.so call execveat(AT_CHECK,libc.so) <-- proposed by this patch,
require dynamic linker change.
  - ld.so mmap(libc.so,rx)


> >
> > > However, we must be careful with programs using the (deprecated)
> > > uselib(2). They should also check with AT_CHECK because this syscall
> > > opens the shared library without __FMODE_EXEC (similar to a simple file
> > > open). See
> > > https://lore.kernel.org/all/CAHk-=wiUwRG7LuR=z5sbkFVGQh+7qVB6_1NM0Ny9SVNL1Un4Sw@mail.gmail.com/
> > >
> > > >
> > > > For case 4>
> > > > same as case 2.
> > > >
> > > > Consider those cases: I think:
> > > > a> relying purely on userspace for enforcement does't seem to be
> > > > effective,  e.g. it is trivial  to call open(), then mmap() it into
> > > > executable memory.
> > >
> > > As Steve explained (and is also explained in the patches), it is trivial
> > > if the attacker can already execute its own code, which is too late to
> > > enforce any script execution control.
> > >
> > > > b> if both user space and kernel need to call AT_CHECK, the faccessat
> > > > seems to be a better place for AT_CHECK, e.g. kernel can call
> > > > do_faccessat(AT_CHECK) and userspace can call faccessat(). This will
> > > > avoid complicating the execveat() code path.
> > >
> > > A previous version of this patches series already patched faccessat(2),
> > > but this is not the right place.  faccessat2(2) is dedicated to check
> > > file permissions, not executability (e.g. with mount's noexec).
> > >
> > > >
> > > > What do you think ?
> > >
> > > I think there are some misunderstandings.  Please let me know if it's
> > > clearer now.
> > >
> > I'm still not sure about the user case for dynamic linker (elf
> > loading) case. Maybe this patch is more suitable for scripts?
>
> It's suitable for both, but we could add more restriction on mmap
> with an (existing) LSM.  The kernel already checks for mount's noexec
> when mapping a file, but not for the file permission, which is OK
> because it could be bypassed by coping the content of the file and
> mprotecting it anyway.  For a consistent memory execution control, all
> memory mapping need to be restricted, which is out of scope for this
> patch series.
>
Ok.

> > A detailed user case will help demonstrate the use case for dynamic
> > linker, e.g. what kind of app will benefit from
> > SECBIT_EXEC_RESTRICT_FILE = 1, what kind of threat model are we
> > dealing with , what kind of attack chain we blocked as a result.
>
> I explained that in the patches and in the description of these new
> securebits.  Please point which part is not clear.  The full threat
> model is simple: the TCB includes the kernel and system's files, which
> are integrity-protected, but we don't trust arbitrary data/scripts that
> can be written to user-owned files or directly provided to script
> interpreters.  As for the ptrace restrictions, the dynamic linker
> restrictions helps to avoid trivial bypasses (e.g. with LD_PRELOAD)
> with consistent executability checks.
>
On elf loading case, I'm clear after your last email. However, I'm not
sure if everyone else follows,  I will try to summarize here:
- Problem:  ld.so /tmp/a.out will happily pass, even /tmp/a.out is
mounted as non-exec.
  Solution: ld.so call execveat(AT_CHECK) for a.out before mmap a.out
into memory.

- Problem: a poorly built application (a.out) can have a dependency on
/tmp/a.o, when /tmp/a.o is on non-exec mount,
  Solution: ld.so call execveat(AT_CHECK) for a.o, before mmap a.o into memory.

- Problem: application can call mmap (/tmp/a.out, rx), where /tmp is
on non-exec mount
  This is out of scope, i.e. will require enforcement on mmap(), maybe
through LSM

Thanks
Best regards
-Jeff

-Jeff


> >
> > > >
> > > > Thanks
> > > > -Jeff
> > > >
> > > > > With the information that a script interpreter is about to interpret a
> > > > > script, an LSM security policy can adjust caller's access rights or log
> > > > > execution request as for native script execution (e.g. role transition).
> > > > > This is possible thanks to the call to security_bprm_creds_for_exec().
> > > > >
> > > > > Because LSMs may only change bprm's credentials, use of AT_CHECK with
> > > > > current kernel code should not be a security issue (e.g. unexpected role
> > > > > transition).  LSMs willing to update the caller's credential could now
> > > > > do so when bprm->is_check is set.  Of course, such policy change should
> > > > > be in line with the new user space code.
> > > > >
> > > > > Because AT_CHECK is dedicated to user space interpreters, it doesn't
> > > > > make sense for the kernel to parse the checked files, look for
> > > > > interpreters known to the kernel (e.g. ELF, shebang), and return ENOEXEC
> > > > > if the format is unknown.  Because of that, security_bprm_check() is
> > > > > never called when AT_CHECK is used.
> > > > >
> > > > > It should be noted that script interpreters cannot directly use
> > > > > execveat(2) (without this new AT_CHECK flag) because this could lead to
> > > > > unexpected behaviors e.g., `python script.sh` could lead to Bash being
> > > > > executed to interpret the script.  Unlike the kernel, script
> > > > > interpreters may just interpret the shebang as a simple comment, which
> > > > > should not change for backward compatibility reasons.
> > > > >
> > > > > Because scripts or libraries files might not currently have the
> > > > > executable permission set, or because we might want specific users to be
> > > > > allowed to run arbitrary scripts, the following patch provides a dynamic
> > > > > configuration mechanism with the SECBIT_SHOULD_EXEC_CHECK and
> > > > > SECBIT_SHOULD_EXEC_RESTRICT securebits.
> > > > >
> > > > > This is a redesign of the CLIP OS 4's O_MAYEXEC:
> > > > > https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> > > > > This patch has been used for more than a decade with customized script
> > > > > interpreters.  Some examples can be found here:
> > > > > https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
> > > > >
> > > > > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > > > > Cc: Christian Brauner <brauner@kernel.org>
> > > > > Cc: Kees Cook <keescook@chromium.org>
> > > > > Cc: Paul Moore <paul@paul-moore.com>
> > > > > Link: https://docs.python.org/3/library/io.html#io.open_code [1]
> > > > > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > > > > Link: https://lore.kernel.org/r/20240704190137.696169-2-mic@digikod.net
> > > > > ---
> > > > >
> > > > > New design since v18:
> > > > > https://lore.kernel.org/r/20220104155024.48023-3-mic@digikod.net
> > > > > ---
> > > > >  fs/exec.c                  |  5 +++--
> > > > >  include/linux/binfmts.h    |  7 ++++++-
> > > > >  include/uapi/linux/fcntl.h | 30 ++++++++++++++++++++++++++++++
> > > > >  kernel/audit.h             |  1 +
> > > > >  kernel/auditsc.c           |  1 +
> > > > >  5 files changed, 41 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/fs/exec.c b/fs/exec.c
> > > > > index 40073142288f..ea2a1867afdc 100644
> > > > > --- a/fs/exec.c
> > > > > +++ b/fs/exec.c
> > > > > @@ -931,7 +931,7 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags)
> > > > >                 .lookup_flags = LOOKUP_FOLLOW,
> > > > >         };
> > > > >
> > > > > -       if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > > > > +       if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH | AT_CHECK)) != 0)
> > > > >                 return ERR_PTR(-EINVAL);
> > > > >         if (flags & AT_SYMLINK_NOFOLLOW)
> > > > >                 open_exec_flags.lookup_flags &= ~LOOKUP_FOLLOW;
> > > > > @@ -1595,6 +1595,7 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl
> > > > >                 bprm->filename = bprm->fdpath;
> > > > >         }
> > > > >         bprm->interp = bprm->filename;
> > > > > +       bprm->is_check = !!(flags & AT_CHECK);
> > > > >
> > > > >         retval = bprm_mm_init(bprm);
> > > > >         if (!retval)
> > > > > @@ -1885,7 +1886,7 @@ static int bprm_execve(struct linux_binprm *bprm)
> > > > >
> > > > >         /* Set the unchanging part of bprm->cred */
> > > > >         retval = security_bprm_creds_for_exec(bprm);
> > > > > -       if (retval)
> > > > > +       if (retval || bprm->is_check)
> > > > >                 goto out;
> > > > >
> > > > >         retval = exec_binprm(bprm);
> > > > > diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> > > > > index 70f97f685bff..8ff9c9e33aed 100644
> > > > > --- a/include/linux/binfmts.h
> > > > > +++ b/include/linux/binfmts.h
> > > > > @@ -42,7 +42,12 @@ struct linux_binprm {
> > > > >                  * Set when errors can no longer be returned to the
> > > > >                  * original userspace.
> > > > >                  */
> > > > > -               point_of_no_return:1;
> > > > > +               point_of_no_return:1,
> > > > > +               /*
> > > > > +                * Set by user space to check executability according to the
> > > > > +                * caller's environment.
> > > > > +                */
> > > > > +               is_check:1;
> > > > >         struct file *executable; /* Executable to pass to the interpreter */
> > > > >         struct file *interpreter;
> > > > >         struct file *file;
> > > > > diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> > > > > index c0bcc185fa48..bcd05c59b7df 100644
> > > > > --- a/include/uapi/linux/fcntl.h
> > > > > +++ b/include/uapi/linux/fcntl.h
> > > > > @@ -118,6 +118,36 @@
> > > > >  #define AT_HANDLE_FID          AT_REMOVEDIR    /* file handle is needed to
> > > > >                                         compare object identity and may not
> > > > >                                         be usable to open_by_handle_at(2) */
> > > > > +
> > > > > +/*
> > > > > + * AT_CHECK only performs a check on a regular file and returns 0 if execution
> > > > > + * of this file would be allowed, ignoring the file format and then the related
> > > > > + * interpreter dependencies (e.g. ELF libraries, script's shebang).  AT_CHECK
> > > > > + * should only be used if SECBIT_SHOULD_EXEC_CHECK is set for the calling
> > > > > + * thread.  See securebits.h documentation.
> > > > > + *
> > > > > + * Programs should use this check to apply kernel-level checks against files
> > > > > + * that are not directly executed by the kernel but directly passed to a user
> > > > > + * space interpreter instead.  All files that contain executable code, from the
> > > > > + * point of view of the interpreter, should be checked.  The main purpose of
> > > > > + * this flag is to improve the security and consistency of an execution
> > > > > + * environment to ensure that direct file execution (e.g. ./script.sh) and
> > > > > + * indirect file execution (e.g. sh script.sh) lead to the same result.  For
> > > > > + * instance, this can be used to check if a file is trustworthy according to
> > > > > + * the caller's environment.
> > > > > + *
> > > > > + * In a secure environment, libraries and any executable dependencies should
> > > > > + * also be checked.  For instance dynamic linking should make sure that all
> > > > > + * libraries are allowed for execution to avoid trivial bypass (e.g. using
> > > > > + * LD_PRELOAD).  For such secure execution environment to make sense, only
> > > > > + * trusted code should be executable, which also requires integrity guarantees.
> > > > > + *
> > > > > + * To avoid race conditions leading to time-of-check to time-of-use issues,
> > > > > + * AT_CHECK should be used with AT_EMPTY_PATH to check against a file
> > > > > + * descriptor instead of a path.
> > > > > + */
> > > > > +#define AT_CHECK               0x10000
> > > > > +
> > > > >  #if defined(__KERNEL__)
> > > > >  #define AT_GETATTR_NOSEC       0x80000000
> > > > >  #endif
> > > > > diff --git a/kernel/audit.h b/kernel/audit.h
> > > > > index a60d2840559e..8ebdabd2ab81 100644
> > > > > --- a/kernel/audit.h
> > > > > +++ b/kernel/audit.h
> > > > > @@ -197,6 +197,7 @@ struct audit_context {
> > > > >                 struct open_how openat2;
> > > > >                 struct {
> > > > >                         int                     argc;
> > > > > +                       bool                    is_check;
> > > > >                 } execve;
> > > > >                 struct {
> > > > >                         char                    *name;
> > > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > > > index 6f0d6fb6523f..b6316e284342 100644
> > > > > --- a/kernel/auditsc.c
> > > > > +++ b/kernel/auditsc.c
> > > > > @@ -2662,6 +2662,7 @@ void __audit_bprm(struct linux_binprm *bprm)
> > > > >
> > > > >         context->type = AUDIT_EXECVE;
> > > > >         context->execve.argc = bprm->argc;
> > > > > +       context->execve.is_check = bprm->is_check;
> > > > >  }
> > > > >
> > > > >
> > > > > --
> > > > > 2.45.2
> > > > >
> > > >
> >

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-07-19  8:44 UTC (permalink / raw)
  To: Jeff Xu
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <CALmYWFto4sw-Q2+J0Gc54POhnM9C8YpnJ44wMz=fd_K3_+dWmw@mail.gmail.com>

On Thu, Jul 18, 2024 at 06:29:54PM -0700, Jeff Xu wrote:
> On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@digikod.net> wrote:
> >
> > On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > >
> > > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > >
> > > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > > executable code, scripts could also use this check [1].
> > > > > >
> > > > > > This is different than faccessat(2) which only checks file access
> > > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > > real execution, user space gets the same error codes.
> > > > > >
> > > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > > exec, shared object, script and config file (such as seccomp config),

> > > > > I think binfmt_elf.c in the kernel needs to check the ld.so to make
> > > > > sure it passes AT_CHECK, before loading it into memory.
> > > >
> > > > All ELF dependencies are opened and checked with open_exec(), which
> > > > perform the main executability checks (with the __FMODE_EXEC flag).
> > > > Did I miss something?
> > > >
> > > I mean the ld-linux-x86-64.so.2 which is loaded by binfmt in the kernel.
> > > The app can choose its own dynamic linker path during build, (maybe
> > > even statically link one ?)  This is another reason that relying on a
> > > userspace only is not enough.
> >
> > The kernel calls open_exec() on all dependencies, including
> > ld-linux-x86-64.so.2, so these files are checked for executability too.
> >
> This might not be entirely true. iiuc, kernel  calls open_exec for
> open_exec for interpreter, but not all its dependency (e.g. libc.so.6)

Correct, the dynamic linker is in charge of that, which is why it must
be enlighten with execveat+AT_CHECK and securebits checks.

> load_elf_binary() {
>    interpreter = open_exec(elf_interpreter);
> }
> 
> libc.so.6 is opened and mapped by dynamic linker.
> so the call sequence is:
>  execve(a.out)
>   - open exec(a.out)
>   - security_bprm_creds(a.out)
>   - open the exec(ld.so)
>   - call open_exec() for interruptor (ld.so)
>   - call execveat(AT_CHECK, ld.so) <-- do we want ld.so going through
> the same check and code path as libc.so below ?

open_exec() checks are enough.  LSMs can use this information (open +
__FMODE_EXEC) if needed.  execveat+AT_CHECK is only a user space
request.

>   - transfer the control to ld.so)
>   - ld.so open (libc.so)
>   - ld.so call execveat(AT_CHECK,libc.so) <-- proposed by this patch,
> require dynamic linker change.
>   - ld.so mmap(libc.so,rx)

Explaining these steps is useful. I'll include that in the next patch
series.

> > > A detailed user case will help demonstrate the use case for dynamic
> > > linker, e.g. what kind of app will benefit from
> > > SECBIT_EXEC_RESTRICT_FILE = 1, what kind of threat model are we
> > > dealing with , what kind of attack chain we blocked as a result.
> >
> > I explained that in the patches and in the description of these new
> > securebits.  Please point which part is not clear.  The full threat
> > model is simple: the TCB includes the kernel and system's files, which
> > are integrity-protected, but we don't trust arbitrary data/scripts that
> > can be written to user-owned files or directly provided to script
> > interpreters.  As for the ptrace restrictions, the dynamic linker
> > restrictions helps to avoid trivial bypasses (e.g. with LD_PRELOAD)
> > with consistent executability checks.
> >
> On elf loading case, I'm clear after your last email. However, I'm not
> sure if everyone else follows,  I will try to summarize here:
> - Problem:  ld.so /tmp/a.out will happily pass, even /tmp/a.out is
> mounted as non-exec.
>   Solution: ld.so call execveat(AT_CHECK) for a.out before mmap a.out
> into memory.
> 
> - Problem: a poorly built application (a.out) can have a dependency on
> /tmp/a.o, when /tmp/a.o is on non-exec mount,
>   Solution: ld.so call execveat(AT_CHECK) for a.o, before mmap a.o into memory.
> 
> - Problem: application can call mmap (/tmp/a.out, rx), where /tmp is
> on non-exec mount

I'd say "malicious or non-enlightened processes" can call mmap without
execveat+AT_CHECK...

>   This is out of scope, i.e. will require enforcement on mmap(), maybe
> through LSM

Cool, I'll include that as well. Thanks.

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Jeff Xu @ 2024-07-19 14:16 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <20240719.shaeK6PaiSie@digikod.net>

On Fri, Jul 19, 2024 at 1:45 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> On Thu, Jul 18, 2024 at 06:29:54PM -0700, Jeff Xu wrote:
> > On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@digikod.net> wrote:
> > >
> > > On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > > > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > > >
> > > > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > > >
> > > > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > > > executable code, scripts could also use this check [1].
> > > > > > >
> > > > > > > This is different than faccessat(2) which only checks file access
> > > > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > > > real execution, user space gets the same error codes.
> > > > > > >
> > > > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > > > exec, shared object, script and config file (such as seccomp config),
>
> > > > > > I think binfmt_elf.c in the kernel needs to check the ld.so to make
> > > > > > sure it passes AT_CHECK, before loading it into memory.
> > > > >
> > > > > All ELF dependencies are opened and checked with open_exec(), which
> > > > > perform the main executability checks (with the __FMODE_EXEC flag).
> > > > > Did I miss something?
> > > > >
> > > > I mean the ld-linux-x86-64.so.2 which is loaded by binfmt in the kernel.
> > > > The app can choose its own dynamic linker path during build, (maybe
> > > > even statically link one ?)  This is another reason that relying on a
> > > > userspace only is not enough.
> > >
> > > The kernel calls open_exec() on all dependencies, including
> > > ld-linux-x86-64.so.2, so these files are checked for executability too.
> > >
> > This might not be entirely true. iiuc, kernel  calls open_exec for
> > open_exec for interpreter, but not all its dependency (e.g. libc.so.6)
>
> Correct, the dynamic linker is in charge of that, which is why it must
> be enlighten with execveat+AT_CHECK and securebits checks.
>
> > load_elf_binary() {
> >    interpreter = open_exec(elf_interpreter);
> > }
> >
> > libc.so.6 is opened and mapped by dynamic linker.
> > so the call sequence is:
> >  execve(a.out)
> >   - open exec(a.out)
> >   - security_bprm_creds(a.out)
> >   - open the exec(ld.so)
> >   - call open_exec() for interruptor (ld.so)
> >   - call execveat(AT_CHECK, ld.so) <-- do we want ld.so going through
> > the same check and code path as libc.so below ?
>
> open_exec() checks are enough.  LSMs can use this information (open +
> __FMODE_EXEC) if needed.  execveat+AT_CHECK is only a user space
> request.
>
Then the ld.so doesn't go through the same security_bprm_creds() check
as other .so.

As my previous email, the ChromeOS LSM restricts executable mfd
through security_bprm_creds(), the end result is that ld.so can still
be executable memfd, but not other .so.

One way to address this is to refactor the necessary code from
execveat() code patch, and make it available to call from both kernel
and execveat() code paths., but if we do that, we might as well use
faccessat2(AT_CHECK)


> >   - transfer the control to ld.so)
> >   - ld.so open (libc.so)
> >   - ld.so call execveat(AT_CHECK,libc.so) <-- proposed by this patch,
> > require dynamic linker change.
> >   - ld.so mmap(libc.so,rx)
>
> Explaining these steps is useful. I'll include that in the next patch
> series.
>
> > > > A detailed user case will help demonstrate the use case for dynamic
> > > > linker, e.g. what kind of app will benefit from
> > > > SECBIT_EXEC_RESTRICT_FILE = 1, what kind of threat model are we
> > > > dealing with , what kind of attack chain we blocked as a result.
> > >
> > > I explained that in the patches and in the description of these new
> > > securebits.  Please point which part is not clear.  The full threat
> > > model is simple: the TCB includes the kernel and system's files, which
> > > are integrity-protected, but we don't trust arbitrary data/scripts that
> > > can be written to user-owned files or directly provided to script
> > > interpreters.  As for the ptrace restrictions, the dynamic linker
> > > restrictions helps to avoid trivial bypasses (e.g. with LD_PRELOAD)
> > > with consistent executability checks.
> > >
> > On elf loading case, I'm clear after your last email. However, I'm not
> > sure if everyone else follows,  I will try to summarize here:
> > - Problem:  ld.so /tmp/a.out will happily pass, even /tmp/a.out is
> > mounted as non-exec.
> >   Solution: ld.so call execveat(AT_CHECK) for a.out before mmap a.out
> > into memory.
> >
> > - Problem: a poorly built application (a.out) can have a dependency on
> > /tmp/a.o, when /tmp/a.o is on non-exec mount,
> >   Solution: ld.so call execveat(AT_CHECK) for a.o, before mmap a.o into memory.
> >
> > - Problem: application can call mmap (/tmp/a.out, rx), where /tmp is
> > on non-exec mount
>
> I'd say "malicious or non-enlightened processes" can call mmap without
> execveat+AT_CHECK...
>
> >   This is out of scope, i.e. will require enforcement on mmap(), maybe
> > through LSM
>
> Cool, I'll include that as well. Thanks.

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-07-19 15:04 UTC (permalink / raw)
  To: Jeff Xu
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <CALmYWFsd-=pOPZZmiKvYJ8pOhACsTvW_d+pRjG_C4jD6+Li0AQ@mail.gmail.com>

On Fri, Jul 19, 2024 at 07:16:55AM -0700, Jeff Xu wrote:
> On Fri, Jul 19, 2024 at 1:45 AM Mickaël Salaün <mic@digikod.net> wrote:
> >
> > On Thu, Jul 18, 2024 at 06:29:54PM -0700, Jeff Xu wrote:
> > > On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > >
> > > > On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > > > > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > >
> > > > > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > > > >
> > > > > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > > > > executable code, scripts could also use this check [1].
> > > > > > > >
> > > > > > > > This is different than faccessat(2) which only checks file access
> > > > > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > > > > real execution, user space gets the same error codes.
> > > > > > > >
> > > > > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > > > > exec, shared object, script and config file (such as seccomp config),
> >
> > > > > > > I think binfmt_elf.c in the kernel needs to check the ld.so to make
> > > > > > > sure it passes AT_CHECK, before loading it into memory.
> > > > > >
> > > > > > All ELF dependencies are opened and checked with open_exec(), which
> > > > > > perform the main executability checks (with the __FMODE_EXEC flag).
> > > > > > Did I miss something?
> > > > > >
> > > > > I mean the ld-linux-x86-64.so.2 which is loaded by binfmt in the kernel.
> > > > > The app can choose its own dynamic linker path during build, (maybe
> > > > > even statically link one ?)  This is another reason that relying on a
> > > > > userspace only is not enough.
> > > >
> > > > The kernel calls open_exec() on all dependencies, including
> > > > ld-linux-x86-64.so.2, so these files are checked for executability too.
> > > >
> > > This might not be entirely true. iiuc, kernel  calls open_exec for
> > > open_exec for interpreter, but not all its dependency (e.g. libc.so.6)
> >
> > Correct, the dynamic linker is in charge of that, which is why it must
> > be enlighten with execveat+AT_CHECK and securebits checks.
> >
> > > load_elf_binary() {
> > >    interpreter = open_exec(elf_interpreter);
> > > }
> > >
> > > libc.so.6 is opened and mapped by dynamic linker.
> > > so the call sequence is:
> > >  execve(a.out)
> > >   - open exec(a.out)
> > >   - security_bprm_creds(a.out)
> > >   - open the exec(ld.so)
> > >   - call open_exec() for interruptor (ld.so)
> > >   - call execveat(AT_CHECK, ld.so) <-- do we want ld.so going through
> > > the same check and code path as libc.so below ?
> >
> > open_exec() checks are enough.  LSMs can use this information (open +
> > __FMODE_EXEC) if needed.  execveat+AT_CHECK is only a user space
> > request.
> >
> Then the ld.so doesn't go through the same security_bprm_creds() check
> as other .so.

Indeed, but...

> 
> As my previous email, the ChromeOS LSM restricts executable mfd
> through security_bprm_creds(), the end result is that ld.so can still
> be executable memfd, but not other .so.

The chromeOS LSM can check that with the security_file_open() hook and
the __FMODE_EXEC flag, see Landlock's implementation.  I think this
should be the only hook implementation that chromeOS LSM needs to add.

> 
> One way to address this is to refactor the necessary code from
> execveat() code patch, and make it available to call from both kernel
> and execveat() code paths., but if we do that, we might as well use
> faccessat2(AT_CHECK)

That's why I think it makes sense to rely on the existing __FMODE_EXEC
information.

> 
> 
> > >   - transfer the control to ld.so)
> > >   - ld.so open (libc.so)
> > >   - ld.so call execveat(AT_CHECK,libc.so) <-- proposed by this patch,
> > > require dynamic linker change.
> > >   - ld.so mmap(libc.so,rx)
> >
> > Explaining these steps is useful. I'll include that in the next patch
> > series.

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Jeff Xu @ 2024-07-19 15:12 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <20240718.kaePhei9Ahm9@digikod.net>

On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@digikod.net> wrote:
> > >
> > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > > >
> > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > executable code, scripts could also use this check [1].
> > > > >
> > > > > This is different than faccessat(2) which only checks file access
> > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > real execution, user space gets the same error codes.
> > > > >
> > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > exec, shared object, script and config file (such as seccomp config),
> > >
> > > "config file" that contains executable code.
> > >
> > Is seccomp config  considered as "contains executable code", seccomp
> > config is translated into bpf, so maybe yes ? but bpf is running in
> > the kernel.
>
> Because seccomp filters alter syscalls, they are similar to code
> injection.
>
> >
> > > > I'm still thinking  execveat(AT_CHECK) vs faccessat(AT_CHECK) in
> > > > different use cases:
> > > >
> > > > execveat clearly has less code change, but that also means: we can't
> > > > add logic specific to exec (i.e. logic that can't be applied to
> > > > config) for this part (from do_execveat_common to
> > > > security_bprm_creds_for_exec) in future.  This would require some
> > > > agreement/sign-off, I'm not sure from whom.
> > >
> > > I'm not sure to follow. We could still add new flags, but for now I
> > > don't see use cases.  This patch series is not meant to handle all
> > > possible "trust checks", only executable code, which makes sense for the
> > > kernel.
> > >
> > I guess the "configfile" discussion is where I get confused, at one
> > point, I think this would become a generic "trust checks" api for
> > everything related to "generating executable code", e.g. javascript,
> > java code, and more.
> > We will want to clearly define the scope of execveat(AT_CHECK)
>
> The line between data and code is blurry.  For instance, a configuration
> file can impact the execution flow of a program.  So, where to draw the
> line?
>
> It might makes sense to follow the kernel and interpreter semantic: if a
> file can be executed by the kernel (e.g. ELF binary, file containing a
> shebang, or just configured with binfmt_misc), then this should be
> considered as executable code.  This applies to Bash, Python,
> Javascript, NodeJS, PE, PHP...  However, we can also make a picture
> executable with binfmt_misc.  So, again, where to draw the line?
>
> I'd recommend to think about interaction with the outside, through
> function calls, IPCs, syscalls...  For instance, "running" an image
> should not lead to reading or writing to arbitrary files, or accessing
> the network, but in practice it is legitimate for some file formats...
> PostScript is a programming language, but mostly used to draw pictures.
> So, again, where to draw the line?
>
The javascript is run by browser and java code by java runtime, do
they meet the criteria? they do not interact with the kernel directly,
however they might have the same "executable" characteristics and the
app might not want them to be put into non-exec mount.

If the answer is yes, they can also use execveat(AT_CHECK),  the next
question is: does it make sense for javacript/java code to go through
execveat() code path, allocate bprm, etc ? (I don't have answer, maybe
it is)

> We should follow the principle of least astonishment.  What most users
> would expect?  This should follow the *common usage* of executable
> files.  At the end, the script interpreters will be patched by security
> folks for security reasons.  I think the right question to ask should
> be: could this file format be (ab)used to leak or modify arbitrary
> files, or to perform arbitrary syscalls?  If the answer is yes, then it
> should be checked for executability.  Of course, this excludes bugs
> exploited in the file format parser.
>
> I'll extend the next patch series with this rationale.
>

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Jeff Xu @ 2024-07-19 15:27 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <20240719.sah7oeY9pha4@digikod.net>

On Fri, Jul 19, 2024 at 8:04 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> On Fri, Jul 19, 2024 at 07:16:55AM -0700, Jeff Xu wrote:
> > On Fri, Jul 19, 2024 at 1:45 AM Mickaël Salaün <mic@digikod.net> wrote:
> > >
> > > On Thu, Jul 18, 2024 at 06:29:54PM -0700, Jeff Xu wrote:
> > > > On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > > >
> > > > > On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > > > > > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > > >
> > > > > > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > > > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > > > > >
> > > > > > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > > > > > executable code, scripts could also use this check [1].
> > > > > > > > >
> > > > > > > > > This is different than faccessat(2) which only checks file access
> > > > > > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > > > > > real execution, user space gets the same error codes.
> > > > > > > > >
> > > > > > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > > > > > exec, shared object, script and config file (such as seccomp config),
> > >
> > > > > > > > I think binfmt_elf.c in the kernel needs to check the ld.so to make
> > > > > > > > sure it passes AT_CHECK, before loading it into memory.
> > > > > > >
> > > > > > > All ELF dependencies are opened and checked with open_exec(), which
> > > > > > > perform the main executability checks (with the __FMODE_EXEC flag).
> > > > > > > Did I miss something?
> > > > > > >
> > > > > > I mean the ld-linux-x86-64.so.2 which is loaded by binfmt in the kernel.
> > > > > > The app can choose its own dynamic linker path during build, (maybe
> > > > > > even statically link one ?)  This is another reason that relying on a
> > > > > > userspace only is not enough.
> > > > >
> > > > > The kernel calls open_exec() on all dependencies, including
> > > > > ld-linux-x86-64.so.2, so these files are checked for executability too.
> > > > >
> > > > This might not be entirely true. iiuc, kernel  calls open_exec for
> > > > open_exec for interpreter, but not all its dependency (e.g. libc.so.6)
> > >
> > > Correct, the dynamic linker is in charge of that, which is why it must
> > > be enlighten with execveat+AT_CHECK and securebits checks.
> > >
> > > > load_elf_binary() {
> > > >    interpreter = open_exec(elf_interpreter);
> > > > }
> > > >
> > > > libc.so.6 is opened and mapped by dynamic linker.
> > > > so the call sequence is:
> > > >  execve(a.out)
> > > >   - open exec(a.out)
> > > >   - security_bprm_creds(a.out)
> > > >   - open the exec(ld.so)
> > > >   - call open_exec() for interruptor (ld.so)
> > > >   - call execveat(AT_CHECK, ld.so) <-- do we want ld.so going through
> > > > the same check and code path as libc.so below ?
> > >
> > > open_exec() checks are enough.  LSMs can use this information (open +
> > > __FMODE_EXEC) if needed.  execveat+AT_CHECK is only a user space
> > > request.
> > >
> > Then the ld.so doesn't go through the same security_bprm_creds() check
> > as other .so.
>
> Indeed, but...
>
My point is: we will want all the .so going through the same code
path, so  security_ functions are called consistently across all the
objects, And in the future, if we want to develop additional LSM
functionality based on AT_CHECK, it will be applied to all objects.

Another thing to consider is:  we are asking userspace to make
additional syscall before  loading the file into memory/get executed,
there is a possibility for future expansion of the mechanism, without
asking user space to add another syscall again.

I m still not convinced yet that execveat(AT_CHECK) fits more than
faccessat(AT_CHECK)


> >
> > As my previous email, the ChromeOS LSM restricts executable mfd
> > through security_bprm_creds(), the end result is that ld.so can still
> > be executable memfd, but not other .so.
>
> The chromeOS LSM can check that with the security_file_open() hook and
> the __FMODE_EXEC flag, see Landlock's implementation.  I think this
> should be the only hook implementation that chromeOS LSM needs to add.
>
> >
> > One way to address this is to refactor the necessary code from
> > execveat() code patch, and make it available to call from both kernel
> > and execveat() code paths., but if we do that, we might as well use
> > faccessat2(AT_CHECK)
>
> That's why I think it makes sense to rely on the existing __FMODE_EXEC
> information.
>
> >
> >
> > > >   - transfer the control to ld.so)
> > > >   - ld.so open (libc.so)
> > > >   - ld.so call execveat(AT_CHECK,libc.so) <-- proposed by this patch,
> > > > require dynamic linker change.
> > > >   - ld.so mmap(libc.so,rx)
> > >
> > > Explaining these steps is useful. I'll include that in the next patch
> > > series.

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-07-19 15:31 UTC (permalink / raw)
  To: Jeff Xu
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <CALmYWFupWw2_BKu1FF=ooXFpA=GtJr1ehZSK3p+1+1WH34eX=w@mail.gmail.com>

On Fri, Jul 19, 2024 at 08:12:37AM -0700, Jeff Xu wrote:
> On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@digikod.net> wrote:
> >
> > On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > >
> > > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > >
> > > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > > executable code, scripts could also use this check [1].
> > > > > >
> > > > > > This is different than faccessat(2) which only checks file access
> > > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > > real execution, user space gets the same error codes.
> > > > > >
> > > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > > exec, shared object, script and config file (such as seccomp config),
> > > >
> > > > "config file" that contains executable code.
> > > >
> > > Is seccomp config  considered as "contains executable code", seccomp
> > > config is translated into bpf, so maybe yes ? but bpf is running in
> > > the kernel.
> >
> > Because seccomp filters alter syscalls, they are similar to code
> > injection.
> >
> > >
> > > > > I'm still thinking  execveat(AT_CHECK) vs faccessat(AT_CHECK) in
> > > > > different use cases:
> > > > >
> > > > > execveat clearly has less code change, but that also means: we can't
> > > > > add logic specific to exec (i.e. logic that can't be applied to
> > > > > config) for this part (from do_execveat_common to
> > > > > security_bprm_creds_for_exec) in future.  This would require some
> > > > > agreement/sign-off, I'm not sure from whom.
> > > >
> > > > I'm not sure to follow. We could still add new flags, but for now I
> > > > don't see use cases.  This patch series is not meant to handle all
> > > > possible "trust checks", only executable code, which makes sense for the
> > > > kernel.
> > > >
> > > I guess the "configfile" discussion is where I get confused, at one
> > > point, I think this would become a generic "trust checks" api for
> > > everything related to "generating executable code", e.g. javascript,
> > > java code, and more.
> > > We will want to clearly define the scope of execveat(AT_CHECK)
> >
> > The line between data and code is blurry.  For instance, a configuration
> > file can impact the execution flow of a program.  So, where to draw the
> > line?
> >
> > It might makes sense to follow the kernel and interpreter semantic: if a
> > file can be executed by the kernel (e.g. ELF binary, file containing a
> > shebang, or just configured with binfmt_misc), then this should be
> > considered as executable code.  This applies to Bash, Python,
> > Javascript, NodeJS, PE, PHP...  However, we can also make a picture
> > executable with binfmt_misc.  So, again, where to draw the line?
> >
> > I'd recommend to think about interaction with the outside, through
> > function calls, IPCs, syscalls...  For instance, "running" an image
> > should not lead to reading or writing to arbitrary files, or accessing
> > the network, but in practice it is legitimate for some file formats...
> > PostScript is a programming language, but mostly used to draw pictures.
> > So, again, where to draw the line?
> >
> The javascript is run by browser and java code by java runtime, do
> they meet the criteria? they do not interact with the kernel directly,
> however they might have the same "executable" characteristics and the
> app might not want them to be put into non-exec mount.
> 
> If the answer is yes, they can also use execveat(AT_CHECK),  the next
> question is: does it make sense for javacript/java code to go through
> execveat() code path, allocate bprm, etc ? (I don't have answer, maybe
> it is)

Java and NodeJS can do arbitrary syscalls (through their runtime) and
they can access arbitrary files, so according to my below comment, yes
they should be managed as potentially dangerous executable code.

The question should be: is this code trusted? Most of the time it is
not, hence the security model of web browser and their heavy use of
sandboxing.  So no, I don't think it would make sense to check this kind
of code more than what the browser already do.

I'll talk about this use case in the next patch series.

> 
> > We should follow the principle of least astonishment.  What most users
> > would expect?  This should follow the *common usage* of executable
> > files.  At the end, the script interpreters will be patched by security
> > folks for security reasons.  I think the right question to ask should
> > be: could this file format be (ab)used to leak or modify arbitrary
> > files, or to perform arbitrary syscalls?  If the answer is yes, then it
> > should be checked for executability.  Of course, this excludes bugs
> > exploited in the file format parser.
> >
> > I'll extend the next patch series with this rationale.
> >
> 

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Jeff Xu @ 2024-07-19 17:36 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <20240719.Ooxeithah8Sh@digikod.net>

On Fri, Jul 19, 2024 at 8:31 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> On Fri, Jul 19, 2024 at 08:12:37AM -0700, Jeff Xu wrote:
> > On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@digikod.net> wrote:
> > >
> > > On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > > > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > > >
> > > > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > > >
> > > > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > > > executable code, scripts could also use this check [1].
> > > > > > >
> > > > > > > This is different than faccessat(2) which only checks file access
> > > > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > > > real execution, user space gets the same error codes.
> > > > > > >
> > > > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > > > exec, shared object, script and config file (such as seccomp config),
> > > > >
> > > > > "config file" that contains executable code.
> > > > >
> > > > Is seccomp config  considered as "contains executable code", seccomp
> > > > config is translated into bpf, so maybe yes ? but bpf is running in
> > > > the kernel.
> > >
> > > Because seccomp filters alter syscalls, they are similar to code
> > > injection.
> > >
> > > >
> > > > > > I'm still thinking  execveat(AT_CHECK) vs faccessat(AT_CHECK) in
> > > > > > different use cases:
> > > > > >
> > > > > > execveat clearly has less code change, but that also means: we can't
> > > > > > add logic specific to exec (i.e. logic that can't be applied to
> > > > > > config) for this part (from do_execveat_common to
> > > > > > security_bprm_creds_for_exec) in future.  This would require some
> > > > > > agreement/sign-off, I'm not sure from whom.
> > > > >
> > > > > I'm not sure to follow. We could still add new flags, but for now I
> > > > > don't see use cases.  This patch series is not meant to handle all
> > > > > possible "trust checks", only executable code, which makes sense for the
> > > > > kernel.
> > > > >
> > > > I guess the "configfile" discussion is where I get confused, at one
> > > > point, I think this would become a generic "trust checks" api for
> > > > everything related to "generating executable code", e.g. javascript,
> > > > java code, and more.
> > > > We will want to clearly define the scope of execveat(AT_CHECK)
> > >
> > > The line between data and code is blurry.  For instance, a configuration
> > > file can impact the execution flow of a program.  So, where to draw the
> > > line?
> > >
> > > It might makes sense to follow the kernel and interpreter semantic: if a
> > > file can be executed by the kernel (e.g. ELF binary, file containing a
> > > shebang, or just configured with binfmt_misc), then this should be
> > > considered as executable code.  This applies to Bash, Python,
> > > Javascript, NodeJS, PE, PHP...  However, we can also make a picture
> > > executable with binfmt_misc.  So, again, where to draw the line?
> > >
> > > I'd recommend to think about interaction with the outside, through
> > > function calls, IPCs, syscalls...  For instance, "running" an image
> > > should not lead to reading or writing to arbitrary files, or accessing
> > > the network, but in practice it is legitimate for some file formats...
> > > PostScript is a programming language, but mostly used to draw pictures.
> > > So, again, where to draw the line?
> > >
> > The javascript is run by browser and java code by java runtime, do
> > they meet the criteria? they do not interact with the kernel directly,
> > however they might have the same "executable" characteristics and the
> > app might not want them to be put into non-exec mount.
> >
> > If the answer is yes, they can also use execveat(AT_CHECK),  the next
> > question is: does it make sense for javacript/java code to go through
> > execveat() code path, allocate bprm, etc ? (I don't have answer, maybe
> > it is)
>
> Java and NodeJS can do arbitrary syscalls (through their runtime) and
> they can access arbitrary files, so according to my below comment, yes
> they should be managed as potentially dangerous executable code.
>
> The question should be: is this code trusted? Most of the time it is
> not, hence the security model of web browser and their heavy use of
> sandboxing.  So no, I don't think it would make sense to check this kind
> of code more than what the browser already do.
>

If I understand you correctly, Java/NodeJS won't use
execveat(AT_CHECK), we will leave that work to the web browser/java
runtime's sandboxer.
This is good because the scope is more narrow/clear.

Thanks
-Jeff

> I'll talk about this use case in the next patch series.
>
> >
> > > We should follow the principle of least astonishment.  What most users
> > > would expect?  This should follow the *common usage* of executable
> > > files.  At the end, the script interpreters will be patched by security
> > > folks for security reasons.  I think the right question to ask should
> > > be: could this file format be (ab)used to leak or modify arbitrary
> > > files, or to perform arbitrary syscalls?  If the answer is yes, then it
> > > should be checked for executability.  Of course, this excludes bugs
> > > exploited in the file format parser.
> > >
> > > I'll extend the next patch series with this rationale.
> > >
> >

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Andy Lutomirski @ 2024-07-20  1:59 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Steve Dower, Jeff Xu, Al Viro, Christian Brauner, Kees Cook,
	Linus Torvalds, Paul Moore, Theodore Ts'o, Alejandro Colomar,
	Aleksa Sarai, Andrew Morton, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Christian Heimes, Dmitry Vyukov, Eric Biggers,
	Eric Chiang, Fan Wu, Florian Weimer, Geert Uytterhoeven,
	James Morris, Jan Kara, Jann Horn, Jonathan Corbet,
	Jordan R Abrahams, Lakshmi Ramasubramanian, Luca Boccassi,
	Luis Chamberlain, Madhavan T . Venkataraman, Matt Bobrowski,
	Matthew Garrett, Matthew Wilcox, Miklos Szeredi, Mimi Zohar,
	Nicolas Bouchinet, Scott Shell, Shuah Khan, Stephen Rothwell,
	Steve Grubb, Thibaut Sautereau, Vincent Strubel, Xiaoming Ni,
	Yin Fengwei, kernel-hardening, linux-api, linux-fsdevel,
	linux-integrity, linux-kernel, linux-security-module,
	Elliott Hughes
In-Reply-To: <20240718.Niexoo0ahch0@digikod.net>

> On Jul 18, 2024, at 8:22 PM, Mickaël Salaün <mic@digikod.net> wrote:
>
> On Thu, Jul 18, 2024 at 09:02:56AM +0800, Andy Lutomirski wrote:
>>>> On Jul 17, 2024, at 6:01 PM, Mickaël Salaün <mic@digikod.net> wrote:
>>>
>>> On Wed, Jul 17, 2024 at 09:26:22AM +0100, Steve Dower wrote:
>>>>> On 17/07/2024 07:33, Jeff Xu wrote:
>>>>> Consider those cases: I think:
>>>>> a> relying purely on userspace for enforcement does't seem to be
>>>>> effective,  e.g. it is trivial  to call open(), then mmap() it into
>>>>> executable memory.
>>>>
>>>> If there's a way to do this without running executable code that had to pass
>>>> a previous execveat() check, then yeah, it's not effective (e.g. a Python
>>>> interpreter that *doesn't* enforce execveat() is a trivial way to do it).
>>>>
>>>> Once arbitrary code is running, all bets are off. So long as all arbitrary
>>>> code is being checked itself, it's allowed to do things that would bypass
>>>> later checks (and it's up to whoever audited it in the first place to
>>>> prevent this by not giving it the special mark that allows it to pass the
>>>> check).
>>>
>>> Exactly.  As explained in the patches, one crucial prerequisite is that
>>> the executable code is trusted, and the system must provide integrity
>>> guarantees.  We cannot do anything without that.  This patches series is
>>> a building block to fix a blind spot on Linux systems to be able to
>>> fully control executability.
>>
>> Circling back to my previous comment (did that ever get noticed?), I
>
> Yes, I replied to your comments.  Did I miss something?

I missed that email in the pile, sorry. I’ll reply separately.

>
>> don’t think this is quite right:
>>
>> https://lore.kernel.org/all/CALCETrWYu=PYJSgyJ-vaa+3BGAry8Jo8xErZLiGR3U5h6+U0tA@mail.gmail.com/
>>
>> On a basic system configuration, a given path either may or may not be
>> executed. And maybe that path has some integrity check (dm-verity,
>> etc).  So the kernel should tell the interpreter/loader whether the
>> target may be executed. All fine.
>>
>> But I think the more complex cases are more interesting, and the
>> “execute a program” process IS NOT BINARY.  An attempt to execute can
>> be rejected outright, or it can be allowed *with a change to creds or
>> security context*.  It would be entirely reasonable to have a policy
>> that allows execution of non-integrity-checked files but in a very
>> locked down context only.
>
> I guess you mean to transition to a sandbox when executing an untrusted
> file.  This is a good idea.  I talked about role transition in the
> patch's description:
>
> With the information that a script interpreter is about to interpret a
> script, an LSM security policy can adjust caller's access rights or log
> execution request as for native script execution (e.g. role transition).
> This is possible thanks to the call to security_bprm_creds_for_exec().
> This patch series brings the minimal building blocks to have a
> consistent execution environment.  Role transitions for script execution
> are left to LSMs.  For instance, we could extend Landlock to
> automatically sandbox untrusted scripts.

I’m not really convinced.  There’s more to building an API that
enables LSM hooks than merely sticking the hook somewhere in kernel
code. It needs to be a defined API. If you call an operation “check”,
then people will expect it to check, not to change the caller’s
credentials.  And people will mess it up in both directions (e.g.
callers will call it and then open try to load some library that they
should have loaded first, or callers will call it and forget to close
fds first.

And there should probably be some interaction with dumpable as well.
If I “check” a file for executability, that should not suddenly allow
someone to ptrace me?

And callers need to know to exit on failure, not carry on.


More concretely, a runtime that fully opts in to this may well "check"
multiple things.  For example, if I do:

$ ld.so ~/.local/bin/some_program   (i.e. I literally execve ld.so)

then ld.so will load several things:

~/.local/bin/some_program
libc.so
other random DSOs, some of which may well be in my home directory

And for all ld.so knows, some_program is actually an interpreter and
will "check" something else.  And the LSMs have absolutely no clue
what's what.  So I think for this to work right, the APIs need to be a
lot more expressive and explicit:

check_library(fd to libc.so);  <-- does not transition or otherwise drop privs
check_transition_main_program(fd to ~/.local/bin/some_program);  <--
may drop privs

and if some_program is really an interpreter, then it will do:

check_library(fd to some thing imported by the script);
check_transition_main_program(fd to the actual script);

And maybe that takes a parameter that gets run eval-style:

check_unsafe_user_script("actual contents of snippet");

The actual spelling of all this doesn't matter so much.  But the user
code and the kernel code need to be on the same page as to what the
user program is doing and what it's asking the kernel program to do.

--Andy

^ permalink raw reply

* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Andy Lutomirski @ 2024-07-20  2:06 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module
In-Reply-To: <20240704190137.696169-3-mic@digikod.net>

On Fri, Jul 5, 2024 at 3:02 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> These new SECBIT_SHOULD_EXEC_CHECK, SECBIT_SHOULD_EXEC_RESTRICT, and
> their *_LOCKED counterparts are designed to be set by processes setting
> up an execution environment, such as a user session, a container, or a
> security sandbox.  Like seccomp filters or Landlock domains, the
> securebits are inherited across proceses.
>
> When SECBIT_SHOULD_EXEC_CHECK is set, programs interpreting code should
> check executable resources with execveat(2) + AT_CHECK (see previous
> patch).
>
> When SECBIT_SHOULD_EXEC_RESTRICT is set, a process should only allow
> execution of approved resources, if any (see SECBIT_SHOULD_EXEC_CHECK).

I read this twice, slept on it, read them again, and I *still* can't
understand it.  See below...

> The only restriction enforced by the kernel is the right to ptrace
> another process.  Processes are denied to ptrace less restricted ones,
> unless the tracer has CAP_SYS_PTRACE.  This is mainly a safeguard to
> avoid trivial privilege escalations e.g., by a debugging process being
> abused with a confused deputy attack.

What's the actual issue?  And why can't I, as root, do, in a carefully
checked, CHECK'd and RESTRICT'd environment, # gdb -p <pid>?  Adding
weird restrictions to ptrace can substantially *weaken* security
because it forces people to do utterly daft things to work around the
restrictions.

...

> +/*
> + * When SECBIT_SHOULD_EXEC_CHECK is set, a process should check all executable
> + * files with execveat(2) + AT_CHECK.  However, such check should only be
> + * performed if all to-be-executed code only comes from regular files.  For
> + * instance, if a script interpreter is called with both a script snipped as

s/snipped/snippet/

> + * argument and a regular file, the interpreter should not check any file.
> + * Doing otherwise would mislead the kernel to think that only the script file
> + * is being executed, which could for instance lead to unexpected permission
> + * change and break current use cases.

This is IMO not nearly clear enough to result in multiple user
implementations and a kernel implementation and multiple LSM
implementations and LSM policy authors actually agreeing as to what
this means.

I also think it's wrong to give user code instructions about what
kernel checks it should do.  Have the user code call the kernel and
have the kernel implement the policy.

> +/*
> + * When SECBIT_SHOULD_EXEC_RESTRICT is set, a process should only allow
> + * execution of approved files, if any (see SECBIT_SHOULD_EXEC_CHECK).  For
> + * instance, script interpreters called with a script snippet as argument
> + * should always deny such execution if SECBIT_SHOULD_EXEC_RESTRICT is set.
> + * However, if a script interpreter is called with both
> + * SECBIT_SHOULD_EXEC_CHECK and SECBIT_SHOULD_EXEC_RESTRICT, they should
> + * interpret the provided script files if no unchecked code is also provided
> + * (e.g. directly as argument).

I think you're trying to say that this is like (the inverse of)
Content-Security-Policy: unsafe-inline.  In other words, you're saying
that, if RESTRICT is set, then programs should not execute code-like
text that didn't come from a file.  Is that right?

I feel like it would be worth looking at the state of the art of
Content-Security-Policy and all the lessons people have learned from
it.  Whatever the result is should be at least as comprehensible and
at least as carefully engineered as Content-Security-Policy.

--Andy

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Jarkko Sakkinen @ 2024-07-20 11:43 UTC (permalink / raw)
  To: Andy Lutomirski, Mickaël Salaün
  Cc: Steve Dower, Jeff Xu, Al Viro, Christian Brauner, Kees Cook,
	Linus Torvalds, Paul Moore, Theodore Ts'o, Alejandro Colomar,
	Aleksa Sarai, Andrew Morton, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Christian Heimes, Dmitry Vyukov, Eric Biggers,
	Eric Chiang, Fan Wu, Florian Weimer, Geert Uytterhoeven,
	James Morris, Jan Kara, Jann Horn, Jonathan Corbet,
	Jordan R Abrahams, Lakshmi Ramasubramanian, Luca Boccassi,
	Luis Chamberlain, Madhavan T . Venkataraman, Matt Bobrowski,
	Matthew Garrett, Matthew Wilcox, Miklos Szeredi, Mimi Zohar,
	Nicolas Bouchinet, Scott Shell, Shuah Khan, Stephen Rothwell,
	Steve Grubb, Thibaut Sautereau, Vincent Strubel, Xiaoming Ni,
	Yin Fengwei, kernel-hardening, linux-api, linux-fsdevel,
	linux-integrity, linux-kernel, linux-security-module,
	Elliott Hughes
In-Reply-To: <CALCETrVVq4DJZ2q9V9TMuvZ1nb+-Qf4Eu8LVBgUy3XiTa=jFCQ@mail.gmail.com>

On Sat Jul 20, 2024 at 4:59 AM EEST, Andy Lutomirski wrote:
> > On Jul 18, 2024, at 8:22 PM, Mickaël Salaün <mic@digikod.net> wrote:
> >
> > On Thu, Jul 18, 2024 at 09:02:56AM +0800, Andy Lutomirski wrote:
> >>>> On Jul 17, 2024, at 6:01 PM, Mickaël Salaün <mic@digikod.net> wrote:
> >>>
> >>> On Wed, Jul 17, 2024 at 09:26:22AM +0100, Steve Dower wrote:
> >>>>> On 17/07/2024 07:33, Jeff Xu wrote:
> >>>>> Consider those cases: I think:
> >>>>> a> relying purely on userspace for enforcement does't seem to be
> >>>>> effective,  e.g. it is trivial  to call open(), then mmap() it into
> >>>>> executable memory.
> >>>>
> >>>> If there's a way to do this without running executable code that had to pass
> >>>> a previous execveat() check, then yeah, it's not effective (e.g. a Python
> >>>> interpreter that *doesn't* enforce execveat() is a trivial way to do it).
> >>>>
> >>>> Once arbitrary code is running, all bets are off. So long as all arbitrary
> >>>> code is being checked itself, it's allowed to do things that would bypass
> >>>> later checks (and it's up to whoever audited it in the first place to
> >>>> prevent this by not giving it the special mark that allows it to pass the
> >>>> check).
> >>>
> >>> Exactly.  As explained in the patches, one crucial prerequisite is that
> >>> the executable code is trusted, and the system must provide integrity
> >>> guarantees.  We cannot do anything without that.  This patches series is
> >>> a building block to fix a blind spot on Linux systems to be able to
> >>> fully control executability.
> >>
> >> Circling back to my previous comment (did that ever get noticed?), I
> >
> > Yes, I replied to your comments.  Did I miss something?
>
> I missed that email in the pile, sorry. I’ll reply separately.
>
> >
> >> don’t think this is quite right:
> >>
> >> https://lore.kernel.org/all/CALCETrWYu=PYJSgyJ-vaa+3BGAry8Jo8xErZLiGR3U5h6+U0tA@mail.gmail.com/
> >>
> >> On a basic system configuration, a given path either may or may not be
> >> executed. And maybe that path has some integrity check (dm-verity,
> >> etc).  So the kernel should tell the interpreter/loader whether the
> >> target may be executed. All fine.
> >>
> >> But I think the more complex cases are more interesting, and the
> >> “execute a program” process IS NOT BINARY.  An attempt to execute can
> >> be rejected outright, or it can be allowed *with a change to creds or
> >> security context*.  It would be entirely reasonable to have a policy
> >> that allows execution of non-integrity-checked files but in a very
> >> locked down context only.
> >
> > I guess you mean to transition to a sandbox when executing an untrusted
> > file.  This is a good idea.  I talked about role transition in the
> > patch's description:
> >
> > With the information that a script interpreter is about to interpret a
> > script, an LSM security policy can adjust caller's access rights or log
> > execution request as for native script execution (e.g. role transition).
> > This is possible thanks to the call to security_bprm_creds_for_exec().
>
> …
>
> > This patch series brings the minimal building blocks to have a
> > consistent execution environment.  Role transitions for script execution
> > are left to LSMs.  For instance, we could extend Landlock to
> > automatically sandbox untrusted scripts.
>
> I’m not really convinced.  There’s more to building an API that
> enables LSM hooks than merely sticking the hook somewhere in kernel
> code. It needs to be a defined API. If you call an operation “check”,
> then people will expect it to check, not to change the caller’s
> credentials.  And people will mess it up in both directions (e.g.
> callers will call it and then open try to load some library that they
> should have loaded first, or callers will call it and forget to close
> fds first.
>
> And there should probably be some interaction with dumpable as well.
> If I “check” a file for executability, that should not suddenly allow
> someone to ptrace me?
>
> And callers need to know to exit on failure, not carry on.
>
>
> More concretely, a runtime that fully opts in to this may well "check"
> multiple things.  For example, if I do:
>
> $ ld.so ~/.local/bin/some_program   (i.e. I literally execve ld.so)
>
> then ld.so will load several things:
>
> ~/.local/bin/some_program
> libc.so
> other random DSOs, some of which may well be in my home directory

What would really help to comprehend this patch set would be a set of
test scripts, preferably something that you can run easily with
BuildRoot or similar.

Scripts would demonstrate the use cases for the patch set. Then it
would be easier to develop scripts that would underline the corner
cases. I would keep all this out of kselftest shenanigans for now.

I feel that the patch set is hovering in abstractions with examples
that you cannot execute.

I added the patches to standard test CI hack:

https://codeberg.org/jarkko/linux-tpmdd-test

But after I booted up a kernel I had no idea what to do with it. And
all this lenghty discussion makes it even more confusing.

Please find some connection to the real world before sending any new
version of this (e.g. via test scripts). I think this should not be
pulled before almost anyone doing kernel dev can comprehend the "gist"
at least in some reasonable level.

BR, Jarkko

^ permalink raw reply

* Re: [PATCH v2] vfs: add RWF_NOAPPEND flag for pwritev2
From: John Garry @ 2024-07-22  9:28 UTC (permalink / raw)
  To: Rich Felker, Jann Horn
  Cc: linux-fsdevel, kernel list, Linux API, Alexander Viro,
	Pavel Begunkov
In-Reply-To: <20200831153207.GO3265@brightrain.aerifal.cx>

On 31/08/2020 16:32, Rich Felker wrote:
> The pwrite function, originally defined by POSIX (thus the "p"), is
> defined to ignore O_APPEND and write at the offset passed as its
> argument. However, historically Linux honored O_APPEND if set and
> ignored the offset. This cannot be changed due to stability policy,
> but is documented in the man page as a bug.
> 
> Now that there's a pwritev2 syscall providing a superset of the pwrite
> functionality that has a flags argument, the conforming behavior can
> be offered to userspace via a new flag. Since pwritev2 checks flag
> validity (in kiocb_set_rw_flags) and reports unknown ones with
> EOPNOTSUPP, callers will not get wrong behavior on old kernels that
> don't support the new flag; the error is reported and the caller can
> decide how to handle it.
> 
> Signed-off-by: Rich Felker <dalias@libc.org>

What about updating the linux man pages for this flag?

If someone gives me a description of RWF_NOAPPEND for 
rehttps://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man/man2/readv.2, 
then I can do it.

^ permalink raw reply

* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Mickaël Salaün @ 2024-07-23 13:15 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Arnd Bergmann, Casey Schaufler, Christian Heimes, Dmitry Vyukov,
	Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module
In-Reply-To: <CALCETrWpk5Es9GPoAdDD=m_vgSePm=cA16zCor_aJV0EPXBw1A@mail.gmail.com>

On Sat, Jul 20, 2024 at 10:06:28AM +0800, Andy Lutomirski wrote:
> On Fri, Jul 5, 2024 at 3:02 AM Mickaël Salaün <mic@digikod.net> wrote:
> >
> > These new SECBIT_SHOULD_EXEC_CHECK, SECBIT_SHOULD_EXEC_RESTRICT, and
> > their *_LOCKED counterparts are designed to be set by processes setting
> > up an execution environment, such as a user session, a container, or a
> > security sandbox.  Like seccomp filters or Landlock domains, the
> > securebits are inherited across proceses.
> >
> > When SECBIT_SHOULD_EXEC_CHECK is set, programs interpreting code should
> > check executable resources with execveat(2) + AT_CHECK (see previous
> > patch).
> >
> > When SECBIT_SHOULD_EXEC_RESTRICT is set, a process should only allow
> > execution of approved resources, if any (see SECBIT_SHOULD_EXEC_CHECK).
> 
> I read this twice, slept on it, read them again, and I *still* can't
> understand it.  See below...

There is a new proposal:
https://lore.kernel.org/all/20240710.eiKohpa4Phai@digikod.net/
The new securebits will be SECBIT_EXEC_RESTRICT_FILE and
SECBIT_EXEC_DENY_INTERACTIVE.  I'll send a new patch series with that.

> 
> > The only restriction enforced by the kernel is the right to ptrace
> > another process.  Processes are denied to ptrace less restricted ones,
> > unless the tracer has CAP_SYS_PTRACE.  This is mainly a safeguard to
> > avoid trivial privilege escalations e.g., by a debugging process being
> > abused with a confused deputy attack.
> 
> What's the actual issue?  And why can't I, as root, do, in a carefully
> checked, CHECK'd and RESTRICT'd environment, # gdb -p <pid>?  Adding
> weird restrictions to ptrace can substantially *weaken* security
> because it forces people to do utterly daft things to work around the
> restrictions.

Restricting ptrace was a cautious approach, but I get you point and I
agree.  I'll remove the ptrace restrictions in the next patch series.

> 
> ...
> 
> > +/*
> > + * When SECBIT_SHOULD_EXEC_CHECK is set, a process should check all executable
> > + * files with execveat(2) + AT_CHECK.  However, such check should only be
> > + * performed if all to-be-executed code only comes from regular files.  For
> > + * instance, if a script interpreter is called with both a script snipped as
> 
> s/snipped/snippet/
> 
> > + * argument and a regular file, the interpreter should not check any file.
> > + * Doing otherwise would mislead the kernel to think that only the script file
> > + * is being executed, which could for instance lead to unexpected permission
> > + * change and break current use cases.
> 
> This is IMO not nearly clear enough to result in multiple user
> implementations and a kernel implementation and multiple LSM
> implementations and LSM policy authors actually agreeing as to what
> this means.

Right, no kernel parts (e.g. LSMs) should try to infer anything other
than an executability check.  We should handle things such as role
transitions with something else (e.g. a complementary dedicated flag),
and that should be decorrelated from this patch series.

> 
> I also think it's wrong to give user code instructions about what
> kernel checks it should do.  Have the user code call the kernel and
> have the kernel implement the policy.

Call the kernel for what?  Script interpreter is a user space thing, and
restrictions enforced on interpreters need to be a user space thing.
The kernel cannot restrict user space according to a semantic only
defined by user space, such as Python interpretation, CLI arguments,
content of environment variables...  If a process wants to interpret
some data and turn than into code, there is no way for the kernel to
know about that.

> 
> > +/*
> > + * When SECBIT_SHOULD_EXEC_RESTRICT is set, a process should only allow
> > + * execution of approved files, if any (see SECBIT_SHOULD_EXEC_CHECK).  For
> > + * instance, script interpreters called with a script snippet as argument
> > + * should always deny such execution if SECBIT_SHOULD_EXEC_RESTRICT is set.
> > + * However, if a script interpreter is called with both
> > + * SECBIT_SHOULD_EXEC_CHECK and SECBIT_SHOULD_EXEC_RESTRICT, they should
> > + * interpret the provided script files if no unchecked code is also provided
> > + * (e.g. directly as argument).
> 
> I think you're trying to say that this is like (the inverse of)
> Content-Security-Policy: unsafe-inline.  In other words, you're saying
> that, if RESTRICT is set, then programs should not execute code-like
> text that didn't come from a file.  Is that right?

That is the definition of the new SECBIT_EXEC_DENY_INTERACTIVE, which
should be clearer.

> 
> I feel like it would be worth looking at the state of the art of
> Content-Security-Policy and all the lessons people have learned from
> it.  Whatever the result is should be at least as comprehensible and
> at least as carefully engineered as Content-Security-Policy.

That's a good idea, but I guess Content-Security-Policy cannot be
directly applied here.  My understanding is that CSP enables web servers
to request restrictions on code they provide.  In the
AT_CHECK+securebits case, the policy is defined and enforced by the
interpreter, not necessarily the script provider. One big difference is
that web servers (should) know the scripts they provide, and can then
request the browser to ensure that they do what they should do, while
the script interpreter trusts the kernel to check security properties of
a script.  In other words, something like CSP could be implemented with
AT_CHECK+securebits and a LSM policy (e.g. according to file's xattr).

> 
> --Andy

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-07-23 13:15 UTC (permalink / raw)
  To: Jeff Xu
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <CALmYWFsAZjU5sMcXTT23Mtw2Y30ewc94FAjKsnuSv1Ex=7fgLQ@mail.gmail.com>

On Fri, Jul 19, 2024 at 08:27:18AM -0700, Jeff Xu wrote:
> On Fri, Jul 19, 2024 at 8:04 AM Mickaël Salaün <mic@digikod.net> wrote:
> >
> > On Fri, Jul 19, 2024 at 07:16:55AM -0700, Jeff Xu wrote:
> > > On Fri, Jul 19, 2024 at 1:45 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > >
> > > > On Thu, Jul 18, 2024 at 06:29:54PM -0700, Jeff Xu wrote:
> > > > > On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > >
> > > > > > On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > > > > > > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > > > > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > > > > > >
> > > > > > > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > > > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > > > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > > > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > > > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > > > > > > executable code, scripts could also use this check [1].
> > > > > > > > > >
> > > > > > > > > > This is different than faccessat(2) which only checks file access
> > > > > > > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > > > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > > > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > > > > > > real execution, user space gets the same error codes.
> > > > > > > > > >
> > > > > > > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > > > > > > exec, shared object, script and config file (such as seccomp config),
> > > >
> > > > > > > > > I think binfmt_elf.c in the kernel needs to check the ld.so to make
> > > > > > > > > sure it passes AT_CHECK, before loading it into memory.
> > > > > > > >
> > > > > > > > All ELF dependencies are opened and checked with open_exec(), which
> > > > > > > > perform the main executability checks (with the __FMODE_EXEC flag).
> > > > > > > > Did I miss something?
> > > > > > > >
> > > > > > > I mean the ld-linux-x86-64.so.2 which is loaded by binfmt in the kernel.
> > > > > > > The app can choose its own dynamic linker path during build, (maybe
> > > > > > > even statically link one ?)  This is another reason that relying on a
> > > > > > > userspace only is not enough.
> > > > > >
> > > > > > The kernel calls open_exec() on all dependencies, including
> > > > > > ld-linux-x86-64.so.2, so these files are checked for executability too.
> > > > > >
> > > > > This might not be entirely true. iiuc, kernel  calls open_exec for
> > > > > open_exec for interpreter, but not all its dependency (e.g. libc.so.6)
> > > >
> > > > Correct, the dynamic linker is in charge of that, which is why it must
> > > > be enlighten with execveat+AT_CHECK and securebits checks.
> > > >
> > > > > load_elf_binary() {
> > > > >    interpreter = open_exec(elf_interpreter);
> > > > > }
> > > > >
> > > > > libc.so.6 is opened and mapped by dynamic linker.
> > > > > so the call sequence is:
> > > > >  execve(a.out)
> > > > >   - open exec(a.out)
> > > > >   - security_bprm_creds(a.out)
> > > > >   - open the exec(ld.so)
> > > > >   - call open_exec() for interruptor (ld.so)
> > > > >   - call execveat(AT_CHECK, ld.so) <-- do we want ld.so going through
> > > > > the same check and code path as libc.so below ?
> > > >
> > > > open_exec() checks are enough.  LSMs can use this information (open +
> > > > __FMODE_EXEC) if needed.  execveat+AT_CHECK is only a user space
> > > > request.
> > > >
> > > Then the ld.so doesn't go through the same security_bprm_creds() check
> > > as other .so.
> >
> > Indeed, but...
> >
> My point is: we will want all the .so going through the same code
> path, so  security_ functions are called consistently across all the
> objects, And in the future, if we want to develop additional LSM
> functionality based on AT_CHECK, it will be applied to all objects.

I'll extend the doc to encourage LSMs to check for __FMODE_EXEC, which
already is the common security check for all executable dependencies.
As extra information, they can get explicit requests by looking at
execveat+AT_CHECK call.

> 
> Another thing to consider is:  we are asking userspace to make
> additional syscall before  loading the file into memory/get executed,
> there is a possibility for future expansion of the mechanism, without
> asking user space to add another syscall again.

AT_CHECK is defined with a specific semantic.  Other mechanisms (e.g.
LSM policies) could enforce other restrictions following the same
semantic.  We need to keep in mind backward compatibility.

> 
> I m still not convinced yet that execveat(AT_CHECK) fits more than
> faccessat(AT_CHECK)

faccessat2(2) is dedicated to file permission/attribute check.
execveat(2) is dedicated to execution, which is a superset of file
permission for executability, plus other checks (e.g. noexec).

> 
> 
> > >
> > > As my previous email, the ChromeOS LSM restricts executable mfd
> > > through security_bprm_creds(), the end result is that ld.so can still
> > > be executable memfd, but not other .so.
> >
> > The chromeOS LSM can check that with the security_file_open() hook and
> > the __FMODE_EXEC flag, see Landlock's implementation.  I think this
> > should be the only hook implementation that chromeOS LSM needs to add.
> >
> > >
> > > One way to address this is to refactor the necessary code from
> > > execveat() code patch, and make it available to call from both kernel
> > > and execveat() code paths., but if we do that, we might as well use
> > > faccessat2(AT_CHECK)
> >
> > That's why I think it makes sense to rely on the existing __FMODE_EXEC
> > information.
> >
> > >
> > >
> > > > >   - transfer the control to ld.so)
> > > > >   - ld.so open (libc.so)
> > > > >   - ld.so call execveat(AT_CHECK,libc.so) <-- proposed by this patch,
> > > > > require dynamic linker change.
> > > > >   - ld.so mmap(libc.so,rx)
> > > >
> > > > Explaining these steps is useful. I'll include that in the next patch
> > > > series.
> 

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-07-23 13:15 UTC (permalink / raw)
  To: Jeff Xu
  Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
	Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
	Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
	Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
	Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
	linux-api, linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <CALmYWFsR+fWVuPLCoNhisTV5Lviv_dzb=5ajoKs0LaHsei9qTQ@mail.gmail.com>

On Fri, Jul 19, 2024 at 10:36:01AM -0700, Jeff Xu wrote:
> On Fri, Jul 19, 2024 at 8:31 AM Mickaël Salaün <mic@digikod.net> wrote:
> >
> > On Fri, Jul 19, 2024 at 08:12:37AM -0700, Jeff Xu wrote:
> > > On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > >
> > > > On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > > > > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > >
> > > > > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > > > > > >
> > > > > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > > > > executable code, scripts could also use this check [1].
> > > > > > > >
> > > > > > > > This is different than faccessat(2) which only checks file access
> > > > > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > > > > real execution, user space gets the same error codes.
> > > > > > > >
> > > > > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > > > > exec, shared object, script and config file (such as seccomp config),
> > > > > >
> > > > > > "config file" that contains executable code.
> > > > > >
> > > > > Is seccomp config  considered as "contains executable code", seccomp
> > > > > config is translated into bpf, so maybe yes ? but bpf is running in
> > > > > the kernel.
> > > >
> > > > Because seccomp filters alter syscalls, they are similar to code
> > > > injection.
> > > >
> > > > >
> > > > > > > I'm still thinking  execveat(AT_CHECK) vs faccessat(AT_CHECK) in
> > > > > > > different use cases:
> > > > > > >
> > > > > > > execveat clearly has less code change, but that also means: we can't
> > > > > > > add logic specific to exec (i.e. logic that can't be applied to
> > > > > > > config) for this part (from do_execveat_common to
> > > > > > > security_bprm_creds_for_exec) in future.  This would require some
> > > > > > > agreement/sign-off, I'm not sure from whom.
> > > > > >
> > > > > > I'm not sure to follow. We could still add new flags, but for now I
> > > > > > don't see use cases.  This patch series is not meant to handle all
> > > > > > possible "trust checks", only executable code, which makes sense for the
> > > > > > kernel.
> > > > > >
> > > > > I guess the "configfile" discussion is where I get confused, at one
> > > > > point, I think this would become a generic "trust checks" api for
> > > > > everything related to "generating executable code", e.g. javascript,
> > > > > java code, and more.
> > > > > We will want to clearly define the scope of execveat(AT_CHECK)
> > > >
> > > > The line between data and code is blurry.  For instance, a configuration
> > > > file can impact the execution flow of a program.  So, where to draw the
> > > > line?
> > > >
> > > > It might makes sense to follow the kernel and interpreter semantic: if a
> > > > file can be executed by the kernel (e.g. ELF binary, file containing a
> > > > shebang, or just configured with binfmt_misc), then this should be
> > > > considered as executable code.  This applies to Bash, Python,
> > > > Javascript, NodeJS, PE, PHP...  However, we can also make a picture
> > > > executable with binfmt_misc.  So, again, where to draw the line?
> > > >
> > > > I'd recommend to think about interaction with the outside, through
> > > > function calls, IPCs, syscalls...  For instance, "running" an image
> > > > should not lead to reading or writing to arbitrary files, or accessing
> > > > the network, but in practice it is legitimate for some file formats...
> > > > PostScript is a programming language, but mostly used to draw pictures.
> > > > So, again, where to draw the line?
> > > >
> > > The javascript is run by browser and java code by java runtime, do
> > > they meet the criteria? they do not interact with the kernel directly,
> > > however they might have the same "executable" characteristics and the
> > > app might not want them to be put into non-exec mount.
> > >
> > > If the answer is yes, they can also use execveat(AT_CHECK),  the next
> > > question is: does it make sense for javacript/java code to go through
> > > execveat() code path, allocate bprm, etc ? (I don't have answer, maybe
> > > it is)
> >
> > Java and NodeJS can do arbitrary syscalls (through their runtime) and
> > they can access arbitrary files, so according to my below comment, yes
> > they should be managed as potentially dangerous executable code.
> >
> > The question should be: is this code trusted? Most of the time it is
> > not, hence the security model of web browser and their heavy use of
> > sandboxing.  So no, I don't think it would make sense to check this kind
> > of code more than what the browser already do.
> >
> 
> If I understand you correctly, Java/NodeJS won't use
> execveat(AT_CHECK), we will leave that work to the web browser/java
> runtime's sandboxer.
> This is good because the scope is more narrow/clear.

Yes for browser's sandboxes because the code comes from the network
(i.e. not authenticated at the kernel level, and mostly untrusted).

For standalone Java applications (stored in the filesystem), the Java
runtime(s) should be patched as other script interpreters.

> 
> Thanks
> -Jeff
> 
> > I'll talk about this use case in the next patch series.
> >
> > >
> > > > We should follow the principle of least astonishment.  What most users
> > > > would expect?  This should follow the *common usage* of executable
> > > > files.  At the end, the script interpreters will be patched by security
> > > > folks for security reasons.  I think the right question to ask should
> > > > be: could this file format be (ab)used to leak or modify arbitrary
> > > > files, or to perform arbitrary syscalls?  If the answer is yes, then it
> > > > should be checked for executability.  Of course, this excludes bugs
> > > > exploited in the file format parser.
> > > >
> > > > I'll extend the next patch series with this rationale.
> > > >
> > >

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-07-23 13:16 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Steve Dower, Jeff Xu, Al Viro, Christian Brauner, Kees Cook,
	Linus Torvalds, Paul Moore, Theodore Ts'o, Alejandro Colomar,
	Aleksa Sarai, Andrew Morton, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Christian Heimes, Dmitry Vyukov, Eric Biggers,
	Eric Chiang, Fan Wu, Florian Weimer, Geert Uytterhoeven,
	James Morris, Jan Kara, Jann Horn, Jonathan Corbet,
	Jordan R Abrahams, Lakshmi Ramasubramanian, Luca Boccassi,
	Luis Chamberlain, Madhavan T . Venkataraman, Matt Bobrowski,
	Matthew Garrett, Matthew Wilcox, Miklos Szeredi, Mimi Zohar,
	Nicolas Bouchinet, Scott Shell, Shuah Khan, Stephen Rothwell,
	Steve Grubb, Thibaut Sautereau, Vincent Strubel, Xiaoming Ni,
	Yin Fengwei, kernel-hardening, linux-api, linux-fsdevel,
	linux-integrity, linux-kernel, linux-security-module,
	Elliott Hughes
In-Reply-To: <CALCETrVVq4DJZ2q9V9TMuvZ1nb+-Qf4Eu8LVBgUy3XiTa=jFCQ@mail.gmail.com>

On Sat, Jul 20, 2024 at 09:59:33AM +0800, Andy Lutomirski wrote:
> > On Jul 18, 2024, at 8:22 PM, Mickaël Salaün <mic@digikod.net> wrote:
> >
> > On Thu, Jul 18, 2024 at 09:02:56AM +0800, Andy Lutomirski wrote:
> >>>> On Jul 17, 2024, at 6:01 PM, Mickaël Salaün <mic@digikod.net> wrote:
> >>>
> >>> On Wed, Jul 17, 2024 at 09:26:22AM +0100, Steve Dower wrote:
> >>>>> On 17/07/2024 07:33, Jeff Xu wrote:
> >>>>> Consider those cases: I think:
> >>>>> a> relying purely on userspace for enforcement does't seem to be
> >>>>> effective,  e.g. it is trivial  to call open(), then mmap() it into
> >>>>> executable memory.
> >>>>
> >>>> If there's a way to do this without running executable code that had to pass
> >>>> a previous execveat() check, then yeah, it's not effective (e.g. a Python
> >>>> interpreter that *doesn't* enforce execveat() is a trivial way to do it).
> >>>>
> >>>> Once arbitrary code is running, all bets are off. So long as all arbitrary
> >>>> code is being checked itself, it's allowed to do things that would bypass
> >>>> later checks (and it's up to whoever audited it in the first place to
> >>>> prevent this by not giving it the special mark that allows it to pass the
> >>>> check).
> >>>
> >>> Exactly.  As explained in the patches, one crucial prerequisite is that
> >>> the executable code is trusted, and the system must provide integrity
> >>> guarantees.  We cannot do anything without that.  This patches series is
> >>> a building block to fix a blind spot on Linux systems to be able to
> >>> fully control executability.
> >>
> >> Circling back to my previous comment (did that ever get noticed?), I
> >
> > Yes, I replied to your comments.  Did I miss something?
> 
> I missed that email in the pile, sorry. I’ll reply separately.
> 
> >
> >> don’t think this is quite right:
> >>
> >> https://lore.kernel.org/all/CALCETrWYu=PYJSgyJ-vaa+3BGAry8Jo8xErZLiGR3U5h6+U0tA@mail.gmail.com/
> >>
> >> On a basic system configuration, a given path either may or may not be
> >> executed. And maybe that path has some integrity check (dm-verity,
> >> etc).  So the kernel should tell the interpreter/loader whether the
> >> target may be executed. All fine.
> >>
> >> But I think the more complex cases are more interesting, and the
> >> “execute a program” process IS NOT BINARY.  An attempt to execute can
> >> be rejected outright, or it can be allowed *with a change to creds or
> >> security context*.  It would be entirely reasonable to have a policy
> >> that allows execution of non-integrity-checked files but in a very
> >> locked down context only.
> >
> > I guess you mean to transition to a sandbox when executing an untrusted
> > file.  This is a good idea.  I talked about role transition in the
> > patch's description:
> >
> > With the information that a script interpreter is about to interpret a
> > script, an LSM security policy can adjust caller's access rights or log
> > execution request as for native script execution (e.g. role transition).
> > This is possible thanks to the call to security_bprm_creds_for_exec().
> 
> …
> 
> > This patch series brings the minimal building blocks to have a
> > consistent execution environment.  Role transitions for script execution
> > are left to LSMs.  For instance, we could extend Landlock to
> > automatically sandbox untrusted scripts.
> 
> I’m not really convinced.  There’s more to building an API that
> enables LSM hooks than merely sticking the hook somewhere in kernel
> code. It needs to be a defined API. If you call an operation “check”,
> then people will expect it to check, not to change the caller’s
> credentials.  And people will mess it up in both directions (e.g.
> callers will call it and then open try to load some library that they
> should have loaded first, or callers will call it and forget to close
> fds first.
> 
> And there should probably be some interaction with dumpable as well.
> If I “check” a file for executability, that should not suddenly allow
> someone to ptrace me?
> 
> And callers need to know to exit on failure, not carry on.
> 
> 
> More concretely, a runtime that fully opts in to this may well "check"
> multiple things.  For example, if I do:
> 
> $ ld.so ~/.local/bin/some_program   (i.e. I literally execve ld.so)
> 
> then ld.so will load several things:
> 
> ~/.local/bin/some_program
> libc.so
> other random DSOs, some of which may well be in my home directory
> 
> And for all ld.so knows, some_program is actually an interpreter and
> will "check" something else.  And the LSMs have absolutely no clue
> what's what.  So I think for this to work right, the APIs need to be a
> lot more expressive and explicit:
> 
> check_library(fd to libc.so);  <-- does not transition or otherwise drop privs
> check_transition_main_program(fd to ~/.local/bin/some_program);  <--
> may drop privs
> 
> and if some_program is really an interpreter, then it will do:
> 
> check_library(fd to some thing imported by the script);
> check_transition_main_program(fd to the actual script);
> 
> And maybe that takes a parameter that gets run eval-style:
> 
> check_unsafe_user_script("actual contents of snippet");
> 
> The actual spelling of all this doesn't matter so much.  But the user
> code and the kernel code need to be on the same page as to what the
> user program is doing and what it's asking the kernel program to do.

I agree.  I'll remove any references to "role transition".  This kind of
feature should come with something like getpeercon/setexeccon(3).

> 
> --Andy
> 

^ permalink raw reply

* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-07-23 13:16 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Andy Lutomirski, Steve Dower, Jeff Xu, Al Viro, Christian Brauner,
	Kees Cook, Linus Torvalds, Paul Moore, Theodore Ts'o,
	Alejandro Colomar, Aleksa Sarai, Andrew Morton, Andy Lutomirski,
	Arnd Bergmann, Casey Schaufler, Christian Heimes, Dmitry Vyukov,
	Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
	Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
	Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
	Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
	Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
	Stephen Rothwell, Steve Grubb, Thibaut Sautereau, Vincent Strubel,
	Xiaoming Ni, Yin Fengwei, kernel-hardening, linux-api,
	linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, Elliott Hughes
In-Reply-To: <D2UC8YVOX9WU.1DRD4QFQ92L1U@kernel.org>

On Sat, Jul 20, 2024 at 02:43:41PM +0300, Jarkko Sakkinen wrote:
> On Sat Jul 20, 2024 at 4:59 AM EEST, Andy Lutomirski wrote:
> > > On Jul 18, 2024, at 8:22 PM, Mickaël Salaün <mic@digikod.net> wrote:
> > >
> > > On Thu, Jul 18, 2024 at 09:02:56AM +0800, Andy Lutomirski wrote:
> > >>>> On Jul 17, 2024, at 6:01 PM, Mickaël Salaün <mic@digikod.net> wrote:
> > >>>
> > >>> On Wed, Jul 17, 2024 at 09:26:22AM +0100, Steve Dower wrote:
> > >>>>> On 17/07/2024 07:33, Jeff Xu wrote:
> > >>>>> Consider those cases: I think:
> > >>>>> a> relying purely on userspace for enforcement does't seem to be
> > >>>>> effective,  e.g. it is trivial  to call open(), then mmap() it into
> > >>>>> executable memory.
> > >>>>
> > >>>> If there's a way to do this without running executable code that had to pass
> > >>>> a previous execveat() check, then yeah, it's not effective (e.g. a Python
> > >>>> interpreter that *doesn't* enforce execveat() is a trivial way to do it).
> > >>>>
> > >>>> Once arbitrary code is running, all bets are off. So long as all arbitrary
> > >>>> code is being checked itself, it's allowed to do things that would bypass
> > >>>> later checks (and it's up to whoever audited it in the first place to
> > >>>> prevent this by not giving it the special mark that allows it to pass the
> > >>>> check).
> > >>>
> > >>> Exactly.  As explained in the patches, one crucial prerequisite is that
> > >>> the executable code is trusted, and the system must provide integrity
> > >>> guarantees.  We cannot do anything without that.  This patches series is
> > >>> a building block to fix a blind spot on Linux systems to be able to
> > >>> fully control executability.
> > >>
> > >> Circling back to my previous comment (did that ever get noticed?), I
> > >
> > > Yes, I replied to your comments.  Did I miss something?
> >
> > I missed that email in the pile, sorry. I’ll reply separately.
> >
> > >
> > >> don’t think this is quite right:
> > >>
> > >> https://lore.kernel.org/all/CALCETrWYu=PYJSgyJ-vaa+3BGAry8Jo8xErZLiGR3U5h6+U0tA@mail.gmail.com/
> > >>
> > >> On a basic system configuration, a given path either may or may not be
> > >> executed. And maybe that path has some integrity check (dm-verity,
> > >> etc).  So the kernel should tell the interpreter/loader whether the
> > >> target may be executed. All fine.
> > >>
> > >> But I think the more complex cases are more interesting, and the
> > >> “execute a program” process IS NOT BINARY.  An attempt to execute can
> > >> be rejected outright, or it can be allowed *with a change to creds or
> > >> security context*.  It would be entirely reasonable to have a policy
> > >> that allows execution of non-integrity-checked files but in a very
> > >> locked down context only.
> > >
> > > I guess you mean to transition to a sandbox when executing an untrusted
> > > file.  This is a good idea.  I talked about role transition in the
> > > patch's description:
> > >
> > > With the information that a script interpreter is about to interpret a
> > > script, an LSM security policy can adjust caller's access rights or log
> > > execution request as for native script execution (e.g. role transition).
> > > This is possible thanks to the call to security_bprm_creds_for_exec().
> >
> > …
> >
> > > This patch series brings the minimal building blocks to have a
> > > consistent execution environment.  Role transitions for script execution
> > > are left to LSMs.  For instance, we could extend Landlock to
> > > automatically sandbox untrusted scripts.
> >
> > I’m not really convinced.  There’s more to building an API that
> > enables LSM hooks than merely sticking the hook somewhere in kernel
> > code. It needs to be a defined API. If you call an operation “check”,
> > then people will expect it to check, not to change the caller’s
> > credentials.  And people will mess it up in both directions (e.g.
> > callers will call it and then open try to load some library that they
> > should have loaded first, or callers will call it and forget to close
> > fds first.
> >
> > And there should probably be some interaction with dumpable as well.
> > If I “check” a file for executability, that should not suddenly allow
> > someone to ptrace me?
> >
> > And callers need to know to exit on failure, not carry on.
> >
> >
> > More concretely, a runtime that fully opts in to this may well "check"
> > multiple things.  For example, if I do:
> >
> > $ ld.so ~/.local/bin/some_program   (i.e. I literally execve ld.so)
> >
> > then ld.so will load several things:
> >
> > ~/.local/bin/some_program
> > libc.so
> > other random DSOs, some of which may well be in my home directory
> 
> What would really help to comprehend this patch set would be a set of
> test scripts, preferably something that you can run easily with
> BuildRoot or similar.
> 
> Scripts would demonstrate the use cases for the patch set. Then it
> would be easier to develop scripts that would underline the corner
> cases. I would keep all this out of kselftest shenanigans for now.

I'll include a toy script interpreter with the next patch series.  This
one was an RFC.

> 
> I feel that the patch set is hovering in abstractions with examples
> that you cannot execute.
> 
> I added the patches to standard test CI hack:
> 
> https://codeberg.org/jarkko/linux-tpmdd-test
> 
> But after I booted up a kernel I had no idea what to do with it. And
> all this lenghty discussion makes it even more confusing.

You can run the tests in the CI.

> 
> Please find some connection to the real world before sending any new
> version of this (e.g. via test scripts). I think this should not be
> pulled before almost anyone doing kernel dev can comprehend the "gist"
> at least in some reasonable level.

You'll find in this patch series (cover letter, patch description, and
comments) connection to the real world. :)
The next patch series should take into account the current discussions.

> 
> BR, Jarkko

^ permalink raw reply

* Re: [PATCH 1/2] uprobe: Change uretprobe syscall scope and number
From: Dmitry V. Levin @ 2024-07-24  7:46 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra,
	Andrii Nakryiko, Arnd Bergmann, linux-kernel, linux-trace-kernel,
	linux-api, x86, bpf, Thomas Gleixner, Borislav Petkov (AMD),
	Ingo Molnar, Andy Lutomirski, Deepak Gupta, Stephen Rothwell
In-Reply-To: <20240712135228.1619332-2-jolsa@kernel.org>

On Fri, Jul 12, 2024 at 03:52:27PM +0200, Jiri Olsa wrote:
> After discussing with Arnd [1] it's preferable to change uretprobe
> syscall number to 467 to omit the merge conflict with xattrat syscalls.
> 
> Also changing the ABI to 'common' which will ease up the global
> scripts/syscall.tbl management. One consequence is we generate uretprobe
> syscall numbers for ABIs that do not support uretprobe syscall, but the
> syscall still returns -ENOSYS when called in that ABI.
> 
> [1] https://lore.kernel.org/lkml/784a34e5-4654-44c9-9c07-f9f4ffd952a0@app.fastmail.com/
> 
> Fixes: 190fec72df4a ("uprobe: Wire up uretprobe system call")
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  arch/x86/entry/syscalls/syscall_64.tbl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index 6452c2ec469a..dabf1982de6d 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -384,7 +384,7 @@
>  460	common	lsm_set_self_attr	sys_lsm_set_self_attr
>  461	common	lsm_list_modules	sys_lsm_list_modules
>  462 	common  mseal			sys_mseal
> -463	64	uretprobe		sys_uretprobe
> +467	common	uretprobe		sys_uretprobe
>  
>  #
>  # Due to a historical design error, certain syscalls are numbered differently

Isn't include/uapi/asm-generic/unistd.h expected to be updated as well?
As of mainline commit v6.10-12246-g786c8248dbd3, it still contains

#define __NR_uretprobe 463


-- 
ldv

^ permalink raw reply

* Re: [PATCH 1/2] uprobe: Change uretprobe syscall scope and number
From: Arnd Bergmann @ 2024-07-24  8:11 UTC (permalink / raw)
  To: Dmitry V. Levin, Jiri Olsa
  Cc: Steven Rostedt, Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra,
	Andrii Nakryiko, linux-kernel, linux-trace-kernel, linux-api, x86,
	bpf, Thomas Gleixner, Borislav Petkov, Ingo Molnar,
	Andy Lutomirski, Deepak Gupta, Stephen Rothwell
In-Reply-To: <20240724074629.GA11265@altlinux.org>

On Wed, Jul 24, 2024, at 09:46, Dmitry V. Levin wrote:
> On Fri, Jul 12, 2024 at 03:52:27PM +0200, Jiri Olsa wrote:
>> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
>> index 6452c2ec469a..dabf1982de6d 100644
>> --- a/arch/x86/entry/syscalls/syscall_64.tbl
>> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
>> @@ -384,7 +384,7 @@
>>  460	common	lsm_set_self_attr	sys_lsm_set_self_attr
>>  461	common	lsm_list_modules	sys_lsm_list_modules
>>  462 	common  mseal			sys_mseal
>> -463	64	uretprobe		sys_uretprobe
>> +467	common	uretprobe		sys_uretprobe
>>  
>>  #
>>  # Due to a historical design error, certain syscalls are numbered differently
>
> Isn't include/uapi/asm-generic/unistd.h expected to be updated as well?
> As of mainline commit v6.10-12246-g786c8248dbd3, it still contains
>
> #define __NR_uretprobe 463

The file is currently unused and replaced with scripts/syscall.tbl,
my plan was to remove the old file in the 6.12 syscall cleanups.

The number in scripts/syscall.tbl is now 467, so its users (arc,
arm64, csky, hegagon, loongarch, nios2 openrisc and riscv) have
the same number as on x86.
However, the corresponding change did not make it into the
other syscall.tbl files (alpha, arm, m68k, microblaze, parisc,
powerpc, s390, sh, sparc and xtensa), which is rather
inconsistent.

I think we should definitely make all non-x86 architectures
behave the same way, either with or without an entry for
uretprobe. There are three ways do do this:

a) remove it from both include/uapi/asm/unistd.h and
   scripts/syscall.tbl, and change the x86-64 system call
   to a private number such as 335

b) remove it from both include/uapi/asm/unistd.h and
   scripts/syscall.tbl, but leave the number at 467

c) add the syscall to all other architectures for
   consistency, but continue to have it return -ENOSYS.

From Linus' earlier comments, I would guess that a) would
be the least bad of those. I'm also unsure about the status
of the xattrat patches, which were the reason for
changing uretprobe from 463 to 467. Those patches are still
not merged either, and disappeared from linux-next between
Friday and Monday.

      Arnd

^ permalink raw reply

* Re: [PATCH] epoll.7: clarify the event distribution under edge-triggered mode
From: Alejandro Colomar @ 2024-07-27 12:56 UTC (permalink / raw)
  To: i; +Cc: linux-man, linux-api
In-Reply-To: <20240727-epoll-et-desc-v1-1-390bafc678b9@andypan.me>

[-- Attachment #1: Type: text/plain, Size: 3476 bytes --]

On Sat, Jul 27, 2024 at 12:36:29PM GMT, Andy Pan via B4 Relay wrote:
> From: Andy Pan <i@andypan.me>

Hi Andy,

> 
> ---

I think the text below should go into the commit message.

> For the moment, the edge-triggered epoll generates an event for each
> receipt of a chunk of data, that is to say, epoll_wait() will return
> and tell us a monitored file descriptor is ready whenever there is a
> new activity on that FD since we were last informed about that FD.
> This is not a real _edge_ implementation for epoll, but it's been
> working this way for years and plenty of projects are relying on it. 
> 
> There are several renowned open-source projects relying on this feature
> for notification function (with eventfd), such as nginx [1], netty [2],
> tokio [3], libevent [4], ect. [5] These projects are widely used in today's
> Internet infrastructures. Thus, changing this behavior of epoll ET will
> fundamentally break them and cause a significant negative impact.
> Linux has changed it for pipe before [6], breaking some Android libraries,
> which had got "reverted" somehow. [7] [8]
> 
> Nevertheless, the paragraph in the manual pages describing this
> characteristic of epoll ET seems ambiguous, I think a more explict
> sentence should be used to clarify it. We're improving the notification
> mechanism for libuv recently by exploiting this feature with eventfd,
> which brings us a significant performance boost. [9]
> 
> Therefore, we (as well as the maintainers of nginx, netty, tokio, etc.)
> would have a sense of security to build an enhanced notification function
> based on this feature if there is a guarantee of retaining this implementation
> of epoll ET for the backward compatibility in the man pages.

You should CC <linux-api@vger.kernel.org>.

> [1]: https://github.com/nginx/nginx/blob/efc6a217b92985a1ee211b6bb7337cd2f62deb90/src/event/modules/ngx_epoll_module.c#L386-L457
> [2]: https://github.com/netty/netty/pull/9192
> [3]: https://github.com/tokio-rs/mio/blob/309daae21ecb1d46203a7dbc0cf4c80310240cba/src/sys/unix/waker.rs#L111-L143
> [4]: https://github.com/libevent/libevent/blob/525f5d0a14c9c103be750f2ca175328c25505ea4/event.c#L2597-L2614
> [5]: https://github.com/libuv/libuv/pull/4400#issuecomment-2123798748
> [6]: https://lkml.iu.edu/hypermail/linux/kernel/2010.1/04363.html
> [7]: https://github.com/torvalds/linux/commit/3a34b13a88caeb2800ab44a4918f230041b37dd9
> [8]: https://github.com/torvalds/linux/commit/3b844826b6c6affa80755254da322b017358a2f4
> [9]: https://github.com/libuv/libuv/pull/4400#issuecomment-2103232402
> 
> Signed-off-by: Andy Pan <i@andypan.me>
> ---
>  man/man7/epoll.7 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/man/man7/epoll.7 b/man/man7/epoll.7
> index 951500131..361d9db99 100644
> --- a/man/man7/epoll.7
> +++ b/man/man7/epoll.7
> @@ -172,6 +172,7 @@ .SS Level-triggered and edge-triggered
>  Since even with edge-triggered
>  .BR epoll ,
>  multiple events can be generated upon receipt of multiple chunks of data,
> +that is, an event will be generated upon each receipt of a chunk of data,
>  the caller has the option to specify the
>  .B EPOLLONESHOT
>  flag, to tell
> 
> ---
> base-commit: cbc0a111e4dceea2037c51098de33e6bc8c16a5c
> change-id: 20240727-epoll-et-desc-04ea9a590f3b
> 
> Best regards,
> -- 
> Andy Pan <i@andypan.me>

Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH v2] epoll.7: clarify the event distribution under edge-triggered mode
From: Andy Pan via B4 Relay @ 2024-07-27 14:52 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: linux-man, linux-api, Andy Pan

From: Andy Pan <i@andypan.me>

For the moment, the edge-triggered epoll generates an event for each
receipt of a chunk of data, that is to say, epoll_wait() will return
and tell us a monitored file descriptor is ready whenever there is a
new activity on that FD since we were last informed about that FD.
This is not a real _edge_ implementation for epoll, but it's been
working this way for years and plenty of projects are relying on it.

There are several renowned open-source projects relying on this feature
for notification function (with eventfd), such as nginx [1], netty [2],
tokio [3], libevent [4], ect. [5] These projects are widely used in today's
Internet infrastructures. Thus, changing this behavior of epoll ET will
fundamentally break them and cause a significant negative impact.
Linux has changed it for pipe before [6], breaking some Android libraries,
which had got "reverted" somehow. [7] [8]

Nevertheless, the paragraph in the manual pages describing this
characteristic of epoll ET seems ambiguous, I think a more explict
sentence should be used to clarify it. We're improving the notification
mechanism for libuv recently by exploiting this feature with eventfd,
which brings us a significant performance boost. [9]

Therefore, we (as well as the maintainers of nginx, netty, tokio, etc.)
would have a sense of security to build an enhanced notification function
based on this feature if there is a guarantee of retaining this implementation
of epoll ET for the backward compatibility in the man pages.

[1]: https://github.com/nginx/nginx/blob/efc6a217b92985a1ee211b6bb7337cd2f62deb90/src/event/modules/ngx_epoll_module.c#L386-L457
[2]: https://github.com/netty/netty/pull/9192
[3]: https://github.com/tokio-rs/mio/blob/309daae21ecb1d46203a7dbc0cf4c80310240cba/src/sys/unix/waker.rs#L111-L143
[4]: https://github.com/libevent/libevent/blob/525f5d0a14c9c103be750f2ca175328c25505ea4/event.c#L2597-L2614
[5]: https://github.com/libuv/libuv/pull/4400#issuecomment-2123798748
[6]: https://lkml.iu.edu/hypermail/linux/kernel/2010.1/04363.html
[7]: https://github.com/torvalds/linux/commit/3a34b13a88caeb2800ab44a4918f230041b37dd9
[8]: https://github.com/torvalds/linux/commit/3b844826b6c6affa80755254da322b017358a2f4
[9]: https://github.com/libuv/libuv/pull/4400#issuecomment-2103232402

Signed-off-by: Andy Pan <i@andypan.me>
---
I've added it to the commit description, please take a look again, thanks!
---
Changes in v2:
- [PATCH v2] epoll.7: add git commit description
- Link to v1: https://lore.kernel.org/r/20240727-epoll-et-desc-v1-1-390bafc678b9@andypan.me
---
 man/man7/epoll.7 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/man/man7/epoll.7 b/man/man7/epoll.7
index 951500131..361d9db99 100644
--- a/man/man7/epoll.7
+++ b/man/man7/epoll.7
@@ -172,6 +172,7 @@ .SS Level-triggered and edge-triggered
 Since even with edge-triggered
 .BR epoll ,
 multiple events can be generated upon receipt of multiple chunks of data,
+that is, an event will be generated upon each receipt of a chunk of data,
 the caller has the option to specify the
 .B EPOLLONESHOT
 flag, to tell

---
base-commit: cbc0a111e4dceea2037c51098de33e6bc8c16a5c
change-id: 20240727-epoll-et-desc-04ea9a590f3b

Best regards,
-- 
Andy Pan <i@andypan.me>



^ permalink raw reply related

* Testing if two open descriptors refer to the same inode
From: Florian Weimer @ 2024-07-29  6:55 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel, linux-api; +Cc: Dave Chinner

It was pointed out to me that inode numbers on Linux are no longer
expected to be unique per file system, even for local file systems.
Applications sometimes need to check if two (open) files are the same.
For example, a program may want to use a temporary file if is invoked
with input and output files referring to the same file.

How can we check for this?  The POSIX way is to compare st_ino and
st_dev in stat output, but if inode numbers are not unique, that will
result in files falsely being reported as identical.  It's harmless in
the temporary file case, but it in other scenarios, it may result in
data loss.

Thanks,
Florian


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox