Message-ID: <20241108204102.1752206-1-stsp2@yandex.ru>
Date: Fri,  8 Nov 2024 23:41:00 +0300
From: Stas Sergeev <stsp2@...dex.ru>
To: linux-kernel@...r.kernel.org
Cc: Stas Sergeev <stsp2@...dex.ru>,
	Eric Biederman <ebiederm@...ssion.com>,
	Andy Lutomirski <luto@...nel.org>,
	Aleksa Sarai <cyphar@...har.com>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Christian Brauner <brauner@...nel.org>,
	Jan Kara <jack@...e.cz>,
	Thomas Gleixner <tglx@...utronix.de>,
	Jeff Layton <jlayton@...nel.org>,
	John Johansen <john.johansen@...onical.com>,
	Chengming Zhou <chengming.zhou@...ux.dev>,
	Casey Schaufler <casey@...aufler-ca.com>,
	Adrian Ratiu <adrian.ratiu@...labora.com>,
	Felix Moessbauer <felix.moessbauer@...mens.com>,
	Jens Axboe <axboe@...nel.dk>,
	Oleg Nesterov <oleg@...hat.com>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Kees Cook <kees@...nel.org>,
	linux-fsdevel@...r.kernel.org
Subject: [PATCH v3 0/2] implement PROCFS_SET_GROUPS ioctl

Changes in v3: NULLify private data arg of single_open() calls
  as suggested by Kees Cook <kees@...nel.org>
Changes in v2: define set_current_groups() for !CONFIG_MULTIUSER
  addressing a test robot-reported failure.

This patch set implements the PROCFS_SET_GROUPS ioctl that allows
to set the group list from the fd referring to /proc/<pid>/status.
It consists of 2 patches: a small preparatory patch and an implementation
itself. The very detailed explanation of usage, security considerations
and implementation details are documented in the commit log of the
second patch. Brief summary below.

The problem:
If you use suid/sgid bits to switch to a less-privileged (home-less)
user, then the group list can't be changed, effectively nullifying
any supposed restrictions. As such, suid/sgid to non-root creds is
currently practically useless.

Previous solutions:
https://www.spinics.net/lists/kernel/msg5383847.html
This solution allows to restrict the groups from group list.
It failed to get any attention for probably being too ad-hoc.
https://lore.kernel.org/all/0895c1f268bc0b01cc6c8ed4607d7c3953f49728.1416041823.git.josh@xxxxxxxxxxxxxxxx/
This solution from Josh Tripplett was considered insecure.

New proposal:
Given that /proc/<pid>/status file carries the cred info including the
group list, it seems natural to use that file to transfer and apply the
group list within. The trusted entity should permit such operation and
send the needed group info to client via SCM_RIGHTS. Client can check
the received info by reading from fd. If he is satisfied, he can use
the new ioctl to try to set the group list from the received status file.
Kernel does all the needed security and sanity checks, and either returns
an error or applies the group list. For more details and security
considerations please refer to the commit message of the second patch.
As the result, given that the process did the suid/sgid-assisted switch,
it can obtain the correct group info that matches his new credentials.
None of the previous proposals allowed to get the right group info:
it was either cleared or "restricted" but never correct. This proposal
aims to amend all of the previous short-comings with the hope to make
the suid/sgid-assisted switches useful for dropping access rights.

Usage example:
I put the user-space usage example here:
https://github.com/stsp/cred_test
`tst.sh` script sets the needed permissions and runs server and client.
Client does the suid/sgid-assisted identity switch and asks the server
for the new group info. Server grants the needed group info based on
client's credentials (using SO_PEERCRED) and client executes `id`
command to show the result.

Signed-off-by: Stas Sergeev <stsp2@...dex.ru>

CC: Eric Biederman <ebiederm@...ssion.com>
CC: Andy Lutomirski <luto@...nel.org>
CC: Aleksa Sarai <cyphar@...har.com>
CC: Alexander Viro <viro@...iv.linux.org.uk>
CC: Christian Brauner <brauner@...nel.org>
CC: Jan Kara <jack@...e.cz>
CC: Thomas Gleixner <tglx@...utronix.de>
CC: Jeff Layton <jlayton@...nel.org>
CC: John Johansen <john.johansen@...onical.com>
CC: Chengming Zhou <chengming.zhou@...ux.dev>
CC: Casey Schaufler <casey@...aufler-ca.com>
CC: Adrian Ratiu <adrian.ratiu@...labora.com>
CC: Felix Moessbauer <felix.moessbauer@...mens.com>
CC: Jens Axboe <axboe@...nel.dk>
CC: Oleg Nesterov <oleg@...hat.com>
Cc: "Serge E. Hallyn" <serge@...lyn.com>
CC: Kees Cook <kees@...nel.org>
CC: linux-kernel@...r.kernel.org
CC: linux-fsdevel@...r.kernel.org

Stas Sergeev (2):
  procfs: avoid some usages of seq_file private data
  procfs: implement PROCFS_SET_GROUPS ioctl

 fs/proc/base.c          | 148 +++++++++++++++++++++++++++++++++++++---
 include/linux/cred.h    |   4 ++
 include/uapi/linux/fs.h |   2 +
 3 files changed, 146 insertions(+), 8 deletions(-)

-- 
2.47.0

Powered by blists - more mailing lists