Messages in this thread |  | | | Subject | Re: [RFC][PATCH] security: split ptrace checking in proc | | From | Stephen Smalley <> | | Date | Tue, 13 May 2008 10:01:10 -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.
It isn't quite a read vs. readwrite distinction. A normal ptrace attach implies full control of the target process (which goes even beyond simple read/write to control flow). Read access to /proc/pid/mem (the process memory) has also always required full ptrace access, and we aren't changing that situation with this patch as I'm not aware of any need to allow it w/o allowing full ptrace access.
What the patch is doing is enabling distinctions to be made by security modules (without disturbing the base DAC or capability checks) between full ptrace access and limited reading of specific elements of state, such as reading /proc/pid/environ or reading the special symlinks /proc/pid/exec and /proc/pid/fd/<n>. We often encounter the latter in various programs that do not need to be able to ptrace the target process (e.g. monitoring programs, procps, lsof, PolicyKit). At present we have to choose between allowing full ptrace in policy (more permissive than required/desired) or breaking functionality (or in some cases, just silencing the denials via dontaudit but this can hide genuine attacks).
> > 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.
As above, it isn't quite a read vs. readwrite mode distinction (which is why I called it ptrace_may_readstate rather than just ptrace_may_read), and the advantage of implementing it via a new interface is that we only need to update the callers where we want to apply this different checking, leaving all other callers unmodified and unaffected. So while I could do it the way you describe, I'm not sure it would yield a better result. Maybe others can chime in with their opinions.
-- Stephen Smalley National Security Agency
|  |