keys: request_key_auth shows a global pid in /proc/keys across pid namespaces

[Maoyi Xie <maoyixie.tju@gmail.com>]

Hi all,

I think request_key_auth leaks a global pid into /proc/keys. I would
appreciate it if you could take a look.

request_key_auth_describe() prints the requestor's pid:

	if (key_is_positive(key))
		seq_printf(m, " pid:%d ci:%zu", rka->pid, rka->callout_len);

rka->pid is current->pid, captured in request_key_auth_new() when the
auth key is made. It is a raw pid_t in the init pid namespace. But
/proc/keys is read by whoever opens it, and their pid namespace can
differ. The uid and gid on that line already go through seq_user_ns(m).
The pid does not. So a reader in another pid namespace gets a number
that means nothing there, or worse lands on a different task.

Not everyone can read the line. The owner uid must map into the reader's
user namespace, and the reader needs VIEW on the key. The auth key gives
VIEW to the requestor's own uid and to a possessor. So the reader has to
be the requestor's uid, in a different pid namespace. That fits a
sandbox at the same uid, made with unshare, systemd-nspawn or
bubblewrap. A default Docker container does not hit it, since it hides
/proc/keys.

I found this with a static check for ids that reach userspace with no
namespace translation. I reproduced it in a qemu VM on 7.1-rc1 as a
normal user, no hardware or kernel changes needed. The reproducer calls
request_key, then reads /proc/keys from a sibling pid namespace. Before
the patch it reads the requestor's global pid, which does not exist in
that namespace. After the patch it reads pid 0, since the requestor has
no pid there. There is no keyrings selftest in the tree, so this is the
only test.

I am happy to send a patch. The raw pid_t cannot be translated later. So
I stored a struct pid and print it with pid_nr_ns, the way the uid is
already scoped.

Thanks,
Maoyi
https://maoyixie.com/

---

diff --git a/include/keys/request_key_auth-type.h b/include/keys/request_key_auth-type.h
index 01e42ee5f4099..464636278c4f8 100644
--- a/include/keys/request_key_auth-type.h
+++ b/include/keys/request_key_auth-type.h
@@ -22,7 +22,7 @@ struct request_key_auth {
 	const struct cred	*cred;
 	void			*callout_info;
 	size_t			callout_len;
-	pid_t			pid;
+	struct pid		*pid;
 	char			op[8];
 } __randomize_layout;
 
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 282e09d8fa46c..e8d1037526b19 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -13,6 +13,8 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include "internal.h"
+#include <linux/pid_namespace.h>
+#include <linux/proc_fs.h>
 #include <keys/request_key_auth-type.h>
 
 static int request_key_auth_preparse(struct key_preparsed_payload *);
@@ -73,7 +75,10 @@ static void request_key_auth_describe(const struct key *key,
 	seq_puts(m, "key:");
 	seq_puts(m, key->description);
 	if (key_is_positive(key))
-		seq_printf(m, " pid:%d ci:%zu", rka->pid, rka->callout_len);
+		seq_printf(m, " pid:%d ci:%zu",
+			   pid_nr_ns(rka->pid,
+				     proc_pid_ns(file_inode(m->file)->i_sb)),
+			   rka->callout_len);
 }
 
 /*
@@ -113,6 +118,7 @@ static void free_request_key_auth(struct request_key_auth *rka)
 	if (rka->cred)
 		put_cred(rka->cred);
 	kfree(rka->callout_info);
+	put_pid(rka->pid);
 	kfree(rka);
 }
 
@@ -226,14 +232,14 @@ struct key *request_key_auth_new(struct key *target, const char *op,
 
 		irka = cred->request_key_auth->payload.data[0];
 		rka->cred = get_cred(irka->cred);
-		rka->pid = irka->pid;
+		rka->pid = get_pid(irka->pid);
 
 		up_read(&cred->request_key_auth->sem);
 	}
 	else {
 		/* it isn't - use this process as the context */
 		rka->cred = get_cred(cred);
-		rka->pid = current->pid;
+		rka->pid = get_pid(task_pid(current));
 	}
 
 	rka->target_key = key_get(target);