[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110605194746.GA6484@albatros>
Date: Sun, 5 Jun 2011 23:47:46 +0400
From: Vasiliy Kulikov <segoon@...nwall.com>
To: kernel-hardening@...ts.openwall.com
Subject: Re: [RFC v1] procfs mount options
Solar,
On Sun, Jun 05, 2011 at 23:26 +0400, Solar Designer wrote:
> On Sun, Jun 05, 2011 at 10:24:31PM +0400, Vasiliy Kulikov wrote:
> > This patch introduces support of procfs mount options and adds mount
> > options to restrict access to /proc/PID/ directories and /proc/PID/net/
> > contents. The default backward-compatible 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
> > /proc/PID/* files.
>
> Aren't some /prod/PID/* files restricted by default, in stock kernels?
> I think several are (auxv, fd/, mem). So perhaps re-word this.
Yes, I've mentioned still accessible files in hidepid=1. Will do.
> > hidepid=1 means all files not running as current user and group are
>
> "... files not running ..." needs to be re-worded.
Oops, made a mistake while rewording.
> > TODO/thoughs:
> > - /proc/pid/net/ currently doesn't show ANYTHING, even "." and "..".
> > This is confusing :)
>
> Ouch. Can't you simply restrict its perms such that this directory
> can't be listed unless you have privs?
Well, yes, but it would touch too many code - currently it is handled as
an entry in a static table. Changing this would touch many high level
loops of the table handling. Hiding its contents is just simplier.
Another solution - create a fake net namespace and process this
namespace if not enough permissions :) It also removes weird netstat
errors like "seems like networking was disabled for this kernel".
> It should act as a normal mode
> 550 directory on a regular filesystem.
What 550 perms would give? /proc/pid/net/ contains all network
information about _all_ network connections in current net namespace.
So, /proc/1/net and /proc/2/net are logically the same directory.
However, changing the mode to 550 _and_ changing uid and gid will help.
> > - need to alter "(inode->i_mode == (S_IFDIR|S_IRUGO|S_IXUGO))" checks
> > to honour non-pid directories.
>
> I see this in the patch, but can't really comment without reviewing the
> code in full context myself.
As Spender commented, this is a weird thing to recognize pids from other
files, which is needed due to some weird caching :)
> > - what if one keeps open /proc/PID/ while executing set*id/capable
> > binary?
>
> Then they deliberately grant this privilege to this process (and maybe
> to its heirs). I see no problem with that.
Ehh... I mean another thing:
Process A with UID=1000 opens /proc/123/, while 123 has UID=1000.
123 exec's setuid binary, /proc/123/ becomes unaccessible to A.
However, A still keeps the directory opened and may read its contents.
> > + if (pid->hide_net &&
> > + (!capable(CAP_NET_ADMIN) && !in_group_p(pid->pid_gid)))
>
> capable() sets a flag when it makes use of the capability (or at least
> it used to), visible via pacct. So, unless anything has changed in this
> area, it is best to check capable() last, such that it's only reached
> when it actually makes a difference. Thus, I'd write:
>
> if (pid->hide_net && !in_group_p(pid->pid_gid) &&
> !capable(CAP_NET_ADMIN))
OK.
> Also, what did you mean by the extra braces? Just separating the
> setting check from the permissions check for readability?
Initially I wrote some ||, so the braces were needed. Will remove.
Thanks you for the comments, will fix and repost,
--
Vasiliy
Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
