All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] procfs: protect /proc/<pid>/* files with file->f_cred
@ 2013-10-01 20:26 ` Djalal Harouni
  0 siblings, 0 replies; 179+ messages in thread
From: Djalal Harouni @ 2013-10-01 20:26 UTC (permalink / raw)
  To: Eric W. Biederman, Kees Cook, Al Viro, Andrew Morton,
	Linus Torvalds, Ingo Molnar, Serge E. Hallyn, Cyrill Gorcunov,
	David Rientjes, LKML, linux-fsdevel, kernel-hardening
  Cc: tixxdz, Djalal Harouni

/proc/<pid>/* entries varies at runtime, appropriate permission checks
need to happen during each system call.

Currently some of these sensitive entries are protected by performing
the ptrace_may_access() check. However even with that the /proc file
descriptors can be passed to a more privileged process
(e.g. a suid-exec) which will pass the classic ptrace_may_access()
check. In general the ->open() call will be issued by an unprivileged
process while the ->read(),->write() calls by a more privileged one.

Example of these files are:
/proc/*/syscall, /proc/*/stack etc.

And any open(/proc/self/*) then suid-exec to read()/write() /proc/self/*


These files are protected during read() by the ptrace_may_access(),
however the file descriptor can be passed to a suid-exec which can be
used to read data and bypass ASLR. Of course this was discussed several
times on LKML.


Solution:
Here we propose a clean solution that uses available mechanisms. No
additions, nor new structs/memory allocation...

After a discussion about some /proc/<pid>/* file permissions [1],
Eric W. Biederman proposed to use the file->f_cred to check if current's
cred have changed [2], actually he said that Linus was looking
on using the file->f_cred to implement a similar thing! But run in
problems with Chromes sandbox ? a link please ?


So here are my experiments:
The idea is to track the cred of current. If the cred change between
->open() and read()/write() this means that current has lost or gained
some X privileges. If so, in addition to the classic ptrace_may_access()
check, perform a second check using the file's opener cred. This means,
if an unprivileged process that tries to use a privileged one
(e.g. suid-exec) to read privileged files will get caught. The original
process that opened the file does not have the appropriate permissions
to read()/write() the target /proc/<pid>/* entry.

This second check is done of course during read(),write()...


Advantages of the proposed solution:
* It uses available mechanisms: file->f_cred which is a const cred
  that reference the cred of the opener.

* The file->f_cred can be easily referenced

* It allows to pass file descriptors under certain conditions:
  (1) current at open time may have enough permissions
  (2) current does a suid-exec or change its ruid/euid (new cred)
  (3) current or suid-exec tries to read from the task /proc entry
     Allow the ->read() only if the file's opener cred at (1)
     have enough permissions on *this* task /proc entry during
     *this* ->read() moment. Otherwise fail.

  IOW allow it, if the opener does not need the help of a suid-exec to
  read/write data.


Disadvantage:
* Currently only /proc/*/{stack,syscall,stat,personality} are handled.
  If the solution is accepted I'll continue with other files, taking
  care to not break userspace. All the facilities are provided.
* Your feedback


[1] https://lkml.org/lkml/2013/8/26/354
[2] https://lkml.org/lkml/2013/8/31/209


Change log
----------
v1->v2:
 - Removed the file->f_cred member that was added to seq_file struct.
   Al Viro didn't like it, and Linus suggested to have a pointer on
   'file struct', so it's done by using seq_file->private

 - Added Acked-by: Kees Cook to
   [PATCH v2 4/9] procfs: make /proc/*/{stack,syscall} 0400

 - Added suggested-by: Eric W. Biederman  to
   [PATCH v2 1/9] procfs: add proc_same_open_cred() to check if the cred
   have changed
   [PATCH v2 2/9] procfs: add proc_allow_access() to check if file's
   opener may access task
   [PATCH v2 3/9] procfs: Document the proposed solution to protect
   procfs entries

 - Patchset cleaned

Version 1 was discussed here:
https://lkml.org/lkml/2013/9/25/459


The following series tries to implement what I describe.


Djalal Harouni (9):
 procfs: add proc_same_open_cred() to check if the cred have changed
 procfs: add proc_allow_access() to check if file's opener may access task
 procfs: Document the proposed solution to protect procfs entries
 procfs: make /proc/*/{stack,syscall} 0400
 procfs: make /proc entries that use seq files able to access file->f_cred
 procfs: add permission checks on the file's opener of /proc/*/stat
 procfs: add permission checks on the file's opener of /proc/*/personality
 procfs: improve permission checks on /proc/*/stack
 procfs: improve permission checks on /proc/*/syscall

 fs/proc/array.c    |  16 ++++++-
 fs/proc/base.c     | 301 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 fs/proc/internal.h |   3 ++
 3 files changed, 292 insertions(+), 28 deletions(-)

