From 504b69eb3c95180bc59f1ae9096ad4b10bbbf254 Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 19 Jun 2019 16:10:15 +0100 Subject: [PATCH 1/6] keys: Fix request_key() lack of Link perm check on found key The request_key() syscall allows a process to gain access to the 'possessor' permits of any key that grants it Search permission by virtue of request_key() not checking whether a key it finds grants Link permission to the caller. Signed-off-by: David Howells --- Documentation/security/keys/core.rst | 4 ++++ security/keys/request_key.c | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst index 823d29bf44f7..82dd457ff78d 100644 --- a/Documentation/security/keys/core.rst +++ b/Documentation/security/keys/core.rst @@ -433,6 +433,10 @@ The main syscalls are: /sbin/request-key will be invoked in an attempt to obtain a key. The callout_info string will be passed as an argument to the program. + To link a key into the destination keyring the key must grant link + permission on the key to the caller and the keyring must grant write + permission. + See also Documentation/security/keys/request-key.rst. diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 857da65e1940..a6543ed98b1f 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -564,6 +564,16 @@ struct key *request_key_and_link(struct key_type *type, key_ref = search_process_keyrings(&ctx); if (!IS_ERR(key_ref)) { + if (dest_keyring) { + ret = key_task_permission(key_ref, current_cred(), + KEY_NEED_LINK); + if (ret < 0) { + key_ref_put(key_ref); + key = ERR_PTR(ret); + goto error_free; + } + } + key = key_ref_to_ptr(key_ref); if (dest_keyring) { ret = key_link(dest_keyring, key); From a09003b5d7cea71ce4b59e409d5a7158c789e1b4 Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 19 Jun 2019 16:10:15 +0100 Subject: [PATCH 2/6] keys: Invalidate used request_key authentication keys Invalidate used request_key authentication keys rather than revoking them so that they get cleaned up immediately rather than potentially hanging around. There doesn't seem any need to keep the revoked keys around. Signed-off-by: David Howells --- security/keys/key.c | 4 ++-- security/keys/request_key.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/security/keys/key.c b/security/keys/key.c index bba71acec886..e792d65c0af8 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -459,7 +459,7 @@ static int __key_instantiate_and_link(struct key *key, /* disable the authorisation key */ if (authkey) - key_revoke(authkey); + key_invalidate(authkey); if (prep->expiry != TIME64_MAX) { key->expiry = prep->expiry; @@ -616,7 +616,7 @@ int key_reject_and_link(struct key *key, /* disable the authorisation key */ if (authkey) - key_revoke(authkey); + key_invalidate(authkey); } mutex_unlock(&key_construction_mutex); diff --git a/security/keys/request_key.c b/security/keys/request_key.c index a6543ed98b1f..244e538d113f 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -222,7 +222,7 @@ static int construct_key(struct key *key, const void *callout_info, /* check that the actor called complete_request_key() prior to * returning an error */ WARN_ON(ret < 0 && - !test_bit(KEY_FLAG_REVOKED, &authkey->flags)); + !test_bit(KEY_FLAG_INVALIDATED, &authkey->flags)); key_put(authkey); kleave(" = %d", ret); From e59428f721ee096d8a020504ea908a6f0d952735 Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 19 Jun 2019 16:10:15 +0100 Subject: [PATCH 3/6] keys: Move the RCU locks outwards from the keyring search functions Move the RCU locks outwards from the keyring search functions so that it will become possible to provide an RCU-capable partial request_key() function in a later commit. Signed-off-by: David Howells --- Documentation/security/keys/request-key.rst | 2 +- include/keys/request_key_auth-type.h | 1 + security/keys/internal.h | 6 +-- security/keys/keyring.c | 16 +++--- security/keys/proc.c | 4 +- security/keys/process_keys.c | 41 +++++++------- security/keys/request_key.c | 8 ++- security/keys/request_key_auth.c | 60 ++++++++++++--------- 8 files changed, 77 insertions(+), 61 deletions(-) diff --git a/Documentation/security/keys/request-key.rst b/Documentation/security/keys/request-key.rst index 600ad67d1707..07af991463b5 100644 --- a/Documentation/security/keys/request-key.rst +++ b/Documentation/security/keys/request-key.rst @@ -148,7 +148,7 @@ The Search Algorithm A search of any particular keyring proceeds in the following fashion: - 1) When the key management code searches for a key (keyring_search_aux) it + 1) When the key management code searches for a key (keyring_search_rcu) it firstly calls key_permission(SEARCH) on the keyring it's starting with, if this denies permission, it doesn't search further. diff --git a/include/keys/request_key_auth-type.h b/include/keys/request_key_auth-type.h index a726dd3f1dc6..2a046062bb42 100644 --- a/include/keys/request_key_auth-type.h +++ b/include/keys/request_key_auth-type.h @@ -18,6 +18,7 @@ * Authorisation record for request_key(). */ struct request_key_auth { + struct rcu_head rcu; struct key *target_key; struct key *dest_keyring; const struct cred *cred; diff --git a/security/keys/internal.h b/security/keys/internal.h index d04bff631227..3d5c08db74d2 100644 --- a/security/keys/internal.h +++ b/security/keys/internal.h @@ -139,11 +139,11 @@ struct keyring_search_context { extern bool key_default_cmp(const struct key *key, const struct key_match_data *match_data); -extern key_ref_t keyring_search_aux(key_ref_t keyring_ref, +extern key_ref_t keyring_search_rcu(key_ref_t keyring_ref, struct keyring_search_context *ctx); -extern key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx); -extern key_ref_t search_process_keyrings(struct keyring_search_context *ctx); +extern key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx); +extern key_ref_t search_process_keyrings_rcu(struct keyring_search_context *ctx); extern struct key *find_keyring_by_name(const char *name, bool uid_keyring); diff --git a/security/keys/keyring.c b/security/keys/keyring.c index 67066bb58b83..afa6d4024c67 100644 --- a/security/keys/keyring.c +++ b/security/keys/keyring.c @@ -835,7 +835,7 @@ found: } /** - * keyring_search_aux - Search a keyring tree for a key matching some criteria + * keyring_search_rcu - Search a keyring tree for a matching key under RCU * @keyring_ref: A pointer to the keyring with possession indicator. * @ctx: The keyring search context. * @@ -847,7 +847,9 @@ found: * addition, the LSM gets to forbid keyring searches and key matches. * * The search is performed as a breadth-then-depth search up to the prescribed - * limit (KEYRING_SEARCH_MAX_DEPTH). + * limit (KEYRING_SEARCH_MAX_DEPTH). The caller must hold the RCU read lock to + * prevent keyrings from being destroyed or rearranged whilst they are being + * searched. * * Keys are matched to the type provided and are then filtered by the match * function, which is given the description to use in any way it sees fit. The @@ -866,7 +868,7 @@ found: * In the case of a successful return, the possession attribute from * @keyring_ref is propagated to the returned key reference. */ -key_ref_t keyring_search_aux(key_ref_t keyring_ref, +key_ref_t keyring_search_rcu(key_ref_t keyring_ref, struct keyring_search_context *ctx) { struct key *keyring; @@ -888,11 +890,9 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref, return ERR_PTR(err); } - rcu_read_lock(); ctx->now = ktime_get_real_seconds(); if (search_nested_keyrings(keyring, ctx)) __key_get(key_ref_to_ptr(ctx->result)); - rcu_read_unlock(); return ctx->result; } @@ -902,7 +902,7 @@ key_ref_t keyring_search_aux(key_ref_t keyring_ref, * @type: The type of keyring we want to find. * @description: The name of the keyring we want to find. * - * As keyring_search_aux() above, but using the current task's credentials and + * As keyring_search_rcu() above, but using the current task's credentials and * type's default matching function and preferred search method. */ key_ref_t keyring_search(key_ref_t keyring, @@ -928,7 +928,9 @@ key_ref_t keyring_search(key_ref_t keyring, return ERR_PTR(ret); } - key = keyring_search_aux(keyring, &ctx); + rcu_read_lock(); + key = keyring_search_rcu(keyring, &ctx); + rcu_read_unlock(); if (type->match_free) type->match_free(&ctx.match_data); diff --git a/security/keys/proc.c b/security/keys/proc.c index 78ac305d715e..f081dceae3b9 100644 --- a/security/keys/proc.c +++ b/security/keys/proc.c @@ -179,7 +179,9 @@ static int proc_keys_show(struct seq_file *m, void *v) * skip if the key does not indicate the possessor can view it */ if (key->perm & KEY_POS_VIEW) { - skey_ref = search_my_process_keyrings(&ctx); + rcu_read_lock(); + skey_ref = search_cred_keyrings_rcu(&ctx); + rcu_read_unlock(); if (!IS_ERR(skey_ref)) { key_ref_put(skey_ref); key_ref = make_key_ref(key, 1); diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index 39aaa21462bf..f8ffb06d0297 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -318,7 +318,8 @@ void key_fsgid_changed(struct cred *new_cred) /* * Search the process keyrings attached to the supplied cred for the first - * matching key. + * matching key under RCU conditions (the caller must be holding the RCU read + * lock). * * The search criteria are the type and the match function. The description is * given to the match function as a parameter, but doesn't otherwise influence @@ -337,7 +338,7 @@ void key_fsgid_changed(struct cred *new_cred) * In the case of a successful return, the possession attribute is set on the * returned key reference. */ -key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) +key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx) { key_ref_t key_ref, ret, err; const struct cred *cred = ctx->cred; @@ -355,7 +356,7 @@ key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) /* search the thread keyring first */ if (cred->thread_keyring) { - key_ref = keyring_search_aux( + key_ref = keyring_search_rcu( make_key_ref(cred->thread_keyring, 1), ctx); if (!IS_ERR(key_ref)) goto found; @@ -373,7 +374,7 @@ key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) /* search the process keyring second */ if (cred->process_keyring) { - key_ref = keyring_search_aux( + key_ref = keyring_search_rcu( make_key_ref(cred->process_keyring, 1), ctx); if (!IS_ERR(key_ref)) goto found; @@ -394,7 +395,7 @@ key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) /* search the session keyring */ if (cred->session_keyring) { - key_ref = keyring_search_aux( + key_ref = keyring_search_rcu( make_key_ref(cred->session_keyring, 1), ctx); if (!IS_ERR(key_ref)) @@ -415,7 +416,7 @@ key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) } /* or search the user-session keyring */ else if (READ_ONCE(cred->user->session_keyring)) { - key_ref = keyring_search_aux( + key_ref = keyring_search_rcu( make_key_ref(READ_ONCE(cred->user->session_keyring), 1), ctx); if (!IS_ERR(key_ref)) @@ -448,16 +449,16 @@ found: * the keys attached to the assumed authorisation key using its credentials if * one is available. * - * Return same as search_my_process_keyrings(). + * The caller must be holding the RCU read lock. + * + * Return same as search_cred_keyrings_rcu(). */ -key_ref_t search_process_keyrings(struct keyring_search_context *ctx) +key_ref_t search_process_keyrings_rcu(struct keyring_search_context *ctx) { struct request_key_auth *rka; key_ref_t key_ref, ret = ERR_PTR(-EACCES), err; - might_sleep(); - - key_ref = search_my_process_keyrings(ctx); + key_ref = search_cred_keyrings_rcu(ctx); if (!IS_ERR(key_ref)) goto found; err = key_ref; @@ -472,24 +473,17 @@ key_ref_t search_process_keyrings(struct keyring_search_context *ctx) ) { const struct cred *cred = ctx->cred; - /* defend against the auth key being revoked */ - down_read(&cred->request_key_auth->sem); - - if (key_validate(ctx->cred->request_key_auth) == 0) { + if (key_validate(cred->request_key_auth) == 0) { rka = ctx->cred->request_key_auth->payload.data[0]; + //// was search_process_keyrings() [ie. recursive] ctx->cred = rka->cred; - key_ref = search_process_keyrings(ctx); + key_ref = search_cred_keyrings_rcu(ctx); ctx->cred = cred; - up_read(&cred->request_key_auth->sem); - if (!IS_ERR(key_ref)) goto found; - ret = key_ref; - } else { - up_read(&cred->request_key_auth->sem); } } @@ -504,7 +498,6 @@ key_ref_t search_process_keyrings(struct keyring_search_context *ctx) found: return key_ref; } - /* * See if the key we're looking at is the target key. */ @@ -691,7 +684,9 @@ try_again: ctx.index_key = key->index_key; ctx.match_data.raw_data = key; kdebug("check possessed"); - skey_ref = search_process_keyrings(&ctx); + rcu_read_lock(); + skey_ref = search_process_keyrings_rcu(&ctx); + rcu_read_unlock(); kdebug("possessed=%p", skey_ref); if (!IS_ERR(skey_ref)) { diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 244e538d113f..bf1d223ec21c 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -385,7 +385,9 @@ static int construct_alloc_key(struct keyring_search_context *ctx, * waited for locks */ mutex_lock(&key_construction_mutex); - key_ref = search_process_keyrings(ctx); + rcu_read_lock(); + key_ref = search_process_keyrings_rcu(ctx); + rcu_read_unlock(); if (!IS_ERR(key_ref)) goto key_already_present; @@ -561,7 +563,9 @@ struct key *request_key_and_link(struct key_type *type, } /* search all the process keyrings for a key */ - key_ref = search_process_keyrings(&ctx); + rcu_read_lock(); + key_ref = search_process_keyrings_rcu(&ctx); + rcu_read_unlock(); if (!IS_ERR(key_ref)) { if (dest_keyring) { diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c index ec5226557023..99ed7a8a273d 100644 --- a/security/keys/request_key_auth.c +++ b/security/keys/request_key_auth.c @@ -58,7 +58,7 @@ static void request_key_auth_free_preparse(struct key_preparsed_payload *prep) static int request_key_auth_instantiate(struct key *key, struct key_preparsed_payload *prep) { - key->payload.data[0] = (struct request_key_auth *)prep->data; + rcu_assign_keypointer(key, (struct request_key_auth *)prep->data); return 0; } @@ -68,7 +68,7 @@ static int request_key_auth_instantiate(struct key *key, static void request_key_auth_describe(const struct key *key, struct seq_file *m) { - struct request_key_auth *rka = get_request_key_auth(key); + struct request_key_auth *rka = dereference_key_rcu(key); seq_puts(m, "key:"); seq_puts(m, key->description); @@ -83,7 +83,7 @@ static void request_key_auth_describe(const struct key *key, static long request_key_auth_read(const struct key *key, char __user *buffer, size_t buflen) { - struct request_key_auth *rka = get_request_key_auth(key); + struct request_key_auth *rka = dereference_key_locked(key); size_t datalen; long ret; @@ -102,23 +102,6 @@ static long request_key_auth_read(const struct key *key, return ret; } -/* - * Handle revocation of an authorisation token key. - * - * Called with the key sem write-locked. - */ -static void request_key_auth_revoke(struct key *key) -{ - struct request_key_auth *rka = get_request_key_auth(key); - - kenter("{%d}", key->serial); - - if (rka->cred) { - put_cred(rka->cred); - rka->cred = NULL; - } -} - static void free_request_key_auth(struct request_key_auth *rka) { if (!rka) @@ -131,16 +114,43 @@ static void free_request_key_auth(struct request_key_auth *rka) kfree(rka); } +/* + * Dispose of the request_key_auth record under RCU conditions + */ +static void request_key_auth_rcu_disposal(struct rcu_head *rcu) +{ + struct request_key_auth *rka = + container_of(rcu, struct request_key_auth, rcu); + + free_request_key_auth(rka); +} + +/* + * Handle revocation of an authorisation token key. + * + * Called with the key sem write-locked. + */ +static void request_key_auth_revoke(struct key *key) +{ + struct request_key_auth *rka = dereference_key_locked(key); + + kenter("{%d}", key->serial); + rcu_assign_keypointer(key, NULL); + call_rcu(&rka->rcu, request_key_auth_rcu_disposal); +} + /* * Destroy an instantiation authorisation token key. */ static void request_key_auth_destroy(struct key *key) { - struct request_key_auth *rka = get_request_key_auth(key); + struct request_key_auth *rka = rcu_access_pointer(key->payload.rcu_data0); kenter("{%d}", key->serial); - - free_request_key_auth(rka); + if (rka) { + rcu_assign_keypointer(key, NULL); + call_rcu(&rka->rcu, request_key_auth_rcu_disposal); + } } /* @@ -249,7 +259,9 @@ struct key *key_get_instantiation_authkey(key_serial_t target_id) ctx.index_key.desc_len = sprintf(description, "%x", target_id); - authkey_ref = search_process_keyrings(&ctx); + rcu_read_lock(); + authkey_ref = search_process_keyrings_rcu(&ctx); + rcu_read_unlock(); if (IS_ERR(authkey_ref)) { authkey = ERR_CAST(authkey_ref); From 896f1950e5944532b971d880a6bae7fba3b6a8d3 Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 19 Jun 2019 16:10:15 +0100 Subject: [PATCH 4/6] keys: Provide request_key_rcu() Provide a request_key_rcu() function that can be used to request a key under RCU conditions. It can only search and check permissions; it cannot allocate a new key, upcall or wait for an upcall to complete. It may return a partially constructed key. Signed-off-by: David Howells --- Documentation/security/keys/core.rst | 10 +++++ Documentation/security/keys/request-key.rst | 9 +++++ include/linux/key.h | 3 ++ security/keys/request_key.c | 44 +++++++++++++++++++++ 4 files changed, 66 insertions(+) diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst index 82dd457ff78d..003f1452a5b7 100644 --- a/Documentation/security/keys/core.rst +++ b/Documentation/security/keys/core.rst @@ -1147,6 +1147,16 @@ payload contents" for more information. case error ERESTARTSYS will be returned. + * To search for a key under RCU conditions, call:: + + struct key *request_key_rcu(const struct key_type *type, + const char *description); + + which is similar to request_key() except that it does not check for keys + that are under construction and it will not call out to userspace to + construct a key if it can't find a match. + + * When it is no longer required, the key should be released using:: void key_put(struct key *key); diff --git a/Documentation/security/keys/request-key.rst b/Documentation/security/keys/request-key.rst index 07af991463b5..7caedc4d29f1 100644 --- a/Documentation/security/keys/request-key.rst +++ b/Documentation/security/keys/request-key.rst @@ -36,6 +36,11 @@ or:: size_t callout_len, void *aux); +or:: + + struct key *request_key_rcu(const struct key_type *type, + const char *description); + Or by userspace invoking the request_key system call:: key_serial_t request_key(const char *type, @@ -57,6 +62,10 @@ The two async in-kernel calls may return keys that are still in the process of being constructed. The two non-async ones will wait for construction to complete first. +The request_key_rcu() call is like the in-kernel request_key() call, except +that it doesn't check for keys that are under construction and doesn't attempt +to construct missing keys. + The userspace interface links the key to a keyring associated with the process to prevent the key from going away, and returns the serial number of the key to the caller. diff --git a/include/linux/key.h b/include/linux/key.h index 612e1cf84049..3604a554df99 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -274,6 +274,9 @@ extern struct key *request_key(struct key_type *type, const char *description, const char *callout_info); +extern struct key *request_key_rcu(struct key_type *type, + const char *description); + extern struct key *request_key_with_auxdata(struct key_type *type, const char *description, const void *callout_info, diff --git a/security/keys/request_key.c b/security/keys/request_key.c index bf1d223ec21c..b4b3677657d6 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -756,3 +756,47 @@ struct key *request_key_async_with_auxdata(struct key_type *type, callout_len, aux, NULL, KEY_ALLOC_IN_QUOTA); } EXPORT_SYMBOL(request_key_async_with_auxdata); + +/** + * request_key_rcu - Request key from RCU-read-locked context + * @type: The type of key we want. + * @description: The name of the key we want. + * + * Request a key from a context that we may not sleep in (such as RCU-mode + * pathwalk). Keys under construction are ignored. + * + * Return a pointer to the found key if successful, -ENOKEY if we couldn't find + * a key or some other error if the key found was unsuitable or inaccessible. + */ +struct key *request_key_rcu(struct key_type *type, const char *description) +{ + struct keyring_search_context ctx = { + .index_key.type = type, + .index_key.description = description, + .index_key.desc_len = strlen(description), + .cred = current_cred(), + .match_data.cmp = key_default_cmp, + .match_data.raw_data = description, + .match_data.lookup_type = KEYRING_SEARCH_LOOKUP_DIRECT, + .flags = (KEYRING_SEARCH_DO_STATE_CHECK | + KEYRING_SEARCH_SKIP_EXPIRED), + }; + struct key *key; + key_ref_t key_ref; + + kenter("%s,%s", type->name, description); + + /* search all the process keyrings for a key */ + key_ref = search_process_keyrings_rcu(&ctx); + if (IS_ERR(key_ref)) { + key = ERR_CAST(key_ref); + if (PTR_ERR(key_ref) == -EAGAIN) + key = ERR_PTR(-ENOKEY); + } else { + key = key_ref_to_ptr(key_ref); + } + + kleave(" = %p", key); + return key; +} +EXPORT_SYMBOL(request_key_rcu); From 7743c48e54ee9be9c799cbf3b8e3e9f2b8d19e72 Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 19 Jun 2019 16:10:15 +0100 Subject: [PATCH 5/6] keys: Cache result of request_key*() temporarily in task_struct If a filesystem uses keys to hold authentication tokens, then it needs a token for each VFS operation that might perform an authentication check - either by passing it to the server, or using to perform a check based on authentication data cached locally. For open files this isn't a problem, since the key should be cached in the file struct since it represents the subject performing operations on that file descriptor. During pathwalk, however, there isn't anywhere to cache the key, except perhaps in the nameidata struct - but that isn't exposed to the filesystems. Further, a pathwalk can incur a lot of operations, calling one or more of the following, for instance: ->lookup() ->permission() ->d_revalidate() ->d_automount() ->get_acl() ->getxattr() on each dentry/inode it encounters - and each one may need to call request_key(). And then, at the end of pathwalk, it will call the actual operation: ->mkdir() ->mknod() ->getattr() ->open() ... which may need to go and get the token again. However, it is very likely that all of the operations on a single dentry/inode - and quite possibly a sequence of them - will all want to use the same authentication token, which suggests that caching it would be a good idea. To this end: (1) Make it so that a positive result of request_key() and co. that didn't require upcalling to userspace is cached temporarily in task_struct. (2) The cache is 1 deep, so a new result displaces the old one. (3) The key is released by exit and by notify-resume. (4) The cache is cleared in a newly forked process. Signed-off-by: David Howells --- Documentation/security/keys/request-key.rst | 7 +++- include/linux/sched.h | 5 +++ include/linux/tracehook.h | 7 ++++ kernel/cred.c | 9 +++++ security/keys/Kconfig | 18 ++++++++++ security/keys/request_key.c | 37 +++++++++++++++++++++ 6 files changed, 82 insertions(+), 1 deletion(-) diff --git a/Documentation/security/keys/request-key.rst b/Documentation/security/keys/request-key.rst index 7caedc4d29f1..45049abdf290 100644 --- a/Documentation/security/keys/request-key.rst +++ b/Documentation/security/keys/request-key.rst @@ -176,6 +176,9 @@ The process stops immediately a valid key is found with permission granted to use it. Any error from a previous match attempt is discarded and the key is returned. +When request_key() is invoked, if CONFIG_KEYS_REQUEST_CACHE=y, a per-task +one-key cache is first checked for a match. + When search_process_keyrings() is invoked, it performs the following searches until one succeeds: @@ -195,7 +198,9 @@ until one succeeds: c) The calling process's session keyring is searched. The moment one succeeds, all pending errors are discarded and the found key is -returned. +returned. If CONFIG_KEYS_REQUEST_CACHE=y, then that key is placed in the +per-task cache, displacing the previous key. The cache is cleared on exit or +just prior to resumption of userspace. Only if all these fail does the whole thing fail with the highest priority error. Note that several errors may have come from LSM. diff --git a/include/linux/sched.h b/include/linux/sched.h index 11837410690f..e5f18857dd53 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -831,6 +831,11 @@ struct task_struct { /* Effective (overridable) subjective task credentials (COW): */ const struct cred __rcu *cred; +#ifdef CONFIG_KEYS + /* Cached requested key. */ + struct key *cached_requested_key; +#endif + /* * executable name, excluding path. * diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h index df20f8bdbfa3..81824467e6a6 100644 --- a/include/linux/tracehook.h +++ b/include/linux/tracehook.h @@ -187,6 +187,13 @@ static inline void tracehook_notify_resume(struct pt_regs *regs) if (unlikely(current->task_works)) task_work_run(); +#ifdef CONFIG_KEYS_REQUEST_CACHE + if (unlikely(current->cached_requested_key)) { + key_put(current->cached_requested_key); + current->cached_requested_key = NULL; + } +#endif + mem_cgroup_handle_over_high(); blkcg_maybe_throttle_current(); } diff --git a/kernel/cred.c b/kernel/cred.c index 3bd40de9e192..26da7e77098f 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -174,6 +174,11 @@ void exit_creds(struct task_struct *tsk) validate_creds(cred); alter_cred_subscribers(cred, -1); put_cred(cred); + +#ifdef CONFIG_KEYS_REQUEST_CACHE + key_put(current->cached_requested_key); + current->cached_requested_key = NULL; +#endif } /** @@ -327,6 +332,10 @@ int copy_creds(struct task_struct *p, unsigned long clone_flags) struct cred *new; int ret; +#ifdef CONFIG_KEYS_REQUEST_CACHE + p->cached_requested_key = NULL; +#endif + if ( #ifdef CONFIG_KEYS !p->cred->thread_keyring && diff --git a/security/keys/Kconfig b/security/keys/Kconfig index 6462e6654ccf..12f70b556d09 100644 --- a/security/keys/Kconfig +++ b/security/keys/Kconfig @@ -24,6 +24,24 @@ config KEYS_COMPAT def_bool y depends on COMPAT && KEYS +config KEYS_REQUEST_CACHE + bool "Enable temporary caching of the last request_key() result" + depends on KEYS + help + This option causes the result of the last successful request_key() + call that didn't upcall to the kernel to be cached temporarily in the + task_struct. The cache is cleared by exit and just prior to the + resumption of userspace. + + This allows the key used for multiple step processes where each step + wants to request a key that is likely the same as the one requested + by the last step to save on the searching. + + An example of such a process is a pathwalk through a network + filesystem in which each method needs to request an authentication + key. Pathwalk will call multiple methods for each dentry traversed + (permission, d_revalidate, lookup, getxattr, getacl, ...). + config PERSISTENT_KEYRINGS bool "Enable register of persistent per-UID keyrings" depends on KEYS diff --git a/security/keys/request_key.c b/security/keys/request_key.c index b4b3677657d6..f289982cb5db 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -22,6 +22,31 @@ #define key_negative_timeout 60 /* default timeout on a negative key's existence */ +static struct key *check_cached_key(struct keyring_search_context *ctx) +{ +#ifdef CONFIG_KEYS_REQUEST_CACHE + struct key *key = current->cached_requested_key; + + if (key && + ctx->match_data.cmp(key, &ctx->match_data) && + !(key->flags & ((1 << KEY_FLAG_INVALIDATED) | + (1 << KEY_FLAG_REVOKED)))) + return key_get(key); +#endif + return NULL; +} + +static void cache_requested_key(struct key *key) +{ +#ifdef CONFIG_KEYS_REQUEST_CACHE + struct task_struct *t = current; + + key_put(t->cached_requested_key); + t->cached_requested_key = key_get(key); + set_tsk_thread_flag(t, TIF_NOTIFY_RESUME); +#endif +} + /** * complete_request_key - Complete the construction of a key. * @authkey: The authorisation key. @@ -562,6 +587,10 @@ struct key *request_key_and_link(struct key_type *type, } } + key = check_cached_key(&ctx); + if (key) + return key; + /* search all the process keyrings for a key */ rcu_read_lock(); key_ref = search_process_keyrings_rcu(&ctx); @@ -587,6 +616,9 @@ struct key *request_key_and_link(struct key_type *type, goto error_free; } } + + /* Only cache the key on immediate success */ + cache_requested_key(key); } else if (PTR_ERR(key_ref) != -EAGAIN) { key = ERR_CAST(key_ref); } else { @@ -786,6 +818,10 @@ struct key *request_key_rcu(struct key_type *type, const char *description) kenter("%s,%s", type->name, description); + key = check_cached_key(&ctx); + if (key) + return key; + /* search all the process keyrings for a key */ key_ref = search_process_keyrings_rcu(&ctx); if (IS_ERR(key_ref)) { @@ -794,6 +830,7 @@ struct key *request_key_rcu(struct key_type *type, const char *description) key = ERR_PTR(-ENOKEY); } else { key = key_ref_to_ptr(key_ref); + cache_requested_key(key); } kleave(" = %p", key); From 3b8c4a08a471d56ecaaca939c972fdf5b8255629 Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 19 Jun 2019 16:10:16 +0100 Subject: [PATCH 6/6] keys: Kill off request_key_async{,_with_auxdata} Kill off request_key_async{,_with_auxdata}() as they're not currently used. Signed-off-by: David Howells --- Documentation/security/keys/core.rst | 32 ------------- Documentation/security/keys/request-key.rst | 23 +--------- include/linux/key.h | 11 ----- security/keys/request_key.c | 50 --------------------- 4 files changed, 2 insertions(+), 114 deletions(-) diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst index 003f1452a5b7..a0e245f9576f 100644 --- a/Documentation/security/keys/core.rst +++ b/Documentation/security/keys/core.rst @@ -1115,38 +1115,6 @@ payload contents" for more information. is a blob of length callout_len, if given (the length may be 0). - * A key can be requested asynchronously by calling one of:: - - struct key *request_key_async(const struct key_type *type, - const char *description, - const void *callout_info, - size_t callout_len); - - or:: - - struct key *request_key_async_with_auxdata(const struct key_type *type, - const char *description, - const char *callout_info, - size_t callout_len, - void *aux); - - which are asynchronous equivalents of request_key() and - request_key_with_auxdata() respectively. - - These two functions return with the key potentially still under - construction. To wait for construction completion, the following should be - called:: - - int wait_for_key_construction(struct key *key, bool intr); - - The function will wait for the key to finish being constructed and then - invokes key_validate() to return an appropriate value to indicate the state - of the key (0 indicates the key is usable). - - If intr is true, then the wait can be interrupted by a signal, in which - case error ERESTARTSYS will be returned. - - * To search for a key under RCU conditions, call:: struct key *request_key_rcu(const struct key_type *type, diff --git a/Documentation/security/keys/request-key.rst b/Documentation/security/keys/request-key.rst index 45049abdf290..5a210baa583a 100644 --- a/Documentation/security/keys/request-key.rst +++ b/Documentation/security/keys/request-key.rst @@ -21,21 +21,6 @@ or:: size_t callout_len, void *aux); -or:: - - struct key *request_key_async(const struct key_type *type, - const char *description, - const char *callout_info, - size_t callout_len); - -or:: - - struct key *request_key_async_with_auxdata(const struct key_type *type, - const char *description, - const char *callout_info, - size_t callout_len, - void *aux); - or:: struct key *request_key_rcu(const struct key_type *type, @@ -53,15 +38,11 @@ does not need to link the key to a keyring to prevent it from being immediately destroyed. The kernel interface returns a pointer directly to the key, and it's up to the caller to destroy the key. -The request_key*_with_auxdata() calls are like the in-kernel request_key*() -calls, except that they permit auxiliary data to be passed to the upcaller (the +The request_key_with_auxdata() calls is like the in-kernel request_key() call, +except that they permit auxiliary data to be passed to the upcaller (the default is NULL). This is only useful for those key types that define their own upcall mechanism rather than using /sbin/request-key. -The two async in-kernel calls may return keys that are still in the process of -being constructed. The two non-async ones will wait for construction to -complete first. - The request_key_rcu() call is like the in-kernel request_key() call, except that it doesn't check for keys that are under construction and doesn't attempt to construct missing keys. diff --git a/include/linux/key.h b/include/linux/key.h index 3604a554df99..4cd5669184f3 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -283,17 +283,6 @@ extern struct key *request_key_with_auxdata(struct key_type *type, size_t callout_len, void *aux); -extern struct key *request_key_async(struct key_type *type, - const char *description, - const void *callout_info, - size_t callout_len); - -extern struct key *request_key_async_with_auxdata(struct key_type *type, - const char *description, - const void *callout_info, - size_t callout_len, - void *aux); - extern int wait_for_key_construction(struct key *key, bool intr); extern int key_validate(const struct key *key); diff --git a/security/keys/request_key.c b/security/keys/request_key.c index f289982cb5db..36c55ef47b9e 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -739,56 +739,6 @@ struct key *request_key_with_auxdata(struct key_type *type, } EXPORT_SYMBOL(request_key_with_auxdata); -/* - * request_key_async - Request a key (allow async construction) - * @type: Type of key. - * @description: The searchable description of the key. - * @callout_info: The data to pass to the instantiation upcall (or NULL). - * @callout_len: The length of callout_info. - * - * As for request_key_and_link() except that it does not add the returned key - * to a keyring if found, new keys are always allocated in the user's quota and - * no auxiliary data can be passed. - * - * The caller should call wait_for_key_construction() to wait for the - * completion of the returned key if it is still undergoing construction. - */ -struct key *request_key_async(struct key_type *type, - const char *description, - const void *callout_info, - size_t callout_len) -{ - return request_key_and_link(type, description, callout_info, - callout_len, NULL, NULL, - KEY_ALLOC_IN_QUOTA); -} -EXPORT_SYMBOL(request_key_async); - -/* - * request a key with auxiliary data for the upcaller (allow async construction) - * @type: Type of key. - * @description: The searchable description of the key. - * @callout_info: The data to pass to the instantiation upcall (or NULL). - * @callout_len: The length of callout_info. - * @aux: Auxiliary data for the upcall. - * - * As for request_key_and_link() except that it does not add the returned key - * to a keyring if found and new keys are always allocated in the user's quota. - * - * The caller should call wait_for_key_construction() to wait for the - * completion of the returned key if it is still undergoing construction. - */ -struct key *request_key_async_with_auxdata(struct key_type *type, - const char *description, - const void *callout_info, - size_t callout_len, - void *aux) -{ - return request_key_and_link(type, description, callout_info, - callout_len, aux, NULL, KEY_ALLOC_IN_QUOTA); -} -EXPORT_SYMBOL(request_key_async_with_auxdata); - /** * request_key_rcu - Request key from RCU-read-locked context * @type: The type of key we want.