* [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications
@ 2014-05-14 15:58 Paul Moore
2014-05-14 16:16 ` Andy Lutomirski
2014-05-14 16:28 ` Stephen Smalley
0 siblings, 2 replies; 9+ messages in thread
From: Paul Moore @ 2014-05-14 15:58 UTC (permalink / raw)
To: selinux; +Cc: sds, luto
We presently prevent processes from explicitly setting an arbitrary
security label on new processes when NO_NEW_PRIVS is enabled; in an
attempt for more consistency, this patch extends this to prevent
setting an arbitrary label when the new application lives on a
filesystem mounted with MNT_NOSUID.
Signed-off-by: Paul Moore <pmoore@redhat.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Stephen Smalley <sds@tycho.nsa.gov>
---
security/selinux/hooks.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 57b0b49..6fafe86 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2106,11 +2106,13 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
new_tsec->exec_sid = 0;
/*
- * Minimize confusion: if no_new_privs and a transition is
- * explicitly requested, then fail the exec.
+ * Minimize confusion: if no_new_privs or nosuid and a
+ * transition is explicitly requested, then fail the exec.
*/
if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)
return -EPERM;
+ if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
+ return -EACCES;
} else {
/* Check for a default transition on this program. */
rc = security_transition_sid(old_tsec->sid, isec->sid,
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications
2014-05-14 15:58 [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications Paul Moore
@ 2014-05-14 16:16 ` Andy Lutomirski
2014-05-14 16:18 ` Andy Lutomirski
2014-05-14 16:23 ` Stephen Smalley
2014-05-14 16:28 ` Stephen Smalley
1 sibling, 2 replies; 9+ messages in thread
From: Andy Lutomirski @ 2014-05-14 16:16 UTC (permalink / raw)
To: Paul Moore; +Cc: Stephen Smalley, selinux
On Wed, May 14, 2014 at 8:58 AM, Paul Moore <pmoore@redhat.com> wrote:
> We presently prevent processes from explicitly setting an arbitrary
> security label on new processes when NO_NEW_PRIVS is enabled; in an
> attempt for more consistency, this patch extends this to prevent
> setting an arbitrary label when the new application lives on a
> filesystem mounted with MNT_NOSUID.
>
> Signed-off-by: Paul Moore <pmoore@redhat.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Stephen Smalley <sds@tycho.nsa.gov>
> ---
Acked-by: Andy Lutomirski <luto@amacapital.net>
However: would it pay to move the check above this:
/* Reset exec SID on execve. */
new_tsec->exec_sid = 0;
I suppose that this shouldn't matter: any correct application already
needs to redo setexeccon if it gets an error from execve. Fixing this
for real would probably involve moving that line of code into
selinux_bprm_committed_creds.
--Andy
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications
2014-05-14 16:16 ` Andy Lutomirski
@ 2014-05-14 16:18 ` Andy Lutomirski
2014-05-14 16:23 ` Stephen Smalley
1 sibling, 0 replies; 9+ messages in thread
From: Andy Lutomirski @ 2014-05-14 16:18 UTC (permalink / raw)
To: Paul Moore; +Cc: Stephen Smalley, selinux
On Wed, May 14, 2014 at 9:16 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, May 14, 2014 at 8:58 AM, Paul Moore <pmoore@redhat.com> wrote:
>> We presently prevent processes from explicitly setting an arbitrary
>> security label on new processes when NO_NEW_PRIVS is enabled; in an
>> attempt for more consistency, this patch extends this to prevent
>> setting an arbitrary label when the new application lives on a
>> filesystem mounted with MNT_NOSUID.
>>
>> Signed-off-by: Paul Moore <pmoore@redhat.com>
>> CC: Andy Lutomirski <luto@amacapital.net>
>> CC: Stephen Smalley <sds@tycho.nsa.gov>
>> ---
>
> Acked-by: Andy Lutomirski <luto@amacapital.net>
>
> However: would it pay to move the check above this:
>
> /* Reset exec SID on execve. */
> new_tsec->exec_sid = 0;
>
> I suppose that this shouldn't matter: any correct application already
> needs to redo setexeccon if it gets an error from execve. Fixing this
> for real would probably involve moving that line of code into
> selinux_bprm_committed_creds.
I'm also unconvinced by the subject line -- would "selinux: return an
error when rejecting settexeccon on MNT_NOSUID applications" be
better?
--Andy
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications
2014-05-14 16:16 ` Andy Lutomirski
2014-05-14 16:18 ` Andy Lutomirski
@ 2014-05-14 16:23 ` Stephen Smalley
2014-05-14 16:48 ` Andy Lutomirski
1 sibling, 1 reply; 9+ messages in thread
From: Stephen Smalley @ 2014-05-14 16:23 UTC (permalink / raw)
To: Andy Lutomirski, Paul Moore; +Cc: selinux
On 05/14/2014 12:16 PM, Andy Lutomirski wrote:
> On Wed, May 14, 2014 at 8:58 AM, Paul Moore <pmoore@redhat.com> wrote:
>> We presently prevent processes from explicitly setting an arbitrary
>> security label on new processes when NO_NEW_PRIVS is enabled; in an
>> attempt for more consistency, this patch extends this to prevent
>> setting an arbitrary label when the new application lives on a
>> filesystem mounted with MNT_NOSUID.
>>
>> Signed-off-by: Paul Moore <pmoore@redhat.com>
>> CC: Andy Lutomirski <luto@amacapital.net>
>> CC: Stephen Smalley <sds@tycho.nsa.gov>
>> ---
>
> Acked-by: Andy Lutomirski <luto@amacapital.net>
>
> However: would it pay to move the check above this:
>
> /* Reset exec SID on execve. */
> new_tsec->exec_sid = 0;
>
> I suppose that this shouldn't matter: any correct application already
> needs to redo setexeccon if it gets an error from execve. Fixing this
> for real would probably involve moving that line of code into
> selinux_bprm_committed_creds.
Shouldn't matter as that is a change to new_tsec (i.e.
bprm->cred->security), which will only be applied to the process if we
reach install_exec_creds(); otherwise it will just be discarded.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications
2014-05-14 15:58 [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications Paul Moore
2014-05-14 16:16 ` Andy Lutomirski
@ 2014-05-14 16:28 ` Stephen Smalley
2014-05-14 21:00 ` Paul Moore
1 sibling, 1 reply; 9+ messages in thread
From: Stephen Smalley @ 2014-05-14 16:28 UTC (permalink / raw)
To: Paul Moore, selinux; +Cc: luto
On 05/14/2014 11:58 AM, Paul Moore wrote:
> We presently prevent processes from explicitly setting an arbitrary
> security label on new processes when NO_NEW_PRIVS is enabled; in an
> attempt for more consistency, this patch extends this to prevent
> setting an arbitrary label when the new application lives on a
> filesystem mounted with MNT_NOSUID.
It is never arbitrary (the new value is always controlled by policy),
and it isn't set on "new processes" per se but rather transitioned to
upon an exec. Anyway, the point of the change is to return an error
rather than silently ignore any /proc/self/attr/exec value when
executing from a nosuid mount.
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
>
> Signed-off-by: Paul Moore <pmoore@redhat.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> security/selinux/hooks.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 57b0b49..6fafe86 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2106,11 +2106,13 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
> new_tsec->exec_sid = 0;
>
> /*
> - * Minimize confusion: if no_new_privs and a transition is
> - * explicitly requested, then fail the exec.
> + * Minimize confusion: if no_new_privs or nosuid and a
> + * transition is explicitly requested, then fail the exec.
> */
> if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)
> return -EPERM;
> + if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
> + return -EACCES;
> } else {
> /* Check for a default transition on this program. */
> rc = security_transition_sid(old_tsec->sid, isec->sid,
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications
2014-05-14 16:23 ` Stephen Smalley
@ 2014-05-14 16:48 ` Andy Lutomirski
0 siblings, 0 replies; 9+ messages in thread
From: Andy Lutomirski @ 2014-05-14 16:48 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux
On Wed, May 14, 2014 at 9:23 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 05/14/2014 12:16 PM, Andy Lutomirski wrote:
>> On Wed, May 14, 2014 at 8:58 AM, Paul Moore <pmoore@redhat.com> wrote:
>>> We presently prevent processes from explicitly setting an arbitrary
>>> security label on new processes when NO_NEW_PRIVS is enabled; in an
>>> attempt for more consistency, this patch extends this to prevent
>>> setting an arbitrary label when the new application lives on a
>>> filesystem mounted with MNT_NOSUID.
>>>
>>> Signed-off-by: Paul Moore <pmoore@redhat.com>
>>> CC: Andy Lutomirski <luto@amacapital.net>
>>> CC: Stephen Smalley <sds@tycho.nsa.gov>
>>> ---
>>
>> Acked-by: Andy Lutomirski <luto@amacapital.net>
>>
>> However: would it pay to move the check above this:
>>
>> /* Reset exec SID on execve. */
>> new_tsec->exec_sid = 0;
>>
>> I suppose that this shouldn't matter: any correct application already
>> needs to redo setexeccon if it gets an error from execve. Fixing this
>> for real would probably involve moving that line of code into
>> selinux_bprm_committed_creds.
>
> Shouldn't matter as that is a change to new_tsec (i.e.
> bprm->cred->security), which will only be applied to the process if we
> reach install_exec_creds(); otherwise it will just be discarded.
>
>
Indeed. Sorry for the early-morning failure to read the code.
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications
2014-05-14 16:28 ` Stephen Smalley
@ 2014-05-14 21:00 ` Paul Moore
2014-05-14 21:26 ` Andy Lutomirski
0 siblings, 1 reply; 9+ messages in thread
From: Paul Moore @ 2014-05-14 21:00 UTC (permalink / raw)
To: Stephen Smalley, luto; +Cc: selinux
On Wednesday, May 14, 2014 09:18:35 AM Andy Lutomirski wrote:
> > On Wed, May 14, 2014 at 8:58 AM, Paul Moore <pmoore@redhat.com> wrote:
> >> We presently prevent processes from explicitly setting an arbitrary
> >> security label on new processes when NO_NEW_PRIVS is enabled; in an
> >> attempt for more consistency, this patch extends this to prevent
> >> setting an arbitrary label when the new application lives on a
> >> filesystem mounted with MNT_NOSUID.
> >>
> >> Signed-off-by: Paul Moore <pmoore@redhat.com>
> >> CC: Andy Lutomirski <luto@amacapital.net>
> >> CC: Stephen Smalley <sds@tycho.nsa.gov>
> >> ---
> >
> Acked-by: Andy Lutomirski <luto@amacapital.net>
>
> I'm also unconvinced by the subject line -- would "selinux: return an
> error when rejecting settexeccon on MNT_NOSUID applications" be
> better?
...
On Wednesday, May 14, 2014 12:28:24 PM Stephen Smalley wrote:
> On 05/14/2014 11:58 AM, Paul Moore wrote:
> > We presently prevent processes from explicitly setting an arbitrary
> > security label on new processes when NO_NEW_PRIVS is enabled; in an
> > attempt for more consistency, this patch extends this to prevent
> > setting an arbitrary label when the new application lives on a
> > filesystem mounted with MNT_NOSUID.
>
> It is never arbitrary (the new value is always controlled by policy),
> and it isn't set on "new processes" per se but rather transitioned to
> upon an exec. Anyway, the point of the change is to return an error
> rather than silently ignore any /proc/self/attr/exec value when
> executing from a nosuid mount.
>
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
I had a feeling while writing the patch description yesterday that someone was
not going to be happy with the text ... does the subject/description below
sound better to you guys?
***
selinux: reject setexeccon() on MNT_NOSUID applications with -EACCES
We presently prevent processes from using setexecon() to set the security
label of exec()'d processes when NO_NEW_PRIVS is enabled by returning an
error; however, we silently ignore setexeccon() when exec()'ing from a nosuid
mounted filesystem. This patch makes things a bit more consistent by
returning an error in the setexeccon()/nosuid case.
***
> > Signed-off-by: Paul Moore <pmoore@redhat.com>
> > CC: Andy Lutomirski <luto@amacapital.net>
> > CC: Stephen Smalley <sds@tycho.nsa.gov>
> > ---
> >
> > security/selinux/hooks.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 57b0b49..6fafe86 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2106,11 +2106,13 @@ static int selinux_bprm_set_creds(struct
> > linux_binprm *bprm)>
> > new_tsec->exec_sid = 0;
> >
> > /*
> >
> > - * Minimize confusion: if no_new_privs and a transition is
> > - * explicitly requested, then fail the exec.
> > + * Minimize confusion: if no_new_privs or nosuid and a
> > + * transition is explicitly requested, then fail the exec.
> >
> > */
> >
> > if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)
> >
> > return -EPERM;
> >
> > + if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
> > + return -EACCES;
> >
> > } else {
> >
> > /* Check for a default transition on this program. */
> > rc = security_transition_sid(old_tsec->sid, isec->sid,
--
paul moore
security and virtualization @ redhat
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications
2014-05-14 21:00 ` Paul Moore
@ 2014-05-14 21:26 ` Andy Lutomirski
2014-05-15 15:14 ` Paul Moore
0 siblings, 1 reply; 9+ messages in thread
From: Andy Lutomirski @ 2014-05-14 21:26 UTC (permalink / raw)
To: Paul Moore; +Cc: Stephen Smalley, selinux
On Wed, May 14, 2014 at 2:00 PM, Paul Moore <pmoore@redhat.com> wrote:
> On Wednesday, May 14, 2014 09:18:35 AM Andy Lutomirski wrote:
>> > On Wed, May 14, 2014 at 8:58 AM, Paul Moore <pmoore@redhat.com> wrote:
>> >> We presently prevent processes from explicitly setting an arbitrary
>> >> security label on new processes when NO_NEW_PRIVS is enabled; in an
>> >> attempt for more consistency, this patch extends this to prevent
>> >> setting an arbitrary label when the new application lives on a
>> >> filesystem mounted with MNT_NOSUID.
>> >>
>> >> Signed-off-by: Paul Moore <pmoore@redhat.com>
>> >> CC: Andy Lutomirski <luto@amacapital.net>
>> >> CC: Stephen Smalley <sds@tycho.nsa.gov>
>> >> ---
>> >
>> Acked-by: Andy Lutomirski <luto@amacapital.net>
>>
>> I'm also unconvinced by the subject line -- would "selinux: return an
>> error when rejecting settexeccon on MNT_NOSUID applications" be
>> better?
>
> ...
>
> On Wednesday, May 14, 2014 12:28:24 PM Stephen Smalley wrote:
>> On 05/14/2014 11:58 AM, Paul Moore wrote:
>> > We presently prevent processes from explicitly setting an arbitrary
>> > security label on new processes when NO_NEW_PRIVS is enabled; in an
>> > attempt for more consistency, this patch extends this to prevent
>> > setting an arbitrary label when the new application lives on a
>> > filesystem mounted with MNT_NOSUID.
>>
>> It is never arbitrary (the new value is always controlled by policy),
>> and it isn't set on "new processes" per se but rather transitioned to
>> upon an exec. Anyway, the point of the change is to return an error
>> rather than silently ignore any /proc/self/attr/exec value when
>> executing from a nosuid mount.
>>
>> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
>
> I had a feeling while writing the patch description yesterday that someone was
> not going to be happy with the text ... does the subject/description below
> sound better to you guys?
>
> ***
> selinux: reject setexeccon() on MNT_NOSUID applications with -EACCES
>
> We presently prevent processes from using setexecon() to set the security
> label of exec()'d processes when NO_NEW_PRIVS is enabled by returning an
> error; however, we silently ignore setexeccon() when exec()'ing from a nosuid
> mounted filesystem. This patch makes things a bit more consistent by
> returning an error in the setexeccon()/nosuid case.
> ***
Looks good to me.
--Andy
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications
2014-05-14 21:26 ` Andy Lutomirski
@ 2014-05-15 15:14 ` Paul Moore
0 siblings, 0 replies; 9+ messages in thread
From: Paul Moore @ 2014-05-15 15:14 UTC (permalink / raw)
To: Andy Lutomirski; +Cc: Stephen Smalley, selinux
On Wednesday, May 14, 2014 02:26:59 PM Andy Lutomirski wrote:
> On Wed, May 14, 2014 at 2:00 PM, Paul Moore <pmoore@redhat.com> wrote:
> > On Wednesday, May 14, 2014 09:18:35 AM Andy Lutomirski wrote:
> >> > On Wed, May 14, 2014 at 8:58 AM, Paul Moore <pmoore@redhat.com> wrote:
> >> >> We presently prevent processes from explicitly setting an arbitrary
> >> >> security label on new processes when NO_NEW_PRIVS is enabled; in an
> >> >> attempt for more consistency, this patch extends this to prevent
> >> >> setting an arbitrary label when the new application lives on a
> >> >> filesystem mounted with MNT_NOSUID.
> >> >>
> >> >> Signed-off-by: Paul Moore <pmoore@redhat.com>
> >> >> CC: Andy Lutomirski <luto@amacapital.net>
> >> >> CC: Stephen Smalley <sds@tycho.nsa.gov>
> >> >> ---
> >>
> >> Acked-by: Andy Lutomirski <luto@amacapital.net>
> >>
> >> I'm also unconvinced by the subject line -- would "selinux: return an
> >> error when rejecting settexeccon on MNT_NOSUID applications" be
> >> better?
> >
> > ...
> >
> > On Wednesday, May 14, 2014 12:28:24 PM Stephen Smalley wrote:
> >> On 05/14/2014 11:58 AM, Paul Moore wrote:
> >> > We presently prevent processes from explicitly setting an arbitrary
> >> > security label on new processes when NO_NEW_PRIVS is enabled; in an
> >> > attempt for more consistency, this patch extends this to prevent
> >> > setting an arbitrary label when the new application lives on a
> >> > filesystem mounted with MNT_NOSUID.
> >>
> >> It is never arbitrary (the new value is always controlled by policy),
> >> and it isn't set on "new processes" per se but rather transitioned to
> >> upon an exec. Anyway, the point of the change is to return an error
> >> rather than silently ignore any /proc/self/attr/exec value when
> >> executing from a nosuid mount.
> >>
> >> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
> >
> > I had a feeling while writing the patch description yesterday that someone
> > was not going to be happy with the text ... does the subject/description
> > below sound better to you guys?
> >
> > ***
> > selinux: reject setexeccon() on MNT_NOSUID applications with -EACCES
> >
> > We presently prevent processes from using setexecon() to set the security
> > label of exec()'d processes when NO_NEW_PRIVS is enabled by returning an
> > error; however, we silently ignore setexeccon() when exec()'ing from a
> > nosuid mounted filesystem. This patch makes things a bit more consistent
> > by returning an error in the setexeccon()/nosuid case.
> > ***
>
> Looks good to me.
Okay, I expect if Stephen had a strong objection to the new text he would have
commented by now. I'm going to toss this into #next with the new text now.
--
paul moore
security and virtualization @ redhat
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-05-15 15:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-14 15:58 [RFC PATCH] selinux: prevent setting a security label on MNT_NOSUID applications Paul Moore
2014-05-14 16:16 ` Andy Lutomirski
2014-05-14 16:18 ` Andy Lutomirski
2014-05-14 16:23 ` Stephen Smalley
2014-05-14 16:48 ` Andy Lutomirski
2014-05-14 16:28 ` Stephen Smalley
2014-05-14 21:00 ` Paul Moore
2014-05-14 21:26 ` Andy Lutomirski
2014-05-15 15:14 ` Paul Moore
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.