Date:   Fri, 20 Jan 2017 16:44:31 +0100
From:   Lafcadio Wluiki <wluikil@...il.com>
To:     Andy Lutomirski <luto@...capital.net>
Cc:     Djalal Harouni <tixxdz@...il.com>,
        Linux API <linux-api@...r.kernel.org>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs
 hidepid= field

On Thu, Jan 19, 2017 at 12:35 AM, Andy Lutomirski <luto@...capital.net> wrote:

>>>>> And from that point on neither nginx itself, nor any of its child
>>>>> processes may see processes in /proc anymore that belong to a different
>>>>> user than "www-data". Other services running on the same system remain
>>>>> unaffected.
>>>
>>> What affect, if any, does this have on ptrace() permissions?
>>
>> This should not affect ptrace() permissions or other system calls that
>> work directly on pids, the test in procfs is related to inodes before
>> the ptrace check, hmm what do you have in mind ?
>
> I'm wondering what problem you're trying to solve, then.  hidepid
> helps lock down procfs, but ISTM you might still want to lock down
> other PID-based APIs.

The (pseudo-) mount option hidepid= is about reducing information
leakage, that's all. It's not about enforcing access to PIDs if you
happen to know them.

This patch set simply make this global option usable locally, that's
all. It does not change semantics, it does not alter permission
checks, it doesn't turn the thing into something different than it is
right now. It just permits to turn on its effect in a more local way,
without having to change the whole system.

>>> Also, this one-way thing seems wrong to me.  I think it should roughly
>>> follow the no_new_privs rules instead.  IOW, if you unshare your
>>> pidns, it gets cleared.  Also, maybe you shouldn't be able to set it
>>
>> Andy I don't follow here, no_new_privs is never cleared right ? I
>> can't see the corresponding clear bit code for it.
>
> I believe that unsharing userns clears no_new_privs.

Reverting the a zero hidepid value if you create your own pid/user
namespace sounds OK to me. After all, if you create your own kingdom
anyway,  it should be up to you what you want to be able to see inside
of it...

> I feel like this feature (per-task hidepid) is subtle and complex
> enough that it should have a very clear purpose and use case before
> it's merged and that we should make sure that there isn't a better way
> to accomplish what you're trying to do.

The purpose for me is really reducing information leakage, the same
thing hidepid= always has done, and not more. To quote the proc(5) man
page about this:

"… This doesn't hide the fact that a process with a specific PID value
exists (it can be learned by other means, for example, by "kill -0
$PID"), but it hides a process's UID and GID, which could otherwise be
learned by employing stat(2) on a /proc/[pid] directory.  This greatly
complicates an attacker's task of gathering information about running
processes  (e.g.,  discovering whether some daemon is running with
elevated privileges, whether another user is running some sensitive
program, whether other users are running any program at all, and so
on). …"

And, yeah, I think this is useful for services, but to make it usable
in general-purpose distros an individual per-service and per-process
opt-in would be much better than a global opt-in.

Yes, I think there's value in reducing information leakage. And yes,
what was true when the proc(5) man page was written, is still relevant
today, I think.

L.

Powered by blists - more mailing lists