Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1562251 > unrolled thread

Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs hidepid= field

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Djalal Harouni <tixxdz@gmail.com> - 2017-01-19 00:00 +0100
    Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Andy Lutomirski <luto@amacapital.net> - 2017-01-19 00:40 +0100
      Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Djalal Harouni <tixxdz@gmail.com> - 2017-01-19 15:00 +0100
        Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Andy Lutomirski <luto@amacapital.net> - 2017-01-19 21:00 +0100
          Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Lafcadio Wluiki <wluikil@gmail.com> - 2017-01-20 17:00 +0100
          Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Djalal Harouni <tixxdz@gmail.com> - 2017-01-20 17:40 +0100
            Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Andy Lutomirski <luto@amacapital.net> - 2017-01-21 02:00 +0100
              Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Djalal Harouni <tixxdz@gmail.com> - 2017-01-23 12:50 +0100
                Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Andy Lutomirski <luto@amacapital.net> - 2017-01-23 21:10 +0100
                  Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Djalal Harouni <tixxdz@gmail.com> - 2017-01-26 14:30 +0100
      Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs  hidepid= field Lafcadio Wluiki <wluikil@gmail.com> - 2017-01-20 16:50 +0100

#1562251 — Re: [PATCH v4 2/2] procfs/tasks: add a simple per-task procfs hidepid= field

On Tue, Jan 17, 2017 at 9:33 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Mon, Jan 16, 2017 at 9:15 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
>> Cc linux-api
>>
>> On Mon, Jan 16, 2017 at 2:23 PM, Djalal Harouni <tixxdz@gmail.com> wrote:
>>>
>>> From: Djalal Harouni <tixxdz@gmail.com>
>>>
>>> This adds a new per-task hidepid= flag that is honored by procfs when
>>> presenting /proc to the user, in addition to the existing hidepid= mount
>>> option. So far, hidepid= was exclusively a per-pidns setting. Locking
>>> down a set of processes so that they cannot see other user's processes
>>> without affecting the rest of the system thus currently requires
>>> creation of a private PID namespace, with all the complexity it brings,
>>> including maintaining a stub init process as PID 1 and losing the
>>> ability to see processes of the same user on the rest of the system.
>>>
>>> With this patch all acesss and visibility checks in procfs now
>>> honour two fields:
>>>
>>>         a) the existing hide_pid field in the PID namespace
>>>         b) the new hide_pid in struct task_struct
>>>
>>> Access/visibility is only granted if both fields permit it; the more
>>> restrictive one wins. By default the new task_struct hide_pid value
>>> defaults to 0, which means behaviour is not changed from the status quo.
>>>
>>> Setting the per-process hide_pid value is done via a new PR_SET_HIDEPID
>>> prctl() option which takes the same three supported values as the
>>> hidepid= mount option. The per-process hide_pid may only be increased,
>>> never decreased, thus ensuring that once applied, processes can never
>>> escape such a hide_pid jail.  When a process forks it inherits its
>>> parent's hide_pid value.
>>>
>>> Suggested usecase: let's say nginx runs as user "www-data". After
>>> dropping privileges it may now call:
>>>
>>>         …
>>>         prctl(PR_SET_HIDEPID, 2);
>>>         …
>>>
>>> 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 ?


> 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.

For this one I want it to act like no_new_privs. Also pidns can be
created with userns which means it can be revoked. For my use case I
want it to be part of *one* single operation where it is set with the
other sandbox operations that are all preserved... instead of setting
it *again* each time where it can already be late.


> without either having CAP_SYS_ADMIN over your userns or having
> no_new_privs set.

For this one I can add it sure. Historically that logic was added to
make seccomp more usable, for this patch the values can't be relaxed,
they are always increased never decreased. However one minor advantage
if you require no_new_privs is that this option hidepid will also
assert that you can't setuid to access some procfs inodes... though
you can also just set 'no_new_privs + hidepid' both of them in any
order. Also it allows unprivileged without userns to setup a minimal
jail while performing some operations that can be blocked by
no_new_privs.

Andy, Kees any other comments please on it ? I'm not sure if overusing
no_new_privs in this case is a good idea. Seems to me that seccomp +
no_new_privs is different than this hidepid feature that overlaps
nicely with no_new_privs.

If there are no responses for this question, then I will just add the
"CAP_SYS_ADMIN || no_new_privs" test in the next iteration.

> --Andy

Thanks!

-- 
tixxdz
http://opendz.org

[toc] | [next] | [standalone]

#1562300

On Wed, Jan 18, 2017 at 2:50 PM, Djalal Harouni <tixxdz@gmail.com> wrote:
> On Tue, Jan 17, 2017 at 9:33 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Mon, Jan 16, 2017 at 9:15 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
>>> Cc linux-api
>>>
>>> On Mon, Jan 16, 2017 at 2:23 PM, Djalal Harouni <tixxdz@gmail.com> wrote:
>>>>
>>>> From: Djalal Harouni <tixxdz@gmail.com>
>>>>
>>>> This adds a new per-task hidepid= flag that is honored by procfs when
>>>> presenting /proc to the user, in addition to the existing hidepid= mount
>>>> option. So far, hidepid= was exclusively a per-pidns setting. Locking
>>>> down a set of processes so that they cannot see other user's processes
>>>> without affecting the rest of the system thus currently requires
>>>> creation of a private PID namespace, with all the complexity it brings,
>>>> including maintaining a stub init process as PID 1 and losing the
>>>> ability to see processes of the same user on the rest of the system.
>>>>
>>>> With this patch all acesss and visibility checks in procfs now
>>>> honour two fields:
>>>>
>>>>         a) the existing hide_pid field in the PID namespace
>>>>         b) the new hide_pid in struct task_struct
>>>>
>>>> Access/visibility is only granted if both fields permit it; the more
>>>> restrictive one wins. By default the new task_struct hide_pid value
>>>> defaults to 0, which means behaviour is not changed from the status quo.
>>>>
>>>> Setting the per-process hide_pid value is done via a new PR_SET_HIDEPID
>>>> prctl() option which takes the same three supported values as the
>>>> hidepid= mount option. The per-process hide_pid may only be increased,
>>>> never decreased, thus ensuring that once applied, processes can never
>>>> escape such a hide_pid jail.  When a process forks it inherits its
>>>> parent's hide_pid value.
>>>>
>>>> Suggested usecase: let's say nginx runs as user "www-data". After
>>>> dropping privileges it may now call:
>>>>
>>>>         …
>>>>         prctl(PR_SET_HIDEPID, 2);
>>>>         …
>>>>
>>>> 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.

>
>> 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.

>
> For this one I want it to act like no_new_privs. Also pidns can be
> created with userns which means it can be revoked. For my use case I
> want it to be part of *one* single operation where it is set with the
> other sandbox operations that are all preserved... instead of setting
> it *again* each time where it can already be late.
>

I don't see the problem as long as this gets implemented carefully
enough.  If you unshare your userns and your pidns, then you should be
able to see all tasks in the new pidns, even if you mount a fresh
procfs pointing at that pidns -- after all, you are privileged in that
namespace.

>
>> without either having CAP_SYS_ADMIN over your userns or having
>> no_new_privs set.
>
> For this one I can add it sure. Historically that logic was added to
> make seccomp more usable, for this patch the values can't be relaxed,
> they are always increased never decreased. However one minor advantage
> if you require no_new_privs is that this option hidepid will also
> assert that you can't setuid to access some procfs inodes... though
> you can also just set 'no_new_privs + hidepid' both of them in any
> order. Also it allows unprivileged without userns to setup a minimal
> jail while performing some operations that can be blocked by
> no_new_privs.
>
> Andy, Kees any other comments please on it ? I'm not sure if overusing
> no_new_privs in this case is a good idea. Seems to me that seccomp +
> no_new_privs is different than this hidepid feature that overlaps
> nicely with no_new_privs.
>
> If there are no responses for this question, then I will just add the
> "CAP_SYS_ADMIN || no_new_privs" test in the next iteration.

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.

[toc] | [prev] | [next] | [standalone]

#1562746

On Thu, Jan 19, 2017 at 12:35 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Jan 18, 2017 at 2:50 PM, Djalal Harouni <tixxdz@gmail.com> wrote:
[...]
>>>>>
>>>>>         …
>>>>>         prctl(PR_SET_HIDEPID, 2);
>>>>>         …
>>>>>
>>>>> 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.

Yes but they are already locked based on uid checks. procfs was not
and this patch is specifically to align it, and to reduce the ability
to peek data from other processes.

>>
>>> 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.
No, it is not cleared, and I can't see the clear bit for it. Maybe due
to userns+filesystems limitations it was not noticed.


>>
>> For this one I want it to act like no_new_privs. Also pidns can be
>> created with userns which means it can be revoked. For my use case I
>> want it to be part of *one* single operation where it is set with the
>> other sandbox operations that are all preserved... instead of setting
>> it *again* each time where it can already be late.
>>
>
> I don't see the problem as long as this gets implemented carefully
> enough.  If you unshare your userns and your pidns, then you should be
> able to see all tasks in the new pidns, even if you mount a fresh
> procfs pointing at that pidns -- after all, you are privileged in that
> namespace.

That's already the case, if you are privileged you can see all tasks,
the code is written that the per-task hidepid does not overwrite
capabilities.


>>
>>> without either having CAP_SYS_ADMIN over your userns or having
>>> no_new_privs set.
>>
>> For this one I can add it sure. Historically that logic was added to
>> make seccomp more usable, for this patch the values can't be relaxed,
>> they are always increased never decreased. However one minor advantage
>> if you require no_new_privs is that this option hidepid will also
>> assert that you can't setuid to access some procfs inodes... though
>> you can also just set 'no_new_privs + hidepid' both of them in any
>> order. Also it allows unprivileged without userns to setup a minimal
>> jail while performing some operations that can be blocked by
>> no_new_privs.
>>
>> Andy, Kees any other comments please on it ? I'm not sure if overusing
>> no_new_privs in this case is a good idea. Seems to me that seccomp +
>> no_new_privs is different than this hidepid feature that overlaps
>> nicely with no_new_privs.
>>
>> If there are no responses for this question, then I will just add the
>> "CAP_SYS_ADMIN || no_new_privs" test in the next iteration.
>
> 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.

Sure, the hidepid mount option is old enough, and this per-task
hidepid is clearly defined only for procfs and per task, we can't add
another switch that's relate to both a filesystem and pid namespaces,
it will be a bit complicated and not really useful for cases that are
in *same* pidns where *each* one have to mount its procfs, it will
propagate. Also as noted by Lafcadio, the gid thing is a bit hard to
use now.

Thanks!

-- 
tixxdz
http://opendz.org

[toc] | [prev] | [next] | [standalone]

#1563049

On Thu, Jan 19, 2017 at 5:53 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
> On Thu, Jan 19, 2017 at 12:35 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Wed, Jan 18, 2017 at 2:50 PM, Djalal Harouni <tixxdz@gmail.com> wrote:
>>>> 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.
> No, it is not cleared, and I can't see the clear bit for it. Maybe due
> to userns+filesystems limitations it was not noticed.

Hmm, maybe I remembered wrong.

>> 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.
>
> Sure, the hidepid mount option is old enough, and this per-task
> hidepid is clearly defined only for procfs and per task, we can't add
> another switch that's relate to both a filesystem and pid namespaces,
> it will be a bit complicated and not really useful for cases that are
> in *same* pidns where *each* one have to mount its procfs, it will
> propagate. Also as noted by Lafcadio, the gid thing is a bit hard to
> use now.

What I'm trying to say is that I want to understand a complete,
real-world use case.  Adding a security-related per-task flag is can
be quite messy and requires a lot of careful thought to get right, and
I'd rather avoid it if at all possible.

I'm imaging something like a new RestrictPidVisisbility= option in
systemd.  I agree that this is currently a mess to do.  But maybe a
simpler solution would be to add a new mount option local_hidepid to
procfs.  If you set that option, then it overrides hidepid for that
instance.  Most of these semi-sandboxed daemon processes already have
their own mount namespace, so the overhead should be minimal.

--Andy

[toc] | [prev] | [next] | [standalone]

#1563735

On Thu, Jan 19, 2017 at 8:52 PM, Andy Lutomirski <luto@amacapital.net> wrote:

>> Sure, the hidepid mount option is old enough, and this per-task
>> hidepid is clearly defined only for procfs and per task, we can't add
>> another switch that's relate to both a filesystem and pid namespaces,
>> it will be a bit complicated and not really useful for cases that are
>> in *same* pidns where *each* one have to mount its procfs, it will
>> propagate. Also as noted by Lafcadio, the gid thing is a bit hard to
>> use now.
>
> What I'm trying to say is that I want to understand a complete,
> real-world use case.  Adding a security-related per-task flag is can
> be quite messy and requires a lot of careful thought to get right, and
> I'd rather avoid it if at all possible.
>
> I'm imaging something like a new RestrictPidVisisbility= option in
> systemd.  I agree that this is currently a mess to do.  But maybe a

It's not just a "mess" to do, it's not possible afaics. The hidepid
thing is after all not really a mount option, but an option of the pid
namespace. Hence, if you want to restrict the visibility for one
service only, then you have to get your own PID namespace, but that
does a lot more than just hide visibility: it renumbers everything
after all...

> simpler solution would be to add a new mount option local_hidepid to
> procfs.  If you set that option, then it overrides hidepid for that
> instance.  Most of these semi-sandboxed daemon processes already have
> their own mount namespace, so the overhead should be minimal.

When I worked on the patches originally, I actually wanted to
implement this as true per-superblock procfs mount option. But this is
really hard to do, as the private superblock pointer of the procfs
instance currently points to the pid namespace, and breaking that up,
so that you can have multiple procfs superblocks per pid namespace is
a ton of work, and I doubt anyone would really like the complexity
this brings, just for adding a single 2bit option...

The per-process option is much simpler code-wise. It also has
semantical benefits: if the thing isn't a mount option it is
accessible with absolutely minimal privileges, as it does not imply
namespaces and mounting. This means, my Firefox can run with hidepid
turned on without my KDE Konsole instance also having to turn it on.
Or to say this differently: your suggested RestrictPidVisibility=
works nicely both for "systemd" as PID 1 and for "systemd --user" as
user "lafcadio", when it is per-process, but is much more complex to
implement if it was a true mount option.

L.

[toc] | [prev] | [next] | [standalone]

#1563769

On Thu, Jan 19, 2017 at 8:52 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Jan 19, 2017 at 5:53 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
[...]
>> Sure, the hidepid mount option is old enough, and this per-task
>> hidepid is clearly defined only for procfs and per task, we can't add
>> another switch that's relate to both a filesystem and pid namespaces,
>> it will be a bit complicated and not really useful for cases that are
>> in *same* pidns where *each* one have to mount its procfs, it will
>> propagate. Also as noted by Lafcadio, the gid thing is a bit hard to
>> use now.
>
> What I'm trying to say is that I want to understand a complete,
> real-world use case.  Adding a security-related per-task flag is can
> be quite messy and requires a lot of careful thought to get right, and
> I'd rather avoid it if at all possible.

I do agree, but that's not what we are proposing here. This use case
is limited we do not manipulate the creds of the task, there are no
security transitions. The task does not change, its only related to
procfs and pid entries there. Also the flag applies only to current
task and not on remote ones... Nothing new here it's an extension of
procfs hidepid.

> I'm imaging something like a new RestrictPidVisisbility= option in
> systemd.  I agree that this is currently a mess to do.  But maybe a

Yes that's one use case, If we manage to land this I'll follow up with
it... plus there is, I've a use case related to kubernetes where I do
want to reduce the number of processes inside containers per pod to
minimal. Some other cases are: lock down children where being
unprivileged. Also as noted in other replies on today's desktop
systems, under a normal user session, the user should see all
processes of the system where the media player, browser etc have no
business to see the process tree. This can be easily implemented when
launching apps without the need to regain privileges...

> simpler solution would be to add a new mount option local_hidepid to
> procfs.  If you set that option, then it overrides hidepid for that
> instance.  Most of these semi-sandboxed daemon processes already have
> their own mount namespace, so the overhead should be minimal.

Andy If that could work :-/    we have to re-write or adapt lot of
things inside procfs... plus:
Procfs is a miror to the current pid namespace. Mount options are not
procfs but rather pid namespace. That would not work.
Also having multiple mount namespaces where each one with its setup
and having to migrate tasks between these environements: I would
rather have the security information or context or any minor flag
attached to the task itself rather than the object.
The prctl() interface is really simple for userspace. The kernel
change is not intrusive, and current approach does not require any
privileges. For others you may have to gain privileges or ask some one
to set it up. The current tendency is to allow more unprivileged
code/containers... to setup such mini jails. Note to mention that this
schema is simple and can be isolated from other complexities, set it
once and that's it.


> --Andy



-- 
tixxdz
http://opendz.org

[toc] | [prev] | [next] | [standalone]

#1564032

On Fri, Jan 20, 2017 at 8:33 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
> On Thu, Jan 19, 2017 at 8:52 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Thu, Jan 19, 2017 at 5:53 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
> [...]
>>> Sure, the hidepid mount option is old enough, and this per-task
>>> hidepid is clearly defined only for procfs and per task, we can't add
>>> another switch that's relate to both a filesystem and pid namespaces,
>>> it will be a bit complicated and not really useful for cases that are
>>> in *same* pidns where *each* one have to mount its procfs, it will
>>> propagate. Also as noted by Lafcadio, the gid thing is a bit hard to
>>> use now.
>>
>> What I'm trying to say is that I want to understand a complete,
>> real-world use case.  Adding a security-related per-task flag is can
>> be quite messy and requires a lot of careful thought to get right, and
>> I'd rather avoid it if at all possible.
>
> I do agree, but that's not what we are proposing here. This use case
> is limited we do not manipulate the creds of the task, there are no
> security transitions. The task does not change, its only related to
> procfs and pid entries there. Also the flag applies only to current
> task and not on remote ones... Nothing new here it's an extension of
> procfs hidepid.
>
>> I'm imaging something like a new RestrictPidVisisbility= option in
>> systemd.  I agree that this is currently a mess to do.  But maybe a
>
> Yes that's one use case, If we manage to land this I'll follow up with
> it... plus there is, I've a use case related to kubernetes where I do
> want to reduce the number of processes inside containers per pod to
> minimal. Some other cases are: lock down children where being
> unprivileged. Also as noted in other replies on today's desktop
> systems, under a normal user session, the user should see all
> processes of the system where the media player, browser etc have no
> business to see the process tree. This can be easily implemented when
> launching apps without the need to regain privileges...
>
>> simpler solution would be to add a new mount option local_hidepid to
>> procfs.  If you set that option, then it overrides hidepid for that
>> instance.  Most of these semi-sandboxed daemon processes already have
>> their own mount namespace, so the overhead should be minimal.
>
> Andy If that could work :-/    we have to re-write or adapt lot of
> things inside procfs... plus:
> Procfs is a miror to the current pid namespace. Mount options are not
> procfs but rather pid namespace. That would not work.

I agree that the kernel change to do it per task is very simple.  But
this is an unfortunate slippery slope.  What if you want to block off
everything in /proc that isn't associated with a PID?  What if you
want to suppress /sys access?  What if you want ot block *all*
non-current PIDs from being revealed in /proc?  What if you want to
hide /proc/PID/cmdline?

I think that the right solution here is to fix procfs to understand
per-superblock mount options.

--Andy

[toc] | [prev] | [next] | [standalone]

#1564910

On Sat, Jan 21, 2017 at 1:53 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Jan 20, 2017 at 8:33 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
>> On Thu, Jan 19, 2017 at 8:52 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Thu, Jan 19, 2017 at 5:53 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
>> [...]
>>>> Sure, the hidepid mount option is old enough, and this per-task
>>>> hidepid is clearly defined only for procfs and per task, we can't add
>>>> another switch that's relate to both a filesystem and pid namespaces,
>>>> it will be a bit complicated and not really useful for cases that are
>>>> in *same* pidns where *each* one have to mount its procfs, it will
>>>> propagate. Also as noted by Lafcadio, the gid thing is a bit hard to
>>>> use now.
>>>
>>> What I'm trying to say is that I want to understand a complete,
>>> real-world use case.  Adding a security-related per-task flag is can
>>> be quite messy and requires a lot of careful thought to get right, and
>>> I'd rather avoid it if at all possible.
>>
>> I do agree, but that's not what we are proposing here. This use case
>> is limited we do not manipulate the creds of the task, there are no
>> security transitions. The task does not change, its only related to
>> procfs and pid entries there. Also the flag applies only to current
>> task and not on remote ones... Nothing new here it's an extension of
>> procfs hidepid.
>>
>>> I'm imaging something like a new RestrictPidVisisbility= option in
>>> systemd.  I agree that this is currently a mess to do.  But maybe a
>>
>> Yes that's one use case, If we manage to land this I'll follow up with
>> it... plus there is, I've a use case related to kubernetes where I do
>> want to reduce the number of processes inside containers per pod to
>> minimal. Some other cases are: lock down children where being
>> unprivileged. Also as noted in other replies on today's desktop
>> systems, under a normal user session, the user should see all
>> processes of the system where the media player, browser etc have no
>> business to see the process tree. This can be easily implemented when
>> launching apps without the need to regain privileges...
>>
>>> simpler solution would be to add a new mount option local_hidepid to
>>> procfs.  If you set that option, then it overrides hidepid for that
>>> instance.  Most of these semi-sandboxed daemon processes already have
>>> their own mount namespace, so the overhead should be minimal.
>>
>> Andy If that could work :-/    we have to re-write or adapt lot of
>> things inside procfs... plus:
>> Procfs is a miror to the current pid namespace. Mount options are not
>> procfs but rather pid namespace. That would not work.
>
> I agree that the kernel change to do it per task is very simple.  But
> this is an unfortunate slippery slope.  What if you want to block off
> everything in /proc that isn't associated with a PID?  What if you
> want to suppress /sys access?  What if you want ot block *all*
> non-current PIDs from being revealed in /proc?  What if you want to
> hide /proc/PID/cmdline?

For /sys we mount an inaccessible directory on top, we even do that
for some static /sys and /proc inodes, of course that doesn't scale
but we try... please see below.

For non-current PIDs from being revealed in /proc, actually the use
case did not come, it will be complex to handle TOCTOU, other races
etc. We don't want that and we don't have a use case for it. The patch
here is a clear parent -> child relation.

> I think that the right solution here is to fix procfs to understand
> per-superblock mount options.

Unfortunately and as also noted by Lafcadio and you this is too
complex. Also from what you have said above and from what /proc
reports to userspace and today's use cases with containers, namespaces
etc. maybe the kernel needs a new way to report *some* kernel objects
and other things to userspace which are not based on /proc... or move
them out of /proc... in some cases the kernel may need to know if the
calling process is in a namespace...
but lets please stay focused here, fixing procfs is a bit out of the
scope for this *specific* use case and patch, we don't have the
resources to explore something new...
The aim here is a simple fix of 2bits that preserves the semantics of
procfs and hidepid, at same time makes the hidepid option local to
current process. It does not require or mess up with privileges,
namespaces etc. Easy to review and maintain.

Also as said in other emails we have clear use cases: some
cloud/container providers tax users for extra processes and resources
that they do not really need, we have mini jails for desktop systems
too... all this can be improved.

Thanks!

> --Andy



-- 
tixxdz
http://opendz.org

[toc] | [prev] | [next] | [standalone]

#1565274

On Mon, Jan 23, 2017 at 3:46 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
> On Sat, Jan 21, 2017 at 1:53 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> I agree that the kernel change to do it per task is very simple.  But
>> this is an unfortunate slippery slope.  What if you want to block off
>> everything in /proc that isn't associated with a PID?  What if you
>> want to suppress /sys access?  What if you want ot block *all*
>> non-current PIDs from being revealed in /proc?  What if you want to
>> hide /proc/PID/cmdline?
>
> For /sys we mount an inaccessible directory on top, we even do that
> for some static /sys and /proc inodes, of course that doesn't scale
> but we try... please see below.

So you do have a private fs namespace.

>
>> I think that the right solution here is to fix procfs to understand
>> per-superblock mount options.
>
> Unfortunately and as also noted by Lafcadio and you this is too
> complex.
> ... but lets please stay focused here, fixing procfs is a bit out of the
> scope for this *specific* use case and patch, we don't have the
> resources to explore something new...

I'm not the final authority on this (that's probably Eric), but NAK.
For upstream Linux, you can't say "doing it right is too hard so we're
going to introduce a hackish ABI with questionable security
properties".

The right solution is IMO quite clearly to fix /proc.  This isn't even
particularly hard -- there are only 17 instances of s_fs_info in
fs/proc/.

> ... Easy to review and maintain.

Au contraire.  It's a pain in the arse to review because of the
security implications and it's unpleasant to maintain because it's a
special case in core kernel code.

[toc] | [prev] | [next] | [standalone]

#1567430

On Mon, Jan 23, 2017 at 9:07 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Mon, Jan 23, 2017 at 3:46 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
>> On Sat, Jan 21, 2017 at 1:53 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> I agree that the kernel change to do it per task is very simple.  But
>>> this is an unfortunate slippery slope.  What if you want to block off
>>> everything in /proc that isn't associated with a PID?  What if you
>>> want to suppress /sys access?  What if you want ot block *all*
>>> non-current PIDs from being revealed in /proc?  What if you want to
>>> hide /proc/PID/cmdline?
>>
>> For /sys we mount an inaccessible directory on top, we even do that
>> for some static /sys and /proc inodes, of course that doesn't scale
>> but we try... please see below.
>
> So you do have a private fs namespace.

Yes for some cases and they are cheap, however I don't see the relation here ?


>>
>>> I think that the right solution here is to fix procfs to understand
>>> per-superblock mount options.
>>
>> Unfortunately and as also noted by Lafcadio and you this is too
>> complex.
>> ... but lets please stay focused here, fixing procfs is a bit out of the
>> scope for this *specific* use case and patch, we don't have the
>> resources to explore something new...
>
> I'm not the final authority on this (that's probably Eric), but NAK.

(I don't know the final authority. The *only* thing I know is that we
have technical problems here that are *not* fixed, and we try to fix
them).


> For upstream Linux, you can't say "doing it right is too hard so we're
> going to introduce a hackish ABI with questionable security
> properties".

No one said this.

Maybe you think that procfs is the right way, but I certainly can't
predict how much damage any fundamental change on procfs will make.
procfs is a special fs that has its own rules and hacks... everyone
would like to avoid major changes on it...

Could you please explain in clear words what are the benefits to use
mount, retain or regain CAP_SYS_ADMIN for something that we can set
without any privileges ?


> The right solution is IMO quite clearly to fix /proc.  This isn't even
> particularly hard -- there are only 17 instances of s_fs_info in
> fs/proc/.

pid namespaces are tied to procfs to the heart, any change on procfs
has also to count on pid namespaces, flush cached entries of a task
from each /proc... ? pid namespaces have then to be made smarter.
Forward port vulnerability and bug fixes that have been stacked in
procfs ? or at least do not break them. "procfs has always been a
special fs" by git logs. Also cases of some specific directories
/proc/fs/nfs and maybe devices id  ?
persistent of mount options and other userspace information -
https://lkml.org/lkml/2012/3/26/486

These prevent me from asserting that's the best way... where in a
previous thread you said that we should go with the easiest way to fix
the problem.


-- 
tixxdz
http://opendz.org

[toc] | [prev] | [next] | [standalone]

#1563731

On Thu, Jan 19, 2017 at 12:35 AM, Andy Lutomirski <luto@amacapital.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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web