Skip to contentSkip to conversation

Keys: Pass session keyring to call_usermodehelper()

PATCH In review David Howells 6 participants 19 messages Mar 23, 2005 – Apr 12, 2005

Subsystems (15)
lore
Share & tools
lore

David Howells

author maintainer
#0 lore raw

PATCH 1/3 Keys: Pass session keyring to call_usermodehelper()

The attached patch makes it possible to pass a session keyring through to the process spawned by call_usermodehelper(). This allows patch 3/3 to pass an authorisation key through to /sbin/request-key, thus permitting better access controls when doing just-in-time key creation.

---
warthog>diffstat -p1 keys-umhelper-2612rc1mm1.diff

 arch/i386/mach-voyager/voyager_thread.c |    2 +-
 drivers/acpi/thermal.c                  |    2 +-
 drivers/input/input.c                   |    2 +-
 drivers/macintosh/therm_pm72.c          |    2 +-
 drivers/net/hamradio/baycom_epp.c       |    2 +-
 drivers/pnp/pnpbios/core.c              |    2 +-
 drivers/s390/crypto/z90main.c           |    2 +-
 include/linux/key.h                     |    8 ++++++++
 include/linux/kmod.h                    |    5 ++++-
 kernel/cpuset.c                         |    2 +-
 kernel/kmod.c                           |   15 ++++++++++++---
 lib/kobject_uevent.c                    |    2 +-
 security/keys/request_key.c             |    2 +-
 13 files changed, 34 insertions(+), 14 deletions(-)
arch/i386/mach-voyager/voyager_thread.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/arch/i386/mach-voyager/voyager_thread.c linux-2.6.12-rc1-mm1-keys-umhelper/arch/i386/mach-voyager/voyager_thread.c
@@ -73,7 +73,7 @@ execute(const char *string)
 		NULL,
 	};
 
-	if ((ret = call_usermodehelper(argv[0], argv, envp, 1)) != 0) {
+	if ((ret = call_usermodehelper(argv[0], argv, envp, NULL, 1)) != 0) {
 		printk(KERN_ERR "Voyager failed to run \"%s\": %i\n",
 		       string, ret);
 	}
drivers/acpi/thermal.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/drivers/acpi/thermal.c linux-2.6.12-rc1-mm1-keys-umhelper/drivers/acpi/thermal.c
@@ -441,7 +441,7 @@ acpi_thermal_call_usermode (
 	envp[0] = "HOME=/";
 	envp[1] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
 	
-	call_usermodehelper(argv[0], argv, envp, 0);
+	call_usermodehelper(argv[0], argv, envp, NULL, 0);
 
 	return_VALUE(0);
 }
drivers/input/input.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/drivers/input/input.c linux-2.6.12-rc1-mm1-keys-umhelper/drivers/input/input.c
@@ -415,7 +415,7 @@ static void input_call_hotplug(char *ver
 		argv[0], argv[1], envp[0], envp[1], envp[2], envp[3], envp[4]);
 #endif
 
-	value = call_usermodehelper(argv [0], argv, envp, 0);
+	value = call_usermodehelper(argv [0], argv, envp, NULL, 0);
 
 	kfree(buf);
 	kfree(envp);
drivers/macintosh/therm_pm72.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/drivers/macintosh/therm_pm72.c linux-2.6.12-rc1-mm1-keys-umhelper/drivers/macintosh/therm_pm72.c
@@ -1598,7 +1598,7 @@ static int call_critical_overtemp(void)
 				"PATH=/sbin:/usr/sbin:/bin:/usr/bin",
 				NULL };
 
-	return call_usermodehelper(critical_overtemp_path, argv, envp, 0);
+	return call_usermodehelper(critical_overtemp_path, argv, envp, NULL, 0);
 }
 
 
drivers/net/hamradio/baycom_epp.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/drivers/net/hamradio/baycom_epp.c linux-2.6.12-rc1-mm1-keys-umhelper/drivers/net/hamradio/baycom_epp.c
@@ -323,7 +323,7 @@ static int eppconfig(struct baycom_state
 	sprintf(portarg, "%ld", bc->pdev->port->base);
 	printk(KERN_DEBUG "%s: %s -s -p %s -m %s\n", bc_drvname, eppconfig_path, portarg, modearg);
 
-	return call_usermodehelper(eppconfig_path, argv, envp, 1);
+	return call_usermodehelper(eppconfig_path, argv, envp, NULL, 1);
 }
 
 /* ---------------------------------------------------------------------- */
drivers/pnp/pnpbios/core.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/drivers/pnp/pnpbios/core.c linux-2.6.12-rc1-mm1-keys-umhelper/drivers/pnp/pnpbios/core.c
@@ -157,7 +157,7 @@ static int pnp_dock_event(int dock, stru
 		info->location_id, info->serial, info->capabilities);
 	envp[i] = NULL;
 	
-	value = call_usermodehelper (argv [0], argv, envp, 0);
+	value = call_usermodehelper (argv [0], argv, envp, NULL, 0);
 	kfree (buf);
 	kfree (envp);
 	return 0;
drivers/s390/crypto/z90main.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/drivers/s390/crypto/z90main.c linux-2.6.12-rc1-mm1-keys-umhelper/drivers/s390/crypto/z90main.c
@@ -3554,7 +3554,7 @@ z90crypt_hotplug_event(int dev_major, in
 	envp[4] = minor;
 	envp[5] = 0;
 
-	call_usermodehelper(argv[0], argv, envp, 0);
+	call_usermodehelper(argv[0], argv, envp, NULL, 0);
 #endif
 }
 #endif
