diff mbox

[RFC,01/18] capabilities: track actually used capabilities

Message ID 1465847065-3577-2-git-send-email-toiwoton@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Topi Miettinen June 13, 2016, 7:44 p.m. UTC
Track what capabilities are actually used and present the current
situation in /proc/self/status.

Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
---
 fs/exec.c             | 1 +
 fs/proc/array.c       | 1 +
 include/linux/sched.h | 1 +
 kernel/capability.c   | 1 +
 4 files changed, 4 insertions(+)

Comments

Andy Lutomirski June 13, 2016, 8:32 p.m. UTC | #1
On Mon, Jun 13, 2016 at 12:44 PM, Topi Miettinen <toiwoton@gmail.com> wrote:
> Track what capabilities are actually used and present the current
> situation in /proc/self/status.

What for?

What is the intended behavior on fork()?  Whatever the intended
behavior is, there should IMO be a selftest for it.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Topi Miettinen June 13, 2016, 8:45 p.m. UTC | #2
On 06/13/16 20:32, Andy Lutomirski wrote:
> On Mon, Jun 13, 2016 at 12:44 PM, Topi Miettinen <toiwoton@gmail.com> wrote:
>> Track what capabilities are actually used and present the current
>> situation in /proc/self/status.
> 
> What for?

Excerpt from the cover letter:

"There are many basic ways to control processes, including capabilities,
cgroups and resource limits. However, there are far fewer ways to find out
useful values for the limits, except blind trial and error.

This patch series attempts to fix that by giving at least a nice starting
point from the actual maximum values. I looked where each limit is checked
and added a call to limit bump nearby.


Capabilities
[RFC 01/18] capabilities: track actually used capabilities

Currently, there is no way to know which capabilities are actually used.
Even
the source code is only implicit, in-depth knowledge of each capability must
be used when analyzing a program to judge which capabilities the program
will
exercise."

Should I perhaps cite some of this in the commit?

>
> What is the intended behavior on fork()?  Whatever the intended
> behavior is, there should IMO be a selftest for it.
>
> --Andy
>

The capabilities could be tracked from three points of daemon
initialization sequence onwards:
fork()
setpcap()
exec()

fork() case would be logical as the /proc entry is per task. But if you
consider the tools to set the capabilities (for example systemd unit
files), there can be between fork() and exec() further preparations
which need more capabilities than the program itself needs.

setpcap() is probably the real point after which we are interested if
the capabilities are enough.

The amount of setup between setpcap() and exec() is probably very low.

-Topi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Lutomirski June 13, 2016, 9:12 p.m. UTC | #3
On Mon, Jun 13, 2016 at 1:45 PM, Topi Miettinen <toiwoton@gmail.com> wrote:
> On 06/13/16 20:32, Andy Lutomirski wrote:
>> On Mon, Jun 13, 2016 at 12:44 PM, Topi Miettinen <toiwoton@gmail.com> wrote:
>>> Track what capabilities are actually used and present the current
>>> situation in /proc/self/status.
>>
>> What for?
>

>
> Capabilities
> [RFC 01/18] capabilities: track actually used capabilities
>
> Currently, there is no way to know which capabilities are actually used.
> Even
> the source code is only implicit, in-depth knowledge of each capability must
> be used when analyzing a program to judge which capabilities the program
> will
> exercise."
>
> Should I perhaps cite some of this in the commit?

Yes, but you should also clarify what users are supposed to do with
this.  Given ambient capabilities, I suspect that you'll find that
your patch doesn't actually work very well.  For example, if you run a
shell script with ambient caps, then you won't notice caps used by
short-lived helper processes.

>
>>
>> What is the intended behavior on fork()?  Whatever the intended
>> behavior is, there should IMO be a selftest for it.
>>
>> --Andy
>>
>
> The capabilities could be tracked from three points of daemon
> initialization sequence onwards:
> fork()
> setpcap()
> exec()
>
> fork() case would be logical as the /proc entry is per task. But if you
> consider the tools to set the capabilities (for example systemd unit
> files), there can be between fork() and exec() further preparations
> which need more capabilities than the program itself needs.
>
> setpcap() is probably the real point after which we are interested if
> the capabilities are enough.
>
> The amount of setup between setpcap() and exec() is probably very low.

When I asked "what is the intended behavior on fork()?", I mean "what
should CapUsed be after fork()?".  The answer should be about four
words long and should have a test case.  There should maybe also be an
explanation of why the intended behavior is useful.

But, as I said above, I think that you may need to rethink this
entirely to make it useful.  You might need to do it per process tree
or per cgroup or something.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Topi Miettinen June 13, 2016, 9:48 p.m. UTC | #4
On 06/13/16 21:12, Andy Lutomirski wrote:
> On Mon, Jun 13, 2016 at 1:45 PM, Topi Miettinen <toiwoton@gmail.com> wrote:
>> On 06/13/16 20:32, Andy Lutomirski wrote:
>>> On Mon, Jun 13, 2016 at 12:44 PM, Topi Miettinen <toiwoton@gmail.com> wrote:
>>>> Track what capabilities are actually used and present the current
>>>> situation in /proc/self/status.
>>>
>>> What for?
>>
> 
>>
>> Capabilities
>> [RFC 01/18] capabilities: track actually used capabilities
>>
>> Currently, there is no way to know which capabilities are actually used.
>> Even
>> the source code is only implicit, in-depth knowledge of each capability must
>> be used when analyzing a program to judge which capabilities the program
>> will
>> exercise."
>>
>> Should I perhaps cite some of this in the commit?
> 
> Yes, but you should also clarify what users are supposed to do with
> this.  Given ambient capabilities, I suspect that you'll find that
> your patch doesn't actually work very well.  For example, if you run a
> shell script with ambient caps, then you won't notice caps used by
> short-lived helper processes.
> 

Right, I suppose this model works well only within a single process, or
where the helper processes are always unprivileged (like Xorg runs
xkbcomp) or less privileged.

>>
>>>
>>> What is the intended behavior on fork()?  Whatever the intended
>>> behavior is, there should IMO be a selftest for it.
>>>
>>> --Andy
>>>
>>
>> The capabilities could be tracked from three points of daemon
>> initialization sequence onwards:
>> fork()
>> setpcap()
>> exec()
>>
>> fork() case would be logical as the /proc entry is per task. But if you
>> consider the tools to set the capabilities (for example systemd unit
>> files), there can be between fork() and exec() further preparations
>> which need more capabilities than the program itself needs.
>>
>> setpcap() is probably the real point after which we are interested if
>> the capabilities are enough.
>>
>> The amount of setup between setpcap() and exec() is probably very low.
> 
> When I asked "what is the intended behavior on fork()?", I mean "what
> should CapUsed be after fork()?".  The answer should be about four
> words long and should have a test case.  There should maybe also be an
> explanation of why the intended behavior is useful.

In this model:
fork: no change
setpcap: no change
exec: reset

But I hadn't thought that much where the reset happens.

> 
> But, as I said above, I think that you may need to rethink this
> entirely to make it useful.  You might need to do it per process tree
> or per cgroup or something.
> 
> --Andy
> 

I'd actually prefer the cgroup approach. Though that's much more work
than this simple patch which already gives somewhat useful information
in limited cases (once the logic is correct).

-Topi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/exec.c b/fs/exec.c
index 887c1c9..ff6f644 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1269,6 +1269,7 @@  void setup_new_exec(struct linux_binprm * bprm)
 		if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP)
 			set_dumpable(current->mm, suid_dumpable);
 	}
+	cap_clear(current->cap_used);
 
 	/* An exec changes our domain. We are no longer part of the thread
 	   group */
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 88c7de1..cccc9ee 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -343,6 +343,7 @@  static inline void task_cap(struct seq_file *m, struct task_struct *p)
 	render_cap_t(m, "CapEff:\t", &cap_effective);
 	render_cap_t(m, "CapBnd:\t", &cap_bset);
 	render_cap_t(m, "CapAmb:\t", &cap_ambient);
+	render_cap_t(m, "CapUsd:\t", &p->cap_used);
 }
 
 static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6e42ada..9c48a08 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1918,6 +1918,7 @@  struct task_struct {
 #ifdef CONFIG_MMU
 	struct task_struct *oom_reaper_list;
 #endif
+	kernel_cap_t	cap_used;	/* Capabilities actually used */
 /* CPU-specific state of this task */
 	struct thread_struct thread;
 /*
diff --git a/kernel/capability.c b/kernel/capability.c
index 45432b5..aad8854 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -380,6 +380,7 @@  bool ns_capable(struct user_namespace *ns, int cap)
 	}
 
 	if (security_capable(current_cred(), ns, cap) == 0) {
+		cap_raise(current->cap_used, cap);
 		current->flags |= PF_SUPERPRIV;
 		return true;
 	}