[RFC][PATCH 0/2] KEYS: Kerberos caching support

[RFC][PATCH 0/2] KEYS: Kerberos caching support

[David Howells]

Here are a couple of keyrings patches I'd like to propose to aid the support
for Kerberos to cache tickets in keys:

 (1) A large-capacity key type.

     This is a general purpose key type that can hold up to 1MB of data.  The
     data is stored in a SHM file and so can be stored out to swap if need be.

     I think it might be worth storing the data encrypted in the SHM file with
     a randomly generated transient symmetric key that the kernel creates and
     retains in memory for each key.  This would then be lost when the power
     went off, rendering the tickets irretrievable.

     This is necessary because some Kerberos tickets, particularly those
     returned by Windows Active Directory, for instance, can be huge, easily
     64KB or more, due to additional information stored in the ticket (the
     MS-PAC).

 (2) Per-user_namespace registers of per-UID kerberos caches.

     This allows the kerberos cache to be retained beyond the life of all a
     user's processes so that the user's cron jobs can work.

     The user asks for their Kerberos cache by:

	krb_cache = keyctl_get_krbcache(uid, dest_keyring);

     The uid is -1 or the user's own UID for the user's own cache or the uid
     of some other user's cache (requires CAP_SETUID).

     The cache returned is a keyring named "_krb.<uid>" that the possessor can
     read, search, clear, invalidate, unlink from and add links to.  SELinux
     and co. get a say as to whether this call will succeed as the caller must
     have LINK permission on the cache keyring.

     Each uid's cache keyring is created when it first accessed and is given a
     timeout that is extended each time this function is called so that the
     keyring goes away after a while.  The timeout is configurable by sysctl
     but defaults to 3 days.

     Each user_namespace gets a lazily-created keyring that serves as the
     register.  The cache keyrings are added to it.  This means that standard
     key search and garbage collection facilities are available.

     The user_namespace's register goes away when it does.

Note that these patches were constructed on top of my keyring capacity
expansion patches - but there shouldn't be much to change to apply them on top
of the vanilla kernel.

David
---
David Howells (2):
      KEYS: Implement a big key type that can save to tmpfs
      KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches


 include/keys/big_key-type.h    |   27 ++++++
 include/linux/key.h            |    1 
 include/linux/user_namespace.h |    6 +
 include/uapi/linux/keyctl.h    |    1 
 kernel/user.c                  |    4 +
 kernel/user_namespace.c        |    2 
 security/keys/Kconfig          |   23 +++++
 security/keys/Makefile         |    2 
 security/keys/big_key.c        |  181 ++++++++++++++++++++++++++++++++++++++++
 security/keys/compat.c         |    3 +
 security/keys/internal.h       |    9 ++
 security/keys/keyctl.c         |    3 +
 security/keys/krbcache.c       |  132 +++++++++++++++++++++++++++++
 security/keys/sysctl.c         |   11 ++
 14 files changed, 405 insertions(+)
 create mode 100644 include/keys/big_key-type.h
 create mode 100644 security/keys/big_key.c
 create mode 100644 security/keys/krbcache.c

[PATCH 1/2] KEYS: Implement a big key type that can save to tmpfs

[David Howells]

Implement a big key type that can save its contents to tmpfs and thus
swapspace when memory is tight.  This is useful for Kerberos ticket caches.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/keys/big_key-type.h |   27 ++++++
 include/linux/key.h         |    1 
 security/keys/Kconfig       |   11 +++
 security/keys/Makefile      |    1 
 security/keys/big_key.c     |  181 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 221 insertions(+)
 create mode 100644 include/keys/big_key-type.h
 create mode 100644 security/keys/big_key.c

diff --git a/include/keys/big_key-type.h b/include/keys/big_key-type.h
new file mode 100644
index 0000000..6610d46
--- /dev/null
+++ b/include/keys/big_key-type.h
@@ -0,0 +1,27 @@
+/* Big capacity key type.
+ *
+ * Copyright (C) 2013 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#ifndef _KEYS_BIG_KEY_TYPE_H
+#define _KEYS_BIG_KEY_TYPE_H
+
+#include <linux/key-type.h>
+
+extern struct key_type key_type_big_key;
+
+struct key_preparsed_payload;
+
+extern int big_key_instantiate(struct key *key, struct key_preparsed_payload *prep);
+extern void big_key_revoke(struct key *key);
+extern void big_key_destroy(struct key *key);
+extern void big_key_describe(const struct key *big_key, struct seq_file *m);
+extern long big_key_read(const struct key *key, char __user *buffer, size_t buflen);
+
+#endif /* _KEYS_BIG_KEY_TYPE_H */
diff --git a/include/linux/key.h b/include/linux/key.h
index 2417f78..010dbb6 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -201,6 +201,7 @@ struct key {
 			unsigned long		value;
 			void __rcu		*rcudata;
 			void			*data;
+			void			*data2[2];
 		} payload;
 		struct assoc_array keys;
 	};
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index 15e0dfe..eafb335 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -20,6 +20,17 @@ config KEYS
 
 	  If you are unsure as to whether this is required, answer N.
 
+config BIG_KEYS
+	tristate "Large payload keys"
+	depends on KEYS
+	select TMPFS
+	help
+	  This option provides support for holding large keys within the kernel
+	  (for example Kerberos ticket caches).  The data may be stored out to
+	  swapspace by tmpfs.
+
+	  If you are unsure as to whether this is required, answer N.
+
 config TRUSTED_KEYS
 	tristate "TRUSTED KEYS"
 	depends on KEYS && TCG_TPM
diff --git a/security/keys/Makefile b/security/keys/Makefile
index 504aaa0..c487c77 100644
--- a/security/keys/Makefile
+++ b/security/keys/Makefile
@@ -22,5 +22,6 @@ obj-$(CONFIG_SYSCTL) += sysctl.o
 #
 # Key types
 #
+obj-$(CONFIG_BIG_KEYS) += big_key.o
 obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
 obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys/
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
new file mode 100644
index 0000000..bd19afc
--- /dev/null
+++ b/security/keys/big_key.c
@@ -0,0 +1,181 @@
+/* Big capacity key type
+ *
+ * Copyright (C) 2013 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/seq_file.h>
+#include <linux/file.h>
+#include <linux/shmem_fs.h>
+#include <linux/err.h>
+#include <keys/user-type.h>
+#include <keys/big_key-type.h>
+
+MODULE_LICENSE("GPL");
+
+/*
+ * big_key defined keys take an arbitrary string as the description and an
+ * arbitrary blob of data as the payload
+ */
+struct key_type key_type_big_key = {
+	.name			= "big_key",
+	.def_lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+	.instantiate		= big_key_instantiate,
+	.match			= user_match,
+	.revoke			= big_key_revoke,
+	.destroy		= big_key_destroy,
+	.describe		= big_key_describe,
+	.read			= big_key_read,
+};
+
+/*
+ * Instantiate a big key
+ */
+int big_key_instantiate(struct key *key, struct key_preparsed_payload *prep)
+{
+	struct path *path = (struct path *)&key->payload.data2;
+	struct file *file;
+	ssize_t written;
+	size_t datalen = prep->datalen;
+	int ret;
+
+	ret = -EINVAL;
+	if (datalen <= 0 || datalen > 1024 * 1024 || !prep->data)
+		goto error;
+
+	/* Set an arbitrary quota */
+	ret = key_payload_reserve(key, 16);
+	if (ret < 0)
+		goto error;
+
+	/* Create a shmem file to store the data in.  This will permit the data
+	 * to be swapped out if needed.
+	 *
+	 * TODO: Encrypt the stored data with a temporary key.
+	 */
+	file = shmem_file_setup("", datalen, 0);
+	if (IS_ERR(file))
+		goto err_quota;
+
+	written = kernel_write(file, prep->data, prep->datalen, 0);
+	if (written != datalen) {
+		if (written >= 0)
+			ret = -ENOMEM;
+		goto err_fput;
+	}
+
+	/* Pin the mount and dentry to the key so that we can open it again
+	 * later
+	 */
+	*path = file->f_path;
+	path_get(path);
+	fput(file);
+	return 0;
+
+err_fput:
+	fput(file);
+err_quota:
+	key_payload_reserve(key, 0);
+error:
+	return ret;
+}
+
+/*
+ * match big_keys on their name
+ */
+int big_key_match(const struct key *key, const void *description)
+{
+	return strcmp(key->description, description) == 0;
+}
+
+/*
+ * dispose of the links from a revoked keyring
+ * - called with the key sem write-locked
+ */
+void big_key_revoke(struct key *key)
+{
+	struct path *path = (struct path *)&key->payload.data2;
+
+	/* clear the quota */
+	key_payload_reserve(key, 0);
+	if (key_is_instantiated(key))
+		vfs_truncate(path, 0);
+}
+
+/*
+ * dispose of the data dangling from the corpse of a big_key key
+ */
+void big_key_destroy(struct key *key)
+{
+	struct path *path = (struct path *)&key->payload.data2;
+	path_put(path);
+	path->mnt = NULL;
+	path->dentry = NULL;
+}
+
+/*
+ * describe the big_key key
+ */
+void big_key_describe(const struct key *key, struct seq_file *m)
+{
+	struct path *path = (struct path *)&key->payload.data2;
+	struct dentry *dentry = path->dentry;
+	struct inode *inode = dentry ? dentry->d_inode : NULL;
+
+	seq_puts(m, key->description);
+
+	if (inode &&
+	    key_is_instantiated(key) &&
+	    !test_bit(KEY_FLAG_REVOKED, &key->flags))
+		seq_printf(m, ": %llu", i_size_read(inode));
+}
+
+/*
+ * read the key data
+ * - the key's semaphore is read-locked
+ */
+long big_key_read(const struct key *key, char __user *buffer, size_t buflen)
+{
+	struct path *path = (struct path *)&key->payload.data2;
+	struct file *file;
+	loff_t pos, size = i_size_read(path->dentry->d_inode);
+	long ret;
+
+	ret = size;
+	if (buffer && buflen >= size) {
+		file = dentry_open(path, O_RDONLY, current_cred());
+		if (IS_ERR(file))
+			return PTR_ERR(file);
+
+		pos = 0;
+		ret = vfs_read(file, buffer, size, &pos);
+		fput(file);
+		if (ret >= 0 && ret != size)
+			return -EIO;
+	}
+
+	return ret;
+}
+
+/*
+ * Module stuff
+ */
+static int __init big_key_init(void)
+{
+	return register_key_type(&key_type_big_key);
+}
+
+static void __exit big_key_cleanup(void)
+{
+	unregister_key_type(&key_type_big_key);
+}
+
+module_init(big_key_init);
+module_exit(big_key_cleanup);

[PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[David Howells]

Add support for per-user_namespace registers of persistent per-UID kerberos
caches held within the kernel.

This allows the kerberos cache to be retained beyond the life of all a user's
processes so that the user's cron jobs can work.

The kerberos cache is envisioned as a keyring/key tree looking something like:

	struct user_namespace
	  \___ .krb_cache keyring		- The register
		\___ _krb.0 keyring		- Root's Kerberos cache
		\___ _krb.5000 keyring		- User 5000's Kerberos cache
		\___ _krb.5001 keyring		- User 5001's Kerberos cache
			\___ tkt785 big_key	- A ccache blob
			\___ tkt12345 big_key	- Another ccache blob

Or possibly:

	struct user_namespace
	  \___ .krb_cache keyring		- The register
		\___ _krb.0 keyring		- Root's Kerberos cache
		\___ _krb.5000 keyring		- User 5000's Kerberos cache
		\___ _krb.5001 keyring		- User 5001's Kerberos cache
			\___ tkt785 keyring	- A ccache
				\___ krbtgt/REDHAT.COM@REDHAT.COM big_key
				\___ http/REDHAT.COM@REDHAT.COM user
				\___ afs/REDHAT.COM@REDHAT.COM user
				\___ nfs/REDHAT.COM@REDHAT.COM user
				\___ krbtgt/KERNEL.ORG@KERNEL.ORG big_key
				\___ http/KERNEL.ORG@KERNEL.ORG big_key

What goes into a particular Kerberos cache is entirely up to userspace.  Kernel
support is limited to giving you the Kerberos cache keyring that you want.

The user asks for their Kerberos cache by:

	krb_cache = keyctl_get_krbcache(uid, dest_keyring);

The uid is -1 or the user's own UID for the user's own cache or the uid of some
other user's cache (requires CAP_SETUID).  This permits rpc.gssd or whatever to
mess with the cache.

The cache returned is a keyring named "_krb.<uid>" that the possessor can read,
search, clear, invalidate, unlink from and add links to.  SELinux and co. get a
say as to whether this call will succeed as the caller must have LINK
permission on the cache keyring.

Each uid's cache keyring is created when it first accessed and is given a
timeout that is extended each time this function is called so that the keyring
goes away after a while.  The timeout is configurable by sysctl but defaults to
three days.

Each user_namespace struct gets a lazily-created keyring that serves as the
register.  The cache keyrings are added to it.  This means that standard key
search and garbage collection facilities are available.

The user_namespace struct's register goes away when it does and anything left
in it is then automatically gc'd.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Serge E. Hallyn <serge.hallyn@ubuntu.com>
cc: Eric W. Biederman <ebiederm@xmission.com>
---

 include/linux/user_namespace.h |    6 ++
 include/uapi/linux/keyctl.h    |    1 
 kernel/user.c                  |    4 +
 kernel/user_namespace.c        |    2 +
 security/keys/Kconfig          |   12 ++++
 security/keys/Makefile         |    1 
 security/keys/compat.c         |    3 +
 security/keys/internal.h       |    9 +++
 security/keys/keyctl.c         |    3 +
 security/keys/krbcache.c       |  132 ++++++++++++++++++++++++++++++++++++++++
 security/keys/sysctl.c         |   11 +++
 11 files changed, 184 insertions(+)
 create mode 100644 security/keys/krbcache.c

diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index b6b215f..3cce8c7 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -28,6 +28,12 @@ struct user_namespace {
 	unsigned int		proc_inum;
 	bool			may_mount_sysfs;
 	bool			may_mount_proc;
+
+	/* Register of per-UID Kerberos caches for this namespace */
+#ifdef CONFIG_KEYS_KERBEROS_CACHE
+	struct key		*krb_cache_register;
+	struct rw_semaphore	krb_cache_register_sem;
+#endif
 };
 
 extern struct user_namespace init_user_ns;
diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index c9b7f4fa..a37c62b 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -56,5 +56,6 @@
 #define KEYCTL_REJECT			19	/* reject a partially constructed key */
 #define KEYCTL_INSTANTIATE_IOV		20	/* instantiate a partially constructed key */
 #define KEYCTL_INVALIDATE		21	/* invalidate a key */
+#define KEYCTL_GET_KRBCACHE		22	/* get a user's kerberos cache keyring */
 
 #endif /*  _LINUX_KEYCTL_H */
diff --git a/kernel/user.c b/kernel/user.c
index 69b4c3d..6c9e1b9 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -53,6 +53,10 @@ struct user_namespace init_user_ns = {
 	.proc_inum = PROC_USER_INIT_INO,
 	.may_mount_sysfs = true,
 	.may_mount_proc = true,
+#ifdef CONFIG_KEYS_KERBEROS_CACHE
+	.krb_cache_register_sem =
+	__RWSEM_INITIALIZER(init_user_ns.krb_cache_register_sem),
+#endif
 };
 EXPORT_SYMBOL_GPL(init_user_ns);
 
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index d8c30db..098d954 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -99,6 +99,7 @@ int create_user_ns(struct cred *new)
 
 	update_mnt_policy(ns);
 
+	rwsem_init(&ns->krb_cache_register_sem);
 	return 0;
 }
 
@@ -123,6 +124,7 @@ void free_user_ns(struct user_namespace *ns)
 
 	do {
 		parent = ns->parent;
+		key_put(ns->krb_cache_register);
 		proc_free_inum(ns->proc_inum);
 		kmem_cache_free(user_ns_cachep, ns);
 		ns = parent;
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index eafb335..ee3f5a5 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -20,6 +20,18 @@ config KEYS
 
 	  If you are unsure as to whether this is required, answer N.
 
+config KEYS_KERBEROS_CACHE
+	bool "Enable persistent keyring-based kerberos cache"
+	depends on KEYS
+	help
+	  This option provides a register of per-UID kerberos cache keyrings.
+	  A particular keyring may be accessed by either the user whose keyring
+	  it is or by a process with administrative privileges.  SELinux gets
+	  to rule on which admin-level processes get to access the cache.
+
+	  Keyrings are created and added into the register upon demand and get
+	  removed if they expire (a default timeout is set upon creation).
+
 config BIG_KEYS
 	tristate "Large payload keys"
 	depends on KEYS
diff --git a/security/keys/Makefile b/security/keys/Makefile
index c487c77..c168ad6 100644
--- a/security/keys/Makefile
+++ b/security/keys/Makefile
@@ -18,6 +18,7 @@ obj-y := \
 obj-$(CONFIG_KEYS_COMPAT) += compat.o
 obj-$(CONFIG_PROC_FS) += proc.o
 obj-$(CONFIG_SYSCTL) += sysctl.o
+obj-$(CONFIG_KEYS_KERBEROS_CACHE) += krbcache.o
 
 #
 # Key types
diff --git a/security/keys/compat.c b/security/keys/compat.c
index d65fa7f..ead383b 100644
--- a/security/keys/compat.c
+++ b/security/keys/compat.c
@@ -138,6 +138,9 @@ asmlinkage long compat_sys_keyctl(u32 option,
 	case KEYCTL_INVALIDATE:
 		return keyctl_invalidate_key(arg2);
 
+	case KEYCTL_GET_KRBCACHE:
+		return keyctl_get_krbcache(arg2, arg3);
+
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 581c6f6..fa379c6 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -255,6 +255,15 @@ extern long keyctl_invalidate_key(key_serial_t);
 extern long keyctl_instantiate_key_common(key_serial_t,
 					  const struct iovec *,
 					  unsigned, size_t, key_serial_t);
+#ifdef CONFIG_KEYS_KERBEROS_CACHE
+extern long keyctl_get_krbcache(uid_t, key_serial_t);
+extern unsigned krb_cache_expiry;
+#else
+static inline long keyctl_get_krbcache(uid_t uid, key_serial_t destring)
+{
+	return -EOPNOTSUPP;
+}
+#endif
 
 /*
  * Debugging key validation
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 33cfd27..c4fae05 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1667,6 +1667,9 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case KEYCTL_INVALIDATE:
 		return keyctl_invalidate_key((key_serial_t) arg2);
 
+	case KEYCTL_GET_KRBCACHE:
+		return keyctl_get_krbcache((uid_t)arg2, (key_serial_t)arg3);
+
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/security/keys/krbcache.c b/security/keys/krbcache.c
new file mode 100644
index 0000000..4e2aa9c
--- /dev/null
+++ b/security/keys/krbcache.c
@@ -0,0 +1,132 @@
+/* Kerberos persistent cache register
+ *
+ * Copyright (C) 2013 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/user_namespace.h>
+#include "internal.h"
+
+unsigned krb_cache_expiry = 3 * 24 * 3600; /* Expire after 3 days of non-use */
+
+/*
+ * Get the Kerberos cache keyring for a specific UID and link it to the
+ * nominated keyring.
+ */
+long keyctl_get_krbcache(uid_t _uid, key_serial_t destid)
+{
+	struct user_namespace *ns = current_user_ns();
+	struct keyring_index_key index_key;
+	struct key *krb;
+	key_ref_t reg_ref, dest_ref, krb_ref;
+	kuid_t uid;
+	char buf[24];
+	long ret;
+
+	/* -1 indicates the current user */
+	if (_uid == (uid_t)-1) {
+		uid = current_uid();
+	} else {
+		uid = make_kuid(ns, _uid);
+		if (!uid_valid(uid))
+			return -EINVAL;
+
+		/* You can only see your own kerberos cache if you're not
+		 * sufficiently privileged.
+		 */
+		if (uid != current_uid() &&
+		    uid != current_suid() &&
+		    uid != current_euid() &&
+		    uid != current_fsuid() &&
+		    !nsown_capable(CAP_SETUID))
+			return -EPERM;
+	}
+
+	/* There must be a destination keyring */
+	dest_ref = lookup_user_key(destid, KEY_LOOKUP_CREATE, KEY_WRITE);
+	if (IS_ERR(dest_ref))
+		return PTR_ERR(dest_ref);
+	if (key_ref_to_ptr(dest_ref)->type != &key_type_keyring) {
+		ret = -ENOTDIR;
+		goto out_put_dest;
+	}
+
+	/* Look in the register if it exists */
+	index_key.type = &key_type_keyring;
+	index_key.description = buf;
+	index_key.desc_len = sprintf(buf, "_krb.%u", __kuid_val(uid));
+
+	if (ns->krb_cache_register) {
+		reg_ref = make_key_ref(ns->krb_cache_register, true);
+		down_read(&ns->krb_cache_register_sem);
+		krb_ref = find_key_to_update(reg_ref, &index_key);
+		up_read(&ns->krb_cache_register_sem);
+
+		if (krb_ref)
+			goto found;
+	}
+
+	/* It wasn't in the register, so we'll need to create it.  We might
+	 * also need to create the register.
+	 */
+	down_write(&ns->krb_cache_register_sem);
+
+	if (!ns->krb_cache_register) {
+		struct key *reg = 
+			keyring_alloc(".krb_cache",
+				      KUIDT_INIT(0), KGIDT_INIT(0),
+				      current_cred(),
+				      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+				       KEY_USR_VIEW | KEY_USR_READ),
+				      KEY_ALLOC_NOT_IN_QUOTA, NULL);
+		if (IS_ERR(reg)) {
+			up_write(&ns->krb_cache_register_sem);
+			ret = PTR_ERR(reg);
+			goto out_put_dest;
+		}
+
+		ns->krb_cache_register = reg;
+	} else {
+		reg_ref = make_key_ref(ns->krb_cache_register, true);
+		krb_ref = find_key_to_update(reg_ref, &index_key);
+		if (krb_ref) {
+			up_write(&ns->krb_cache_register_sem);
+			goto found;
+		}
+	}
+
+	krb = keyring_alloc(buf,
+			    uid, INVALID_GID, current_cred(),
+			    ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+			     KEY_USR_VIEW | KEY_USR_READ),
+			    KEY_ALLOC_NOT_IN_QUOTA,
+			    ns->krb_cache_register);
+	up_write(&ns->krb_cache_register_sem);
+	if (!IS_ERR(krb)) {
+		krb_ref = make_key_ref(krb, true);
+		goto found;
+	}
+
+out_put_krb:
+	key_ref_to_ptr(krb_ref);
+out_put_dest:
+	key_ref_to_ptr(dest_ref);
+	return ret;
+
+found:
+	ret = key_task_permission(krb_ref, current_cred(), KEY_LINK);
+	if (ret < 0)
+		goto out_put_krb;
+
+	ret = key_link(key_ref_to_ptr(dest_ref), key_ref_to_ptr(krb_ref));
+	if (ret == 0) {
+		key_set_timeout(key_ref_to_ptr(krb_ref), krb_cache_expiry);
+		ret = key_ref_to_ptr(krb_ref)->serial;		
+	}
+	goto out_put_krb;
+}
diff --git a/security/keys/sysctl.c b/security/keys/sysctl.c
index ee32d18..3af1798 100644
--- a/security/keys/sysctl.c
+++ b/security/keys/sysctl.c
@@ -61,5 +61,16 @@ ctl_table key_sysctls[] = {
 		.extra1 = (void *) &zero,
 		.extra2 = (void *) &max,
 	},
+#ifdef CONFIG_KEYS_KERBEROS_CACHE
+	{
+		.procname = "krb_expiry",
+		.data = &krb_cache_expiry,
+		.maxlen = sizeof(unsigned),
+		.mode = 0644,
+		.proc_handler = proc_dointvec_minmax,
+		.extra1 = (void *) &zero,
+		.extra2 = (void *) &max,
+	},
+#endif
 	{ }
 };

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

On 08/01/2013 01:39 PM, David Howells wrote:
> The uid is -1 or the user's own UID for the user's own cache or the uid of some
> other user's cache (requires CAP_SETUID).  This permits rpc.gssd or whatever to
> mess with the cache.

Is the goal here eventually to be able to avoid the upcall to rpc.gssd
entirely?  It seems a little bit roundabout to have the kernel call up
into userspace for the credentials, only to talk to a process which then
calls back into the kernel for something that the kernel has already
well-defined internally.

It seems like a non-privileged user could use this to store arbitrary
data in this keyring as a way of hiding what would otherwise be
filesystem activity or using it for some sort of odd/sneaky IPC
mechanism.  Is this an intentional side effect?

Sorry if these are obvious questions.  feel free to point me to
already-documented answers if they exist.

Thanks for all your work on this!

Regards,

	--dkg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1027 bytes --]

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Simo Sorce]

On Thu, 2013-08-01 at 13:54 -0400, Daniel Kahn Gillmor wrote:
> On 08/01/2013 01:39 PM, David Howells wrote:
> > The uid is -1 or the user's own UID for the user's own cache or the uid of some
> > other user's cache (requires CAP_SETUID).  This permits rpc.gssd or whatever to
> > mess with the cache.
> 
> Is the goal here eventually to be able to avoid the upcall to rpc.gssd
> entirely?

No, the kernel does not have a GSSAPI implementation anyway, and you do
not want one in kernel.

>   It seems a little bit roundabout to have the kernel call up
> into userspace for the credentials, only to talk to a process which then
> calls back into the kernel for something that the kernel has already
> well-defined internally.

It's called 'abstraction' :-)
The fact that nfs client is in kernel and that the keys api is in kernel
is basically just a coincidence.

> It seems like a non-privileged user could use this to store arbitrary
> data in this keyring as a way of hiding what would otherwise be
> filesystem activity or using it for some sort of odd/sneaky IPC
> mechanism.  Is this an intentional side effect?

Just as a user can add data into a shm segment ?
Is there any difference ?

> Sorry if these are obvious questions.  feel free to point me to
> already-documented answers if they exist.

There isn't much documentation, but it is certainly good to sort out any
questions so we can add answer to any documentation we will come up
with.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]

On 08/01/2013 02:29 PM, Simo Sorce wrote:

> It's called 'abstraction' :-)

Good, I like abstraction :)

>> It seems like a non-privileged user could use this to store arbitrary
>> data in this keyring as a way of hiding what would otherwise be
>> filesystem activity or using it for some sort of odd/sneaky IPC
>> mechanism.  Is this an intentional side effect?
> 
> Just as a user can add data into a shm segment ?
> Is there any difference ?

I guess this raises the question from a different perspective: if the
kernel already supports arbitrary shm segments, filesystem locations,
etc, which can be used for storing/passing opaque bytestrings between
different parts of userspace, what advantages do we gain from having
this new specific mechanism in the kernel?  Why couldn't those parts of
userspace just rely on already-existing mechanisms instead of
introducing this new interface?

Again, i'm not trying to say it's a bad idea; there's probably a
big-picture piece of the puzzle that i don't see that makes this all
obvious.  i just want to understand what it is.  Thanks for your
explanations!

Regards,

	--dkg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1027 bytes --]

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Simo Sorce]

On Thu, 2013-08-01 at 14:55 -0400, Daniel Kahn Gillmor wrote:
> On 08/01/2013 02:29 PM, Simo Sorce wrote:
> 
> > It's called 'abstraction' :-)
> 
> Good, I like abstraction :)
> 
> >> It seems like a non-privileged user could use this to store arbitrary
> >> data in this keyring as a way of hiding what would otherwise be
> >> filesystem activity or using it for some sort of odd/sneaky IPC
> >> mechanism.  Is this an intentional side effect?
> > 
> > Just as a user can add data into a shm segment ?
> > Is there any difference ?
> 
> I guess this raises the question from a different perspective: if the
> kernel already supports arbitrary shm segments, filesystem locations,
> etc, which can be used for storing/passing opaque bytestrings between
> different parts of userspace, what advantages do we gain from having
> this new specific mechanism in the kernel?  Why couldn't those parts of
> userspace just rely on already-existing mechanisms instead of
> introducing this new interface?
> 
> Again, i'm not trying to say it's a bad idea; there's probably a
> big-picture piece of the puzzle that i don't see that makes this all
> obvious.  i just want to understand what it is.  Thanks for your
> explanations!

A few things deal with creation of the keyring and access control,
automatic garbage collection and in future the fact this data will not
leak in an intelligible form to disc (once we add the encryption bit to
the big_key type). Avoidance of naming collision and so on.
There is probably something else I am forgetting now.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Eric W. Biederman]

David Howells <dhowells@redhat.com> writes:

> Add support for per-user_namespace registers of persistent per-UID kerberos
> caches held within the kernel.

Out of curiosity is this cache per user namspace because the key lookup
is per user namespace?

Some minor nits below. But I don't see anything particulary scary about
this patch.  Other than seeming to make it easy for root to get my
kerbose tickets.

Eric

> This allows the kerberos cache to be retained beyond the life of all a user's
> processes so that the user's cron jobs can work.
>
> The kerberos cache is envisioned as a keyring/key tree looking something like:
>
> 	struct user_namespace
> 	  \___ .krb_cache keyring		- The register
> 		\___ _krb.0 keyring		- Root's Kerberos cache
> 		\___ _krb.5000 keyring		- User 5000's Kerberos cache
> 		\___ _krb.5001 keyring		- User 5001's Kerberos cache
> 			\___ tkt785 big_key	- A ccache blob
> 			\___ tkt12345 big_key	- Another ccache blob
>
> Or possibly:
>
> 	struct user_namespace
> 	  \___ .krb_cache keyring		- The register
> 		\___ _krb.0 keyring		- Root's Kerberos cache
> 		\___ _krb.5000 keyring		- User 5000's Kerberos cache
> 		\___ _krb.5001 keyring		- User 5001's Kerberos cache
> 			\___ tkt785 keyring	- A ccache
> 				\___ krbtgt/REDHAT.COM@REDHAT.COM big_key
> 				\___ http/REDHAT.COM@REDHAT.COM user
> 				\___ afs/REDHAT.COM@REDHAT.COM user
> 				\___ nfs/REDHAT.COM@REDHAT.COM user
> 				\___ krbtgt/KERNEL.ORG@KERNEL.ORG big_key
> 				\___ http/KERNEL.ORG@KERNEL.ORG big_key
>
> What goes into a particular Kerberos cache is entirely up to userspace.  Kernel
> support is limited to giving you the Kerberos cache keyring that you want.
>
> The user asks for their Kerberos cache by:
>
> 	krb_cache = keyctl_get_krbcache(uid, dest_keyring);
>
> The uid is -1 or the user's own UID for the user's own cache or the uid of some
> other user's cache (requires CAP_SETUID).  This permits rpc.gssd or whatever to
> mess with the cache.
>
> The cache returned is a keyring named "_krb.<uid>" that the possessor can read,
> search, clear, invalidate, unlink from and add links to.  SELinux and co. get a
> say as to whether this call will succeed as the caller must have LINK
> permission on the cache keyring.

I think it would be more accurate to say you use the existing LSM
security hooks for security keys.

Calling out SELinux in particular just seems odd as there is absolutely
nothing SELinux specific in this patch.

> Each uid's cache keyring is created when it first accessed and is given a
> timeout that is extended each time this function is called so that the keyring
> goes away after a while.  The timeout is configurable by sysctl but defaults to
> three days.
>
> Each user_namespace struct gets a lazily-created keyring that serves as the
> register.  The cache keyrings are added to it.  This means that standard key
> search and garbage collection facilities are available.
>
> The user_namespace struct's register goes away when it does and anything left
> in it is then automatically gc'd.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> cc: Eric W. Biederman <ebiederm@xmission.com>
> ---
>
>  include/linux/user_namespace.h |    6 ++
>  include/uapi/linux/keyctl.h    |    1 
>  kernel/user.c                  |    4 +
>  kernel/user_namespace.c        |    2 +
>  security/keys/Kconfig          |   12 ++++
>  security/keys/Makefile         |    1 
>  security/keys/compat.c         |    3 +
>  security/keys/internal.h       |    9 +++
>  security/keys/keyctl.c         |    3 +
>  security/keys/krbcache.c       |  132 ++++++++++++++++++++++++++++++++++++++++
>  security/keys/sysctl.c         |   11 +++
>  11 files changed, 184 insertions(+)
>  create mode 100644 security/keys/krbcache.c
>
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index b6b215f..3cce8c7 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -28,6 +28,12 @@ struct user_namespace {
>  	unsigned int		proc_inum;
>  	bool			may_mount_sysfs;
>  	bool			may_mount_proc;
> +
> +	/* Register of per-UID Kerberos caches for this namespace */
> +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> +	struct key		*krb_cache_register;
> +	struct rw_semaphore	krb_cache_register_sem;
> +#endif
>  };
>  
>  extern struct user_namespace init_user_ns;
> diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
> index c9b7f4fa..a37c62b 100644
> --- a/include/uapi/linux/keyctl.h
> +++ b/include/uapi/linux/keyctl.h
> @@ -56,5 +56,6 @@
>  #define KEYCTL_REJECT			19	/* reject a partially constructed key */
>  #define KEYCTL_INSTANTIATE_IOV		20	/* instantiate a partially constructed key */
>  #define KEYCTL_INVALIDATE		21	/* invalidate a key */
> +#define KEYCTL_GET_KRBCACHE		22	/* get a user's kerberos cache keyring */
>  
>  #endif /*  _LINUX_KEYCTL_H */
> diff --git a/kernel/user.c b/kernel/user.c
> index 69b4c3d..6c9e1b9 100644
> --- a/kernel/user.c
> +++ b/kernel/user.c
> @@ -53,6 +53,10 @@ struct user_namespace init_user_ns = {
>  	.proc_inum = PROC_USER_INIT_INO,
>  	.may_mount_sysfs = true,
>  	.may_mount_proc = true,
> +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> +	.krb_cache_register_sem =
> +	__RWSEM_INITIALIZER(init_user_ns.krb_cache_register_sem),
> +#endif
>  };
>  EXPORT_SYMBOL_GPL(init_user_ns);
>  
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index d8c30db..098d954 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -99,6 +99,7 @@ int create_user_ns(struct cred *new)
>  
>  	update_mnt_policy(ns);
>  
> +	rwsem_init(&ns->krb_cache_register_sem);
>  	return 0;
>  }
>  
> @@ -123,6 +124,7 @@ void free_user_ns(struct user_namespace *ns)
>  
>  	do {
>  		parent = ns->parent;
> +		key_put(ns->krb_cache_register);
>  		proc_free_inum(ns->proc_inum);
>  		kmem_cache_free(user_ns_cachep, ns);
>  		ns = parent;
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index eafb335..ee3f5a5 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -20,6 +20,18 @@ config KEYS
>  
>  	  If you are unsure as to whether this is required, answer N.
>  
> +config KEYS_KERBEROS_CACHE
> +	bool "Enable persistent keyring-based kerberos cache"
> +	depends on KEYS
> +	help
> +	  This option provides a register of per-UID kerberos cache keyrings.
> +	  A particular keyring may be accessed by either the user whose keyring
> +	  it is or by a process with administrative privileges.  SELinux gets
> +	  to rule on which admin-level processes get to access the cache.
> +
> +	  Keyrings are created and added into the register upon demand and get
> +	  removed if they expire (a default timeout is set upon creation).
> +
>  config BIG_KEYS
>  	tristate "Large payload keys"
>  	depends on KEYS
> diff --git a/security/keys/Makefile b/security/keys/Makefile
> index c487c77..c168ad6 100644
> --- a/security/keys/Makefile
> +++ b/security/keys/Makefile
> @@ -18,6 +18,7 @@ obj-y := \
>  obj-$(CONFIG_KEYS_COMPAT) += compat.o
>  obj-$(CONFIG_PROC_FS) += proc.o
>  obj-$(CONFIG_SYSCTL) += sysctl.o
> +obj-$(CONFIG_KEYS_KERBEROS_CACHE) += krbcache.o
>  
>  #
>  # Key types
> diff --git a/security/keys/compat.c b/security/keys/compat.c
> index d65fa7f..ead383b 100644
> --- a/security/keys/compat.c
> +++ b/security/keys/compat.c
> @@ -138,6 +138,9 @@ asmlinkage long compat_sys_keyctl(u32 option,
>  	case KEYCTL_INVALIDATE:
>  		return keyctl_invalidate_key(arg2);
>  
> +	case KEYCTL_GET_KRBCACHE:
> +		return keyctl_get_krbcache(arg2, arg3);
> +
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index 581c6f6..fa379c6 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -255,6 +255,15 @@ extern long keyctl_invalidate_key(key_serial_t);
>  extern long keyctl_instantiate_key_common(key_serial_t,
>  					  const struct iovec *,
>  					  unsigned, size_t, key_serial_t);
> +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> +extern long keyctl_get_krbcache(uid_t, key_serial_t);
> +extern unsigned krb_cache_expiry;
> +#else
> +static inline long keyctl_get_krbcache(uid_t uid, key_serial_t destring)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif
>  
>  /*
>   * Debugging key validation
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index 33cfd27..c4fae05 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -1667,6 +1667,9 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
>  	case KEYCTL_INVALIDATE:
>  		return keyctl_invalidate_key((key_serial_t) arg2);
>  
> +	case KEYCTL_GET_KRBCACHE:
> +		return keyctl_get_krbcache((uid_t)arg2, (key_serial_t)arg3);
> +
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> diff --git a/security/keys/krbcache.c b/security/keys/krbcache.c
> new file mode 100644
> index 0000000..4e2aa9c
> --- /dev/null
> +++ b/security/keys/krbcache.c
> @@ -0,0 +1,132 @@
> +/* Kerberos persistent cache register
> + *
> + * Copyright (C) 2013 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +
> +#include <linux/user_namespace.h>
> +#include "internal.h"
> +
> +unsigned krb_cache_expiry = 3 * 24 * 3600; /* Expire after 3 days of non-use */
> +
> +/*
> + * Get the Kerberos cache keyring for a specific UID and link it to the
> + * nominated keyring.
> + */
> +long keyctl_get_krbcache(uid_t _uid, key_serial_t destid)
> +{
> +	struct user_namespace *ns = current_user_ns();
> +	struct keyring_index_key index_key;
> +	struct key *krb;
> +	key_ref_t reg_ref, dest_ref, krb_ref;
> +	kuid_t uid;
> +	char buf[24];
> +	long ret;
> +
> +	/* -1 indicates the current user */
> +	if (_uid == (uid_t)-1) {
> +		uid = current_uid();
> +	} else {
> +		uid = make_kuid(ns, _uid);
> +		if (!uid_valid(uid))
> +			return -EINVAL;
> +
> +		/* You can only see your own kerberos cache if you're not
> +		 * sufficiently privileged.
> +		 */
> +		if (uid != current_uid() &&
> +		    uid != current_suid() &&
> +		    uid != current_euid() &&
 > +		    uid != current_fsuid() &&
> +		    !nsown_capable(CAP_SETUID))
You you make this ns_capable(ns, CAP_SETUID);

nsown_capable is the right thing here but I am trying to remove the
function because it makes it too easy to not think about which
user namespace you are in.

> +			return -EPERM;
> +	}
> +
> +	/* There must be a destination keyring */
> +	dest_ref = lookup_user_key(destid, KEY_LOOKUP_CREATE, KEY_WRITE);
> +	if (IS_ERR(dest_ref))
> +		return PTR_ERR(dest_ref);
> +	if (key_ref_to_ptr(dest_ref)->type != &key_type_keyring) {
> +		ret = -ENOTDIR;
> +		goto out_put_dest;
> +	}
> +
> +	/* Look in the register if it exists */
> +	index_key.type = &key_type_keyring;
> +	index_key.description = buf;
> +	index_key.desc_len = sprintf(buf, "_krb.%u", __kuid_val(uid));

Please don't use the implementation detail __kuid_val.  Please use
from_kuid(&init_user_ns, uid) instead so it is explicitly documented
which user namespace you are using.

> +	if (ns->krb_cache_register) {
> +		reg_ref = make_key_ref(ns->krb_cache_register, true);
> +		down_read(&ns->krb_cache_register_sem);
> +		krb_ref = find_key_to_update(reg_ref, &index_key);
> +		up_read(&ns->krb_cache_register_sem);
> +
> +		if (krb_ref)
> +			goto found;
> +	}
> +
> +	/* It wasn't in the register, so we'll need to create it.  We might
> +	 * also need to create the register.
> +	 */
> +	down_write(&ns->krb_cache_register_sem);
> +
> +	if (!ns->krb_cache_register) {
> +		struct key *reg = 
> +			keyring_alloc(".krb_cache",
> +				      KUIDT_INIT(0), KGIDT_INIT(0),
> +				      current_cred(),
> +				      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +				       KEY_USR_VIEW | KEY_USR_READ),
> +				      KEY_ALLOC_NOT_IN_QUOTA, NULL);
> +		if (IS_ERR(reg)) {
> +			up_write(&ns->krb_cache_register_sem);
> +			ret = PTR_ERR(reg);
> +			goto out_put_dest;
> +		}
> +
> +		ns->krb_cache_register = reg;
> +	} else {
> +		reg_ref = make_key_ref(ns->krb_cache_register, true);
> +		krb_ref = find_key_to_update(reg_ref, &index_key);
> +		if (krb_ref) {
> +			up_write(&ns->krb_cache_register_sem);
> +			goto found;
> +		}
> +	}
> +
> +	krb = keyring_alloc(buf,
> +			    uid, INVALID_GID, current_cred(),
> +			    ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +			     KEY_USR_VIEW | KEY_USR_READ),
> +			    KEY_ALLOC_NOT_IN_QUOTA,
> +			    ns->krb_cache_register);
> +	up_write(&ns->krb_cache_register_sem);
> +	if (!IS_ERR(krb)) {
> +		krb_ref = make_key_ref(krb, true);
> +		goto found;
> +	}
> +
> +out_put_krb:
> +	key_ref_to_ptr(krb_ref);
> +out_put_dest:
> +	key_ref_to_ptr(dest_ref);
> +	return ret;
> +
> +found:
> +	ret = key_task_permission(krb_ref, current_cred(), KEY_LINK);
> +	if (ret < 0)
> +		goto out_put_krb;
> +
> +	ret = key_link(key_ref_to_ptr(dest_ref), key_ref_to_ptr(krb_ref));
> +	if (ret == 0) {
> +		key_set_timeout(key_ref_to_ptr(krb_ref), krb_cache_expiry);
> +		ret = key_ref_to_ptr(krb_ref)->serial;		
> +	}
> +	goto out_put_krb;
> +}
> diff --git a/security/keys/sysctl.c b/security/keys/sysctl.c
> index ee32d18..3af1798 100644
> --- a/security/keys/sysctl.c
> +++ b/security/keys/sysctl.c
> @@ -61,5 +61,16 @@ ctl_table key_sysctls[] = {
>  		.extra1 = (void *) &zero,
>  		.extra2 = (void *) &max,
>  	},
> +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> +	{
> +		.procname = "krb_expiry",
> +		.data = &krb_cache_expiry,
> +		.maxlen = sizeof(unsigned),
> +		.mode = 0644,
> +		.proc_handler = proc_dointvec_minmax,
> +		.extra1 = (void *) &zero,
> +		.extra2 = (void *) &max,
> +	},
> +#endif
>  	{ }
>  };

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Jeff Layton]

On Thu, 01 Aug 2013 18:39:02 +0100
David Howells <dhowells@redhat.com> wrote:

> Add support for per-user_namespace registers of persistent per-UID kerberos
> caches held within the kernel.
> 
> This allows the kerberos cache to be retained beyond the life of all a user's
> processes so that the user's cron jobs can work.
> 
> The kerberos cache is envisioned as a keyring/key tree looking something like:
> 
> 	struct user_namespace
> 	  \___ .krb_cache keyring		- The register
> 		\___ _krb.0 keyring		- Root's Kerberos cache
> 		\___ _krb.5000 keyring		- User 5000's Kerberos cache
> 		\___ _krb.5001 keyring		- User 5001's Kerberos cache
> 			\___ tkt785 big_key	- A ccache blob
> 			\___ tkt12345 big_key	- Another ccache blob
> 
> Or possibly:
> 
> 	struct user_namespace
> 	  \___ .krb_cache keyring		- The register
> 		\___ _krb.0 keyring		- Root's Kerberos cache
> 		\___ _krb.5000 keyring		- User 5000's Kerberos cache
> 		\___ _krb.5001 keyring		- User 5001's Kerberos cache
> 			\___ tkt785 keyring	- A ccache
> 				\___ krbtgt/REDHAT.COM@REDHAT.COM big_key
> 				\___ http/REDHAT.COM@REDHAT.COM user
> 				\___ afs/REDHAT.COM@REDHAT.COM user
> 				\___ nfs/REDHAT.COM@REDHAT.COM user
> 				\___ krbtgt/KERNEL.ORG@KERNEL.ORG big_key
> 				\___ http/KERNEL.ORG@KERNEL.ORG big_key
> 
> What goes into a particular Kerberos cache is entirely up to userspace.  Kernel
> support is limited to giving you the Kerberos cache keyring that you want.
> 
> The user asks for their Kerberos cache by:
> 
> 	krb_cache = keyctl_get_krbcache(uid, dest_keyring);
> 
> The uid is -1 or the user's own UID for the user's own cache or the uid of some
> other user's cache (requires CAP_SETUID).  This permits rpc.gssd or whatever to
> mess with the cache.
> 
> The cache returned is a keyring named "_krb.<uid>" that the possessor can read,
> search, clear, invalidate, unlink from and add links to.  SELinux and co. get a
> say as to whether this call will succeed as the caller must have LINK
> permission on the cache keyring.
> 
> Each uid's cache keyring is created when it first accessed and is given a
> timeout that is extended each time this function is called so that the keyring
> goes away after a while.  The timeout is configurable by sysctl but defaults to
> three days.
> 
> Each user_namespace struct gets a lazily-created keyring that serves as the
> register.  The cache keyrings are added to it.  This means that standard key
> search and garbage collection facilities are available.
> 
> The user_namespace struct's register goes away when it does and anything left
> in it is then automatically gc'd.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> cc: Eric W. Biederman <ebiederm@xmission.com>
> ---
> 
>  include/linux/user_namespace.h |    6 ++
>  include/uapi/linux/keyctl.h    |    1 
>  kernel/user.c                  |    4 +
>  kernel/user_namespace.c        |    2 +
>  security/keys/Kconfig          |   12 ++++
>  security/keys/Makefile         |    1 
>  security/keys/compat.c         |    3 +
>  security/keys/internal.h       |    9 +++
>  security/keys/keyctl.c         |    3 +
>  security/keys/krbcache.c       |  132 ++++++++++++++++++++++++++++++++++++++++
>  security/keys/sysctl.c         |   11 +++
>  11 files changed, 184 insertions(+)
>  create mode 100644 security/keys/krbcache.c
> 
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index b6b215f..3cce8c7 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -28,6 +28,12 @@ struct user_namespace {
>  	unsigned int		proc_inum;
>  	bool			may_mount_sysfs;
>  	bool			may_mount_proc;
> +
> +	/* Register of per-UID Kerberos caches for this namespace */
> +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> +	struct key		*krb_cache_register;
> +	struct rw_semaphore	krb_cache_register_sem;
> +#endif
>  };
>  
>  extern struct user_namespace init_user_ns;
> diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
> index c9b7f4fa..a37c62b 100644
> --- a/include/uapi/linux/keyctl.h
> +++ b/include/uapi/linux/keyctl.h
> @@ -56,5 +56,6 @@
>  #define KEYCTL_REJECT			19	/* reject a partially constructed key */
>  #define KEYCTL_INSTANTIATE_IOV		20	/* instantiate a partially constructed key */
>  #define KEYCTL_INVALIDATE		21	/* invalidate a key */
> +#define KEYCTL_GET_KRBCACHE		22	/* get a user's kerberos cache keyring */
>  
>  #endif /*  _LINUX_KEYCTL_H */
> diff --git a/kernel/user.c b/kernel/user.c
> index 69b4c3d..6c9e1b9 100644
> --- a/kernel/user.c
> +++ b/kernel/user.c
> @@ -53,6 +53,10 @@ struct user_namespace init_user_ns = {
>  	.proc_inum = PROC_USER_INIT_INO,
>  	.may_mount_sysfs = true,
>  	.may_mount_proc = true,
> +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> +	.krb_cache_register_sem =
> +	__RWSEM_INITIALIZER(init_user_ns.krb_cache_register_sem),
> +#endif
>  };
>  EXPORT_SYMBOL_GPL(init_user_ns);
>  
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index d8c30db..098d954 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -99,6 +99,7 @@ int create_user_ns(struct cred *new)
>  
>  	update_mnt_policy(ns);
>  
> +	rwsem_init(&ns->krb_cache_register_sem);
>  	return 0;
>  }
>  
> @@ -123,6 +124,7 @@ void free_user_ns(struct user_namespace *ns)
>  
>  	do {
>  		parent = ns->parent;
> +		key_put(ns->krb_cache_register);
>  		proc_free_inum(ns->proc_inum);
>  		kmem_cache_free(user_ns_cachep, ns);
>  		ns = parent;
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index eafb335..ee3f5a5 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -20,6 +20,18 @@ config KEYS
>  
>  	  If you are unsure as to whether this is required, answer N.
>  
> +config KEYS_KERBEROS_CACHE
> +	bool "Enable persistent keyring-based kerberos cache"
> +	depends on KEYS
> +	help
> +	  This option provides a register of per-UID kerberos cache keyrings.
> +	  A particular keyring may be accessed by either the user whose keyring
> +	  it is or by a process with administrative privileges.  SELinux gets
> +	  to rule on which admin-level processes get to access the cache.
> +
> +	  Keyrings are created and added into the register upon demand and get
> +	  removed if they expire (a default timeout is set upon creation).
> +
>  config BIG_KEYS
>  	tristate "Large payload keys"
>  	depends on KEYS
> diff --git a/security/keys/Makefile b/security/keys/Makefile
> index c487c77..c168ad6 100644
> --- a/security/keys/Makefile
> +++ b/security/keys/Makefile
> @@ -18,6 +18,7 @@ obj-y := \
>  obj-$(CONFIG_KEYS_COMPAT) += compat.o
>  obj-$(CONFIG_PROC_FS) += proc.o
>  obj-$(CONFIG_SYSCTL) += sysctl.o
> +obj-$(CONFIG_KEYS_KERBEROS_CACHE) += krbcache.o
>  
>  #
>  # Key types
> diff --git a/security/keys/compat.c b/security/keys/compat.c
> index d65fa7f..ead383b 100644
> --- a/security/keys/compat.c
> +++ b/security/keys/compat.c
> @@ -138,6 +138,9 @@ asmlinkage long compat_sys_keyctl(u32 option,
>  	case KEYCTL_INVALIDATE:
>  		return keyctl_invalidate_key(arg2);
>  
> +	case KEYCTL_GET_KRBCACHE:
> +		return keyctl_get_krbcache(arg2, arg3);
> +
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index 581c6f6..fa379c6 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -255,6 +255,15 @@ extern long keyctl_invalidate_key(key_serial_t);
>  extern long keyctl_instantiate_key_common(key_serial_t,
>  					  const struct iovec *,
>  					  unsigned, size_t, key_serial_t);
> +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> +extern long keyctl_get_krbcache(uid_t, key_serial_t);
> +extern unsigned krb_cache_expiry;
> +#else
> +static inline long keyctl_get_krbcache(uid_t uid, key_serial_t destring)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif
>  
>  /*
>   * Debugging key validation
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index 33cfd27..c4fae05 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -1667,6 +1667,9 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
>  	case KEYCTL_INVALIDATE:
>  		return keyctl_invalidate_key((key_serial_t) arg2);
>  
> +	case KEYCTL_GET_KRBCACHE:
> +		return keyctl_get_krbcache((uid_t)arg2, (key_serial_t)arg3);
> +
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> diff --git a/security/keys/krbcache.c b/security/keys/krbcache.c
> new file mode 100644
> index 0000000..4e2aa9c
> --- /dev/null
> +++ b/security/keys/krbcache.c
> @@ -0,0 +1,132 @@
> +/* Kerberos persistent cache register
> + *
> + * Copyright (C) 2013 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +
> +#include <linux/user_namespace.h>
> +#include "internal.h"
> +
> +unsigned krb_cache_expiry = 3 * 24 * 3600; /* Expire after 3 days of non-use */
> +
> +/*
> + * Get the Kerberos cache keyring for a specific UID and link it to the
> + * nominated keyring.
> + */
> +long keyctl_get_krbcache(uid_t _uid, key_serial_t destid)
> +{
> +	struct user_namespace *ns = current_user_ns();
> +	struct keyring_index_key index_key;
> +	struct key *krb;
> +	key_ref_t reg_ref, dest_ref, krb_ref;
> +	kuid_t uid;
> +	char buf[24];
> +	long ret;
> +
> +	/* -1 indicates the current user */
> +	if (_uid == (uid_t)-1) {
> +		uid = current_uid();


Isn't it possible to have a valid uid of (unsigned int)-1? I know that
at least some sites use that for "nobody". Why not just require passing
in the correct UID?

> +	} else {
> +		uid = make_kuid(ns, _uid);
> +		if (!uid_valid(uid))
> +			return -EINVAL;
> +
> +		/* You can only see your own kerberos cache if you're not
> +		 * sufficiently privileged.
> +		 */
> +		if (uid != current_uid() &&
> +		    uid != current_suid() &&
> +		    uid != current_euid() &&
> +		    uid != current_fsuid() &&
> +		    !nsown_capable(CAP_SETUID))
> +			return -EPERM;
> +	}
> +
> +	/* There must be a destination keyring */
> +	dest_ref = lookup_user_key(destid, KEY_LOOKUP_CREATE, KEY_WRITE);
> +	if (IS_ERR(dest_ref))
> +		return PTR_ERR(dest_ref);
> +	if (key_ref_to_ptr(dest_ref)->type != &key_type_keyring) {
> +		ret = -ENOTDIR;
> +		goto out_put_dest;
> +	}
> +
> +	/* Look in the register if it exists */
> +	index_key.type = &key_type_keyring;
> +	index_key.description = buf;
> +	index_key.desc_len = sprintf(buf, "_krb.%u", __kuid_val(uid));
> +
> +	if (ns->krb_cache_register) {
> +		reg_ref = make_key_ref(ns->krb_cache_register, true);
> +		down_read(&ns->krb_cache_register_sem);
> +		krb_ref = find_key_to_update(reg_ref, &index_key);
> +		up_read(&ns->krb_cache_register_sem);
> +
> +		if (krb_ref)
> +			goto found;
> +	}
> +
> +	/* It wasn't in the register, so we'll need to create it.  We might
> +	 * also need to create the register.
> +	 */
> +	down_write(&ns->krb_cache_register_sem);
> +
> +	if (!ns->krb_cache_register) {
> +		struct key *reg = 
> +			keyring_alloc(".krb_cache",
> +				      KUIDT_INIT(0), KGIDT_INIT(0),
> +				      current_cred(),
> +				      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +				       KEY_USR_VIEW | KEY_USR_READ),
> +				      KEY_ALLOC_NOT_IN_QUOTA, NULL);
> +		if (IS_ERR(reg)) {
> +			up_write(&ns->krb_cache_register_sem);
> +			ret = PTR_ERR(reg);
> +			goto out_put_dest;
> +		}
> +
> +		ns->krb_cache_register = reg;
> +	} else {
> +		reg_ref = make_key_ref(ns->krb_cache_register, true);
> +		krb_ref = find_key_to_update(reg_ref, &index_key);
> +		if (krb_ref) {
> +			up_write(&ns->krb_cache_register_sem);
> +			goto found;
> +		}
> +	}
> +
> +	krb = keyring_alloc(buf,
> +			    uid, INVALID_GID, current_cred(),
> +			    ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +			     KEY_USR_VIEW | KEY_USR_READ),
> +			    KEY_ALLOC_NOT_IN_QUOTA,
> +			    ns->krb_cache_register);
> +	up_write(&ns->krb_cache_register_sem);
> +	if (!IS_ERR(krb)) {
> +		krb_ref = make_key_ref(krb, true);
> +		goto found;
> +	}
> +
> +out_put_krb:
> +	key_ref_to_ptr(krb_ref);
> +out_put_dest:
> +	key_ref_to_ptr(dest_ref);
> +	return ret;
> +
> +found:
> +	ret = key_task_permission(krb_ref, current_cred(), KEY_LINK);
> +	if (ret < 0)
> +		goto out_put_krb;
> +
> +	ret = key_link(key_ref_to_ptr(dest_ref), key_ref_to_ptr(krb_ref));
> +	if (ret == 0) {
> +		key_set_timeout(key_ref_to_ptr(krb_ref), krb_cache_expiry);
> +		ret = key_ref_to_ptr(krb_ref)->serial;		
> +	}
> +	goto out_put_krb;
> +}
> diff --git a/security/keys/sysctl.c b/security/keys/sysctl.c
> index ee32d18..3af1798 100644
> --- a/security/keys/sysctl.c
> +++ b/security/keys/sysctl.c
> @@ -61,5 +61,16 @@ ctl_table key_sysctls[] = {
>  		.extra1 = (void *) &zero,
>  		.extra2 = (void *) &max,
>  	},
> +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> +	{
> +		.procname = "krb_expiry",
> +		.data = &krb_cache_expiry,
> +		.maxlen = sizeof(unsigned),
> +		.mode = 0644,
> +		.proc_handler = proc_dointvec_minmax,
> +		.extra1 = (void *) &zero,
> +		.extra2 = (void *) &max,
> +	},
> +#endif
>  	{ }
>  };
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Looks good overall, but I share Daniel's concerns about making
krb5-specific infrastructure like this. Essentially this is just a
persistent keyring that's associated with a kuid, right? Perhaps this
could be done in such a way that it could be usable for other
applications in the future?

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Simo Sorce]

On Fri, 2013-08-02 at 09:55 -0400, Jeff Layton wrote:
> On Thu, 01 Aug 2013 18:39:02 +0100
> David Howells <dhowells@redhat.com> wrote:
> 
> > Add support for per-user_namespace registers of persistent per-UID kerberos
> > caches held within the kernel.
> > 
> > This allows the kerberos cache to be retained beyond the life of all a user's
> > processes so that the user's cron jobs can work.
> > 
> > The kerberos cache is envisioned as a keyring/key tree looking something like:
> > 
> > 	struct user_namespace
> > 	  \___ .krb_cache keyring		- The register
> > 		\___ _krb.0 keyring		- Root's Kerberos cache
> > 		\___ _krb.5000 keyring		- User 5000's Kerberos cache
> > 		\___ _krb.5001 keyring		- User 5001's Kerberos cache
> > 			\___ tkt785 big_key	- A ccache blob
> > 			\___ tkt12345 big_key	- Another ccache blob
> > 
> > Or possibly:
> > 
> > 	struct user_namespace
> > 	  \___ .krb_cache keyring		- The register
> > 		\___ _krb.0 keyring		- Root's Kerberos cache
> > 		\___ _krb.5000 keyring		- User 5000's Kerberos cache
> > 		\___ _krb.5001 keyring		- User 5001's Kerberos cache
> > 			\___ tkt785 keyring	- A ccache
> > 				\___ krbtgt/REDHAT.COM@REDHAT.COM big_key
> > 				\___ http/REDHAT.COM@REDHAT.COM user
> > 				\___ afs/REDHAT.COM@REDHAT.COM user
> > 				\___ nfs/REDHAT.COM@REDHAT.COM user
> > 				\___ krbtgt/KERNEL.ORG@KERNEL.ORG big_key
> > 				\___ http/KERNEL.ORG@KERNEL.ORG big_key
> > 
> > What goes into a particular Kerberos cache is entirely up to userspace.  Kernel
> > support is limited to giving you the Kerberos cache keyring that you want.
> > 
> > The user asks for their Kerberos cache by:
> > 
> > 	krb_cache = keyctl_get_krbcache(uid, dest_keyring);
> > 
> > The uid is -1 or the user's own UID for the user's own cache or the uid of some
> > other user's cache (requires CAP_SETUID).  This permits rpc.gssd or whatever to
> > mess with the cache.
> > 
> > The cache returned is a keyring named "_krb.<uid>" that the possessor can read,
> > search, clear, invalidate, unlink from and add links to.  SELinux and co. get a
> > say as to whether this call will succeed as the caller must have LINK
> > permission on the cache keyring.
> > 
> > Each uid's cache keyring is created when it first accessed and is given a
> > timeout that is extended each time this function is called so that the keyring
> > goes away after a while.  The timeout is configurable by sysctl but defaults to
> > three days.
> > 
> > Each user_namespace struct gets a lazily-created keyring that serves as the
> > register.  The cache keyrings are added to it.  This means that standard key
> > search and garbage collection facilities are available.
> > 
> > The user_namespace struct's register goes away when it does and anything left
> > in it is then automatically gc'd.
> > 
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > cc: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> > cc: Eric W. Biederman <ebiederm@xmission.com>
> > ---
> > 
> >  include/linux/user_namespace.h |    6 ++
> >  include/uapi/linux/keyctl.h    |    1 
> >  kernel/user.c                  |    4 +
> >  kernel/user_namespace.c        |    2 +
> >  security/keys/Kconfig          |   12 ++++
> >  security/keys/Makefile         |    1 
> >  security/keys/compat.c         |    3 +
> >  security/keys/internal.h       |    9 +++
> >  security/keys/keyctl.c         |    3 +
> >  security/keys/krbcache.c       |  132 ++++++++++++++++++++++++++++++++++++++++
> >  security/keys/sysctl.c         |   11 +++
> >  11 files changed, 184 insertions(+)
> >  create mode 100644 security/keys/krbcache.c
> > 
> > diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> > index b6b215f..3cce8c7 100644
> > --- a/include/linux/user_namespace.h
> > +++ b/include/linux/user_namespace.h
> > @@ -28,6 +28,12 @@ struct user_namespace {
> >  	unsigned int		proc_inum;
> >  	bool			may_mount_sysfs;
> >  	bool			may_mount_proc;
> > +
> > +	/* Register of per-UID Kerberos caches for this namespace */
> > +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> > +	struct key		*krb_cache_register;
> > +	struct rw_semaphore	krb_cache_register_sem;
> > +#endif
> >  };
> >  
> >  extern struct user_namespace init_user_ns;
> > diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
> > index c9b7f4fa..a37c62b 100644
> > --- a/include/uapi/linux/keyctl.h
> > +++ b/include/uapi/linux/keyctl.h
> > @@ -56,5 +56,6 @@
> >  #define KEYCTL_REJECT			19	/* reject a partially constructed key */
> >  #define KEYCTL_INSTANTIATE_IOV		20	/* instantiate a partially constructed key */
> >  #define KEYCTL_INVALIDATE		21	/* invalidate a key */
> > +#define KEYCTL_GET_KRBCACHE		22	/* get a user's kerberos cache keyring */
> >  
> >  #endif /*  _LINUX_KEYCTL_H */
> > diff --git a/kernel/user.c b/kernel/user.c
> > index 69b4c3d..6c9e1b9 100644
> > --- a/kernel/user.c
> > +++ b/kernel/user.c
> > @@ -53,6 +53,10 @@ struct user_namespace init_user_ns = {
> >  	.proc_inum = PROC_USER_INIT_INO,
> >  	.may_mount_sysfs = true,
> >  	.may_mount_proc = true,
> > +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> > +	.krb_cache_register_sem =
> > +	__RWSEM_INITIALIZER(init_user_ns.krb_cache_register_sem),
> > +#endif
> >  };
> >  EXPORT_SYMBOL_GPL(init_user_ns);
> >  
> > diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> > index d8c30db..098d954 100644
> > --- a/kernel/user_namespace.c
> > +++ b/kernel/user_namespace.c
> > @@ -99,6 +99,7 @@ int create_user_ns(struct cred *new)
> >  
> >  	update_mnt_policy(ns);
> >  
> > +	rwsem_init(&ns->krb_cache_register_sem);
> >  	return 0;
> >  }
> >  
> > @@ -123,6 +124,7 @@ void free_user_ns(struct user_namespace *ns)
> >  
> >  	do {
> >  		parent = ns->parent;
> > +		key_put(ns->krb_cache_register);
> >  		proc_free_inum(ns->proc_inum);
> >  		kmem_cache_free(user_ns_cachep, ns);
> >  		ns = parent;
> > diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> > index eafb335..ee3f5a5 100644
> > --- a/security/keys/Kconfig
> > +++ b/security/keys/Kconfig
> > @@ -20,6 +20,18 @@ config KEYS
> >  
> >  	  If you are unsure as to whether this is required, answer N.
> >  
> > +config KEYS_KERBEROS_CACHE
> > +	bool "Enable persistent keyring-based kerberos cache"
> > +	depends on KEYS
> > +	help
> > +	  This option provides a register of per-UID kerberos cache keyrings.
> > +	  A particular keyring may be accessed by either the user whose keyring
> > +	  it is or by a process with administrative privileges.  SELinux gets
> > +	  to rule on which admin-level processes get to access the cache.
> > +
> > +	  Keyrings are created and added into the register upon demand and get
> > +	  removed if they expire (a default timeout is set upon creation).
> > +
> >  config BIG_KEYS
> >  	tristate "Large payload keys"
> >  	depends on KEYS
> > diff --git a/security/keys/Makefile b/security/keys/Makefile
> > index c487c77..c168ad6 100644
> > --- a/security/keys/Makefile
> > +++ b/security/keys/Makefile
> > @@ -18,6 +18,7 @@ obj-y := \
> >  obj-$(CONFIG_KEYS_COMPAT) += compat.o
> >  obj-$(CONFIG_PROC_FS) += proc.o
> >  obj-$(CONFIG_SYSCTL) += sysctl.o
> > +obj-$(CONFIG_KEYS_KERBEROS_CACHE) += krbcache.o
> >  
> >  #
> >  # Key types
> > diff --git a/security/keys/compat.c b/security/keys/compat.c
> > index d65fa7f..ead383b 100644
> > --- a/security/keys/compat.c
> > +++ b/security/keys/compat.c
> > @@ -138,6 +138,9 @@ asmlinkage long compat_sys_keyctl(u32 option,
> >  	case KEYCTL_INVALIDATE:
> >  		return keyctl_invalidate_key(arg2);
> >  
> > +	case KEYCTL_GET_KRBCACHE:
> > +		return keyctl_get_krbcache(arg2, arg3);
> > +
> >  	default:
> >  		return -EOPNOTSUPP;
> >  	}
> > diff --git a/security/keys/internal.h b/security/keys/internal.h
> > index 581c6f6..fa379c6 100644
> > --- a/security/keys/internal.h
> > +++ b/security/keys/internal.h
> > @@ -255,6 +255,15 @@ extern long keyctl_invalidate_key(key_serial_t);
> >  extern long keyctl_instantiate_key_common(key_serial_t,
> >  					  const struct iovec *,
> >  					  unsigned, size_t, key_serial_t);
> > +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> > +extern long keyctl_get_krbcache(uid_t, key_serial_t);
> > +extern unsigned krb_cache_expiry;
> > +#else
> > +static inline long keyctl_get_krbcache(uid_t uid, key_serial_t destring)
> > +{
> > +	return -EOPNOTSUPP;
> > +}
> > +#endif
> >  
> >  /*
> >   * Debugging key validation
> > diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> > index 33cfd27..c4fae05 100644
> > --- a/security/keys/keyctl.c
> > +++ b/security/keys/keyctl.c
> > @@ -1667,6 +1667,9 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
> >  	case KEYCTL_INVALIDATE:
> >  		return keyctl_invalidate_key((key_serial_t) arg2);
> >  
> > +	case KEYCTL_GET_KRBCACHE:
> > +		return keyctl_get_krbcache((uid_t)arg2, (key_serial_t)arg3);
> > +
> >  	default:
> >  		return -EOPNOTSUPP;
> >  	}
> > diff --git a/security/keys/krbcache.c b/security/keys/krbcache.c
> > new file mode 100644
> > index 0000000..4e2aa9c
> > --- /dev/null
> > +++ b/security/keys/krbcache.c
> > @@ -0,0 +1,132 @@
> > +/* Kerberos persistent cache register
> > + *
> > + * Copyright (C) 2013 Red Hat, Inc. All Rights Reserved.
> > + * Written by David Howells (dhowells@redhat.com)
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public Licence
> > + * as published by the Free Software Foundation; either version
> > + * 2 of the Licence, or (at your option) any later version.
> > + */
> > +
> > +#include <linux/user_namespace.h>
> > +#include "internal.h"
> > +
> > +unsigned krb_cache_expiry = 3 * 24 * 3600; /* Expire after 3 days of non-use */
> > +
> > +/*
> > + * Get the Kerberos cache keyring for a specific UID and link it to the
> > + * nominated keyring.
> > + */
> > +long keyctl_get_krbcache(uid_t _uid, key_serial_t destid)
> > +{
> > +	struct user_namespace *ns = current_user_ns();
> > +	struct keyring_index_key index_key;
> > +	struct key *krb;
> > +	key_ref_t reg_ref, dest_ref, krb_ref;
> > +	kuid_t uid;
> > +	char buf[24];
> > +	long ret;
> > +
> > +	/* -1 indicates the current user */
> > +	if (_uid == (uid_t)-1) {
> > +		uid = current_uid();
> 
> 
> Isn't it possible to have a valid uid of (unsigned int)-1? I know that
> at least some sites use that for "nobody". Why not just require passing
> in the correct UID?
> 
> > +	} else {
> > +		uid = make_kuid(ns, _uid);
> > +		if (!uid_valid(uid))
> > +			return -EINVAL;
> > +
> > +		/* You can only see your own kerberos cache if you're not
> > +		 * sufficiently privileged.
> > +		 */
> > +		if (uid != current_uid() &&
> > +		    uid != current_suid() &&
> > +		    uid != current_euid() &&
> > +		    uid != current_fsuid() &&
> > +		    !nsown_capable(CAP_SETUID))
> > +			return -EPERM;
> > +	}
> > +
> > +	/* There must be a destination keyring */
> > +	dest_ref = lookup_user_key(destid, KEY_LOOKUP_CREATE, KEY_WRITE);
> > +	if (IS_ERR(dest_ref))
> > +		return PTR_ERR(dest_ref);
> > +	if (key_ref_to_ptr(dest_ref)->type != &key_type_keyring) {
> > +		ret = -ENOTDIR;
> > +		goto out_put_dest;
> > +	}
> > +
> > +	/* Look in the register if it exists */
> > +	index_key.type = &key_type_keyring;
> > +	index_key.description = buf;
> > +	index_key.desc_len = sprintf(buf, "_krb.%u", __kuid_val(uid));
> > +
> > +	if (ns->krb_cache_register) {
> > +		reg_ref = make_key_ref(ns->krb_cache_register, true);
> > +		down_read(&ns->krb_cache_register_sem);
> > +		krb_ref = find_key_to_update(reg_ref, &index_key);
> > +		up_read(&ns->krb_cache_register_sem);
> > +
> > +		if (krb_ref)
> > +			goto found;
> > +	}
> > +
> > +	/* It wasn't in the register, so we'll need to create it.  We might
> > +	 * also need to create the register.
> > +	 */
> > +	down_write(&ns->krb_cache_register_sem);
> > +
> > +	if (!ns->krb_cache_register) {
> > +		struct key *reg = 
> > +			keyring_alloc(".krb_cache",
> > +				      KUIDT_INIT(0), KGIDT_INIT(0),
> > +				      current_cred(),
> > +				      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> > +				       KEY_USR_VIEW | KEY_USR_READ),
> > +				      KEY_ALLOC_NOT_IN_QUOTA, NULL);
> > +		if (IS_ERR(reg)) {
> > +			up_write(&ns->krb_cache_register_sem);
> > +			ret = PTR_ERR(reg);
> > +			goto out_put_dest;
> > +		}
> > +
> > +		ns->krb_cache_register = reg;
> > +	} else {
> > +		reg_ref = make_key_ref(ns->krb_cache_register, true);
> > +		krb_ref = find_key_to_update(reg_ref, &index_key);
> > +		if (krb_ref) {
> > +			up_write(&ns->krb_cache_register_sem);
> > +			goto found;
> > +		}
> > +	}
> > +
> > +	krb = keyring_alloc(buf,
> > +			    uid, INVALID_GID, current_cred(),
> > +			    ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> > +			     KEY_USR_VIEW | KEY_USR_READ),
> > +			    KEY_ALLOC_NOT_IN_QUOTA,
> > +			    ns->krb_cache_register);
> > +	up_write(&ns->krb_cache_register_sem);
> > +	if (!IS_ERR(krb)) {
> > +		krb_ref = make_key_ref(krb, true);
> > +		goto found;
> > +	}
> > +
> > +out_put_krb:
> > +	key_ref_to_ptr(krb_ref);
> > +out_put_dest:
> > +	key_ref_to_ptr(dest_ref);
> > +	return ret;
> > +
> > +found:
> > +	ret = key_task_permission(krb_ref, current_cred(), KEY_LINK);
> > +	if (ret < 0)
> > +		goto out_put_krb;
> > +
> > +	ret = key_link(key_ref_to_ptr(dest_ref), key_ref_to_ptr(krb_ref));
> > +	if (ret == 0) {
> > +		key_set_timeout(key_ref_to_ptr(krb_ref), krb_cache_expiry);
> > +		ret = key_ref_to_ptr(krb_ref)->serial;		
> > +	}
> > +	goto out_put_krb;
> > +}
> > diff --git a/security/keys/sysctl.c b/security/keys/sysctl.c
> > index ee32d18..3af1798 100644
> > --- a/security/keys/sysctl.c
> > +++ b/security/keys/sysctl.c
> > @@ -61,5 +61,16 @@ ctl_table key_sysctls[] = {
> >  		.extra1 = (void *) &zero,
> >  		.extra2 = (void *) &max,
> >  	},
> > +#ifdef CONFIG_KEYS_KERBEROS_CACHE
> > +	{
> > +		.procname = "krb_expiry",
> > +		.data = &krb_cache_expiry,
> > +		.maxlen = sizeof(unsigned),
> > +		.mode = 0644,
> > +		.proc_handler = proc_dointvec_minmax,
> > +		.extra1 = (void *) &zero,
> > +		.extra2 = (void *) &max,
> > +	},
> > +#endif
> >  	{ }
> >  };
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Looks good overall, but I share Daniel's concerns about making
> krb5-specific infrastructure like this. Essentially this is just a
> persistent keyring that's associated with a kuid, right? Perhaps this
> could be done in such a way that it could be usable for other
> applications in the future?

Well to be honest there is nothing kerberos specific here but the name
at this stage, however we do not want 'extraneous' keys in the kerberos
keyring. So in order to make it generic we'd have to pass in a prefix at
request time, something like:
keyctl_get_userkeycache(uid, prefix, id);

And then in the kerberos case call it like:
krbcache_id = keyctl_get_userkeycache(uid, "_krb", parent_id);

I see no problem from the kerberos end about using this, but it depends
on whether you want to allow to create arbitrary keyrings like this.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[David Howells]

Jeff Layton <jlayton@redhat.com> wrote:

> > +	/* -1 indicates the current user */
> > +	if (_uid == (uid_t)-1) {
> > +		uid = current_uid();
>
> Isn't it possible to have a valid uid of (unsigned int)-1? I know that
> at least some sites use that for "nobody". Why not just require passing
> in the correct UID?

See setresuid() and co. - there -1 is "don't change".

> Looks good overall, but I share Daniel's concerns about making
> krb5-specific infrastructure like this. Essentially this is just a
> persistent keyring that's associated with a kuid, right? Perhaps this
> could be done in such a way that it could be usable for other
> applications in the future?

It's not too hard, I suppose:

	keyctl_get_persistent(uid, prefix, destring)

eg:

	keyctl_get_persistent(-1, "_krb.", KEYCTL_SPEC_PROCESS_KEYRING)

giving:

	struct user_namespace
	  \___ .krb_cache keyring
		\___ _krb.0 keyring
		\___ _krb.5000 keyring
		\___ _krb.5001 keyring
		|	\___ tkt785 big_key
		|	\___ tkt12345 big_key
		\___ _afs.5000 keyring
			\___ afs.redhat.com rxrpc

The other way to do it is create one keyring per user and let userspace create
subkeyrings under that:

	struct user_namespace
	  \___ .krb_cache keyring
		\___ _uid_p.0 keyring
		\___ _uid_p.5000 keyring
		\___ _uid_p.5001 keyring
			\___ krb keyring
			|	\___ tkt785 big_key
			|	\___ tkt12345 big_key
			\___ afs keyring
				\___ afs.redhat.com rxrpc

In the above scheme, it might be worth just making these the same as the user
keyring - which means KEYCTL_SPEC_USER_KEYRING will automatically target it.

Simo:  I believe the problem you have with the user keyring is that it's not
persistent beyond the life of the processes of that UID, right?

David

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Simo Sorce]

On Fri, 2013-08-02 at 17:53 +0100, David Howells wrote:
> Jeff Layton <jlayton@redhat.com> wrote:
> 
> > > +	/* -1 indicates the current user */
> > > +	if (_uid == (uid_t)-1) {
> > > +		uid = current_uid();
> >
> > Isn't it possible to have a valid uid of (unsigned int)-1? I know that
> > at least some sites use that for "nobody". Why not just require passing
> > in the correct UID?
> 
> See setresuid() and co. - there -1 is "don't change".
> 
> > Looks good overall, but I share Daniel's concerns about making
> > krb5-specific infrastructure like this. Essentially this is just a
> > persistent keyring that's associated with a kuid, right? Perhaps this
> > could be done in such a way that it could be usable for other
> > applications in the future?
> 
> It's not too hard, I suppose:
> 
> 	keyctl_get_persistent(uid, prefix, destring)
> 
> eg:
> 
> 	keyctl_get_persistent(-1, "_krb.", KEYCTL_SPEC_PROCESS_KEYRING)
> 
> giving:
> 
> 	struct user_namespace
> 	  \___ .krb_cache keyring
> 		\___ _krb.0 keyring
> 		\___ _krb.5000 keyring
> 		\___ _krb.5001 keyring
> 		|	\___ tkt785 big_key
> 		|	\___ tkt12345 big_key
> 		\___ _afs.5000 keyring
> 			\___ afs.redhat.com rxrpc
> 
> The other way to do it is create one keyring per user and let userspace create
> subkeyrings under that:
> 
> 	struct user_namespace
> 	  \___ .krb_cache keyring
> 		\___ _uid_p.0 keyring
> 		\___ _uid_p.5000 keyring
> 		\___ _uid_p.5001 keyring
> 			\___ krb keyring
> 			|	\___ tkt785 big_key
> 			|	\___ tkt12345 big_key
> 			\___ afs keyring
> 				\___ afs.redhat.com rxrpc
> 
> In the above scheme, it might be worth just making these the same as the user
> keyring - which means KEYCTL_SPEC_USER_KEYRING will automatically target it.
> 
> Simo:  I believe the problem you have with the user keyring is that it's not
> persistent beyond the life of the processes of that UID, right?

Correct.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[David Howells]

Eric W. Biederman <ebiederm@xmission.com> wrote:

> > Add support for per-user_namespace registers of persistent per-UID kerberos
> > caches held within the kernel.
> 
> Out of curiosity is this cache per user namspace because the key lookup
> is per user namespace?

Yes.  You can't see keys in another namespace.  I occasionally wonder if I
should make the key serial number tree per namespace so that you don't search
keys outside your namespace and can't try looking them up by ID - but it
complicates the garbage collector which iterates over the entire tree (though
it could maintain a list of per-ns trees).

> Some minor nits below. But I don't see anything particulary scary about
> this patch.  Other than seeming to make it easy for root to get my
> kerbose tickets.

Root can do that anyway with file-based ccaches, I believe.  However, you can
change the key permissions to prevent root even seeing that your keys/keyrings
exist, let alone stealing them.

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[David Howells]

Simo Sorce <simo@redhat.com> wrote:

> > Simo:  I believe the problem you have with the user keyring is that it's not
> > persistent beyond the life of the processes of that UID, right?
> 
> Correct.

In which case, pinning the user keyring in this fashion should make this work
without the need to add another keyctl, I think.

David

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[David Howells]

Eric W. Biederman <ebiederm@xmission.com> wrote:

> > The cache returned is a keyring named "_krb.<uid>" that the possessor can
> > read, search, clear, invalidate, unlink from and add links to.  SELinux
> > and co. get a say as to whether this call will succeed as the caller must
> > have LINK permission on the cache keyring.
> 
> I think it would be more accurate to say you use the existing LSM
> security hooks for security keys.

Yes.

> Calling out SELinux in particular just seems odd as there is absolutely
> nothing SELinux specific in this patch.

Sorry, I normally think of SELinux as that's what I usually deal with.  Yes,
any and all LSMs.

> > +		    !nsown_capable(CAP_SETUID))
>
> You you make this ns_capable(ns, CAP_SETUID);
> 
> nsown_capable is the right thing here but I am trying to remove the
> function because it makes it too easy to not think about which
> user namespace you are in.

Okay.

> > +	index_key.desc_len = sprintf(buf, "_krb.%u", __kuid_val(uid));
> 
> Please don't use the implementation detail __kuid_val.  Please use
> from_kuid(&init_user_ns, uid) instead so it is explicitly documented
> which user namespace you are using.

Actually, I don't want that either.  I want the user-visible UID from the
namespace.

David

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[David Howells]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:

> > The uid is -1 or the user's own UID for the user's own cache or the uid of
> > some other user's cache (requires CAP_SETUID).  This permits rpc.gssd or
> > whatever to mess with the cache.
> 
> Is the goal here eventually to be able to avoid the upcall to rpc.gssd
> entirely?  It seems a little bit roundabout to have the kernel call up
> into userspace for the credentials, only to talk to a process which then
> calls back into the kernel for something that the kernel has already
> well-defined internally.

I would like to see use of request_key() eventually so that the kernel can
fish a key directly out of the keyring if it needs one.  My kAFS filesystem
could pretty much use that immediately.

However, I don't really want to put all the Kerberos communications into the
kernel.  The keys obtained by request_key() on behalf of a filesystem might be
kerberos tickets or they might be something else.

> It seems like a non-privileged user could use this to store arbitrary data
> in this keyring as a way of hiding what would otherwise be filesystem
> activity or using it for some sort of odd/sneaky IPC mechanism.  Is this an
> intentional side effect?

It is true that a non-privileged user can store arbitrary keys in their
keyring and they could use it as an IPC mechanism if they really wanted to -
but if they could do that, they could use socketpairs, pipes or shm instead so
I'm not sure they can do anything that they couldn't do by some other
mechanism anyway.

Note that key accesses are regulated by the active LSM and I think can appear
in the audit log.

David

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Jeff Layton]

On Fri, 02 Aug 2013 17:53:25 +0100
David Howells <dhowells@redhat.com> wrote:

> Jeff Layton <jlayton@redhat.com> wrote:
> 
> > > +	/* -1 indicates the current user */
> > > +	if (_uid == (uid_t)-1) {
> > > +		uid = current_uid();
> >
> > Isn't it possible to have a valid uid of (unsigned int)-1? I know that
> > at least some sites use that for "nobody". Why not just require passing
> > in the correct UID?
> 
> See setresuid() and co. - there -1 is "don't change".
> 

<facepalm>

Good point. I got confused between -1 and -2. I think Solaris uses
(uid_t)-2 for nobody. Using -1 in this case should be fine.

> > Looks good overall, but I share Daniel's concerns about making
> > krb5-specific infrastructure like this. Essentially this is just a
> > persistent keyring that's associated with a kuid, right? Perhaps this
> > could be done in such a way that it could be usable for other
> > applications in the future?
> 
> It's not too hard, I suppose:
> 
> 	keyctl_get_persistent(uid, prefix, destring)
> 
> eg:
> 
> 	keyctl_get_persistent(-1, "_krb.", KEYCTL_SPEC_PROCESS_KEYRING)
> 
> giving:
> 
> 	struct user_namespace
> 	  \___ .krb_cache keyring
> 		\___ _krb.0 keyring
> 		\___ _krb.5000 keyring
> 		\___ _krb.5001 keyring
> 		|	\___ tkt785 big_key
> 		|	\___ tkt12345 big_key
> 		\___ _afs.5000 keyring
> 			\___ afs.redhat.com rxrpc
> 
> The other way to do it is create one keyring per user and let userspace create
> subkeyrings under that:
> 
> 	struct user_namespace
> 	  \___ .krb_cache keyring
> 		\___ _uid_p.0 keyring
> 		\___ _uid_p.5000 keyring
> 		\___ _uid_p.5001 keyring
> 			\___ krb keyring
> 			|	\___ tkt785 big_key
> 			|	\___ tkt12345 big_key
> 			\___ afs keyring
> 				\___ afs.redhat.com rxrpc
> 


That's probably what I'd suggest. Allow one persistent keyring per
user, and expect userland to organize things sanely under it.

nit: I probably wouldn't call the top-level keyring "krb_cache"
though ;)

