* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: David Drysdale @ 2014-07-27 12:26 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Eric W. Biederman, Julien Tinnes, Al Viro, Paolo Bonzini,
LSM List, Greg Kroah-Hartman, Paul Moore, James Morris, Linux API,
Meredydd Luff, Christoph Hellwig,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kees Cook,
Theodore Ts'o, Henrique de Moraes Holschuh,
linux-crypto-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CALCETrWGvBfrX1W5XaFgJZhKHLf+oac9YrwPR3ppK=deQtu5hA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jul 25, 2014 at 7:30 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> [new thread because this sort of combines two threads]
>
> There is recent interest in having a way to turn generally-available
> kernel features off. Maybe we should add a good one so we can stop
> bikeshedding and avoid proliferating dumb interfaces.
>
> Things that might want to be turn-off-able include:
> - getrandom with GRND_RANDOM [from the getrandom threads]
> - Any lookup of a non-self pid [from the capsicum thread]
> - Any lookup of a pid outside the caller thread group [capsicum]
> - Various architectural things (personal wishlist), e.g.:
> - RDTSC and userspace HPET access
> - CPUID?
> - 32-bit GDT code segments [huge attack surface]
> - 64-bit GDT code segments [probably pointless]
>
> I would propose a new syscall for this:
>
> long restrict_userspace(int mode, int type, int value, int flags);
>
> mode is RESTRICT_SET, RESTRICT_GET, or RESTRICT_LOCK.
>
> type is RESTRICT_GRND_RANDOM, RESTRICT_PID_SCOPE, RESTRICT_X86_TIMING, etc.
>
> Value is zero if RESTRICT_GET. Otherwise value is the desired value,
> generally 0 or 1. For RESTRICT_PID_SCOPE, value would be
> RESTRICT_PID_SCOPE_ANY, RESTRICT_PID_SCOPE_THREADGROUP, or
> RESTRICT_PID_SCOPE_SELF.
>
> flags must be zero. Someday, someone will propose a thread-sync flag.
Today, me: proposed :-)
> restrict_userspace requires either no_new_privs or CAP_SYS_ADMIN in
> the current user namespace.
>
> Thoughts?
>
> --Andy
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: David Drysdale @ 2014-07-27 12:10 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Julien Tinnes, Kees Cook, Eric W. Biederman, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALCETrWrCU1bw+-xP_xxoRfv6L7j+GhZS_YwrWFHd2uhSp8ySw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jul 25, 2014 at 7:32 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Fri, Jul 25, 2014 at 11:22 AM, Julien Tinnes <jln-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>> On Fri, Jul 25, 2014 at 10:38 AM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>>>
>>> On Fri, Jul 25, 2014 at 10:18 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
>>> wrote:
>>> > [cc: Eric Biederman]
>>> >
>>> > On Fri, Jul 25, 2014 at 10:10 AM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>> > wrote:
>>>
>>> >> Julien had been wanting something like this too (though he'd suggested
>>> >> it via prctl): limit the signal functions to "self" only. I wonder if
>>> >> adding a prctl like done for O_BENEATH could work for signal sending?
>>> >>
>>> >
>>> >
>>> > Can we do one better and add a flag to prevent any non-self pid
>>> > lookups? This might actually be easy on top of the pid namespace work
>>> > (e.g. we could change the way that find_task_by_vpid works).
>>>
>>> Ooh, that would be extremely interesting, yes. Kind of an extreme form
>>> of pid namespace without actually being a namespace.
>>>
>>> > It's far from just being signals. There's access_process_vm, ptrace,
>>> > all the signal functions, clock_gettime (see CPUCLOCK_PID -- yes, this
>>> > is ridiculous), and probably some others that I've forgotten about or
>>> > never noticed in the first place.
>>>
>>> Yeah, that would be very interesting.
>>
>>
>> Yes, this would be incredibly useful.
>>
>> 1. For Chromium [1], I dislike relying on seccomp purely for
>> "access-control" (to other processes or files). Because it's really hard to
>> think about everything (things like CPUCLOCK_PID bite, see
>> https://crbug.com/374479).
>
> Not public :(
>
>> Se we have a first layer of sandboxing (using PID + NET namespaces and
>> chroot) for "access-control" and a second layer for kernel attack surface
>> reduction and a few other things using seccomp-bpf.
>>
>> The first layer isn't currently very good; it's heavyweight and complex (you
>> need an init(1) per namespace and that init cannot be multi-purposed as a
>> useful process because pid = 1 can never receive signals). One PID namespace
>> per process isn't something that scales well. (Also before USER_NS it
>> required a setuid root program).
>>
>> 2. Even with a safe pure seccomp-bpf sandbox that prevents sending signals
>> to other process / ptrace() et al and that restrict clock_gettime(2)
>> properly, things become quickly very tedious because as far as the kernel is
>> concerned, the process under this BPF program can still pass
>> ptrace_may_access() to other processes. This means for instance that no
>> matter what you do, a model where open() is allowed can't work if /proc is
>> available. We need a mode that says "ptrace_may_access()" will never pass.
>>
>> So yes, I really would like:
>> - a prctl that says: "I'm dropping privileges and I now can't interact with
>> other thread groups (via signals, ptrace, etc..)".
>> - Something to drop access to the file system. It could be an unprivileged
>> way to chroot() to an empty directory (unprivileged namespaces work for
>> that, - except if you're already in a chroot -). This is a little tricky
>> without allowing chroot escapes, so I suspect we would want to express it in
>> terms of mount namespace, or something else, rather than chroot.
>
> Capsicum will give you this.
Yep, that's the idea. As long as there aren't any open DFDs for "/proc" on
entry to capability mode, there shouldn't be a way to access it later -- but it
is still possible to openat(2) new files (relative to a pre-opened DFD).
> See the other thread for a more concrete proposal. prctl is getting
> out of hand.
>
> --Andy
^ permalink raw reply
* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: David Drysdale @ 2014-07-27 12:09 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Kees Cook, Eric W. Biederman, Julien Tinnes, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrVChObsQpL6dt-ByiCjbPrtpXAXQgy_apBY-OpGQHaPjg@mail.gmail.com>
On Fri, Jul 25, 2014 at 6:18 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> [cc: Eric Biederman]
>
> On Fri, Jul 25, 2014 at 10:10 AM, Kees Cook <keescook@chromium.org> wrote:
>> On Fri, Jul 25, 2014 at 8:59 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Jul 25, 2014 6:48 AM, "David Drysdale" <drysdale@google.com> wrote:
>>>>
>>>> Add the current thread and thread group IDs into the data
>>>> available for seccomp-bpf programs to work on. This allows
>>>> installation of filters that police syscalls based on thread
>>>> or process ID, e.g. tgkill(2)/kill(2)/prctl(2).
>>>>
>>>> Signed-off-by: David Drysdale <drysdale@google.com>
>>>> ---
>>>> include/uapi/linux/seccomp.h | 10 ++++++++++
>>>> kernel/seccomp.c | 2 ++
>>>> 2 files changed, 12 insertions(+)
>>>>
>>>> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
>>>> index ac2dc9f72973..b88370d6f6ca 100644
>>>> --- a/include/uapi/linux/seccomp.h
>>>> +++ b/include/uapi/linux/seccomp.h
>>>> @@ -36,12 +36,22 @@
>>>> * @instruction_pointer: at the time of the system call.
>>>> * @args: up to 6 system call arguments always stored as 64-bit values
>>>> * regardless of the architecture.
>>>> + * @tgid: thread group ID of the thread executing the BPF program.
>>>> + * @tid: thread ID of the thread executing the BPF program.
>>>> + * The SECCOMP_DATA_TID_PRESENT macro indicates the presence of the
>>>> + * tgid and tid fields; user programs may use this macro to conditionally
>>>> + * compile code against older versions of the kernel. Note also that
>>>> + * BPF programs should cope with the absence of these fields by testing
>>>> + * the length of data available.
>>>> */
>>>> struct seccomp_data {
>>>> int nr;
>>>> __u32 arch;
>>>> __u64 instruction_pointer;
>>>> __u64 args[6];
>>>> + __u32 tgid;
>>>> + __u32 tid;
>>>> };
>>>> +#define SECCOMP_DATA_TID_PRESENT 1
>>>>
>>>> #endif /* _UAPI_LINUX_SECCOMP_H */
>>>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>>>> index 301bbc24739c..dd5146f15d6d 100644
>>>> --- a/kernel/seccomp.c
>>>> +++ b/kernel/seccomp.c
>>>> @@ -80,6 +80,8 @@ static void populate_seccomp_data(struct seccomp_data *sd)
>>>> sd->args[4] = args[4];
>>>> sd->args[5] = args[5];
>>>> sd->instruction_pointer = KSTK_EIP(task);
>>>> + sd->tgid = task_tgid_vnr(current);
>>>> + sd->tid = task_pid_vnr(current);
>>>> }
>>>
>>> This is, IMO, problematic. These should probably be relative to the
>>> filter creator, not the filtered task. This will also hurt
>>> performance.
>>
>> Yeah, we can't change the seccomp_data structure without a lot of
>> care, and tgid/tid really should be encoded in the filter. However, it
>> is tricky in the forking case.
>>
>>>
>>> What's the use case? Can it be better achieved with a new eBPF function?
The specific use case is to be able to write a filter that allows kill(2)
or tgkill(2) to self, where the filter still works after forking. Capsicum
capability mode in general locks down system calls that access PIDs
(as they're a global namespace), but allows kill(self) as a pragmatic
compromise to make it easier to migrate applications to use Capsicum.
>> Julien had been wanting something like this too (though he'd suggested
>> it via prctl): limit the signal functions to "self" only. I wonder if
>> adding a prctl like done for O_BENEATH could work for signal sending?
>>
>
>
> Can we do one better and add a flag to prevent any non-self pid
> lookups? This might actually be easy on top of the pid namespace work
> (e.g. we could change the way that find_task_by_vpid works).
That sounds like a good idea, as long as it's possible for
non-CAP_SYS_ADMIN processes to do....
> It's far from just being signals. There's access_process_vm, ptrace,
> all the signal functions, clock_gettime (see CPUCLOCK_PID -- yes, this
> is ridiculous), and probably some others that I've forgotten about or
> never noticed in the first place.
For the Capsicum case in particular, most of these are restricted
by the capability mode filter anyhow (although I need to fix it for
CPUCLOCK_PID -- thanks for pointing that out); the kill(2) case
was a special case to make migrations easier. But a more general
mechanism seems sensible.
> --Andy
>
>> -Kees
>>
>> --
>> Kees Cook
>> Chrome OS Security
>
>
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH 10/11] capsicum: prctl(2) to force use of O_BENEATH
From: David Drysdale @ 2014-07-27 12:08 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Paolo Bonzini, Al Viro, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Kees Cook, Linux API, Meredydd Luff,
Christoph Hellwig, linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrUGUuYzvQtHLABh78V4fZJmwqb1Lqk-P65VUdRyrDWLug@mail.gmail.com>
On Fri, Jul 25, 2014 at 5:00 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> On Jul 25, 2014 7:02 AM, "Paolo Bonzini" <pbonzini@redhat.com> wrote:
> >
> > Il 25/07/2014 15:47, David Drysdale ha scritto:
> > > @@ -1996,6 +2013,17 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> > > if (arg2 || arg3 || arg4 || arg5)
> > > return -EINVAL;
> > > return current->no_new_privs ? 1 : 0;
> > > + case PR_SET_OPENAT_BENEATH:
> > > + if (arg2 != 1 || arg4 || arg5)
> > > + return -EINVAL;
> > > + if ((arg3 & ~(PR_SET_OPENAT_BENEATH_TSYNC)) != 0)
> > > + return -EINVAL;
> > > + error = prctl_set_openat_beneath(me, arg3);
> > > + break;
> > > + case PR_GET_OPENAT_BENEATH:
> > > + if (arg2 || arg3 || arg4 || arg5)
> > > + return -EINVAL;
> > > + return me->openat_beneath;
> > > case PR_GET_THP_DISABLE:
> > > if (arg2 || arg3 || arg4 || arg5)
> > > return -EINVAL;
> > >
> >
> > Why are you always forbidding a change of prctl from 1 to 0? It should
> > be safe if current->no_new_privs is clear.
>
> I don't immediately see why you're forbidding unsettling it at all.
> If you need it to be sticky, then use seccomp or Capsicum to make it
> sticky.
Good point, that would make the function more generic -- needing to
latch is specific to Capsicum's use of it.
>
> Also, the way implementation is dangerously racy -- if anyone pokes at
> adjacent bitfields without the lock, they can get corrupted. Try
> basing on Kees' seccomp tree or security-next and using the new atomic
> flags field.
Ah yes, sorry -- I hadn't yet shifted the implementation to line up with
the work you and Kees have put into the seccomp stuff.
>
>
> --Andy
>
> >
> > Do new threads inherit from the parent?
> >
> > Also, I wonder if you need something like this check:
> >
> > /*
> > * Installing a seccomp filter requires that the task has
> > * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
> > * This avoids scenarios where unprivileged tasks can affect the
> > * behavior of privileged children.
> > */
> > if (!current->no_new_privs &&
> > security_capable_noaudit(current_cred(), current_user_ns(),
> > CAP_SYS_ADMIN) != 0)
> > return -EACCES;
> >
> > Paolo
Yes, new threads inherit the flag from the parent so the
NNP||CAP_SYS_ADMIN check is probably needed.
^ permalink raw reply
* Re: [RFC PATCH 1/1] ethtool: adding support for multiple slave port configuration
From: Ben Hutchings @ 2014-07-27 2:47 UTC (permalink / raw)
To: Mugunthan V N
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1406291305-22286-1-git-send-email-mugunthanvnm-l0cyMroinI0@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2779 bytes --]
On Fri, 2014-07-25 at 17:58 +0530, Mugunthan V N wrote:
> Some Ethernet Swtich controllers like CPSW in AM335x, TI814x, DRA7x and
> AM43xx SoCs, Network Coprocessor in AM5K2E0x, Realtek Switch controllers
> etc has to capability of conneting multiple networks using L2 switching
> and has multiple phys. With the existing code, ethtool can communicate
> only to one phy.
>
> To enable user to communicate multiple phy connected to single Ethernet
> Switch controller, intoducing a optional new parameter in Ethtool interface
> to pass which slave to set/get the phy configuration.
There was some discussion about configuration APIs for hardware/firmware
bridges earlier this year and I thought there was a consensus for
assigning a network device to each port. This would remove the need to
identify ports within a device. But I may have misremembered.
> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
> ---
> include/uapi/linux/ethtool.h | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 96ade34..3011427 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -60,6 +60,9 @@
> * and other link features that the link partner advertised
> * through autonegotiation; 0 if unknown or not applicable.
> * Read-only.
> + * @slave_port: Specify which slave port to be used to set/get
> + * parmeters, for example which slave port phy to be used for
> + * set/get phy capabilities
The difficulty with assigning the reserved fields in struct ethtool_cmd
is that nothing has ever checked that they are set to 0. So if we were
to assign this field and support it in ethtool, someone might run it on
an older kernel version and all configuration changes will be made to
port 0 rather than the one they specified. I don't think it would be
acceptable to tell users that 'oh, the port number option silently fails
on older kernel versions'.
So at the very least you would also need to add some way for userland to
find out whether the driver will check the value of this field.
Ben.
> * The link speed in Mbps is split between @speed and @speed_hi. Use
> * the ethtool_cmd_speed() and ethtool_cmd_speed_set() functions to
> @@ -107,7 +110,8 @@ struct ethtool_cmd {
> __u8 eth_tp_mdix;
> __u8 eth_tp_mdix_ctrl;
> __u32 lp_advertising;
> - __u32 reserved[2];
> + __u32 slave_port;
> + __u32 reserved;
> };
>
> static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
--
Ben Hutchings
The generation of random numbers is too important to be left to chance.
- Robert Coveyou
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [RFC PATCHv2 00/11] Adding FreeBSD's Capsicum security framework
From: Eric W. Biederman @ 2014-07-26 21:04 UTC (permalink / raw)
To: David Drysdale
Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
Alexander Viro, Meredydd Luff, Kees Cook, James Morris,
Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1406296033-32693-1-git-send-email-drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> writes:
> The last couple of versions of FreeBSD (9.x/10.x) have included the
> Capsicum security framework [1], which allows security-aware
> applications to sandbox themselves in a very fine-grained way. For
> example, OpenSSH now (>= 6.5) uses Capsicum in its FreeBSD version to
> restrict sshd's credentials checking process, to reduce the chances of
> credential leakage.
>
> It would be good to have equivalent functionality in Linux, so I've been
> working on getting the Capsicum framework running in the kernel, and I'd
> appreciate some feedback/opinions on the general approach.
>
> I'm attaching a corresponding draft patchset for reference, but
> hopefully this cover email can cover the significant features to save
> everyone having to look through the code details. (It does mean this is
> a long email though -- apologies for that.)
>
>
> 1) Capsicum Capabilities
> ------------------------
>
> The most significant aspect of Capsicum is associating *rights* with
> (some) file descriptors, so that the kernel only allows operations on an
> FD if the rights permit it. This allows userspace applications to
> sandbox themselves by tightly constraining what's allowed with both
> input and outputs; for example, tcpdump might restrict itself so it can
> only read from the network FD, and only write to stdout.
>
> The kernel thus needs to police the rights checks for these file
> descriptors (referred to as 'Capsicum capabilities', completely
> different than POSIX.1e capabilities), and the best place to do this is
> at the points where a file descriptor from userspace is converted to a
> struct file * within the kernel.
>
> [Policing the rights checks anywhere else, for example at the system
> call boundary, isn't a good idea because it opens up the possibility
> of time-of-check/time-of-use (TOCTOU) attacks [2] where FDs are
> changed (as openat/close/dup2 are allowed in capability mode) between
> the 'check' at syscall entry and the 'use' at fget() invocation.]
>
> However, this does lead to quite an invasive change to the kernel --
> every invocation of fget() or similar functions (fdget(),
> sockfd_lookup(), user_path_at(),...) needs to be annotated with the
> rights associated with the specific operations that will be performed on
> the struct file. There are ~100 such invocations that need
> annotation.
And it is silly. Roughly you just need a locking version of
fcntl(F_SETFL).
That is make the restriction in the struct file not in the fd to file
lookup.
Files in unix have been capabilities for more than 20 years. That is
what file descriptor passing in unix domain sockets are all about. We
don't need an additional ``capability rights'' layer on top of file
descriptors.
In fact internal to linux with FMODE_READ and friends we already have
restrictions on which methods are allowed on linux file descriptors.
So this whole entire abstraction layer you are adding seems just plain
broken.
Going farther one huge thing your work and the capsicum work in general
is missing is an implementation of revoke. With a little care a good
implementation of bits reporting and controlling which methods are
available on which file descriptors should be a good start on
a revoke implementation for linux as well.
> 2) Capsicum Capabilities Data Structure
> ---------------------------------------
>
> Internally, the rights associated with a Capsicum capability FD are
> stored in a special struct file wrapper. For a normal file, the rights
> check inside fget() falls through, but for a capability wrapper the
> rights in the wrapper are checked and (if capable) the underlying
> wrapped struct file is returned.
>
> [This is approximately the implementation that was present in FreeBSD
> 9.x. For FreeBSD 10.x, the wrapper file was removed and the rights
> associated with a file descriptor are now stored in the fdtable. As
> that impacts memory use for all processes, whether Capsicum users or
> not, I've stuck with the FreeBSD 9.x approach.]
I have already mentioned that this is an insane choice of semantics
right? Adding an extra layer on top of a data structure that is a
perfectly good restrictor of rights.
If you can't add the restriction on struct file itself I would argue
that the semantics of capsicum are fundamentally broken.
I can not imagine why in the world you would want and extra layer of
indirection, complication, and maintenance.
> 3) Allowing Capability Mode
> ---------------------------
>
> Capsicum also includes 'capability mode', which locks down the available
> syscalls so the rights restrictions can't just be bypassed by opening
> new file descriptors. More precisely, capability mode prevents access
> to syscalls that access global namespaces, such as the filesystem or the
> IP:port space.
>
> The existing seccomp-bpf functionality of the kernel is a good mechanism
> for implementing capability mode, but there are a few additional details
> that also need to be addressed.
>
> a) The capability mode filter program needs to apply process-wide, not
> just to the current thread.
>
> b) In capability mode, new files can still be opened with openat(2) but
> only if they are beneath an existing directory file descriptor.
Which raises the question is that worth it?
> c) In capability mode it should still be possible for a process to send
> signals to itself with kill(2)/tgkill(2).
Again is it worth it?
I would think you would want capability mode to default to the minium
set of system calls you could get away with (to keep kernel code
auditing to a minium) and only add things if the performance gain of
using the syscall exceeds the pain.
If you look at a kernel like sel4 it succeeds in with an object
capability model with many fewer system calls than you are proposing
to export. Roughly just read, write and close.
Consider the fact if you really want a kernel layer you can completely
trust and rely on someone needs to write a formal proof of that layer.
Short of that someone certainly needs to audit the kernel code very
closely so simplicity of semantics and simplicity of implementation are
very important.
> This v2 patchset copes with these as follows:
>
> a) Kees Cook's incoming seccomp(2) patchset covers thread
> synchronization of filters.
>
> b) A new prctl(PR_SET_OPENAT_BENEATH) operation implicitly sets the
> O_BENEATH flag (see below) for all file-open operations for all
> threads of the current process, by adding a new openat_beneath
> flag in task_struct.
>
> c) An extension to the seccomp_data structure that includes the current
> task's tid and tgid values allows for BPF programs that check a
> kill(2)/tgkill(2) argument against the current thread, in a manner
> that is robust against fork(2)/clone(2).
>
> The combination of these features with the existing seccomp-bpf
> functionality gives the tools needed to implement capability mode.
>
>
> 4) New System Calls
> -------------------
>
> To allow userspace applications to access the Capsicum capability
> functionality, I'm proposing two new system calls: cap_rights_limit(2)
> and cap_rights_get(2). I guess these could potentially be implemented
> elsewhere (e.g. as fcntl(2) operations?) but the changes seem
> significant enough that new syscalls are warranted.
>
> [FreeBSD 10.x actually includes six new syscalls for manipulating the
> rights associated with a Capsicum capability -- the capability rights
> can police that only specific fcntl(2) or ioctl(2) commands are
> allowed, and FreeBSD sets these with distinct syscalls.]
ioctls? In a sandbox? Ick.
> 5) New openat(2) O_BENEATH Flag
> -------------------------------
>
> For Capsicum capabilities that are directory file descriptors, the
> Capsicum framework only allows openat(cap_dfd, path, ...) operations to
> work for files that are beneath the specified directory (and even that
> only when the directory FD has the CAP_LOOKUP right), rejecting paths
> that start with "/" or include "..". The same restriction applies
> process-wide for a process in capability mode.
>
> As this seemed like functionality that might be more generally useful,
> I've implemented it independently as a new O_BENEATH flag for openat(2).
> The Capsicum code then always triggers the use of that flag when the dfd
> is a Capsicum capability, or when the prctl(2) command described above
> is in play.
>
> [FreeBSD has the openat(2) relative-only behaviour for capability DFDs
> and processes in capability mode, but does not include the O_BENEATH
> flag.]
If you are going to allow open I would think the simple solution here is
to just create a mount namespace that only has available the files you
would like to export to the process, and force all of the file
descriptors into that mount namespace.
Eric
^ permalink raw reply
* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: Eric W. Biederman @ 2014-07-26 19:48 UTC (permalink / raw)
To: Andy Lutomirski
Cc: One Thousand Gnomes, Julien Tinnes, David Drysdale, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel@vger.kernel.org, Kees Cook, Theodore Ts'o,
Henrique de Moraes Holschuh, linux-crypto
In-Reply-To: <CALCETrXHNFYaQjksBJyN3O0HJpKw_D_tU7b4O23=nyJt9CPSLg@mail.gmail.com>
Andy Lutomirski <luto@amacapital.net> writes:
> On Fri, Jul 25, 2014 at 2:35 PM, One Thousand Gnomes
> <gnomes@lxorguk.ukuu.org.uk> wrote:
>> On Fri, 25 Jul 2014 11:30:48 -0700
>> Andy Lutomirski <luto@amacapital.net> wrote:
>>
>>> [new thread because this sort of combines two threads]
>>>
>>> There is recent interest in having a way to turn generally-available
>>> kernel features off. Maybe we should add a good one so we can stop
>>> bikeshedding and avoid proliferating dumb interfaces.
>>
>> We sort of have one. It's called capable(). Just needs extending to cover
>> anything else you care about, and probably all the numeric constants
>> replacing with textual names.
The big difference is capable only subdivides roots powers (aka things
most applications should not have). When we start talking about things
that things that are safe for most applications capable is probably not
the right tool for the job.
A much closer match is the personality system call. Look at setarch
to see how it is used. My biggest concern with personality is there
are only 32bits to play with. Still I expect what you want may be a
sandbox personality, that disables everything that could possibly be
a problem (including access to the personality syscall).
Eric
^ permalink raw reply
* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: Andy Lutomirski @ 2014-07-25 23:54 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Eric W. Biederman, Julien Tinnes, David Drysdale, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel@vger.kernel.org, Kees Cook, Theodore Ts'o,
Henrique de Moraes Holschuh, linux-crypto
In-Reply-To: <53D2EB98.3090802@zytor.com>
On Fri, Jul 25, 2014 at 4:43 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 07/25/2014 11:30 AM, Andy Lutomirski wrote:
>> - 32-bit GDT code segments [huge attack surface]
>> - 64-bit GDT code segments [probably pointless]
>
> I presume you mean s/GDT/LDT/.
>
> We already don't allow 64-bit LDT code segments. Also, it is unclear to
> me how 32-bit LDT segments have a huge attack surface, given that there
> will realistically always be a 32-bit *GDT* segment present.
I really did mean GDT :) Setting the 32-bit code segment to "not
present" (and using seccomp to block modify_ldt) prevents any attempt
to exploit bugs in the sysenter and cstar code. It also might prevent
exploiting CPU bugs, although I've never heard of a relevant CPU bug
in this area.
If I actually tried to implement this (which wouldn't be part of the
initial implementation), I'd split out the unusual things in
__switch_to and friends to a slow path that's only used if weird
settings are present (e.g. this, TSC restrictions, etc). But
twiddling the present bit on a GDT entry is very fast, I assume --
it's just memory, and I don't think that any flush is needed.
Also, if I implement this, I will curse Xen. I might even go so far
as to disable the feature entirely if there's a paravirt GDT.
Hmm. A separate flag to turn int $0x80 into GPF could have some value, too.
--Andy
^ permalink raw reply
* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: H. Peter Anvin @ 2014-07-25 23:43 UTC (permalink / raw)
To: Andy Lutomirski, Eric W. Biederman, Julien Tinnes, David Drysdale,
Al Viro, Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kees Cook,
Theodore Ts'o, Henrique de Moraes Holschuh,
linux-crypto-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CALCETrWGvBfrX1W5XaFgJZhKHLf+oac9YrwPR3ppK=deQtu5hA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 07/25/2014 11:30 AM, Andy Lutomirski wrote:
> - 32-bit GDT code segments [huge attack surface]
> - 64-bit GDT code segments [probably pointless]
I presume you mean s/GDT/LDT/.
We already don't allow 64-bit LDT code segments. Also, it is unclear to
me how 32-bit LDT segments have a huge attack surface, given that there
will realistically always be a 32-bit *GDT* segment present.
-hpa
^ permalink raw reply
* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: Andy Lutomirski @ 2014-07-25 21:41 UTC (permalink / raw)
To: One Thousand Gnomes
Cc: Eric W. Biederman, Julien Tinnes, David Drysdale, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel@vger.kernel.org, Kees Cook, Theodore Ts'o,
Henrique de Moraes Holschuh, linux-crypto
In-Reply-To: <20140725223550.3153f436@alan.etchedpixels.co.uk>
On Fri, Jul 25, 2014 at 2:35 PM, One Thousand Gnomes
<gnomes@lxorguk.ukuu.org.uk> wrote:
> On Fri, 25 Jul 2014 11:30:48 -0700
> Andy Lutomirski <luto@amacapital.net> wrote:
>
>> [new thread because this sort of combines two threads]
>>
>> There is recent interest in having a way to turn generally-available
>> kernel features off. Maybe we should add a good one so we can stop
>> bikeshedding and avoid proliferating dumb interfaces.
>
> We sort of have one. It's called capable(). Just needs extending to cover
> anything else you care about, and probably all the numeric constants
> replacing with textual names.
>
Except that it's all backwards: these are things that default to *on*,
and people might want them to turn off. capable() is totally fscked
if you want otherwise unprivileged users to carry capabilities around,
and fixing it seems to run into endless claims that the "capability"
system is carefully designed, flawless, perfect, ideal, amazing, and
shouldn't be changed, despite the fact that it's empirically damn near
useless.
Also, capabilities do the wrong thing wrt namespaces. The things I'm
talking about aren't namespaced. They're either on or off.
--Andy
^ permalink raw reply
* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: One Thousand Gnomes @ 2014-07-25 21:35 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Eric W. Biederman, Julien Tinnes, David Drysdale, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kees Cook,
Theodore Ts'o, Henrique de Moraes Holschuh,
linux-crypto-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CALCETrWGvBfrX1W5XaFgJZhKHLf+oac9YrwPR3ppK=deQtu5hA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, 25 Jul 2014 11:30:48 -0700
Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> [new thread because this sort of combines two threads]
>
> There is recent interest in having a way to turn generally-available
> kernel features off. Maybe we should add a good one so we can stop
> bikeshedding and avoid proliferating dumb interfaces.
We sort of have one. It's called capable(). Just needs extending to cover
anything else you care about, and probably all the numeric constants
replacing with textual names.
Alan
^ permalink raw reply
* Re: [PATCH 0/5] RFC: CGroup Namespaces
From: Andy Lutomirski @ 2014-07-25 20:27 UTC (permalink / raw)
To: Aditya Kali
Cc: Serge Hallyn, Tejun Heo, Li Zefan, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
Ingo Molnar, Linux Containers
In-Reply-To: <CAGr1F2GcAema-E2q6PFj=R0Z505iD7JshrMuMdfPTJ95wMiQMA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jul 25, 2014 at 12:29 PM, Aditya Kali <adityakali-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> Thank you for your review. I have tried to respond to both your emails here.
>
> On Thu, Jul 24, 2014 at 9:36 AM, Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org> wrote:
>> 2. What would be the reprecussions of allowing cgroupns unshare so
>> long as you have ns_capable(CAP_SYS_ADMIN) to the user_ns which
>> created your current ns cgroup? It'd be a shame if that wasn't
>> on the roadmap.
>>
>
> Its certainly on the roadmap, just that some logistics were not clear
> at this time. As pointed out by Andy Lutomirski on [PATCH 5/5] of this
> series, if we allow cgroupns creation to ns_capable(CAP_SYS_ADMIN)
> processes, we may need some kind of explicit permission from the
> cgroup subsystem to allow this. One approach could be an explicit
> cgroup.may_unshare setting. Alternatively, the cgroup directory (which
> is going to become the cgroupns-root) ownership could also be used
> here. i.e., the process is ns_capable(CAP_SYS_ADMIN) && it owns the
> cgroup directory. There seems to be already a function that allows
> similar thing and might be sufficient:
>
> /**
> * capable_wrt_inode_uidgid - Check nsown_capable and uid and gid mapped
> * @inode: The inode in question
> * @cap: The capability in question
> *
> * Return true if the current task has the given capability targeted at
> * its own user namespace and that the given inode's uid and gid are
> * mapped into the current user namespace.
> */
> bool capable_wrt_inode_uidgid(const struct inode *inode, int cap)
>
> What do you think? We can enable this for non-init userns once this is
> decided on.
>
I think I'd rather it just check that it's owned by the userns owner
if we were going down that route. But maybe there's a good reason to
do it this way.
>
>> 3. The un-namespaced view of /proc/self/cgroup from a sibling cgroupns
>> makes me wonder whether it wouldn't be more appropriate to leave
>> /proc/self/cgroup always un-filtered, and use /proc/self/nscgroup
>> (or somesuch) to provide the namespaced view. /proc/self/nscgroup
>> would simply be empty (or say (invalid) or (unreachable)) from a
>> sibling ns. That will give criu and admin tools like lxc/docker all
>> they need to do simple cgroup setup.
>>
>
> It may work for lxc/docker and new applications that use the new
> interface. But its difficult to change numerous existing user
> applications and libraries that depend on /proc/self/cgroup. Moreover,
> even with the new interface, /proc/self/cgroup will continue to leak
> system level cgroup information. And fixing this leak is critical to
> make the container migratable.
>
> Its easy to correctly handle the read of /proc/<pid>/cgroup from a
> sibling cgroupns. Instead of showing unfiltered view, we could just
> not show anything (same behavior when the cgroup hierarchy is not
> mounted). Will that be more acceptable? I can make that change in the
> next version of this series.
>
>
>>> (5) setns() is not supported for cgroup namespace in the initial
>>> version.
>>
>> This combined with the full-path reporting for peer ns cgroups could make
>> for fun antics when attaching to an existing container (since we'd have
>> to unshare into a new ns cgroup with the same roto as the container).
>> I understand you are implying this will be fixed soon though.
>>
>
> I am thinking the setns() will be only allowed if
> target_cgrpns->cgroupns_root is_descendant_of
> current_cgrpns->cgroupns_root. i.e., you will only be setns to a
> cgroup namespace which is rooted deeper in hierarchy than your own (in
> addition to checking capable_wrt_inode_uidgid(target_cgrpns_inode)).
I'm not sure why the capable_wrt_inode_uidgid is needed here -- I
imagine that the hierarchy check and the usual CAP_SYS_ADMIN check on
the cgroupns's userns would be sufficient.
>
> In addition to this, we need to decide whether its OK for setns() to
> also change the cgroup of the task. Consider following example:
>
> [A] ----> [B] ----> C
> ----> D
>
> [A] and [B] are cgroupns-roots. Now, if a task in Cgroup D (which is
> under cgroupns [A]) attempts to setns() to cgroupns [B], then its
> cgroup should change from /A/D to /A/B. I am concerned about the
> side-effects this might cause. Though otherwise, this is a very useful
> feature for containers. One could argue that this is similar to
> setns() to a mount-namespace which is pivot_root'd somewhere else (in
> which case, the attaching task's root "/" moves implicitly with
> setns).
Off the top of my head, I think that making setns do this would be too
magical. How about just requiring that you already be in (a
descendent of) the requested cgroupns's root cgroup if you try to
setns?
>
> Alternatively, we could only allow setns() if
> target_cgrpns->cgroupns_root == current->cgroup . I.e., taking above
> example again, if process in Cgroup D wants to setns() to cgroupns
> [B], then it will first need to move to Cgroup B, and only then the
> setns() will succeed. This makes sure that there is no implicit cgroup
> move.
I like this one, but I think that descendant cgroups should probably
be allowed, too.
--Andy
^ permalink raw reply
* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: Andy Lutomirski @ 2014-07-25 20:21 UTC (permalink / raw)
To: Dave Jones, Andy Lutomirski, Eric W. Biederman, Julien Tinnes,
David Drysdale, Al Viro, Paolo Bonzini, LSM List,
Greg Kroah-Hartman, Paul Moore, James Morris, Linux API,
Meredydd Luff, Christoph Hellwig,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kees Cook,
Theodore Ts'o, Henrique de Moraes Holschuh,
linux-crypto-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140725201507.GA23410-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Fri, Jul 25, 2014 at 1:15 PM, Dave Jones <davej-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, Jul 25, 2014 at 11:30:48AM -0700, Andy Lutomirski wrote:
>
> > There is recent interest in having a way to turn generally-available
> > kernel features off. Maybe we should add a good one so we can stop
> > bikeshedding and avoid proliferating dumb interfaces.
> >
> > Things that might want to be turn-off-able include:
> > - getrandom with GRND_RANDOM [from the getrandom threads]
> > - Any lookup of a non-self pid [from the capsicum thread]
> > - Any lookup of a pid outside the caller thread group [capsicum]
> > - Various architectural things (personal wishlist), e.g.:
> > - RDTSC and userspace HPET access
> > - CPUID?
> > - 32-bit GDT code segments [huge attack surface]
> > - 64-bit GDT code segments [probably pointless]
>
> I'm not sure there's value in disabling cpuid dev interface,
> when the instruction is unprivileged.
I meant the CPUID instruction. Some CPUs have a setting that turns
off the CPUID instruction for user code. In principle, all VMs can do
this, too, if the hypervisor would be kind enough to help out.
I only mentioned the x86 stuff here to make the point that there are
quite a few possibilities along these lines. There's actually already
a way to turn off RDTSC, but it's not currently very useful because it
doesn't do the right thing for the vDSO. That could be fixed, but
there's certainly no reason to make any of the other stuff here wait
for that.
>
> > I would propose a new syscall for this:
> >
> > long restrict_userspace(int mode, int type, int value, int flags);
>
> do the restrictions happen system-wide like in say SELinux,
> or only within the calling process, like seccomp ?
>
The calling process and children, like seccomp.
--Andy
^ permalink raw reply
* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: Dave Jones @ 2014-07-25 20:15 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Eric W. Biederman, Julien Tinnes, David Drysdale, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel@vger.kernel.org, Kees Cook, Theodore Ts'o,
Henrique de Moraes Holschuh, linux-crypto
In-Reply-To: <CALCETrWGvBfrX1W5XaFgJZhKHLf+oac9YrwPR3ppK=deQtu5hA@mail.gmail.com>
On Fri, Jul 25, 2014 at 11:30:48AM -0700, Andy Lutomirski wrote:
> There is recent interest in having a way to turn generally-available
> kernel features off. Maybe we should add a good one so we can stop
> bikeshedding and avoid proliferating dumb interfaces.
>
> Things that might want to be turn-off-able include:
> - getrandom with GRND_RANDOM [from the getrandom threads]
> - Any lookup of a non-self pid [from the capsicum thread]
> - Any lookup of a pid outside the caller thread group [capsicum]
> - Various architectural things (personal wishlist), e.g.:
> - RDTSC and userspace HPET access
> - CPUID?
> - 32-bit GDT code segments [huge attack surface]
> - 64-bit GDT code segments [probably pointless]
I'm not sure there's value in disabling cpuid dev interface,
when the instruction is unprivileged.
> I would propose a new syscall for this:
>
> long restrict_userspace(int mode, int type, int value, int flags);
do the restrictions happen system-wide like in say SELinux,
or only within the calling process, like seccomp ?
Dave
^ permalink raw reply
* Re: [PATCH 0/5] RFC: CGroup Namespaces
From: Aditya Kali @ 2014-07-25 19:29 UTC (permalink / raw)
To: Serge Hallyn
Cc: Tejun Heo, Li Zefan, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
Ingo Molnar, Linux Containers, Andy Lutomirski
In-Reply-To: <20140724163628.GN26600@ubuntumail>
Thank you for your review. I have tried to respond to both your emails here.
On Thu, Jul 24, 2014 at 9:36 AM, Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org> wrote:
> Quoting Aditya Kali (adityakali-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org):
>> Background
>> Cgroups and Namespaces are used together to create “virtual”
>> containers that isolates the host environment from the processes
>> running in container. But since cgroups themselves are not
>> “virtualized”, the task is always able to see global cgroups view
>> through cgroupfs mount and via /proc/self/cgroup file.
>>
> Hi,
>
> A few questions/comments:
>
> 1. Based on this description, am I to understand that after doing a
> cgroupns unshare, 'mount -t cgroup cgroup /mnt' by default will
> still mount the global root cgroup? Any plans on "changing" that?
This is suggested in the "Possible Extensions of CGROUPNS" section.
More details below.
> Will attempts to change settings of a cgroup which is not under
> our current ns be rejected? (That should be easy to do given your
> patch 1/5). Sorry if it's done in the set, I'm jumping around...
>
Currently, only 'cgroup_attach_task', 'cgroup_mkdir' and
'cgroup_rmdir' of cgroups outside of cgroupns-root are prevented. The
read/write of actual cgroup properties are not prevented. Usual
permission checks continue to apply for those. I was hoping that
should be enough, but see more comments towards the end.
> 2. What would be the reprecussions of allowing cgroupns unshare so
> long as you have ns_capable(CAP_SYS_ADMIN) to the user_ns which
> created your current ns cgroup? It'd be a shame if that wasn't
> on the roadmap.
>
Its certainly on the roadmap, just that some logistics were not clear
at this time. As pointed out by Andy Lutomirski on [PATCH 5/5] of this
series, if we allow cgroupns creation to ns_capable(CAP_SYS_ADMIN)
processes, we may need some kind of explicit permission from the
cgroup subsystem to allow this. One approach could be an explicit
cgroup.may_unshare setting. Alternatively, the cgroup directory (which
is going to become the cgroupns-root) ownership could also be used
here. i.e., the process is ns_capable(CAP_SYS_ADMIN) && it owns the
cgroup directory. There seems to be already a function that allows
similar thing and might be sufficient:
/**
* capable_wrt_inode_uidgid - Check nsown_capable and uid and gid mapped
* @inode: The inode in question
* @cap: The capability in question
*
* Return true if the current task has the given capability targeted at
* its own user namespace and that the given inode's uid and gid are
* mapped into the current user namespace.
*/
bool capable_wrt_inode_uidgid(const struct inode *inode, int cap)
What do you think? We can enable this for non-init userns once this is
decided on.
> 3. The un-namespaced view of /proc/self/cgroup from a sibling cgroupns
> makes me wonder whether it wouldn't be more appropriate to leave
> /proc/self/cgroup always un-filtered, and use /proc/self/nscgroup
> (or somesuch) to provide the namespaced view. /proc/self/nscgroup
> would simply be empty (or say (invalid) or (unreachable)) from a
> sibling ns. That will give criu and admin tools like lxc/docker all
> they need to do simple cgroup setup.
>
It may work for lxc/docker and new applications that use the new
interface. But its difficult to change numerous existing user
applications and libraries that depend on /proc/self/cgroup. Moreover,
even with the new interface, /proc/self/cgroup will continue to leak
system level cgroup information. And fixing this leak is critical to
make the container migratable.
Its easy to correctly handle the read of /proc/<pid>/cgroup from a
sibling cgroupns. Instead of showing unfiltered view, we could just
not show anything (same behavior when the cgroup hierarchy is not
mounted). Will that be more acceptable? I can make that change in the
next version of this series.
>> $ cat /proc/self/cgroup
>> 0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
>>
>> This exposure of cgroup names to the processes running inside a
>> container results in some problems:
>> (1) The container names are typically host-container-management-agent
>> (systemd, docker/libcontainer, etc.) data and leaking its name (or
>> leaking the hierarchy) reveals too much information about the host
>> system.
>> (2) It makes the container migration across machines (CRIU) more
>> difficult as the container names need to be unique across the
>> machines in the migration domain.
>> (3) It makes it difficult to run container management tools (like
>> docker/libcontainer, lmctfy, etc.) within virtual containers
>> without adding dependency on some state/agent present outside the
>> container.
>>
>> Note that the feature proposed here is completely different than the
>> “ns cgroup” feature which existed in the linux kernel until recently.
>> The ns cgroup also attempted to connect cgroups and namespaces by
>> creating a new cgroup every time a new namespace was created. It did
>> not solve any of the above mentioned problems and was later dropped
>> from the kernel.
>>
>> Introducing CGroup Namespaces
>> With unified cgroup hierarchy
>> (Documentation/cgroups/unified-hierarchy.txt), the containers can now
>> have a much more coherent cgroup view and its easy to associate a
>> container with a single cgroup. This also allows us to virtualize the
>> cgroup view for tasks inside the container.
>>
>> The new CGroup Namespace allows a process to “unshare” its cgroup
>> hierarchy starting from the cgroup its currently in.
>> For Ex:
>> $ cat /proc/self/cgroup
>> 0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
>> $ ls -l /proc/self/ns/cgroup
>> lrwxrwxrwx 1 root root 0 2014-07-15 10:37 /proc/self/ns/cgroup -> cgroup:[4026531835]
>> $ ~/unshare -c # calls unshare(CLONE_NEWCGROUP) and exec’s /bin/bash
>> [ns]$ ls -l /proc/self/ns/cgroup
>> lrwxrwxrwx 1 root root 0 2014-07-15 10:35 /proc/self/ns/cgroup -> cgroup:[4026532183]
>> # From within new cgroupns, process sees that its in the root cgroup
>> [ns]$ cat /proc/self/cgroup
>> 0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/
>>
>> # From global cgroupns:
>> $ cat /proc/<pid>/cgroup
>> 0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
>>
>> The virtualization of /proc/self/cgroup file combined with restricting
>> the view of cgroup hierarchy by bind-mounting for the
>> $CGROUP_MOUNT/batchjobs/c_job_id1/ directory to
>> $CONTAINER_CHROOT/sys/fs/cgroup/) should provide a completely isolated
>> cgroup view inside the container.
>>
>> In its current simplistic form, the cgroup namespaces provide
>> following behavior:
>>
>> (1) The “root” cgroup for a cgroup namespace is the cgroup in which
>> the process calling unshare is running.
>> For ex. if a process in /batchjobs/c_job_id1 cgroup calls unshare,
>> cgroup /batchjobs/c_job_id1 becomes the cgroupns-root.
>> For the init_cgroup_ns, this is the real root (“/”) cgroup
>> (identified in code as cgrp_dfl_root.cgrp).
>>
>> (2) The cgroupns-root cgroup does not change even if the namespace
>> creator process later moves to a different cgroup.
>> $ ~/unshare -c # unshare cgroupns in some cgroup
>> [ns]$ cat /proc/self/cgroup
>> 0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/
>> [ns]$ mkdir sub_cgrp_1
>> [ns]$ echo 0 > sub_cgrp_1/cgroup.procs
>> [ns]$ cat /proc/self/cgroup
>> 0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/sub_cgrp_1
>>
>> (3) Each process gets its CGROUPNS specific view of
>> /proc/<pid>/cgroup.
>> (a) Processes running inside the cgroup namespace will be able to see
>> cgroup paths (in /proc/self/cgroup) only inside their root cgroup
>> [ns]$ sleep 100000 & # From within unshared cgroupns
>> [1] 7353
>> [ns]$ echo 7353 > sub_cgrp_1/cgroup.procs
>> [ns]$ cat /proc/7353/cgroup
>> 0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/sub_cgrp_1
>>
>> (b) From global cgroupns, the real cgroup path will be visible:
>> $ cat /proc/7353/cgroup
>> 0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1/sub_cgrp_1
>>
>> (c) From a sibling cgroupns, the real path will be visible:
>> [ns2]$ cat /proc/7353/cgroup
>> 0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1/sub_cgrp_1
>> (In correct container setup though, it should not be possible to
>> access PIDs in another container in the first place. This can be
>> detected changed if desired.)
>>
>> (4) Processes inside a cgroupns are not allowed to move out of the
>> cgroupns-root. This is true even if a privileged process in global
>> cgroupns tries to move the process out of its cgroupns-root.
>>
>> # From global cgroupns
>> $ cat /proc/7353/cgroup
>> 0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1/sub_cgrp_1
>> # cgroupns-root for 7353 is /batchjobs/c_job_id1
>> $ echo 7353 > batchjobs/c_job_id2/cgroup.procs
>> -bash: echo: write error: Operation not permitted
>>
>> (5) setns() is not supported for cgroup namespace in the initial
>> version.
>
> This combined with the full-path reporting for peer ns cgroups could make
> for fun antics when attaching to an existing container (since we'd have
> to unshare into a new ns cgroup with the same roto as the container).
> I understand you are implying this will be fixed soon though.
>
I am thinking the setns() will be only allowed if
target_cgrpns->cgroupns_root is_descendant_of
current_cgrpns->cgroupns_root. i.e., you will only be setns to a
cgroup namespace which is rooted deeper in hierarchy than your own (in
addition to checking capable_wrt_inode_uidgid(target_cgrpns_inode)).
In addition to this, we need to decide whether its OK for setns() to
also change the cgroup of the task. Consider following example:
[A] ----> [B] ----> C
----> D
[A] and [B] are cgroupns-roots. Now, if a task in Cgroup D (which is
under cgroupns [A]) attempts to setns() to cgroupns [B], then its
cgroup should change from /A/D to /A/B. I am concerned about the
side-effects this might cause. Though otherwise, this is a very useful
feature for containers. One could argue that this is similar to
setns() to a mount-namespace which is pivot_root'd somewhere else (in
which case, the attaching task's root "/" moves implicitly with
setns).
Alternatively, we could only allow setns() if
target_cgrpns->cgroupns_root == current->cgroup . I.e., taking above
example again, if process in Cgroup D wants to setns() to cgroupns
[B], then it will first need to move to Cgroup B, and only then the
setns() will succeed. This makes sure that there is no implicit cgroup
move.
WDYT? I haven't prototyped this yet, but will send out a patch after
this series is accepted.
>> (6) When some thread from a multi-threaded process unshares its
>> cgroup-namespace, the new cgroupns gets applied to the entire
>> process (all the threads). This should be OK since
>> unified-hierarchy only allows process-level containerization. So
>> all the threads in the process will have the same cgroup. And both
>> - changing cgroups and unsharing namespaces - are protected under
>> threadgroup_lock(task).
>>
>> (7) The cgroup namespace is alive as long as there is atleast 1
>> process inside it. When the last process exits, the cgroup
>> namespace is destroyed. The cgroupns-root and the actual cgroups
>> remain though.
>>
>> Implementation
>> The current patch-set is based on top of Tejun's cgroup tree (for-next
>> branch). Its fairly non-intrusive and provides above mentioned
>> features.
>>
>> Possible extensions of CGROUPNS:
>> (1) The Documentation/cgroups/unified-hierarchy.txt mentions use of
>> capabilities to restrict cgroups to administrative users. CGroup
>> namespaces could be of help here. With cgroup namespaces, it might
>> be possible to delegate administration of sub-cgroups under a
>> cgroupns-root to the cgroupns owner.
>
> That would be nice.
>
>> (2) Provide a cgroupns specific cgroupfs mount. i.e., the following
>> command when ran from inside a cgroupns should only mount the
>> hierarchy from cgroupns-root cgroup:
>> $ mount -t cgroup cgroup <cgroup-mountpoint>
>> # -o __DEVEL__sane_behavior should be implicit
>>
>> This is similar to how procfs can be mounted for every PIDNS. This
>> may have some usecases.
>
> Sorry - I see this answers the first part of a question in my previous email.
> However, the question of whether changes to limits in cgroups which are not
> under our cgroup-ns-root are allowed.
>
> Admittedly the current case with cgmanager is the same - in that it depends
> on proper setup of the container - but cgmanager is geared to recommend
> not mounting the cgroups in the container at all (and we can reject such
> mounts in the contaienr altogether with no loss in functionality) whereas
> you are here encouraging such mounts. Which is fine - so long as you then
> fully address the potential issues.
It will be nice to have this, but frankly, it may add a bit of
complexity in the cgroup/kernfs code (I will have to prototype and
see). Also same behavior can be obtained simply by bind-mounting
cgroupns-root inside the container. So I am currently inclining
towards rejecting such mounts in favor of simplicity.
Regarding disallowing writes to cgroup files outside of your
cgroupns-root, I think it should possible implement it easily. I will
include it in the next revision of this series.
Thanks,
--
Aditya
^ permalink raw reply
* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: Andy Lutomirski @ 2014-07-25 18:32 UTC (permalink / raw)
To: Julien Tinnes
Cc: Kees Cook, Eric W. Biederman, David Drysdale, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel@vger.kernel.org
In-Reply-To: <CAKyRK=j-f92xHTL3+TNr9WOv_y47dkZR=WZkpY_a5YW3Q8HfaQ@mail.gmail.com>
On Fri, Jul 25, 2014 at 11:22 AM, Julien Tinnes <jln@google.com> wrote:
> On Fri, Jul 25, 2014 at 10:38 AM, Kees Cook <keescook@chromium.org> wrote:
>>
>> On Fri, Jul 25, 2014 at 10:18 AM, Andy Lutomirski <luto@amacapital.net>
>> wrote:
>> > [cc: Eric Biederman]
>> >
>> > On Fri, Jul 25, 2014 at 10:10 AM, Kees Cook <keescook@chromium.org>
>> > wrote:
>>
>> >> Julien had been wanting something like this too (though he'd suggested
>> >> it via prctl): limit the signal functions to "self" only. I wonder if
>> >> adding a prctl like done for O_BENEATH could work for signal sending?
>> >>
>> >
>> >
>> > Can we do one better and add a flag to prevent any non-self pid
>> > lookups? This might actually be easy on top of the pid namespace work
>> > (e.g. we could change the way that find_task_by_vpid works).
>>
>> Ooh, that would be extremely interesting, yes. Kind of an extreme form
>> of pid namespace without actually being a namespace.
>>
>> > It's far from just being signals. There's access_process_vm, ptrace,
>> > all the signal functions, clock_gettime (see CPUCLOCK_PID -- yes, this
>> > is ridiculous), and probably some others that I've forgotten about or
>> > never noticed in the first place.
>>
>> Yeah, that would be very interesting.
>
>
> Yes, this would be incredibly useful.
>
> 1. For Chromium [1], I dislike relying on seccomp purely for
> "access-control" (to other processes or files). Because it's really hard to
> think about everything (things like CPUCLOCK_PID bite, see
> https://crbug.com/374479).
Not public :(
> Se we have a first layer of sandboxing (using PID + NET namespaces and
> chroot) for "access-control" and a second layer for kernel attack surface
> reduction and a few other things using seccomp-bpf.
>
> The first layer isn't currently very good; it's heavyweight and complex (you
> need an init(1) per namespace and that init cannot be multi-purposed as a
> useful process because pid = 1 can never receive signals). One PID namespace
> per process isn't something that scales well. (Also before USER_NS it
> required a setuid root program).
>
> 2. Even with a safe pure seccomp-bpf sandbox that prevents sending signals
> to other process / ptrace() et al and that restrict clock_gettime(2)
> properly, things become quickly very tedious because as far as the kernel is
> concerned, the process under this BPF program can still pass
> ptrace_may_access() to other processes. This means for instance that no
> matter what you do, a model where open() is allowed can't work if /proc is
> available. We need a mode that says "ptrace_may_access()" will never pass.
>
> So yes, I really would like:
> - a prctl that says: "I'm dropping privileges and I now can't interact with
> other thread groups (via signals, ptrace, etc..)".
> - Something to drop access to the file system. It could be an unprivileged
> way to chroot() to an empty directory (unprivileged namespaces work for
> that, - except if you're already in a chroot -). This is a little tricky
> without allowing chroot escapes, so I suspect we would want to express it in
> terms of mount namespace, or something else, rather than chroot.
Capsicum will give you this.
See the other thread for a more concrete proposal. prctl is getting
out of hand.
--Andy
^ permalink raw reply
* General flags to turn things off (getrandom, pid lookup, etc)
From: Andy Lutomirski @ 2014-07-25 18:30 UTC (permalink / raw)
To: Eric W. Biederman, Julien Tinnes, David Drysdale, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel@vger.kernel.org, Kees Cook, Theodore Ts'o,
Andy Lutomirski, Henrique de Moraes Holschuh, linux-crypto
[new thread because this sort of combines two threads]
There is recent interest in having a way to turn generally-available
kernel features off. Maybe we should add a good one so we can stop
bikeshedding and avoid proliferating dumb interfaces.
Things that might want to be turn-off-able include:
- getrandom with GRND_RANDOM [from the getrandom threads]
- Any lookup of a non-self pid [from the capsicum thread]
- Any lookup of a pid outside the caller thread group [capsicum]
- Various architectural things (personal wishlist), e.g.:
- RDTSC and userspace HPET access
- CPUID?
- 32-bit GDT code segments [huge attack surface]
- 64-bit GDT code segments [probably pointless]
I would propose a new syscall for this:
long restrict_userspace(int mode, int type, int value, int flags);
mode is RESTRICT_SET, RESTRICT_GET, or RESTRICT_LOCK.
type is RESTRICT_GRND_RANDOM, RESTRICT_PID_SCOPE, RESTRICT_X86_TIMING, etc.
Value is zero if RESTRICT_GET. Otherwise value is the desired value,
generally 0 or 1. For RESTRICT_PID_SCOPE, value would be
RESTRICT_PID_SCOPE_ANY, RESTRICT_PID_SCOPE_THREADGROUP, or
RESTRICT_PID_SCOPE_SELF.
flags must be zero. Someday, someone will propose a thread-sync flag.
restrict_userspace requires either no_new_privs or CAP_SYS_ADMIN in
the current user namespace.
Thoughts?
--Andy
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: Julien Tinnes @ 2014-07-25 18:24 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, Eric W. Biederman, David Drysdale, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAGXu5jLPrKA5LR-9=M6jAfPXYoztGzXPiaSiXgEcUE=+na73GA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jul 25, 2014 at 10:38 AM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Fri, Jul 25, 2014 at 10:18 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> [cc: Eric Biederman]
>>
>> On Fri, Jul 25, 2014 at 10:10 AM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>>> Julien had been wanting something like this too (though he'd suggested
>>> it via prctl): limit the signal functions to "self" only. I wonder if
>>> adding a prctl like done for O_BENEATH could work for signal sending?
>>>
>>
>>
>> Can we do one better and add a flag to prevent any non-self pid
>> lookups? This might actually be easy on top of the pid namespace work
>> (e.g. we could change the way that find_task_by_vpid works).
>
> Ooh, that would be extremely interesting, yes. Kind of an extreme form
> of pid namespace without actually being a namespace.
>
>> It's far from just being signals. There's access_process_vm, ptrace,
>> all the signal functions, clock_gettime (see CPUCLOCK_PID -- yes, this
>> is ridiculous), and probably some others that I've forgotten about or
>> never noticed in the first place.
>
> Yeah, that would be very interesting.
Yes, this would be incredibly useful.
1. For Chromium [1], I dislike relying on seccomp purely for
"access-control" (to other processes or files). Because it's really
hard to think about everything (things like CPUCLOCK_PID bite,
seehttps://crbug.com/374479).
Se we have a first layer of sandboxing (using PID + NET namespaces and
chroot) for "access-control" and a second layer for kernel attack
surface reduction and a few other things using seccomp-bpf.
The first layer isn't currently very good; it's heavyweight and
complex (you need an init(1) per namespace and that init cannot be
multi-purposed as a useful process because pid = 1 can never receive
signals). One PID namespace per process isn't something that scales
well. (Also before USER_NS it required a setuid root program).
2. Even with a safe pure seccomp-bpf sandbox that prevents sending
signals to other process / ptrace() et al and that restrict
clock_gettime(2) properly, things become quickly very tedious because
as far as the kernel is concerned, the process under this BPF program
can still pass ptrace_may_access() to other processes. This means for
instance that no matter what you do, a model where open() is allowed
can't work if /proc is available. We need a mode that says
"ptrace_may_access()" will never pass.
So yes, I really would like:
- a prctl that says: "I'm dropping privileges and I now can't interact
with other thread groups (via signals, ptrace, etc..)".
- Something to drop access to the file system. It could be an
unprivileged way to chroot() to an empty directory (unprivileged
namespaces work for that, - except if you're already in a chroot -).
This is a little tricky without allowing chroot escapes, so I suspect
we would want to express it in terms of mount namespace, or something
else, rather than chroot.
Then we have the primitives we need to build sandboxes in a simple
way and we can add seccomp-bpf on top to do things such as open()
hooking (via SECCOMP_RET_TRAP) and to restrict the kernel attack
surface.
Julien
[1] https://code.google.com/p/chromium/wiki/LinuxSandboxing
^ permalink raw reply
* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: Kees Cook @ 2014-07-25 17:38 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Eric W. Biederman, Julien Tinnes, David Drysdale, Al Viro,
Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrVChObsQpL6dt-ByiCjbPrtpXAXQgy_apBY-OpGQHaPjg@mail.gmail.com>
On Fri, Jul 25, 2014 at 10:18 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> [cc: Eric Biederman]
>
> On Fri, Jul 25, 2014 at 10:10 AM, Kees Cook <keescook@chromium.org> wrote:
>> On Fri, Jul 25, 2014 at 8:59 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Jul 25, 2014 6:48 AM, "David Drysdale" <drysdale@google.com> wrote:
>>>>
>>>> Add the current thread and thread group IDs into the data
>>>> available for seccomp-bpf programs to work on. This allows
>>>> installation of filters that police syscalls based on thread
>>>> or process ID, e.g. tgkill(2)/kill(2)/prctl(2).
>>>>
>>>> Signed-off-by: David Drysdale <drysdale@google.com>
>>>> ---
>>>> include/uapi/linux/seccomp.h | 10 ++++++++++
>>>> kernel/seccomp.c | 2 ++
>>>> 2 files changed, 12 insertions(+)
>>>>
>>>> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
>>>> index ac2dc9f72973..b88370d6f6ca 100644
>>>> --- a/include/uapi/linux/seccomp.h
>>>> +++ b/include/uapi/linux/seccomp.h
>>>> @@ -36,12 +36,22 @@
>>>> * @instruction_pointer: at the time of the system call.
>>>> * @args: up to 6 system call arguments always stored as 64-bit values
>>>> * regardless of the architecture.
>>>> + * @tgid: thread group ID of the thread executing the BPF program.
>>>> + * @tid: thread ID of the thread executing the BPF program.
>>>> + * The SECCOMP_DATA_TID_PRESENT macro indicates the presence of the
>>>> + * tgid and tid fields; user programs may use this macro to conditionally
>>>> + * compile code against older versions of the kernel. Note also that
>>>> + * BPF programs should cope with the absence of these fields by testing
>>>> + * the length of data available.
>>>> */
>>>> struct seccomp_data {
>>>> int nr;
>>>> __u32 arch;
>>>> __u64 instruction_pointer;
>>>> __u64 args[6];
>>>> + __u32 tgid;
>>>> + __u32 tid;
>>>> };
>>>> +#define SECCOMP_DATA_TID_PRESENT 1
>>>>
>>>> #endif /* _UAPI_LINUX_SECCOMP_H */
>>>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>>>> index 301bbc24739c..dd5146f15d6d 100644
>>>> --- a/kernel/seccomp.c
>>>> +++ b/kernel/seccomp.c
>>>> @@ -80,6 +80,8 @@ static void populate_seccomp_data(struct seccomp_data *sd)
>>>> sd->args[4] = args[4];
>>>> sd->args[5] = args[5];
>>>> sd->instruction_pointer = KSTK_EIP(task);
>>>> + sd->tgid = task_tgid_vnr(current);
>>>> + sd->tid = task_pid_vnr(current);
>>>> }
>>>
>>> This is, IMO, problematic. These should probably be relative to the
>>> filter creator, not the filtered task. This will also hurt
>>> performance.
>>
>> Yeah, we can't change the seccomp_data structure without a lot of
>> care, and tgid/tid really should be encoded in the filter. However, it
>> is tricky in the forking case.
>>
>>>
>>> What's the use case? Can it be better achieved with a new eBPF function?
>>
>> Julien had been wanting something like this too (though he'd suggested
>> it via prctl): limit the signal functions to "self" only. I wonder if
>> adding a prctl like done for O_BENEATH could work for signal sending?
>>
>
>
> Can we do one better and add a flag to prevent any non-self pid
> lookups? This might actually be easy on top of the pid namespace work
> (e.g. we could change the way that find_task_by_vpid works).
Ooh, that would be extremely interesting, yes. Kind of an extreme form
of pid namespace without actually being a namespace.
> It's far from just being signals. There's access_process_vm, ptrace,
> all the signal functions, clock_gettime (see CPUCLOCK_PID -- yes, this
> is ridiculous), and probably some others that I've forgotten about or
> never noticed in the first place.
Yeah, that would be very interesting.
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: Andy Lutomirski @ 2014-07-25 17:18 UTC (permalink / raw)
To: Kees Cook, Eric W. Biederman
Cc: Julien Tinnes, David Drysdale, Al Viro, Paolo Bonzini, LSM List,
Greg Kroah-Hartman, Paul Moore, James Morris, Linux API,
Meredydd Luff, Christoph Hellwig, linux-kernel@vger.kernel.org
In-Reply-To: <CAGXu5jJZ7mhmq1BrdTP5Ww15+C2iLQKjLy1Xh0=9qZvVK5E9Cw@mail.gmail.com>
[cc: Eric Biederman]
On Fri, Jul 25, 2014 at 10:10 AM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Jul 25, 2014 at 8:59 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Jul 25, 2014 6:48 AM, "David Drysdale" <drysdale@google.com> wrote:
>>>
>>> Add the current thread and thread group IDs into the data
>>> available for seccomp-bpf programs to work on. This allows
>>> installation of filters that police syscalls based on thread
>>> or process ID, e.g. tgkill(2)/kill(2)/prctl(2).
>>>
>>> Signed-off-by: David Drysdale <drysdale@google.com>
>>> ---
>>> include/uapi/linux/seccomp.h | 10 ++++++++++
>>> kernel/seccomp.c | 2 ++
>>> 2 files changed, 12 insertions(+)
>>>
>>> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
>>> index ac2dc9f72973..b88370d6f6ca 100644
>>> --- a/include/uapi/linux/seccomp.h
>>> +++ b/include/uapi/linux/seccomp.h
>>> @@ -36,12 +36,22 @@
>>> * @instruction_pointer: at the time of the system call.
>>> * @args: up to 6 system call arguments always stored as 64-bit values
>>> * regardless of the architecture.
>>> + * @tgid: thread group ID of the thread executing the BPF program.
>>> + * @tid: thread ID of the thread executing the BPF program.
>>> + * The SECCOMP_DATA_TID_PRESENT macro indicates the presence of the
>>> + * tgid and tid fields; user programs may use this macro to conditionally
>>> + * compile code against older versions of the kernel. Note also that
>>> + * BPF programs should cope with the absence of these fields by testing
>>> + * the length of data available.
>>> */
>>> struct seccomp_data {
>>> int nr;
>>> __u32 arch;
>>> __u64 instruction_pointer;
>>> __u64 args[6];
>>> + __u32 tgid;
>>> + __u32 tid;
>>> };
>>> +#define SECCOMP_DATA_TID_PRESENT 1
>>>
>>> #endif /* _UAPI_LINUX_SECCOMP_H */
>>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>>> index 301bbc24739c..dd5146f15d6d 100644
>>> --- a/kernel/seccomp.c
>>> +++ b/kernel/seccomp.c
>>> @@ -80,6 +80,8 @@ static void populate_seccomp_data(struct seccomp_data *sd)
>>> sd->args[4] = args[4];
>>> sd->args[5] = args[5];
>>> sd->instruction_pointer = KSTK_EIP(task);
>>> + sd->tgid = task_tgid_vnr(current);
>>> + sd->tid = task_pid_vnr(current);
>>> }
>>
>> This is, IMO, problematic. These should probably be relative to the
>> filter creator, not the filtered task. This will also hurt
>> performance.
>
> Yeah, we can't change the seccomp_data structure without a lot of
> care, and tgid/tid really should be encoded in the filter. However, it
> is tricky in the forking case.
>
>>
>> What's the use case? Can it be better achieved with a new eBPF function?
>
> Julien had been wanting something like this too (though he'd suggested
> it via prctl): limit the signal functions to "self" only. I wonder if
> adding a prctl like done for O_BENEATH could work for signal sending?
>
Can we do one better and add a flag to prevent any non-self pid
lookups? This might actually be easy on top of the pid namespace work
(e.g. we could change the way that find_task_by_vpid works).
It's far from just being signals. There's access_process_vm, ptrace,
all the signal functions, clock_gettime (see CPUCLOCK_PID -- yes, this
is ridiculous), and probably some others that I've forgotten about or
never noticed in the first place.
--Andy
> -Kees
>
> --
> Kees Cook
> Chrome OS Security
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: Kees Cook @ 2014-07-25 17:10 UTC (permalink / raw)
To: Andy Lutomirski, Julien Tinnes
Cc: David Drysdale, Al Viro, Paolo Bonzini, LSM List,
Greg Kroah-Hartman, Paul Moore, James Morris, Linux API,
Meredydd Luff, Christoph Hellwig, linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrVJX4+-6vkRaDj4kV_bXiYL5fj_PtO53g9fRf=i4X2Tww@mail.gmail.com>
On Fri, Jul 25, 2014 at 8:59 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Jul 25, 2014 6:48 AM, "David Drysdale" <drysdale@google.com> wrote:
>>
>> Add the current thread and thread group IDs into the data
>> available for seccomp-bpf programs to work on. This allows
>> installation of filters that police syscalls based on thread
>> or process ID, e.g. tgkill(2)/kill(2)/prctl(2).
>>
>> Signed-off-by: David Drysdale <drysdale@google.com>
>> ---
>> include/uapi/linux/seccomp.h | 10 ++++++++++
>> kernel/seccomp.c | 2 ++
>> 2 files changed, 12 insertions(+)
>>
>> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
>> index ac2dc9f72973..b88370d6f6ca 100644
>> --- a/include/uapi/linux/seccomp.h
>> +++ b/include/uapi/linux/seccomp.h
>> @@ -36,12 +36,22 @@
>> * @instruction_pointer: at the time of the system call.
>> * @args: up to 6 system call arguments always stored as 64-bit values
>> * regardless of the architecture.
>> + * @tgid: thread group ID of the thread executing the BPF program.
>> + * @tid: thread ID of the thread executing the BPF program.
>> + * The SECCOMP_DATA_TID_PRESENT macro indicates the presence of the
>> + * tgid and tid fields; user programs may use this macro to conditionally
>> + * compile code against older versions of the kernel. Note also that
>> + * BPF programs should cope with the absence of these fields by testing
>> + * the length of data available.
>> */
>> struct seccomp_data {
>> int nr;
>> __u32 arch;
>> __u64 instruction_pointer;
>> __u64 args[6];
>> + __u32 tgid;
>> + __u32 tid;
>> };
>> +#define SECCOMP_DATA_TID_PRESENT 1
>>
>> #endif /* _UAPI_LINUX_SECCOMP_H */
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index 301bbc24739c..dd5146f15d6d 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -80,6 +80,8 @@ static void populate_seccomp_data(struct seccomp_data *sd)
>> sd->args[4] = args[4];
>> sd->args[5] = args[5];
>> sd->instruction_pointer = KSTK_EIP(task);
>> + sd->tgid = task_tgid_vnr(current);
>> + sd->tid = task_pid_vnr(current);
>> }
>
> This is, IMO, problematic. These should probably be relative to the
> filter creator, not the filtered task. This will also hurt
> performance.
Yeah, we can't change the seccomp_data structure without a lot of
care, and tgid/tid really should be encoded in the filter. However, it
is tricky in the forking case.
>
> What's the use case? Can it be better achieved with a new eBPF function?
Julien had been wanting something like this too (though he'd suggested
it via prctl): limit the signal functions to "self" only. I wonder if
adding a prctl like done for O_BENEATH could work for signal sending?
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH 10/11] capsicum: prctl(2) to force use of O_BENEATH
From: Andy Lutomirski @ 2014-07-25 16:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Al Viro, LSM List, Greg Kroah-Hartman, Paul Moore, James Morris,
David Drysdale, Kees Cook, Linux API, Meredydd Luff,
Christoph Hellwig, linux-kernel@vger.kernel.org
In-Reply-To: <53D26355.9020809@redhat.com>
On Jul 25, 2014 7:02 AM, "Paolo Bonzini" <pbonzini@redhat.com> wrote:
>
> Il 25/07/2014 15:47, David Drysdale ha scritto:
> > @@ -1996,6 +2013,17 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> > if (arg2 || arg3 || arg4 || arg5)
> > return -EINVAL;
> > return current->no_new_privs ? 1 : 0;
> > + case PR_SET_OPENAT_BENEATH:
> > + if (arg2 != 1 || arg4 || arg5)
> > + return -EINVAL;
> > + if ((arg3 & ~(PR_SET_OPENAT_BENEATH_TSYNC)) != 0)
> > + return -EINVAL;
> > + error = prctl_set_openat_beneath(me, arg3);
> > + break;
> > + case PR_GET_OPENAT_BENEATH:
> > + if (arg2 || arg3 || arg4 || arg5)
> > + return -EINVAL;
> > + return me->openat_beneath;
> > case PR_GET_THP_DISABLE:
> > if (arg2 || arg3 || arg4 || arg5)
> > return -EINVAL;
> >
>
> Why are you always forbidding a change of prctl from 1 to 0? It should
> be safe if current->no_new_privs is clear.
I don't immediately see why you're forbidding unsettling it at all.
If you need it to be sticky, then use seccomp or Capsicum to make it
sticky.
Also, the way implementation is dangerously racy -- if anyone pokes at
adjacent bitfields without the lock, they can get corrupted. Try
basing on Kees' seccomp tree or security-next and using the new atomic
flags field.
--Andy
>
> Do new threads inherit from the parent?
>
> Also, I wonder if you need something like this check:
>
> /*
> * Installing a seccomp filter requires that the task has
> * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
> * This avoids scenarios where unprivileged tasks can affect the
> * behavior of privileged children.
> */
> if (!current->no_new_privs &&
> security_capable_noaudit(current_cred(), current_user_ns(),
> CAP_SYS_ADMIN) != 0)
> return -EACCES;
>
> Paolo
^ permalink raw reply
* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: Andy Lutomirski @ 2014-07-25 15:59 UTC (permalink / raw)
To: David Drysdale
Cc: Al Viro, Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
James Morris, Kees Cook, Linux API, Meredydd Luff,
Christoph Hellwig, linux-kernel@vger.kernel.org
In-Reply-To: <1406296033-32693-12-git-send-email-drysdale@google.com>
On Jul 25, 2014 6:48 AM, "David Drysdale" <drysdale@google.com> wrote:
>
> Add the current thread and thread group IDs into the data
> available for seccomp-bpf programs to work on. This allows
> installation of filters that police syscalls based on thread
> or process ID, e.g. tgkill(2)/kill(2)/prctl(2).
>
> Signed-off-by: David Drysdale <drysdale@google.com>
> ---
> include/uapi/linux/seccomp.h | 10 ++++++++++
> kernel/seccomp.c | 2 ++
> 2 files changed, 12 insertions(+)
>
> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> index ac2dc9f72973..b88370d6f6ca 100644
> --- a/include/uapi/linux/seccomp.h
> +++ b/include/uapi/linux/seccomp.h
> @@ -36,12 +36,22 @@
> * @instruction_pointer: at the time of the system call.
> * @args: up to 6 system call arguments always stored as 64-bit values
> * regardless of the architecture.
> + * @tgid: thread group ID of the thread executing the BPF program.
> + * @tid: thread ID of the thread executing the BPF program.
> + * The SECCOMP_DATA_TID_PRESENT macro indicates the presence of the
> + * tgid and tid fields; user programs may use this macro to conditionally
> + * compile code against older versions of the kernel. Note also that
> + * BPF programs should cope with the absence of these fields by testing
> + * the length of data available.
> */
> struct seccomp_data {
> int nr;
> __u32 arch;
> __u64 instruction_pointer;
> __u64 args[6];
> + __u32 tgid;
> + __u32 tid;
> };
> +#define SECCOMP_DATA_TID_PRESENT 1
>
> #endif /* _UAPI_LINUX_SECCOMP_H */
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 301bbc24739c..dd5146f15d6d 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -80,6 +80,8 @@ static void populate_seccomp_data(struct seccomp_data *sd)
> sd->args[4] = args[4];
> sd->args[5] = args[5];
> sd->instruction_pointer = KSTK_EIP(task);
> + sd->tgid = task_tgid_vnr(current);
> + sd->tid = task_pid_vnr(current);
> }
This is, IMO, problematic. These should probably be relative to the
filter creator, not the filtered task. This will also hurt
performance.
What's the use case? Can it be better achieved with a new eBPF function?
--Andy
>
> /**
> --
> 2.0.0.526.g5318336
>
^ permalink raw reply
* Re: [PATCH 10/11] capsicum: prctl(2) to force use of O_BENEATH
From: Paolo Bonzini @ 2014-07-25 14:01 UTC (permalink / raw)
To: David Drysdale, linux-security-module, linux-kernel,
Greg Kroah-Hartman
Cc: Alexander Viro, Meredydd Luff, Kees Cook, James Morris,
Andy Lutomirski, Paul Moore, Christoph Hellwig, linux-api
In-Reply-To: <1406296033-32693-11-git-send-email-drysdale@google.com>
Il 25/07/2014 15:47, David Drysdale ha scritto:
> @@ -1996,6 +2013,17 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> if (arg2 || arg3 || arg4 || arg5)
> return -EINVAL;
> return current->no_new_privs ? 1 : 0;
> + case PR_SET_OPENAT_BENEATH:
> + if (arg2 != 1 || arg4 || arg5)
> + return -EINVAL;
> + if ((arg3 & ~(PR_SET_OPENAT_BENEATH_TSYNC)) != 0)
> + return -EINVAL;
> + error = prctl_set_openat_beneath(me, arg3);
> + break;
> + case PR_GET_OPENAT_BENEATH:
> + if (arg2 || arg3 || arg4 || arg5)
> + return -EINVAL;
> + return me->openat_beneath;
> case PR_GET_THP_DISABLE:
> if (arg2 || arg3 || arg4 || arg5)
> return -EINVAL;
>
Why are you always forbidding a change of prctl from 1 to 0? It should
be safe if current->no_new_privs is clear.
Do new threads inherit from the parent?
Also, I wonder if you need something like this check:
/*
* Installing a seccomp filter requires that the task has
* CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
* This avoids scenarios where unprivileged tasks can affect the
* behavior of privileged children.
*/
if (!current->no_new_privs &&
security_capable_noaudit(current_cred(), current_user_ns(),
CAP_SYS_ADMIN) != 0)
return -EACCES;
Paolo
^ permalink raw reply
* [PATCH 6/6] prctl.2: describe PR_SET_OPENAT_BENEATH/PR_GET_OPENAT_BENEATH
From: David Drysdale @ 2014-07-25 13:47 UTC (permalink / raw)
To: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman
Cc: Alexander Viro, Meredydd Luff, Kees Cook, James Morris,
Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig,
linux-api-u79uwXL29TY76Z2rM5mHXA, David Drysdale
In-Reply-To: <1406296033-32693-1-git-send-email-drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
man2/prctl.2 | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/man2/prctl.2 b/man2/prctl.2
index 119989183ed3..f5f71af249f2 100644
--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -295,6 +295,41 @@ A value of 1 indicates
.BR execve (2)
will operate in the privilege-restricting mode described above.
.TP
+.BR PR_SET_OPENAT_BENEATH " (since Linux 3.??)"
+Set the calling process's
+.I openat_beneath
+bit to the value in
+.IR arg2 .
+With
+.I openat_beneath
+set to 1, all
+.BR openat (2)
+and
+.BR open (2)
+operations act as though the
+.B O_BENEATH
+flag is set.
+Once set, this bit cannot be unset.
+The setting of this bit is inherited by children created by
+.BR fork (2)
+and
+.BR clone (2),
+and preserved across
+.BR execve (2).
+.TP
+.BR PR_GET_OPENAT_BENEATH " (since Linux 3.??)"
+Return (as the function result) the value of the
+.I openat_beneath
+bit for the current process.
+A value of 0 indicates the regular behavior.
+A value of 1 indicates that
+.BR openat (2)
+and
+.BR open (2)
+will operate in the implicit
+.B O_BENEATH
+mode described above.
+.TP
.BR PR_SET_PDEATHSIG " (since Linux 2.1.57)"
Set the parent process death signal
of the calling process to \fIarg2\fP (either a signal value
--
2.0.0.526.g5318336
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox