p9auth fs: introduction

Hi,

Here is an updated version of the p9auth setuid capability module, which hopefully addresses all previous feedback. It is now a separate filesystem instead of a device, as per Eric's suggestion.

During the last round, Alan Cox made a great suggestion of sending credentials over AF_UNIX sock allowing the recipient to 'become me.' I think that's still an interesting idea, and intend to pursue it as Eric pushes the patches to translate userids across user namespaces.

The tradeoffs are worth discussing. On the one
hand, p9auth requires a scary CAP_GRANT_ID capability, while a SO_PASSAUTH would be more akin to an extension of CAP_SETUID. Also, SO_PASSAUTH would be usable by any unprivileged app, while one would hope there would be only one p9auth service for the whole system. On the other hand, the p9auth API appears to be pretty well settled and understood, and only provides for a very simple setting of all uids and all gids to one value, plus some auxiliary groups, which is perfect for use by simple login servers.

p9auth and SO_PASSAUTH don't appear to be mutually exclusive. I don't know how painful it would be for plan-9 folks to make use of the SO_PASSAUTH feature. (It should definately be possible) But in any case here is the next iteration of p9auth fs for discussion and consideration.

thanks,
-serge

Break the core functionality of set{fs,res}{u,g}id into cred_setX which performs the access checks based on current_cred(), but performs the requested change on a passed-in cred.

Export the helpers, since p9auth can be compiled as a module. It might be worth not allowing modular p9auth to avoid having to export them.

Really the setfs{u,g}id helper isn't needed, but move it as well to keep the code consistent.

This patch also changes set_user() to use new->user->user_ns. While technically not needed as all callers should have new->user->user_ns equal to current_userns(), it is more correct and may prevent surprises in the future.

Changelog:
Apr 24: (David Howells) make cred_setresuid etc extern, and
document the helpers in Documentation/credentials.txt.

---
 Documentation/credentials.txt |   18 ++++++
 include/linux/cred.h          |   12 ++++
 kernel/cred.c                 |  119 +++++++++++++++++++++++++++++++++++++
 kernel/sys.c                  |  131 ++++++----------------------------------
 4 files changed, 169 insertions(+), 111 deletions(-)

index df03169..7da876b 100644
@@ -529,6 +529,24 @@ A typical credentials alteration function would look something like this:
 		return commit_creds(new);
 	}
 
+SETUID/SETGID HELPERS
+---------------------
+
+Helpers exist to perform the core of uid and gid alterations:
+
+cred_setresuid(struct cred *new, uid_t ruid, uid_t euid, uid_t suid,
+                int force);
+cred_setresgid(struct cred *new, gid_t rgid, gid_t egid, gid_t sgid,
+                int force);
+cred_setfsuid(struct cred *new, uid_t uid, uid_t *old_fsuid);
+cred_setfsgid(struct cred *new, gid_t gid, gid_t *old_fsgid);
+
+The force argument means that while the caller does not have CAP_SETUID
+or CAP_SETUID, the credentials were received from a task with CAP_GRANT_ID.
+
+These helpers are used in kernel/sys.c for the analogous syscalls.
+As can be seen in those examples, these helpers are to be wrapped
+between calls to prepare_creds() and commit_creds() or abort_creds().
 
 MANAGING CREDENTIALS
 --------------------

index 52507c3..8034e22 100644
@@ -22,6 +22,9 @@ struct user_struct;
 struct cred;
 struct inode;
 
+/* defined in sys.c, used in cred_setresuid */
+extern int set_user(struct cred *new);
+
 /*
  * COW Supplementary groups list
  */

@@ -396,4 +399,13 @@ do {						\
 	*(_fsgid) = __cred->fsgid;		\
 } while(0)
 
+#define CRED_SETID_NOFORCE 0
+#define CRED_SETID_FORCE 1
+extern int cred_setresuid(struct cred *new, uid_t ruid, uid_t euid, uid_t suid,
+		   int force);
+extern int cred_setresgid(struct cred *new, gid_t rgid, gid_t egid, gid_t sgid,
+		   int force);
+extern int cred_setfsuid(struct cred *new, uid_t uid, uid_t *old_fsuid);
+extern int cred_setfsgid(struct cred *new, gid_t gid, gid_t *old_fsgid);
+
 #endif /* _LINUX_CRED_H */

1.7.0.4

Granting userid capabilities to another task is a dangerous privilege. Don't just let file permissions authorize it. Define CAP_GRANT_ID as a new capability needed to write to /dev/caphash.

For one thing this lets us start a factotum server early on in init, then have init drop CAP_GRANT_ID from its bounding set so the rest of the system cannot regain it.

(This patch is only useful if the next patch, introducing p9auth fs, is upstreamed)

TODO - patch for capabilities.7 manpage

---
 include/linux/capability.h          |    6 +++++-
 security/selinux/include/classmap.h |    2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

index 39e5ff5..ba2cbfe 100644
@@ -355,7 +355,11 @@ struct cpu_vfs_cap_data {
 
 #define CAP_MAC_ADMIN        33
 
-#define CAP_LAST_CAP         CAP_MAC_ADMIN
+/* Allow granting setuid capabilities through p9auth /dev/caphash */
+
+#define CAP_GRANT_ID         34
+
+#define CAP_LAST_CAP         CAP_GRANT_ID
 
 #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
 

index 8b32e95..f0ec53a 100644
@@ -142,7 +142,7 @@ struct security_class_mapping secclass_map[] = {
 	    "node_bind", "name_connect", NULL } },
 	{ "memprotect", { "mmap_zero", NULL } },
 	{ "peer", { "recv", NULL } },
-	{ "capability2", { "mac_override", "mac_admin", NULL } },
+	{ "capability2", { "mac_override", "mac_admin", "grant_id", NULL } },
 	{ "kernel_service", { "use_as_override", "create_files_as", NULL } },
 	{ "tun_socket",
 	  { COMMON_SOCK_PERMS, NULL } },

1.7.0.4

This introduces a Plan 9 style setuid capability filesystem. See Documentation/p9auth.txt for a description of how to use this.

This fs allows the implementation of completely unprivileged login daemons. However, doing so requires a fundamental change regarding linux userids: a server privileged with the new CAP_GRANT_ID capability can create a one-time setuid capability allowing another process to change to one specific new userid. This is a change which must be discussed. The use of this privilege can be completely prevented by having init remove CAP_GRANT_ID from its capability bounding set before forking any processes.

Changelog
Apr 24:
return commit_creds (David Howells)
switch from dev to fs (Eric Biederman)
and move p9auth from drivers/char into kernel/

---
 Documentation/p9auth.txt |   42 ++++
 MAINTAINERS              |    6 +
 init/Kconfig             |    2 +
 kernel/Kconfig.p9auth    |    9 +
 kernel/Makefile          |    1 +
 kernel/p9auth.c          |  464 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 524 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/p9auth.txt
 create mode 100644 kernel/Kconfig.p9auth
 create mode 100644 kernel/p9auth.c

new file mode 100644
index 0000000..9e9f674
@@ -0,0 +1,42 @@
+The p9auth filesystem provides a plan-9 factotum-like setuid capability
+API.  Tasks which are privileged (authorized by possession of the
+CAP_GRANT_ID privilege (POSIX capability)) can write new capabilities to
+the p9authfs file called cred_grant.  The kernel then stores these until
+a task uses them by writing to the cred_use file.  Each capability
+represents the ability for a task running as userid X to switch to
+userid Y and some set of groups.  Each capability may be used only once,
+and unused capabilities are cleared after two minutes.
+
+The following examples shows how to use the API.  Shell 1 contains a
+privileged root shell.  Shell 2 contains an unprivileged shell as user
+501 in the same user namespace.  If not already done, the privileged
+shell should mount the p9auth filesystem:
+
+	mkdir /mnt/p9auth
+	mount -t p9auth p9auth /mnt/p9auth
+
+Now shell 2 somehow communicates to shell 1 that it possesses valid
+login credentials to switch to userid 502.  Shell 2 then looks up the
+groups which uid 502 is a member of, and builds a capability string to
+pass to the kernel.  It does this by concatenating the old userid, new
+userid, new primary group, number of auxiliary groups, and each
+auxiliary group, all as integers separated by '@'.  The resulting string
+is hashed with a random string.  In our example, userid 501 may
+transition to userid 502, with primary group 502 and auxiliary group 29.
+
+	capstr="501@502@502@1@29"
+	echo -n "$capstr" > /tmp/txtfile
+	randstr=`dd if=/dev/urandom count=1 2>/dev/null | \
+			uuencode -m - | head -n 2 | tail -n 1 | cut -c -8 `
+	openssl sha1 -hmac "$randstr" /tmp/txtfile | awk '{ print $2 '} \
+		> /tmp/hex
+	./unhex < /tmp/hex > /mnt/p9auth/cred_grant
+
+Note that to use an empty set of auxiliary groups, you may use
+	capstr = "501@502@02@0"
+
+The source for unhex.c can be found in the ltp testsuite under
+ltp-dev/testcases/kernel/security/p9auth.  To shell 2 it passes $capstr
+and $randstr.  Shell 2 can then transition to the new userid by doing
+
+	echo -n "$capstr@$randstr" > /mnt/p9auth/cred_use

