From: "Frank Filz" <ffilzlnx@mindspring.com>
To: "'Andy Lutomirski'" <luto@amacapital.net>,
"'Ben Hutchings'" <ben@decadent.org.uk>
Cc: "'Andy Lutomirski'" <luto@kernel.org>, <security@kernel.org>,
"'Konstantin Khlebnikov'" <koct9i@gmail.com>,
"'Alexander Viro'" <viro@zeniv.linux.org.uk>,
"'Kees Cook'" <keescook@chromium.org>,
"'Willy Tarreau'" <w@1wt.eu>, <linux-mm@kvack.org>,
"'Andrew Morton'" <akpm@linux-foundation.org>,
"'yalin wang'" <yalin.wang2010@gmail.com>,
"'Linux Kernel Mailing List'" <linux-kernel@vger.kernel.org>,
"'Jan Kara'" <jack@suse.cz>,
"'Linux FS Devel'" <linux-fsdevel@vger.kernel.org>,
"'stable'" <stable@vger.kernel.org>
Subject: RE: [PATCH 1/2] fs: Check f_cred instead of current's creds in should_remove_suid()
Date: Wed, 25 Jan 2017 15:15:16 -0800 [thread overview]
Message-ID: <014301d27760$e4d8b9a0$ae8a2ce0$@mindspring.com> (raw)
In-Reply-To: <CALCETrUyWGF7WWVxv5e1tznkdV07YCrOcUeoJE8wUn-qCZMAKw@mail.gmail.com>
> On Wed, Jan 25, 2017 at 1:43 PM, Ben Hutchings <ben@decadent.org.uk>
> wrote:
> > On Wed, 2017-01-25 at 13:06 -0800, Andy Lutomirski wrote:
> >> If an unprivileged program opens a setgid file for write and passes
> >> the fd to a privileged program and the privileged program writes to
> >> it, we currently fail to clear the setgid bit. Fix it by checking
> >> f_cred instead of current's creds whenever a struct file is involved.
> > [...]
> >
> > What if, instead, a privileged program passes the fd to an un
> > unprivileged program? It sounds like a bad idea to start with, but at
> > least currently the unprivileged program is going to clear the setgid
> > bit when it writes. This change would make that behaviour more
> > dangerous.
>
> Hmm. Although, if a privileged program does something like:
>
> (sudo -u nobody echo blah) >setuid_program
>
> presumably it wanted to make the change.
I'm not following all the intricacies here, though I need to...
What about a privileged program that drops privilege for certain operations?
Specifically the Ganesha user space NFS server runs as root, but sets fsuid/fsgid for specific threads performing I/O operations on behalf of NFS clients.
I want to make sure setgid bit handling is proper for these cases.
Ganesha does some permission checking, but this is one area I want to defer to the underlying filesystem because it's not easy for Ganesha to get it right.
> > Perhaps there should be a capability check on both the current
> > credentials and file credentials? (I realise that we've considered
> > file credential checks to be sufficient elsewhere, but those cases
> > involved virtual files with special semantics, where it's clearer that
> > a privileged process should not pass them to an unprivileged process.)
> >
>
> I could go either way.
>
> What I really want to do is to write a third patch that isn't for -stable that just
> removes the capable() check entirely. I'm reasonably confident it won't
> break things for a silly reason: because it's capable() and not ns_capable(),
> anything it would break would also be broken in an unprivileged container,
> and I haven't seen any reports of package managers or similar breaking for
> this reason.
Frank
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
WARNING: multiple messages have this Message-ID (diff)
From: "Frank Filz" <ffilzlnx@mindspring.com>
To: "'Andy Lutomirski'" <luto@amacapital.net>,
"'Ben Hutchings'" <ben@decadent.org.uk>
Cc: "'Andy Lutomirski'" <luto@kernel.org>, <security@kernel.org>,
"'Konstantin Khlebnikov'" <koct9i@gmail.com>,
"'Alexander Viro'" <viro@zeniv.linux.org.uk>,
"'Kees Cook'" <keescook@chromium.org>,
"'Willy Tarreau'" <w@1wt.eu>, <linux-mm@kvack.org>,
"'Andrew Morton'" <akpm@linux-foundation.org>,
"'yalin wang'" <yalin.wang2010@gmail.com>,
"'Linux Kernel Mailing List'" <linux-kernel@vger.kernel.org>,
"'Jan Kara'" <jack@suse.cz>,
"'Linux FS Devel'" <linux-fsdevel@vger.kernel.org>,
"'stable'" <stable@vger.kernel.org>
Subject: RE: [PATCH 1/2] fs: Check f_cred instead of current's creds in should_remove_suid()
Date: Wed, 25 Jan 2017 15:15:16 -0800 [thread overview]
Message-ID: <014301d27760$e4d8b9a0$ae8a2ce0$@mindspring.com> (raw)
In-Reply-To: <CALCETrUyWGF7WWVxv5e1tznkdV07YCrOcUeoJE8wUn-qCZMAKw@mail.gmail.com>
> On Wed, Jan 25, 2017 at 1:43 PM, Ben Hutchings <ben@decadent.org.uk>
> wrote:
> > On Wed, 2017-01-25 at 13:06 -0800, Andy Lutomirski wrote:
> >> If an unprivileged program opens a setgid file for write and passes
> >> the fd to a privileged program and the privileged program writes to
> >> it, we currently fail to clear the setgid bit. Fix it by checking
> >> f_cred instead of current's creds whenever a struct file is involved.
> > [...]
> >
> > What if, instead, a privileged program passes the fd to an un
> > unprivileged program? It sounds like a bad idea to start with, but at
> > least currently the unprivileged program is going to clear the setgid
> > bit when it writes. This change would make that behaviour more
> > dangerous.
>
> Hmm. Although, if a privileged program does something like:
>
> (sudo -u nobody echo blah) >setuid_program
>
> presumably it wanted to make the change.
I'm not following all the intricacies here, though I need to...
What about a privileged program that drops privilege for certain operations?
Specifically the Ganesha user space NFS server runs as root, but sets fsuid/fsgid for specific threads performing I/O operations on behalf of NFS clients.
I want to make sure setgid bit handling is proper for these cases.
Ganesha does some permission checking, but this is one area I want to defer to the underlying filesystem because it's not easy for Ganesha to get it right.
> > Perhaps there should be a capability check on both the current
> > credentials and file credentials? (I realise that we've considered
> > file credential checks to be sufficient elsewhere, but those cases
> > involved virtual files with special semantics, where it's clearer that
> > a privileged process should not pass them to an unprivileged process.)
> >
>
> I could go either way.
>
> What I really want to do is to write a third patch that isn't for -stable that just
> removes the capable() check entirely. I'm reasonably confident it won't
> break things for a silly reason: because it's capable() and not ns_capable(),
> anything it would break would also be broken in an unprivileged container,
> and I haven't seen any reports of package managers or similar breaking for
> this reason.
Frank
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: "Frank Filz" <ffilzlnx@mindspring.com>
To: 'Andy Lutomirski' <luto@amacapital.net>,
'Ben Hutchings' <ben@decadent.org.uk>
Cc: 'Andy Lutomirski' <luto@kernel.org>,
security@kernel.org, 'Konstantin Khlebnikov' <koct9i@gmail.com>,
'Alexander Viro' <viro@zeniv.linux.org.uk>,
'Kees Cook' <keescook@chromium.org>, 'Willy Tarreau' <w@1wt.eu>,
linux-mm@kvack.org, 'Andrew Morton' <akpm@linux-foundation.org>,
'yalin wang' <yalin.wang2010@gmail.com>,
'Linux Kernel Mailing List' <linux-kernel@vger.kernel.org>,
'Jan Kara' <jack@suse.cz>,
'Linux FS Devel' <linux-fsdevel@vger.kernel.org>,
'stable' <stable@vger.kernel.org>
Subject: RE: [PATCH 1/2] fs: Check f_cred instead of current's creds in should_remove_suid()
Date: Wed, 25 Jan 2017 15:15:16 -0800 [thread overview]
Message-ID: <014301d27760$e4d8b9a0$ae8a2ce0$@mindspring.com> (raw)
In-Reply-To: <CALCETrUyWGF7WWVxv5e1tznkdV07YCrOcUeoJE8wUn-qCZMAKw@mail.gmail.com>
> On Wed, Jan 25, 2017 at 1:43 PM, Ben Hutchings <ben@decadent.org.uk>
> wrote:
> > On Wed, 2017-01-25 at 13:06 -0800, Andy Lutomirski wrote:
> >> If an unprivileged program opens a setgid file for write and passes
> >> the fd to a privileged program and the privileged program writes to
> >> it, we currently fail to clear the setgid bit. Fix it by checking
> >> f_cred instead of current's creds whenever a struct file is involved.
> > [...]
> >
> > What if, instead, a privileged program passes the fd to an un
> > unprivileged program? It sounds like a bad idea to start with, but at
> > least currently the unprivileged program is going to clear the setgid
> > bit when it writes. This change would make that behaviour more
> > dangerous.
>
> Hmm. Although, if a privileged program does something like:
>
> (sudo -u nobody echo blah) >setuid_program
>
> presumably it wanted to make the change.
I'm not following all the intricacies here, though I need to...
What about a privileged program that drops privilege for certain operations?
Specifically the Ganesha user space NFS server runs as root, but sets fsuid/fsgid for specific threads performing I/O operations on behalf of NFS clients.
I want to make sure setgid bit handling is proper for these cases.
Ganesha does some permission checking, but this is one area I want to defer to the underlying filesystem because it's not easy for Ganesha to get it right.
> > Perhaps there should be a capability check on both the current
> > credentials and file credentials? (I realise that we've considered
> > file credential checks to be sufficient elsewhere, but those cases
> > involved virtual files with special semantics, where it's clearer that
> > a privileged process should not pass them to an unprivileged process.)
> >
>
> I could go either way.
>
> What I really want to do is to write a third patch that isn't for -stable that just
> removes the capable() check entirely. I'm reasonably confident it won't
> break things for a silly reason: because it's capable() and not ns_capable(),
> anything it would break would also be broken in an unprivileged container,
> and I haven't seen any reports of package managers or similar breaking for
> this reason.
Frank
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-01-25 23:32 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-25 21:06 [PATCH 0/2] setgid hardening Andy Lutomirski
2017-01-25 21:06 ` Andy Lutomirski
2017-01-25 21:06 ` [PATCH 1/2] fs: Check f_cred instead of current's creds in should_remove_suid() Andy Lutomirski
2017-01-25 21:06 ` Andy Lutomirski
2017-01-25 21:43 ` Ben Hutchings
2017-01-25 21:48 ` Andy Lutomirski
2017-01-25 21:48 ` Andy Lutomirski
2017-01-25 23:15 ` Frank Filz [this message]
2017-01-25 23:15 ` Frank Filz
2017-01-25 23:15 ` Frank Filz
2017-01-26 0:12 ` Kees Cook
2017-01-26 0:12 ` Kees Cook
2017-01-25 21:06 ` [PATCH 2/2] fs: Harden against open(..., O_CREAT, 02777) in a setgid directory Andy Lutomirski
2017-01-25 21:06 ` Andy Lutomirski
2017-01-25 21:31 ` Ben Hutchings
2017-01-25 21:44 ` Andy Lutomirski
2017-01-25 21:44 ` Andy Lutomirski
2017-01-25 23:17 ` Frank Filz
2017-01-25 23:17 ` Frank Filz
2017-01-25 23:17 ` Frank Filz
2017-01-25 23:50 ` Willy Tarreau
2017-01-25 23:50 ` Willy Tarreau
2017-01-25 23:59 ` [PATCH 0/2] setgid hardening Willy Tarreau
2017-01-25 23:59 ` Willy Tarreau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='014301d27760$e4d8b9a0$ae8a2ce0$@mindspring.com' \
--to=ffilzlnx@mindspring.com \
--cc=akpm@linux-foundation.org \
--cc=ben@decadent.org.uk \
--cc=jack@suse.cz \
--cc=keescook@chromium.org \
--cc=koct9i@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@amacapital.net \
--cc=luto@kernel.org \
--cc=security@kernel.org \
--cc=stable@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
--cc=w@1wt.eu \
--cc=yalin.wang2010@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.