include/linux/key.h +8 -0
diff -uNrp linux-2.6.12-rc1-mm1/include/linux/key.h linux-2.6.12-rc1-mm1-keys-umhelper/include/linux/key.h
@@ -273,6 +273,13 @@ extern void key_fsuid_changed(struct tas
 extern void key_fsgid_changed(struct task_struct *tsk);
 extern void key_init(void);
 
+#define __install_session_keyring(tsk, keyring)			\
+({								\
+	struct key *old_session = tsk->signal->session_keyring;	\
+	tsk->signal->session_keyring = keyring;			\
+	old_session;						\
+})
+
 #else /* CONFIG_KEYS */
 
 #define key_validate(k)			0
@@ -281,6 +288,7 @@ extern void key_init(void);
 #define key_put(k)			do { } while(0)
 #define alloc_uid_keyring(u)		0
 #define switch_uid_keyring(u)		do { } while(0)
+#define __install_session_keyring(t, k)	NULL
 #define copy_keys(f,t)			0
 #define copy_thread_group_keys(t)	0
 #define exit_keys(t)			do { } while(0)
include/linux/kmod.h +4 -1
diff -uNrp linux-2.6.12-rc1-mm1/include/linux/kmod.h linux-2.6.12-rc1-mm1-keys-umhelper/include/linux/kmod.h
@@ -34,7 +34,10 @@ static inline int request_module(const c
 #endif
 
 #define try_then_request_module(x, mod...) ((x) ?: (request_module(mod), (x)))
-extern int call_usermodehelper(char *path, char *argv[], char *envp[], int wait);
+
+struct key;
+extern int call_usermodehelper(char *path, char *argv[], char *envp[],
+			       struct key *session_keyring, int wait);
 extern void usermodehelper_init(void);
 
 #endif /* __LINUX_KMOD_H__ */
kernel/cpuset.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/kernel/cpuset.c linux-2.6.12-rc1-mm1-keys-umhelper/kernel/cpuset.c
@@ -428,7 +428,7 @@ static int cpuset_release_agent(char *cp
 	envp[i++] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
 	envp[i] = NULL;
 
-	return call_usermodehelper(argv[0], argv, envp, 0);
+	return call_usermodehelper(argv[0], argv, envp, NULL, 0);
 }
 
 /*
kernel/kmod.c +12 -3
diff -uNrp linux-2.6.12-rc1-mm1/kernel/kmod.c linux-2.6.12-rc1-mm1-keys-umhelper/kernel/kmod.c
@@ -108,7 +108,7 @@ int request_module(const char *fmt, ...)
 		return -ENOMEM;
 	}
 
-	ret = call_usermodehelper(modprobe_path, argv, envp, 1);
+	ret = call_usermodehelper(modprobe_path, argv, envp, NULL, 1);
 	atomic_dec(&kmod_concurrent);
 	return ret;
 }
@@ -120,6 +120,7 @@ struct subprocess_info {
 	char *path;
 	char **argv;
 	char **envp;
+	struct key *ring;
 	int wait;
 	int retval;
 };
@@ -130,16 +131,21 @@ struct subprocess_info {
 static int ____call_usermodehelper(void *data)
 {
 	struct subprocess_info *sub_info = data;
+	struct key *old_session;
 	int retval;
 
-	/* Unblock all signals. */
+	/* Unblock all signals and set the session keyring. */
+	key_get(sub_info->ring);
 	flush_signals(current);
 	spin_lock_irq(&current->sighand->siglock);
+	old_session = __install_session_keyring(current, sub_info->ring);
 	flush_signal_handlers(current, 1);
 	sigemptyset(&current->blocked);
 	recalc_sigpending();
 	spin_unlock_irq(&current->sighand->siglock);
 
+	key_put(old_session);
+
 	/* We can run anywhere, unlike our parent keventd(). */
 	set_cpus_allowed(current, CPU_MASK_ALL);
 
@@ -215,6 +221,7 @@ static void __call_usermodehelper(void *
  * @path: pathname for the application
  * @argv: null-terminated argument list
  * @envp: null-terminated environment list
+ * @session_keyring: session keyring for process (NULL for an empty keyring)
  * @wait: wait for the application to finish and return status.
  *
  * Runs a user-space application.  The application is started
@@ -224,7 +231,8 @@ static void __call_usermodehelper(void *
  * Must be called from process context.  Returns a negative error code
  * if program was not execed successfully, or 0.
  */
-int call_usermodehelper(char *path, char **argv, char **envp, int wait)
+int call_usermodehelper(char *path, char **argv, char **envp,
+			struct key *session_keyring, int wait)
 {
 	DECLARE_COMPLETION(done);
 	struct subprocess_info sub_info = {
@@ -232,6 +240,7 @@ int call_usermodehelper(char *path, char
 		.path		= path,
 		.argv		= argv,
 		.envp		= envp,
+		.ring		= session_keyring,
 		.wait		= wait,
 		.retval		= 0,
 	};
lib/kobject_uevent.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/lib/kobject_uevent.c linux-2.6.12-rc1-mm1-keys-umhelper/lib/kobject_uevent.c
@@ -370,7 +370,7 @@ void kobject_hotplug(struct kobject *kob
 	if (!hotplug_path[0])
 		goto exit;
 
-	retval = call_usermodehelper (argv[0], argv, envp, 0);
+	retval = call_usermodehelper (argv[0], argv, envp, NULL, 0);
 	if (retval)
 		pr_debug ("%s - call_usermodehelper returned %d\n",
 			  __FUNCTION__, retval);
security/keys/request_key.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/security/keys/request_key.c linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/request_key.c
@@ -88,7 +88,7 @@ static int call_request_key(struct key *
 	argv[i] = NULL;
 
 	/* do it */
-	return call_usermodehelper(argv[0], argv, envp, 1);
+	return call_usermodehelper(argv[0], argv, envp, NULL, 1);
 
 } /* end call_request_key() */
 
author maintainer
#1 lore raw

Re: [PATCH 2/3] Keys: Use RCU to manage session keyring pointer

The attached patch uses RCU to manage the session keyring pointer in struct signal_struct. This means that searching need not disable interrupts and get a the sighand spinlock to access this pointer. Furthermore, by judicious use of rcu_read_(un)lock(), this patch also avoids the need to take and put refcounts on the session keyring itself, thus saving on even more atomic ops.

---
warthog>diffstat -p1 keys-rcu-session-2612rc1mm1.diff

 security/keys/process_keys.c |   42 +++++++++++++++++++++---------------------
 security/keys/request_key.c  |    7 +++----
 2 files changed, 24 insertions(+), 25 deletions(-)
security/keys/process_keys.c +21 -21
diff -uNrp linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/process_keys.c linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/process_keys.c
@@ -1,6 +1,6 @@
 /* process_keys.c: management of a process's keyrings
  *
- * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells ([email protected])
  *
  * This program is free software; you can redistribute it and/or
@@ -181,7 +181,7 @@ static int install_process_keyring(struc
 			goto error;
 		}
 
-		/* attach or swap keyrings */
+		/* attach keyring */
 		spin_lock_irqsave(&tsk->sighand->siglock, flags);
 		if (!tsk->signal->process_keyring) {
 			tsk->signal->process_keyring = keyring;
@@ -227,12 +227,14 @@ static int install_session_keyring(struc
 
 	/* install the keyring */
 	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-	old = tsk->signal->session_keyring;
-	tsk->signal->session_keyring = keyring;
+	old = rcu_dereference(tsk->signal->session_keyring);
+	rcu_assign_pointer(tsk->signal->session_keyring, keyring);
 	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
 
 	ret = 0;
 
+	/* we're using RCU on the pointer */
+	synchronize_kernel();
 	key_put(old);
  error:
 	return ret;
@@ -245,8 +247,6 @@ static int install_session_keyring(struc
  */
 int copy_thread_group_keys(struct task_struct *tsk)
 {
-	unsigned long flags;
-
 	key_check(current->thread_group->session_keyring);
 	key_check(current->thread_group->process_keyring);
 
@@ -254,10 +254,10 @@ int copy_thread_group_keys(struct task_s
 	tsk->signal->process_keyring = NULL;
 
 	/* same session keyring */
-	spin_lock_irqsave(&current->sighand->siglock, flags);
+	rcu_read_lock();
 	tsk->signal->session_keyring =
-		key_get(current->signal->session_keyring);
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
+		key_get(rcu_dereference(current->signal->session_keyring));
+	rcu_read_unlock();
 
 	return 0;
 
@@ -381,8 +381,7 @@ struct key *search_process_keyrings_aux(
 					key_match_func_t match)
 {
 	struct task_struct *tsk = current;
-	unsigned long flags;
-	struct key *key, *ret, *err, *tmp;
+	struct key *key, *ret, *err;
 
 	/* we want to return -EAGAIN or -ENOKEY if any of the keyrings were
 	 * searchable, but we failed to find a key or we found a negative key;
@@ -436,17 +435,18 @@ struct key *search_process_keyrings_aux(
 	}
 
 	/* search the session keyring last */
-	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-
-	tmp = tsk->signal->session_keyring;
-	if (!tmp)
-		tmp = tsk->user->session_keyring;
-	atomic_inc(&tmp->usage);
-
-	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+	if (tsk->signal->session_keyring) {
+		rcu_read_lock();
+		key = keyring_search_aux(
+			rcu_dereference(tsk->signal->session_keyring),
+			type, description, match);
+		rcu_read_unlock();
+	}
+	else {
+		key = keyring_search_aux(tsk->user->session_keyring,
+					 type, description, match);
+	}
 
-	key = keyring_search_aux(tmp, type, description, match);
-	key_put(tmp);
 	if (!IS_ERR(key))
 		goto found;
 
security/keys/request_key.c +3 -4
diff -uNrp linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/request_key.c linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/request_key.c
@@ -175,13 +175,12 @@ static struct key *__request_key_constru
 	key->expiry = now.tv_sec + key_negative_timeout;
 
 	if (current->signal->session_keyring) {
-		unsigned long flags;
 		struct key *keyring;
 
-		spin_lock_irqsave(&current->sighand->siglock, flags);
-		keyring = current->signal->session_keyring;
+		rcu_read_lock();
+		keyring = rcu_dereference(current->signal->session_keyring);
 		atomic_inc(&keyring->usage);
-		spin_unlock_irqrestore(&current->sighand->siglock, flags);
+		rcu_read_unlock();
 
 		key_link(keyring, key);
 		key_put(keyring);
#2 lore raw

Re: [PATCH 2/3] Keys: Use RCU to manage session keyring pointer

Quoted 3 lines · David Howells

The attached patch uses RCU to manage the session keyring pointer in struct
signal_struct.

So are these patches dependent upon the keys-use-rcu-to-manage-session-keyring-pointer work?

author maintainer
#3 lore raw

Re: [PATCH 2/3] Keys: Use RCU to manage session keyring pointer

Quoted 7 lines · Andrew Morton
Quoted 3 lines · David Howells

The attached patch uses RCU to manage the session keyring pointer in struct
signal_struct.

So are these patches dependent upon the keys-use-rcu-to-manage-session-keyring-pointer work?

This patch (2/3) shouldn't be dependent on that. It does extra RCU-age on a different pointer entirely. The other RCU-age patch deals with specific key types.

However, the third patch adds extra stuff into keyring support, and so is RCU'd keyrings dependent.

David

author maintainer
#4 lore raw

PATCH 1/3 Keys: Pass session keyring to call_usermodehelper() [try #2]

In reply to David Howells

The attached patch makes it possible to pass a session keyring through to the process spawned by call_usermodehelper(). This allows patch 3/3 to pass an authorisation key through to /sbin/request-key, thus permitting better access controls when doing just-in-time key creation.

---
warthog>diffstat -p1 keys-umhelper-2612rc1mm1-2.diff

 include/linux/key.h         |   10 +++++++++-
 include/linux/kmod.h        |   12 +++++++++++-
 kernel/kmod.c               |   15 ++++++++++++---
 security/keys/request_key.c |    2 +-
 4 files changed, 33 insertions(+), 6 deletions(-)
include/linux/key.h +9 -1
diff -uNrp linux-2.6.12-rc1-mm1/include/linux/key.h linux-2.6.12-rc1-mm1-keys-umhelper/include/linux/key.h
@@ -273,14 +273,22 @@ extern void key_fsuid_changed(struct tas
 extern void key_fsgid_changed(struct task_struct *tsk);
 extern void key_init(void);
 
+#define __install_session_keyring(tsk, keyring)			\
+({								\
+	struct key *old_session = tsk->signal->session_keyring;	\
+	tsk->signal->session_keyring = keyring;			\
+	old_session;						\
+})
+
 #else /* CONFIG_KEYS */
 
 #define key_validate(k)			0
 #define key_serial(k)			0
-#define key_get(k) 			NULL
+#define key_get(k) 			({ NULL; })
 #define key_put(k)			do { } while(0)
 #define alloc_uid_keyring(u)		0
 #define switch_uid_keyring(u)		do { } while(0)
+#define __install_session_keyring(t, k)	({ NULL; })
 #define copy_keys(f,t)			0
 #define copy_thread_group_keys(t)	0
 #define exit_keys(t)			do { } while(0)
include/linux/kmod.h +11 -1
diff -uNrp linux-2.6.12-rc1-mm1/include/linux/kmod.h linux-2.6.12-rc1-mm1-keys-umhelper/include/linux/kmod.h
@@ -34,7 +34,17 @@ static inline int request_module(const c
 #endif
 
 #define try_then_request_module(x, mod...) ((x) ?: (request_module(mod), (x)))
-extern int call_usermodehelper(char *path, char *argv[], char *envp[], int wait);
+
+struct key;
+extern int call_usermodehelper_keys(char *path, char *argv[], char *envp[],
+				    struct key *session_keyring, int wait);
+
+static inline int
+call_usermodehelper(char *path, char **argv, char **envp, int wait)
+{
+	return call_usermodehelper_keys(path, argv, envp, NULL, wait);
+}
+
 extern void usermodehelper_init(void);
 
 #endif /* __LINUX_KMOD_H__ */
kernel/kmod.c +12 -3
diff -uNrp linux-2.6.12-rc1-mm1/kernel/kmod.c linux-2.6.12-rc1-mm1-keys-umhelper/kernel/kmod.c
@@ -120,6 +120,7 @@ struct subprocess_info {
 	char *path;
 	char **argv;
 	char **envp;
+	struct key *ring;
 	int wait;
 	int retval;
 };
@@ -130,16 +131,21 @@ struct subprocess_info {
 static int ____call_usermodehelper(void *data)
 {
 	struct subprocess_info *sub_info = data;
+	struct key *old_session;
 	int retval;
 
-	/* Unblock all signals. */
+	/* Unblock all signals and set the session keyring. */
+	key_get(sub_info->ring);
 	flush_signals(current);
 	spin_lock_irq(&current->sighand->siglock);
+	old_session = __install_session_keyring(current, sub_info->ring);
 	flush_signal_handlers(current, 1);
 	sigemptyset(&current->blocked);
 	recalc_sigpending();
 	spin_unlock_irq(&current->sighand->siglock);
 
+	key_put(old_session);
+
 	/* We can run anywhere, unlike our parent keventd(). */
 	set_cpus_allowed(current, CPU_MASK_ALL);
 
@@ -211,10 +217,11 @@ static void __call_usermodehelper(void *
 }
 
 /**
- * call_usermodehelper - start a usermode application
+ * call_usermodehelper_keys - start a usermode application
  * @path: pathname for the application
  * @argv: null-terminated argument list
  * @envp: null-terminated environment list
+ * @session_keyring: session keyring for process (NULL for an empty keyring)
  * @wait: wait for the application to finish and return status.
  *
  * Runs a user-space application.  The application is started
@@ -224,7 +231,8 @@ static void __call_usermodehelper(void *
  * Must be called from process context.  Returns a negative error code
  * if program was not execed successfully, or 0.
  */
-int call_usermodehelper(char *path, char **argv, char **envp, int wait)
+int call_usermodehelper_keys(char *path, char **argv, char **envp,
+			     struct key *session_keyring, int wait)
 {
 	DECLARE_COMPLETION(done);
 	struct subprocess_info sub_info = {
@@ -232,6 +240,7 @@ int call_usermodehelper(char *path, char
 		.path		= path,
 		.argv		= argv,
 		.envp		= envp,
+		.ring		= session_keyring,
 		.wait		= wait,
 		.retval		= 0,
 	};
security/keys/request_key.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1/security/keys/request_key.c linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/request_key.c
@@ -88,7 +88,7 @@ static int call_request_key(struct key *
 	argv[i] = NULL;
 
 	/* do it */
-	return call_usermodehelper(argv[0], argv, envp, 1);
+	return call_usermodehelper_keys(argv[0], argv, envp, NULL, 1);
 
 } /* end call_request_key() */
 
author maintainer
#5 lore raw

PATCH 2/3 Keys: Use RCU to manage session keyring pointer

In reply to David Howells

The attached patch uses RCU to manage the session keyring pointer in struct signal_struct. This means that searching need not disable interrupts and get a the sighand spinlock to access this pointer. Furthermore, by judicious use of rcu_read_(un)lock(), this patch also avoids the need to take and put refcounts on the session keyring itself, thus saving on even more atomic ops.

---
warthog>diffstat -p1 keys-rcu-session-2612rc1mm1.diff

 security/keys/process_keys.c |   42 +++++++++++++++++++++---------------------
 security/keys/request_key.c  |    7 +++----
 2 files changed, 24 insertions(+), 25 deletions(-)
security/keys/process_keys.c +21 -21
diff -uNrp linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/process_keys.c linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/process_keys.c
@@ -1,6 +1,6 @@
 /* process_keys.c: management of a process's keyrings
  *
- * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells ([email protected])
  *
  * This program is free software; you can redistribute it and/or
@@ -181,7 +181,7 @@ static int install_process_keyring(struc
 			goto error;
 		}
 
-		/* attach or swap keyrings */
+		/* attach keyring */
 		spin_lock_irqsave(&tsk->sighand->siglock, flags);
 		if (!tsk->signal->process_keyring) {
 			tsk->signal->process_keyring = keyring;
@@ -227,12 +227,14 @@ static int install_session_keyring(struc
 
 	/* install the keyring */
 	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-	old = tsk->signal->session_keyring;
-	tsk->signal->session_keyring = keyring;
+	old = rcu_dereference(tsk->signal->session_keyring);
+	rcu_assign_pointer(tsk->signal->session_keyring, keyring);
 	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
 
 	ret = 0;
 
+	/* we're using RCU on the pointer */
+	synchronize_kernel();
 	key_put(old);
  error:
 	return ret;
@@ -245,8 +247,6 @@ static int install_session_keyring(struc
  */
 int copy_thread_group_keys(struct task_struct *tsk)
 {
-	unsigned long flags;
-
 	key_check(current->thread_group->session_keyring);
 	key_check(current->thread_group->process_keyring);
 
@@ -254,10 +254,10 @@ int copy_thread_group_keys(struct task_s
 	tsk->signal->process_keyring = NULL;
 
 	/* same session keyring */
-	spin_lock_irqsave(&current->sighand->siglock, flags);
+	rcu_read_lock();
 	tsk->signal->session_keyring =
-		key_get(current->signal->session_keyring);
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
+		key_get(rcu_dereference(current->signal->session_keyring));
+	rcu_read_unlock();
 
 	return 0;
 
@@ -381,8 +381,7 @@ struct key *search_process_keyrings_aux(
 					key_match_func_t match)
 {
 	struct task_struct *tsk = current;
-	unsigned long flags;
-	struct key *key, *ret, *err, *tmp;
+	struct key *key, *ret, *err;
 
 	/* we want to return -EAGAIN or -ENOKEY if any of the keyrings were
 	 * searchable, but we failed to find a key or we found a negative key;
@@ -436,17 +435,18 @@ struct key *search_process_keyrings_aux(
 	}
 
 	/* search the session keyring last */
-	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-
-	tmp = tsk->signal->session_keyring;
-	if (!tmp)
-		tmp = tsk->user->session_keyring;
-	atomic_inc(&tmp->usage);
-
-	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+	if (tsk->signal->session_keyring) {
+		rcu_read_lock();
+		key = keyring_search_aux(
+			rcu_dereference(tsk->signal->session_keyring),
+			type, description, match);
+		rcu_read_unlock();
+	}
+	else {
+		key = keyring_search_aux(tsk->user->session_keyring,
+					 type, description, match);
+	}
 
-	key = keyring_search_aux(tmp, type, description, match);
-	key_put(tmp);
 	if (!IS_ERR(key))
 		goto found;
 
security/keys/request_key.c +3 -4
diff -uNrp linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/request_key.c linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/request_key.c
@@ -175,13 +175,12 @@ static struct key *__request_key_constru
 	key->expiry = now.tv_sec + key_negative_timeout;
 
 	if (current->signal->session_keyring) {
-		unsigned long flags;
 		struct key *keyring;
 
-		spin_lock_irqsave(&current->sighand->siglock, flags);
-		keyring = current->signal->session_keyring;
+		rcu_read_lock();
+		keyring = rcu_dereference(current->signal->session_keyring);
 		atomic_inc(&keyring->usage);
-		spin_unlock_irqrestore(&current->sighand->siglock, flags);
+		rcu_read_unlock();
 
 		key_link(keyring, key);
 		key_put(keyring);
#6 lore raw

Re: [PATCH 2/3] Keys: Use RCU to manage session keyring pointer

Quoted 8 lines · David Howells

The attached patch uses RCU to manage the session keyring pointer in struct signal_struct. This means that searching need not disable interrupts and get a the sighand spinlock to access this pointer. Furthermore, by judicious use of rcu_read_(un)lock(), this patch also avoids the need to take and put refcounts on the session keyring itself, thus saving on even more atomic ops.

This looks quite good! A few questions interspersed below.

Thanx, Paul

PS. Sorry to be so slow in getting to this one!

Quoted 27 lines

---
warthog>diffstat -p1 keys-rcu-session-2612rc1mm1.diff

 security/keys/process_keys.c |   42 +++++++++++++++++++++---------------------
 security/keys/request_key.c  |    7 +++----
 2 files changed, 24 insertions(+), 25 deletions(-)

diff -uNrp linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/process_keys.c linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/process_keys.c

security/keys/process_keys.c +2 -2
@@ -1,6 +1,6 @@
 /* process_keys.c: management of a process's keyrings
  *
- * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells ([email protected])
  *
  * This program is free software; you can redistribute it and/or
@@ -181,7 +181,7 @@ static int install_process_keyring(struc
 			goto error;
 		}
 
-		/* attach or swap keyrings */
+		/* attach keyring */
 		spin_lock_irqsave(&tsk->sighand->siglock, flags);
 		if (!tsk->signal->process_keyring) {
 			tsk->signal->process_keyring = keyring;

security/keys/process_keys.c static int install_session_keyring(struc · line 230

Quoted 6 lines
security/keys/process_keys.c +1 -2
@@ -227,12 +227,14 @@ static int install_session_keyring(struc
 
 	/* install the keyring */
 	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-	old = tsk->signal->session_keyring;
-	tsk->signal->session_keyring = keyring;
+	old = rcu_dereference(tsk->signal->session_keyring);

I don't understand why rcu_dereference() is needed in this case. Since we are holding the lock, it should not be possible for this to change, right? Or am I missing something? (Quite possible, am not all that familiar with this code.)

security/keys/process_keys.c static int install_session_keyring(struc · line 237

Quoted 7 lines
security/keys/process_keys.c +3 -0
+	rcu_assign_pointer(tsk->signal->session_keyring, keyring);
 	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
 
 	ret = 0;
 
+	/* we're using RCU on the pointer */
+	synchronize_kernel();

This would want to become synchronize_rcu().

Quoted 48 lines
security/keys/process_keys.c +4 -15
 	key_put(old);
  error:
 	return ret;
@@ -245,8 +247,6 @@ static int install_session_keyring(struc
  */
 int copy_thread_group_keys(struct task_struct *tsk)
 {
-	unsigned long flags;
-
 	key_check(current->thread_group->session_keyring);
 	key_check(current->thread_group->process_keyring);
 
@@ -254,10 +254,10 @@ int copy_thread_group_keys(struct task_s
 	tsk->signal->process_keyring = NULL;
 
 	/* same session keyring */
-	spin_lock_irqsave(&current->sighand->siglock, flags);
+	rcu_read_lock();
 	tsk->signal->session_keyring =
-		key_get(current->signal->session_keyring);
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
+		key_get(rcu_dereference(current->signal->session_keyring));
+	rcu_read_unlock();
 
 	return 0;
 
@@ -381,8 +381,7 @@ struct key *search_process_keyrings_aux(
 					key_match_func_t match)
 {
 	struct task_struct *tsk = current;
-	unsigned long flags;
-	struct key *key, *ret, *err, *tmp;
+	struct key *key, *ret, *err;
 
 	/* we want to return -EAGAIN or -ENOKEY if any of the keyrings were
 	 * searchable, but we failed to find a key or we found a negative key;
@@ -436,17 +435,18 @@ struct key *search_process_keyrings_aux(
 	}
 
 	/* search the session keyring last */
-	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-
-	tmp = tsk->signal->session_keyring;
-	if (!tmp)
-		tmp = tsk->user->session_keyring;
-	atomic_inc(&tmp->usage);
-
-	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);

security/keys/process_keys.c struct key *search_process_keyrings_aux( · line 447

Quoted 10 lines
security/keys/process_keys.c +10 -0
+	if (tsk->signal->session_keyring) {
+		rcu_read_lock();
+		key = keyring_search_aux(
+			rcu_dereference(tsk->signal->session_keyring),
+			type, description, match);
+		rcu_read_unlock();
+	}
+	else {
+		key = keyring_search_aux(tsk->user->session_keyring,
+					 type, description, match);

This one is constant, right? If not, I don't understand the locking design.

Quoted 34 lines
security/keys/process_keys.c +1 -2
+	}
 
-	key = keyring_search_aux(tmp, type, description, match);
-	key_put(tmp);
 	if (!IS_ERR(key))
 		goto found;
 

diff -uNrp linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/request_key.c linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/request_key.c

security/keys/request_key.c +3 -5
@@ -175,13 +175,12 @@ static struct key *__request_key_constru
 	key->expiry = now.tv_sec + key_negative_timeout;
 
 	if (current->signal->session_keyring) {
-		unsigned long flags;
 		struct key *keyring;
 
-		spin_lock_irqsave(&current->sighand->siglock, flags);
-		keyring = current->signal->session_keyring;
+		rcu_read_lock();
+		keyring = rcu_dereference(current->signal->session_keyring);
 		atomic_inc(&keyring->usage);
-		spin_unlock_irqrestore(&current->sighand->siglock, flags);
+		rcu_read_unlock();
 
 		key_link(keyring, key);
 		key_put(keyring);
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

author maintainer
#7 lore raw

Re: [PATCH 2/3] Keys: Use RCU to manage session keyring pointer

Quoted 9 lines · Paul E. McKenney

security/keys/process_keys.c static int install_session_keyring(struc · line 230

Quoted 4 lines
security/keys/process_keys.c +1 -2
 	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-	old = tsk->signal->session_keyring;
-	tsk->signal->session_keyring = keyring;
+	old = rcu_dereference(tsk->signal->session_keyring);

I don't understand why rcu_dereference() is needed in this case. Since we are holding the lock, it should not be possible for this to change, right? Or am I missing something? (Quite possible, am not all that familiar with this code.)

Erm... you're right. I stuck the rcu_dereference() in then added the locks back in when I realised I still needed them.

Quoted 3 lines

security/keys/process_keys.c static int install_session_keyring(struc · line 237

Quoted 1 line
security/keys/process_keys.c +1 -0
+	synchronize_kernel();

This would want to become synchronize_rcu().

I think the deprecation happened since I wrote my patch.

Quoted 4 lines
Quoted 4 lines
security/keys/process_keys.c +4 -0
+	if (tsk->signal->session_keyring) {
+		rcu_read_lock();
+		key = keyring_search_aux(
+			rcu_dereference(tsk->signal->session_keyring),
Quoted 8 lines

security/keys/process_keys.c struct key *search_process_keyrings_aux( · line 447

Quoted 6 lines
security/keys/process_keys.c +6 -0
+			type, description, match);
+		rcu_read_unlock();
+	}
+	else {
+		key = keyring_search_aux(tsk->user->session_keyring,
+					 type, description, match);

This one is constant, right? If not, I don't understand the locking design.

Which one? tsk->user->session_keyring is, tsk->signal->session_keyring is not.

Thanks for the review.

David

#8 lore raw

Re: [PATCH 2/3] Keys: Use RCU to manage session keyring pointer

Quoted 12 lines · David Howells
Quoted 9 lines · Paul E. McKenney

security/keys/process_keys.c static int install_session_keyring(struc · line 230

Quoted 4 lines
security/keys/process_keys.c +1 -2
 	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-	old = tsk->signal->session_keyring;
-	tsk->signal->session_keyring = keyring;
+	old = rcu_dereference(tsk->signal->session_keyring);

I don't understand why rcu_dereference() is needed in this case. Since we are holding the lock, it should not be possible for this to change, right? Or am I missing something? (Quite possible, am not all that familiar with this code.)

Quoted 8 lines

Erm... you're right. I stuck the rcu_dereference() in then added the locks back in when I realised I still needed them.

Quoted 3 lines

security/keys/process_keys.c static int install_session_keyring(struc · line 237

Quoted 1 line
security/keys/process_keys.c +1 -0
+	synchronize_kernel();

This would want to become synchronize_rcu().

I think the deprecation happened since I wrote my patch.

Yes, sorry, I should have made it clear that this was a change that affected your code rather than an error on your part.

Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
security/keys/process_keys.c +4 -0
+	if (tsk->signal->session_keyring) {
+		rcu_read_lock();
+		key = keyring_search_aux(
+			rcu_dereference(tsk->signal->session_keyring),
Quoted 8 lines

security/keys/process_keys.c struct key *search_process_keyrings_aux( · line 447

Quoted 6 lines
security/keys/process_keys.c +6 -0
+			type, description, match);
+		rcu_read_unlock();
+	}
+	else {
+		key = keyring_search_aux(tsk->user->session_keyring,
+					 type, description, match);

This one is constant, right? If not, I don't understand the locking design.

Quoted 1 line

Which one? tsk->user->session_keyring is, tsk->signal->session_keyring is not.

Good, that matches the code!

Thanx, Paul

Quoted 4 lines

Thanks for the review.

David

#9 lore raw
In reply to David Howells
Quoted 3 lines · David Howells

The attached patch makes it possible to pass a session keyring through to the
process spawned by call_usermodehelper().

hm. Seems likely to attract angry emails due to breakage of out-of-tree stuff. Did you consider

static inline int
call_usermodehelper(char *path, char **argv, char **envp, int wait)

{
	return call_usermodehelper_keys(path, argv, envp, NULL, wait);
}

?

author maintainer
#10 lore raw
Quoted 11 lines · Andrew Morton
Quoted 2 lines

The attached patch makes it possible to pass a session keyring through to
the process spawned by call_usermodehelper().

hm. Seems likely to attract angry emails due to breakage of out-of-tree stuff. Did you consider

static inline int
call_usermodehelper(char *path, char **argv, char **envp, int wait)

{
	return call_usermodehelper_keys(path, argv, envp, NULL, wait);
}

No. I can do that if you want. It seems a bit excessive though.

David

#11 lore raw
Quoted 10 lines · David Howells
Quoted 6 lines · Andrew Morton
Quoted 2 lines

The attached patch makes it possible to pass a session keyring through to
the process spawned by call_usermodehelper().

hm. Seems likely to attract angry emails due to breakage of out-of-tree stuff. Did you consider

Quoted 8 lines
Quoted 5 lines

static inline int
call_usermodehelper(char *path, char **argv, char **envp, int wait)

{
	return call_usermodehelper_keys(path, argv, envp, NULL, wait);
}

No. I can do that if you want. It seems a bit excessive though.

Well one question is "does it make sense to make a keyring session a part of the call_usermodehelper() API?". As it appears that only one caller will ever want to do that then I'd say no, and that it should be some specialised thing private to the key code and the call_usermodehelper() implementation.

So unless you think that a significant number of callers will appear who are actually using the new capability then it would be better to keep the existing call_usermodehelper() API.

author maintainer
#12 lore raw
Quoted 9 lines · Andrew Morton

Well one question is "does it make sense to make a keyring session a part of the call_usermodehelper() API?". As it appears that only one caller will ever want to do that then I'd say no, and that it should be some specialised thing private to the key code and the call_usermodehelper() implementation.

So unless you think that a significant number of callers will appear who are actually using the new capability then it would be better to keep the existing call_usermodehelper() API.

That's a good question, and one that's not easy to answer. Obviously, at the moment there will only be that one user. I'm not sure that other users will necessarily want to make use of it.

On the other hand, I can see the authorisation key bit being extended to provide all of these things with access to the authorising process's keyrings, whether or not they're constructing keys; but that can probably be made transparent.

David

#13 lore raw
In reply to Andrew Morton
Quoted 9 lines · Andrew Morton

Well one question is "does it make sense to make a keyring session a part of the call_usermodehelper() API?". As it appears that only one caller will ever want to do that then I'd say no, and that it should be some specialised thing private to the key code and the call_usermodehelper() implementation.

So unless you think that a significant number of callers will appear who are actually using the new capability then it would be better to keep the existing call_usermodehelper() API.

I'm fairly sure that OpenAFS or other AFS clients will need to make use of this when they move to the kernel keyring system. I had a discussion with Jeffrey Hutzelman on this topic a couple days ago on OpenAFS-Devel. The OpenAFS cache manager would want to call into userspace to convert between Kerberos and AFS tickets/tokens.

Cheers,
Kyle Moffett

-----BEGIN GEEK CODE BLOCK-----
Version: 3.12
GCM/CS/IT/U d- s++: a18 C++++>$ UB/L/X/*++++(+)>$ P+++(++++)>$ L++++(+++) E W++(+) N+++(++) o? K? w--- O? M++ V? PS+() PE+(-) Y+ PGP+++ t+(+++) 5 X R? tv-(--) b++++(++) DI+ D+ G e->++++$ h!*()>++$ r !y?(-) ------END GEEK CODE BLOCK------

#14 lore raw
In reply to Andrew Morton
Quoted 6 lines · Andrew Morton <akpm <at> osdl.org> writes:
Quoted 3 lines · David Howells <dhowells <at> redhat.com> wrote:

The attached patch makes it possible to pass a session keyring through to the
process spawned by call_usermodehelper().

Quoted 9 lines

hm. Seems likely to attract angry emails due to breakage of out-of-tree stuff. Did you consider

static inline int
call_usermodehelper(char *path, char **argv, char **envp, int wait)

{
	return call_usermodehelper_keys(path, argv, envp, NULL, wait);
}

An alternative is to have the execve happen in a callback, similar to the following patch I posted a couple months ago:

http://marc.theaimsgroup.com/?l=linux-kernel&m=109871749900732&w=2

IMHO, this is a better interface as it allows module writers to make arbitrary changes.

(Some interface for allowing modules to 'safely' do execve is still required though).

Mike Waychison

author maintainer
#15 lore raw

PATCH 3/3 Keys: Make request-key create an authorisation key

In reply to David Howells

The attached patch makes the following changes:

(1) There's a new special key type called ".request_key_auth".

This is an authorisation key for when one process requests a key and another process is started to construct it. This type of key cannot be created by the user; nor can it be requested by kernel services.

Authorisation keys hold two references:

(a) Each refers to a key being constructed. When the key being constructed is instantiated the authorisation key is revoked, rendering it of no further use.

(b) The "authorising process". This is either:

(i) the process that called request_key(), or:

(ii) if the process that called request_key() itself had an
authorisation key in its session keyring, then the authorising process referred to by that authorisation key will also be referred to by the new authorisation key.

This means that the process that initiated a chain of key requests will authorise the lot of them, and will, by default, wind up with the keys obtained from them in its keyrings.

(2) request_key() creates an authorisation key which is then passed to /sbin/request-key in as part of a new session keyring.

(3) When request_key() is searching for a key to hand back to the caller, if it comes across an authorisation key in the session keyring of the calling process, it will also search the keyrings of the process specified therein and it will use the specified process's credentials (fsuid, fsgid, groups) to do that rather than the calling process's credentials.

This allows a process started by /sbin/request-key to find keys belonging to the authorising process.

(4) A key can be read, even if the process executing KEYCTL_READ doesn't have direct read or search permission if that key is contained within the keyrings of a process specified by an authorisation key found within the calling process's session keyring, and is searchable using the credentials of the authorising process.

This allows a process started by /sbin/request-key to read keys belonging to the authorising process.

(5) The magic KEY_SPEC_*_KEYRING key IDs when passed to KEYCTL_INSTANTIATE or KEYCTL_NEGATE will specify a keyring of the authorising process, rather than the process doing the instantiation.

(6) One of the process keyrings can be nominated as the default to which request_key() should attach new keys if not otherwise specified. This is done with KEYCTL_SET_REQKEY_KEYRING and one of the KEY_REQKEY_DEFL_* constants. The current setting can also be read using this call.

(7) request_key() is partially interruptible. If it is waiting for another process to finish constructing a key, it can be interrupted. This permits a request-key cycle to be broken without recourse to rebooting.

---
warthog>diffstat -p1 keys-reqkey-2612rc1mm1.diff

 Documentation/keys.txt           |   34 +++++++
 include/linux/key-ui.h           |   41 ++++++++
 include/linux/key.h              |    9 -
 include/linux/keyctl.h           |   11 ++
 include/linux/sched.h            |    8 +
 kernel/sys.c                     |    2 
 security/keys/Makefile           |    5 -
 security/keys/compat.c           |    7 +
 security/keys/internal.h         |   45 ++++++++-
 security/keys/key.c              |   24 +++--
 security/keys/keyctl.c           |  174 ++++++++++++++++++++++++-------------
 security/keys/keyring.c          |   67 ++++++++++++--
 security/keys/process_keys.c     |  179 +++++++++++++++++++++++---------------
 security/keys/request_key.c      |  183 ++++++++++++++++++++++++++++++++-------
 security/keys/request_key_auth.c |  180 ++++++++++++++++++++++++++++++++++++++
 15 files changed, 779 insertions(+), 190 deletions(-)
Documentation/keys.txt +34 -0
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/Documentation/keys.txt linux-2.6.12-rc1-mm1-keys-reqkey/Documentation/keys.txt
@@ -591,6 +591,37 @@ The keyctl syscall functions are:
      this case too.
 
 
+ (*) Set the default request-key destination keyring.
+
+	long keyctl(KEYCTL_SET_REQKEY_KEYRING, int reqkey_defl);
+
+     This sets the default keyring to which implicitly requested keys will be
+     attached for this thread. reqkey_defl should be one of these constants:
+
+	CONSTANT				VALUE	NEW DEFAULT KEYRING
+	======================================	======	=======================
+	KEY_REQKEY_DEFL_NO_CHANGE		-1	No change
+	KEY_REQKEY_DEFL_DEFAULT			0	Default[1]
+	KEY_REQKEY_DEFL_THREAD_KEYRING		1	Thread keyring
+	KEY_REQKEY_DEFL_PROCESS_KEYRING		2	Process keyring
+	KEY_REQKEY_DEFL_SESSION_KEYRING		3	Session keyring
+	KEY_REQKEY_DEFL_USER_KEYRING		4	User keyring
+	KEY_REQKEY_DEFL_USER_SESSION_KEYRING	5	User session keyring
+	KEY_REQKEY_DEFL_GROUP_KEYRING		6	Group keyring
+
+     The old default will be returned if successful and error EINVAL will be
+     returned if reqkey_defl is not one of the above values.
+
+     The default keyring can be overridden by the keyring indicated to the
+     request_key() system call.
+
+     Note that this setting is inherited across fork/exec.
+
+     [1] The default default is: the thread keyring if there is one, otherwise
+     the process keyring if there is one, otherwise the session keyring if
+     there is one, otherwise the user default session keyring.
+
+
 ===============
 KERNEL SERVICES
 ===============
@@ -626,6 +657,9 @@ payload contents" for more information.
     Should the function fail error ENOKEY, EKEYEXPIRED or EKEYREVOKED will be
     returned.
 
+    If successful, the key will have been attached to the default keyring for
+    implicitly obtained request-key keys, as set by KEYCTL_SET_REQKEY_KEYRING.
+
 
 (*) When it is no longer required, the key should be released using:
 
include/linux/keyctl.h +11 -0
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/include/linux/keyctl.h linux-2.6.12-rc1-mm1-keys-reqkey/include/linux/keyctl.h
@@ -20,6 +20,16 @@
 #define KEY_SPEC_USER_SESSION_KEYRING	-5	/* - key ID for UID-session keyring */
 #define KEY_SPEC_GROUP_KEYRING		-6	/* - key ID for GID-specific keyring */
 
+/* request-key default keyrings */
+#define KEY_REQKEY_DEFL_NO_CHANGE		-1
+#define KEY_REQKEY_DEFL_DEFAULT			0
+#define KEY_REQKEY_DEFL_THREAD_KEYRING		1
+#define KEY_REQKEY_DEFL_PROCESS_KEYRING		2
+#define KEY_REQKEY_DEFL_SESSION_KEYRING		3
+#define KEY_REQKEY_DEFL_USER_KEYRING		4
+#define KEY_REQKEY_DEFL_USER_SESSION_KEYRING	5
+#define KEY_REQKEY_DEFL_GROUP_KEYRING		6
+
 /* keyctl commands */
 #define KEYCTL_GET_KEYRING_ID		0	/* ask for a keyring's ID */
 #define KEYCTL_JOIN_SESSION_KEYRING	1	/* join or start named session keyring */
@@ -35,5 +45,6 @@
 #define KEYCTL_READ			11	/* read a key or keyring's contents */
 #define KEYCTL_INSTANTIATE		12	/* instantiate a partially constructed key */
 #define KEYCTL_NEGATE			13	/* negate a partially constructed key */
+#define KEYCTL_SET_REQKEY_KEYRING	14	/* set default request-key keyring */
 
 #endif /*  _LINUX_KEYCTL_H */
include/linux/key.h +4 -5
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/include/linux/key.h linux-2.6.12-rc1-mm1-keys-reqkey/include/linux/key.h
@@ -199,10 +199,12 @@ extern int key_payload_reserve(struct ke
 extern int key_instantiate_and_link(struct key *key,
 				    const void *data,
 				    size_t datalen,
-				    struct key *keyring);
+				    struct key *keyring,
+				    struct key *instkey);
 extern int key_negate_and_link(struct key *key,
 			       unsigned timeout,
-			       struct key *keyring);
+			       struct key *keyring,
+			       struct key *instkey);
 extern void key_revoke(struct key *key);
 extern void key_put(struct key *key);
 
@@ -245,9 +247,6 @@ extern struct key *keyring_search(struct
 				  struct key_type *type,
 				  const char *description);
 
-extern struct key *search_process_keyrings(struct key_type *type,
-					   const char *description);
-
 extern int keyring_add_key(struct key *keyring,
 			   struct key *key);
 
include/linux/key-ui.h +39 -2

51 lines not on this page · raw message

include/linux/sched.h +5 -3
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/include/linux/sched.h linux-2.6.12-rc1-mm1-keys-reqkey/include/linux/sched.h
@@ -596,9 +596,10 @@ struct group_info {
 		groups_free(group_info); \
 } while (0)
 
-struct group_info *groups_alloc(int gidsetsize);
-void groups_free(struct group_info *group_info);
-int set_current_groups(struct group_info *group_info);
+extern struct group_info *groups_alloc(int gidsetsize);
+extern void groups_free(struct group_info *group_info);
+extern int set_current_groups(struct group_info *group_info);
+extern int groups_search(struct group_info *group_info, gid_t grp);
 /* access the groups "array" with this macro */
 #define GROUP_AT(gi, i) \
     ((gi)->blocks[(i)/NGROUPS_PER_BLOCK][(i)%NGROUPS_PER_BLOCK])
@@ -697,6 +698,7 @@ struct task_struct {
 	struct user_struct *user;
 #ifdef CONFIG_KEYS
 	struct key *thread_keyring;	/* keyring private to this thread */
+	unsigned char jit_keyring;	/* default keyring to attach requested keys to */
 #endif
 	int oomkilladj; /* OOM kill score adjustment (bit shift). */
 	char comm[TASK_COMM_LEN]; /* executable name excluding path
kernel/sys.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/kernel/sys.c linux-2.6.12-rc1-mm1-keys-reqkey/kernel/sys.c
@@ -1244,7 +1244,7 @@ static void groups_sort(struct group_inf
 }
 
 /* a simple bsearch */
-static int groups_search(struct group_info *group_info, gid_t grp)
+int groups_search(struct group_info *group_info, gid_t grp)
 {
 	int left, right;
 
security/keys/compat.c +5 -2
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/compat.c linux-2.6.12-rc1-mm1-keys-reqkey/security/keys/compat.c
@@ -1,6 +1,6 @@
 /* compat.c: 32-bit compatibility syscall for 64-bit systems
  *
- * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells ([email protected])
  *
  * This program is free software; you can redistribute it and/or
@@ -24,7 +24,7 @@
  * - if you can, you should call sys_keyctl directly
  */
 asmlinkage long compat_sys_keyctl(u32 option,
-			      u32 arg2, u32 arg3, u32 arg4, u32 arg5)
+				  u32 arg2, u32 arg3, u32 arg4, u32 arg5)
 {
 	switch (option) {
 	case KEYCTL_GET_KEYRING_ID:
@@ -71,6 +71,9 @@ asmlinkage long compat_sys_keyctl(u32 op
 	case KEYCTL_NEGATE:
 		return keyctl_negate_key(arg2, arg3, arg4);
 
+	case KEYCTL_SET_REQKEY_KEYRING:
+		return keyctl_set_reqkey_keyring(arg2);
+
 	default:
 		return -EOPNOTSUPP;
 	}
security/keys/internal.h +41 -4

79 lines not on this page · raw message

security/keys/key.c +18 -6
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/key.c linux-2.6.12-rc1-mm1-keys-reqkey/security/keys/key.c
@@ -1,6 +1,6 @@
 /* key.c: basic authentication token and access key management
  *
- * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells ([email protected])
  *
  * This program is free software; you can redistribute it and/or
@@ -391,7 +391,8 @@ EXPORT_SYMBOL(key_payload_reserve);
 static int __key_instantiate_and_link(struct key *key,
 				      const void *data,
 				      size_t datalen,
-				      struct key *keyring)
+				      struct key *keyring,
+				      struct key *instkey)
 {
 	int ret, awaken;
 
@@ -419,6 +420,10 @@ static int __key_instantiate_and_link(st
 			/* and link it into the destination keyring */
 			if (keyring)
 				ret = __key_link(keyring, key);
+
+			/* disable the authorisation key */
+			if (instkey)
+				key_revoke(instkey);
 		}
 	}
 
@@ -439,19 +444,21 @@ static int __key_instantiate_and_link(st
 int key_instantiate_and_link(struct key *key,
 			     const void *data,
 			     size_t datalen,
-			     struct key *keyring)
+			     struct key *keyring,
+			     struct key *instkey)
 {
 	int ret;
 
 	if (keyring)
 		down_write(&keyring->sem);
 
-	ret = __key_instantiate_and_link(key, data, datalen, keyring);
+	ret = __key_instantiate_and_link(key, data, datalen, keyring, instkey);
 
 	if (keyring)
 		up_write(&keyring->sem);
 
 	return ret;
+
 } /* end key_instantiate_and_link() */
 
 EXPORT_SYMBOL(key_instantiate_and_link);
@@ -462,7 +469,8 @@ EXPORT_SYMBOL(key_instantiate_and_link);
  */
 int key_negate_and_link(struct key *key,
 			unsigned timeout,
-			struct key *keyring)
+			struct key *keyring,
+			struct key *instkey)
 {
 	struct timespec now;
 	int ret, awaken;
@@ -495,6 +503,10 @@ int key_negate_and_link(struct key *key,
 		/* and link it into the destination keyring */
 		if (keyring)
 			ret = __key_link(keyring, key);
+
+		/* disable the authorisation key */
+		if (instkey)
+			key_revoke(instkey);
 	}
 
 	up_write(&key_construction_sem);
@@ -781,7 +793,7 @@ struct key *key_create_or_update(struct 
 	}
 
 	/* instantiate it and link it into the target keyring */
-	ret = __key_instantiate_and_link(key, payload, plen, keyring);
+	ret = __key_instantiate_and_link(key, payload, plen, keyring, NULL);
 	if (ret < 0) {
 		key_put(key);
 		key = ERR_PTR(ret);
security/keys/keyctl.c +117 -57

368 lines not on this page · raw message

security/keys/keyring.c +59 -8

124 lines not on this page · raw message

security/keys/Makefile +3 -2
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/Makefile linux-2.6.12-rc1-mm1-keys-reqkey/security/keys/Makefile
@@ -7,8 +7,9 @@ obj-y := \
 	keyring.o \
 	keyctl.o \
 	process_keys.o \
-	user_defined.o \
-	request_key.o
+	request_key.o \
+	request_key_auth.o \
+	user_defined.o
 
 obj-$(CONFIG_KEYS_COMPAT) += compat.o
 obj-$(CONFIG_PROC_FS) += proc.o
security/keys/process_keys.c +111 -68

304 lines not on this page · raw message

security/keys/request_key_auth.c +180 -0

180 lines not on this page · raw message

security/keys/request_key.c +151 -32

293 lines not on this page · raw message

author maintainer
#16 lore raw

PATCH 3/3 Keys: Make request-key create an authorisation key [try #2]

The attached patch makes the following changes:

(1) There's a new special key type called ".request_key_auth".

This is an authorisation key for when one process requests a key and another process is started to construct it. This type of key cannot be created by the user; nor can it be requested by kernel services.

Authorisation keys hold two references:

(a) Each refers to a key being constructed. When the key being constructed is instantiated the authorisation key is revoked, rendering it of no further use.

(b) The "authorising process". This is either:

(i) the process that called request_key(), or:

(ii) if the process that called request_key() itself had an
authorisation key in its session keyring, then the authorising process referred to by that authorisation key will also be referred to by the new authorisation key.

This means that the process that initiated a chain of key requests will authorise the lot of them, and will, by default, wind up with the keys obtained from them in its keyrings.

(2) request_key() creates an authorisation key which is then passed to /sbin/request-key in as part of a new session keyring.

(3) When request_key() is searching for a key to hand back to the caller, if it comes across an authorisation key in the session keyring of the calling process, it will also search the keyrings of the process specified therein and it will use the specified process's credentials (fsuid, fsgid, groups) to do that rather than the calling process's credentials.

This allows a process started by /sbin/request-key to find keys belonging to the authorising process.

(4) A key can be read, even if the process executing KEYCTL_READ doesn't have direct read or search permission if that key is contained within the keyrings of a process specified by an authorisation key found within the calling process's session keyring, and is searchable using the credentials of the authorising process.

This allows a process started by /sbin/request-key to read keys belonging to the authorising process.

(5) The magic KEY_SPEC_*_KEYRING key IDs when passed to KEYCTL_INSTANTIATE or KEYCTL_NEGATE will specify a keyring of the authorising process, rather than the process doing the instantiation.

(6) One of the process keyrings can be nominated as the default to which request_key() should attach new keys if not otherwise specified. This is done with KEYCTL_SET_REQKEY_KEYRING and one of the KEY_REQKEY_DEFL_* constants. The current setting can also be read using this call.

(7) request_key() is partially interruptible. If it is waiting for another process to finish constructing a key, it can be interrupted. This permits a request-key cycle to be broken without recourse to rebooting.

---
warthog>diffstat -p1 keys-reqkey-2612rc1mm1-2.diff

 Documentation/keys.txt           |   34 +++++++
 include/linux/key-ui.h           |   41 ++++++++
 include/linux/key.h              |    9 -
 include/linux/keyctl.h           |   11 ++
 include/linux/sched.h            |    8 +
 kernel/sys.c                     |    2 
 security/keys/Makefile           |    5 -
 security/keys/compat.c           |    7 +
 security/keys/internal.h         |   45 ++++++++-
 security/keys/key.c              |   24 +++--
 security/keys/keyctl.c           |  174 ++++++++++++++++++++++++-------------
 security/keys/keyring.c          |   67 ++++++++++++--
 security/keys/process_keys.c     |  179 +++++++++++++++++++++++---------------
 security/keys/request_key.c      |  183 ++++++++++++++++++++++++++++++++-------
 security/keys/request_key_auth.c |  180 ++++++++++++++++++++++++++++++++++++++
 15 files changed, 779 insertions(+), 190 deletions(-)
Documentation/keys.txt +34 -0
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/Documentation/keys.txt linux-2.6.12-rc1-mm1-keys-reqkey/Documentation/keys.txt
@@ -591,6 +591,37 @@ The keyctl syscall functions are:
      this case too.
 
 
+ (*) Set the default request-key destination keyring.
+
+	long keyctl(KEYCTL_SET_REQKEY_KEYRING, int reqkey_defl);
+
+     This sets the default keyring to which implicitly requested keys will be
+     attached for this thread. reqkey_defl should be one of these constants:
+
+	CONSTANT				VALUE	NEW DEFAULT KEYRING
+	======================================	======	=======================
+	KEY_REQKEY_DEFL_NO_CHANGE		-1	No change
+	KEY_REQKEY_DEFL_DEFAULT			0	Default[1]
+	KEY_REQKEY_DEFL_THREAD_KEYRING		1	Thread keyring
+	KEY_REQKEY_DEFL_PROCESS_KEYRING		2	Process keyring
+	KEY_REQKEY_DEFL_SESSION_KEYRING		3	Session keyring
+	KEY_REQKEY_DEFL_USER_KEYRING		4	User keyring
+	KEY_REQKEY_DEFL_USER_SESSION_KEYRING	5	User session keyring
+	KEY_REQKEY_DEFL_GROUP_KEYRING		6	Group keyring
+
+     The old default will be returned if successful and error EINVAL will be
+     returned if reqkey_defl is not one of the above values.
+
+     The default keyring can be overridden by the keyring indicated to the
+     request_key() system call.
+
+     Note that this setting is inherited across fork/exec.
+
+     [1] The default default is: the thread keyring if there is one, otherwise
+     the process keyring if there is one, otherwise the session keyring if
+     there is one, otherwise the user default session keyring.
+
+
 ===============
 KERNEL SERVICES
 ===============
@@ -626,6 +657,9 @@ payload contents" for more information.
     Should the function fail error ENOKEY, EKEYEXPIRED or EKEYREVOKED will be
     returned.
 
+    If successful, the key will have been attached to the default keyring for
+    implicitly obtained request-key keys, as set by KEYCTL_SET_REQKEY_KEYRING.
+
 
 (*) When it is no longer required, the key should be released using:
 
include/linux/keyctl.h +11 -0
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/include/linux/keyctl.h linux-2.6.12-rc1-mm1-keys-reqkey/include/linux/keyctl.h
@@ -20,6 +20,16 @@
 #define KEY_SPEC_USER_SESSION_KEYRING	-5	/* - key ID for UID-session keyring */
 #define KEY_SPEC_GROUP_KEYRING		-6	/* - key ID for GID-specific keyring */
 
+/* request-key default keyrings */
+#define KEY_REQKEY_DEFL_NO_CHANGE		-1
+#define KEY_REQKEY_DEFL_DEFAULT			0
+#define KEY_REQKEY_DEFL_THREAD_KEYRING		1
+#define KEY_REQKEY_DEFL_PROCESS_KEYRING		2
+#define KEY_REQKEY_DEFL_SESSION_KEYRING		3
+#define KEY_REQKEY_DEFL_USER_KEYRING		4
+#define KEY_REQKEY_DEFL_USER_SESSION_KEYRING	5
+#define KEY_REQKEY_DEFL_GROUP_KEYRING		6
+
 /* keyctl commands */
 #define KEYCTL_GET_KEYRING_ID		0	/* ask for a keyring's ID */
 #define KEYCTL_JOIN_SESSION_KEYRING	1	/* join or start named session keyring */
@@ -35,5 +45,6 @@
 #define KEYCTL_READ			11	/* read a key or keyring's contents */
 #define KEYCTL_INSTANTIATE		12	/* instantiate a partially constructed key */
 #define KEYCTL_NEGATE			13	/* negate a partially constructed key */
+#define KEYCTL_SET_REQKEY_KEYRING	14	/* set default request-key keyring */
 
 #endif /*  _LINUX_KEYCTL_H */
include/linux/key.h +4 -5
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/include/linux/key.h linux-2.6.12-rc1-mm1-keys-reqkey/include/linux/key.h
@@ -199,10 +199,12 @@ extern int key_payload_reserve(struct ke
 extern int key_instantiate_and_link(struct key *key,
 				    const void *data,
 				    size_t datalen,
-				    struct key *keyring);
+				    struct key *keyring,
+				    struct key *instkey);
 extern int key_negate_and_link(struct key *key,
 			       unsigned timeout,
-			       struct key *keyring);
+			       struct key *keyring,
+			       struct key *instkey);
 extern void key_revoke(struct key *key);
 extern void key_put(struct key *key);
 
@@ -245,9 +247,6 @@ extern struct key *keyring_search(struct
 				  struct key_type *type,
 				  const char *description);
 
-extern struct key *search_process_keyrings(struct key_type *type,
-					   const char *description);
-
 extern int keyring_add_key(struct key *keyring,
 			   struct key *key);
 
include/linux/key-ui.h +39 -2

51 lines not on this page · raw message

include/linux/sched.h +5 -3
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/include/linux/sched.h linux-2.6.12-rc1-mm1-keys-reqkey/include/linux/sched.h
@@ -596,9 +596,10 @@ struct group_info {
 		groups_free(group_info); \
 } while (0)
 
-struct group_info *groups_alloc(int gidsetsize);
-void groups_free(struct group_info *group_info);
-int set_current_groups(struct group_info *group_info);
+extern struct group_info *groups_alloc(int gidsetsize);
+extern void groups_free(struct group_info *group_info);
+extern int set_current_groups(struct group_info *group_info);
+extern int groups_search(struct group_info *group_info, gid_t grp);
 /* access the groups "array" with this macro */
 #define GROUP_AT(gi, i) \
     ((gi)->blocks[(i)/NGROUPS_PER_BLOCK][(i)%NGROUPS_PER_BLOCK])
@@ -697,6 +698,7 @@ struct task_struct {
 	struct user_struct *user;
 #ifdef CONFIG_KEYS
 	struct key *thread_keyring;	/* keyring private to this thread */
+	unsigned char jit_keyring;	/* default keyring to attach requested keys to */
 #endif
 	int oomkilladj; /* OOM kill score adjustment (bit shift). */
 	char comm[TASK_COMM_LEN]; /* executable name excluding path
kernel/sys.c +1 -1
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/kernel/sys.c linux-2.6.12-rc1-mm1-keys-reqkey/kernel/sys.c
@@ -1244,7 +1244,7 @@ static void groups_sort(struct group_inf
 }
 
 /* a simple bsearch */
-static int groups_search(struct group_info *group_info, gid_t grp)
+int groups_search(struct group_info *group_info, gid_t grp)
 {
 	int left, right;
 
security/keys/compat.c +5 -2
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/compat.c linux-2.6.12-rc1-mm1-keys-reqkey/security/keys/compat.c
@@ -1,6 +1,6 @@
 /* compat.c: 32-bit compatibility syscall for 64-bit systems
  *
- * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells ([email protected])
  *
  * This program is free software; you can redistribute it and/or
@@ -24,7 +24,7 @@
  * - if you can, you should call sys_keyctl directly
  */
 asmlinkage long compat_sys_keyctl(u32 option,
-			      u32 arg2, u32 arg3, u32 arg4, u32 arg5)
+				  u32 arg2, u32 arg3, u32 arg4, u32 arg5)
 {
 	switch (option) {
 	case KEYCTL_GET_KEYRING_ID:
@@ -71,6 +71,9 @@ asmlinkage long compat_sys_keyctl(u32 op
 	case KEYCTL_NEGATE:
 		return keyctl_negate_key(arg2, arg3, arg4);
 
+	case KEYCTL_SET_REQKEY_KEYRING:
+		return keyctl_set_reqkey_keyring(arg2);
+
 	default:
 		return -EOPNOTSUPP;
 	}
security/keys/internal.h +41 -4

79 lines not on this page · raw message

security/keys/key.c +18 -6
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/key.c linux-2.6.12-rc1-mm1-keys-reqkey/security/keys/key.c
@@ -1,6 +1,6 @@
 /* key.c: basic authentication token and access key management
  *
- * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells ([email protected])
  *
  * This program is free software; you can redistribute it and/or
@@ -391,7 +391,8 @@ EXPORT_SYMBOL(key_payload_reserve);
 static int __key_instantiate_and_link(struct key *key,
 				      const void *data,
 				      size_t datalen,
-				      struct key *keyring)
+				      struct key *keyring,
+				      struct key *instkey)
 {
 	int ret, awaken;
 
@@ -419,6 +420,10 @@ static int __key_instantiate_and_link(st
 			/* and link it into the destination keyring */
 			if (keyring)
 				ret = __key_link(keyring, key);
+
+			/* disable the authorisation key */
+			if (instkey)
+				key_revoke(instkey);
 		}
 	}
 
@@ -439,19 +444,21 @@ static int __key_instantiate_and_link(st
 int key_instantiate_and_link(struct key *key,
 			     const void *data,
 			     size_t datalen,
-			     struct key *keyring)
+			     struct key *keyring,
+			     struct key *instkey)
 {
 	int ret;
 
 	if (keyring)
 		down_write(&keyring->sem);
 
-	ret = __key_instantiate_and_link(key, data, datalen, keyring);
+	ret = __key_instantiate_and_link(key, data, datalen, keyring, instkey);
 
 	if (keyring)
 		up_write(&keyring->sem);
 
 	return ret;
+
 } /* end key_instantiate_and_link() */
 
 EXPORT_SYMBOL(key_instantiate_and_link);
@@ -462,7 +469,8 @@ EXPORT_SYMBOL(key_instantiate_and_link);
  */
 int key_negate_and_link(struct key *key,
 			unsigned timeout,
-			struct key *keyring)
+			struct key *keyring,
+			struct key *instkey)
 {
 	struct timespec now;
 	int ret, awaken;
@@ -495,6 +503,10 @@ int key_negate_and_link(struct key *key,
 		/* and link it into the destination keyring */
 		if (keyring)
 			ret = __key_link(keyring, key);
+
+		/* disable the authorisation key */
+		if (instkey)
+			key_revoke(instkey);
 	}
 
 	up_write(&key_construction_sem);
@@ -781,7 +793,7 @@ struct key *key_create_or_update(struct 
 	}
 
 	/* instantiate it and link it into the target keyring */
-	ret = __key_instantiate_and_link(key, payload, plen, keyring);
+	ret = __key_instantiate_and_link(key, payload, plen, keyring, NULL);
 	if (ret < 0) {
 		key_put(key);
 		key = ERR_PTR(ret);
security/keys/keyctl.c +117 -57

368 lines not on this page · raw message

security/keys/keyring.c +59 -8

124 lines not on this page · raw message

security/keys/Makefile +3 -2
diff -uNrp linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/Makefile linux-2.6.12-rc1-mm1-keys-reqkey/security/keys/Makefile
@@ -7,8 +7,9 @@ obj-y := \
 	keyring.o \
 	keyctl.o \
 	process_keys.o \
-	user_defined.o \
-	request_key.o
+	request_key.o \
+	request_key_auth.o \
+	user_defined.o
 
 obj-$(CONFIG_KEYS_COMPAT) += compat.o
 obj-$(CONFIG_PROC_FS) += proc.o
security/keys/process_keys.c +111 -68

304 lines not on this page · raw message

security/keys/request_key_auth.c +180 -0

180 lines not on this page · raw message

security/keys/request_key.c +151 -32

293 lines not on this page · raw message

#17 lore raw

Re: [PATCH 3/3] Keys: Make request-key create an authorisation key

In reply to David Howells
Quoted 35 lines · David Howells

The attached patch makes the following changes:

(6) One of the process keyrings can be nominated as the default to which request_key() should attach new keys if not otherwise specified. This is done with KEYCTL_SET_REQKEY_KEYRING and one of the KEY_REQKEY_DEFL_* constants. The current setting can also be read using this call.

---

(quoted diff) +20 -0
@@ -903,6 +922,44 @@ long keyctl_negate_key(key_serial_t id,
 
 /*****************************************************************************/
 /*
+ * set the default keyring in which request_key() will cache keys
+ * - return the old setting
+ */
+long keyctl_set_reqkey_keyring(int reqkey_defl)
+{
+       int ret;
+
+       switch (reqkey_defl) {
+       case KEY_REQKEY_DEFL_THREAD_KEYRING:
+               ret = install_thread_keyring(current);
+               if (ret < 0)
+                       return ret;
+               goto set;
+
+       case KEY_REQKEY_DEFL_PROCESS_KEYRING:
+               ret = install_process_keyring(current);
+               if (ret < 0)
+                       return ret;
+
+       case KEY_REQKEY_DEFL_DEFAULT:

Review long keyctl_negate_key(key_serial_t id, · line 954

Quoted 10 lines
(quoted diff) +10 -0
+       case KEY_REQKEY_DEFL_SESSION_KEYRING:
+       case KEY_REQKEY_DEFL_USER_KEYRING:
+       case KEY_REQKEY_DEFL_USER_SESSION_KEYRING:
+       set:
+               current->jit_keyring = reqkey_defl;
+
+       case KEY_REQKEY_DEFL_NO_CHANGE:
+               return current->jit_keyring;
+
+       case KEY_SPEC_GROUP_KEYRING:

KEY_REQKEY_DEFL__GROUP_KEYRING

Quoted 43 lines
(quoted diff) +38 -0
+       default:
+               return -EINVAL;
+       }
+
+} /* end keyctl_set_reqkey_keyring() */
+
 
@@ -267,21 +294,84 @@ static struct key *request_key_construct
 
 /*****************************************************************************/
 /*
+ * link a freshly minted key to an appropriate destination keyring
+ */
+static void request_key_link(struct key *key, struct key *dest_keyring)
+{
+       struct task_struct *tsk = current;
+       struct key *drop = NULL;
+
+       kenter("{%d},%p", key->serial, dest_keyring);
+
+       /* find the appropriate keyring */
+       if (!dest_keyring) {
+               switch (tsk->jit_keyring) {
+               case KEY_REQKEY_DEFL_DEFAULT:
+               case KEY_REQKEY_DEFL_THREAD_KEYRING:
+                       dest_keyring = tsk->thread_keyring;
+                       if (dest_keyring)
+                               break;
+
+               case KEY_REQKEY_DEFL_PROCESS_KEYRING:
+                       dest_keyring = tsk->signal->process_keyring;
+                       if (dest_keyring)
+                               break;
+
+               case KEY_REQKEY_DEFL_SESSION_KEYRING:
+                       rcu_read_lock();
+                       dest_keyring = key_get(
+                               rcu_dereference(tsk->signal->session_keyring));
+                       rcu_read_unlock();
+                       drop = dest_keyring;
+
+                       if (dest_keyring)
+                               break;

Review static struct key *request_key_construct · line 338

Quoted 10 lines
(quoted diff) +10 -0
+
+               case KEY_REQKEY_DEFL_USER_SESSION_KEYRING:
+                       dest_keyring = current->user->session_keyring;
+                       break;
+
+               case KEY_REQKEY_DEFL_USER_KEYRING:
+                       dest_keyring = current->user->uid_keyring;
+                       break;
+
+               case KEY_REQKEY_DEFL_NO_CHANGE:

gcc-4 warns about this (warning: case label value is less than minimum value for type) and it shouldn't be in jit_keyring anyway.

Review static struct key *request_key_construct · line 339

Quoted 1 line
(quoted diff) +1 -0
+               case KEY_SPEC_GROUP_KEYRING:

KEY_REQKEY_DEFL_GROUP_KEYRING

Review static struct key *request_key_construct · line 346

Quoted 7 lines
(quoted diff) +7 -0
+               default:
+                       BUG();
+               }
+       }
+
+       /* and attach the key to it */
+       key_link(dest_keyring, key);

patch attached.

regards,

Benoit

security/keys/request_key.c renamed from security/keys/request_key.c.orig +1 -2
@@ -335,8 +335,7 @@ static void request_key_link(struct key 
 			dest_keyring = current->user->uid_keyring;
 			break;
 
-		case KEY_REQKEY_DEFL_NO_CHANGE:
-		case KEY_SPEC_GROUP_KEYRING:
+		case KEY_REQKEY_DEFL_GROUP_KEYRING:
 		default:
 			BUG();
 		}
security/keys/keyctl.c renamed from security/keys/keyctl.c.orig +1 -1
@@ -951,7 +951,7 @@ long keyctl_set_reqkey_keyring(int reqke
 	case KEY_REQKEY_DEFL_NO_CHANGE:
 		return current->jit_keyring;
 
-	case KEY_SPEC_GROUP_KEYRING:
+	case KEY_REQKEY_DEFL_GROUP_KEYRING:
 	default:
 		return -EINVAL;
 	}
author maintainer
#18 lore raw

PATCH 1/1 Keys: Fix request_key default keyring handling

The attached patch fixes the way request_key handles the default destination key when it's the group keyring. It also removes the check for the no-change default keyring spec, which shouldn't appear in the task_struct::jit_keyring member (it's purely for getting the old value from the keyctl function).

---
warthog>diffstat -p1 /tmp/keys.patch

 security/keys/keyctl.c      |    2 +-
 security/keys/request_key.c |    3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)
security/keys/request_key.c renamed from security/keys/request_key.c.orig +1 -2
@@ -335,8 +335,7 @@ static void request_key_link(struct key 
 			dest_keyring = current->user->uid_keyring;
 			break;
 
-		case KEY_REQKEY_DEFL_NO_CHANGE:
-		case KEY_SPEC_GROUP_KEYRING:
+		case KEY_REQKEY_DEFL_GROUP_KEYRING:
 		default:
 			BUG();
 		}
security/keys/keyctl.c renamed from security/keys/keyctl.c.orig +1 -1
@@ -951,7 +951,7 @@ long keyctl_set_reqkey_keyring(int reqke
 	case KEY_REQKEY_DEFL_NO_CHANGE:
 		return current->jit_keyring;
 
-	case KEY_SPEC_GROUP_KEYRING:
+	case KEY_REQKEY_DEFL_GROUP_KEYRING:
 	default:
 		return -EINVAL;
 	}