^ permalink raw reply	[flat|nested] 179+ messages in thread

end of thread, other threads:[~2013-10-13 10:18 UTC | newest]

Thread overview: 179+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-01 20:26 [PATCH v2 0/9] procfs: protect /proc/<pid>/* files with file->f_cred Djalal Harouni
2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni
2013-10-01 20:26 ` [PATCH v2 1/9] procfs: add proc_same_open_cred() to check if the cred have changed Djalal Harouni
2013-10-01 20:26   ` [kernel-hardening] " Djalal Harouni
2013-10-01 20:26 ` [PATCH v2 2/9] procfs: add proc_allow_access() to check if file's opener may access task Djalal Harouni
2013-10-01 20:26   ` [kernel-hardening] " Djalal Harouni
2013-10-02  1:36   ` Andy Lutomirski
2013-10-02  1:36     ` [kernel-hardening] " Andy Lutomirski
2013-10-02 14:55     ` Djalal Harouni
2013-10-02 14:55       ` [kernel-hardening] " Djalal Harouni
2013-10-02 16:44       ` Andy Lutomirski
2013-10-02 16:44         ` [kernel-hardening] " Andy Lutomirski
2013-10-03 14:36         ` Djalal Harouni
2013-10-03 14:36           ` [kernel-hardening] " Djalal Harouni
2013-10-03 15:12           ` Andy Lutomirski
2013-10-03 15:12             ` [kernel-hardening] " Andy Lutomirski
2013-10-03 15:12             ` Andy Lutomirski
2013-10-03 19:29             ` Djalal Harouni
2013-10-03 19:29               ` [kernel-hardening] " Djalal Harouni
2013-10-03 19:29               ` Djalal Harouni
2013-10-03 19:37               ` Andy Lutomirski
2013-10-03 19:37                 ` [kernel-hardening] " Andy Lutomirski
2013-10-03 19:37                 ` Andy Lutomirski
2013-10-03 20:13                 ` Djalal Harouni
2013-10-03 20:13                   ` [kernel-hardening] " Djalal Harouni
2013-10-03 20:13                   ` Djalal Harouni
2013-10-03 21:09                   ` Andy Lutomirski
2013-10-03 21:09                     ` [kernel-hardening] " Andy Lutomirski
2013-10-03 21:09                     ` Andy Lutomirski
2013-10-04  8:59                     ` Djalal Harouni
2013-10-04  8:59                       ` [kernel-hardening] " Djalal Harouni
2013-10-04  8:59                       ` Djalal Harouni
2013-10-04 15:40                       ` Andy Lutomirski
2013-10-04 15:40                         ` [kernel-hardening] " Andy Lutomirski
2013-10-04 15:40                         ` Andy Lutomirski
2013-10-04 18:23                         ` Djalal Harouni
2013-10-04 18:23                           ` [kernel-hardening] " Djalal Harouni
2013-10-04 18:23                           ` Djalal Harouni
2013-10-04 18:34                           ` Andy Lutomirski
2013-10-04 18:34                             ` [kernel-hardening] " Andy Lutomirski
2013-10-04 18:34                             ` Andy Lutomirski
2013-10-04 19:11                             ` Djalal Harouni
2013-10-04 19:11                               ` [kernel-hardening] " Djalal Harouni
2013-10-04 19:11                               ` Djalal Harouni
2013-10-04 19:16                               ` Andy Lutomirski
2013-10-04 19:16                                 ` [kernel-hardening] " Andy Lutomirski
2013-10-04 19:16                                 ` Andy Lutomirski
2013-10-04 19:27                                 ` Djalal Harouni
2013-10-04 19:27                                   ` [kernel-hardening] " Djalal Harouni
2013-10-04 19:27                                   ` Djalal Harouni
2013-10-04 19:32                                   ` Andy Lutomirski
2013-10-04 19:32                                     ` [kernel-hardening] " Andy Lutomirski
2013-10-04 19:32                                     ` Andy Lutomirski
2013-10-04 19:41                                     ` Djalal Harouni
2013-10-04 19:41                                       ` [kernel-hardening] " Djalal Harouni
2013-10-04 19:41                                       ` Djalal Harouni
2013-10-04 22:17                                       ` Andy Lutomirski
2013-10-04 22:17                                         ` [kernel-hardening] " Andy Lutomirski
2013-10-04 22:17                                         ` Andy Lutomirski
2013-10-04 22:55                                         ` Eric W. Biederman
2013-10-04 22:55                                           ` [kernel-hardening] " Eric W. Biederman
2013-10-04 22:55                                           ` Eric W. Biederman
2013-10-04 22:59                                           ` Andy Lutomirski
2013-10-04 22:59                                             ` [kernel-hardening] " Andy Lutomirski
2013-10-04 22:59                                             ` Andy Lutomirski
2013-10-04 23:08                                             ` Andy Lutomirski
2013-10-04 23:08                                               ` [kernel-hardening] " Andy Lutomirski
2013-10-04 23:08                                               ` Andy Lutomirski
2013-10-05  0:35                                             ` Eric W. Biederman
2013-10-05  0:35                                               ` [kernel-hardening] " Eric W. Biederman
2013-10-05  0:35                                               ` Eric W. Biederman
2013-10-09 10:35                                               ` Djalal Harouni
2013-10-09 10:35                                                 ` [kernel-hardening] " Djalal Harouni
2013-10-09 10:35                                                 ` Djalal Harouni
2013-10-05 13:23                                         ` Djalal Harouni
2013-10-05 13:23                                           ` [kernel-hardening] " Djalal Harouni
2013-10-05 13:23                                           ` Djalal Harouni
2013-10-07 21:41                                           ` Andy Lutomirski
2013-10-07 21:41                                             ` [kernel-hardening] " Andy Lutomirski
2013-10-07 21:41                                             ` Andy Lutomirski
2013-10-09 10:54                                             ` Djalal Harouni
2013-10-09 10:54                                               ` [kernel-hardening] " Djalal Harouni
2013-10-09 10:54                                               ` Djalal Harouni
2013-10-09 11:15                                               ` Djalal Harouni
2013-10-09 11:15                                                 ` [kernel-hardening] " Djalal Harouni
2013-10-09 11:15                                                 ` Djalal Harouni
2013-10-09 17:27                                               ` Andy Lutomirski
2013-10-09 17:27                                                 ` [kernel-hardening] " Andy Lutomirski
2013-10-09 17:27                                                 ` Andy Lutomirski
2013-10-13 10:18                                                 ` Djalal Harouni
2013-10-13 10:18                                                   ` [kernel-hardening] " Djalal Harouni
2013-10-13 10:18                                                   ` Djalal Harouni
2013-10-01 20:26 ` [PATCH v2 3/9] procfs: Document the proposed solution to protect procfs entries Djalal Harouni
2013-10-01 20:26   ` [kernel-hardening] " Djalal Harouni
2013-10-01 20:26 ` [PATCH v2 4/9] procfs: make /proc/*/{stack,syscall} 0400 Djalal Harouni
2013-10-01 20:26   ` [kernel-hardening] " Djalal Harouni
2013-10-01 20:26 ` [PATCH v2 5/9] procfs: make /proc entries that use seq files able to access file->f_cred Djalal Harouni
2013-10-01 20:26   ` [kernel-hardening] " Djalal Harouni
2013-10-01 20:26 ` [PATCH v2 6/9] procfs: add permission checks on the file's opener of /proc/*/stat Djalal Harouni
2013-10-01 20:26   ` [kernel-hardening] " Djalal Harouni
2013-10-02  1:39   ` Andy Lutomirski
2013-10-02  1:39     ` [kernel-hardening] " Andy Lutomirski
2013-10-02 15:14     ` Djalal Harouni
2013-10-02 15:14       ` [kernel-hardening] " Djalal Harouni
2013-10-02 16:46       ` Andy Lutomirski
2013-10-02 16:46         ` [kernel-hardening] " Andy Lutomirski
2013-10-02 19:00         ` Djalal Harouni
2013-10-02 19:00           ` [kernel-hardening] " Djalal Harouni
2013-10-01 20:26 ` [PATCH v2 7/9] procfs: add permission checks on the file's opener of /proc/*/personality Djalal Harouni
2013-10-01 20:26   ` [kernel-hardening] " Djalal Harouni
2013-10-01 20:26 ` [PATCH v2 8/9] procfs: improve permission checks on /proc/*/stack Djalal Harouni
2013-10-01 20:26   ` [kernel-hardening] " Djalal Harouni
2013-10-01 20:26 ` [PATCH v2 9/9] procfs: improve permission checks on /proc/*/syscall Djalal Harouni
2013-10-01 20:26   ` [kernel-hardening] " Djalal Harouni
2013-10-02  1:40 ` [PATCH v2 0/9] procfs: protect /proc/<pid>/* files with file->f_cred Andy Lutomirski
2013-10-02  1:40   ` [kernel-hardening] " Andy Lutomirski
2013-10-02 14:37   ` Djalal Harouni
2013-10-02 14:37     ` [kernel-hardening] " Djalal Harouni
2013-10-02 16:51     ` Andy Lutomirski
2013-10-02 16:51       ` [kernel-hardening] " Andy Lutomirski
2013-10-02 17:48       ` Kees Cook
2013-10-02 17:48         ` [kernel-hardening] " Kees Cook
2013-10-02 17:48         ` Kees Cook
2013-10-02 18:00         ` Andy Lutomirski
2013-10-02 18:00           ` [kernel-hardening] " Andy Lutomirski
2013-10-02 18:00           ` Andy Lutomirski
2013-10-02 18:07           ` Kees Cook
2013-10-02 18:07             ` [kernel-hardening] " Kees Cook
2013-10-02 18:07             ` Kees Cook
2013-10-03 23:14             ` Julien Tinnes
2013-10-03 23:14               ` [kernel-hardening] " Julien Tinnes
2013-10-03 23:14               ` Julien Tinnes
2013-10-02 18:26           ` Djalal Harouni
2013-10-02 18:26             ` [kernel-hardening] " Djalal Harouni
2013-10-02 18:26             ` Djalal Harouni
2013-10-02 18:41             ` Djalal Harouni
2013-10-02 18:41               ` [kernel-hardening] " Djalal Harouni
2013-10-02 18:41               ` Djalal Harouni
2013-10-02 18:22         ` Djalal Harouni
2013-10-02 18:22           ` [kernel-hardening] " Djalal Harouni
2013-10-02 18:22           ` Djalal Harouni
2013-10-02 18:35           ` Kees Cook
2013-10-02 18:35             ` [kernel-hardening] " Kees Cook
2013-10-02 18:35             ` Kees Cook
2013-10-02 18:48             ` Djalal Harouni
2013-10-02 18:48               ` [kernel-hardening] " Djalal Harouni
2013-10-02 18:48               ` Djalal Harouni
2013-10-02 19:43               ` Kees Cook
2013-10-02 19:43                 ` [kernel-hardening] " Kees Cook
2013-10-02 19:43                 ` Kees Cook
2013-10-03  6:12               ` Ingo Molnar
2013-10-03  6:12                 ` [kernel-hardening] " Ingo Molnar
2013-10-03  6:12                 ` Ingo Molnar
2013-10-03 12:29                 ` Djalal Harouni
2013-10-03 12:29                   ` [kernel-hardening] " Djalal Harouni
2013-10-03 12:29                   ` Djalal Harouni
2013-10-03 15:15                   ` Andy Lutomirski
2013-10-03 15:15                     ` [kernel-hardening] " Andy Lutomirski
2013-10-03 15:15                     ` Andy Lutomirski
2013-10-03 15:40                     ` Djalal Harouni
2013-10-03 15:40                       ` [kernel-hardening] " Djalal Harouni
2013-10-03 15:40                       ` Djalal Harouni
2013-10-03 15:50                       ` Andy Lutomirski
2013-10-03 15:50                         ` [kernel-hardening] " Andy Lutomirski
2013-10-03 15:50                         ` Andy Lutomirski
2013-10-03 18:37                         ` Djalal Harouni
2013-10-03 18:37                           ` [kernel-hardening] " Djalal Harouni
2013-10-03 18:37                           ` Djalal Harouni
2013-10-04  9:05                 ` Djalal Harouni
2013-10-04  9:05                   ` [kernel-hardening] " Djalal Harouni
2013-10-04  9:05                   ` Djalal Harouni
2013-10-02 18:12       ` Djalal Harouni
2013-10-02 18:12         ` [kernel-hardening] " Djalal Harouni
2013-10-03  6:22         ` Ingo Molnar
2013-10-03  6:22           ` [kernel-hardening] " Ingo Molnar
2013-10-03 12:56           ` Djalal Harouni
2013-10-03 12:56             ` [kernel-hardening] " Djalal Harouni
2013-10-03 13:39             ` Ingo Molnar
2013-10-03 13:39               ` [kernel-hardening] " Ingo Molnar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.