Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
On 19/05/2020 04:23, Aleksa Sarai wrote:
> On 2020-05-15, Kees Cook wrote:
>> On Fri, May 15, 2020 at 04:43:37PM +0200, Florian Weimer wrote:
>>> * Kees Cook:
>>>
On Fri, May 15, 2020 at 10:43:34AM +0200, Florian Weimer wrote:
> * Kees Cook:
>
>> Maybe I've missed some earlier discussion that ruled this out, but I
>> couldn't find it: let's just add O_EXEC and be done with it. It actually
>> makes the execve() path more like openat2() and is much cleaner after
>> a little refactoring. Here are the results, though I haven't emailed it
>> yet since I still want to do some more testing:
>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/o_exec/v1
>
> I think POSIX specifies O_EXEC in such a way that it does not confer
> read permissions. This seems incompatible with what we are trying to
> achieve here.
I was trying to retain this behavior, since we already make this
distinction between execve() and uselib() with the MAY_* flags:
execve():
struct open_flags open_exec_flags = {
.open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
.acc_mode = MAY_EXEC,
uselib():
static const struct open_flags uselib_flags = {
.open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
.acc_mode = MAY_READ | MAY_EXEC,
I tried to retain this in my proposal, in the O_EXEC does not imply
MAY_READ:
>>>
>>> That doesn't quite parse for me, sorry.
>>>
>>> The point is that the script interpreter actually needs to *read* those
>>> files in order to execute them.
>>
>> I think I misunderstood what you meant (Mickaël got me sorted out
>> now). If O_EXEC is already meant to be "EXEC and _not_ READ nor WRITE",
>> then yes, this new flag can't be O_EXEC. I was reading the glibc
>> documentation (which treats it as a permission bit flag, not POSIX,
>> which treats it as a complete mode description).
>
> On the other hand, if we had O_EXEC (or O_EXONLY a-la O_RDONLY) then the
> interpreter could re-open the file descriptor as O_RDONLY after O_EXEC
> succeeds. Not ideal, but I don't think it's a deal-breaker.
>
> Regarding O_MAYEXEC, I do feel a little conflicted.
>
> I do understand that its goal is not to be what O_EXEC was supposed to
> be (which is loosely what O_PATH has effectively become), so I think
> that this is not really a huge problem -- especially since you could
> just do O_MAYEXEC|O_PATH if you wanted to disallow reading explicitly.
> It would be nice to have an O_EXONLY concept, but it's several decades
> too late to make it mandatory (and making it optional has questionable
> utility IMHO).
>
> However, the thing I still feel mildly conflicted about is the sysctl. I
> do understand the argument for it (ultimately, whether O_MAYEXEC is
> usable on a system depends on the distribution) but it means that any
> program which uses O_MAYEXEC cannot rely on it to provide the security
> guarantees they expect. Even if the program goes and reads the sysctl
> value, it could change underneath them. If this is just meant to be a
> best-effort protection then this doesn't matter too much, but I just
> feel uneasy about these kinds of best-effort protections.
I think there is a cognitive bias here. There is a difference between
application-centric policies and system policies. For example, openat2
RESOLVE_* flags targets application developers and are self-sufficient:
the kernel provides features (applied to FDs, owned and managed by user
space) which must be known (by the application) to be supported (by the
kernel), otherwise the application may give more privileges than
expected. However, the O_MAYEXEC flag targets system administrators: it
does not make sense to enable an application to know nor enforce the
system(-wide) policy, but only to enable applications to follow this
policy (i.e. best-effort *from the application developer point of
view*). Indeed, access-control such as file executability depends on
multiple layers (e.g. file permission, mount options, ACL, SELinux
policy), most of them managed and enforced in a consistent way by
(multiple parts of) the system.
Applications should not and it does not make sense for them to expect
anything from O_MAYEXEC. This flag only enables the system to enforce a
security policy and that's all. It is really a different use case than
FD management. This feature is meant to extend the system ability thanks
to applications collaboration. Here the sysctl should not be looked at
by applications, the same way an application should not look at the
currently enforced SELinux policy nor the mount options. An application
may be launched differently according to the system-wide policy, but
this is again a system configuration. There is a difference between ABI
compatibility (i.e. does this feature is supported by the kernel?) and
system-wide security policy (what
Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
On 2020-05-15, Kees Cook wrote:
> On Fri, May 15, 2020 at 04:43:37PM +0200, Florian Weimer wrote:
> > * Kees Cook:
> >
> > > On Fri, May 15, 2020 at 10:43:34AM +0200, Florian Weimer wrote:
> > >> * Kees Cook:
> > >>
> > >> > Maybe I've missed some earlier discussion that ruled this out, but I
> > >> > couldn't find it: let's just add O_EXEC and be done with it. It
> > >> > actually
> > >> > makes the execve() path more like openat2() and is much cleaner after
> > >> > a little refactoring. Here are the results, though I haven't emailed it
> > >> > yet since I still want to do some more testing:
> > >> > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/o_exec/v1
> > >>
> > >> I think POSIX specifies O_EXEC in such a way that it does not confer
> > >> read permissions. This seems incompatible with what we are trying to
> > >> achieve here.
> > >
> > > I was trying to retain this behavior, since we already make this
> > > distinction between execve() and uselib() with the MAY_* flags:
> > >
> > > execve():
> > > struct open_flags open_exec_flags = {
> > > .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
> > > .acc_mode = MAY_EXEC,
> > >
> > > uselib():
> > > static const struct open_flags uselib_flags = {
> > > .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
> > > .acc_mode = MAY_READ | MAY_EXEC,
> > >
> > > I tried to retain this in my proposal, in the O_EXEC does not imply
> > > MAY_READ:
> >
> > That doesn't quite parse for me, sorry.
> >
> > The point is that the script interpreter actually needs to *read* those
> > files in order to execute them.
>
> I think I misunderstood what you meant (Mickaël got me sorted out
> now). If O_EXEC is already meant to be "EXEC and _not_ READ nor WRITE",
> then yes, this new flag can't be O_EXEC. I was reading the glibc
> documentation (which treats it as a permission bit flag, not POSIX,
> which treats it as a complete mode description).
On the other hand, if we had O_EXEC (or O_EXONLY a-la O_RDONLY) then the
interpreter could re-open the file descriptor as O_RDONLY after O_EXEC
succeeds. Not ideal, but I don't think it's a deal-breaker.
Regarding O_MAYEXEC, I do feel a little conflicted.
I do understand that its goal is not to be what O_EXEC was supposed to
be (which is loosely what O_PATH has effectively become), so I think
that this is not really a huge problem -- especially since you could
just do O_MAYEXEC|O_PATH if you wanted to disallow reading explicitly.
It would be nice to have an O_EXONLY concept, but it's several decades
too late to make it mandatory (and making it optional has questionable
utility IMHO).
However, the thing I still feel mildly conflicted about is the sysctl. I
do understand the argument for it (ultimately, whether O_MAYEXEC is
usable on a system depends on the distribution) but it means that any
program which uses O_MAYEXEC cannot rely on it to provide the security
guarantees they expect. Even if the program goes and reads the sysctl
value, it could change underneath them. If this is just meant to be a
best-effort protection then this doesn't matter too much, but I just
feel uneasy about these kinds of best-effort protections.
I do wonder if we could require that fexecve(3) can only be done with
file descriptors that have been opened with O_MAYEXEC (obviously this
would also need to be a sysctl -- *sigh*). This would tie in to some of
the magic-link changes I wanted to push (namely, upgrade_mask).
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
* Kees Cook: > I think I misunderstood what you meant (Mickaël got me sorted out > now). If O_EXEC is already meant to be "EXEC and _not_ READ nor WRITE", > then yes, this new flag can't be O_EXEC. I was reading the glibc > documentation (which treats it as a permission bit flag, not POSIX, > which treats it as a complete mode description). I see. I think this part of the manual is actually very Hurd-specific (before the O_ACCMODE description). I'll see if I can make this clearer in the markup. Thanks, Florian
Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
On 15/05/2020 17:46, Kees Cook wrote: > On Fri, May 15, 2020 at 01:04:08PM +0200, Mickaël Salaün wrote: >> >> On 15/05/2020 10:01, Kees Cook wrote: >>> On Thu, May 14, 2020 at 09:16:13PM +0200, Mickaël Salaün wrote: On 14/05/2020 18:10, Stephen Smalley wrote: > On Thu, May 14, 2020 at 11:45 AM Kees Cook wrote: >> So, it looks like adding FMODE_EXEC into f_flags in do_open() is needed >> in >> addition to injecting MAY_EXEC into acc_mode in do_open()? Hmmm > > Just do both in build_open_flags() and be done with it? Looks like he > was already setting FMODE_EXEC in patch 1 so we just need to teach > AppArmor/TOMOYO to check for it and perform file execute checking in > that case if !current->in_execve? I can postpone the file permission check for another series to make this one simpler (i.e. mount noexec only). Because it depends on the sysctl setting, it is OK to add this check later, if needed. In the meantime, AppArmor and Tomoyo could be getting ready for this. >>> >>> So, after playing around with this series, investigating Stephen's >>> comments, digging through the existing FMODE_EXEC uses, and spending a >>> bit more time thinking about Lev and Aleksa's dislike of the sysctls, I've >>> got a much more radically simplified solution that I think could work. >> >> Not having a sysctl would mean that distros will probably have to patch >> script interpreters to remove the use of O_MAYEXEC. Or distros would >> have to exclude newer version of script interpreters because they >> implement O_MAYEXEC. Or distros would have to patch their kernel to >> implement themselves the sysctl knob I'm already providing. Sysadmins >> may not control the kernel build nor the user space build, they control >> the system configuration (some mount point options and some file >> execution permissions) but I guess that a distro update breaking a >> running system is not acceptable. Either way, unfortunately, I think it >> doesn't help anyone to not have a controlling sysctl. The same apply for >> access-control LSMs relying on a security policy which can be defined by >> sysadmins. >> >> Your commits enforce file exec checks, which is a good thing from a >> security point of view, but unfortunately that would requires distros to >> update all the packages providing shared objects once the dynamic linker >> uses O_MAYEXEC. > > I used to agree with this, but I'm now convinced now that the sysctls are > redundant and will ultimately impede adoption. In looking at what levels > the existing (CLIP OS, Chrome OS) and future (PEP 578) implementations > have needed to do to meaningfully provide the protection, it seems > like software will not be using this flag out of the blue. It'll need > careful addition way beyond the scope of just a sysctl. (As in, I don't > think using O_MAYEXEC is going to just get added without thought to all > interpreters. And developers that DO add it will want to know that the > system will behave in the specified way: having it be off by default > will defeat the purpose of adding the flag for the end users.) I think that the different points of view should be the following: - kernel developer: the app *may* behave this way; - user space developer: the app *should* behave this way; - sysadmin: the app *must* behave this way (either enforcing O_MAYEXEC or not). The idea is to push adoption of O_MAYEXEC to upstream interpreters, knowing that it will not break anything on running systems which do not care about this features. However, on systems which want this feature enforced, there will be knowledgeable peoples (i.e. sysadmins who enforced O_MAYEXEC deliberately) to manage it. If we don't give the opportunity to sysadmins to control this feature, no upstream interpreters will adopt it. Only tailored distro will use it, maybe with custom LSM or other way to enforce it anyway, and having a sysctl or not is not an issue neither. I think it would be a missed opportunity to help harden most Linux systems. > > I think it boils down to deciding how to control enforcement: should it > be up to the individual piece of software, or should it be system-wide? > Looking at the patches Chrome OS has made to the shell (and the > accompanying system changes), and Python's overall plans, it seems to > me that the requirements for meaningfully using this flag is going to > be very software-specific. > > Now, if the goal is to try to get O_MAYEXEC into every interpreter as > widely as possible without needing to wait for the software-specific > design changes, then I can see the reason to want a default-off global > sysctl. Yes, that is our intention: to make this flag used by most interpreters, without breaking any existing systems. > (Though in that case, I suspect it needs to be tied to userns or > something to support containers with different enforcement levels.) The LSMs already manage security policies, but the sysctl could indeed be ti
Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
On Fri, May 15, 2020 at 04:43:37PM +0200, Florian Weimer wrote:
> * Kees Cook:
>
> > On Fri, May 15, 2020 at 10:43:34AM +0200, Florian Weimer wrote:
> >> * Kees Cook:
> >>
> >> > Maybe I've missed some earlier discussion that ruled this out, but I
> >> > couldn't find it: let's just add O_EXEC and be done with it. It actually
> >> > makes the execve() path more like openat2() and is much cleaner after
> >> > a little refactoring. Here are the results, though I haven't emailed it
> >> > yet since I still want to do some more testing:
> >> > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/o_exec/v1
> >>
> >> I think POSIX specifies O_EXEC in such a way that it does not confer
> >> read permissions. This seems incompatible with what we are trying to
> >> achieve here.
> >
> > I was trying to retain this behavior, since we already make this
> > distinction between execve() and uselib() with the MAY_* flags:
> >
> > execve():
> > struct open_flags open_exec_flags = {
> > .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
> > .acc_mode = MAY_EXEC,
> >
> > uselib():
> > static const struct open_flags uselib_flags = {
> > .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
> > .acc_mode = MAY_READ | MAY_EXEC,
> >
> > I tried to retain this in my proposal, in the O_EXEC does not imply
> > MAY_READ:
>
> That doesn't quite parse for me, sorry.
>
> The point is that the script interpreter actually needs to *read* those
> files in order to execute them.
I think I misunderstood what you meant (Mickaël got me sorted out
now). If O_EXEC is already meant to be "EXEC and _not_ READ nor WRITE",
then yes, this new flag can't be O_EXEC. I was reading the glibc
documentation (which treats it as a permission bit flag, not POSIX,
which treats it as a complete mode description).
--
Kees Cook
Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
On Fri, May 15, 2020 at 01:04:08PM +0200, Mickaël Salaün wrote: > > On 15/05/2020 10:01, Kees Cook wrote: > > On Thu, May 14, 2020 at 09:16:13PM +0200, Mickaël Salaün wrote: > >> On 14/05/2020 18:10, Stephen Smalley wrote: > >>> On Thu, May 14, 2020 at 11:45 AM Kees Cook wrote: > So, it looks like adding FMODE_EXEC into f_flags in do_open() is needed > in > addition to injecting MAY_EXEC into acc_mode in do_open()? Hmmm > >>> > >>> Just do both in build_open_flags() and be done with it? Looks like he > >>> was already setting FMODE_EXEC in patch 1 so we just need to teach > >>> AppArmor/TOMOYO to check for it and perform file execute checking in > >>> that case if !current->in_execve? > >> > >> I can postpone the file permission check for another series to make this > >> one simpler (i.e. mount noexec only). Because it depends on the sysctl > >> setting, it is OK to add this check later, if needed. In the meantime, > >> AppArmor and Tomoyo could be getting ready for this. > > > > So, after playing around with this series, investigating Stephen's > > comments, digging through the existing FMODE_EXEC uses, and spending a > > bit more time thinking about Lev and Aleksa's dislike of the sysctls, I've > > got a much more radically simplified solution that I think could work. > > Not having a sysctl would mean that distros will probably have to patch > script interpreters to remove the use of O_MAYEXEC. Or distros would > have to exclude newer version of script interpreters because they > implement O_MAYEXEC. Or distros would have to patch their kernel to > implement themselves the sysctl knob I'm already providing. Sysadmins > may not control the kernel build nor the user space build, they control > the system configuration (some mount point options and some file > execution permissions) but I guess that a distro update breaking a > running system is not acceptable. Either way, unfortunately, I think it > doesn't help anyone to not have a controlling sysctl. The same apply for > access-control LSMs relying on a security policy which can be defined by > sysadmins. > > Your commits enforce file exec checks, which is a good thing from a > security point of view, but unfortunately that would requires distros to > update all the packages providing shared objects once the dynamic linker > uses O_MAYEXEC. I used to agree with this, but I'm now convinced now that the sysctls are redundant and will ultimately impede adoption. In looking at what levels the existing (CLIP OS, Chrome OS) and future (PEP 578) implementations have needed to do to meaningfully provide the protection, it seems like software will not be using this flag out of the blue. It'll need careful addition way beyond the scope of just a sysctl. (As in, I don't think using O_MAYEXEC is going to just get added without thought to all interpreters. And developers that DO add it will want to know that the system will behave in the specified way: having it be off by default will defeat the purpose of adding the flag for the end users.) I think it boils down to deciding how to control enforcement: should it be up to the individual piece of software, or should it be system-wide? Looking at the patches Chrome OS has made to the shell (and the accompanying system changes), and Python's overall plans, it seems to me that the requirements for meaningfully using this flag is going to be very software-specific. Now, if the goal is to try to get O_MAYEXEC into every interpreter as widely as possible without needing to wait for the software-specific design changes, then I can see the reason to want a default-off global sysctl. (Though in that case, I suspect it needs to be tied to userns or something to support containers with different enforcement levels.) > > Maybe I've missed some earlier discussion that ruled this out, but I > > couldn't find it: let's just add O_EXEC and be done with it. It actually > > makes the execve() path more like openat2() and is much cleaner after > > a little refactoring. Here are the results, though I haven't emailed it > > yet since I still want to do some more testing: > > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/o_exec/v1 > > > > I look forward to flames! ;) > > > > Like Florian said, O_EXEC is for execute-only (which obviously doesn't > work for scripts): > https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html > On the other hand, the semantic of O_MAYEXEC is complementary to other > O_* flags. It is inspired by the VM_MAYEXEC flag. Ah! I see now -- it's intended to be like the O_*ONLY flags. I misunderstood what Florian meant. Okay, sure that's a good enough reason for me to retain the O_MAYEXEC name. (And then I think this distinction from O_EXEC needs to be well documented.) > The O_EXEC flag is specified for open(2). openat2(2) is Linux-specific > and it is highly unlikely that new flags will be added to open(2) or > openat(2) because of compatibility i
Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
* Kees Cook:
> On Fri, May 15, 2020 at 10:43:34AM +0200, Florian Weimer wrote:
>> * Kees Cook:
>>
>> > Maybe I've missed some earlier discussion that ruled this out, but I
>> > couldn't find it: let's just add O_EXEC and be done with it. It actually
>> > makes the execve() path more like openat2() and is much cleaner after
>> > a little refactoring. Here are the results, though I haven't emailed it
>> > yet since I still want to do some more testing:
>> > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/o_exec/v1
>>
>> I think POSIX specifies O_EXEC in such a way that it does not confer
>> read permissions. This seems incompatible with what we are trying to
>> achieve here.
>
> I was trying to retain this behavior, since we already make this
> distinction between execve() and uselib() with the MAY_* flags:
>
> execve():
> struct open_flags open_exec_flags = {
> .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
> .acc_mode = MAY_EXEC,
>
> uselib():
> static const struct open_flags uselib_flags = {
> .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
> .acc_mode = MAY_READ | MAY_EXEC,
>
> I tried to retain this in my proposal, in the O_EXEC does not imply
> MAY_READ:
That doesn't quite parse for me, sorry.
The point is that the script interpreter actually needs to *read* those
files in order to execute them.
Thanks,
Florian
Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
On Fri, May 15, 2020 at 10:43:34AM +0200, Florian Weimer wrote:
> * Kees Cook:
>
> > Maybe I've missed some earlier discussion that ruled this out, but I
> > couldn't find it: let's just add O_EXEC and be done with it. It actually
> > makes the execve() path more like openat2() and is much cleaner after
> > a little refactoring. Here are the results, though I haven't emailed it
> > yet since I still want to do some more testing:
> > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/o_exec/v1
>
> I think POSIX specifies O_EXEC in such a way that it does not confer
> read permissions. This seems incompatible with what we are trying to
> achieve here.
I was trying to retain this behavior, since we already make this
distinction between execve() and uselib() with the MAY_* flags:
execve():
struct open_flags open_exec_flags = {
.open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
.acc_mode = MAY_EXEC,
uselib():
static const struct open_flags uselib_flags = {
.open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
.acc_mode = MAY_READ | MAY_EXEC,
I tried to retain this in my proposal, in the O_EXEC does not imply
MAY_READ:
+ /* Should execution permissions be checked on open? */
+ if (flags & O_EXEC) {
+ flags |= __FMODE_EXEC;
+ acc_mode |= MAY_EXEC;
+ }
--
Kees Cook
Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
On 15/05/2020 10:01, Kees Cook wrote: > On Thu, May 14, 2020 at 09:16:13PM +0200, Mickaël Salaün wrote: >> On 14/05/2020 18:10, Stephen Smalley wrote: >>> On Thu, May 14, 2020 at 11:45 AM Kees Cook wrote: So, it looks like adding FMODE_EXEC into f_flags in do_open() is needed in addition to injecting MAY_EXEC into acc_mode in do_open()? Hmmm >>> >>> Just do both in build_open_flags() and be done with it? Looks like he >>> was already setting FMODE_EXEC in patch 1 so we just need to teach >>> AppArmor/TOMOYO to check for it and perform file execute checking in >>> that case if !current->in_execve? >> >> I can postpone the file permission check for another series to make this >> one simpler (i.e. mount noexec only). Because it depends on the sysctl >> setting, it is OK to add this check later, if needed. In the meantime, >> AppArmor and Tomoyo could be getting ready for this. > > So, after playing around with this series, investigating Stephen's > comments, digging through the existing FMODE_EXEC uses, and spending a > bit more time thinking about Lev and Aleksa's dislike of the sysctls, I've > got a much more radically simplified solution that I think could work. Not having a sysctl would mean that distros will probably have to patch script interpreters to remove the use of O_MAYEXEC. Or distros would have to exclude newer version of script interpreters because they implement O_MAYEXEC. Or distros would have to patch their kernel to implement themselves the sysctl knob I'm already providing. Sysadmins may not control the kernel build nor the user space build, they control the system configuration (some mount point options and some file execution permissions) but I guess that a distro update breaking a running system is not acceptable. Either way, unfortunately, I think it doesn't help anyone to not have a controlling sysctl. The same apply for access-control LSMs relying on a security policy which can be defined by sysadmins. Your commits enforce file exec checks, which is a good thing from a security point of view, but unfortunately that would requires distros to update all the packages providing shared objects once the dynamic linker uses O_MAYEXEC. > > Maybe I've missed some earlier discussion that ruled this out, but I > couldn't find it: let's just add O_EXEC and be done with it. It actually > makes the execve() path more like openat2() and is much cleaner after > a little refactoring. Here are the results, though I haven't emailed it > yet since I still want to do some more testing: > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/o_exec/v1 > > I look forward to flames! ;) > Like Florian said, O_EXEC is for execute-only (which obviously doesn't work for scripts): https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html On the other hand, the semantic of O_MAYEXEC is complementary to other O_* flags. It is inspired by the VM_MAYEXEC flag. The O_EXEC flag is specified for open(2). openat2(2) is Linux-specific and it is highly unlikely that new flags will be added to open(2) or openat(2) because of compatibility issues. FYI, musl implements O_EXEC on Linux with O_PATH: https://www.openwall.com/lists/musl/2013/02/22/1 https://git.musl-libc.org/cgit/musl/commit/?id=6d05d862975188039e648273ceab350d9ab5b69e However, the O_EXEC flag/semantic could be useful for the dynamic linkers, i.e. to only be able to map files in an executable (and read-only) way. If this is OK, then we may want to rename O_MAYEXEC to something like O_INTERPRET. This way we could have two new flags for sightly (but important) different use cases. The sysctl bitfield could be extended to manage both of these flags. Other than that, the other commits are interesting. I'm a bit worried about the implication of the f_flags/f_mode change though. >From a practical point of view, I'm also wondering how you intent to submit this series on LKML without conflicting with the current O_MAYEXEC series (versions, changes…). I would like you to keep the warnings from my patches about other ways to execute/interpret code and the threat model (patch 1/6 and 5/6).
Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
* Kees Cook: > Maybe I've missed some earlier discussion that ruled this out, but I > couldn't find it: let's just add O_EXEC and be done with it. It actually > makes the execve() path more like openat2() and is much cleaner after > a little refactoring. Here are the results, though I haven't emailed it > yet since I still want to do some more testing: > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/o_exec/v1 I think POSIX specifies O_EXEC in such a way that it does not confer read permissions. This seems incompatible with what we are trying to achieve here. Thanks, Florian