> In the above scheme, it might be worth just making these the same as the user
> keyring - which means KEYCTL_SPEC_USER_KEYRING will automatically target it.
> 
> Simo:  I believe the problem you have with the user keyring is that it's not
> persistent beyond the life of the processes of that UID, right?
> 

Possibly. It really comes down to what sort of lifecycle you expect here.

Some applications might be caught by surprise if the per-user keyring
was already populated in certain situations. OTOH, they have the same
problem if there's even one running process with that uid so maybe it's
not a big deal.

If you do this, it might make sense to allow the admin to tune the
expiry sysctl in such a way that user keyrings go away as soon as
the last reference is gone (maybe by setting it to 0?).

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Eric W. Biederman]

David Howells <dhowells@redhat.com> writes:

> Eric W. Biederman <ebiederm@xmission.com> wrote:
>
>> > The cache returned is a keyring named "_krb.<uid>" that the possessor can
>> > read, search, clear, invalidate, unlink from and add links to.  SELinux
>> > and co. get a say as to whether this call will succeed as the caller must
>> > have LINK permission on the cache keyring.
>> 
>> I think it would be more accurate to say you use the existing LSM
>> security hooks for security keys.
>
> Yes.
>
>> Calling out SELinux in particular just seems odd as there is absolutely
>> nothing SELinux specific in this patch.
>
> Sorry, I normally think of SELinux as that's what I usually deal with.  Yes,
> any and all LSMs.
>
>> > +		    !nsown_capable(CAP_SETUID))
>>
>> You you make this ns_capable(ns, CAP_SETUID);
>> 
>> nsown_capable is the right thing here but I am trying to remove the
>> function because it makes it too easy to not think about which
>> user namespace you are in.
>
> Okay.
>
>> > +	index_key.desc_len = sprintf(buf, "_krb.%u", __kuid_val(uid));
>> 
>> Please don't use the implementation detail __kuid_val.  Please use
>> from_kuid(&init_user_ns, uid) instead so it is explicitly documented
>> which user namespace you are using.
>
> Actually, I don't want that either.  I want the user-visible UID from the
> namespace.

Which is a definite reason to use from_kuid().  So you think about which
user namespace you want this to be seen in.

I guess if this is all in the user namespace from_kuid(ns, uid) is what
you are after.  I was thinking this was heading to the upcall which only
runs in the initial user namespace.  When passing things to the upcall
it makes sense to use values in the initial user namespace.

Eric

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[David Howells]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:

> I guess this raises the question from a different perspective: if the
> kernel already supports arbitrary shm segments, filesystem locations,
> etc, which can be used for storing/passing opaque bytestrings between
> different parts of userspace, what advantages do we gain from having
> this new specific mechanism in the kernel?  Why couldn't those parts of
> userspace just rely on already-existing mechanisms instead of
> introducing this new interface?

Kernel services can use keys directly by calling request_key() without
necessarily having to upcall.


The keys service in the kernel was created initially for use by the kAFS
filesystem in the kernel which uses it for passing authentication tokens
around.  Let me describe how I originally envisioned this working as it may
help the discussion.


Every time a non-fd-based operation[*] is called, kAFS calls request_key() to
get a key containing an authentication token for the cell.

 [*] That is things like lookup, open(), rmdir() and stat() but not read() and
     write() which use a key cached in the file struct.

Keys can be preemptively added for kAFS by calling a klog or an aklog program
talk to the authentication server and then use add_key() to add a key into the
kernel.  The filesystem subsequently finds this and uses it without the need to
upcall.

Currently, that's all you can do with kAFS.

However, on my todo list is to make it able to upcall when it doesn't have a
key available.  This is provided for by the request_key() mechanism.
request_key() searches for the key and if one if is available, it returns it
otherwise it can[*] partially construct a key and then upcall to ask userspace
to instantiate it.

 [*] If the caller permits it to.

request_key() can upcall to a program it execs itself or it can upcall to a
running program (though the exact mechanism of the latter is to be worked out).

Say userspace then decides to do a Kerberos ticket request.  It could call
request_key() itself to search the original process's keyrings for a TGT which
it can then use to request a ticket.

Once it has the ticket, it can place the ticket data into the partially
constructed key and link the key into the kerberos ccache keyring to retain it.


Now, one of the problems I've got is that the name in the partially constructed
key is from the filesystem's view of the world - and this doesn't necessarily
coincide with that of the authentication server.  For example, the kAFS
filesystem wants a key named for the cell, eg.:

	afs.redhat.com

but the Kerberos ticket might look like:

	AFS/afs.redhat.com@REDHAT.COM

Now, I do permit partial matching, so I could ignore the "@REDHAT.COM" and just
slap "AFS/" on the front of what I'm looking for.

A further problem is the fact that keys have types and search only finds the
type asked for - though that can be dealt with in the keyring search algorithm.

David

Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[Nico Williams]

On Fri, Aug 2, 2013 at 8:55 AM, Jeff Layton <jlayton@redhat.com> wrote:
> Isn't it possible to have a valid uid of (unsigned int)-1? I know that
> at least some sites use that for "nobody". Why not just require passing
> in the correct UID?

POSIX requires valid UIDs to be non-nengative.  POSIX does not require
uid_t to be signed or unsigned.  POSIX does make use of (uid_t)-1 as a
sentinel (e.g., in setreuid(2)).  (uid_t)-1 is special.  Do not use
it.

As an aside, note that on Solaris 10 and less uid_t was signed, so
UIDs 2^31..2^32-2 were unusable.  Interop with S10 and less requires
that you not use such UIDs.  Ditto gid_t and GIDs.

Also note that in Solaris 11 uid_t is unsigned BUT the range of UIDs
2^31..2^32-2 is still reserved (for automatic, on-demand,
non-persistent allocation for ID mapping purposes).  Interop with S11
requires that you not use such UIDs.  Ditto gid_t and GIDs.

Nico
--

Re: [PATCH 1/2] KEYS: Implement a big key type that can save to tmpfs

[Nico Williams]

I think this is the wrong design.

There are two problems you're trying to solve:

a) how rpc.gssd finds credentials for processes on behalf of which it's acting

b) how to create tmpfs locations in which to store credentials (which
can be unbounded in size, so storing them in the kernel is silly; also
the apps that use them are all user-land apps, so that too means
there's no point in storing them in the kernel).

Keyrings for the forst problem (or PAGs, or anything similar) is fine:
on upcall to gssd the kernel tells it what it needs to know to find
those credentials.

(b) can be solved in many ways, and the simplest is to have a
filesystem where top-level directories named after UIDs "exist" as
soon as they are referenced and as long as they are non-empty.  You
can use autofs + tmpfs, or a variant of tmpfs for this.

Solving (b) in a way that does not add a new ccache type (though
having a KEYRING: ccache type that means "find the ccache URI in my
keyring" is fine) is important because many of us run multiple
implementations of Kerberos on any given host, and we do it because:

 - vendors are always behind the curve
 - legacy software linked with old versions of libkrb5 and friends
 - third party software

Please review the above.  Thanks,

Nico
--

Re: [PATCH 1/2] KEYS: Implement a big key type that can save to tmpfs

[Nico Williams]

On Fri, Aug 2, 2013 at 3:49 PM, Nico Williams <nico@cryptonector.com> wrote:
> Solving (b) in a way that does not add a new ccache type (though
> having a KEYRING: ccache type that means "find the ccache URI in my
> keyring" is fine) is important because many of us run multiple
> implementations of Kerberos on any given host, and we do it because:
>
>  - vendors are always behind the curve
>  - legacy software linked with old versions of libkrb5 and friends
>  - third party software

I forgot to add:

 - special needs

Nico
--

Re: [PATCH 1/2] KEYS: Implement a big key type that can save to tmpfs

[David Howells]

Nico Williams <nico@cryptonector.com> wrote:

> b) how to create tmpfs locations in which to store credentials (which
> can be unbounded in size, so storing them in the kernel is silly;

Ummm...  tmpfs stores them in the kernel too - though it can page them out to
swap.

I have altered my big-key implementation to just store small items in an
internal buffer and big items in a tmpfs file.  This means that small items
will use up _less_ kernel memory if they're in a key because they won't require
the overhead of a dentry struct and an inode struct.

> (b) can be solved in many ways, and the simplest is to have a
> filesystem where top-level directories named after UIDs "exist" as
> soon as they are referenced and as long as they are non-empty.  You
> can use autofs + tmpfs, or a variant of tmpfs for this.

Don't forget to add user namespaces into the mix :-/

David

Re: [PATCH 1/2] KEYS: Implement a big key type that can save to tmpfs

[Nico Williams]

On Thu, Aug 8, 2013 at 9:46 AM, David Howells <dhowells@redhat.com> wrote:
> Nico Williams <nico@cryptonector.com> wrote:
>
>> b) how to create tmpfs locations in which to store credentials (which
>> can be unbounded in size, so storing them in the kernel is silly;
>
> Ummm...  tmpfs stores them in the kernel too - though it can page them out to
> swap.

Any filesystem uses kernel, doesn't it?  Notionally, however, they
don't.  And notionally tmpfs is not about using memory but about being
non-persistent.  This is important because tmpfs *is* subject to
swapping and that does create security problems (you should want to
encrypt swap).

There's a difference between "notionally not persistent across
reboots" and "notionally stored in the kernel's address space".  The
real question is: why do you want the latter?

Some people have argued (not in this thread, and not recently on this
list) that credentials should be stored in the kernel and not leave it
(i.e., you shouldn't be able to extract them from the kernel).  This
implies a number of things, but one of those is a bad thing: unbounded
kernel credential caches OR LRU/LFU cache eviction.  Some of us rely
on caching to keep latency predictable, so cache eviction could be a
bit rough.  Unbounded kernel credential caches not subject to paging
are a DoS.  Unbounded-but-paged cred caches would be fine, but really,
what's the difference vis-a-vis FILE ccaches in /tmp?  The answer
depends on whether you allow the creds to leave kernel space.

That brings us to: what do we really want?  If we really want users to
not be able to see their tickets' session keys, then we don't need to
store their tickets in kernel land and we don't need to put
krb5_mk_req*() in the kernel either: we need only *wrap* the keys to
that low-level crypto use of those keys happens in the kernel and the
only thing the kernel stores is the wrapping key.  You might even have
a TPM or similar crypto decelerator handle this.

ISTM that a new ccache type whose purpose is to defeat the tmpfs race
conditions of the FILE ccache is a good thing.  I would prefer it to
be *portable*, and it could be, but keyring stuff isn't.  And FILE
ccache remains the lowest common denominator, so there's that too.
And since I've been dealing with some FILE ccache race conditions (see
new thread I'll start next), I think a) I hate FILE ccaches, b) I
don't see how to avoid having them.

> I have altered my big-key implementation to just store small items in an
> internal buffer and big items in a tmpfs file.  This means that small items
> will use up _less_ kernel memory if they're in a key because they won't require
> the overhead of a dentry struct and an inode struct.

Please make sure to have a cache eviction policy...  I don't know what
is "small" when it comes to Kerberos credentials either.  If the
difference between small and large is "lacks / has PAC and/or CAMMAC"
then you'll find that this difference mostly depends on the
environment, and generally your items will be either all "small" or
all large.  Are you building dead code?

>> (b) can be solved in many ways, and the simplest is to have a
>> filesystem where top-level directories named after UIDs "exist" as
>> soon as they are referenced and as long as they are non-empty.  You
>> can use autofs + tmpfs, or a variant of tmpfs for this.
>
> Don't forget to add user namespaces into the mix :-/

Sure; why the ambivalence smiley?

Nico
--