| Message ID | 1465847065-3577-2-git-send-email-toiwoton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }
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(+)