LinuxLists.cc - [capabilities] Allow normal inheritance for a configurable set of capabilities

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Mon, Feb 2, 2015 at 9:12 AM, Serge Hallyn <[email protected]> wrote:
> A key concept behind posix capabilities is that the privilege comes from
> both the person and the file being executed. As you say below basically
> anything can be executed by the program so that is completely violated.
>
> Still, it's not that different from mmapping some arbitrary code and
> jumping into it whlie retaining caps.
>
> If we were to support such a feature, I'm thinking I'd prefer we do
> it somewhat analogously to the capability bounding set. Perhaps add a
> ambient_inh_caps set or something. Empty by default. To add caps to it you
> must have the cap in your permitted set already. (Ok to do in a user
> namespace). Then at exec,
>
> pP' = (X & fP) | (pI & fI) | (pI & pA)
>
> pA being your ambient_inh set
>
> Not saying this is a good idea necessarily, but worth thinking about.

This isn't obviously a bad formulation. We could control pA using some syscall.

Another formulation would be a single per-user-ns or
inherited-per-process bit that sets fI to the full set regardless of
file caps. Dealing with the file effective bit will be an added
complication, as will dealing with setuid binaries.

How many of you will be at LSF/MM? This might be a decent topic.

--Andy

2015-02-02 17:54:07

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On 2/2/2015 8:21 AM, Christoph Lameter wrote:
> Linux capabilities suffer from the problem that they are not inheritable
> like regular process characteristics under Unix. This is behavior that
> is counter intuitive to the expected behavior of processes in Unix.

http://wt.tuxomania.net/publications/posix.1e/download.html

The POSIX* capability scheme is the simplest mechanism we could
come up with that allows existing setuid programs to work unmodified
and still make it possible to constrain specific capabilities.
Is it complicated? Yes. Why is it complicated? Because you need
the option of using the file capabilities to raise and lower the
privilege of a program. Had we the option of requiring the programs
to do that themselves, the whole thing would have been easier.
You also need the option of having a capability aware program
manipulate it's own capabilities.

All the UNIX systems that implemented capabilities did so
using one variate or another of the POSIX scheme. One,
Trusted IRIX, successfully eliminated root privilege.

---
Disclaimer: The DRAFT is withdrawn. There is no standard.
You can't claim conformance.

> In particular there has been recently software that controls NICs from user
> space and provides IP stack like behavior also in user space (DPDK and RDMA
> kernel API based implementations). Those typically need either capabilities
> to allow raw network access or have to be run setsuid. There is scripting and
> LD_PREFLOAD etc involved, arbitrary binaries may be run from those scripts.

You're getting into pretty sketchy territory using that kind
of a programming model in a security enforcing environment. Kids!

> That does not go well with having file capabilities set that would enable
> the capabilities. Maybe it would work if one would setup capabilities on
> all executables but that would also defeat a secure design since these
> binaries may only need those caps for certain situations. Ok setting the
> inheritable flags on everything may also get one there (if there would not
> be the issues with LD_PRELOAD, debugging etc etc).

It *can* be done with the combination of inheritable, permitted and
effective capabilities. I admit it's not convenient. If you're willing
to modify your code to handle dropping capabilities you can simplify
the file based configuration significantly.

As for debugging, that's always a security nightmare.

> The easy solution is that capabilities need to be inherited like setsuid
> is. We really prefer to use capabilities instead of setsuid (we want to
> limit what damage someone can do after all!). Therefore we have been
> running a patch like this in production for the last 6 years. At some
> point it becomes tedious to run your own custom kernel so we would like
> to have this functionality upstream.
>
> See some of the earlier related discussions on the problems with capability
> inheritance:
>
> 0. Recent surprise:
> https://lkml.org/lkml/2014/1/21/175
>
> 1. Attempt to revise caps
> http://www.madore.org/~david/linux/newcaps/
>
> 2. Problems of passing caps through exec
> http://unix.stackexchange.com/questions/128394/passing-capabilities-through-exec
>
> 3. Problems of binding to privileged ports
> http://stackoverflow.com/questions/413807/is-there-a-way-for-non-root-processes-to-bind-to-privileged-ports-1024-on-l
>
> 4. Reviving capabilities
> http://lwn.net/Articles/199004/
>
>
>
> There does not seem to be an alternative on the horizon. Some involved
> in security development under Linux have even stated that they want to
> rip out the whole thing and replace it. Its been a couple of years now
> and we are still suffering from the capabilities mess. Let us just
> fix it.

I'm game to participate in such an effort. The POSIX scheme
is workable, but given that it's 20 years old and hasn't
developed real traction it's hard to call it successful.

> This patch does not change the default behavior but it allows to set up
> a list of capabilities in the proc filesystem that will enable regular
> unix inheritance only for the selected group of capabilities.
>
> With that it is then possible to do something trivial like setting
> CAP_NET_RAW on an executable that can then allow that capability to
> be inherited by others.
>
> e.g
>
> echo 12,13,23 >/proc/sys/kernel/cap_inheritable
>
> Allows the inheritance of CAP_SYS_NICE, CAP_NET_RAW and CAP_NET_ADMIN.
> With that device raw access is possible and also real time priorities
> can be set from user space. This is a frequently needed set of
> priviledged operations in HPC and HFT applications. User space
> processes need to be able to directly access devices as well as
> have full control over scheduling.
>
> Setting capabilities on an executable is not always possible if
> for example LD_PRELOAD or other things also have to be used. In that
> case it is possible to build a classic wrapper after applying this
> patch that sets up the proper privileges for running processes
> that need these.
>
> I usually do not dabble in security and I am not sure if this is
> done correctly. If someone has a better solution then please tell
> me but so far we have not seen anything else that actually works.
> This keeps on coming up in various context and we need the issue
> fixed!
>
> Signed-off-by: Christoph Lameter <[email protected]>
>
> Index: linux/include/linux/capability.h
> ===================================================================
> --- linux.orig/include/linux/capability.h
> +++ linux/include/linux/capability.h
> @@ -44,6 +44,7 @@ struct user_namespace *current_user_ns(v
>
> extern const kernel_cap_t __cap_empty_set;
> extern const kernel_cap_t __cap_init_eff_set;
> +extern const unsigned long *sysctl_cap_inheritable;
>
> /*
> * Internal kernel functions only
> Index: linux/kernel/capability.c
> ===================================================================
> --- linux.orig/kernel/capability.c
> +++ linux/kernel/capability.c
> @@ -26,6 +26,16 @@
> const kernel_cap_t __cap_empty_set = CAP_EMPTY_SET;
> EXPORT_SYMBOL(__cap_empty_set);
>
> +/*
> + * Allow inheritance with typical unix semantics for capabilities.
> + * This means that the inheritable flag can be omitted on the file
> + * that inherits the capabilities. Capabilities will be passed down
> + * via exec like other process characteristics. This is the behavior
> + * sysadmins expect.
> + */
> +static unsigned long cap_inheritable[BITS_TO_LONGS(CAP_LAST_CAP)];
> +const unsigned long *sysctl_cap_inheritable = cap_inheritable;
> +
> int file_caps_enabled = 1;
>
> static int __init file_caps_disable(char *str)
> Index: linux/kernel/sysctl.c
> ===================================================================
> --- linux.orig/kernel/sysctl.c
> +++ linux/kernel/sysctl.c
> @@ -840,6 +840,14 @@ static struct ctl_table kern_table[] = {
> .mode = 0444,
> .proc_handler = proc_dointvec,
> },
> + {
> + .procname = "cap_inheritable",
> + .data = &sysctl_cap_inheritable,
> + .maxlen = CAP_LAST_CAP,
> + .mode = 0644,
> + .proc_handler = proc_do_large_bitmap,
> + },
> +
> #if defined(CONFIG_LOCKUP_DETECTOR)
> {
> .procname = "watchdog",
> Index: linux/security/commoncap.c
> ===================================================================
> --- linux.orig/security/commoncap.c
> +++ linux/security/commoncap.c
> @@ -437,6 +437,9 @@ static int get_file_caps(struct linux_bi
> struct dentry *dentry;
> int rc = 0;
> struct cpu_vfs_cap_data vcaps;
> + kernel_cap_t inherit = CAP_EMPTY_SET;
> + bool does_inherit = false;
> + int i;
>
> bprm_clear_caps(bprm);
>
> @@ -446,6 +449,17 @@ static int get_file_caps(struct linux_bi
> if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
> return 0;
>
> + /*
> + * Figure out if any capabilities are inheritable without
> + * setting bits in the target file.
> + */
> + for_each_set_bit(i, sysctl_cap_inheritable, CAP_LAST_CAP)
> + if (capable(i)) {
> + cap_raise(inherit, i);
> + does_inherit = true;
> + }
> +
> +
> dentry = dget(bprm->file->f_path.dentry);
>
> rc = get_vfs_caps_from_disk(dentry, &vcaps);
> @@ -455,7 +469,8 @@ static int get_file_caps(struct linux_bi
> __func__, rc, bprm->filename);
> else if (rc == -ENODATA)
> rc = 0;
> - goto out;
> + if (!does_inherit)
> + goto out;
> }
>
> rc = bprm_caps_from_vfs_caps(&vcaps, bprm, effective, has_cap);
> @@ -463,6 +478,15 @@ static int get_file_caps(struct linux_bi
> printk(KERN_NOTICE "%s: cap_from_disk returned %d for %s\n",
> __func__, rc, bprm->filename);
>
> + if (does_inherit) {
> + struct cred *new = bprm->cred;
> + /* Add new capabilies from inheritance mask */
> + new->cap_inheritable = cap_combine(inherit, new->cap_inheritable);
> + new->cap_permitted = cap_combine(inherit, new->cap_permitted);
> + *effective = true;
> + *has_cap = true;
> + }
> +
> out:
> dput(dentry);
> if (rc)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2015-02-02 18:08:17

by Serge Hallyn

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Casey Schaufler ([email protected]):
> I'm game to participate in such an effort. The POSIX scheme
> is workable, but given that it's 20 years old and hasn't
> developed real traction it's hard to call it successful.

Over the years we've several times discussed possible reasons for this
and how to help. I personally think it's two things: 1. lack of
toolchain and fs support. The fact that we cannot to this day enable
ping using capabilities by default because of cpio, tar and non-xattr
filesystems is disheartening. 2. It's hard for users and applications
to know what caps they need. yes the API is a bear to use, but we can
hide that behind fancier libraries. But using capabilities requires too
much in-depth knowledge of precisely what caps you might need for
whatever operations library may now do when you asked for something.

2015-02-02 18:10:01

by Serge Hallyn

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Andy Lutomirski ([email protected]):
> On Mon, Feb 2, 2015 at 9:12 AM, Serge Hallyn <[email protected]> wrote:
> > A key concept behind posix capabilities is that the privilege comes from
> > both the person and the file being executed. As you say below basically
> > anything can be executed by the program so that is completely violated.
> >
> > Still, it's not that different from mmapping some arbitrary code and
> > jumping into it whlie retaining caps.
> >
> > If we were to support such a feature, I'm thinking I'd prefer we do
> > it somewhat analogously to the capability bounding set. Perhaps add a
> > ambient_inh_caps set or something. Empty by default. To add caps to it you
> > must have the cap in your permitted set already. (Ok to do in a user
> > namespace). Then at exec,
> >
> > pP' = (X & fP) | (pI & fI) | (pI & pA)
> >
> > pA being your ambient_inh set
> >
> > Not saying this is a good idea necessarily, but worth thinking about.
>
> This isn't obviously a bad formulation. We could control pA using some syscall.

My first thought was prctl (since we have PR_CAPBSET_DROP)

> Another formulation would be a single per-user-ns or
> inherited-per-process bit that sets fI to the full set regardless of
> file caps. Dealing with the file effective bit will be an added
> complication, as will dealing with setuid binaries.
>
> How many of you will be at LSF/MM? This might be a decent topic.

I'm not scheduled to be there.

2015-02-02 18:48:12

by Mimi Zohar

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Mon, 2015-02-02 at 18:08 +0000, Serge Hallyn wrote:
> Quoting Casey Schaufler ([email protected]):
> > I'm game to participate in such an effort. The POSIX scheme
> > is workable, but given that it's 20 years old and hasn't
> > developed real traction it's hard to call it successful.
>
> Over the years we've several times discussed possible reasons for this
> and how to help. I personally think it's two things: 1. lack of
> toolchain and fs support. The fact that we cannot to this day enable
> ping using capabilities by default because of cpio, tar and non-xattr
> filesystems is disheartening.

We're working on resolving the CPIO issue. tar currently supports
xattrs. At this point, how many non-xattr filesystems are there really?

Mimi

> 2. It's hard for users and applications
> to know what caps they need. yes the API is a bear to use, but we can
> hide that behind fancier libraries. But using capabilities requires too
> much in-depth knowledge of precisely what caps you might need for
> whatever operations library may now do when you asked for something.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2015-02-02 19:00:22

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On 2/2/2015 10:08 AM, Serge Hallyn wrote:
> Quoting Casey Schaufler ([email protected]):
>> I'm game to participate in such an effort. The POSIX scheme
>> is workable, but given that it's 20 years old and hasn't
>> developed real traction it's hard to call it successful.
> Over the years we've several times discussed possible reasons for this
> and how to help. I personally think it's two things: 1. lack of
> toolchain and fs support. The fact that we cannot to this day enable
> ping using capabilities by default because of cpio, tar and non-xattr
> filesystems is disheartening. 2. It's hard for users and applications
> to know what caps they need. yes the API is a bear to use, but we can
> hide that behind fancier libraries. But using capabilities requires too
> much in-depth knowledge of precisely what caps you might need for
> whatever operations library may now do when you asked for something.

The fix for that is to a change to the audit system. If the audit system
reported the capabilities relevant to the decision you'd have what you
need. If you failed because you didn't have CAP_CHMOD or you succeeded
because you had CAP_SYS_ADMIN it should show up in the audit record.
Other systems have used this approach.

You could, of course, create a separate capability result log, and I
believe that Nokia had done something along those lines. I think that
adding it to the audit trail is a more rational approach.

2015-02-02 19:06:01

by Austin S Hemmelgarn

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On 2015-02-02 13:47, Mimi Zohar wrote:
> On Mon, 2015-02-02 at 18:08 +0000, Serge Hallyn wrote:
>> Quoting Casey Schaufler ([email protected]):
>>> I'm game to participate in such an effort. The POSIX scheme
>>> is workable, but given that it's 20 years old and hasn't
>>> developed real traction it's hard to call it successful.
>>
>> Over the years we've several times discussed possible reasons for this
>> and how to help. I personally think it's two things: 1. lack of
>> toolchain and fs support. The fact that we cannot to this day enable
>> ping using capabilities by default because of cpio, tar and non-xattr
>> filesystems is disheartening.
>
> We're working on resolving the CPIO issue. tar currently supports
> xattrs. At this point, how many non-xattr filesystems are there really?
>

FAT*, and UFS immediately come to mind, and I know of people who use UFS
for their root filesystem. There are a handful (ext* included) that
need an option turned on in the kernel config, and possibly also a mount
option added.

IIRC, the Linux NFS client has no xattr support, and that is very widely
used because it's easier to set up than any alternatives.

Attachments:

smime.p7s (2.40 kB)
S/MIME Cryptographic Signature

2015-02-02 20:35:21

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On 2/2/2015 11:05 AM, Austin S Hemmelgarn wrote:
> On 2015-02-02 13:47, Mimi Zohar wrote:
>> On Mon, 2015-02-02 at 18:08 +0000, Serge Hallyn wrote:
>>> Quoting Casey Schaufler ([email protected]):
>>>> I'm game to participate in such an effort. The POSIX scheme
>>>> is workable, but given that it's 20 years old and hasn't
>>>> developed real traction it's hard to call it successful.
>>>
>>> Over the years we've several times discussed possible reasons for this
>>> and how to help. I personally think it's two things: 1. lack of
>>> toolchain and fs support. The fact that we cannot to this day enable
>>> ping using capabilities by default because of cpio, tar and non-xattr
>>> filesystems is disheartening.
>>
>> We're working on resolving the CPIO issue. tar currently supports
>> xattrs. At this point, how many non-xattr filesystems are there really?
>>
>
> FAT*, and UFS immediately come to mind, and I know of people who use
> UFS for their root filesystem. There are a handful (ext* included)
> that need an option turned on in the kernel config, and possibly also
> a mount option added.
>
> IIRC, the Linux NFS client has no xattr support, and that is very
> widely used because it's easier to set up than any alternatives.

There is NFSv4 support for Mandatory Access Control labels, but
so far it only works with SELinux contexts. It is not a general
solution. Networking people think poorly of the notion of extended
attributes over the wire.

2015-02-02 20:38:19

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Mon, Feb 2, 2015 at 10:08 AM, Serge Hallyn <[email protected]> wrote:
> Quoting Casey Schaufler ([email protected]):
>> I'm game to participate in such an effort. The POSIX scheme
>> is workable, but given that it's 20 years old and hasn't
>> developed real traction it's hard to call it successful.
>
> Over the years we've several times discussed possible reasons for this
> and how to help. I personally think it's two things: 1. lack of
> toolchain and fs support. The fact that we cannot to this day enable
> ping using capabilities by default because of cpio, tar and non-xattr
> filesystems is disheartening. 2. It's hard for users and applications
> to know what caps they need. yes the API is a bear to use, but we can
> hide that behind fancier libraries. But using capabilities requires too
> much in-depth knowledge of precisely what caps you might need for
> whatever operations library may now do when you asked for something.

None of this could address the problem here, though: if I hold a
capability and I want to pass that capability to an exec'd helper, I
shouldn't need the fs's help to do this.

--Andy

2015-02-02 20:54:04

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On 2/2/2015 12:37 PM, Andy Lutomirski wrote:
> On Mon, Feb 2, 2015 at 10:08 AM, Serge Hallyn <[email protected]> wrote:
>> Quoting Casey Schaufler ([email protected]):
>>> I'm game to participate in such an effort. The POSIX scheme
>>> is workable, but given that it's 20 years old and hasn't
>>> developed real traction it's hard to call it successful.
>> Over the years we've several times discussed possible reasons for this
>> and how to help. I personally think it's two things: 1. lack of
>> toolchain and fs support. The fact that we cannot to this day enable
>> ping using capabilities by default because of cpio, tar and non-xattr
>> filesystems is disheartening. 2. It's hard for users and applications
>> to know what caps they need. yes the API is a bear to use, but we can
>> hide that behind fancier libraries. But using capabilities requires too
>> much in-depth knowledge of precisely what caps you might need for
>> whatever operations library may now do when you asked for something.
> None of this could address the problem here, though: if I hold a
> capability and I want to pass that capability to an exec'd helper, I
> shouldn't need the fs's help to do this.

One of the holes in the 1003.1e spec is what to do with a program file
that does not have a capability set attached to it. The two options are
drop all capabilities and leave the capabilities alone. The latter gives
you what you're asking for. The former is arguably safer.

>
> --Andy
>

2015-02-03 15:16:34

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Mon, 2 Feb 2015, Andy Lutomirski wrote:

> How many of you will be at LSF/MM? This might be a decent topic.

I will be there for the mm portion and can talk about this if you want.
But note my limited knowledge about security issues.

2015-02-03 15:17:43

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Mon, 2 Feb 2015, Andy Lutomirski wrote:

> None of this could address the problem here, though: if I hold a
> capability and I want to pass that capability to an exec'd helper, I
> shouldn't need the fs's help to do this.

Amen!

2015-02-03 15:23:17

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Mon, 2 Feb 2015, Serge Hallyn wrote:

> A key concept behind posix capabilities is that the privilege comes from
> both the person and the file being executed. As you say below basically
> anything can be executed by the program so that is completely violated.

Well this patch does not change that. Its just that the file being
executed can inherit the parent caps without having to set caps in the new
executable.

> Still, it's not that different from mmapping some arbitrary code and
> jumping into it whlie retaining caps.

Owww. Thats a pretty far jump from what this patch does.

> Not saying this is a good idea necessarily, but worth thinking about.

Well we keep on having actual problems with caps for long years now. We
need a solution. This means a patch that solves the issue.

2015-02-03 15:40:30

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On 2/3/2015 7:17 AM, Christoph Lameter wrote:
> On Mon, 2 Feb 2015, Andy Lutomirski wrote:
>
>> None of this could address the problem here, though: if I hold a
>> capability and I want to pass that capability to an exec'd helper, I
>> shouldn't need the fs's help to do this.
> Amen!
>
That's completely consistent with the notion that if a binary has no
file capabilities (as opposed to a set that contains no capabilities)
the process capabilities are unchanged by exec(). If the binary does
have capabilities, however, it must always apply them. That should be
obvious. In your case, the helper would have no file capabilities, and
hence get whatever the invoker has. A program that should never run with
capabilities should have a file attribute stating such. Where it gets
sticky is the case where you want inheritance when invoked by one service
and no capabilities when invoked from another. If we live with the notion
that you have to choose this is easy enough to solve.

2015-02-03 15:52:47

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Andy Lutomirski ([email protected]):
> On Mon, Feb 2, 2015 at 10:08 AM, Serge Hallyn <[email protected]> wrote:
> > Quoting Casey Schaufler ([email protected]):
> >> I'm game to participate in such an effort. The POSIX scheme
> >> is workable, but given that it's 20 years old and hasn't
> >> developed real traction it's hard to call it successful.
> >
> > Over the years we've several times discussed possible reasons for this
> > and how to help. I personally think it's two things: 1. lack of
> > toolchain and fs support. The fact that we cannot to this day enable
> > ping using capabilities by default because of cpio, tar and non-xattr
> > filesystems is disheartening. 2. It's hard for users and applications
> > to know what caps they need. yes the API is a bear to use, but we can
> > hide that behind fancier libraries. But using capabilities requires too
> > much in-depth knowledge of precisely what caps you might need for
> > whatever operations library may now do when you asked for something.
>
> None of this could address the problem here, though: if I hold a
> capability and I want to pass that capability to an exec'd helper, I
> shouldn't need the fs's help to do this.

Yes you should. It's not about needing the fs's help, it's about the
binary on the exec being imbued with some part of the privilege. In TE
paralance it would be an entry point to the privilege. I'm not saying "that's
how it should be", just "that's how posix caps work".

So again I think the pA seems like an elegant way to work around this.
I'm interested in other ideas, but I worry about the proc solution Christoph
proposed because it would be system- or namespace-wide, rather than
per-process.

2015-02-03 15:57:47

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Casey Schaufler ([email protected]):
> On 2/2/2015 12:37 PM, Andy Lutomirski wrote:
> > On Mon, Feb 2, 2015 at 10:08 AM, Serge Hallyn <[email protected]> wrote:
> >> Quoting Casey Schaufler ([email protected]):
> >>> I'm game to participate in such an effort. The POSIX scheme
> >>> is workable, but given that it's 20 years old and hasn't
> >>> developed real traction it's hard to call it successful.
> >> Over the years we've several times discussed possible reasons for this
> >> and how to help. I personally think it's two things: 1. lack of
> >> toolchain and fs support. The fact that we cannot to this day enable
> >> ping using capabilities by default because of cpio, tar and non-xattr
> >> filesystems is disheartening. 2. It's hard for users and applications
> >> to know what caps they need. yes the API is a bear to use, but we can
> >> hide that behind fancier libraries. But using capabilities requires too
> >> much in-depth knowledge of precisely what caps you might need for
> >> whatever operations library may now do when you asked for something.
> > None of this could address the problem here, though: if I hold a
> > capability and I want to pass that capability to an exec'd helper, I
> > shouldn't need the fs's help to do this.
>
> One of the holes in the 1003.1e spec is what to do with a program file
> that does not have a capability set attached to it. The two options are
> drop all capabilities and leave the capabilities alone. The latter gives
> you what you're asking for. The former is arguably safer.

Hm, so if we were to change that, what should we do in the case of (a)
an fs which doesn't support xattrs, (2) expanding a tarball/cpio which
didn't have xattrs (should tar/cpio fill them in with empty sets?),
and (3) do we add a default empty set in the case of an fs mounted with
NOSUID?

It's an interesting notion.

2015-02-03 15:55:48

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Christoph Lameter ([email protected]):
> On Mon, 2 Feb 2015, Serge Hallyn wrote:
>
> > A key concept behind posix capabilities is that the privilege comes from
> > both the person and the file being executed. As you say below basically
> > anything can be executed by the program so that is completely violated.
>
> Well this patch does not change that. Its just that the file being
> executed can inherit the parent caps without having to set caps in the new
> executable.
>
> > Still, it's not that different from mmapping some arbitrary code and
> > jumping into it whlie retaining caps.
>
> Owww. Thats a pretty far jump from what this patch does.

Hm? That's not something about a kernel patch - it's just something you
can do in userspace today to not lose your privilege in several mac
systems.

> > Not saying this is a good idea necessarily, but worth thinking about.
>
> Well we keep on having actual problems with caps for long years now. We
> need a solution. This means a patch that solves the issue.

We've currently got two proposals. (Three includig yours; but I explained my
problem with yours earlier this morning - do appreciate the proposal and
the patch though, really, thanks) It's worth digging into the details of
each, but if they end up complicating things then perhaps "dropping
capabilities and going with something new" ought to be another proposal.

2015-02-03 16:04:08

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Mimi Zohar ([email protected]):
> On Mon, 2015-02-02 at 18:08 +0000, Serge Hallyn wrote:
> > Quoting Casey Schaufler ([email protected]):
> > > I'm game to participate in such an effort. The POSIX scheme
> > > is workable, but given that it's 20 years old and hasn't
> > > developed real traction it's hard to call it successful.
> >
> > Over the years we've several times discussed possible reasons for this
> > and how to help. I personally think it's two things: 1. lack of
> > toolchain and fs support. The fact that we cannot to this day enable
> > ping using capabilities by default because of cpio, tar and non-xattr
> > filesystems is disheartening.
>
> We're working on resolving the CPIO issue.

Awesome, thanks for that :)

> tar currently supports
> xattrs.

yes, I guess what i was thinking of was a (recent) bug to do with one
of the longer term support older systems. Problem is people want to
run new releases in containers on older systems :)

Anyway when I said "over the years", the first was I believe in 2009/2010,
the last was in 2013 (at that LSS, when Ted mentioned it); so yeah things have
gotten better with respect to this.

But AFAIK we don't yet have any good ideas for tackling the second issue below.

> At this point, how many non-xattr filesystems are there really?
>
> Mimi
>
> > 2. It's hard for users and applications
> > to know what caps they need. yes the API is a bear to use, but we can
> > hide that behind fancier libraries. But using capabilities requires too
> > much in-depth knowledge of precisely what caps you might need for
> > whatever operations library may now do when you asked for something.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-02-03 16:37:38

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On 2/3/2015 7:51 AM, Serge E. Hallyn wrote:
> Quoting Casey Schaufler ([email protected]):
>> On 2/2/2015 12:37 PM, Andy Lutomirski wrote:
>>> On Mon, Feb 2, 2015 at 10:08 AM, Serge Hallyn <[email protected]> wrote:
>>>> Quoting Casey Schaufler ([email protected]):
>>>>> I'm game to participate in such an effort. The POSIX scheme
>>>>> is workable, but given that it's 20 years old and hasn't
>>>>> developed real traction it's hard to call it successful.
>>>> Over the years we've several times discussed possible reasons for this
>>>> and how to help. I personally think it's two things: 1. lack of
>>>> toolchain and fs support. The fact that we cannot to this day enable
>>>> ping using capabilities by default because of cpio, tar and non-xattr
>>>> filesystems is disheartening. 2. It's hard for users and applications
>>>> to know what caps they need. yes the API is a bear to use, but we can
>>>> hide that behind fancier libraries. But using capabilities requires too
>>>> much in-depth knowledge of precisely what caps you might need for
>>>> whatever operations library may now do when you asked for something.
>>> None of this could address the problem here, though: if I hold a
>>> capability and I want to pass that capability to an exec'd helper, I
>>> shouldn't need the fs's help to do this.
>> One of the holes in the 1003.1e spec is what to do with a program file
>> that does not have a capability set attached to it. The two options are
>> drop all capabilities and leave the capabilities alone. The latter gives
>> you what you're asking for. The former is arguably safer.
> Hm, so if we were to change that, what should we do in the case of (a)
> an fs which doesn't support xattrs,

You have two choices, really. The first is to treat the files on that
filesystem as having no xattrs, thus they have the inheritable behavior.
The alternative is to default to some value for the filesystem (Smack
does this) which may or may not be provided in the mount options.

> (2) expanding a tarball/cpio which
> didn't have xattrs (should tar/cpio fill them in with empty sets?),
> and

Files get no capability sets, hence the inheriting behavior.

> (3) do we add a default empty set in the case of an fs mounted with
> NOSUID?

No, I think that is the opposite of what NOSUID is trying to do.
For the capability behavior to match the setuid bit behavior all
files will be inheriting, as if they had no capability set. It would
be safer to pretend there is an empty set, but that's not what
NOSUID does.

> It's an interesting notion.

It's what we did in Trusted Irix. It made life much easier.

2015-02-03 17:18:18

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Tue, 3 Feb 2015, Serge E. Hallyn wrote:

> We've currently got two proposals. (Three includig yours; but I explained my
> problem with yours earlier this morning - do appreciate the proposal and
> the patch though, really, thanks) It's worth digging into the details of
> each, but if they end up complicating things then perhaps "dropping
> capabilities and going with something new" ought to be another proposal.

Ok that is about the binding to a person and executable?

So you think there should be a cap_inheritable mask settable in the caps
of each file.

2015-02-03 17:20:03

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Tue, 3 Feb 2015, Serge E. Hallyn wrote:

> So again I think the pA seems like an elegant way to work around this.
> I'm interested in other ideas, but I worry about the proc solution Christoph
> proposed because it would be system- or namespace-wide, rather than
> per-process.

Ok can we have a patch that realizes this? Maybe only a rough one?

2015-02-03 17:26:56

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Christoph Lameter ([email protected]):
> On Tue, 3 Feb 2015, Serge E. Hallyn wrote:
>
> > We've currently got two proposals. (Three includig yours; but I explained my
> > problem with yours earlier this morning - do appreciate the proposal and
> > the patch though, really, thanks) It's worth digging into the details of
> > each, but if they end up complicating things then perhaps "dropping
> > capabilities and going with something new" ought to be another proposal.
>
> Ok that is about the binding to a person and executable?

It's about at least making it per-process(-tree).

> So you think there should be a cap_inheritable mask settable in the caps
> of each file.

No. I mean, we have that now. I just want to require a privileged process
to fill in the pA in the first place. If people are currently using file
caps "as intended" I don't want behavior to change for them.

2015-02-03 17:28:41

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Casey Schaufler ([email protected]):
> On 2/3/2015 7:51 AM, Serge E. Hallyn wrote:
> > Quoting Casey Schaufler ([email protected]):
> >> On 2/2/2015 12:37 PM, Andy Lutomirski wrote:
> >>> On Mon, Feb 2, 2015 at 10:08 AM, Serge Hallyn <[email protected]> wrote:
> >>>> Quoting Casey Schaufler ([email protected]):
> >>>>> I'm game to participate in such an effort. The POSIX scheme
> >>>>> is workable, but given that it's 20 years old and hasn't
> >>>>> developed real traction it's hard to call it successful.
> >>>> Over the years we've several times discussed possible reasons for this
> >>>> and how to help. I personally think it's two things: 1. lack of
> >>>> toolchain and fs support. The fact that we cannot to this day enable
> >>>> ping using capabilities by default because of cpio, tar and non-xattr
> >>>> filesystems is disheartening. 2. It's hard for users and applications
> >>>> to know what caps they need. yes the API is a bear to use, but we can
> >>>> hide that behind fancier libraries. But using capabilities requires too
> >>>> much in-depth knowledge of precisely what caps you might need for
> >>>> whatever operations library may now do when you asked for something.
> >>> None of this could address the problem here, though: if I hold a
> >>> capability and I want to pass that capability to an exec'd helper, I
> >>> shouldn't need the fs's help to do this.
> >> One of the holes in the 1003.1e spec is what to do with a program file
> >> that does not have a capability set attached to it. The two options are
> >> drop all capabilities and leave the capabilities alone. The latter gives
> >> you what you're asking for. The former is arguably safer.
> > Hm, so if we were to change that, what should we do in the case of (a)
> > an fs which doesn't support xattrs,
>
> You have two choices, really. The first is to treat the files on that
> filesystem as having no xattrs, thus they have the inheritable behavior.
> The alternative is to default to some value for the filesystem (Smack
> does this) which may or may not be provided in the mount options.
>
> > (2) expanding a tarball/cpio which
> > didn't have xattrs (should tar/cpio fill them in with empty sets?),
> > and
>
> Files get no capability sets, hence the inheriting behavior.
>
> > (3) do we add a default empty set in the case of an fs mounted with
> > NOSUID?
>
> No, I think that is the opposite of what NOSUID is trying to do.
> For the capability behavior to match the setuid bit behavior all
> files will be inheriting, as if they had no capability set. It would
> be safer to pretend there is an empty set, but that's not what
> NOSUID does.
>
> > It's an interesting notion.
>
> It's what we did in Trusted Irix. It made life much easier.

Is there any chance you'd have time to write a patch to implement this?

(I wasn't going to ask bc I assumed not, but heck maybe you're bored
on a desert island or snowed in and just looking for an excuse to hack :)

-serge

2015-02-03 17:30:00

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Christoph Lameter ([email protected]):
> On Tue, 3 Feb 2015, Serge E. Hallyn wrote:
>
> > So again I think the pA seems like an elegant way to work around this.
> > I'm interested in other ideas, but I worry about the proc solution Christoph
> > proposed because it would be system- or namespace-wide, rather than
> > per-process.
>
> Ok can we have a patch that realizes this? Maybe only a rough one?

I can't work on one today, but if noone else gets to it I'd like to
try to get to it later this week.

Would be good to have patches for both approaches, to compare.

2015-02-03 17:50:13

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On 2/3/2015 9:28 AM, Serge E. Hallyn wrote:
> Quoting Casey Schaufler ([email protected]):
>> On 2/3/2015 7:51 AM, Serge E. Hallyn wrote:
>>> Quoting Casey Schaufler ([email protected]):
>>>> On 2/2/2015 12:37 PM, Andy Lutomirski wrote:
>>>>> On Mon, Feb 2, 2015 at 10:08 AM, Serge Hallyn <[email protected]> wrote:
>>>>>> Quoting Casey Schaufler ([email protected]):
>>>>>>> I'm game to participate in such an effort. The POSIX scheme
>>>>>>> is workable, but given that it's 20 years old and hasn't
>>>>>>> developed real traction it's hard to call it successful.
>>>>>> Over the years we've several times discussed possible reasons for this
>>>>>> and how to help. I personally think it's two things: 1. lack of
>>>>>> toolchain and fs support. The fact that we cannot to this day enable
>>>>>> ping using capabilities by default because of cpio, tar and non-xattr
>>>>>> filesystems is disheartening. 2. It's hard for users and applications
>>>>>> to know what caps they need. yes the API is a bear to use, but we can
>>>>>> hide that behind fancier libraries. But using capabilities requires too
>>>>>> much in-depth knowledge of precisely what caps you might need for
>>>>>> whatever operations library may now do when you asked for something.
>>>>> None of this could address the problem here, though: if I hold a
>>>>> capability and I want to pass that capability to an exec'd helper, I
>>>>> shouldn't need the fs's help to do this.
>>>> One of the holes in the 1003.1e spec is what to do with a program file
>>>> that does not have a capability set attached to it. The two options are
>>>> drop all capabilities and leave the capabilities alone. The latter gives
>>>> you what you're asking for. The former is arguably safer.
>>> Hm, so if we were to change that, what should we do in the case of (a)
>>> an fs which doesn't support xattrs,
>> You have two choices, really. The first is to treat the files on that
>> filesystem as having no xattrs, thus they have the inheritable behavior.
>> The alternative is to default to some value for the filesystem (Smack
>> does this) which may or may not be provided in the mount options.
>>
>>> (2) expanding a tarball/cpio which
>>> didn't have xattrs (should tar/cpio fill them in with empty sets?),
>>> and
>> Files get no capability sets, hence the inheriting behavior.
>>
>>> (3) do we add a default empty set in the case of an fs mounted with
>>> NOSUID?
>> No, I think that is the opposite of what NOSUID is trying to do.
>> For the capability behavior to match the setuid bit behavior all
>> files will be inheriting, as if they had no capability set. It would
>> be safer to pretend there is an empty set, but that's not what
>> NOSUID does.
>>
>>> It's an interesting notion.
>> It's what we did in Trusted Irix. It made life much easier.
> Is there any chance you'd have time to write a patch to implement this?

Woof. I'll at least take a look.

>
> (I wasn't going to ask bc I assumed not, but heck maybe you're bored
> on a desert island or snowed in and just looking for an excuse to hack :)

Not at all bored, but I think this could be important.

>
> -serge
>

2015-02-03 19:45:45

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Tue, 3 Feb 2015, Casey Schaufler wrote:

> > (I wasn't going to ask bc I assumed not, but heck maybe you're bored
> > on a desert island or snowed in and just looking for an excuse to hack :)
>
> Not at all bored, but I think this could be important.

Ok here is a draft of a patch that follows these ideas.

It also adds an ambient field and sets the field if a new capability

CAP_AMBIENT_MASK

is set. The perm calcualtion is as suggested by Serge.

If CAP_AMBIENT_MASK is set the inheritable caps become the ambient ones.

If it is not set then the ambient caps are copied from the parent.

DRAFT --- not a working patch:

Index: linux/include/linux/capability.h
===================================================================
--- linux.orig/include/linux/capability.h 2015-02-03 13:25:03.000000000 -0600
+++ linux/include/linux/capability.h 2015-02-03 13:39:23.385424676 -0600
@@ -29,6 +29,7 @@ struct cpu_vfs_cap_data {
__u32 magic_etc;
kernel_cap_t permitted;
kernel_cap_t inheritable;
+ kernel_cap_t ambient;
};

#define _USER_CAP_HEADER_SIZE (sizeof(struct __user_cap_header_struct))
Index: linux/include/uapi/linux/capability.h
===================================================================
--- linux.orig/include/uapi/linux/capability.h 2014-07-10 16:10:29.814424392 -0500
+++ linux/include/uapi/linux/capability.h 2015-02-03 13:26:13.231081452 -0600
@@ -351,8 +351,10 @@ struct vfs_cap_data {

#define CAP_AUDIT_READ 37

+/* Set the current inheritable mask as the ambient inheritable mask */
+#define CAP_AMBIENT_MASK 38

-#define CAP_LAST_CAP CAP_AUDIT_READ
+#define CAP_LAST_CAP CAP_AMBIENT_MASK

#define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)

Index: linux/security/commoncap.c
===================================================================
--- linux.orig/security/commoncap.c 2015-02-03 13:25:03.000000000 -0600
+++ linux/security/commoncap.c 2015-02-03 13:43:16.317859741 -0600
@@ -349,17 +349,24 @@ static inline int bprm_caps_from_vfs_cap
CAP_FOR_EACH_U32(i) {
__u32 permitted = caps->permitted.cap[i];
__u32 inheritable = caps->inheritable.cap[i];
+ __u32 ambient = caps->ambient.cap[i];

/*
* pP' = (X & fP) | (pI & fI)
*/
new->cap_permitted.cap[i] =
(new->cap_bset.cap[i] & permitted) |
- (new->cap_inheritable.cap[i] & inheritable);
+ (new->cap_inheritable.cap[i] & inheritable) |
+ (ambient & inheritable);

if (permitted & ~new->cap_permitted.cap[i])
/* insufficient to execute correctly */
ret = -EPERM;
+
+ if (capable(CAP_AMBIENT_MASK))
+ new->cap_ambient.cap[i] = inheritable;
+ else
+ new->cap_ambient.cap[i] = ambient;
}

/*
Index: linux/include/linux/cred.h
===================================================================
--- linux.orig/include/linux/cred.h 2014-12-18 11:17:49.731948737 -0600
+++ linux/include/linux/cred.h 2015-02-03 13:37:32.701019201 -0600
@@ -122,6 +122,7 @@ struct cred {
kernel_cap_t cap_permitted; /* caps we're permitted */
kernel_cap_t cap_effective; /* caps we can actually use */
kernel_cap_t cap_bset; /* capability bounding set */
+ kernel_cap_t cap_ambient; /* Ambient inherited caps */
#ifdef CONFIG_KEYS
unsigned char jit_keyring; /* default keyring to attach requested
* keys to */

2015-02-03 20:13:34

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Tue, Feb 3, 2015 at 11:45 AM, Christoph Lameter <[email protected]> wrote:
> On Tue, 3 Feb 2015, Casey Schaufler wrote:
>
>> > (I wasn't going to ask bc I assumed not, but heck maybe you're bored
>> > on a desert island or snowed in and just looking for an excuse to hack :)
>>
>> Not at all bored, but I think this could be important.
>
> Ok here is a draft of a patch that follows these ideas.
>
> It also adds an ambient field and sets the field if a new capability
>
> CAP_AMBIENT_MASK
>
> is set. The perm calcualtion is as suggested by Serge.
>
>
> If CAP_AMBIENT_MASK is set the inheritable caps become the ambient ones.
>
> If it is not set then the ambient caps are copied from the parent.
>
>
> DRAFT --- not a working patch:
>
> Index: linux/include/linux/capability.h
> ===================================================================
> --- linux.orig/include/linux/capability.h 2015-02-03 13:25:03.000000000 -0600
> +++ linux/include/linux/capability.h 2015-02-03 13:39:23.385424676 -0600
> @@ -29,6 +29,7 @@ struct cpu_vfs_cap_data {
> __u32 magic_etc;
> kernel_cap_t permitted;
> kernel_cap_t inheritable;
> + kernel_cap_t ambient;
> };
>
> #define _USER_CAP_HEADER_SIZE (sizeof(struct __user_cap_header_struct))
> Index: linux/include/uapi/linux/capability.h
> ===================================================================
> --- linux.orig/include/uapi/linux/capability.h 2014-07-10 16:10:29.814424392 -0500
> +++ linux/include/uapi/linux/capability.h 2015-02-03 13:26:13.231081452 -0600
> @@ -351,8 +351,10 @@ struct vfs_cap_data {
>
> #define CAP_AUDIT_READ 37
>
> +/* Set the current inheritable mask as the ambient inheritable mask */
> +#define CAP_AMBIENT_MASK 38
>
> -#define CAP_LAST_CAP CAP_AUDIT_READ
> +#define CAP_LAST_CAP CAP_AMBIENT_MASK
>
> #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
>
> Index: linux/security/commoncap.c
> ===================================================================
> --- linux.orig/security/commoncap.c 2015-02-03 13:25:03.000000000 -0600
> +++ linux/security/commoncap.c 2015-02-03 13:43:16.317859741 -0600
> @@ -349,17 +349,24 @@ static inline int bprm_caps_from_vfs_cap
> CAP_FOR_EACH_U32(i) {
> __u32 permitted = caps->permitted.cap[i];
> __u32 inheritable = caps->inheritable.cap[i];
> + __u32 ambient = caps->ambient.cap[i];
>
> /*
> * pP' = (X & fP) | (pI & fI)
> */
> new->cap_permitted.cap[i] =
> (new->cap_bset.cap[i] & permitted) |
> - (new->cap_inheritable.cap[i] & inheritable);
> + (new->cap_inheritable.cap[i] & inheritable) |
> + (ambient & inheritable);

Is there a clear reason why no non-permitted bits can be inheritable?
If not, then I think this should be (ambient & inheritable &
permitted).

Do we need to think about the effective mask here? What happens when
we exec a setuid program or a program with a non-empty fP set? I
think that, in these cases, we should strongly consider clearing the
ambient set. For a different approach, see below.

>
> if (permitted & ~new->cap_permitted.cap[i])
> /* insufficient to execute correctly */
> ret = -EPERM;
> +
> + if (capable(CAP_AMBIENT_MASK))
> + new->cap_ambient.cap[i] = inheritable;
> + else
> + new->cap_ambient.cap[i] = ambient;

IMO this is really weird. I don't think that the presence of an
effective cap should change the cap equations. (Also, that should be
nsown_capable.)

Can we please make this an explicit opt-in? For example, allow a
process to set an ambient cap bit if that bit is both permitted and
inheritable. I'd prefer having it be a single control, though -- just
prctl(PR_SET_ALWAYS_INHERIT_CAPS, 1, 0, 0, 0) would set a single bit
that would cause all inheritable bits to be treated as ambient.

Here's a slight variant that might be more clearly safe: add an
inherited per-process bit that causes all files to act as though fI is
the full set. Only allow setting that bit if no_new_privs is set.

--Andy

2015-02-03 23:14:10

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Tue, 3 Feb 2015, Andy Lutomirski wrote:

> > */
> > new->cap_permitted.cap[i] =
> > (new->cap_bset.cap[i] & permitted) |
> > - (new->cap_inheritable.cap[i] & inheritable);
> > + (new->cap_inheritable.cap[i] & inheritable) |
> > + (ambient & inheritable);
>
> Is there a clear reason why no non-permitted bits can be inheritable?
> If not, then I think this should be (ambient & inheritable &
> permitted).

Inherited caps via ambient are always be permitted. Otherwise the pass
through is not working.

> Do we need to think about the effective mask here? What happens when
> we exec a setuid program or a program with a non-empty fP set? I
> think that, in these cases, we should strongly consider clearing the
> ambient set. For a different approach, see below.
>
> >
> > if (permitted & ~new->cap_permitted.cap[i])
> > /* insufficient to execute correctly */
> > ret = -EPERM;
> > +
> > + if (capable(CAP_AMBIENT_MASK))
> > + new->cap_ambient.cap[i] = inheritable;
> > + else
> > + new->cap_ambient.cap[i] = ambient;
>
> IMO this is really weird. I don't think that the presence of an
> effective cap should change the cap equations. (Also, that should be
> nsown_capable.)

Well how would the ambient mask to be set? The other options are adding a
new syscall and having to go through an interation of the capabilities
tools and/or kernel syscall API changes.

> Can we please make this an explicit opt-in? For example, allow a
> process to set an ambient cap bit if that bit is both permitted and
> inheritable. I'd prefer having it be a single control, though -- just
> prctl(PR_SET_ALWAYS_INHERIT_CAPS, 1, 0, 0, 0) would set a single bit
> that would cause all inheritable bits to be treated as ambient.

Opt-in does not work since the caps need to be passed
through binaries that do not use the capabilities.

> Here's a slight variant that might be more clearly safe: add an
> inherited per-process bit that causes all files to act as though fI is
> the full set. Only allow setting that bit if no_new_privs is set.

CAP_INHERIT_ALL ?

2015-02-03 23:18:11

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Tue, Feb 3, 2015 at 3:14 PM, Christoph Lameter <[email protected]> wrote:
> On Tue, 3 Feb 2015, Andy Lutomirski wrote:
>
>> > */
>> > new->cap_permitted.cap[i] =
>> > (new->cap_bset.cap[i] & permitted) |
>> > - (new->cap_inheritable.cap[i] & inheritable);
>> > + (new->cap_inheritable.cap[i] & inheritable) |
>> > + (ambient & inheritable);
>>
>> Is there a clear reason why no non-permitted bits can be inheritable?
>> If not, then I think this should be (ambient & inheritable &
>> permitted).
>
> Inherited caps via ambient are always be permitted. Otherwise the pass
> through is not working.

Sure, but what about inheritable caps before exec? Suppose I drop
some cap from the permitted set but leave it in the inheritable set.
I shouldn't get it back by calling execve.

>
>> Do we need to think about the effective mask here? What happens when
>> we exec a setuid program or a program with a non-empty fP set? I
>> think that, in these cases, we should strongly consider clearing the
>> ambient set. For a different approach, see below.
>>
>> >
>> > if (permitted & ~new->cap_permitted.cap[i])
>> > /* insufficient to execute correctly */
>> > ret = -EPERM;
>> > +
>> > + if (capable(CAP_AMBIENT_MASK))
>> > + new->cap_ambient.cap[i] = inheritable;
>> > + else
>> > + new->cap_ambient.cap[i] = ambient;
>>
>> IMO this is really weird. I don't think that the presence of an
>> effective cap should change the cap equations. (Also, that should be
>> nsown_capable.)
>
> Well how would the ambient mask to be set? The other options are adding a
> new syscall and having to go through an interation of the capabilities
> tools and/or kernel syscall API changes.

prctl?

>
>> Can we please make this an explicit opt-in? For example, allow a
>> process to set an ambient cap bit if that bit is both permitted and
>> inheritable. I'd prefer having it be a single control, though -- just
>> prctl(PR_SET_ALWAYS_INHERIT_CAPS, 1, 0, 0, 0) would set a single bit
>> that would cause all inheritable bits to be treated as ambient.
>
> Opt-in does not work since the caps need to be passed
> through binaries that do not use the capabilities.
>
>> Here's a slight variant that might be more clearly safe: add an
>> inherited per-process bit that causes all files to act as though fI is
>> the full set. Only allow setting that bit if no_new_privs is set.
>
> CAP_INHERIT_ALL ?
>

Sure. Would this approach work for your use case? It would work for
mine, and it avoids needing to think about how this new kind of
inheritance would interact with setuid, setgid, and file caps (i.e. it
wouldn't, because you have to turn on off to get the other).

Opt-in should work fine as long as the opt-in is inherited.

--Andy

--
Andy Lutomirski
AMA Capital Management, LLC

2015-02-04 02:27:37

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Tue, 3 Feb 2015, Andy Lutomirski wrote:

> >> Is there a clear reason why no non-permitted bits can be inheritable?
> >> If not, then I think this should be (ambient & inheritable &
> >> permitted).
> >
> > Inherited caps via ambient are always be permitted. Otherwise the pass
> > through is not working.
>
> Sure, but what about inheritable caps before exec? Suppose I drop
> some cap from the permitted set but leave it in the inheritable set.
> I shouldn't get it back by calling execve.

You should get it back because there may be executables invoked by that
binary that need it.

> > Well how would the ambient mask to be set? The other options are adding a
> > new syscall and having to go through an interation of the capabilities
> > tools and/or kernel syscall API changes.
>
> prctl?

Hmmmm... Ok did not think about that so far.

> >> Here's a slight variant that might be more clearly safe: add an
> >> inherited per-process bit that causes all files to act as though fI is
> >> the full set. Only allow setting that bit if no_new_privs is set.
> >
> > CAP_INHERIT_ALL ?
> >
>
> Sure. Would this approach work for your use case? It would work for
> mine, and it avoids needing to think about how this new kind of
> inheritance would interact with setuid, setgid, and file caps (i.e. it
> wouldn't, because you have to turn on off to get the other).
>
> Opt-in should work fine as long as the opt-in is inherited.

Well then its no longer an optin but automatic.

Sure that would work. The patch also should not be too difficult to do.

2015-02-04 06:11:29

by Markku Savela

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Just a note... We had inheritable capabilities in the linux of Nokia N9
phone.

If a program needed some capabilities, they had to be requested by the
manifest file inside the debian package. Of course, request is only
granted if the package origin had permission to grant them.

2015-02-04 13:17:24

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Wed, 4 Feb 2015, Markku Savela wrote:

> Just a note... We had inheritable capabilities in the linux of Nokia N9 phone.

Could we review the patch please?

2015-02-04 13:41:15

by Markku Savela

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On 04/02/15 15:17, Christoph Lameter wrote:
> On Wed, 4 Feb 2015, Markku Savela wrote:
>
>> Just a note... We had inheritable capabilities in the linux of Nokia N9 phone.
>
> Could we review the patch please?

Unfortunately, I don't have it and with quick search only found "N9 MER"
kernels, where the Aegis part (which included the inheritance logic) has
been removed. It had many other things too, which made it a bit complex
(there are some related messages by me on kernel mailing lists).

I have been out the picture since project was canceled and I didn't save
the kernel git. I suppose Nokia is obliged to distribute the source, but
I guess it comes as big tar ball with all git history lost...

I suppose someone could still have it as git repository somewhere?

2015-02-04 14:57:29

by Jarkko Sakkinen

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Wed, 2015-02-04 at 15:41 +0200, Markku Savela wrote:
> On 04/02/15 15:17, Christoph Lameter wrote:
> > On Wed, 4 Feb 2015, Markku Savela wrote:
> >
> >> Just a note... We had inheritable capabilities in the linux of Nokia N9 phone.
> >
> > Could we review the patch please?
>
> Unfortunately, I don't have it and with quick search only found "N9 MER"
> kernels, where the Aegis part (which included the inheritance logic) has
> been removed. It had many other things too, which made it a bit complex
> (there are some related messages by me on kernel mailing lists).
>
> I have been out the picture since project was canceled and I didn't save
> the kernel git. I suppose Nokia is obliged to distribute the source, but
> I guess it comes as big tar ball with all git history lost...
>
> I suppose someone could still have it as git repository somewhere?

I was able to find this w/ a simple google search: "aegis mssf":

https://gitorious.org/meego-platform-security/linux-mssf/

/Jarkko

2015-02-04 15:15:37

by Andrew G. Morgan

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

I'm not generally in favor of this. Mostly because this seems to be a
mini-root kind of inheritance that propagates privilege to binaries
that aren't prepared for privilege. I don't really buy the mmap code
concern because the model as it stands says that you trust the binary
(and all of the various ways it was programmed to execute code) with
specific privileges. If the binary mmap's some code (PAM modules come
to mind) then that is part of what it was programmed to/allowed to do.

That being said, if you really really want this kind of thing, then
make it a single secure bit (with another lock on/off bit) which, when
set, makes: fI default to X.

pP' = (X & fP) | (pI & fI)

That way the per-process bounding set still works as advertised and
you don't need to worry about the existing semantics being violated.

Cheers

Andrew

On Tue, Feb 3, 2015 at 9:26 AM, Serge E. Hallyn <[email protected]> wrote:
> Quoting Christoph Lameter ([email protected]):
>> On Tue, 3 Feb 2015, Serge E. Hallyn wrote:
>>
>> > We've currently got two proposals. (Three includig yours; but I explained my
>> > problem with yours earlier this morning - do appreciate the proposal and
>> > the patch though, really, thanks) It's worth digging into the details of
>> > each, but if they end up complicating things then perhaps "dropping
>> > capabilities and going with something new" ought to be another proposal.
>>
>> Ok that is about the binding to a person and executable?
>
> It's about at least making it per-process(-tree).
>
>> So you think there should be a cap_inheritable mask settable in the caps
>> of each file.
>
> No. I mean, we have that now. I just want to require a privileged process
> to fill in the pA in the first place. If people are currently using file
> caps "as intended" I don't want behavior to change for them.

2015-02-04 15:50:09

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Wed, 4 Feb 2015, Andrew G. Morgan wrote:

> I'm not generally in favor of this. Mostly because this seems to be a
> mini-root kind of inheritance that propagates privilege to binaries
> that aren't prepared for privilege. I don't really buy the mmap code
> concern because the model as it stands says that you trust the binary
> (and all of the various ways it was programmed to execute code) with
> specific privileges. If the binary mmap's some code (PAM modules come
> to mind) then that is part of what it was programmed to/allowed to do.
>
> That being said, if you really really want this kind of thing, then
> make it a single secure bit (with another lock on/off bit) which, when
> set, makes: fI default to X.
>
> pP' = (X & fP) | (pI & fI)
>
> That way the per-process bounding set still works as advertised and
> you don't need to worry about the existing semantics being violated.

Ok but then also fI needs to be set to X so that the binary f invokes
can also inherit. So if we copy the inheritable flags to fI then we
wont be needing the bounding set anymore.

The changes to brpm_caps_from_vfs_cap would then
be only the following? (substitute capable(CAP_INHERIT_BY_DEFAULT through
any other means like PRCTL if wanted).

Index: linux/security/commoncap.c
===================================================================
--- linux.orig/security/commoncap.c 2015-02-04 09:44:25.000000000 -0600
+++ linux/security/commoncap.c 2015-02-04 09:45:59.381572756 -0600
@@ -350,6 +350,9 @@ static inline int bprm_caps_from_vfs_cap
__u32 permitted = caps->permitted.cap[i];
__u32 inheritable = caps->inheritable.cap[i];

+ if (capable(CAP_INHERIT_BY_DEFAULT)
+ new->cap_inheritable.cap[i] = inheritable;
+
/*
* pP' = (X & fP) | (pI & fI)
*/

2015-02-04 15:56:26

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Christoph Lameter ([email protected]):
> On Wed, 4 Feb 2015, Andrew G. Morgan wrote:
>
> > I'm not generally in favor of this. Mostly because this seems to be a
> > mini-root kind of inheritance that propagates privilege to binaries
> > that aren't prepared for privilege. I don't really buy the mmap code
> > concern because the model as it stands says that you trust the binary
> > (and all of the various ways it was programmed to execute code) with
> > specific privileges. If the binary mmap's some code (PAM modules come
> > to mind) then that is part of what it was programmed to/allowed to do.
> >
> > That being said, if you really really want this kind of thing, then
> > make it a single secure bit (with another lock on/off bit) which, when
> > set, makes: fI default to X.
> >
> > pP' = (X & fP) | (pI & fI)
> >
> > That way the per-process bounding set still works as advertised and
> > you don't need to worry about the existing semantics being violated.
>
> Ok but then also fI needs to be set to X so that the binary f invokes
> can also inherit. So if we copy the inheritable flags to fI then we
> wont be needing the bounding set anymore.
>
> The changes to brpm_caps_from_vfs_cap would then
> be only the following? (substitute capable(CAP_INHERIT_BY_DEFAULT through
> any other means like PRCTL if wanted).
>
>
> Index: linux/security/commoncap.c
> ===================================================================
> --- linux.orig/security/commoncap.c 2015-02-04 09:44:25.000000000 -0600
> +++ linux/security/commoncap.c 2015-02-04 09:45:59.381572756 -0600
> @@ -350,6 +350,9 @@ static inline int bprm_caps_from_vfs_cap
> __u32 permitted = caps->permitted.cap[i];
> __u32 inheritable = caps->inheritable.cap[i];
>
> + if (capable(CAP_INHERIT_BY_DEFAULT)
> + new->cap_inheritable.cap[i] = inheritable;
> +
> /*
> * pP' = (X & fP) | (pI & fI)
> */

Not quite - I think more like

if (secure(SECURE_AMBIENT_PRIVS))
new->cap_inheritable.cap[i] = inheritable;

Then ns_capable(CAP_INHERIT_BY_DEFAULT), or perhaps rather
ns_capable(CAP_SETPCAP), would be required in order to set
SECURE_AMBIENT_PRIVS, which is off by default.

2015-02-04 16:12:08

by Andrew G. Morgan

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

I was thinking more like this:

int override = secure(SECURE_AMBIENT_PRIVS) &&
cap_isclear(caps->inheritable.cap);

CAP_FOR_EACH_U32(i) {
__u32 permitted = caps->permitted.cap[i];
__u32 inheritable = override ? new->cap_bset.cap[i] :
caps->inheritable.cap[i];
[...]

Cheers

Andrew

On Wed, Feb 4, 2015 at 7:56 AM, Serge E. Hallyn <[email protected]> wrote:
> Quoting Christoph Lameter ([email protected]):
>> On Wed, 4 Feb 2015, Andrew G. Morgan wrote:
>>
>> > I'm not generally in favor of this. Mostly because this seems to be a
>> > mini-root kind of inheritance that propagates privilege to binaries
>> > that aren't prepared for privilege. I don't really buy the mmap code
>> > concern because the model as it stands says that you trust the binary
>> > (and all of the various ways it was programmed to execute code) with
>> > specific privileges. If the binary mmap's some code (PAM modules come
>> > to mind) then that is part of what it was programmed to/allowed to do.
>> >
>> > That being said, if you really really want this kind of thing, then
>> > make it a single secure bit (with another lock on/off bit) which, when
>> > set, makes: fI default to X.
>> >
>> > pP' = (X & fP) | (pI & fI)
>> >
>> > That way the per-process bounding set still works as advertised and
>> > you don't need to worry about the existing semantics being violated.
>>
>> Ok but then also fI needs to be set to X so that the binary f invokes
>> can also inherit. So if we copy the inheritable flags to fI then we
>> wont be needing the bounding set anymore.
>>
>> The changes to brpm_caps_from_vfs_cap would then
>> be only the following? (substitute capable(CAP_INHERIT_BY_DEFAULT through
>> any other means like PRCTL if wanted).
>>
>>
>> Index: linux/security/commoncap.c
>> ===================================================================
>> --- linux.orig/security/commoncap.c 2015-02-04 09:44:25.000000000 -0600
>> +++ linux/security/commoncap.c 2015-02-04 09:45:59.381572756 -0600
>> @@ -350,6 +350,9 @@ static inline int bprm_caps_from_vfs_cap
>> __u32 permitted = caps->permitted.cap[i];
>> __u32 inheritable = caps->inheritable.cap[i];
>>
>> + if (capable(CAP_INHERIT_BY_DEFAULT)
>> + new->cap_inheritable.cap[i] = inheritable;
>> +
>> /*
>> * pP' = (X & fP) | (pI & fI)
>> */
>
> Not quite - I think more like
>
> if (secure(SECURE_AMBIENT_PRIVS))
> new->cap_inheritable.cap[i] = inheritable;
>
> Then ns_capable(CAP_INHERIT_BY_DEFAULT), or perhaps rather
> ns_capable(CAP_SETPCAP), would be required in order to set
> SECURE_AMBIENT_PRIVS, which is off by default.

2015-02-04 16:27:43

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Feb 4, 2015 7:56 AM, "Serge E. Hallyn" <[email protected]> wrote:
>
> Quoting Christoph Lameter ([email protected]):
> > On Wed, 4 Feb 2015, Andrew G. Morgan wrote:
> >
> > > I'm not generally in favor of this. Mostly because this seems to be a
> > > mini-root kind of inheritance that propagates privilege to binaries
> > > that aren't prepared for privilege. I don't really buy the mmap code
> > > concern because the model as it stands says that you trust the binary
> > > (and all of the various ways it was programmed to execute code) with
> > > specific privileges. If the binary mmap's some code (PAM modules come
> > > to mind) then that is part of what it was programmed to/allowed to do.
> > >
> > > That being said, if you really really want this kind of thing, then
> > > make it a single secure bit (with another lock on/off bit) which, when
> > > set, makes: fI default to X.
> > >
> > > pP' = (X & fP) | (pI & fI)
> > >
> > > That way the per-process bounding set still works as advertised and
> > > you don't need to worry about the existing semantics being violated.
> >
> > Ok but then also fI needs to be set to X so that the binary f invokes
> > can also inherit. So if we copy the inheritable flags to fI then we
> > wont be needing the bounding set anymore.
> >
> > The changes to brpm_caps_from_vfs_cap would then
> > be only the following? (substitute capable(CAP_INHERIT_BY_DEFAULT through
> > any other means like PRCTL if wanted).
> >
> >
> > Index: linux/security/commoncap.c
> > ===================================================================
> > --- linux.orig/security/commoncap.c 2015-02-04 09:44:25.000000000 -0600
> > +++ linux/security/commoncap.c 2015-02-04 09:45:59.381572756 -0600
> > @@ -350,6 +350,9 @@ static inline int bprm_caps_from_vfs_cap
> > __u32 permitted = caps->permitted.cap[i];
> > __u32 inheritable = caps->inheritable.cap[i];
> >
> > + if (capable(CAP_INHERIT_BY_DEFAULT)
> > + new->cap_inheritable.cap[i] = inheritable;
> > +
> > /*
> > * pP' = (X & fP) | (pI & fI)
> > */
>
> Not quite - I think more like
>
> if (secure(SECURE_AMBIENT_PRIVS))
> new->cap_inheritable.cap[i] = inheritable;

I *still* think this should be inheritable & permitted.

>
> Then ns_capable(CAP_INHERIT_BY_DEFAULT), or perhaps rather
> ns_capable(CAP_SETPCAP), would be required in order to set
> SECURE_AMBIENT_PRIVS, which is off by default.

Can we make this depend on no_new_privs instead of a new cap? I don't
want to see people leaking this securebit into the environment for
reasons they think are good a la CVE-2014-3215. I sincerely doubt
that running, say, sendmail or exim with this bit set and no_new_privs
off is a good idea.

Hmm. On an unrelated note, we should consider allowing no_new_privs
to be cleared in conjunction with unsharing userns.

--Andy

2015-02-04 16:34:55

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Wed, Feb 4, 2015 at 8:12 AM, Andrew G. Morgan <[email protected]> wrote:
> I was thinking more like this:
>
> int override = secure(SECURE_AMBIENT_PRIVS) &&
> cap_isclear(caps->inheritable.cap);
>
> CAP_FOR_EACH_U32(i) {
> __u32 permitted = caps->permitted.cap[i];
> __u32 inheritable = override ? new->cap_bset.cap[i] :
> caps->inheritable.cap[i];
> [...]

To elaborate on my objection:

For better or for worse, as a practical matter, if you drop a cap from
pP but keep it in pI, there's no way to get that cap back on the
average system to get that cap back using execve because nothing will
have that bit set in fI. I am not at all confident that changing this
is safe at this point, since there's lots of legacy code out there.

So, how about:

__u32 inheritable = override ? (new->cap_bset.cap[i] & permitted) :
caps->inheritable.cap[i];

instead?

This still doesn't address the effective set adequately, I think. I
suspect that we'll want to always start with pE' == pP' in the new
mode, or perhaps pE' = (pP' & pE). This latter part is also a bit
dangerous and furthers my desire to restrict this to no_new_privs.

--Andy

2015-02-04 16:43:19

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Wed, 4 Feb 2015, Andrew G. Morgan wrote:

> I was thinking more like this:
>
> int override = secure(SECURE_AMBIENT_PRIVS) &&
> cap_isclear(caps->inheritable.cap);

Uhh.. Then processes that require other capabilties would not pass
them through anymore to other stuff that they invoke.

Also the new caps need to be set somewhere.

2015-02-04 16:54:41

by Andrew G. Morgan

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

If permitted is zero (ie., no file capabilities) then I don't think
this will yield any privilege for such an exec. Perhaps I missed
something prior to being included in the thread, but I was under the
impression that this was a case where it was intended that
capabilities would be inherited..?

If you force pE' too, then this looks more like a mini-root
inheritance which gets me closer to disliking this: you need to
consider that we dangerously close to returning to situations like the
one discussed here:
https://sites.google.com/site/fullycapable/Home/thesendmailcapabilitiesissue

Cheers

Andrew

On Wed, Feb 4, 2015 at 8:34 AM, Andy Lutomirski <[email protected]> wrote:
> On Wed, Feb 4, 2015 at 8:12 AM, Andrew G. Morgan <[email protected]> wrote:
>> I was thinking more like this:
>>
>> int override = secure(SECURE_AMBIENT_PRIVS) &&
>> cap_isclear(caps->inheritable.cap);
>>
>> CAP_FOR_EACH_U32(i) {
>> __u32 permitted = caps->permitted.cap[i];
>> __u32 inheritable = override ? new->cap_bset.cap[i] :
>> caps->inheritable.cap[i];
>> [...]
>
> To elaborate on my objection:
>
> For better or for worse, as a practical matter, if you drop a cap from
> pP but keep it in pI, there's no way to get that cap back on the
> average system to get that cap back using execve because nothing will
> have that bit set in fI. I am not at all confident that changing this
> is safe at this point, since there's lots of legacy code out there.
>
> So, how about:
>
> __u32 inheritable = override ? (new->cap_bset.cap[i] & permitted) :
> caps->inheritable.cap[i];
>
> instead?
>
> This still doesn't address the effective set adequately, I think. I
> suspect that we'll want to always start with pE' == pP' in the new
> mode, or perhaps pE' = (pP' & pE). This latter part is also a bit
> dangerous and furthers my desire to restrict this to no_new_privs.
>
> --Andy

2015-02-04 17:34:51

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Ok this is all sounding too magic for my tastes. My original suggestion
was for an actual new capability set, pA, empty by default. You can
add bits to it using prctl if ns_capable(CAP_SETPCAP) and all the new bits are
in your pE. Once set, they stay until they are removed using prctl. At
exec, pA' = pA, and fI |= pA (after reading fI from disk but before
calculating pI').

Quoting Andrew G. Morgan ([email protected]):
> If permitted is zero (ie., no file capabilities) then I don't think
> this will yield any privilege for such an exec. Perhaps I missed
> something prior to being included in the thread, but I was under the
> impression that this was a case where it was intended that
> capabilities would be inherited..?
>
> If you force pE' too, then this looks more like a mini-root
> inheritance which gets me closer to disliking this: you need to
> consider that we dangerously close to returning to situations like the
> one discussed here:
> https://sites.google.com/site/fullycapable/Home/thesendmailcapabilitiesissue
>
> Cheers
>
> Andrew
>
>
> On Wed, Feb 4, 2015 at 8:34 AM, Andy Lutomirski <[email protected]> wrote:
> > On Wed, Feb 4, 2015 at 8:12 AM, Andrew G. Morgan <[email protected]> wrote:
> >> I was thinking more like this:
> >>
> >> int override = secure(SECURE_AMBIENT_PRIVS) &&
> >> cap_isclear(caps->inheritable.cap);
> >>
> >> CAP_FOR_EACH_U32(i) {
> >> __u32 permitted = caps->permitted.cap[i];
> >> __u32 inheritable = override ? new->cap_bset.cap[i] :
> >> caps->inheritable.cap[i];
> >> [...]
> >
> > To elaborate on my objection:
> >
> > For better or for worse, as a practical matter, if you drop a cap from
> > pP but keep it in pI, there's no way to get that cap back on the
> > average system to get that cap back using execve because nothing will
> > have that bit set in fI. I am not at all confident that changing this
> > is safe at this point, since there's lots of legacy code out there.
> >
> > So, how about:
> >
> > __u32 inheritable = override ? (new->cap_bset.cap[i] & permitted) :
> > caps->inheritable.cap[i];
> >
> > instead?
> >
> > This still doesn't address the effective set adequately, I think. I
> > suspect that we'll want to always start with pE' == pP' in the new
> > mode, or perhaps pE' = (pP' & pE). This latter part is also a bit
> > dangerous and furthers my desire to restrict this to no_new_privs.
> >
> > --Andy

2015-02-04 18:12:10

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Wed, 4 Feb 2015, Serge E. Hallyn wrote:

> Ok this is all sounding too magic for my tastes. My original suggestion
> was for an actual new capability set, pA, empty by default. You can
> add bits to it using prctl if ns_capable(CAP_SETPCAP) and all the new bits are
> in your pE. Once set, they stay until they are removed using prctl. At
> exec, pA' = pA, and fI |= pA (after reading fI from disk but before
> calculating pI').

Sounds good. I think we cannot avoid the additional capability set in the
cred structure since otherwise the regular ("crippled") inheritance bits
may be modified and then not passed on correctly.

2015-02-05 00:20:46

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Casey Schaufler ([email protected]):
> On 2/2/2015 10:08 AM, Serge Hallyn wrote:
> > Quoting Casey Schaufler ([email protected]):
> >> I'm game to participate in such an effort. The POSIX scheme
> >> is workable, but given that it's 20 years old and hasn't
> >> developed real traction it's hard to call it successful.
> > Over the years we've several times discussed possible reasons for this
> > and how to help. I personally think it's two things: 1. lack of
> > toolchain and fs support. The fact that we cannot to this day enable
> > ping using capabilities by default because of cpio, tar and non-xattr
> > filesystems is disheartening. 2. It's hard for users and applications
> > to know what caps they need. yes the API is a bear to use, but we can
> > hide that behind fancier libraries. But using capabilities requires too
> > much in-depth knowledge of precisely what caps you might need for
> > whatever operations library may now do when you asked for something.
>
> The fix for that is to a change to the audit system. If the audit system
> reported the capabilities relevant to the decision you'd have what you
> need. If you failed because you didn't have CAP_CHMOD or you succeeded
> because you had CAP_SYS_ADMIN it should show up in the audit record.
> Other systems have used this approach.
>
> You could, of course, create a separate capability result log, and I
> believe that Nokia had done something along those lines. I think that
> adding it to the audit trail is a more rational approach.

I wonder how much that would end up affecting performance, assuming it
left an audit message at every ns_capable() failure. Even if it was
only done under a certain sysctl, it could certainly be useful.

2015-02-05 00:34:40

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Andrew G. Morgan ([email protected]):
> I'm not generally in favor of this. Mostly because this seems to be a
> mini-root kind of inheritance that propagates privilege to binaries
> that aren't prepared for privilege.

Earlier in this thread, Casey said:

| One of the holes in the 1003.1e spec is what to do with a program file
| that does not have a capability set attached to it. The two options are
| drop all capabilities and leave the capabilities alone. The latter gives
| you what you're asking for. The former is arguably safer.

and

| It's what we did in Trusted Irix. It made life much easier.

I'm going to need to clear my head a bit before I try to compare that to
the root cause of the sendmail capabilities bug.

> I don't really buy the mmap code
> concern because the model as it stands says that you trust the binary
> (and all of the various ways it was programmed to execute code) with
> specific privileges. If the binary mmap's some code (PAM modules come
> to mind) then that is part of what it was programmed to/allowed to do.

That's not really the point... The point is that yes, a mini-root is
exactly what is being asked for :) I'm not saying I expect an adversary
to do the mmap+jump, but that currently it is a, and the only, way to
do unprivileged userid with retaining some privileges to run legacy
programs.

> That being said, if you really really want this kind of thing, then
> make it a single secure bit (with another lock on/off bit) which, when
> set, makes: fI default to X.
>
> pP' = (X & fP) | (pI & fI)
>
> That way the per-process bounding set still works as advertised and
> you don't need to worry about the existing semantics being violated.

Maybe that is the way to go...

-serge

2015-02-05 15:23:32

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Quoting Serge E. Hallyn ([email protected]):
> Quoting Andrew G. Morgan ([email protected]):
> > I'm not generally in favor of this. Mostly because this seems to be a
> > mini-root kind of inheritance that propagates privilege to binaries
> > that aren't prepared for privilege.
>
> Earlier in this thread, Casey said:
>
> | One of the holes in the 1003.1e spec is what to do with a program file
> | that does not have a capability set attached to it. The two options are
> | drop all capabilities and leave the capabilities alone. The latter gives
> | you what you're asking for. The former is arguably safer.
>
> and
>
> | It's what we did in Trusted Irix. It made life much easier.
>
> I'm going to need to clear my head a bit before I try to compare that to
> the root cause of the sendmail capabilities bug.
>
> > I don't really buy the mmap code
> > concern because the model as it stands says that you trust the binary
> > (and all of the various ways it was programmed to execute code) with
> > specific privileges. If the binary mmap's some code (PAM modules come
> > to mind) then that is part of what it was programmed to/allowed to do.
>
> That's not really the point... The point is that yes, a mini-root is
> exactly what is being asked for :) I'm not saying I expect an adversary
> to do the mmap+jump, but that currently it is a, and the only, way to
> do unprivileged userid with retaining some privileges to run legacy
> programs.
>
> > That being said, if you really really want this kind of thing, then
> > make it a single secure bit (with another lock on/off bit) which, when
> > set, makes: fI default to X.
> >
> > pP' = (X & fP) | (pI & fI)
> >
> > That way the per-process bounding set still works as advertised and
> > you don't need to worry about the existing semantics being violated.
>
> Maybe that is the way to go...

We could require nnp to set the new securebit, and add a
CONFIG_SECURITY_LULZ_I_DONT_CARE to skip that requirement.
(Or maybe that just makes things worse by having more
different sets of rules...)

2015-02-25 21:50:23

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

Hi!

> > A key concept behind posix capabilities is that the privilege comes from
> > both the person and the file being executed. As you say below basically
> > anything can be executed by the program so that is completely violated.
>
> Well this patch does not change that. Its just that the file being
> executed can inherit the parent caps without having to set caps in the new
> executable.
>
> > Still, it's not that different from mmapping some arbitrary code and
> > jumping into it whlie retaining caps.
>
> Owww. Thats a pretty far jump from what this patch does.
>
> > Not saying this is a good idea necessarily, but worth thinking about.
>
> Well we keep on having actual problems with caps for long years now. We
> need a solution. This means a patch that solves the issue.

One solution is to put capabilities into the elf executable. I believe
there was patch for that. That means you don't need to add capability
support into filesystems...

Old documentation is at

http://atrey.karlin.mff.cuni.cz/~pavel/elfcap.html
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-02-25 21:50:31

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Mon 2015-02-02 18:08:06, Serge Hallyn wrote:
> Quoting Casey Schaufler ([email protected]):
> > I'm game to participate in such an effort. The POSIX scheme
> > is workable, but given that it's 20 years old and hasn't
> > developed real traction it's hard to call it successful.
>
> Over the years we've several times discussed possible reasons for this
> and how to help. I personally think it's two things: 1. lack of
> toolchain and fs support. The fact that we cannot to this day enable
> ping using capabilities by default because of cpio, tar and non-xattr
> filesystems is disheartening. 2. It's hard for users and

Capabilities in the elf notes actually solve 1.

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-02-25 23:59:08

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Wed, 25 Feb 2015, Pavel Machek wrote:

> One solution is to put capabilities into the elf executable. I believe
> there was patch for that. That means you don't need to add capability
> support into filesystems...

Ummm... So I can just get any caps by modifying the ELF header?
Looking at the docs No, it just drops caps so binaries must be
setsuid.

2015-02-26 12:26:23

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Wed 2015-02-25 17:59:04, Christoph Lameter wrote:
> On Wed, 25 Feb 2015, Pavel Machek wrote:
>
> > One solution is to put capabilities into the elf executable. I believe
> > there was patch for that. That means you don't need to add capability
> > support into filesystems...
>
> Ummm... So I can just get any caps by modifying the ELF header?
> Looking at the docs No, it just drops caps so binaries must be
> setsuid.

exactly. Normal apps are not currently allowed to receive
capabilities, because they may not be ready for them.

So add an elf note marking what capabilities it can deal with.
No need for setuid if caller has the capabilities already.

Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-02-27 20:15:41

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Thu, Feb 26, 2015 at 4:27 AM, Pavel Machek <[email protected]> wrote:
> On Wed 2015-02-25 17:59:04, Christoph Lameter wrote:
>> On Wed, 25 Feb 2015, Pavel Machek wrote:
>>
>> > One solution is to put capabilities into the elf executable. I believe
>> > there was patch for that. That means you don't need to add capability
>> > support into filesystems...
>>
>> Ummm... So I can just get any caps by modifying the ELF header?
>> Looking at the docs No, it just drops caps so binaries must be
>> setsuid.
>
> exactly. Normal apps are not currently allowed to receive
> capabilities, because they may not be ready for them.
>
> So add an elf note marking what capabilities it can deal with.
> No need for setuid if caller has the capabilities already.

We'd need extremely broad coverage for this to be useful because of
shells, pipelines, scripts, etc. We'd need bash, env, python, etc.

--Andy

>
> Pavel
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

--
Andy Lutomirski
AMA Capital Management, LLC

2015-02-27 20:48:43

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Fri 2015-02-27 12:15:15, Andy Lutomirski wrote:
> On Thu, Feb 26, 2015 at 4:27 AM, Pavel Machek <[email protected]> wrote:
> > On Wed 2015-02-25 17:59:04, Christoph Lameter wrote:
> >> On Wed, 25 Feb 2015, Pavel Machek wrote:
> >>
> >> > One solution is to put capabilities into the elf executable. I believe
> >> > there was patch for that. That means you don't need to add capability
> >> > support into filesystems...
> >>
> >> Ummm... So I can just get any caps by modifying the ELF header?
> >> Looking at the docs No, it just drops caps so binaries must be
> >> setsuid.
> >
> > exactly. Normal apps are not currently allowed to receive
> > capabilities, because they may not be ready for them.
> >
> > So add an elf note marking what capabilities it can deal with.
> > No need for setuid if caller has the capabilities already.
>
> We'd need extremely broad coverage for this to be useful because of
> shells, pipelines, scripts, etc. We'd need bash, env, python, etc.

Well.. capabilities for scripts will be "fun" even when you have
proper filesystem support. I'd say that is separate problem... (and
yes, it would have to be solved.)

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-02-27 20:57:06

[permalink] [raw]

Subject: Re: [capabilities] Allow normal inheritance for a configurable set of capabilities

On Fri, Feb 27, 2015 at 12:48 PM, Pavel Machek <[email protected]> wrote:
> On Fri 2015-02-27 12:15:15, Andy Lutomirski wrote:
>> On Thu, Feb 26, 2015 at 4:27 AM, Pavel Machek <[email protected]> wrote:
>> > On Wed 2015-02-25 17:59:04, Christoph Lameter wrote:
>> >> On Wed, 25 Feb 2015, Pavel Machek wrote:
>> >>
>> >> > One solution is to put capabilities into the elf executable. I believe
>> >> > there was patch for that. That means you don't need to add capability
>> >> > support into filesystems...
>> >>
>> >> Ummm... So I can just get any caps by modifying the ELF header?
>> >> Looking at the docs No, it just drops caps so binaries must be
>> >> setsuid.
>> >
>> > exactly. Normal apps are not currently allowed to receive
>> > capabilities, because they may not be ready for them.
>> >
>> > So add an elf note marking what capabilities it can deal with.
>> > No need for setuid if caller has the capabilities already.
>>
>> We'd need extremely broad coverage for this to be useful because of
>> shells, pipelines, scripts, etc. We'd need bash, env, python, etc.
>
> Well.. capabilities for scripts will be "fun" even when you have
> proper filesystem support. I'd say that is separate problem... (and
> yes, it would have to be solved.)

To me, however, the whole point of this thread is that you shouldn't
need filesystem support at all. If I have CAP_WHATEVER, I tell the
kernel that I want my children to have CAP_WHATEVER in their permitted
and effective sets, and I don't try to run a setuid or fP != 0
program, then it should just work.

The insertion of scripts in the way shouldn't matter.

--Andy

>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

--
Andy Lutomirski
AMA Capital Management, LLC

2015-02-27 22:47:18