* [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
[not found] <alpine.LRH.2.00.1106192154220.7503@taiga.selinuxproject.org>
@ 2011-06-20 5:07 ` James Morris
2011-06-20 10:39 ` Vasiliy Kulikov
2011-06-20 13:31 ` [kernel-hardening] " Solar Designer
0 siblings, 2 replies; 17+ messages in thread
From: James Morris @ 2011-06-20 5:07 UTC (permalink / raw)
To: Vasiliy Kulikov; +Cc: kernel-hardening, linux-kernel, linux-security-module
[please cc: the lsm list with this kind of thing]
> This patch adds support of mount options to restrict access to
> /proc/PID/ directories. The default backward-compatible 'relaxed'
> behaviour is left untouched.
Can you provide evidence that this is a useful feature? e.g. examples of
exploits / techniques which would be _usefully_ hampered or blocked.
> The first mount option is called "hidepid" and its value defines how much
> info about processes we want to be available for non-owners:
>
> hidepid=0 (default) means the current behaviour - anybody may read all
> world-readable /proc/PID/* files.
Why not utilize unix perms on the proc files? Perhaps via stricter
overall defaults which are selected at kernel build or runtime.
> hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> own. Sensitive files like cmdline, io, sched*, status, wchan are now
> protected against other users. As permission checking done in
> proc_pid_permission() and files' permissions are left untouched,
> programs expecting specific files' permissions are not confused.
IMHO such programs are beyond broken and have voided their kernel
warranty.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 17+ messages in thread* [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 5:07 ` [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options James Morris
@ 2011-06-20 10:39 ` Vasiliy Kulikov
2011-06-20 10:43 ` James Morris
2011-06-20 13:58 ` Alexey Dobriyan
2011-06-20 13:31 ` [kernel-hardening] " Solar Designer
1 sibling, 2 replies; 17+ messages in thread
From: Vasiliy Kulikov @ 2011-06-20 10:39 UTC (permalink / raw)
To: James Morris; +Cc: kernel-hardening, linux-kernel, linux-security-module
On Mon, Jun 20, 2011 at 15:07 +1000, James Morris wrote:
> > This patch adds support of mount options to restrict access to
> > /proc/PID/ directories. The default backward-compatible 'relaxed'
> > behaviour is left untouched.
>
> Can you provide evidence that this is a useful feature? e.g. examples of
> exploits / techniques which would be _usefully_ hampered or blocked.
First, most of these files are usefull in sense of statistics gathering
and debugging. There is no reason to provide this information to the
world.
Second, yes, it blocks one source of information used in timing attacks,
just use reading the counters as more or less precise time measurement
when actual timing measurements are not precise enough.
Third, such level of privacy (especially cmdline and comm) may be the
goal itself, where users may be anxious whether anybody knows what they
do.
> > The first mount option is called "hidepid" and its value defines how much
> > info about processes we want to be available for non-owners:
> >
> > hidepid=0 (default) means the current behaviour - anybody may read all
> > world-readable /proc/PID/* files.
>
> Why not utilize unix perms on the proc files? Perhaps via stricter
> overall defaults which are selected at kernel build or runtime.
>
> > hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> > own. Sensitive files like cmdline, io, sched*, status, wchan are now
> > protected against other users. As permission checking done in
> > proc_pid_permission() and files' permissions are left untouched,
> > programs expecting specific files' permissions are not confused.
>
> IMHO such programs are beyond broken and have voided their kernel
> warranty.
Policykit, Debian's start-stop-daemon, util-linux use /proc/PID's uid.
procps use both /proc/PID's uid and gid. Are all of them totally broken?
--
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 10:39 ` Vasiliy Kulikov
@ 2011-06-20 10:43 ` James Morris
2011-06-20 11:23 ` KOSAKI Motohiro
` (2 more replies)
2011-06-20 13:58 ` Alexey Dobriyan
1 sibling, 3 replies; 17+ messages in thread
From: James Morris @ 2011-06-20 10:43 UTC (permalink / raw)
To: Vasiliy Kulikov; +Cc: kernel-hardening, linux-kernel, linux-security-module
On Mon, 20 Jun 2011, Vasiliy Kulikov wrote:
> > Can you provide evidence that this is a useful feature? e.g. examples of
> > exploits / techniques which would be _usefully_ hampered or blocked.
>
> First, most of these files are usefull in sense of statistics gathering
> and debugging. There is no reason to provide this information to the
> world.
>
> Second, yes, it blocks one source of information used in timing attacks,
> just use reading the counters as more or less precise time measurement
> when actual timing measurements are not precise enough.
Can you provide concrete examples?
> > > hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> > > own. Sensitive files like cmdline, io, sched*, status, wchan are now
> > > protected against other users. As permission checking done in
> > > proc_pid_permission() and files' permissions are left untouched,
> > > programs expecting specific files' permissions are not confused.
> >
> > IMHO such programs are beyond broken and have voided their kernel
> > warranty.
>
> Policykit, Debian's start-stop-daemon, util-linux use /proc/PID's uid.
> procps use both /proc/PID's uid and gid. Are all of them totally broken?
If they depend on specific permissions, yes.
To access the information, why not just create a group with Unix read
access to these files?
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 17+ messages in thread* [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 10:43 ` James Morris
@ 2011-06-20 11:23 ` KOSAKI Motohiro
2011-06-20 17:06 ` Vasiliy Kulikov
2011-06-21 18:28 ` Vasiliy Kulikov
2 siblings, 0 replies; 17+ messages in thread
From: KOSAKI Motohiro @ 2011-06-20 11:23 UTC (permalink / raw)
To: James Morris
Cc: Vasiliy Kulikov, kernel-hardening, linux-kernel,
linux-security-module
2011/6/20 James Morris <jmorris@namei.org>:
> On Mon, 20 Jun 2011, Vasiliy Kulikov wrote:
>
>> > Can you provide evidence that this is a useful feature? e.g. examples of
>> > exploits / techniques which would be _usefully_ hampered or blocked.
>>
>> First, most of these files are usefull in sense of statistics gathering
>> and debugging. There is no reason to provide this information to the
>> world.
>>
>> Second, yes, it blocks one source of information used in timing attacks,
>> just use reading the counters as more or less precise time measurement
>> when actual timing measurements are not precise enough.
>
> Can you provide concrete examples?
Vasiliy,
I'm now stand aside James. I mean, if we don't understand your usecase
clearly. we can't gurantee to don't break the feature in the future. So,
we strongly hope to understand it.
Moreover, _now_ I haven't understand your concrete usecase, and then
_I_ can't review and be convinced your code. Please please avoid one line
answer as far as possible, please provide us more information.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 10:43 ` James Morris
2011-06-20 11:23 ` KOSAKI Motohiro
@ 2011-06-20 17:06 ` Vasiliy Kulikov
2011-06-20 19:41 ` Eric W. Biederman
2011-06-20 23:19 ` James Morris
2011-06-21 18:28 ` Vasiliy Kulikov
2 siblings, 2 replies; 17+ messages in thread
From: Vasiliy Kulikov @ 2011-06-20 17:06 UTC (permalink / raw)
To: James Morris
Cc: kernel-hardening, linux-kernel, linux-security-module,
Eric W. Biederman
(cc'ed Eric)
On Mon, Jun 20, 2011 at 20:43 +1000, James Morris wrote:
> > > > hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> > > > own. Sensitive files like cmdline, io, sched*, status, wchan are now
> > > > protected against other users. As permission checking done in
> > > > proc_pid_permission() and files' permissions are left untouched,
> > > > programs expecting specific files' permissions are not confused.
> > >
> > > IMHO such programs are beyond broken and have voided their kernel
> > > warranty.
> >
> > Policykit, Debian's start-stop-daemon, util-linux use /proc/PID's uid.
> > procps use both /proc/PID's uid and gid. Are all of them totally broken?
>
> If they depend on specific permissions, yes.
Could you please then clarify why does this patch changes
pid_revalidate() behaviour:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=99f895518368252ba862cc15ce4eb98ebbe1bec6
It changes files permissions to allow userspace apps to quickly stat
files, not looking into /proc/PID/status. So, uid and gid are explicit
ABI. Breaking procfs uid/gid attributes would break these apps.
Or am I missing something?
Thanks,
--
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 17:06 ` Vasiliy Kulikov
@ 2011-06-20 19:41 ` Eric W. Biederman
2011-06-20 23:19 ` James Morris
1 sibling, 0 replies; 17+ messages in thread
From: Eric W. Biederman @ 2011-06-20 19:41 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: James Morris, kernel-hardening, linux-kernel,
linux-security-module
Vasiliy Kulikov <segoon@openwall.com> writes:
> (cc'ed Eric)
>
> On Mon, Jun 20, 2011 at 20:43 +1000, James Morris wrote:
>> > > > hidepid=1 means users may not access any /proc/<pid>/ directories, but their
>> > > > own. Sensitive files like cmdline, io, sched*, status, wchan are now
>> > > > protected against other users. As permission checking done in
>> > > > proc_pid_permission() and files' permissions are left untouched,
>> > > > programs expecting specific files' permissions are not confused.
>> > >
>> > > IMHO such programs are beyond broken and have voided their kernel
>> > > warranty.
>> >
>> > Policykit, Debian's start-stop-daemon, util-linux use /proc/PID's uid.
>> > procps use both /proc/PID's uid and gid. Are all of them totally broken?
>>
>> If they depend on specific permissions, yes.
>
> Could you please then clarify why does this patch changes
> pid_revalidate() behaviour:
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=99f895518368252ba862cc15ce4eb98ebbe1bec6
>
> It changes files permissions to allow userspace apps to quickly stat
> files, not looking into /proc/PID/status. So, uid and gid are explicit
> ABI. Breaking procfs uid/gid attributes would break these apps.
>
> Or am I missing something?
No, you are not missing something. The uid and gid of proc files are
part of the userspace ABI. The files are owned by the uid and gid
of the respective process. The commit in question did not change
the ABI it documented it because I found out the hardware way that
a small change there breaks userspace.
Eric
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 17:06 ` Vasiliy Kulikov
2011-06-20 19:41 ` Eric W. Biederman
@ 2011-06-20 23:19 ` James Morris
1 sibling, 0 replies; 17+ messages in thread
From: James Morris @ 2011-06-20 23:19 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: kernel-hardening, linux-kernel, linux-security-module,
Eric W. Biederman
On Mon, 20 Jun 2011, Vasiliy Kulikov wrote:
> > If they depend on specific permissions, yes.
>
> Could you please then clarify why does this patch changes
> pid_revalidate() behaviour:
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=99f895518368252ba862cc15ce4eb98ebbe1bec6
>
> It changes files permissions to allow userspace apps to quickly stat
> files, not looking into /proc/PID/status. So, uid and gid are explicit
> ABI. Breaking procfs uid/gid attributes would break these apps.
>
Right, but I'm saying that apps which depend on specific permissions are
broken, which is not the uid / gid attributes.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 10:43 ` James Morris
2011-06-20 11:23 ` KOSAKI Motohiro
2011-06-20 17:06 ` Vasiliy Kulikov
@ 2011-06-21 18:28 ` Vasiliy Kulikov
2 siblings, 0 replies; 17+ messages in thread
From: Vasiliy Kulikov @ 2011-06-21 18:28 UTC (permalink / raw)
To: James Morris; +Cc: kernel-hardening, linux-kernel, linux-security-module
On Mon, Jun 20, 2011 at 20:43 +1000, James Morris wrote:
> On Mon, 20 Jun 2011, Vasiliy Kulikov wrote:
>
> > > Can you provide evidence that this is a useful feature? e.g. examples of
> > > exploits / techniques which would be _usefully_ hampered or blocked.
> >
> > First, most of these files are usefull in sense of statistics gathering
> > and debugging. There is no reason to provide this information to the
> > world.
> >
> > Second, yes, it blocks one source of information used in timing attacks,
> > just use reading the counters as more or less precise time measurement
> > when actual timing measurements are not precise enough.
>
> Can you provide concrete examples?
This is a PoC of ~user/.ssh/authorized_keys presence infoleak (and
whether it is empty) using taskstats interface:
http://www.openwall.com/lists/oss-security/2011/06/21/12
/proc/PID/io can be used too.
More close interaction with ssh client would gain authorized_keys' size or,
probably, what pam module denied the access.
Thanks,
--
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 10:39 ` Vasiliy Kulikov
2011-06-20 10:43 ` James Morris
@ 2011-06-20 13:58 ` Alexey Dobriyan
2011-06-20 14:11 ` [kernel-hardening] " Solar Designer
1 sibling, 1 reply; 17+ messages in thread
From: Alexey Dobriyan @ 2011-06-20 13:58 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: James Morris, kernel-hardening, linux-kernel,
linux-security-module
gid= is bad choice because
a) e. g. VFAT uses uid=/gid= mount options to make all inodes to have
certain uid/gid
b) uid=/gid=, IIRC, will be added as generic VFS mount options (like ro)
with semantics described in a)
so having different semantics for /proc won't be good.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [kernel-hardening] [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 13:58 ` Alexey Dobriyan
@ 2011-06-20 14:11 ` Solar Designer
2011-06-20 14:19 ` Vasiliy Kulikov
0 siblings, 1 reply; 17+ messages in thread
From: Solar Designer @ 2011-06-20 14:11 UTC (permalink / raw)
To: kernel-hardening
Vasiliy -
On Mon, Jun 20, 2011 at 04:58:10PM +0300, Alexey Dobriyan wrote:
> gid= is bad choice because
> a) e. g. VFAT uses uid=/gid= mount options to make all inodes to have
> certain uid/gid
> b) uid=/gid=, IIRC, will be added as generic VFS mount options (like ro)
> with semantics described in a)
>
> so having different semantics for /proc won't be good.
I lost track of your proposals/patches. Aren't you currently proposing
that gid= would make all inodes have the specified gid? If not, why
not? Such semantics sound fine to me. That's what gid= does on procfs
on Linux 2.4.x-ow.
Please explain.
Thanks,
Alexander
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [kernel-hardening] [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 14:11 ` [kernel-hardening] " Solar Designer
@ 2011-06-20 14:19 ` Vasiliy Kulikov
2011-06-20 14:25 ` Solar Designer
0 siblings, 1 reply; 17+ messages in thread
From: Vasiliy Kulikov @ 2011-06-20 14:19 UTC (permalink / raw)
To: kernel-hardening
On Mon, Jun 20, 2011 at 18:11 +0400, Solar Designer wrote:
> Vasiliy -
>
> On Mon, Jun 20, 2011 at 04:58:10PM +0300, Alexey Dobriyan wrote:
> > gid= is bad choice because
> > a) e. g. VFAT uses uid=/gid= mount options to make all inodes to have
> > certain uid/gid
> > b) uid=/gid=, IIRC, will be added as generic VFS mount options (like ro)
> > with semantics described in a)
> >
> > so having different semantics for /proc won't be good.
>
> I lost track of your proposals/patches. Aren't you currently proposing
> that gid= would make all inodes have the specified gid? If not, why
> not? Such semantics sound fine to me. That's what gid= does on procfs
> on Linux 2.4.x-ow.
With taskstats and similar mechanisms IMO it's better to use sysctls
instead of procfs mount options as it would influence not only on procfs
files.
Thanks,
--
Vasiliy
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [kernel-hardening] [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 14:19 ` Vasiliy Kulikov
@ 2011-06-20 14:25 ` Solar Designer
2011-06-20 14:35 ` Vasiliy Kulikov
0 siblings, 1 reply; 17+ messages in thread
From: Solar Designer @ 2011-06-20 14:25 UTC (permalink / raw)
To: kernel-hardening
Vasiliy,
On Mon, Jun 20, 2011 at 06:19:51PM +0400, Vasiliy Kulikov wrote:
> On Mon, Jun 20, 2011 at 18:11 +0400, Solar Designer wrote:
> > On Mon, Jun 20, 2011 at 04:58:10PM +0300, Alexey Dobriyan wrote:
> > > gid= is bad choice because
> > > a) e. g. VFAT uses uid=/gid= mount options to make all inodes to have
> > > certain uid/gid
> > > b) uid=/gid=, IIRC, will be added as generic VFS mount options (like ro)
> > > with semantics described in a)
> > >
> > > so having different semantics for /proc won't be good.
> >
> > I lost track of your proposals/patches. Aren't you currently proposing
> > that gid= would make all inodes have the specified gid? If not, why
> > not? Such semantics sound fine to me. That's what gid= does on procfs
> > on Linux 2.4.x-ow.
>
> With taskstats and similar mechanisms IMO it's better to use sysctls
> instead of procfs mount options as it would influence not only on procfs
> files.
OK, but why did we receive a comment about the gid= mount option then
(quote above)? Are you still proposing it or not anymore? Is the
question "outdated" (resulting from your earlier proposal, not the
latest one)?
Thanks,
Alexander
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [kernel-hardening] [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 14:25 ` Solar Designer
@ 2011-06-20 14:35 ` Vasiliy Kulikov
2011-06-20 14:47 ` Solar Designer
0 siblings, 1 reply; 17+ messages in thread
From: Vasiliy Kulikov @ 2011-06-20 14:35 UTC (permalink / raw)
To: kernel-hardening
Solar,
On Mon, Jun 20, 2011 at 18:25 +0400, Solar Designer wrote:
> On Mon, Jun 20, 2011 at 06:19:51PM +0400, Vasiliy Kulikov wrote:
> > On Mon, Jun 20, 2011 at 18:11 +0400, Solar Designer wrote:
> > > On Mon, Jun 20, 2011 at 04:58:10PM +0300, Alexey Dobriyan wrote:
> > > > gid= is bad choice because
> > > > a) e. g. VFAT uses uid=/gid= mount options to make all inodes to have
> > > > certain uid/gid
> > > > b) uid=/gid=, IIRC, will be added as generic VFS mount options (like ro)
> > > > with semantics described in a)
> > > >
> > > > so having different semantics for /proc won't be good.
> > >
> > > I lost track of your proposals/patches. Aren't you currently proposing
> > > that gid= would make all inodes have the specified gid? If not, why
> > > not? Such semantics sound fine to me. That's what gid= does on procfs
> > > on Linux 2.4.x-ow.
> >
> > With taskstats and similar mechanisms IMO it's better to use sysctls
> > instead of procfs mount options as it would influence not only on procfs
> > files.
>
> OK, but why did we receive a comment about the gid= mount option then
> (quote above)? Are you still proposing it or not anymore?
No, till v4 I assumed there is no way to gather processes' information
except procfs:
http://www.openwall.com/lists/kernel-hardening/2011/06/16/6
> Is the
> question "outdated" (resulting from your earlier proposal, not the
> latest one)?
Yes.
I didn't post a patch with taskstats and sysctl variables to LKML yet
(only the changes in ptrace/capabilities code).
Thanks,
--
Vasiliy
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [kernel-hardening] [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 14:35 ` Vasiliy Kulikov
@ 2011-06-20 14:47 ` Solar Designer
2011-06-20 15:00 ` Vasiliy Kulikov
0 siblings, 1 reply; 17+ messages in thread
From: Solar Designer @ 2011-06-20 14:47 UTC (permalink / raw)
To: kernel-hardening
Vasiliy,
On Mon, Jun 20, 2011 at 06:35:50PM +0400, Vasiliy Kulikov wrote:
> I didn't post a patch with taskstats and sysctl variables to LKML yet
> (only the changes in ptrace/capabilities code).
I don't understand the rationale behind the latter. I can try to guess,
but I'd prefer to see a simple explanation from you. (Maybe I missed
one.) It sounds like you're going to spend considerable time on those
changes, but it is not clear to me whether they're needed or not.
So please explain (maybe in a more proper thread than this one).
Unfortunately, I won't have time to participate in a discussion on this
today (nor in the following few days), but I'd like to be informed
anyway and maybe others will comment.
Thanks,
Alexander
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [kernel-hardening] [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 14:47 ` Solar Designer
@ 2011-06-20 15:00 ` Vasiliy Kulikov
0 siblings, 0 replies; 17+ messages in thread
From: Vasiliy Kulikov @ 2011-06-20 15:00 UTC (permalink / raw)
To: kernel-hardening
Solar,
On Mon, Jun 20, 2011 at 18:47 +0400, Solar Designer wrote:
> On Mon, Jun 20, 2011 at 06:35:50PM +0400, Vasiliy Kulikov wrote:
> > I didn't post a patch with taskstats and sysctl variables to LKML yet
> > (only the changes in ptrace/capabilities code).
>
> I don't understand the rationale behind the latter. I can try to guess,
> but I'd prefer to see a simple explanation from you. (Maybe I missed
> one.) It sounds like you're going to spend considerable time on those
> changes, but it is not clear to me whether they're needed or not.
>
> So please explain (maybe in a more proper thread than this one).
>
> Unfortunately, I won't have time to participate in a discussion on this
> today (nor in the following few days), but I'd like to be informed
> anyway and maybe others will comment.
The thing is that taskstats' way of gathering process' information
includes registering a "hook" via netlink socket. When any process
exits, taskstats code enumerates registered hookers and sends all
information about exited process via the sockets. It is handled in the
context of exiting task. So, to know whether the hooker may gather
process' information or no (whether exiting task should send info via
the socket), I need a function answering whether *another task* may
ptrace *current task* (ptrace_may_access does the reverse) as the last
check in procinfo_task_may_stat_current() (reversed proc_may_access() in
the past).
The changes are relatively simple (the patch look huge, though) as the
patch touches many functions (including LSM and user namespaces) adding
new explicit "task" argument instead of implicit "current" without
important functional changes.
Without ptrace changes HARDEN_PROC is unlikely to be consistently
working because of taskstats.
Also you might want to read my another mail where I've clarified why
I've rejected (temporarily, I hope) hidepid=2:
http://www.openwall.com/lists/kernel-hardening/2011/06/18/2
Thanks,
--
Vasiliy
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
2011-06-20 5:07 ` [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options James Morris
2011-06-20 10:39 ` Vasiliy Kulikov
@ 2011-06-20 13:31 ` Solar Designer
1 sibling, 0 replies; 17+ messages in thread
From: Solar Designer @ 2011-06-20 13:31 UTC (permalink / raw)
To: James Morris; +Cc: kernel-hardening, linux-kernel, linux-security-module
Hi,
I am reading this discussion via the kernel-hardening list.
On Mon, Jun 20, 2011 at 03:07:48PM +1000, James Morris wrote:
> [please cc: the lsm list with this kind of thing]
>
> > This patch adds support of mount options to restrict access to
> > /proc/PID/ directories. The default backward-compatible 'relaxed'
> > behaviour is left untouched.
>
> Can you provide evidence that this is a useful feature? e.g. examples of
> exploits / techniques which would be _usefully_ hampered or blocked.
To me, this is primarily a privacy feature on multi-user servers. This
was the primary intent behind my original implementation for Linux 2.0,
for which there was demand. Then I forward-ported this (still as an
unofficial patch) to 2.2 and later to 2.4, and Brad Spengler
forward-ported it to 2.4 and 2.6 in grsecurity. There was always
demand, primarily for privacy.
On shared web hosts that run users' scripts as the users (for greater
security and resource limits separation), process lists reveal website
access activity of each hosting user to others.
Network connections reveal where other users are connecting from and to,
which may be a privacy leak and it may allow for more focused attacks.
As to attacks not limited to reduced privacy for the users, but allowing
for unauthorized access as a next step, real-world'ish examples include
capturing passwords and filenames that another user (or their script)
passes via a command line. Yes, it is wrong to pass sensitive info like
that, yet it is sometimes done. Partial countermeasures such as mysql
overwriting -pXXX in argv (which it does) leave race conditions.
Restricting access to other users' process info is a more complete
countermeasure. Even if a bypass is ever found, it may be fixed as a
bug. Thus, the countermeasure is not as fundamentally flawed as
zeroizing of argv (yet I have nothing against that being done as well, I
find it just fine as long as its limitations are understood).
Then there are things such as usernames to other resources (databases,
etc.), which may allow for more focused attacks. As to filenames, it is
quite common for users to share semi-private files via a mode 711
directory, so the filenames become somewhat sensitive. (Of course, I am
aware of ways for users to avoid that, but security hardening, which is
what we're doing here, is about reducing risks assuming that user
behavior stays the same. This does not imply that it should stay the
same. We may be advocating for changes in behavior towards best
practices. This is just multi-layered security, where one layer does
not depend on another.)
> > The first mount option is called "hidepid" and its value defines how much
> > info about processes we want to be available for non-owners:
> >
> > hidepid=0 (default) means the current behaviour - anybody may read all
> > world-readable /proc/PID/* files.
>
> Why not utilize unix perms on the proc files? Perhaps via stricter
> overall defaults which are selected at kernel build or runtime.
My original patches used stricter permissions only. However, in this
discussion it was pointed out that this could be bypassed via netlink.
[ I am curious to know when this became possible - that is, whether some
of my old patches were similarly insufficient in that respect or not. ]
I did not follow the discussion closely, but my understanding is that
Vasiliy started changing things in response to that finding.
> > hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> > own. Sensitive files like cmdline, io, sched*, status, wchan are now
> > protected against other users. As permission checking done in
> > proc_pid_permission() and files' permissions are left untouched,
> > programs expecting specific files' permissions are not confused.
>
> IMHO such programs are beyond broken and have voided their kernel
> warranty.
My understanding is that they (at least start-stop-daemon) check owner
info (for good reasons), but not permissions. So we're free to change
permissions.
Thanks,
Alexander
^ permalink raw reply [flat|nested] 17+ messages in thread
* [kernel-hardening] [RFC 2/5 v4] procfs: add hidepid= and gid= mount options
@ 2011-06-15 18:51 Vasiliy Kulikov
0 siblings, 0 replies; 17+ messages in thread
From: Vasiliy Kulikov @ 2011-06-15 18:51 UTC (permalink / raw)
To: linux-kernel
Cc: kernel-hardening, Andrew Morton, Greg Kroah-Hartman,
David S. Miller, Arnd Bergmann, Al Viro, David Rientjes,
Stephen Wilson, KOSAKI Motohiro, Daniel Lezcano,
Eric W. Biederman, Serge E. Hallyn
This patch adds support of mount options to restrict access to
/proc/PID/ directories. The default backward-compatible 'relaxed'
behaviour is left untouched.
The first mount option is called "hidepid" and its value defines how much
info about processes we want to be available for non-owners:
hidepid=0 (default) means the current behaviour - anybody may read all
world-readable /proc/PID/* files.
hidepid=1 means users may not access any /proc/<pid>/ directories, but their
own. Sensitive files like cmdline, io, sched*, status, wchan are now
protected against other users. As permission checking done in
proc_pid_permission() and files' permissions are left untouched,
programs expecting specific files' permissions are not confused.
hidepid=2 means hidepid=1 plus all /proc/PID/ will be invisible to
other users. It doesn't mean that it hides a fact whether a process
exists (it can be learned by other means, e.g. by sending signals), but
it hides process' euid and egid. It greatly compicates intruder's task of
gathering info about running processes, whether some daemon runs with
elevated privileges, whether other user runs some sensitive program,
whether other users run any program at all, etc.
gid=XXX defines a group that will be able to gather all processes' info.
v4 - call ptrace_may_access() as a last check to evade unnecessary
calls to security_ptrace_access_check() and capable() when in_group_p()
is sufficient.
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
fs/proc/base.c | 62 ++++++++++++++++++++++++++++++++++++++++-
fs/proc/inode.c | 8 +++++
fs/proc/root.c | 18 +++++++++++-
include/linux/pid_namespace.h | 2 +
4 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 14def99..b1562a3 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -627,8 +627,40 @@ int proc_setattr(struct dentry *dentry, struct iattr *attr)
return 0;
}
+static int proc_pid_permission(struct inode *inode, int mask,
+ unsigned int flags)
+{
+ struct pid_namespace *pid = inode->i_sb->s_fs_info;
+ struct task_struct *task = get_proc_task(inode);
+
+ if (pid->hide_pid &&
+ !in_group_p(pid->pid_gid) &&
+ !ptrace_may_access(task, PTRACE_MODE_READ)) {
+ if (pid->hide_pid == 2)
+ return -ENOENT;
+ else
+ return -EPERM;
+ }
+ return generic_permission(inode, mask, flags, NULL);
+}
+
+/*
+ * May current process learn task's euid/egid?
+ */
+static bool proc_pid_may_getattr(struct pid_namespace *pid,
+ struct task_struct *task)
+{
+ if (pid->hide_pid < 2)
+ return true;
+ if (in_group_p(pid->pid_gid))
+ return true;
+ return ptrace_may_access(task, PTRACE_MODE_READ);
+}
+
+
static const struct inode_operations proc_def_inode_operations = {
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};
static int mounts_open_common(struct inode *inode, struct file *file,
@@ -1670,6 +1702,7 @@ static const struct inode_operations proc_pid_link_inode_operations = {
.readlink = proc_pid_readlink,
.follow_link = proc_pid_follow_link,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};
@@ -1737,6 +1770,7 @@ int pid_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
struct inode *inode = dentry->d_inode;
struct task_struct *task;
const struct cred *cred;
+ struct pid_namespace *pid = dentry->d_sb->s_fs_info;
generic_fillattr(inode, stat);
@@ -1745,6 +1779,14 @@ int pid_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
stat->gid = 0;
task = pid_task(proc_pid(inode), PIDTYPE_PID);
if (task) {
+ if (!proc_pid_may_getattr(pid, task)) {
+ rcu_read_unlock();
+ /*
+ * This doesn't prevent learning whether PID exists,
+ * it only makes getattr() consistent with readdir().
+ */
+ return -ENOENT;
+ }
if ((inode->i_mode == (S_IFDIR|S_IRUGO|S_IXUGO)) ||
task_dumpable(task)) {
cred = __task_cred(task);
@@ -2188,6 +2230,7 @@ static const struct inode_operations proc_fd_inode_operations = {
.lookup = proc_lookupfd,
.permission = proc_fd_permission,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};
static struct dentry *proc_fdinfo_instantiate(struct inode *dir,
@@ -2240,6 +2283,7 @@ static const struct file_operations proc_fdinfo_operations = {
static const struct inode_operations proc_fdinfo_inode_operations = {
.lookup = proc_lookupfdinfo,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};
@@ -2477,6 +2521,7 @@ static const struct inode_operations proc_attr_dir_inode_operations = {
.lookup = proc_attr_dir_lookup,
.getattr = pid_getattr,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};
#endif
@@ -2872,6 +2917,7 @@ static const struct inode_operations proc_tgid_base_inode_operations = {
.lookup = proc_tgid_base_lookup,
.getattr = pid_getattr,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};
static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
@@ -3075,6 +3121,12 @@ static int proc_pid_fill_cache(struct file *filp, void *dirent, filldir_t filldi
proc_pid_instantiate, iter.task, NULL);
}
+static int fake_filldir(void *buf, const char *name, int namelen,
+ loff_t offset, u64 ino, unsigned d_type)
+{
+ return 0;
+}
+
/* for the /proc/ directory itself, after non-process stuff has been done */
int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
{
@@ -3082,6 +3134,7 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
struct task_struct *reaper;
struct tgid_iter iter;
struct pid_namespace *ns;
+ filldir_t __filldir;
if (filp->f_pos >= PID_MAX_LIMIT + TGID_OFFSET)
goto out_no_task;
@@ -3103,8 +3156,13 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
for (iter = next_tgid(ns, iter);
iter.task;
iter.tgid += 1, iter = next_tgid(ns, iter)) {
+ if (proc_pid_may_getattr(ns, iter.task))
+ __filldir = filldir;
+ else
+ __filldir = fake_filldir;
+
filp->f_pos = iter.tgid + TGID_OFFSET;
- if (proc_pid_fill_cache(filp, dirent, filldir, iter) < 0) {
+ if (proc_pid_fill_cache(filp, dirent, __filldir, iter) < 0) {
put_task_struct(iter.task);
goto out;
}
@@ -3214,6 +3272,7 @@ static const struct inode_operations proc_tid_base_inode_operations = {
.lookup = proc_tid_base_lookup,
.getattr = pid_getattr,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};
static struct dentry *proc_task_instantiate(struct inode *dir,
@@ -3439,6 +3498,7 @@ static const struct inode_operations proc_task_inode_operations = {
.lookup = proc_task_lookup,
.getattr = proc_task_getattr,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};
static const struct file_operations proc_task_operations = {
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index b5f49eb..34ab842 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -107,6 +107,14 @@ void __init proc_init_inodecache(void)
static int proc_show_options(struct seq_file *seq, struct vfsmount *vfs)
{
+ struct super_block *sb = vfs->mnt_sb;
+ struct pid_namespace *pid = sb->s_fs_info;
+
+ if (pid->pid_gid)
+ seq_printf(seq, ",gid=%lu", (unsigned long)pid->pid_gid);
+ if (pid->hide_pid != 0)
+ seq_printf(seq, ",hidepid=%u", pid->hide_pid);
+
return 0;
}
diff --git a/fs/proc/root.c b/fs/proc/root.c
index b2571fe..fe6f2c1 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -37,10 +37,12 @@ static int proc_set_super(struct super_block *sb, void *data)
}
enum {
- Opt_err,
+ Opt_gid, Opt_hidepid, Opt_err,
};
static const match_table_t tokens = {
+ {Opt_hidepid, "hidepid=%u"},
+ {Opt_gid, "gid=%u"},
{Opt_err, NULL},
};
@@ -63,6 +65,20 @@ static int proc_parse_options(char *options, struct pid_namespace *pid)
args[0].to = args[0].from = 0;
token = match_token(p, tokens, args);
switch (token) {
+ case Opt_gid:
+ if (match_int(&args[0], &option))
+ return 0;
+ pid->pid_gid = option;
+ break;
+ case Opt_hidepid:
+ if (match_int(&args[0], &option))
+ return 0;
+ if (option < 0 || option > 2) {
+ pr_err("proc: hidepid value must be between 0 and 2.\n");
+ return 0;
+ }
+ pid->hide_pid = option;
+ break;
default:
pr_err("proc: unrecognized mount option \"%s\" "
"or missing value", p);
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 38d1032..e7cf666 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -30,6 +30,8 @@ struct pid_namespace {
#ifdef CONFIG_BSD_PROCESS_ACCT
struct bsd_acct_struct *bacct;
#endif
+ gid_t pid_gid;
+ int hide_pid;
};
extern struct pid_namespace init_pid_ns;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2011-06-21 18:28 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <alpine.LRH.2.00.1106192154220.7503@taiga.selinuxproject.org>
2011-06-20 5:07 ` [kernel-hardening] Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options James Morris
2011-06-20 10:39 ` Vasiliy Kulikov
2011-06-20 10:43 ` James Morris
2011-06-20 11:23 ` KOSAKI Motohiro
2011-06-20 17:06 ` Vasiliy Kulikov
2011-06-20 19:41 ` Eric W. Biederman
2011-06-20 23:19 ` James Morris
2011-06-21 18:28 ` Vasiliy Kulikov
2011-06-20 13:58 ` Alexey Dobriyan
2011-06-20 14:11 ` [kernel-hardening] " Solar Designer
2011-06-20 14:19 ` Vasiliy Kulikov
2011-06-20 14:25 ` Solar Designer
2011-06-20 14:35 ` Vasiliy Kulikov
2011-06-20 14:47 ` Solar Designer
2011-06-20 15:00 ` Vasiliy Kulikov
2011-06-20 13:31 ` [kernel-hardening] " Solar Designer
2011-06-15 18:51 [kernel-hardening] " Vasiliy Kulikov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox