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.

The patch only changes the environ and open file checking in proc.
Other cases such as mem and maps checking still use a full ptrace
check at present.

In the SELinux case, we model such reading of process state as a
reading of the proc file labeled with the process' label.  This enables
SELinux policy to permit such reading of process state without permitting control
or manipulation of the target process, as there are a number of cases
where programs probe for such information via proc but do not need to
be able to control the target.  This restores SELinux behavior prior to
2.6.18.

Signed-off-by:  Stephen Smalley <sds@tycho.nsa.gov>

---

 fs/proc/base.c             |    4 ++--
 include/linux/ptrace.h     |    1 +
 include/linux/security.h   |   15 ++++++++++-----
 kernel/ptrace.c            |   20 +++++++++++++++++---
 security/commoncap.c       |    3 ++-
 security/dummy.c           |    3 ++-
 security/security.c        |    5 +++--
 security/selinux/hooks.c   |   13 +++++++++++--
 security/smack/smack_lsm.c |    5 +++--
 9 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 808cbdc..bbc74a0 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -499,7 +499,7 @@ static int proc_fd_access_allowed(struct inode *inode)
 	 */
 	task = get_proc_task(inode);
 	if (task) {
-		allowed = ptrace_may_attach(task);
+		allowed = ptrace_may_readstate(task);
 		put_task_struct(task);
 	}
 	return allowed;
@@ -885,7 +885,7 @@ static ssize_t environ_read(struct file *file, char __user *buf,
 	if (!task)
 		goto out_no_task;
 
-	if (!ptrace_may_attach(task))
+	if (!ptrace_may_readstate(task))
 		goto ...
[ message continues ]

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

All quite reasonable. Although I wouldn't do it myself, I could

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



Casey Schaufler
casey@schaufler-ca.com
--

[ message continues ]

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

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

--

[ message continues ]

It is slightly ad-hoc.  Is it just the audit messages that you described
that made you pick environ and fd, or was there more specific (threat
based) reasoning?  Would /proc/pid/fd/ + genfs + e.g. anonfd be a little
wider than just readstate?

Perhaps you could update the comments in ptrace_may_inspect() to clarify.

thanks,
-chris
--

[ message continues ]

Well, it is being driven by experience with what applications try to
access w/o requiring full ptrace access, but also by a threat-based
reasoning that it is less dangerous to grant limited read access to
parts of the process state than to grant complete read access to its
entire memory image or full control of the target process.


Ok, will do.

-- 
Stephen Smalley
National Security Agency

--

[ message continues ]

fd/ access gives a view in the ->files, which could include rather
internal bits like pipes, sockets, or anonfd descriptors -- things w/out
external handles.  That view includes ability to open the fd (similar
to dup()) and use it (granted subject to further security checks, but
they may be quite generic at that point).

thanks,
-chris
--

[ message continues ]

What do you mean by "generic" in the above?  Just the fact that there
wouldn't be any distinction between such access and access to a
descriptor received explicitly via local IPC from the target task?

Ok, so perhaps the only distinction that makes sense is read vs.
write/control, with all checks within proc except mem_write using the
former and ptrace_attach and mem_write using the latter?

-- 
Stephen Smalley
National Security Agency

--

[ message continues ]

Yeah, that's what I was wondering, because maps seems to fall into the
readstate category as much as fd/ does (probably fdinfo/ is closer to
maps).

thanks,
-chris
--

[ message continues ]

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

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

--

[ message continues ]