[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130802095555.07a2cda3@tlielax.poochiereds.net>
Date: Fri, 2 Aug 2013 09:55:55 -0400
From: Jeff Layton <jlayton@...hat.com>
To: David Howells <dhowells@...hat.com>
Cc: keyrings@...ux-nfs.org, linux-nfs@...r.kernel.org, krbdev@....edu,
"Serge E. Hallyn" <serge.hallyn@...ntu.com>,
linux-kernel@...r.kernel.org, simo@...hat.com,
"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for
persistent per-UID kerberos caches
On Thu, 01 Aug 2013 18:39:02 +0100
David Howells <dhowells@...hat.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@...HAT.COM big_key
> \___ http/REDHAT.COM@...HAT.COM user
> \___ afs/REDHAT.COM@...HAT.COM user
> \___ nfs/REDHAT.COM@...HAT.COM user
> \___ krbtgt/KERNEL.ORG@...NEL.ORG big_key
> \___ http/KERNEL.ORG@...NEL.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@...hat.com>
> cc: Serge E. Hallyn <serge.hallyn@...ntu.com>
> cc: Eric W. Biederman <ebiederm@...ssion.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@...hat.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@...r.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@...hat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists