Messages in this thread | | | Subject | Re: [RFC][PATCH] security: split ptrace checking in proc | From | Stephen Smalley <> | Date | Mon, 12 May 2008 11:16:15 -0400 |
| |
On Mon, 2008-05-12 at 07:06 -0700, Casey Schaufler wrote: > --- Stephen Smalley <sds@tycho.nsa.gov> wrote: > > > Enable security modules to distinguish reading of process state > > information from full ptrace access by introducing a distinct helper > > function for such checks and passing a boolean flag down to the > > security_ptrace hook. This allows security modules to permit access > > to reading process state without granting full ptrace access. > > This will obviously suffice, but why pass a boolean instead > of the access actually desired? What I mean is that instead > of passing a read-only flag, you could pass in READ or READWRITE > to indicate which access you require. Although I don't have > a case in mind, it seems that your interface is unnecessarily > contrained if you have a read-only boolean.
A normal ptrace attach implies full read-write (and more - control of execution flow) access. Also, access to /proc/pid/mem (i.e. the process memory) has always required full ptrace access, even if only for read (and mem_write is disabled by default). We aren't changing that situation with this patch.
What we are doing in this patch is enabling distinctions to be made by security modules (without disturbing the base DAC/capabilities logic) between such full ptrace access and reading of state such as /proc/pid/environ, /proc/pid/exe and /proc/pid/fd/. We often encounter the latter in various programs that do not in fact need or want to be able to ptrace the target process (e.g. monitoring programs, killproc, mingetty, PolicyKit). At present, we have to choose between allowing this in policy (more permissive than required/desired) or breaking functionality. This was effectively a regression introduced into SELinux by unrelated changes to proc back in 2.6.18 to tighten up the DAC checking, which we aren't disturbing here.
> > diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h > > index f98501b..f8a5e75 100644 > > --- a/include/linux/ptrace.h > > +++ b/include/linux/ptrace.h > > @@ -97,6 +97,7 @@ extern void __ptrace_unlink(struct task_struct *child); > > extern void ptrace_untrace(struct task_struct *child); > > extern int ptrace_may_attach(struct task_struct *task); > > extern int __ptrace_may_attach(struct task_struct *task); > > +extern int ptrace_may_readstate(struct task_struct *task); > > I would prefer a mode parameter to ptrace_may_attach to the > specific function for read access. Again, what you have will > work for your case, but may lead to yet another interface later > if someone wants a slightly different check.
That would require updating all callers to pass the mode vs. only changing a few callers. Also, it isn't just an access mode distinction per se but rather a distinction between read access to parts of the process state vs. full access to the process state, with the former required by a variety of programs and the latter only required/desired for full ptrace purposes. The mem_read case is an example where we are still applying a full ptrace check even though we are only reading (but there we are reading the full process memory vs. just parts of its state like its environ, exe link, and fd links).
I could do it the way you describe, but I'm not sure it will yield a better result. Maybe others can chime in with their preferences and I can go with whatever consensus emerges.
-- Stephen Smalley National Security Agency
| |