* Re: Testing if two open descriptors refer to the same inode
From: Florian Weimer @ 2024-07-29 10:40 UTC (permalink / raw)
To: Mateusz Guzik; +Cc: linux-fsdevel, linux-kernel, linux-api, Dave Chinner
In-Reply-To: <ghqndyn4x7ujxvybbwet5vxiahus4zey6nkfsv6he3d4en6ehu@bq5s23lstzor>
* Mateusz Guzik:
> On Mon, Jul 29, 2024 at 08:55:46AM +0200, Florian Weimer wrote:
>> 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.
>
> I don't know if I'm parsing this correctly.
>
> Are you claiming on-disk inode numbers are not guaranteed unique per
> filesystem? It sounds like utter breakage, with capital 'f'.
Yes, POSIX semantics and traditional Linux semantics for POSIX-like
local file systems are different.
> While the above is not what's needed here, I guess it sets a precedent
> for F_DUPINODE_QUERY (or whatever other name) to be added to handily
> compare inode pointers. It may be worthwhile regardless of the above.
> (or maybe kcmp could be extended?)
I looked at kcmp as well, but I think it's dependent on
checkpoint/restore. File sameness checks are much more basic than that.
Thanks,
Florian
^ permalink raw reply
* Re: Testing if two open descriptors refer to the same inode
From: Mateusz Guzik @ 2024-07-29 10:18 UTC (permalink / raw)
To: Florian Weimer; +Cc: linux-fsdevel, linux-kernel, linux-api, Dave Chinner
In-Reply-To: <874j88sn4d.fsf@oldenburg.str.redhat.com>
On Mon, Jul 29, 2024 at 08:55:46AM +0200, Florian Weimer wrote:
> 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.
I don't know if I'm parsing this correctly.
Are you claiming on-disk inode numbers are not guaranteed unique per
filesystem? It sounds like utter breakage, with capital 'f'.
I know the 32-bit inode allocation code can result in unintentional
duplicates after wrap around (see get_next_ino), but that's for
in-memory stuff only(?) like pipes, so perhaps tolerable.
Anyhow, the kernel recently got F_DUPFD_QUERY which tests if the *file*
object is the same.
While the above is not what's needed here, I guess it sets a precedent
for F_DUPINODE_QUERY (or whatever other name) to be added to handily
compare inode pointers. It may be worthwhile regardless of the above.
(or maybe kcmp could be extended?)
^ permalink raw reply
* Re: Testing if two open descriptors refer to the same inode
From: Florian Weimer @ 2024-07-29 9:29 UTC (permalink / raw)
To: Aleksa Sarai; +Cc: linux-fsdevel, linux-kernel, linux-api, Dave Chinner
In-Reply-To: <20240729.085339-ebony.subplot.isolated.pops-b8estyg9vB9Q@cyphar.com>
* Aleksa Sarai:
> On 2024-07-29, Florian Weimer <fweimer@redhat.com> wrote:
>> 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.
>
> Based on the discussions we had at LSF/MM, I believe the "correct" way
> now is to do
>
> name_to_handle_at(fd, "", ..., AT_EMPTY_PATH|AT_HANDLE_FID)
>
> and then use the fhandle as the key to compare inodes. AT_HANDLE_FID is
> needed for filesystems that don't support decoding file handles, and was
> added in Linux 6.6[1]. However, I think this inode issue is only
> relevant for btree filesystems, and I think both btrfs and bcachefs both
> support decoding fhandles so this should work on fairly old kernels
> without issue (though I haven't checked).
> [1]: commit 96b2b072ee62 ("exportfs: allow exporting non-decodeable file handles to userspace")
Thanks, it's not too bad. The name_to_handle_at manual page says that
the handle is supposed to be treated as an opaque value, although it
mentions AT_HANDLE_FID. I think this needs to be fixed that it's
expected to compare the handle bytes, and also say whether it's
necessary to compare the type or not.
> Lennart suggested there should be a way to get this information from
> statx(2) so that you can get this new inode identifier without doing a
> bunch of extra syscalls to verify that inode didn't change between the
> two syscalls. I have a patchset for this, but I suspect it's too ugly
> (we can't return the full file handle so we need to hash it). I'll send
> an RFC later this week or next.
Hashing these things is rather nasty because it makes things impossible
to test.
>> 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.
>
> (Another problem is that st_dev can be different for the same mount due
> to subvolumes.)
Uh-oh. If st_dev are different, is it still possible that truncating
one path will affect the other with the different st_dev value?
Thanks,
Florian
^ permalink raw reply
* Re: Testing if two open descriptors refer to the same inode
From: Aleksa Sarai @ 2024-07-29 9:09 UTC (permalink / raw)
To: Florian Weimer; +Cc: linux-fsdevel, linux-kernel, linux-api, Dave Chinner
In-Reply-To: <874j88sn4d.fsf@oldenburg.str.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]
On 2024-07-29, Florian Weimer <fweimer@redhat.com> wrote:
> 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.
Based on the discussions we had at LSF/MM, I believe the "correct" way
now is to do
name_to_handle_at(fd, "", ..., AT_EMPTY_PATH|AT_HANDLE_FID)
and then use the fhandle as the key to compare inodes. AT_HANDLE_FID is
needed for filesystems that don't support decoding file handles, and was
added in Linux 6.6[1]. However, I think this inode issue is only
relevant for btree filesystems, and I think both btrfs and bcachefs both
support decoding fhandles so this should work on fairly old kernels
without issue (though I haven't checked).
Lennart suggested there should be a way to get this information from
statx(2) so that you can get this new inode identifier without doing a
bunch of extra syscalls to verify that inode didn't change between the
two syscalls. I have a patchset for this, but I suspect it's too ugly
(we can't return the full file handle so we need to hash it). I'll send
an RFC later this week or next.
[1]: commit 96b2b072ee62 ("exportfs: allow exporting non-decodeable file handles to userspace")
> 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.
(Another problem is that st_dev can be different for the same mount due
to subvolumes.)
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* 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
* [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
* 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
* 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 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: [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: [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: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: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 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: [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 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: [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: 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 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: 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 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: 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: 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 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 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox