* [PATCH 1/9] keys: Simplify key description management [ver #4]
2019-06-19 16:06 [PATCH 0/9] keys: Namespacing [ver #4] David Howells
@ 2019-06-19 16:06 ` David Howells
2019-06-19 16:06 ` [PATCH 2/9] keys: Cache the hash value to avoid lots of recalculation " David Howells
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2019-06-19 16:06 UTC (permalink / raw)
To: ebiederm, keyrings
Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
vgoyal, linux-security-module, linux-fsdevel, linux-kernel
Simplify key description management by cramming the word containing the
length with the first few chars of the description also. This simplifies
the code that generates the index-key used by assoc_array. It should speed
up key searching a bit too.
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/linux/key.h | 14 ++++++++-
security/keys/internal.h | 6 ++++
security/keys/key.c | 2 +
security/keys/keyring.c | 70 +++++++++++++-------------------------------
security/keys/persistent.c | 1 +
5 files changed, 43 insertions(+), 50 deletions(-)
diff --git a/include/linux/key.h b/include/linux/key.h
index 4cd5669184f3..86ccc2d010f6 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -86,9 +86,20 @@ struct keyring_list;
struct keyring_name;
struct keyring_index_key {
+ union {
+ struct {
+#ifdef __LITTLE_ENDIAN /* Put desc_len at the LSB of x */
+ u8 desc_len;
+ char desc[sizeof(long) - 1]; /* First few chars of description */
+#else
+ char desc[sizeof(long) - 1]; /* First few chars of description */
+ u8 desc_len;
+#endif
+ };
+ unsigned long x;
+ };
struct key_type *type;
const char *description;
- size_t desc_len;
};
union key_payload {
@@ -202,6 +213,7 @@ struct key {
union {
struct keyring_index_key index_key;
struct {
+ unsigned long len_desc;
struct key_type *type; /* type of key */
char *description;
};
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 3d5c08db74d2..ee71c72fc5f0 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -90,6 +90,12 @@ extern struct mutex key_construction_mutex;
extern wait_queue_head_t request_key_conswq;
+static inline void key_set_index_key(struct keyring_index_key *index_key)
+{
+ size_t n = min_t(size_t, index_key->desc_len, sizeof(index_key->desc));
+ memcpy(index_key->desc, index_key->description, n);
+}
+
extern struct key_type *key_type_lookup(const char *type);
extern void key_type_put(struct key_type *ktype);
diff --git a/security/keys/key.c b/security/keys/key.c
index e792d65c0af8..0a3828f15f57 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -285,6 +285,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
if (!key->index_key.description)
goto no_memory_3;
+ key_set_index_key(&key->index_key);
refcount_set(&key->usage, 1);
init_rwsem(&key->sem);
@@ -868,6 +869,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
goto error_free_prep;
}
index_key.desc_len = strlen(index_key.description);
+ key_set_index_key(&index_key);
ret = __key_link_lock(keyring, &index_key);
if (ret < 0) {
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index afa6d4024c67..ebf52077598f 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -179,9 +179,9 @@ static unsigned long hash_key_type_and_desc(const struct keyring_index_key *inde
int n, desc_len = index_key->desc_len;
type = (unsigned long)index_key->type;
-
acc = mult_64x32_and_fold(type, desc_len + 13);
acc = mult_64x32_and_fold(acc, 9207);
+
for (;;) {
n = desc_len;
if (n <= 0)
@@ -215,23 +215,13 @@ static unsigned long hash_key_type_and_desc(const struct keyring_index_key *inde
/*
* Build the next index key chunk.
*
- * On 32-bit systems the index key is laid out as:
- *
- * 0 4 5 9...
- * hash desclen typeptr desc[]
- *
- * On 64-bit systems:
- *
- * 0 8 9 17...
- * hash desclen typeptr desc[]
- *
* We return it one word-sized chunk at a time.
*/
static unsigned long keyring_get_key_chunk(const void *data, int level)
{
const struct keyring_index_key *index_key = data;
unsigned long chunk = 0;
- long offset = 0;
+ const u8 *d;
int desc_len = index_key->desc_len, n = sizeof(chunk);
level /= ASSOC_ARRAY_KEY_CHUNK_SIZE;
@@ -239,33 +229,23 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
case 0:
return hash_key_type_and_desc(index_key);
case 1:
- return ((unsigned long)index_key->type << 8) | desc_len;
+ return index_key->x;
case 2:
- if (desc_len == 0)
- return (u8)((unsigned long)index_key->type >>
- (ASSOC_ARRAY_KEY_CHUNK_SIZE - 8));
- n--;
- offset = 1;
- /* fall through */
+ return (unsigned long)index_key->type;
default:
- offset += sizeof(chunk) - 1;
- offset += (level - 3) * sizeof(chunk);
- if (offset >= desc_len)
+ level -= 3;
+ if (desc_len <= sizeof(index_key->desc))
return 0;
- desc_len -= offset;
+
+ d = index_key->description + sizeof(index_key->desc);
+ d += level * sizeof(long);
+ desc_len -= sizeof(index_key->desc);
if (desc_len > n)
desc_len = n;
- offset += desc_len;
do {
chunk <<= 8;
- chunk |= ((u8*)index_key->description)[--offset];
+ chunk |= *d++;
} while (--desc_len > 0);
-
- if (level == 2) {
- chunk <<= 8;
- chunk |= (u8)((unsigned long)index_key->type >>
- (ASSOC_ARRAY_KEY_CHUNK_SIZE - 8));
- }
return chunk;
}
}
@@ -304,39 +284,28 @@ static int keyring_diff_objects(const void *object, const void *data)
seg_b = hash_key_type_and_desc(b);
if ((seg_a ^ seg_b) != 0)
goto differ;
+ level += ASSOC_ARRAY_KEY_CHUNK_SIZE / 8;
/* The number of bits contributed by the hash is controlled by a
* constant in the assoc_array headers. Everything else thereafter we
* can deal with as being machine word-size dependent.
*/
- level += ASSOC_ARRAY_KEY_CHUNK_SIZE / 8;
- seg_a = a->desc_len;
- seg_b = b->desc_len;
+ seg_a = a->x;
+ seg_b = b->x;
if ((seg_a ^ seg_b) != 0)
goto differ;
+ level += sizeof(unsigned long);
/* The next bit may not work on big endian */
- level++;
seg_a = (unsigned long)a->type;
seg_b = (unsigned long)b->type;
if ((seg_a ^ seg_b) != 0)
goto differ;
-
level += sizeof(unsigned long);
- if (a->desc_len == 0)
- goto same;
- i = 0;
- if (((unsigned long)a->description | (unsigned long)b->description) &
- (sizeof(unsigned long) - 1)) {
- do {
- seg_a = *(unsigned long *)(a->description + i);
- seg_b = *(unsigned long *)(b->description + i);
- if ((seg_a ^ seg_b) != 0)
- goto differ_plus_i;
- i += sizeof(unsigned long);
- } while (i < (a->desc_len & (sizeof(unsigned long) - 1)));
- }
+ i = sizeof(a->desc);
+ if (a->desc_len <= i)
+ goto same;
for (; i < a->desc_len; i++) {
seg_a = *(unsigned char *)(a->description + i);
@@ -662,6 +631,9 @@ static bool search_nested_keyrings(struct key *keyring,
BUG_ON((ctx->flags & STATE_CHECKS) == 0 ||
(ctx->flags & STATE_CHECKS) == STATE_CHECKS);
+ if (ctx->index_key.description)
+ key_set_index_key(&ctx->index_key);
+
/* Check to see if this top-level keyring is what we are looking for
* and whether it is valid or not.
*/
diff --git a/security/keys/persistent.c b/security/keys/persistent.c
index d0cb5b32eff7..fc29ec59efa7 100644
--- a/security/keys/persistent.c
+++ b/security/keys/persistent.c
@@ -87,6 +87,7 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
index_key.type = &key_type_keyring;
index_key.description = buf;
index_key.desc_len = sprintf(buf, "_persistent.%u", from_kuid(ns, uid));
+ key_set_index_key(&index_key);
if (ns->persistent_keyring_register) {
reg_ref = make_key_ref(ns->persistent_keyring_register, true);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/9] keys: Cache the hash value to avoid lots of recalculation [ver #4]
2019-06-19 16:06 [PATCH 0/9] keys: Namespacing [ver #4] David Howells
2019-06-19 16:06 ` [PATCH 1/9] keys: Simplify key description management " David Howells
@ 2019-06-19 16:06 ` David Howells
2019-06-19 16:06 ` [PATCH 3/9] keys: Add a 'recurse' flag for keyring searches " David Howells
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2019-06-19 16:06 UTC (permalink / raw)
To: ebiederm, keyrings
Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
vgoyal, linux-security-module, linux-fsdevel, linux-kernel
Cache the hash of the key's type and description in the index key so that
we're not recalculating it every time we look at a key during a search.
The hash function does a bunch of multiplications, so evading those is
probably worthwhile - especially as this is done for every key examined
during a search.
This also allows the methods used by assoc_array to get chunks of index-key
to be simplified.
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/linux/key.h | 3 +++
security/keys/internal.h | 8 +-------
security/keys/key.c | 2 +-
security/keys/keyring.c | 28 ++++++++++++++++++++--------
4 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/include/linux/key.h b/include/linux/key.h
index 86ccc2d010f6..fb2debcacea0 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -86,6 +86,8 @@ struct keyring_list;
struct keyring_name;
struct keyring_index_key {
+ /* [!] If this structure is altered, the union in struct key must change too! */
+ unsigned long hash; /* Hash value */
union {
struct {
#ifdef __LITTLE_ENDIAN /* Put desc_len at the LSB of x */
@@ -213,6 +215,7 @@ struct key {
union {
struct keyring_index_key index_key;
struct {
+ unsigned long hash;
unsigned long len_desc;
struct key_type *type; /* type of key */
char *description;
diff --git a/security/keys/internal.h b/security/keys/internal.h
index ee71c72fc5f0..4305414795ae 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -89,13 +89,7 @@ extern spinlock_t key_serial_lock;
extern struct mutex key_construction_mutex;
extern wait_queue_head_t request_key_conswq;
-
-static inline void key_set_index_key(struct keyring_index_key *index_key)
-{
- size_t n = min_t(size_t, index_key->desc_len, sizeof(index_key->desc));
- memcpy(index_key->desc, index_key->description, n);
-}
-
+extern void key_set_index_key(struct keyring_index_key *index_key);
extern struct key_type *key_type_lookup(const char *type);
extern void key_type_put(struct key_type *ktype);
diff --git a/security/keys/key.c b/security/keys/key.c
index 0a3828f15f57..9d52f2472a09 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -285,12 +285,12 @@ struct key *key_alloc(struct key_type *type, const char *desc,
key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
if (!key->index_key.description)
goto no_memory_3;
+ key->index_key.type = type;
key_set_index_key(&key->index_key);
refcount_set(&key->usage, 1);
init_rwsem(&key->sem);
lockdep_set_class(&key->sem, &type->lock_class);
- key->index_key.type = type;
key->user = user;
key->quotalen = quotalen;
key->datalen = type->def_datalen;
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index ebf52077598f..a5ee3b4d2eb8 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -168,7 +168,7 @@ static u64 mult_64x32_and_fold(u64 x, u32 y)
/*
* Hash a key type and description.
*/
-static unsigned long hash_key_type_and_desc(const struct keyring_index_key *index_key)
+static void hash_key_type_and_desc(struct keyring_index_key *index_key)
{
const unsigned level_shift = ASSOC_ARRAY_LEVEL_STEP;
const unsigned long fan_mask = ASSOC_ARRAY_FAN_MASK;
@@ -206,10 +206,22 @@ static unsigned long hash_key_type_and_desc(const struct keyring_index_key *inde
* zero for keyrings and non-zero otherwise.
*/
if (index_key->type != &key_type_keyring && (hash & fan_mask) == 0)
- return hash | (hash >> (ASSOC_ARRAY_KEY_CHUNK_SIZE - level_shift)) | 1;
- if (index_key->type == &key_type_keyring && (hash & fan_mask) != 0)
- return (hash + (hash << level_shift)) & ~fan_mask;
- return hash;
+ hash |= (hash >> (ASSOC_ARRAY_KEY_CHUNK_SIZE - level_shift)) | 1;
+ else if (index_key->type == &key_type_keyring && (hash & fan_mask) != 0)
+ hash = (hash + (hash << level_shift)) & ~fan_mask;
+ index_key->hash = hash;
+}
+
+/*
+ * Finalise an index key to include a part of the description actually in the
+ * index key and to add in the hash too.
+ */
+void key_set_index_key(struct keyring_index_key *index_key)
+{
+ size_t n = min_t(size_t, index_key->desc_len, sizeof(index_key->desc));
+ memcpy(index_key->desc, index_key->description, n);
+
+ hash_key_type_and_desc(index_key);
}
/*
@@ -227,7 +239,7 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
level /= ASSOC_ARRAY_KEY_CHUNK_SIZE;
switch (level) {
case 0:
- return hash_key_type_and_desc(index_key);
+ return index_key->hash;
case 1:
return index_key->x;
case 2:
@@ -280,8 +292,8 @@ static int keyring_diff_objects(const void *object, const void *data)
int level, i;
level = 0;
- seg_a = hash_key_type_and_desc(a);
- seg_b = hash_key_type_and_desc(b);
+ seg_a = a->hash;
+ seg_b = b->hash;
if ((seg_a ^ seg_b) != 0)
goto differ;
level += ASSOC_ARRAY_KEY_CHUNK_SIZE / 8;
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/9] keys: Add a 'recurse' flag for keyring searches [ver #4]
2019-06-19 16:06 [PATCH 0/9] keys: Namespacing [ver #4] David Howells
2019-06-19 16:06 ` [PATCH 1/9] keys: Simplify key description management " David Howells
2019-06-19 16:06 ` [PATCH 2/9] keys: Cache the hash value to avoid lots of recalculation " David Howells
@ 2019-06-19 16:06 ` David Howells
2019-06-19 16:06 ` [PATCH 4/9] keys: Namespace keyring names " David Howells
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2019-06-19 16:06 UTC (permalink / raw)
To: ebiederm, keyrings
Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
vgoyal, linux-security-module, linux-fsdevel, linux-kernel
Add a 'recurse' flag for keyring searches so that the flag can be omitted
and recursion disabled, thereby allowing just the nominated keyring to be
searched and none of the children.
Signed-off-by: David Howells <dhowells@redhat.com>
---
Documentation/security/keys/core.rst | 10 ++++++----
certs/blacklist.c | 2 +-
crypto/asymmetric_keys/asymmetric_type.c | 2 +-
include/linux/key.h | 3 ++-
lib/digsig.c | 2 +-
net/rxrpc/security.c | 2 +-
security/integrity/digsig_asymmetric.c | 4 ++--
security/keys/internal.h | 1 +
security/keys/keyctl.c | 2 +-
security/keys/keyring.c | 12 ++++++++++--
security/keys/proc.c | 3 ++-
security/keys/process_keys.c | 3 ++-
security/keys/request_key.c | 3 ++-
security/keys/request_key_auth.c | 3 ++-
14 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst
index a0e245f9576f..ae930ae9d590 100644
--- a/Documentation/security/keys/core.rst
+++ b/Documentation/security/keys/core.rst
@@ -1162,11 +1162,13 @@ payload contents" for more information.
key_ref_t keyring_search(key_ref_t keyring_ref,
const struct key_type *type,
- const char *description)
+ const char *description,
+ bool recurse)
- This searches the keyring tree specified for a matching key. Error ENOKEY
- is returned upon failure (use IS_ERR/PTR_ERR to determine). If successful,
- the returned key will need to be released.
+ This searches the specified keyring only (recurse == false) or keyring tree
+ (recurse == true) specified for a matching key. Error ENOKEY is returned
+ upon failure (use IS_ERR/PTR_ERR to determine). If successful, the returned
+ key will need to be released.
The possession attribute from the keyring reference is used to control
access through the permissions mask and is propagated to the returned key
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 3a507b9e2568..181cb7fa9540 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -128,7 +128,7 @@ int is_hash_blacklisted(const u8 *hash, size_t hash_len, const char *type)
*p = 0;
kref = keyring_search(make_key_ref(blacklist_keyring, true),
- &key_type_blacklist, buffer);
+ &key_type_blacklist, buffer, false);
if (!IS_ERR(kref)) {
key_ref_put(kref);
ret = -EKEYREJECTED;
diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 69a0788a7de5..084027ef3121 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -87,7 +87,7 @@ struct key *find_asymmetric_key(struct key *keyring,
pr_debug("Look up: \"%s\"\n", req);
ref = keyring_search(make_key_ref(keyring, 1),
- &key_type_asymmetric, req);
+ &key_type_asymmetric, req, true);
if (IS_ERR(ref))
pr_debug("Request for key '%s' err %ld\n", req, PTR_ERR(ref));
kfree(req);
diff --git a/include/linux/key.h b/include/linux/key.h
index fb2debcacea0..ff102731b3db 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -341,7 +341,8 @@ extern int keyring_clear(struct key *keyring);
extern key_ref_t keyring_search(key_ref_t keyring,
struct key_type *type,
- const char *description);
+ const char *description,
+ bool recurse);
extern int keyring_add_key(struct key *keyring,
struct key *key);
diff --git a/lib/digsig.c b/lib/digsig.c
index 3b0a579bdcdf..3782af401c68 100644
--- a/lib/digsig.c
+++ b/lib/digsig.c
@@ -221,7 +221,7 @@ int digsig_verify(struct key *keyring, const char *sig, int siglen,
/* search in specific keyring */
key_ref_t kref;
kref = keyring_search(make_key_ref(keyring, 1UL),
- &key_type_user, name);
+ &key_type_user, name, true);
if (IS_ERR(kref))
key = ERR_CAST(kref);
else
diff --git a/net/rxrpc/security.c b/net/rxrpc/security.c
index c4479afe8ae7..2cfc7125bc41 100644
--- a/net/rxrpc/security.c
+++ b/net/rxrpc/security.c
@@ -148,7 +148,7 @@ int rxrpc_init_server_conn_security(struct rxrpc_connection *conn)
/* look through the service's keyring */
kref = keyring_search(make_key_ref(rx->securities, 1UL),
- &key_type_rxrpc_s, kdesc);
+ &key_type_rxrpc_s, kdesc, true);
if (IS_ERR(kref)) {
read_unlock(&local->services_lock);
_leave(" = %ld [search]", PTR_ERR(kref));
diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
index 99080871eb9f..358f614811e8 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -39,7 +39,7 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
key_ref_t kref;
kref = keyring_search(make_key_ref(key, 1),
- &key_type_asymmetric, name);
+ &key_type_asymmetric, name, true);
if (!IS_ERR(kref)) {
pr_err("Key '%s' is in ima_blacklist_keyring\n", name);
return ERR_PTR(-EKEYREJECTED);
@@ -51,7 +51,7 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
key_ref_t kref;
kref = keyring_search(make_key_ref(keyring, 1),
- &key_type_asymmetric, name);
+ &key_type_asymmetric, name, true);
if (IS_ERR(kref))
key = ERR_CAST(kref);
else
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 4305414795ae..aa361299a3ec 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -127,6 +127,7 @@ struct keyring_search_context {
#define KEYRING_SEARCH_NO_CHECK_PERM 0x0008 /* Don't check permissions */
#define KEYRING_SEARCH_DETECT_TOO_DEEP 0x0010 /* Give an error on excessive depth */
#define KEYRING_SEARCH_SKIP_EXPIRED 0x0020 /* Ignore expired keys (intention to replace) */
+#define KEYRING_SEARCH_RECURSE 0x0040 /* Search child keyrings also */
int (*iterator)(const void *object, void *iterator_data);
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 9f418e66f067..169409b611b0 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -762,7 +762,7 @@ long keyctl_keyring_search(key_serial_t ringid,
}
/* do the search */
- key_ref = keyring_search(keyring_ref, ktype, description);
+ key_ref = keyring_search(keyring_ref, ktype, description, true);
if (IS_ERR(key_ref)) {
ret = PTR_ERR(key_ref);
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index a5ee3b4d2eb8..20891cd198f0 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -685,6 +685,9 @@ static bool search_nested_keyrings(struct key *keyring,
* Non-keyrings avoid the leftmost branch of the root entirely (root
* slots 1-15).
*/
+ if (!(ctx->flags & KEYRING_SEARCH_RECURSE))
+ goto not_this_keyring;
+
ptr = READ_ONCE(keyring->keys.root);
if (!ptr)
goto not_this_keyring;
@@ -885,13 +888,15 @@ key_ref_t keyring_search_rcu(key_ref_t keyring_ref,
* @keyring: The root of the keyring tree to be searched.
* @type: The type of keyring we want to find.
* @description: The name of the keyring we want to find.
+ * @recurse: True to search the children of @keyring also
*
* 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,
struct key_type *type,
- const char *description)
+ const char *description,
+ bool recurse)
{
struct keyring_search_context ctx = {
.index_key.type = type,
@@ -906,6 +911,8 @@ key_ref_t keyring_search(key_ref_t keyring,
key_ref_t key;
int ret;
+ if (recurse)
+ ctx.flags |= KEYRING_SEARCH_RECURSE;
if (type->match_preparse) {
ret = type->match_preparse(&ctx.match_data);
if (ret < 0)
@@ -1176,7 +1183,8 @@ static int keyring_detect_cycle(struct key *A, struct key *B)
.flags = (KEYRING_SEARCH_NO_STATE_CHECK |
KEYRING_SEARCH_NO_UPDATE_TIME |
KEYRING_SEARCH_NO_CHECK_PERM |
- KEYRING_SEARCH_DETECT_TOO_DEEP),
+ KEYRING_SEARCH_DETECT_TOO_DEEP |
+ KEYRING_SEARCH_RECURSE),
};
rcu_read_lock();
diff --git a/security/keys/proc.c b/security/keys/proc.c
index f081dceae3b9..b4f5ba56b9cb 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -170,7 +170,8 @@ static int proc_keys_show(struct seq_file *m, void *v)
.match_data.cmp = lookup_user_key_possessed,
.match_data.raw_data = key,
.match_data.lookup_type = KEYRING_SEARCH_LOOKUP_DIRECT,
- .flags = KEYRING_SEARCH_NO_STATE_CHECK,
+ .flags = (KEYRING_SEARCH_NO_STATE_CHECK |
+ KEYRING_SEARCH_RECURSE),
};
key_ref = make_key_ref(key, 0);
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index f8ffb06d0297..b07f768d23dc 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -531,7 +531,8 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
struct keyring_search_context ctx = {
.match_data.cmp = lookup_user_key_possessed,
.match_data.lookup_type = KEYRING_SEARCH_LOOKUP_DIRECT,
- .flags = KEYRING_SEARCH_NO_STATE_CHECK,
+ .flags = (KEYRING_SEARCH_NO_STATE_CHECK |
+ KEYRING_SEARCH_RECURSE),
};
struct request_key_auth *rka;
struct key *key;
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 36c55ef47b9e..1ffd3803ce29 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -569,7 +569,8 @@ struct key *request_key_and_link(struct key_type *type,
.match_data.raw_data = description,
.match_data.lookup_type = KEYRING_SEARCH_LOOKUP_DIRECT,
.flags = (KEYRING_SEARCH_DO_STATE_CHECK |
- KEYRING_SEARCH_SKIP_EXPIRED),
+ KEYRING_SEARCH_SKIP_EXPIRED |
+ KEYRING_SEARCH_RECURSE),
};
struct key *key;
key_ref_t key_ref;
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 99ed7a8a273d..f613987e8a63 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -252,7 +252,8 @@ struct key *key_get_instantiation_authkey(key_serial_t target_id)
.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,
+ .flags = (KEYRING_SEARCH_DO_STATE_CHECK |
+ KEYRING_SEARCH_RECURSE),
};
struct key *authkey;
key_ref_t authkey_ref;
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/9] keys: Namespace keyring names [ver #4]
2019-06-19 16:06 [PATCH 0/9] keys: Namespacing [ver #4] David Howells
` (2 preceding siblings ...)
2019-06-19 16:06 ` [PATCH 3/9] keys: Add a 'recurse' flag for keyring searches " David Howells
@ 2019-06-19 16:06 ` David Howells
2019-06-19 16:06 ` [PATCH 5/9] keys: Move the user and user-session keyrings to the user_namespace " David Howells
2019-06-19 16:09 ` [PATCH 0/9] keys: Namespacing " David Howells
5 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2019-06-19 16:06 UTC (permalink / raw)
To: ebiederm, keyrings
Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
vgoyal, linux-security-module, linux-fsdevel, linux-kernel
Keyring names are held in a single global list that any process can pick
from by means of keyctl_join_session_keyring (provided the keyring grants
Search permission). This isn't very container friendly, however.
Make the following changes:
(1) Make default session, process and thread keyring names begin with a
'.' instead of '_'.
(2) Keyrings whose names begin with a '.' aren't added to the list. Such
keyrings are system specials.
(3) Replace the global list with per-user_namespace lists. A keyring adds
its name to the list for the user_namespace that it is currently in.
(4) When a user_namespace is deleted, it just removes itself from the
keyring name list.
The global keyring_name_lock is retained for accessing the name lists.
This allows (4) to work.
This can be tested by:
# keyctl newring foo @s
995906392
# unshare -U
$ keyctl show
...
995906392 --alswrv 65534 65534 \_ keyring: foo
...
$ keyctl session foo
Joined session keyring: 935622349
As can be seen, a new session keyring was created.
The capability bit KEYCTL_CAPS1_NS_KEYRING_NAME is set if the kernel is
employing this feature.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Eric W. Biederman <ebiederm@xmission.com>
---
include/linux/key.h | 2 +
include/linux/user_namespace.h | 5 ++
include/uapi/linux/keyctl.h | 1
kernel/user.c | 3 +
kernel/user_namespace.c | 7 ++-
security/keys/keyctl.c | 1
security/keys/keyring.c | 99 +++++++++++++++++-----------------------
7 files changed, 59 insertions(+), 59 deletions(-)
diff --git a/include/linux/key.h b/include/linux/key.h
index ff102731b3db..ae1177302d70 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -361,6 +361,7 @@ extern void key_set_timeout(struct key *, unsigned);
extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
key_perm_t perm);
+extern void key_free_user_ns(struct user_namespace *);
/*
* The permissions required on a key that we're looking up.
@@ -434,6 +435,7 @@ extern void key_init(void);
#define key_fsuid_changed(c) do { } while(0)
#define key_fsgid_changed(c) do { } while(0)
#define key_init() do { } while(0)
+#define key_free_user_ns(ns) do { } while(0)
#endif /* CONFIG_KEYS */
#endif /* __KERNEL__ */
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index d6b74b91096b..90457015fa3f 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -64,6 +64,11 @@ struct user_namespace {
struct ns_common ns;
unsigned long flags;
+#ifdef CONFIG_KEYS
+ /* List of joinable keyrings in this namespace */
+ struct list_head keyring_name_list;
+#endif
+
/* Register of per-UID persistent keyrings for this namespace */
#ifdef CONFIG_PERSISTENT_KEYRINGS
struct key *persistent_keyring_register;
diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index 551b5814f53e..35b405034674 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -128,5 +128,6 @@ struct keyctl_pkey_params {
#define KEYCTL_CAPS0_INVALIDATE 0x20 /* KEYCTL_INVALIDATE supported */
#define KEYCTL_CAPS0_RESTRICT_KEYRING 0x40 /* KEYCTL_RESTRICT_KEYRING supported */
#define KEYCTL_CAPS0_MOVE 0x80 /* KEYCTL_MOVE supported */
+#define KEYCTL_CAPS1_NS_KEYRING_NAME 0x01 /* Keyring names are per-user_namespace */
#endif /* _LINUX_KEYCTL_H */
diff --git a/kernel/user.c b/kernel/user.c
index 88b834f0eebc..50979fd1b7aa 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -62,6 +62,9 @@ struct user_namespace init_user_ns = {
.ns.ops = &userns_operations,
#endif
.flags = USERNS_INIT_FLAGS,
+#ifdef CONFIG_KEYS
+ .keyring_name_list = LIST_HEAD_INIT(init_user_ns.keyring_name_list),
+#endif
#ifdef CONFIG_PERSISTENT_KEYRINGS
.persistent_keyring_register_sem =
__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 923414a246e9..bda6e890ad88 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -133,6 +133,9 @@ int create_user_ns(struct cred *new)
ns->flags = parent_ns->flags;
mutex_unlock(&userns_state_mutex);
+#ifdef CONFIG_KEYS
+ INIT_LIST_HEAD(&ns->keyring_name_list);
+#endif
#ifdef CONFIG_PERSISTENT_KEYRINGS
init_rwsem(&ns->persistent_keyring_register_sem);
#endif
@@ -196,9 +199,7 @@ static void free_user_ns(struct work_struct *work)
kfree(ns->projid_map.reverse);
}
retire_userns_sysctls(ns);
-#ifdef CONFIG_PERSISTENT_KEYRINGS
- key_put(ns->persistent_keyring_register);
-#endif
+ key_free_user_ns(ns);
ns_free_inum(&ns->ns);
kmem_cache_free(user_ns_cachep, ns);
dec_user_namespaces(ucounts);
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 169409b611b0..37d2940cb10a 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -40,6 +40,7 @@ static const unsigned char keyrings_capabilities[1] = {
KEYCTL_CAPS0_RESTRICT_KEYRING |
KEYCTL_CAPS0_MOVE
),
+ [1] = (KEYCTL_CAPS1_NS_KEYRING_NAME),
};
static int key_get_type_from_user(char *type,
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 20891cd198f0..fe851292509e 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -16,6 +16,7 @@
#include <linux/security.h>
#include <linux/seq_file.h>
#include <linux/err.h>
+#include <linux/user_namespace.h>
#include <keys/keyring-type.h>
#include <keys/user-type.h>
#include <linux/assoc_array_priv.h>
@@ -28,11 +29,6 @@
*/
#define KEYRING_SEARCH_MAX_DEPTH 6
-/*
- * We keep all named keyrings in a hash to speed looking them up.
- */
-#define KEYRING_NAME_HASH_SIZE (1 << 5)
-
/*
* We mark pointers we pass to the associative array with bit 1 set if
* they're keyrings and clear otherwise.
@@ -55,17 +51,20 @@ static inline void *keyring_key_to_ptr(struct key *key)
return key;
}
-static struct list_head keyring_name_hash[KEYRING_NAME_HASH_SIZE];
static DEFINE_RWLOCK(keyring_name_lock);
-static inline unsigned keyring_hash(const char *desc)
+/*
+ * Clean up the bits of user_namespace that belong to us.
+ */
+void key_free_user_ns(struct user_namespace *ns)
{
- unsigned bucket = 0;
-
- for (; *desc; desc++)
- bucket += (unsigned char)*desc;
+ write_lock(&keyring_name_lock);
+ list_del_init(&ns->keyring_name_list);
+ write_unlock(&keyring_name_lock);
- return bucket & (KEYRING_NAME_HASH_SIZE - 1);
+#ifdef CONFIG_PERSISTENT_KEYRINGS
+ key_put(ns->persistent_keyring_register);
+#endif
}
/*
@@ -104,23 +103,17 @@ static DEFINE_MUTEX(keyring_serialise_link_lock);
/*
* Publish the name of a keyring so that it can be found by name (if it has
- * one).
+ * one and it doesn't begin with a dot).
*/
static void keyring_publish_name(struct key *keyring)
{
- int bucket;
-
- if (keyring->description) {
- bucket = keyring_hash(keyring->description);
+ struct user_namespace *ns = current_user_ns();
+ if (keyring->description &&
+ keyring->description[0] &&
+ keyring->description[0] != '.') {
write_lock(&keyring_name_lock);
-
- if (!keyring_name_hash[bucket].next)
- INIT_LIST_HEAD(&keyring_name_hash[bucket]);
-
- list_add_tail(&keyring->name_link,
- &keyring_name_hash[bucket]);
-
+ list_add_tail(&keyring->name_link, &ns->keyring_name_list);
write_unlock(&keyring_name_lock);
}
}
@@ -1097,50 +1090,44 @@ key_ref_t find_key_to_update(key_ref_t keyring_ref,
*/
struct key *find_keyring_by_name(const char *name, bool uid_keyring)
{
+ struct user_namespace *ns = current_user_ns();
struct key *keyring;
- int bucket;
if (!name)
return ERR_PTR(-EINVAL);
- bucket = keyring_hash(name);
-
read_lock(&keyring_name_lock);
- if (keyring_name_hash[bucket].next) {
- /* search this hash bucket for a keyring with a matching name
- * that's readable and that hasn't been revoked */
- list_for_each_entry(keyring,
- &keyring_name_hash[bucket],
- name_link
- ) {
- if (!kuid_has_mapping(current_user_ns(), keyring->user->uid))
- continue;
-
- if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
- continue;
+ /* Search this hash bucket for a keyring with a matching name that
+ * grants Search permission and that hasn't been revoked
+ */
+ list_for_each_entry(keyring, &ns->keyring_name_list, name_link) {
+ if (!kuid_has_mapping(ns, keyring->user->uid))
+ continue;
- if (strcmp(keyring->description, name) != 0)
- continue;
+ if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
+ continue;
- if (uid_keyring) {
- if (!test_bit(KEY_FLAG_UID_KEYRING,
- &keyring->flags))
- continue;
- } else {
- if (key_permission(make_key_ref(keyring, 0),
- KEY_NEED_SEARCH) < 0)
- continue;
- }
+ if (strcmp(keyring->description, name) != 0)
+ continue;
- /* we've got a match but we might end up racing with
- * key_cleanup() if the keyring is currently 'dead'
- * (ie. it has a zero usage count) */
- if (!refcount_inc_not_zero(&keyring->usage))
+ if (uid_keyring) {
+ if (!test_bit(KEY_FLAG_UID_KEYRING,
+ &keyring->flags))
+ continue;
+ } else {
+ if (key_permission(make_key_ref(keyring, 0),
+ KEY_NEED_SEARCH) < 0)
continue;
- keyring->last_used_at = ktime_get_real_seconds();
- goto out;
}
+
+ /* we've got a match but we might end up racing with
+ * key_cleanup() if the keyring is currently 'dead'
+ * (ie. it has a zero usage count) */
+ if (!refcount_inc_not_zero(&keyring->usage))
+ continue;
+ keyring->last_used_at = ktime_get_real_seconds();
+ goto out;
}
keyring = ERR_PTR(-ENOKEY);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/9] keys: Move the user and user-session keyrings to the user_namespace [ver #4]
2019-06-19 16:06 [PATCH 0/9] keys: Namespacing [ver #4] David Howells
` (3 preceding siblings ...)
2019-06-19 16:06 ` [PATCH 4/9] keys: Namespace keyring names " David Howells
@ 2019-06-19 16:06 ` David Howells
2019-06-19 16:09 ` [PATCH 0/9] keys: Namespacing " David Howells
5 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2019-06-19 16:06 UTC (permalink / raw)
To: ebiederm, keyrings
Cc: linux-cifs, linux-nfs, netdev, linux-afs, dhowells, dwalsh,
vgoyal, linux-security-module, linux-fsdevel, linux-kernel
Move the user and user-session keyrings to the user_namespace struct rather
than pinning them from the user_struct struct. This prevents these
keyrings from propagating across user-namespaces boundaries with regard to
the KEY_SPEC_* flags, thereby making them more useful in a containerised
environment.
The issue is that a single user_struct may be represent UIDs in several
different namespaces.
The way the patch does this is by attaching a 'register keyring' in each
user_namespace and then sticking the user and user-session keyrings into
that. It can then be searched to retrieve them.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jann Horn <jannh@google.com>
---
include/linux/sched/user.h | 14 --
include/linux/user_namespace.h | 9 +
kernel/user.c | 7 -
kernel/user_namespace.c | 4 -
security/keys/internal.h | 3
security/keys/keyring.c | 1
security/keys/persistent.c | 8 +
security/keys/process_keys.c | 259 ++++++++++++++++++++++++++--------------
security/keys/request_key.c | 20 ++-
9 files changed, 196 insertions(+), 129 deletions(-)
diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h
index 468d2565a9fe..917d88edb7b9 100644
--- a/include/linux/sched/user.h
+++ b/include/linux/sched/user.h
@@ -7,8 +7,6 @@
#include <linux/refcount.h>
#include <linux/ratelimit.h>
-struct key;
-
/*
* Some day this will be a full-fledged user tracking system..
*/
@@ -30,18 +28,6 @@ struct user_struct {
unsigned long unix_inflight; /* How many files in flight in unix sockets */
atomic_long_t pipe_bufs; /* how many pages are allocated in pipe buffers */
-#ifdef CONFIG_KEYS
- /*
- * These pointers can only change from NULL to a non-NULL value once.
- * Writes are protected by key_user_keyring_mutex.
- * Unlocked readers should use READ_ONCE() unless they know that
- * install_user_keyrings() has been called successfully (which sets
- * these members to non-NULL values, preventing further modifications).
- */
- struct key *uid_keyring; /* UID specific keyring */
- struct key *session_keyring; /* UID's default session keyring */
-#endif
-
/* Hash table maintenance information */
struct hlist_node uidhash_node;
kuid_t uid;
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 90457015fa3f..fb9f4f799554 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -65,14 +65,19 @@ struct user_namespace {
unsigned long flags;
#ifdef CONFIG_KEYS
- /* List of joinable keyrings in this namespace */
+ /* List of joinable keyrings in this namespace. Modification access of
+ * these pointers is controlled by keyring_sem. Once
+ * user_keyring_register is set, it won't be changed, so it can be
+ * accessed directly with READ_ONCE().
+ */
struct list_head keyring_name_list;
+ struct key *user_keyring_register;
+ struct rw_semaphore keyring_sem;
#endif
/* Register of per-UID persistent keyrings for this namespace */
#ifdef CONFIG_PERSISTENT_KEYRINGS
struct key *persistent_keyring_register;
- struct rw_semaphore persistent_keyring_register_sem;
#endif
struct work_struct work;
#ifdef CONFIG_SYSCTL
diff --git a/kernel/user.c b/kernel/user.c
index 50979fd1b7aa..f8519b62cf9a 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -64,10 +64,7 @@ struct user_namespace init_user_ns = {
.flags = USERNS_INIT_FLAGS,
#ifdef CONFIG_KEYS
.keyring_name_list = LIST_HEAD_INIT(init_user_ns.keyring_name_list),
-#endif
-#ifdef CONFIG_PERSISTENT_KEYRINGS
- .persistent_keyring_register_sem =
- __RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
+ .keyring_sem = __RWSEM_INITIALIZER(init_user_ns.keyring_sem),
#endif
};
EXPORT_SYMBOL_GPL(init_user_ns);
@@ -143,8 +140,6 @@ static void free_user(struct user_struct *up, unsigned long flags)
{
uid_hash_remove(up);
spin_unlock_irqrestore(&uidhash_lock, flags);
- key_put(up->uid_keyring);
- key_put(up->session_keyring);
kmem_cache_free(uid_cachep, up);
}
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index bda6e890ad88..c87c2ecc7085 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -135,9 +135,7 @@ int create_user_ns(struct cred *new)
#ifdef CONFIG_KEYS
INIT_LIST_HEAD(&ns->keyring_name_list);
-#endif
-#ifdef CONFIG_PERSISTENT_KEYRINGS
- init_rwsem(&ns->persistent_keyring_register_sem);
+ init_rwsem(&ns->keyring_sem);
#endif
ret = -ENOMEM;
if (!setup_userns_sysctls(ns))
diff --git a/security/keys/internal.h b/security/keys/internal.h
index aa361299a3ec..d3a9439e2386 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -148,7 +148,8 @@ 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);
-extern int install_user_keyrings(void);
+extern int look_up_user_keyrings(struct key **, struct key **);
+extern struct key *get_user_session_keyring_rcu(const struct cred *);
extern int install_thread_keyring_to_cred(struct cred *);
extern int install_process_keyring_to_cred(struct cred *);
extern int install_session_keyring_to_cred(struct cred *, struct key *);
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index fe851292509e..3663e5168583 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -62,6 +62,7 @@ void key_free_user_ns(struct user_namespace *ns)
list_del_init(&ns->keyring_name_list);
write_unlock(&keyring_name_lock);
+ key_put(ns->user_keyring_register);
#ifdef CONFIG_PERSISTENT_KEYRINGS
key_put(ns->persistent_keyring_register);
#endif
diff --git a/security/keys/persistent.c b/security/keys/persistent.c
index fc29ec59efa7..90303fe4a394 100644
--- a/security/keys/persistent.c
+++ b/security/keys/persistent.c
@@ -91,9 +91,9 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
if (ns->persistent_keyring_register) {
reg_ref = make_key_ref(ns->persistent_keyring_register, true);
- down_read(&ns->persistent_keyring_register_sem);
+ down_read(&ns->keyring_sem);
persistent_ref = find_key_to_update(reg_ref, &index_key);
- up_read(&ns->persistent_keyring_register_sem);
+ up_read(&ns->keyring_sem);
if (persistent_ref)
goto found;
@@ -102,9 +102,9 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
/* It wasn't in the register, so we'll need to create it. We might
* also need to create the register.
*/
- down_write(&ns->persistent_keyring_register_sem);
+ down_write(&ns->keyring_sem);
persistent_ref = key_create_persistent(ns, uid, &index_key);
- up_write(&ns->persistent_keyring_register_sem);
+ up_write(&ns->keyring_sem);
if (!IS_ERR(persistent_ref))
goto found;
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index b07f768d23dc..f74d64215942 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -19,15 +19,13 @@
#include <linux/security.h>
#include <linux/user_namespace.h>
#include <linux/uaccess.h>
+#include <linux/init_task.h>
#include <keys/request_key_auth-type.h>
#include "internal.h"
/* Session keyring create vs join semaphore */
static DEFINE_MUTEX(key_session_mutex);
-/* User keyring creation semaphore */
-static DEFINE_MUTEX(key_user_keyring_mutex);
-
/* The root user's tracking struct */
struct key_user root_key_user = {
.usage = REFCOUNT_INIT(3),
@@ -39,98 +37,185 @@ struct key_user root_key_user = {
};
/*
- * Install the user and user session keyrings for the current process's UID.
+ * Get or create a user register keyring.
+ */
+static struct key *get_user_register(struct user_namespace *user_ns)
+{
+ struct key *reg_keyring = READ_ONCE(user_ns->user_keyring_register);
+
+ if (reg_keyring)
+ return reg_keyring;
+
+ down_write(&user_ns->keyring_sem);
+
+ /* Make sure there's a register keyring. It gets owned by the
+ * user_namespace's owner.
+ */
+ reg_keyring = user_ns->user_keyring_register;
+ if (!reg_keyring) {
+ reg_keyring = keyring_alloc(".user_reg",
+ user_ns->owner, INVALID_GID,
+ &init_cred,
+ KEY_POS_WRITE | KEY_POS_SEARCH |
+ KEY_USR_VIEW | KEY_USR_READ,
+ 0,
+ NULL, NULL);
+ if (!IS_ERR(reg_keyring))
+ smp_store_release(&user_ns->user_keyring_register,
+ reg_keyring);
+ }
+
+ up_write(&user_ns->keyring_sem);
+
+ /* We don't return a ref since the keyring is pinned by the user_ns */
+ return reg_keyring;
+}
+
+/*
+ * Look up the user and user session keyrings for the current process's UID,
+ * creating them if they don't exist.
*/
-int install_user_keyrings(void)
+int look_up_user_keyrings(struct key **_user_keyring,
+ struct key **_user_session_keyring)
{
- struct user_struct *user;
- const struct cred *cred;
- struct key *uid_keyring, *session_keyring;
+ const struct cred *cred = current_cred();
+ struct user_namespace *user_ns = current_user_ns();
+ struct key *reg_keyring, *uid_keyring, *session_keyring;
key_perm_t user_keyring_perm;
+ key_ref_t uid_keyring_r, session_keyring_r;
+ uid_t uid = from_kuid(user_ns, cred->user->uid);
char buf[20];
int ret;
- uid_t uid;
user_keyring_perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_ALL;
- cred = current_cred();
- user = cred->user;
- uid = from_kuid(cred->user_ns, user->uid);
- kenter("%p{%u}", user, uid);
+ kenter("%u", uid);
- if (READ_ONCE(user->uid_keyring) && READ_ONCE(user->session_keyring)) {
- kleave(" = 0 [exist]");
- return 0;
- }
+ reg_keyring = get_user_register(user_ns);
+ if (IS_ERR(reg_keyring))
+ return PTR_ERR(reg_keyring);
- mutex_lock(&key_user_keyring_mutex);
+ down_write(&user_ns->keyring_sem);
ret = 0;
- if (!user->uid_keyring) {
- /* get the UID-specific keyring
- * - there may be one in existence already as it may have been
- * pinned by a session, but the user_struct pointing to it
- * may have been destroyed by setuid */
- sprintf(buf, "_uid.%u", uid);
-
- uid_keyring = find_keyring_by_name(buf, true);
+ /* Get the user keyring. Note that there may be one in existence
+ * already as it may have been pinned by a session, but the user_struct
+ * pointing to it may have been destroyed by setuid.
+ */
+ snprintf(buf, sizeof(buf), "_uid.%u", uid);
+ uid_keyring_r = keyring_search(make_key_ref(reg_keyring, true),
+ &key_type_keyring, buf, false);
+ kdebug("_uid %p", uid_keyring_r);
+ if (uid_keyring_r == ERR_PTR(-EAGAIN)) {
+ uid_keyring = keyring_alloc(buf, cred->user->uid, INVALID_GID,
+ cred, user_keyring_perm,
+ KEY_ALLOC_UID_KEYRING |
+ KEY_ALLOC_IN_QUOTA,
+ NULL, reg_keyring);
if (IS_ERR(uid_keyring)) {
- uid_keyring = keyring_alloc(buf, user->uid, INVALID_GID,
- cred, user_keyring_perm,
- KEY_ALLOC_UID_KEYRING |
- KEY_ALLOC_IN_QUOTA,
- NULL, NULL);
- if (IS_ERR(uid_keyring)) {
- ret = PTR_ERR(uid_keyring);
- goto error;
- }
+ ret = PTR_ERR(uid_keyring);
+ goto error;
}
+ } else if (IS_ERR(uid_keyring_r)) {
+ ret = PTR_ERR(uid_keyring_r);
+ goto error;
+ } else {
+ uid_keyring = key_ref_to_ptr(uid_keyring_r);
+ }
- /* get a default session keyring (which might also exist
- * already) */
- sprintf(buf, "_uid_ses.%u", uid);
-
- session_keyring = find_keyring_by_name(buf, true);
+ /* Get a default session keyring (which might also exist already) */
+ snprintf(buf, sizeof(buf), "_uid_ses.%u", uid);
+ session_keyring_r = keyring_search(make_key_ref(reg_keyring, true),
+ &key_type_keyring, buf, false);
+ kdebug("_uid_ses %p", session_keyring_r);
+ if (session_keyring_r == ERR_PTR(-EAGAIN)) {
+ session_keyring = keyring_alloc(buf, cred->user->uid, INVALID_GID,
+ cred, user_keyring_perm,
+ KEY_ALLOC_UID_KEYRING |
+ KEY_ALLOC_IN_QUOTA,
+ NULL, NULL);
if (IS_ERR(session_keyring)) {
- session_keyring =
- keyring_alloc(buf, user->uid, INVALID_GID,
- cred, user_keyring_perm,
- KEY_ALLOC_UID_KEYRING |
- KEY_ALLOC_IN_QUOTA,
- NULL, NULL);
- if (IS_ERR(session_keyring)) {
- ret = PTR_ERR(session_keyring);
- goto error_release;
- }
-
- /* we install a link from the user session keyring to
- * the user keyring */
- ret = key_link(session_keyring, uid_keyring);
- if (ret < 0)
- goto error_release_both;
+ ret = PTR_ERR(session_keyring);
+ goto error_release;
}
- /* install the keyrings */
- /* paired with READ_ONCE() */
- smp_store_release(&user->uid_keyring, uid_keyring);
- /* paired with READ_ONCE() */
- smp_store_release(&user->session_keyring, session_keyring);
+ /* We install a link from the user session keyring to
+ * the user keyring.
+ */
+ ret = key_link(session_keyring, uid_keyring);
+ if (ret < 0)
+ goto error_release_session;
+
+ /* And only then link the user-session keyring to the
+ * register.
+ */
+ ret = key_link(reg_keyring, session_keyring);
+ if (ret < 0)
+ goto error_release_session;
+ } else if (IS_ERR(session_keyring_r)) {
+ ret = PTR_ERR(session_keyring_r);
+ goto error_release;
+ } else {
+ session_keyring = key_ref_to_ptr(session_keyring_r);
}
- mutex_unlock(&key_user_keyring_mutex);
+ up_write(&user_ns->keyring_sem);
+
+ if (_user_session_keyring)
+ *_user_session_keyring = session_keyring;
+ else
+ key_put(session_keyring);
+ if (_user_keyring)
+ *_user_keyring = uid_keyring;
+ else
+ key_put(uid_keyring);
kleave(" = 0");
return 0;
-error_release_both:
+error_release_session:
key_put(session_keyring);
error_release:
key_put(uid_keyring);
error:
- mutex_unlock(&key_user_keyring_mutex);
+ up_write(&user_ns->keyring_sem);
kleave(" = %d", ret);
return ret;
}
+/*
+ * Get the user session keyring if it exists, but don't create it if it
+ * doesn't.
+ */
+struct key *get_user_session_keyring_rcu(const struct cred *cred)
+{
+ struct key *reg_keyring = READ_ONCE(cred->user_ns->user_keyring_register);
+ key_ref_t session_keyring_r;
+ char buf[20];
+
+ struct keyring_search_context ctx = {
+ .index_key.type = &key_type_keyring,
+ .index_key.description = buf,
+ .cred = cred,
+ .match_data.cmp = key_default_cmp,
+ .match_data.raw_data = buf,
+ .match_data.lookup_type = KEYRING_SEARCH_LOOKUP_DIRECT,
+ .flags = KEYRING_SEARCH_DO_STATE_CHECK,
+ };
+
+ if (!reg_keyring)
+ return NULL;
+
+ ctx.index_key.desc_len = snprintf(buf, sizeof(buf), "_uid_ses.%u",
+ from_kuid(cred->user_ns,
+ cred->user->uid));
+
+ session_keyring_r = keyring_search_rcu(make_key_ref(reg_keyring, true),
+ &ctx);
+ if (IS_ERR(session_keyring_r))
+ return NULL;
+ return key_ref_to_ptr(session_keyring_r);
+}
+
/*
* Install a thread keyring to the given credentials struct if it didn't have
* one already. This is allowed to overrun the quota.
@@ -340,6 +425,7 @@ void key_fsgid_changed(struct cred *new_cred)
*/
key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx)
{
+ struct key *user_session;
key_ref_t key_ref, ret, err;
const struct cred *cred = ctx->cred;
@@ -415,10 +501,11 @@ key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx)
}
}
/* or search the user-session keyring */
- else if (READ_ONCE(cred->user->session_keyring)) {
- key_ref = keyring_search_rcu(
- make_key_ref(READ_ONCE(cred->user->session_keyring), 1),
- ctx);
+ else if ((user_session = get_user_session_keyring_rcu(cred))) {
+ key_ref = keyring_search_rcu(make_key_ref(user_session, 1),
+ ctx);
+ key_put(user_session);
+
if (!IS_ERR(key_ref))
goto found;
@@ -535,7 +622,7 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
KEYRING_SEARCH_RECURSE),
};
struct request_key_auth *rka;
- struct key *key;
+ struct key *key, *user_session;
key_ref_t key_ref, skey_ref;
int ret;
@@ -584,20 +671,20 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
if (!ctx.cred->session_keyring) {
/* always install a session keyring upon access if one
* doesn't exist yet */
- ret = install_user_keyrings();
+ ret = look_up_user_keyrings(NULL, &user_session);
if (ret < 0)
goto error;
if (lflags & KEY_LOOKUP_CREATE)
ret = join_session_keyring(NULL);
else
- ret = install_session_keyring(
- ctx.cred->user->session_keyring);
+ ret = install_session_keyring(user_session);
+ key_put(user_session);
if (ret < 0)
goto error;
goto reget_creds;
- } else if (ctx.cred->session_keyring ==
- READ_ONCE(ctx.cred->user->session_keyring) &&
+ } else if (test_bit(KEY_FLAG_UID_KEYRING,
+ &ctx.cred->session_keyring->flags) &&
lflags & KEY_LOOKUP_CREATE) {
ret = join_session_keyring(NULL);
if (ret < 0)
@@ -611,26 +698,16 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
break;
case KEY_SPEC_USER_KEYRING:
- if (!READ_ONCE(ctx.cred->user->uid_keyring)) {
- ret = install_user_keyrings();
- if (ret < 0)
- goto error;
- }
-
- key = ctx.cred->user->uid_keyring;
- __key_get(key);
+ ret = look_up_user_keyrings(&key, NULL);
+ if (ret < 0)
+ goto error;
key_ref = make_key_ref(key, 1);
break;
case KEY_SPEC_USER_SESSION_KEYRING:
- if (!READ_ONCE(ctx.cred->user->session_keyring)) {
- ret = install_user_keyrings();
- if (ret < 0)
- goto error;
- }
-
- key = ctx.cred->user->session_keyring;
- __key_get(key);
+ ret = look_up_user_keyrings(NULL, &key);
+ if (ret < 0)
+ goto error;
key_ref = make_key_ref(key, 1);
break;
@@ -879,7 +956,7 @@ void key_change_session_keyring(struct callback_head *twork)
*/
static int __init init_root_keyring(void)
{
- return install_user_keyrings();
+ return look_up_user_keyrings(NULL, NULL);
}
late_initcall(init_root_keyring);
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 1ffd3803ce29..9201ca96c4df 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -121,7 +121,7 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
struct request_key_auth *rka = get_request_key_auth(authkey);
const struct cred *cred = current_cred();
key_serial_t prkey, sskey;
- struct key *key = rka->target_key, *keyring, *session;
+ struct key *key = rka->target_key, *keyring, *session, *user_session;
char *argv[9], *envp[3], uid_str[12], gid_str[12];
char key_str[12], keyring_str[3][12];
char desc[20];
@@ -129,9 +129,9 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
kenter("{%d},{%d},%s", key->serial, authkey->serial, rka->op);
- ret = install_user_keyrings();
+ ret = look_up_user_keyrings(NULL, &user_session);
if (ret < 0)
- goto error_alloc;
+ goto error_us;
/* allocate a new session keyring */
sprintf(desc, "_req.%u", key->serial);
@@ -169,7 +169,7 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
session = cred->session_keyring;
if (!session)
- session = cred->user->session_keyring;
+ session = user_session;
sskey = session->serial;
sprintf(keyring_str[2], "%d", sskey);
@@ -211,6 +211,8 @@ static int call_sbin_request_key(struct key *authkey, void *aux)
key_put(keyring);
error_alloc:
+ key_put(user_session);
+error_us:
complete_request_key(authkey, ret);
kleave(" = %d", ret);
return ret;
@@ -317,13 +319,15 @@ static int construct_get_dest_keyring(struct key **_dest_keyring)
/* fall through */
case KEY_REQKEY_DEFL_USER_SESSION_KEYRING:
- dest_keyring =
- key_get(READ_ONCE(cred->user->session_keyring));
+ ret = look_up_user_keyrings(NULL, &dest_keyring);
+ if (ret < 0)
+ return ret;
break;
case KEY_REQKEY_DEFL_USER_KEYRING:
- dest_keyring =
- key_get(READ_ONCE(cred->user->uid_keyring));
+ ret = look_up_user_keyrings(&dest_keyring, NULL);
+ if (ret < 0)
+ return ret;
break;
case KEY_REQKEY_DEFL_GROUP_KEYRING:
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/9] keys: Namespacing [ver #4]
2019-06-19 16:06 [PATCH 0/9] keys: Namespacing [ver #4] David Howells
` (4 preceding siblings ...)
2019-06-19 16:06 ` [PATCH 5/9] keys: Move the user and user-session keyrings to the user_namespace " David Howells
@ 2019-06-19 16:09 ` David Howells
5 siblings, 0 replies; 8+ messages in thread
From: David Howells @ 2019-06-19 16:09 UTC (permalink / raw)
Cc: dhowells, ebiederm, keyrings, linux-cifs, linux-nfs, netdev,
linux-afs, dwalsh, vgoyal, linux-security-module, linux-fsdevel,
linux-kernel
Apologies... A couple of the patches don't compile because I forgot to commit
a change:-(
Will repush and repost.
David
^ permalink raw reply [flat|nested] 8+ messages in thread