[RFC,v1,1/5] fs: Add support for an O_MAYEXEC flag on sys_open()

Commit Message

Mickaël Salaün Dec. 12, 2018, 8:17 a.m. UTC

When the O_MAYEXEC flag is passed, sys_open() may be subject to
additional restrictions depending on a security policy implemented by an
LSM through the inode_permission hook.

The underlying idea is to be able to restrict scripts interpretation
according to a policy defined by the system administrator.  For this to
be possible, script interpreters must use the O_MAYEXEC flag
appropriately.  To be fully effective, these interpreters also need to
handle the other ways to execute code (for which the kernel can't help):
command line parameters (e.g., option -e for Perl), module loading
(e.g., option -m for Python), stdin, file sourcing, environment
variables, configuration files...  According to the threat model, it may
be acceptable to allow some script interpreters (e.g. Bash) to interpret
commands from stdin, may it be a TTY or a pipe, because it may not be
enough to (directly) perform syscalls.

A simple security policy implementation is available in a following
patch for Yama.

This is an updated subset of the patch initially written by Vincent
Strubel for CLIP OS:
https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
This patch has been used for more than 10 years with customized script
interpreters.  Some examples can be found here:
https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC

Signed-off-by: Mickaël Salaün <[e-mail]>
Signed-off-by: Thibaut Sautereau <[e-mail]>
Signed-off-by: Vincent Strubel <[e-mail]>
Reviewed-by: Philippe Trébuchet <[e-mail]>
Cc: Al Viro <[e-mail]>
Cc: Kees Cook <[e-mail]>
Cc: Mickaël Salaün <[e-mail]>
---
 fs/fcntl.c                       | 2 +-
 fs/open.c                        | 4 ++++
 include/linux/fcntl.h            | 2 +-
 include/linux/fs.h               | 2 ++
 include/uapi/asm-generic/fcntl.h | 3 +++
 5 files changed, 11 insertions(+), 2 deletions(-)

Comments

Jan Kara Dec. 12, 2018, 2:43 p.m. UTC | #1

On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
> When the O_MAYEXEC flag is passed, sys_open() may be subject to
> additional restrictions depending on a security policy implemented by an
> LSM through the inode_permission hook.
> 
> The underlying idea is to be able to restrict scripts interpretation
> according to a policy defined by the system administrator.  For this to
> be possible, script interpreters must use the O_MAYEXEC flag
> appropriately.  To be fully effective, these interpreters also need to
> handle the other ways to execute code (for which the kernel can't help):
> command line parameters (e.g., option -e for Perl), module loading
> (e.g., option -m for Python), stdin, file sourcing, environment
> variables, configuration files...  According to the threat model, it may
> be acceptable to allow some script interpreters (e.g. Bash) to interpret
> commands from stdin, may it be a TTY or a pipe, because it may not be
> enough to (directly) perform syscalls.
> 
> A simple security policy implementation is available in a following
> patch for Yama.
> 
> This is an updated subset of the patch initially written by Vincent
> Strubel for CLIP OS:
> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> This patch has been used for more than 10 years with customized script
> interpreters.  Some examples can be found here:
> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
> 
> Signed-off-by: Mickaël Salaün <[e-mail]>
> Signed-off-by: Thibaut Sautereau <[e-mail]>
> Signed-off-by: Vincent Strubel <[e-mail]>
> Reviewed-by: Philippe Trébuchet <[e-mail]>
> Cc: Al Viro <[e-mail]>
> Cc: Kees Cook <[e-mail]>
> Cc: Mickaël Salaün <[e-mail]>

...

> diff --git a/fs/open.c b/fs/open.c
> index 0285ce7dbd51..75479b79a58f 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
>  	if (flags & O_APPEND)
>  		acc_mode |= MAY_APPEND;
>  
> +	/* Check execution permissions on open. */
> +	if (flags & O_MAYEXEC)
> +		acc_mode |= MAY_OPENEXEC;
> +
>  	op->acc_mode = acc_mode;
>  
>  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;

I don't feel experienced enough in security to tell whether we want this
functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
on the resulting struct file? That way also security_file_open() can be
used to arbitrate such executable opens and in particular
fanotify permission event FAN_OPEN_EXEC will get properly generated which I
guess is desirable (support for it is sitting in my tree waiting for the
merge window) - adding some audit people involved in FAN_OPEN_EXEC to
CC. Just an idea...

								Honza

Mickaël Salaün Dec. 12, 2018, 5:09 p.m. UTC | #2

Le 12/12/2018 à 15:43, Jan Kara a écrit :
> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
>> When the O_MAYEXEC flag is passed, sys_open() may be subject to
>> additional restrictions depending on a security policy implemented by an
>> LSM through the inode_permission hook.
>>
>> The underlying idea is to be able to restrict scripts interpretation
>> according to a policy defined by the system administrator.  For this to
>> be possible, script interpreters must use the O_MAYEXEC flag
>> appropriately.  To be fully effective, these interpreters also need to
>> handle the other ways to execute code (for which the kernel can't help):
>> command line parameters (e.g., option -e for Perl), module loading
>> (e.g., option -m for Python), stdin, file sourcing, environment
>> variables, configuration files...  According to the threat model, it may
>> be acceptable to allow some script interpreters (e.g. Bash) to interpret
>> commands from stdin, may it be a TTY or a pipe, because it may not be
>> enough to (directly) perform syscalls.
>>
>> A simple security policy implementation is available in a following
>> patch for Yama.
>>
>> This is an updated subset of the patch initially written by Vincent
>> Strubel for CLIP OS:
>> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
>> This patch has been used for more than 10 years with customized script
>> interpreters.  Some examples can be found here:
>> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
>>
>> Signed-off-by: Mickaël Salaün <[e-mail]>
>> Signed-off-by: Thibaut Sautereau <[e-mail]>
>> Signed-off-by: Vincent Strubel <[e-mail]>
>> Reviewed-by: Philippe Trébuchet <[e-mail]>
>> Cc: Al Viro <[e-mail]>
>> Cc: Kees Cook <[e-mail]>
>> Cc: Mickaël Salaün <[e-mail]>
> 
> ...
> 
>> diff --git a/fs/open.c b/fs/open.c
>> index 0285ce7dbd51..75479b79a58f 100644
>> --- a/fs/open.c
>> +++ b/fs/open.c
>> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
>>  	if (flags & O_APPEND)
>>  		acc_mode |= MAY_APPEND;
>>  
>> +	/* Check execution permissions on open. */
>> +	if (flags & O_MAYEXEC)
>> +		acc_mode |= MAY_OPENEXEC;
>> +
>>  	op->acc_mode = acc_mode;
>>  
>>  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> 
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
> CC. Just an idea...

Indeed, it may be useful for other LSM.

 Mickaël

Mimi Zohar Dec. 12, 2018, 8:42 p.m. UTC | #3

On Wed, 2018-12-12 at 15:43 +0100, Jan Kara wrote:


> > diff --git a/fs/open.c b/fs/open.c
> > index 0285ce7dbd51..75479b79a58f 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
> >  	if (flags & O_APPEND)
> >  		acc_mode |= MAY_APPEND;
> >  
> > +	/* Check execution permissions on open. */
> > +	if (flags & O_MAYEXEC)
> > +		acc_mode |= MAY_OPENEXEC;
> > +
> >  	op->acc_mode = acc_mode;
> >  
> >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> 
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
> CC. Just an idea...

Assuming the interpreters are properly modified (and signed),
MAY_OPENEXEC closes a major IMA measurement/appraisal gap.  The kernel
has no insight into the files that the interpreter is opening.  Having
the interpreter annotate the file open, allows IMA to differentiate
scripts opening data files from code.

IMA policy rules could then be written requiring code to be signed.

Example IMA policy rules:
measure func=FILE_CHECK mask=MAY_OPENEXEC
appraise func=FILE_CHECK mask=MAY_OPENEXEC appraise_type=imasig

Mimi

Matthew Bobrowski Dec. 13, 2018, 9:47 a.m. UTC | #4

On Wed, Dec 12, 2018 at 03:43:06PM +0100, Jan Kara wrote:
> > When the O_MAYEXEC flag is passed, sys_open() may be subject to
> > additional restrictions depending on a security policy implemented by an
> > LSM through the inode_permission hook.
> > 
> > The underlying idea is to be able to restrict scripts interpretation
> > according to a policy defined by the system administrator.  For this to
> > be possible, script interpreters must use the O_MAYEXEC flag
> > appropriately.  To be fully effective, these interpreters also need to
> > handle the other ways to execute code (for which the kernel can't help):
> > command line parameters (e.g., option -e for Perl), module loading
> > (e.g., option -m for Python), stdin, file sourcing, environment
> > variables, configuration files...  According to the threat model, it may
> > be acceptable to allow some script interpreters (e.g. Bash) to interpret
> > commands from stdin, may it be a TTY or a pipe, because it may not be
> > enough to (directly) perform syscalls.
> > 
> > A simple security policy implementation is available in a following
> > patch for Yama.
> > 
> > This is an updated subset of the patch initially written by Vincent
> > Strubel for CLIP OS:
> > https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> > This patch has been used for more than 10 years with customized script
> > interpreters.  Some examples can be found here:
> > https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
> > 
> > Signed-off-by: Mickaël Salaün <[e-mail]>
> > Signed-off-by: Thibaut Sautereau <[e-mail]>
> > Signed-off-by: Vincent Strubel <[e-mail]>
> > Reviewed-by: Philippe Trébuchet <[e-mail]>
> > Cc: Al Viro <[e-mail]>
> > Cc: Kees Cook <[e-mail]>
> > Cc: Mickaël Salaün <[e-mail]>
> 
> ...
> 
> > diff --git a/fs/open.c b/fs/open.c
> > index 0285ce7dbd51..75479b79a58f 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
> >  	if (flags & O_APPEND)
> >  		acc_mode |= MAY_APPEND;
> >  
> > +	/* Check execution permissions on open. */
> > +	if (flags & O_MAYEXEC)
> > +		acc_mode |= MAY_OPENEXEC;
> > +
> >  	op->acc_mode = acc_mode;
> >  
> >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> 
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
> CC. Just an idea...

If I'm understanding this patch series correctly, without an enforced LSM
policy there's realistically no added benefit from a security perspective,
right? Also, I'm in agreement with what Jan has mentioned in regards to setting
the __FMODE_EXEC flag when O_MAYEXEC has been specified. This is something that
would work quite nicely in conjunction with some of the new file access
notification events.

Rather than setting it on the resulting struct file, couldn't they simply
incorporate it as part of op->open_flags when flags & O_MAYEXEC? Not entirely
sure whether this is what you actually meant or not though? Pretty much the
same as a call to exec(2)/execat(2) when it builds its open_flags.

Mickaël Salaün Dec. 13, 2018, 2:23 p.m. UTC | #5

On 13/12/2018 10:47, Matthew Bobrowski wrote:
> On Wed, Dec 12, 2018 at 03:43:06PM +0100, Jan Kara wrote:
>>> When the O_MAYEXEC flag is passed, sys_open() may be subject to
>>> additional restrictions depending on a security policy implemented by an
>>> LSM through the inode_permission hook.
>>>
>>> The underlying idea is to be able to restrict scripts interpretation
>>> according to a policy defined by the system administrator.  For this to
>>> be possible, script interpreters must use the O_MAYEXEC flag
>>> appropriately.  To be fully effective, these interpreters also need to
>>> handle the other ways to execute code (for which the kernel can't help):
>>> command line parameters (e.g., option -e for Perl), module loading
>>> (e.g., option -m for Python), stdin, file sourcing, environment
>>> variables, configuration files...  According to the threat model, it may
>>> be acceptable to allow some script interpreters (e.g. Bash) to interpret
>>> commands from stdin, may it be a TTY or a pipe, because it may not be
>>> enough to (directly) perform syscalls.
>>>
>>> A simple security policy implementation is available in a following
>>> patch for Yama.
>>>
>>> This is an updated subset of the patch initially written by Vincent
>>> Strubel for CLIP OS:
>>> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
>>> This patch has been used for more than 10 years with customized script
>>> interpreters.  Some examples can be found here:
>>> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
>>>
>>> Signed-off-by: Mickaël Salaün <[e-mail]>
>>> Signed-off-by: Thibaut Sautereau <[e-mail]>
>>> Signed-off-by: Vincent Strubel <[e-mail]>
>>> Reviewed-by: Philippe Trébuchet <[e-mail]>
>>> Cc: Al Viro <[e-mail]>
>>> Cc: Kees Cook <[e-mail]>
>>> Cc: Mickaël Salaün <[e-mail]>
>>
>> ...
>>
>>> diff --git a/fs/open.c b/fs/open.c
>>> index 0285ce7dbd51..75479b79a58f 100644
>>> --- a/fs/open.c
>>> +++ b/fs/open.c
>>> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
>>>  	if (flags & O_APPEND)
>>>  		acc_mode |= MAY_APPEND;
>>>  
>>> +	/* Check execution permissions on open. */
>>> +	if (flags & O_MAYEXEC)
>>> +		acc_mode |= MAY_OPENEXEC;
>>> +
>>>  	op->acc_mode = acc_mode;
>>>  
>>>  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>>
>> I don't feel experienced enough in security to tell whether we want this
>> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
>> on the resulting struct file? That way also security_file_open() can be
>> used to arbitrate such executable opens and in particular
>> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
>> guess is desirable (support for it is sitting in my tree waiting for the
>> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
>> CC. Just an idea...
> 
> If I'm understanding this patch series correctly, without an enforced LSM
> policy there's realistically no added benefit from a security perspective,
> right?

That's correct. The kernel knows the semantic but the enforcement is
delegated to an LSM and its policy.

> Also, I'm in agreement with what Jan has mentioned in regards to setting
> the __FMODE_EXEC flag when O_MAYEXEC has been specified. This is something that
> would work quite nicely in conjunction with some of the new file access
> notification events.

OK, I will add it in the next patch series (for the new FAN_OPEN_EXEC
support).

Steve Grubb April 15, 2019, 6:47 p.m. UTC | #6

Hello,

On Wednesday, December 12, 2018 9:43:06 AM EDT Jan Kara wrote:
> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
> > When the O_MAYEXEC flag is passed, sys_open() may be subject to
> > additional restrictions depending on a security policy implemented by an
> > LSM through the inode_permission hook.
> > 
> > The underlying idea is to be able to restrict scripts interpretation
> > according to a policy defined by the system administrator.  For this to
> > be possible, script interpreters must use the O_MAYEXEC flag
> > appropriately.  To be fully effective, these interpreters also need to
> > handle the other ways to execute code (for which the kernel can't help):
> > command line parameters (e.g., option -e for Perl), module loading
> > (e.g., option -m for Python), stdin, file sourcing, environment
> > variables, configuration files...  According to the threat model, it may
> > be acceptable to allow some script interpreters (e.g. Bash) to interpret
> > commands from stdin, may it be a TTY or a pipe, because it may not be
> > enough to (directly) perform syscalls.
> > 
> > A simple security policy implementation is available in a following
> > patch for Yama.
> > 
> > This is an updated subset of the patch initially written by Vincent
> > Strubel for CLIP OS:
> > https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d
> > 6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch This patch has
> > been used for more than 10 years with customized script interpreters. 
> > Some examples can be found here:
> > https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYE
> > XEC
> > 
> > Signed-off-by: Mickaël Salaün <[e-mail]>
> > Signed-off-by: Thibaut Sautereau <[e-mail]>
> > Signed-off-by: Vincent Strubel <[e-mail]>
> > Reviewed-by: Philippe Trébuchet <[e-mail]>
> > Cc: Al Viro <[e-mail]>
> > Cc: Kees Cook <[e-mail]>
> > Cc: Mickaël Salaün <[e-mail]>
> 
> ...
> 
> > diff --git a/fs/open.c b/fs/open.c
> > index 0285ce7dbd51..75479b79a58f 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags,
> > umode_t mode, struct open_flags *o> 
> >  	if (flags & O_APPEND)
> >  	
> >  		acc_mode |= MAY_APPEND;
> > 
> > +	/* Check execution permissions on open. */
> > +	if (flags & O_MAYEXEC)
> > +		acc_mode |= MAY_OPENEXEC;
> > +
> > 
> >  	op->acc_mode = acc_mode;
> >  	
> >  	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
> 
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to CC.
> Just an idea...

Late in replying. But I think it's important to have a deep look into the 
issue.

TL;DR - This is a gentle man's handshake. It won't _really_ solve the 
problem.

This flag that is being proposed means that you would have to patch all 
interpreters to use it. If you are sure that upstreams will accept that, why 
not just change the policy to interpreters shouldn't execute anything unless 
the execute bit is set? That is simpler and doesn't need a kernel change. And 
setting the execute bit is an auditable event.

The bottom line is that any interpreter has to become a security policy 
enforcement point whether by indicating it wants to execute by setting a flag 
or by refusing to use a file without execute bit set. But this just moves the 
problem to one that is harder to fix. Why in the world does any programming 
language allow programs to be loaded via stdin? 

It is possible to wget a program and pipe it into python which subsequently 
pulls down an ELF shared object and runs it all without touching disk via 
memfd_create (e.g. SnakeEater). This is all direct to memory execution. And 
direct to memory bypasses anti-virus, selinux, IMA, application whitelisting, 
and other integrity schemes.

So, to fix this problem, you really need to not allow any programs to load via 
stdin so that everything that executes has to touch disk. This way you can 
get a fanotify event and see the application and vote yes/no on allowing it. 
And this will be particularly harder with the memfd_create fix for the runc 
container breakout. Prior to that, there were very few uses of that system 
call. Now it may be very common which means finding malicious use just got 
harder to spot.

But assuming these problems got fixed, then we have yet another place to look. 
Many interpreters allow you to specify a command to run via arguments. Some 
have a small buffer and some allow lengthy programs to be entered as an 
argument. One strategy might be that an attacker can bootstrap a lengthier 
program across the network. Python for example allows loading modules across 
a network. All you need to put in the commandline is the override for the 
module loader and a couple modules to import. It then loads the modules 
remotely. Getting rid of this hole will likely lead to some unhappy people - 
meaning it can't be fixed.

And even if we get that fixed, we have one last hole to plug. Shells. One can 
simply start a shell and paste their program into the shell and then execute 
it. You can easily do this with bash or python or any language that has a 
REPL (read–eval–print loop). To fix this means divorcing the notion of a 
language from a REPL. Production systems really do not need a Python shell, 
they need the interpreter. I doubt that this would be popular. But fixing each 
of these issues is what it would take to prevent unknown software from 
running. Not going this far leaves holes.

Best Regards,
-Steve

Florian Weimer April 16, 2019, 11:49 a.m. UTC | #7

* Steve Grubb:

> This flag that is being proposed means that you would have to patch all 
> interpreters to use it. If you are sure that upstreams will accept that, why 
> not just change the policy to interpreters shouldn't execute anything unless 
> the execute bit is set? That is simpler and doesn't need a kernel change. And 
> setting the execute bit is an auditable event.

I think we need something like O_MAYEXEC so that security policies can
be enforced and noexec mounts can be detected.  I don't think it's a
good idea to do this in userspace, especially the latter.

Thanks,
Florian

Steve Grubb April 16, 2019, 3:34 p.m. UTC | #8

On Tuesday, April 16, 2019 7:49:39 AM EDT Florian Weimer wrote:
> * Steve Grubb:
> > This flag that is being proposed means that you would have to patch all
> > interpreters to use it. If you are sure that upstreams will accept that,
> > why not just change the policy to interpreters shouldn't execute
> > anything unless the execute bit is set? That is simpler and doesn't need
> > a kernel change. And setting the execute bit is an auditable event.
> 
> I think we need something like O_MAYEXEC so that security policies can
> be enforced and noexec mounts can be detected.

Application whitelisting can already today stop unknown software without 
needing O_MAYEXEC.

> I don't think it's a good idea to do this in userspace, especially the
> latter.

The problem is that passing O_MAYEXEC is opt-in. You can use ptrace/seccomp/
bpf/LD_PRELOAD/LD_AUDIT to remove that bit from an otherwise normal program. 
This does not require privs to do so.

But let's consider that this comes to pass and every interpreter is updated 
and IMA can see the O_MAYEXEC flag. Attackers now simply pivot to running 
programs via stdin. It never touches disk and therefore nothing enforces 
security policy. This already is among the most common ways that malware runs 
today to evade detection.

-Steve

Florian Weimer April 17, 2019, 10:01 a.m. UTC | #9

* Steve Grubb:

> On Tuesday, April 16, 2019 7:49:39 AM EDT Florian Weimer wrote:
>> * Steve Grubb:
>> > This flag that is being proposed means that you would have to patch all
>> > interpreters to use it. If you are sure that upstreams will accept that,
>> > why not just change the policy to interpreters shouldn't execute
>> > anything unless the execute bit is set? That is simpler and doesn't need
>> > a kernel change. And setting the execute bit is an auditable event.
>> 
>> I think we need something like O_MAYEXEC so that security policies can
>> be enforced and noexec mounts can be detected.
>
> Application whitelisting can already today stop unknown software without 
> needing O_MAYEXEC.

I'm somewhat interested in using this to add a proper check for
executability to explicit dynamic loader invocations.  In other words,
this

  /lib64/ld-linux-x86-64.so.2 /path/to/noexec/fs/program

should refuse to run the program if the program is located on a file
system mounted with the noexec attribute.

> The problem is that passing O_MAYEXEC is opt-in. You can use ptrace/seccomp/
> bpf/LD_PRELOAD/LD_AUDIT to remove that bit from an otherwise normal program. 
> This does not require privs to do so.

That doesn't really help with the above.

> But let's consider that this comes to pass and every interpreter is
> updated and IMA can see the O_MAYEXEC flag. Attackers now simply pivot
> to running programs via stdin. It never touches disk and therefore
> nothing enforces security policy. This already is among the most
> common ways that malware runs today to evade detection.

Are you referring to Windows malware using Powershell?

I'm not sure this is applicable to Linux.  We do not have much
behavioral monitoring anyway.

Thanks,
Florian

Mickaël Salaün April 17, 2019, 2:55 p.m. UTC | #10

On 15/04/2019 20:47, Steve Grubb wrote:
> Hello,
>
> On Wednesday, December 12, 2018 9:43:06 AM EDT Jan Kara wrote:
>> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
>>> When the O_MAYEXEC flag is passed, sys_open() may be subject to
>>> additional restrictions depending on a security policy implemented by an
>>> LSM through the inode_permission hook.
>>>
>>> The underlying idea is to be able to restrict scripts interpretation
>>> according to a policy defined by the system administrator.  For this to
>>> be possible, script interpreters must use the O_MAYEXEC flag
>>> appropriately.  To be fully effective, these interpreters also need to
>>> handle the other ways to execute code (for which the kernel can't help):
>>> command line parameters (e.g., option -e for Perl), module loading
>>> (e.g., option -m for Python), stdin, file sourcing, environment
>>> variables, configuration files...  According to the threat model, it may
>>> be acceptable to allow some script interpreters (e.g. Bash) to interpret
>>> commands from stdin, may it be a TTY or a pipe, because it may not be
>>> enough to (directly) perform syscalls.
>>>
>>> A simple security policy implementation is available in a following
>>> patch for Yama.
>>>
>>> This is an updated subset of the patch initially written by Vincent
>>> Strubel for CLIP OS:
>>> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d
>>> 6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch This patch has
>>> been used for more than 10 years with customized script interpreters.
>>> Some examples can be found here:
>>> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYE
>>> XEC
>>>
>>> Signed-off-by: Mickaël Salaün <[e-mail]>
>>> Signed-off-by: Thibaut Sautereau <[e-mail]>
>>> Signed-off-by: Vincent Strubel <[e-mail]>
>>> Reviewed-by: Philippe Trébuchet <[e-mail]>
>>> Cc: Al Viro <[e-mail]>
>>> Cc: Kees Cook <[e-mail]>
>>> Cc: Mickaël Salaün <[e-mail]>
>>
>> ...
>>
>>> diff --git a/fs/open.c b/fs/open.c
>>> index 0285ce7dbd51..75479b79a58f 100644
>>> --- a/fs/open.c
>>> +++ b/fs/open.c
>>> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags,
>>> umode_t mode, struct open_flags *o>
>>>     if (flags & O_APPEND)
>>>
>>>             acc_mode |= MAY_APPEND;
>>>
>>> +   /* Check execution permissions on open. */
>>> +   if (flags & O_MAYEXEC)
>>> +           acc_mode |= MAY_OPENEXEC;
>>> +
>>>
>>>     op->acc_mode = acc_mode;
>>>
>>>     op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>>
>> I don't feel experienced enough in security to tell whether we want this
>> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
>> on the resulting struct file? That way also security_file_open() can be
>> used to arbitrate such executable opens and in particular
>> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
>> guess is desirable (support for it is sitting in my tree waiting for the
>> merge window) - adding some audit people involved in FAN_OPEN_EXEC to CC.
>> Just an idea...
>
> Late in replying. But I think it's important to have a deep look into the
> issue.
>
> TL;DR - This is a gentle man's handshake. It won't _really_ solve the
> problem.

Thanks for your comments. You should find most answers in this thread:
https://lore.kernel.org/lkml/[e-mail]/

The threat model targets persistent attacks. This O_MAYEXEC flag is not
a silver bullet but it's a needed block to enforce a security policy on
a trusted system. This means that every component executable on the
system must be controlled, which means they may need some bit of
customization. Today no userspace application use this flag (except in
CLIP OS), but we need to first create a feature before it can be used.

It is very important to have in mind that a system security policy need
to have a (central) security manager, in this case the kernel thanks to
Yama's policy (but it could be SELinux, IMA or any other LSM). The goal
is not to give to the developer the job of defining a security policy
for the *system*; this job is for the system administrator (or the distro).

>
> This flag that is being proposed means that you would have to patch all
> interpreters to use it. If you are sure that upstreams will accept that, why
> not just change the policy to interpreters shouldn't execute anything unless
> the execute bit is set? That is simpler and doesn't need a kernel change. And
> setting the execute bit is an auditable event.

As said above, the definition of a the security policy is the job of the
system administrator. Moreover, the security policy may be defined by
the mount point restrictions (i.e. noexec) but it should be definable
with something else (e.g. a SELinux or IMA policy which may be agnostic
to the mount points).

>
> The bottom line is that any interpreter has to become a security policy
> enforcement point whether by indicating it wants to execute by setting a flag
> or by refusing to use a file without execute bit set. But this just moves the
> problem to one that is harder to fix. Why in the world does any programming
> language allow programs to be loaded via stdin?
>
> It is possible to wget a program and pipe it into python which subsequently
> pulls down an ELF shared object and runs it all without touching disk via
> memfd_create (e.g. SnakeEater). This is all direct to memory execution. And
> direct to memory bypasses anti-virus, selinux, IMA, application whitelisting,
> and other integrity schemes.
>
> So, to fix this problem, you really need to not allow any programs to load via
> stdin so that everything that executes has to touch disk. This way you can
> get a fanotify event and see the application and vote yes/no on allowing it.
> And this will be particularly harder with the memfd_create fix for the runc
> container breakout. Prior to that, there were very few uses of that system
> call. Now it may be very common which means finding malicious use just got
> harder to spot.

As said above, stdin must be restricted in some way. You may want to
take a look at the CLIP OS patches (which doesn't only add the O_MAYEXEC
flag but restrict other way to interpret code). It may be foolish to
block or restrict stdin for interpreters on a developer workstation, but
it may make sense for an embedded custom system.

The same apply for memfd_create. If you want to enforce a security
policy on this kind of *file descriptor*, you should ask to the proper
LSM to do so. The current Yama patch deal with this kind of FD if they
are accessed through /proc/*/fd because the procfs is mounted with
noexec. Anyway, the interpreter must *inform* the LSM that it wants to
execute/interpret something from this FD, which is done thanks to the
O_MAYEXEC flag.

>
> But assuming these problems got fixed, then we have yet another place to look.
> Many interpreters allow you to specify a command to run via arguments. Some
> have a small buffer and some allow lengthy programs to be entered as an
> argument. One strategy might be that an attacker can bootstrap a lengthier
> program across the network. Python for example allows loading modules across
> a network. All you need to put in the commandline is the override for the
> module loader and a couple modules to import. It then loads the modules
> remotely. Getting rid of this hole will likely lead to some unhappy people -
> meaning it can't be fixed.

Again, this depend on the threat model and the corresponding product. If
you want to handle everything on your system, then you may need some
adjustments.

>
> And even if we get that fixed, we have one last hole to plug. Shells. One can
> simply start a shell and paste their program into the shell and then execute
> it. You can easily do this with bash or python or any language that has a
> REPL (read–eval–print loop). To fix this means divorcing the notion of a
> language from a REPL. Production systems really do not need a Python shell,
> they need the interpreter. I doubt that this would be popular. But fixing each
> of these issues is what it would take to prevent unknown software from
> running. Not going this far leaves holes.

This is also covered by the threat model defined in the patch 3/5 (i.e.
protect the kernel by restricting arbitrary syscalls).

Regards,

--
Mickaël Salaün
ANSSI/SDE/ST/LAM

Les données à caractère personnel recueillies et traitées dans le cadre de cet échange, le sont à seule fin d’exécution d’une relation professionnelle et s’opèrent dans cette seule finalité et pour la durée nécessaire à cette relation. Si vous souhaitez faire usage de vos droits de consultation, de rectification et de suppression de vos données, veuillez contacter [e-mail]. Si vous avez reçu ce message par erreur, nous vous remercions d’en informer l’expéditeur et de détruire le message. The personal data collected and processed during this exchange aims solely at completing a business relationship and is limited to the necessary duration of that relationship. If you wish to use your rights of consultation, rectification and deletion of your data, please contact: [e-mail]. If you have received this message in error, we thank you for informing the sender and destroying the message.

Mickaël Salaün April 17, 2019, 3:04 p.m. UTC | #11

On 17/04/2019 12:01, Florian Weimer wrote:
> * Steve Grubb:
>
>> On Tuesday, April 16, 2019 7:49:39 AM EDT Florian Weimer wrote:
>>> * Steve Grubb:
>>>> This flag that is being proposed means that you would have to patch all
>>>> interpreters to use it. If you are sure that upstreams will accept that,
>>>> why not just change the policy to interpreters shouldn't execute
>>>> anything unless the execute bit is set? That is simpler and doesn't need
>>>> a kernel change. And setting the execute bit is an auditable event.
>>>
>>> I think we need something like O_MAYEXEC so that security policies can
>>> be enforced and noexec mounts can be detected.
>>
>> Application whitelisting can already today stop unknown software without
>> needing O_MAYEXEC.

Whitelisting may be a lot of thing (path/TPE, signed binaries…), but
being able to handle this with a global system configuration (instead of
app-specific hardcoded configuration) is a good idea. ;)

>
> I'm somewhat interested in using this to add a proper check for
> executability to explicit dynamic loader invocations.  In other words,
> this
>
>   /lib64/ld-linux-x86-64.so.2 /path/to/noexec/fs/program
>
> should refuse to run the program if the program is located on a file
> system mounted with the noexec attribute.

What if a sysadmin need to do this on an executable mount point? Being
able to enforce a security policy according to a configuration may fit
to much more use cases.

>
>> The problem is that passing O_MAYEXEC is opt-in. You can use ptrace/seccomp/
>> bpf/LD_PRELOAD/LD_AUDIT to remove that bit from an otherwise normal program.
>> This does not require privs to do so.
>
> That doesn't really help with the above.

Right, ptrace/LD_PRELOAD and so on must be addressed by something else
than only O_MAYEXEC.

>
>> But let's consider that this comes to pass and every interpreter is
>> updated and IMA can see the O_MAYEXEC flag. Attackers now simply pivot
>> to running programs via stdin. It never touches disk and therefore
>> nothing enforces security policy. This already is among the most
>> common ways that malware runs today to evade detection.

As my previous reply, use cases like stdin may be restricted as well.

>
> Are you referring to Windows malware using Powershell?
>
> I'm not sure this is applicable to Linux.  We do not have much
> behavioral monitoring anyway.
>
> Thanks,
> Florian
>

--
Mickaël Salaün
ANSSI/SDE/ST/LAM

Les données à caractère personnel recueillies et traitées dans le cadre de cet échange, le sont à seule fin d’exécution d’une relation professionnelle et s’opèrent dans cette seule finalité et pour la durée nécessaire à cette relation. Si vous souhaitez faire usage de vos droits de consultation, de rectification et de suppression de vos données, veuillez contacter [e-mail]. Si vous avez reçu ce message par erreur, nous vous remercions d’en informer l’expéditeur et de détruire le message. The personal data collected and processed during this exchange aims solely at completing a business relationship and is limited to the necessary duration of that relationship. If you wish to use your rights of consultation, rectification and deletion of your data, please contact: [e-mail]. If you have received this message in error, we thank you for informing the sender and destroying the message.

Andy Lutomirski Aug. 4, 2019, 11:55 p.m. UTC | #12

On Wed, Dec 12, 2018 at 6:43 AM Jan Kara <[e-mail]> wrote:
>
> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
> > When the O_MAYEXEC flag is passed, sys_open() may be subject to
> > additional restrictions depending on a security policy implemented by an
> > LSM through the inode_permission hook.
> >
> > The underlying idea is to be able to restrict scripts interpretation
> > according to a policy defined by the system administrator.  For this to
> > be possible, script interpreters must use the O_MAYEXEC flag
> > appropriately.  To be fully effective, these interpreters also need to
> > handle the other ways to execute code (for which the kernel can't help):
> > command line parameters (e.g., option -e for Perl), module loading
> > (e.g., option -m for Python), stdin, file sourcing, environment
> > variables, configuration files...  According to the threat model, it may
> > be acceptable to allow some script interpreters (e.g. Bash) to interpret
> > commands from stdin, may it be a TTY or a pipe, because it may not be
> > enough to (directly) perform syscalls.
> >
> > A simple security policy implementation is available in a following
> > patch for Yama.
> >
> > This is an updated subset of the patch initially written by Vincent
> > Strubel for CLIP OS:
> > https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> > This patch has been used for more than 10 years with customized script
> > interpreters.  Some examples can be found here:
> > https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
> >
> > Signed-off-by: Mickaël Salaün <[e-mail]>
> > Signed-off-by: Thibaut Sautereau <[e-mail]>
> > Signed-off-by: Vincent Strubel <[e-mail]>
> > Reviewed-by: Philippe Trébuchet <[e-mail]>
> > Cc: Al Viro <[e-mail]>
> > Cc: Kees Cook <[e-mail]>
> > Cc: Mickaël Salaün <[e-mail]>
>
> ...
>
> > diff --git a/fs/open.c b/fs/open.c
> > index 0285ce7dbd51..75479b79a58f 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
> >       if (flags & O_APPEND)
> >               acc_mode |= MAY_APPEND;
> >
> > +     /* Check execution permissions on open. */
> > +     if (flags & O_MAYEXEC)
> > +             acc_mode |= MAY_OPENEXEC;
> > +
> >       op->acc_mode = acc_mode;
> >
> >       op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>
> I don't feel experienced enough in security to tell whether we want this
> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
> on the resulting struct file? That way also security_file_open() can be
> used to arbitrate such executable opens and in particular
> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
> guess is desirable (support for it is sitting in my tree waiting for the
> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
> CC. Just an idea...
>

I would really like to land this patch.  I'm fiddling with making
bpffs handle permissions intelligently, and the lack of a way to say
"hey, I want to open this bpf program so that I can run it" is
annoying.

--Andy

Mickaël Salaün Aug. 6, 2019, 4:40 p.m. UTC | #13

On 05/08/2019 01:55, Andy Lutomirski wrote:
> On Wed, Dec 12, 2018 at 6:43 AM Jan Kara <[e-mail]> wrote:
>>
>> On Wed 12-12-18 09:17:08, Mickaël Salaün wrote:
>>> When the O_MAYEXEC flag is passed, sys_open() may be subject to
>>> additional restrictions depending on a security policy implemented by an
>>> LSM through the inode_permission hook.
>>>
>>> The underlying idea is to be able to restrict scripts interpretation
>>> according to a policy defined by the system administrator.  For this to
>>> be possible, script interpreters must use the O_MAYEXEC flag
>>> appropriately.  To be fully effective, these interpreters also need to
>>> handle the other ways to execute code (for which the kernel can't help):
>>> command line parameters (e.g., option -e for Perl), module loading
>>> (e.g., option -m for Python), stdin, file sourcing, environment
>>> variables, configuration files...  According to the threat model, it may
>>> be acceptable to allow some script interpreters (e.g. Bash) to interpret
>>> commands from stdin, may it be a TTY or a pipe, because it may not be
>>> enough to (directly) perform syscalls.
>>>
>>> A simple security policy implementation is available in a following
>>> patch for Yama.
>>>
>>> This is an updated subset of the patch initially written by Vincent
>>> Strubel for CLIP OS:
>>> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
>>> This patch has been used for more than 10 years with customized script
>>> interpreters.  Some examples can be found here:
>>> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
>>>
>>> Signed-off-by: Mickaël Salaün <[e-mail]>
>>> Signed-off-by: Thibaut Sautereau <[e-mail]>
>>> Signed-off-by: Vincent Strubel <[e-mail]>
>>> Reviewed-by: Philippe Trébuchet <[e-mail]>
>>> Cc: Al Viro <[e-mail]>
>>> Cc: Kees Cook <[e-mail]>
>>> Cc: Mickaël Salaün <[e-mail]>
>>
>> ...
>>
>>> diff --git a/fs/open.c b/fs/open.c
>>> index 0285ce7dbd51..75479b79a58f 100644
>>> --- a/fs/open.c
>>> +++ b/fs/open.c
>>> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
>>>       if (flags & O_APPEND)
>>>               acc_mode |= MAY_APPEND;
>>>
>>> +     /* Check execution permissions on open. */
>>> +     if (flags & O_MAYEXEC)
>>> +             acc_mode |= MAY_OPENEXEC;
>>> +
>>>       op->acc_mode = acc_mode;
>>>
>>>       op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>>
>> I don't feel experienced enough in security to tell whether we want this
>> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
>> on the resulting struct file? That way also security_file_open() can be
>> used to arbitrate such executable opens and in particular
>> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
>> guess is desirable (support for it is sitting in my tree waiting for the
>> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
>> CC. Just an idea...
>>
> 
> I would really like to land this patch.  I'm fiddling with making
> bpffs handle permissions intelligently, and the lack of a way to say
> "hey, I want to open this bpf program so that I can run it" is
> annoying.

Are you OK with this series? What about Aleksa's work on openat2(), and
Sean's work on SGX/noexec? Is it time to send a new patch series (with a
dedicated LSM instead of Yama)?

Patch

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 083185174c6d..6c85c4d0c006 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -1031,7 +1031,7 @@  static int __init fcntl_init(void)
 	 * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
 	 * is defined as O_NONBLOCK on some platforms and not on others.
 	 */
-	BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ !=
+	BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ !=
 		HWEIGHT32(
 			(VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
 			__FMODE_EXEC | __FMODE_NONOTIFY));
diff --git a/fs/open.c b/fs/open.c
index 0285ce7dbd51..75479b79a58f 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -974,6 +974,10 @@  static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
 	if (flags & O_APPEND)
 		acc_mode |= MAY_APPEND;
 
+	/* Check execution permissions on open. */
+	if (flags & O_MAYEXEC)
+		acc_mode |= MAY_OPENEXEC;
+
 	op->acc_mode = acc_mode;
 
 	op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index 27dc7a60693e..1fc00cabe9ab 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -9,7 +9,7 @@ 
 	(O_RDONLY | O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC | \
 	 O_APPEND | O_NDELAY | O_NONBLOCK | O_NDELAY | __O_SYNC | O_DSYNC | \
 	 FASYNC	| O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
-	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
+	 O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE | O_MAYEXEC)
 
 #ifndef force_o_largefile
 #define force_o_largefile() (BITS_PER_LONG != 32)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c95c0807471f..584c9329ad78 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -94,6 +94,8 @@  typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 #define MAY_CHDIR		0x00000040
 /* called from RCU mode, don't block */
 #define MAY_NOT_BLOCK		0x00000080
+/* the inode is opened with O_MAYEXEC */
+#define MAY_OPENEXEC		0x00000100
 
 /*
  * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 9dc0bf0c5a6e..cbb9425d6e7c 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -97,6 +97,9 @@ 
 #define O_NDELAY	O_NONBLOCK
 #endif
 
+/* command execution from file is intended, check exec permissions */
+#define O_MAYEXEC	040000000
+
 #define F_DUPFD		0	/* dup */
 #define F_GETFD		1	/* get close_on_exec */
 #define F_SETFD		2	/* set/clear close_on_exec */