Skip to content

Commit 237bbd2

Browse files
ebiggersdhowells
authored andcommitted
KEYS: prevent creating a different user's keyrings
It was possible for an unprivileged user to create the user and user session keyrings for another user. For example: sudo -u '#3000' sh -c 'keyctl add keyring _uid.4000 "" @U keyctl add keyring _uid_ses.4000 "" @U sleep 15' & sleep 1 sudo -u '#4000' keyctl describe @U sudo -u '#4000' keyctl describe @us This is problematic because these "fake" keyrings won't have the right permissions. In particular, the user who created them first will own them and will have full access to them via the possessor permissions, which can be used to compromise the security of a user's keys: -4: alswrv-----v------------ 3000 0 keyring: _uid.4000 -5: alswrv-----v------------ 3000 0 keyring: _uid_ses.4000 Fix it by marking user and user session keyrings with a flag KEY_FLAG_UID_KEYRING. Then, when searching for a user or user session keyring by name, skip all keyrings that don't have the flag set. Fixes: 69664cf ("keys: don't generate user and user session keyrings unless they're accessed") Cc: <stable@vger.kernel.org> [v2.6.26+] Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com>
1 parent e645016 commit 237bbd2

File tree

5 files changed

+23
-12
lines changed

5 files changed

+23
-12
lines changed

Diff for: include/linux/key.h

+2
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ struct key {
187187
#define KEY_FLAG_BUILTIN 8 /* set if key is built in to the kernel */
188188
#define KEY_FLAG_ROOT_CAN_INVAL 9 /* set if key can be invalidated by root without permission */
189189
#define KEY_FLAG_KEEP 10 /* set if key should not be removed */
190+
#define KEY_FLAG_UID_KEYRING 11 /* set if key is a user or user session keyring */
190191

191192
/* the key type and key description string
192193
* - the desc is used to match a key against search criteria
@@ -243,6 +244,7 @@ extern struct key *key_alloc(struct key_type *type,
243244
#define KEY_ALLOC_NOT_IN_QUOTA 0x0002 /* not in quota */
244245
#define KEY_ALLOC_BUILT_IN 0x0004 /* Key is built into kernel */
245246
#define KEY_ALLOC_BYPASS_RESTRICTION 0x0008 /* Override the check on restricted keyrings */
247+
#define KEY_ALLOC_UID_KEYRING 0x0010 /* allocating a user or user session keyring */
246248

247249
extern void key_revoke(struct key *key);
248250
extern void key_invalidate(struct key *key);

Diff for: security/keys/internal.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ extern key_ref_t keyring_search_aux(key_ref_t keyring_ref,
141141
extern key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx);
142142
extern key_ref_t search_process_keyrings(struct keyring_search_context *ctx);
143143

144-
extern struct key *find_keyring_by_name(const char *name, bool skip_perm_check);
144+
extern struct key *find_keyring_by_name(const char *name, bool uid_keyring);
145145

146146
extern int install_user_keyrings(void);
147147
extern int install_thread_keyring_to_cred(struct cred *);

Diff for: security/keys/key.c

+2
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,8 @@ struct key *key_alloc(struct key_type *type, const char *desc,
302302
key->flags |= 1 << KEY_FLAG_IN_QUOTA;
303303
if (flags & KEY_ALLOC_BUILT_IN)
304304
key->flags |= 1 << KEY_FLAG_BUILTIN;
305+
if (flags & KEY_ALLOC_UID_KEYRING)
306+
key->flags |= 1 << KEY_FLAG_UID_KEYRING;
305307

306308
#ifdef KEY_DEBUGGING
307309
key->magic = KEY_DEBUG_MAGIC;

Diff for: security/keys/keyring.c

+14-9
Original file line numberDiff line numberDiff line change
@@ -1097,15 +1097,15 @@ key_ref_t find_key_to_update(key_ref_t keyring_ref,
10971097
/*
10981098
* Find a keyring with the specified name.
10991099
*
1100-
* All named keyrings in the current user namespace are searched, provided they
1101-
* grant Search permission directly to the caller (unless this check is
1102-
* skipped). Keyrings whose usage points have reached zero or who have been
1103-
* revoked are skipped.
1100+
* Only keyrings that have nonzero refcount, are not revoked, and are owned by a
1101+
* user in the current user namespace are considered. If @uid_keyring is %true,
1102+
* the keyring additionally must have been allocated as a user or user session
1103+
* keyring; otherwise, it must grant Search permission directly to the caller.
11041104
*
11051105
* Returns a pointer to the keyring with the keyring's refcount having being
11061106
* incremented on success. -ENOKEY is returned if a key could not be found.
11071107
*/
1108-
struct key *find_keyring_by_name(const char *name, bool skip_perm_check)
1108+
struct key *find_keyring_by_name(const char *name, bool uid_keyring)
11091109
{
11101110
struct key *keyring;
11111111
int bucket;
@@ -1133,10 +1133,15 @@ struct key *find_keyring_by_name(const char *name, bool skip_perm_check)
11331133
if (strcmp(keyring->description, name) != 0)
11341134
continue;
11351135

1136-
if (!skip_perm_check &&
1137-
key_permission(make_key_ref(keyring, 0),
1138-
KEY_NEED_SEARCH) < 0)
1139-
continue;
1136+
if (uid_keyring) {
1137+
if (!test_bit(KEY_FLAG_UID_KEYRING,
1138+
&keyring->flags))
1139+
continue;
1140+
} else {
1141+
if (key_permission(make_key_ref(keyring, 0),
1142+
KEY_NEED_SEARCH) < 0)
1143+
continue;
1144+
}
11401145

11411146
/* we've got a match but we might end up racing with
11421147
* key_cleanup() if the keyring is currently 'dead'

Diff for: security/keys/process_keys.c

+4-2
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ int install_user_keyrings(void)
7777
if (IS_ERR(uid_keyring)) {
7878
uid_keyring = keyring_alloc(buf, user->uid, INVALID_GID,
7979
cred, user_keyring_perm,
80-
KEY_ALLOC_IN_QUOTA,
80+
KEY_ALLOC_UID_KEYRING |
81+
KEY_ALLOC_IN_QUOTA,
8182
NULL, NULL);
8283
if (IS_ERR(uid_keyring)) {
8384
ret = PTR_ERR(uid_keyring);
@@ -94,7 +95,8 @@ int install_user_keyrings(void)
9495
session_keyring =
9596
keyring_alloc(buf, user->uid, INVALID_GID,
9697
cred, user_keyring_perm,
97-
KEY_ALLOC_IN_QUOTA,
98+
KEY_ALLOC_UID_KEYRING |
99+
KEY_ALLOC_IN_QUOTA,
98100
NULL, NULL);
99101
if (IS_ERR(session_keyring)) {
100102
ret = PTR_ERR(session_keyring);

0 commit comments

Comments
 (0)