index a0e3c3a..6bc1bd9 100644
@@ -4209,6 +4209,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mwu/mac80211-drivers.git
 S:	Maintained
 F:	drivers/net/wireless/p54/
 
+P9AUTH setuid capability filesystem
+M:	[email protected]
+L:	[email protected] (suggested Cc:)
+S:	Maintained
+F:	kernel/p9auth.c
+
 PA SEMI ETHERNET DRIVER
 M:	Olof Johansson <[email protected]>
 L:	[email protected]

index eb77e8c..bc7f1da 100644
@@ -715,6 +715,8 @@ config NET_NS
 	  Allow user space to create what appear to be multiple instances
 	  of the network stack.
 
+source "kernel/Kconfig.p9auth"
+
 config BLK_DEV_INITRD
 	bool "Initial RAM filesystem and RAM disk (initramfs/initrd) support"
 	depends on BROKEN || !FRV

new file mode 100644
index 0000000..d1c66d2
@@ -0,0 +1,9 @@
+config PLAN9AUTH
+	tristate "Plan 9 style capability device implementation"
+	default n
+	depends on CRYPTO
+	help
+	  This module implements the Plan 9 style capability device.
+
+	  To compile this driver as a module, choose
+	  M here: the module will be called p9auth.

index a987aa1..d27dae3 100644
@@ -105,6 +105,7 @@ obj-$(CONFIG_PERF_EVENTS) += perf_event.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
 obj-$(CONFIG_USER_RETURN_NOTIFIER) += user-return-notifier.o
 obj-$(CONFIG_PADATA) += padata.o
+obj-$(CONFIG_PLAN9AUTH) += p9auth.o
 
 ifneq ($(CONFIG_SCHED_OMIT_FRAME_POINTER),y)
 # According to Alan Modra <[email protected]>, the -fno-omit-frame-pointer is

1.7.0.4

Can't comment these changes due to the lack of knowledge, just a couple of minor nits.

Thanks, Oleg.

Ashwin had had this as ERESTARTSYS I believe. I'd read something about userspace should only see -EINTR so I changed it. Sounds like I need to follow the caller chain some more and learn a thing or two, before I repost.

Yes, I should check that, thanks!

I kzalloc()d to count+1 before, and only copy_from_user() count bytes, so the last byte should always be 0.

Thanks again,

-serge

Yes, ERESTARTxxx should not be visible to the user-space. But it is OK to return it from the syscall if signal_pending() is true, in this case it will be changed to EINTR or the system call will be restarted.

ERESTARTNOINTR always restarts the syscall, perphaps after handling the signal if it is really pending.

Ah, indeed, thanks.

Oleg.

Thanks for the comments, Oleg. I will fold this patch into the main patch, but I'll spare everyone a full repost right now.

thanks,
-serge

From: Serge E. Hallyn <[email protected]>
Subject: [PATCH 1/1] p9auth: address feedback

Address two comments by Oleg:

Return -ERESTARTNOINTR rather than -EINTR if we are interrupted while waiting on cap_mutex. And check size of input buffer for something sane. A user with hundreds of auxilliary groups could end up with a > PAGE_SIZE message, so maybe this is a bit too stringent, but I'm not sure how realistic that is.

---
 kernel/p9auth.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

index a174373..5f89b4d 100644
@@ -354,7 +354,10 @@ static ssize_t p9auth_grant_write(struct file *file, const char __user *buffer,
 	char *user_buf;
 
 	if (mutex_lock_interruptible(&cap_mutex))
-		return -EINTR;
+		return -ERESTARTNOINTR;
+
+	if (count > PAGE_SIZE)
+		return -E2BIG;
 
 	user_buf = kzalloc(count+1, GFP_KERNEL);
 	if (!user_buf)

@@ -387,7 +390,10 @@ static ssize_t p9auth_use_write(struct file *file, const char __user *buffer,
 	char *user_buf;
 
 	if (mutex_lock_interruptible(&cap_mutex))
-		return -EINTR;
+		return -ERESTARTNOINTR;
+
+	if (count > PAGE_SIZE)
+		return -E2BIG;
 
 	user_buf = kzalloc(count+1, GFP_KERNEL);
 	if (!user_buf)

1.7.0.4

Hm, yeah, the first doesn't need it at all, and the latter two should just kmalloc and set user_buf[count] = '\0'.

thanks,

-serge