Keys: Pass session keyring to call_usermodehelper()
Subsystems (15)
David Howells
maintainerPATCH 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -323,7 +323,7 @@ static int eppconfig(struct baycom_statesprintf (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
@@ -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
@@ -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
@@ -273,6 +273,13 @@ extern void key_fsuid_changed(struct tasextern 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
@@ -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
@@ -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
@@ -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 (¤t->sighand->siglock); + old_session =__install_session_keyring (current, sub_info->ring);flush_signal_handlers (current,1 );sigemptyset (¤t->blocked);recalc_sigpending ();spin_unlock_irq (¤t->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
@@ -370,7 +370,7 @@ void kobject_hotplug(struct kobject *kobif (!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
@@ -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() */
David Howells
maintainerRe: [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
@@ -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 (¤t->sighand->siglock, flags); +rcu_read_lock (); tsk->signal->session_keyring = -key_get (current->signal->session_keyring); -spin_unlock_irqrestore (¤t->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
@@ -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 (¤t->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 (¤t->sighand->siglock, flags); +rcu_read_unlock ();key_link (keyring, key);key_put (keyring);
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?
David Howells
maintainerRe: [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
David Howells
maintainerPATCH 1/3 Keys: Pass session keyring to call_usermodehelper() [try #2]
In reply to David HowellsThe 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
@@ -273,14 +273,22 @@ extern void key_fsuid_changed(struct tasextern 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
@@ -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
@@ -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 (¤t->sighand->siglock); + old_session =__install_session_keyring (current, sub_info->ring);flush_signal_handlers (current,1 );sigemptyset (¤t->blocked);recalc_sigpending ();spin_unlock_irq (¤t->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
@@ -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() */
David Howells
maintainerPATCH 2/3 Keys: Use RCU to manage session keyring pointer
In reply to David HowellsThe 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
@@ -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 (¤t->sighand->siglock, flags); +rcu_read_lock (); tsk->signal->session_keyring = -key_get (current->signal->session_keyring); -spin_unlock_irqrestore (¤t->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
@@ -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 (¤t->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 (¤t->sighand->siglock, flags); +rcu_read_unlock ();key_link (keyring, key);key_put (keyring);
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.Signed-off-by David Howells <[email protected]>
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.diffsecurity/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.clinux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/process_keys.csecurity/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
-15key_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 (¤t->sighand->siglock, flags); +rcu_read_lock (); tsk->signal->session_keyring = -key_get (current->signal->session_keyring); -spin_unlock_irqrestore (¤t->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.clinux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/request_key.csecurity/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 (¤t->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 (¤t->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/
David Howells
maintainerRe: [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
-2spin_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
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
-2spin_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_keyringis,tsk->signal->session_keyringis not.
Good, that matches the code!
Thanx, Paul
Quoted 4 lines
Thanks for the review.
David
Quoted 3 lines · David Howells
The attached patch makes it possible to pass a session keyring through to the
process spawned bycall_usermodehelper().
hm. Seems likely to attract angry emails due to breakage of out-of-tree stuff. Did you consider
static inline intcall_usermodehelper(char *path, char **argv, char **envp, int wait)
{
return call_usermodehelper_keys(path, argv, envp, NULL, wait);
}?
David Howells
maintainerQuoted 11 lines · Andrew Morton
Quoted 2 lines
The attached patch makes it possible to pass a session keyring through to
the process spawned bycall_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
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 bycall_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.
David Howells
maintainerQuoted 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 thecall_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
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 thecall_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------
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 bycall_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
David Howells
maintainerPATCH 3/3 Keys: Make request-key create an authorisation key
In reply to David HowellsThe 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
@@ -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
@@ -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
@@ -199,10 +199,12 @@ extern int key_payload_reserve(struct keextern 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(structstruct 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
@@ -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
@@ -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
@@ -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 opcase 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
@@ -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(stint 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
@@ -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
David Howells
maintainerPATCH 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
@@ -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
@@ -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
@@ -199,10 +199,12 @@ extern int key_payload_reserve(struct keextern 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(structstruct 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
@@ -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
@@ -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
@@ -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 opcase 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
@@ -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(stint 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
@@ -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
Re: [PATCH 3/3] Keys: Make request-key create an authorisation key
In reply to David HowellsQuoted 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 withKEYCTL_SET_REQKEY_KEYRINGand one of the KEY_REQKEY_DEFL_* constants. The current setting can also be read using this call.Signed-off-by David Howells <[email protected]>---
(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 reqkecase KEY_REQKEY_DEFL_NO_CHANGE :return current->jit_keyring; -case KEY_SPEC_GROUP_KEYRING : +case KEY_REQKEY_DEFL_GROUP_KEYRING :default :return -EINVAL ; }
David Howells
maintainerPATCH 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 reqkecase KEY_REQKEY_DEFL_NO_CHANGE :return current->jit_keyring; -case KEY_SPEC_GROUP_KEYRING : +case KEY_REQKEY_DEFL_GROUP_KEYRING :default :return -EINVAL ; }