Re: Documenting ptrace access mode checking

On Wed, Jun 22, 2016 at 09:21:29PM +0200, Michael Kerrisk (man-pages) wrote:

On 06/21/2016 10:55 PM, Jann Horn wrote:

quoted

On Tue, Jun 21, 2016 at 11:41:16AM +0200, Michael Kerrisk (man-pages) wrote:

quoted

Here's the new ptrace(2) text. Any comments, technical or terminological
fixes, other improvements, etc. are welcome.

As others have said, I'm surprised about seeing documentation about
kernel-internal constants in manpages - but I think it might be a good
thing to have there, given that people who look at ptrace(2) are likely
to be interested in low-level details.

I agree that it is a little surprising to add kernel-internal
constants in a man page. (There are precedents, but they are few.)
But see my reply to Kees. It's more than just explaining low level
details: there are various kinds of user-space behavior differences
(real vs filesystem credentials; permitted vs effective capabilities)
produced by the ptrace_may_access() checks, and those behaviors need
to be described and *somehow* labeled for cross-referencing from
other man pages.

Makes sense.

quoted

quoted

      The  algorithm  employed for ptrace access mode checking deter‐
      mines whether the calling process is  allowed  to  perform  the
      corresponding action on the target process, as follows:

      1.  If the calling thread and the target thread are in the same
          thread group, access is always allowed.

      2.  If the access mode specifies PTRACE_MODE_FSCREDS, then  for
          the  check in the next step, employ the caller's filesystem
          user ID and group ID (see credentials(7));  otherwise  (the
          access  mode  specifies  PTRACE_MODE_REALCREDS, so) use the
          caller's real user ID and group ID.

Might want to add a "for historical reasons" or so here.

Can you be a little more precise about "here", and maybe tell me why
you think it helps?

I'm not sure, but it might be a good idea to add something like this at the
end of 2.:
"(Most other APIs that check one of the caller's UIDs use the effective one.
This API uses the real UID instead for historical reasons.)"

In my opinion, it is inconsistent to use the real UID/GID here, the
effective one would be more appropriate. But since the existing code uses
the real UID/GID and that's not a security issue for existing users of
the ptrace API, this wasn't changed when I added the REALCREDS/FSCREDS
distinction.

I think that for a reader, it might help to point out that in most cases,
when a process is the subject in an access check, its effective UID/GID
are used, and this is (together with kill()) an exception to that rule.
But you're the expert on writing documentation, if you think that that's
too much detail / confusing here, it probably is.

I changed this text to:

       Various parts of the kernel-user-space API (not just  ptrace(2)
       operations), require so-called "ptrace access mode permissions"
       which are gated by any enabled Linux Security Module (LSMs)—for
       example,  SELinux,  Yama, or Smack—and by the the commoncap LSM
       (which is always invoked).  Prior to  Linux  2.6.27,  all  such
       checks  were  of a single type.  Since Linux 2.6.27, two access
       mode levels are distinguished:

Sounds good to me.