Return-Path: linux-nfs-owner@vger.kernel.org Received: from out03.mta.xmission.com ([166.70.13.233]:39151 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752461Ab3HAXJ1 (ORCPT ); Thu, 1 Aug 2013 19:09:27 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: David Howells Cc: keyrings@linux-nfs.org, linux-nfs@vger.kernel.org, krbdev@mit.edu, "Serge E. Hallyn" , linux-kernel@vger.kernel.org, simo@redhat.com References: <20130801173846.28023.19009.stgit@warthog.procyon.org.uk> <20130801173902.28023.68819.stgit@warthog.procyon.org.uk> Date: Thu, 01 Aug 2013 16:09:10 -0700 In-Reply-To: <20130801173902.28023.68819.stgit@warthog.procyon.org.uk> (David Howells's message of "Thu, 01 Aug 2013 18:39:02 +0100") Message-ID: <87ob9hovop.fsf@xmission.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches Sender: linux-nfs-owner@vger.kernel.org List-ID: David Howells 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." 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 > cc: Serge E. Hallyn > cc: Eric W. Biederman > --- > > 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 > +#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 > { } > };