Commit 822ad64d authored by David Howells's avatar David Howells Committed by James Morris

keys: Fix dependency loop between construction record and auth key

In the request_key() upcall mechanism there's a dependency loop by which if
a key type driver overrides the ->request_key hook and the userspace side
manages to lose the authorisation key, the auth key and the internal
construction record (struct key_construction) can keep each other pinned.

Fix this by the following changes:

 (1) Killing off the construction record and using the auth key instead.

 (2) Including the operation name in the auth key payload and making the
     payload available outside of security/keys/.

 (3) The ->request_key hook is given the authkey instead of the cons
     record and operation name.

Changes (2) and (3) allow the auth key to naturally be cleaned up if the
keyring it is in is destroyed or cleared or the auth key is unlinked.

Fixes: 7ee02a316600 ("keys: Fix dependency loop between construction record and auth key")
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
parent bb2ba2d7
...@@ -44,6 +44,7 @@ ...@@ -44,6 +44,7 @@
#include <linux/keyctl.h> #include <linux/keyctl.h>
#include <linux/key-type.h> #include <linux/key-type.h>
#include <keys/user-type.h> #include <keys/user-type.h>
#include <keys/request_key_auth-type.h>
#include <linux/module.h> #include <linux/module.h>
#include "internal.h" #include "internal.h"
...@@ -59,7 +60,7 @@ static struct key_type key_type_id_resolver_legacy; ...@@ -59,7 +60,7 @@ static struct key_type key_type_id_resolver_legacy;
struct idmap_legacy_upcalldata { struct idmap_legacy_upcalldata {
struct rpc_pipe_msg pipe_msg; struct rpc_pipe_msg pipe_msg;
struct idmap_msg idmap_msg; struct idmap_msg idmap_msg;
struct key_construction *key_cons; struct key *authkey;
struct idmap *idmap; struct idmap *idmap;
}; };
...@@ -384,7 +385,7 @@ static const match_table_t nfs_idmap_tokens = { ...@@ -384,7 +385,7 @@ static const match_table_t nfs_idmap_tokens = {
{ Opt_find_err, NULL } { Opt_find_err, NULL }
}; };
static int nfs_idmap_legacy_upcall(struct key_construction *, const char *, void *); static int nfs_idmap_legacy_upcall(struct key *, void *);
static ssize_t idmap_pipe_downcall(struct file *, const char __user *, static ssize_t idmap_pipe_downcall(struct file *, const char __user *,
size_t); size_t);
static void idmap_release_pipe(struct inode *); static void idmap_release_pipe(struct inode *);
...@@ -549,11 +550,12 @@ nfs_idmap_prepare_pipe_upcall(struct idmap *idmap, ...@@ -549,11 +550,12 @@ nfs_idmap_prepare_pipe_upcall(struct idmap *idmap,
static void static void
nfs_idmap_complete_pipe_upcall_locked(struct idmap *idmap, int ret) nfs_idmap_complete_pipe_upcall_locked(struct idmap *idmap, int ret)
{ {
struct key_construction *cons = idmap->idmap_upcall_data->key_cons; struct key *authkey = idmap->idmap_upcall_data->authkey;
kfree(idmap->idmap_upcall_data); kfree(idmap->idmap_upcall_data);
idmap->idmap_upcall_data = NULL; idmap->idmap_upcall_data = NULL;
complete_request_key(cons, ret); complete_request_key(authkey, ret);
key_put(authkey);
} }
static void static void
...@@ -563,15 +565,14 @@ nfs_idmap_abort_pipe_upcall(struct idmap *idmap, int ret) ...@@ -563,15 +565,14 @@ nfs_idmap_abort_pipe_upcall(struct idmap *idmap, int ret)
nfs_idmap_complete_pipe_upcall_locked(idmap, ret); nfs_idmap_complete_pipe_upcall_locked(idmap, ret);
} }
static int nfs_idmap_legacy_upcall(struct key_construction *cons, static int nfs_idmap_legacy_upcall(struct key *authkey, void *aux)
const char *op,
void *aux)
{ {
struct idmap_legacy_upcalldata *data; struct idmap_legacy_upcalldata *data;
struct request_key_auth *rka = get_request_key_auth(authkey);
struct rpc_pipe_msg *msg; struct rpc_pipe_msg *msg;
struct idmap_msg *im; struct idmap_msg *im;
struct idmap *idmap = (struct idmap *)aux; struct idmap *idmap = (struct idmap *)aux;
struct key *key = cons->key; struct key *key = rka->target_key;
int ret = -ENOKEY; int ret = -ENOKEY;
if (!aux) if (!aux)
...@@ -586,7 +587,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons, ...@@ -586,7 +587,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
msg = &data->pipe_msg; msg = &data->pipe_msg;
im = &data->idmap_msg; im = &data->idmap_msg;
data->idmap = idmap; data->idmap = idmap;
data->key_cons = cons; data->authkey = key_get(authkey);
ret = nfs_idmap_prepare_message(key->description, idmap, im, msg); ret = nfs_idmap_prepare_message(key->description, idmap, im, msg);
if (ret < 0) if (ret < 0)
...@@ -604,7 +605,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons, ...@@ -604,7 +605,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
out2: out2:
kfree(data); kfree(data);
out1: out1:
complete_request_key(cons, ret); complete_request_key(authkey, ret);
return ret; return ret;
} }
...@@ -651,9 +652,10 @@ static int nfs_idmap_read_and_verify_message(struct idmap_msg *im, ...@@ -651,9 +652,10 @@ static int nfs_idmap_read_and_verify_message(struct idmap_msg *im,
static ssize_t static ssize_t
idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen) idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
{ {
struct request_key_auth *rka;
struct rpc_inode *rpci = RPC_I(file_inode(filp)); struct rpc_inode *rpci = RPC_I(file_inode(filp));
struct idmap *idmap = (struct idmap *)rpci->private; struct idmap *idmap = (struct idmap *)rpci->private;
struct key_construction *cons; struct key *authkey;
struct idmap_msg im; struct idmap_msg im;
size_t namelen_in; size_t namelen_in;
int ret = -ENOKEY; int ret = -ENOKEY;
...@@ -665,7 +667,8 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen) ...@@ -665,7 +667,8 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
if (idmap->idmap_upcall_data == NULL) if (idmap->idmap_upcall_data == NULL)
goto out_noupcall; goto out_noupcall;
cons = idmap->idmap_upcall_data->key_cons; authkey = idmap->idmap_upcall_data->authkey;
rka = get_request_key_auth(authkey);
if (mlen != sizeof(im)) { if (mlen != sizeof(im)) {
ret = -ENOSPC; ret = -ENOSPC;
...@@ -690,9 +693,9 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen) ...@@ -690,9 +693,9 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
ret = nfs_idmap_read_and_verify_message(&im, ret = nfs_idmap_read_and_verify_message(&im,
&idmap->idmap_upcall_data->idmap_msg, &idmap->idmap_upcall_data->idmap_msg,
cons->key, cons->authkey); rka->target_key, authkey);
if (ret >= 0) { if (ret >= 0) {
key_set_timeout(cons->key, nfs_idmap_cache_timeout); key_set_timeout(rka->target_key, nfs_idmap_cache_timeout);
ret = mlen; ret = mlen;
} }
......
/* request_key authorisation token key type
*
* Copyright (C) 2005 Red Hat, Inc. All Rights Reserved.
* Written by David Howells (dhowells@redhat.com)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public Licence
* as published by the Free Software Foundation; either version
* 2 of the Licence, or (at your option) any later version.
*/
#ifndef _KEYS_REQUEST_KEY_AUTH_TYPE_H
#define _KEYS_REQUEST_KEY_AUTH_TYPE_H
#include <linux/key.h>
/*
* Authorisation record for request_key().
*/
struct request_key_auth {
struct key *target_key;
struct key *dest_keyring;
const struct cred *cred;
void *callout_info;
size_t callout_len;
pid_t pid;
char op[8];
} __randomize_layout;
static inline struct request_key_auth *get_request_key_auth(const struct key *key)
{
return key->payload.data[0];
}
#endif /* _KEYS_REQUEST_KEY_AUTH_TYPE_H */
...@@ -20,15 +20,6 @@ ...@@ -20,15 +20,6 @@
struct kernel_pkey_query; struct kernel_pkey_query;
struct kernel_pkey_params; struct kernel_pkey_params;
/*
* key under-construction record
* - passed to the request_key actor if supplied
*/
struct key_construction {
struct key *key; /* key being constructed */
struct key *authkey;/* authorisation for key being constructed */
};
/* /*
* Pre-parsed payload, used by key add, update and instantiate. * Pre-parsed payload, used by key add, update and instantiate.
* *
...@@ -50,8 +41,7 @@ struct key_preparsed_payload { ...@@ -50,8 +41,7 @@ struct key_preparsed_payload {
time64_t expiry; /* Expiry time of key */ time64_t expiry; /* Expiry time of key */
} __randomize_layout; } __randomize_layout;
typedef int (*request_key_actor_t)(struct key_construction *key, typedef int (*request_key_actor_t)(struct key *auth_key, void *aux);
const char *op, void *aux);
/* /*
* Preparsed matching criterion. * Preparsed matching criterion.
...@@ -181,20 +171,20 @@ extern int key_instantiate_and_link(struct key *key, ...@@ -181,20 +171,20 @@ extern int key_instantiate_and_link(struct key *key,
const void *data, const void *data,
size_t datalen, size_t datalen,
struct key *keyring, struct key *keyring,
struct key *instkey); struct key *authkey);
extern int key_reject_and_link(struct key *key, extern int key_reject_and_link(struct key *key,
unsigned timeout, unsigned timeout,
unsigned error, unsigned error,
struct key *keyring, struct key *keyring,
struct key *instkey); struct key *authkey);
extern void complete_request_key(struct key_construction *cons, int error); extern void complete_request_key(struct key *authkey, int error);
static inline int key_negate_and_link(struct key *key, static inline int key_negate_and_link(struct key *key,
unsigned timeout, unsigned timeout,
struct key *keyring, struct key *keyring,
struct key *instkey) struct key *authkey)
{ {
return key_reject_and_link(key, timeout, ENOKEY, keyring, instkey); return key_reject_and_link(key, timeout, ENOKEY, keyring, authkey);
} }
extern int generic_key_instantiate(struct key *key, struct key_preparsed_payload *prep); extern int generic_key_instantiate(struct key *key, struct key_preparsed_payload *prep);
......
...@@ -186,20 +186,9 @@ static inline int key_permission(const key_ref_t key_ref, unsigned perm) ...@@ -186,20 +186,9 @@ static inline int key_permission(const key_ref_t key_ref, unsigned perm)
return key_task_permission(key_ref, current_cred(), perm); return key_task_permission(key_ref, current_cred(), perm);
} }
/*
* Authorisation record for request_key().
*/
struct request_key_auth {
struct key *target_key;
struct key *dest_keyring;
const struct cred *cred;
void *callout_info;
size_t callout_len;
pid_t pid;
} __randomize_layout;
extern struct key_type key_type_request_key_auth; extern struct key_type key_type_request_key_auth;
extern struct key *request_key_auth_new(struct key *target, extern struct key *request_key_auth_new(struct key *target,
const char *op,
const void *callout_info, const void *callout_info,
size_t callout_len, size_t callout_len,
struct key *dest_keyring); struct key *dest_keyring);
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include <linux/security.h> #include <linux/security.h>
#include <linux/uio.h> #include <linux/uio.h>
#include <linux/uaccess.h> #include <linux/uaccess.h>
#include <keys/request_key_auth-type.h>
#include "internal.h" #include "internal.h"
#define KEY_MAX_DESC_SIZE 4096 #define KEY_MAX_DESC_SIZE 4096
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include <linux/security.h> #include <linux/security.h>
#include <linux/user_namespace.h> #include <linux/user_namespace.h>
#include <linux/uaccess.h> #include <linux/uaccess.h>
#include <keys/request_key_auth-type.h>
#include "internal.h" #include "internal.h"
/* Session keyring create vs join semaphore */ /* Session keyring create vs join semaphore */
......
...@@ -18,31 +18,30 @@ ...@@ -18,31 +18,30 @@
#include <linux/keyctl.h> #include <linux/keyctl.h>
#include <linux/slab.h> #include <linux/slab.h>
#include "internal.h" #include "internal.h"
#include <keys/request_key_auth-type.h>
#define key_negative_timeout 60 /* default timeout on a negative key's existence */ #define key_negative_timeout 60 /* default timeout on a negative key's existence */
/** /**
* complete_request_key - Complete the construction of a key. * complete_request_key - Complete the construction of a key.
* @cons: The key construction record. * @auth_key: The authorisation key.
* @error: The success or failute of the construction. * @error: The success or failute of the construction.
* *
* Complete the attempt to construct a key. The key will be negated * Complete the attempt to construct a key. The key will be negated
* if an error is indicated. The authorisation key will be revoked * if an error is indicated. The authorisation key will be revoked
* unconditionally. * unconditionally.
*/ */
void complete_request_key(struct key_construction *cons, int error) void complete_request_key(struct key *authkey, int error)
{ {
kenter("{%d,%d},%d", cons->key->serial, cons->authkey->serial, error); struct request_key_auth *rka = get_request_key_auth(authkey);
struct key *key = rka->target_key;
kenter("%d{%d},%d", authkey->serial, key->serial, error);
if (error < 0) if (error < 0)
key_negate_and_link(cons->key, key_negative_timeout, NULL, key_negate_and_link(key, key_negative_timeout, NULL, authkey);
cons->authkey);
else else
key_revoke(cons->authkey); key_revoke(authkey);
key_put(cons->key);
key_put(cons->authkey);
kfree(cons);
} }
EXPORT_SYMBOL(complete_request_key); EXPORT_SYMBOL(complete_request_key);
...@@ -91,21 +90,19 @@ static int call_usermodehelper_keys(const char *path, char **argv, char **envp, ...@@ -91,21 +90,19 @@ static int call_usermodehelper_keys(const char *path, char **argv, char **envp,
* Request userspace finish the construction of a key * Request userspace finish the construction of a key
* - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>" * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"
*/ */
static int call_sbin_request_key(struct key_construction *cons, static int call_sbin_request_key(struct key *authkey, void *aux)
const char *op,
void *aux)
{ {
static char const request_key[] = "/sbin/request-key"; static char const request_key[] = "/sbin/request-key";
struct request_key_auth *rka = get_request_key_auth(authkey);
const struct cred *cred = current_cred(); const struct cred *cred = current_cred();
key_serial_t prkey, sskey; key_serial_t prkey, sskey;
struct key *key = cons->key, *authkey = cons->authkey, *keyring, struct key *key = rka->target_key, *keyring, *session;
*session;
char *argv[9], *envp[3], uid_str[12], gid_str[12]; char *argv[9], *envp[3], uid_str[12], gid_str[12];
char key_str[12], keyring_str[3][12]; char key_str[12], keyring_str[3][12];
char desc[20]; char desc[20];
int ret, i; int ret, i;
kenter("{%d},{%d},%s", key->serial, authkey->serial, op); kenter("{%d},{%d},%s", key->serial, authkey->serial, rka->op);
ret = install_user_keyrings(); ret = install_user_keyrings();
if (ret < 0) if (ret < 0)
...@@ -163,7 +160,7 @@ static int call_sbin_request_key(struct key_construction *cons, ...@@ -163,7 +160,7 @@ static int call_sbin_request_key(struct key_construction *cons,
/* set up the argument list */ /* set up the argument list */
i = 0; i = 0;
argv[i++] = (char *)request_key; argv[i++] = (char *)request_key;
argv[i++] = (char *) op; argv[i++] = (char *)rka->op;
argv[i++] = key_str; argv[i++] = key_str;
argv[i++] = uid_str; argv[i++] = uid_str;
argv[i++] = gid_str; argv[i++] = gid_str;
...@@ -191,7 +188,7 @@ static int call_sbin_request_key(struct key_construction *cons, ...@@ -191,7 +188,7 @@ static int call_sbin_request_key(struct key_construction *cons,
key_put(keyring); key_put(keyring);
error_alloc: error_alloc:
complete_request_key(cons, ret); complete_request_key(authkey, ret);
kleave(" = %d", ret); kleave(" = %d", ret);
return ret; return ret;
} }
...@@ -205,42 +202,31 @@ static int construct_key(struct key *key, const void *callout_info, ...@@ -205,42 +202,31 @@ static int construct_key(struct key *key, const void *callout_info,
size_t callout_len, void *aux, size_t callout_len, void *aux,
struct key *dest_keyring) struct key *dest_keyring)
{ {
struct key_construction *cons;
request_key_actor_t actor; request_key_actor_t actor;
struct key *authkey; struct key *authkey;
int ret; int ret;
kenter("%d,%p,%zu,%p", key->serial, callout_info, callout_len, aux); kenter("%d,%p,%zu,%p", key->serial, callout_info, callout_len, aux);
cons = kmalloc(sizeof(*cons), GFP_KERNEL);
if (!cons)
return -ENOMEM;
/* allocate an authorisation key */ /* allocate an authorisation key */
authkey = request_key_auth_new(key, callout_info, callout_len, authkey = request_key_auth_new(key, "create", callout_info, callout_len,
dest_keyring); dest_keyring);
if (IS_ERR(authkey)) { if (IS_ERR(authkey))
kfree(cons); return PTR_ERR(authkey);
ret = PTR_ERR(authkey);
authkey = NULL;
} else {
cons->authkey = key_get(authkey);
cons->key = key_get(key);
/* make the call */ /* Make the call */
actor = call_sbin_request_key; actor = call_sbin_request_key;
if (key->type->request_key) if (key->type->request_key)
actor = key->type->request_key; actor = key->type->request_key;
ret = actor(cons, "create", aux); ret = actor(authkey, aux);
/* check that the actor called complete_request_key() prior to /* check that the actor called complete_request_key() prior to
* returning an error */ * returning an error */
WARN_ON(ret < 0 && WARN_ON(ret < 0 &&
!test_bit(KEY_FLAG_REVOKED, &authkey->flags)); !test_bit(KEY_FLAG_REVOKED, &authkey->flags));
key_put(authkey);
}
key_put(authkey);
kleave(" = %d", ret); kleave(" = %d", ret);
return ret; return ret;
} }
...@@ -275,7 +261,7 @@ static int construct_get_dest_keyring(struct key **_dest_keyring) ...@@ -275,7 +261,7 @@ static int construct_get_dest_keyring(struct key **_dest_keyring)
if (cred->request_key_auth) { if (cred->request_key_auth) {
authkey = cred->request_key_auth; authkey = cred->request_key_auth;
down_read(&authkey->sem); down_read(&authkey->sem);
rka = authkey->payload.data[0]; rka = get_request_key_auth(authkey);
if (!test_bit(KEY_FLAG_REVOKED, if (!test_bit(KEY_FLAG_REVOKED,
&authkey->flags)) &authkey->flags))
dest_keyring = dest_keyring =
......
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/uaccess.h> #include <linux/uaccess.h>
#include "internal.h" #include "internal.h"
#include <keys/user-type.h> #include <keys/request_key_auth-type.h>
static int request_key_auth_preparse(struct key_preparsed_payload *); static int request_key_auth_preparse(struct key_preparsed_payload *);
static void request_key_auth_free_preparse(struct key_preparsed_payload *); static void request_key_auth_free_preparse(struct key_preparsed_payload *);
...@@ -68,7 +68,7 @@ static int request_key_auth_instantiate(struct key *key, ...@@ -68,7 +68,7 @@ static int request_key_auth_instantiate(struct key *key,
static void request_key_auth_describe(const struct key *key, static void request_key_auth_describe(const struct key *key,
struct seq_file *m) struct seq_file *m)
{ {
struct request_key_auth *rka = key->payload.data[0]; struct request_key_auth *rka = get_request_key_auth(key);
seq_puts(m, "key:"); seq_puts(m, "key:");
seq_puts(m, key->description); seq_puts(m, key->description);
...@@ -83,7 +83,7 @@ static void request_key_auth_describe(const struct key *key, ...@@ -83,7 +83,7 @@ static void request_key_auth_describe(const struct key *key,
static long request_key_auth_read(const struct key *key, static long request_key_auth_read(const struct key *key,
char __user *buffer, size_t buflen) char __user *buffer, size_t buflen)
{ {
struct request_key_auth *rka = key->payload.data[0]; struct request_key_auth *rka = get_request_key_auth(key);
size_t datalen; size_t datalen;
long ret; long ret;
...@@ -109,7 +109,7 @@ static long request_key_auth_read(const struct key *key, ...@@ -109,7 +109,7 @@ static long request_key_auth_read(const struct key *key,
*/ */
static void request_key_auth_revoke(struct key *key) static void request_key_auth_revoke(struct key *key)
{ {
struct request_key_auth *rka = key->payload.data[0]; struct request_key_auth *rka = get_request_key_auth(key);
kenter("{%d}", key->serial); kenter("{%d}", key->serial);
...@@ -136,7 +136,7 @@ static void free_request_key_auth(struct request_key_auth *rka) ...@@ -136,7 +136,7 @@ static void free_request_key_auth(struct request_key_auth *rka)
*/ */
static void request_key_auth_destroy(struct key *key) static void request_key_auth_destroy(struct key *key)
{ {
struct request_key_auth *rka = key->payload.data[0]; struct request_key_auth *rka = get_request_key_auth(key);
kenter("{%d}", key->serial); kenter("{%d}", key->serial);
...@@ -147,8 +147,9 @@ static void request_key_auth_destroy(struct key *key) ...@@ -147,8 +147,9 @@ static void request_key_auth_destroy(struct key *key)
* Create an authorisation token for /sbin/request-key or whoever to gain * Create an authorisation token for /sbin/request-key or whoever to gain
* access to the caller's security data. * access to the caller's security data.
*/ */
struct key *request_key_auth_new(struct key *target, const void *callout_info, struct key *request_key_auth_new(struct key *target, const char *op,
size_t callout_len, struct key *dest_keyring) const void *callout_info, size_t callout_len,
struct key *dest_keyring)
{ {
struct request_key_auth *rka, *irka; struct request_key_auth *rka, *irka;
const struct cred *cred = current->cred; const struct cred *cred = current->cred;
...@@ -166,6 +167,7 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info, ...@@ -166,6 +167,7 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
if (!rka->callout_info) if (!rka->callout_info)
goto error_free_rka; goto error_free_rka;
rka->callout_len = callout_len; rka->callout_len = callout_len;
strlcpy(rka->op, op, sizeof(rka->op));
/* see if the calling process is already servicing the key request of /* see if the calling process is already servicing the key request of
* another process */ * another process */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment