KEYS: prevent creating a different user's keyrings

commit 237bbd29f7a049d310d907f4b2716a7feef9abf3 upstream.

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: 69664cf16af4 ("keys: don't generate user and user session keyrings unless they're accessed")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/linux/key.h b/include/linux/key.h
index 66f70524398594598fb721cf284ebdf1403d8566..dcc115e8dd03d725cdd12fc6e2e3cde3ae51d8ca 100644 (file)
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -177,6 +177,7 @@ struct key {
 #define KEY_FLAG_TRUSTED_ONLY  9       /* set if keyring only accepts links to trusted keys */
 #define KEY_FLAG_BUILTIN       10      /* set if key is builtin */
 #define KEY_FLAG_ROOT_CAN_INVAL        11      /* set if key can be invalidated by root without permission */
+#define KEY_FLAG_UID_KEYRING   12      /* set if key is a user or user session keyring */
 
        /* the key type and key description string
         * - the desc is used to match a key against search criteria
@@ -218,6 +219,7 @@ extern struct key *key_alloc(struct key_type *type,
 #define KEY_ALLOC_QUOTA_OVERRUN        0x0001  /* add to quota, permit even if overrun */
 #define KEY_ALLOC_NOT_IN_QUOTA 0x0002  /* not in quota */
 #define KEY_ALLOC_TRUSTED      0x0004  /* Key should be flagged as trusted */
+#define KEY_ALLOC_UID_KEYRING  0x0010  /* allocating a user or user session keyring */
 
 extern void key_revoke(struct key *key);
 extern void key_invalidate(struct key *key);

diff --git a/security/keys/internal.h b/security/keys/internal.h
index 5105c2c2da75b0e13dec1196be67c88f4d789e72..51ffb9cde0733e59b193d7de898989f90298c628 100644 (file)
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -136,7 +136,7 @@ extern key_ref_t keyring_search_aux(key_ref_t keyring_ref,
 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 struct key *find_keyring_by_name(const char *name, bool skip_perm_check);
+extern struct key *find_keyring_by_name(const char *name, bool uid_keyring);
 
 extern int install_user_keyrings(void);
 extern int install_thread_keyring_to_cred(struct cred *);

diff --git a/security/keys/key.c b/security/keys/key.c
index 09c10b1818813c66d734b3887990d202ddefeb6f..51d23c6234247ec53d708932d0d453023f6908e0 100644 (file)
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -296,6 +296,8 @@ struct key *key_alloc(struct key_type *type, const char *desc,
                key->flags |= 1 << KEY_FLAG_IN_QUOTA;
        if (flags & KEY_ALLOC_TRUSTED)
                key->flags |= 1 << KEY_FLAG_TRUSTED;
+       if (flags & KEY_ALLOC_UID_KEYRING)
+               key->flags |= 1 << KEY_FLAG_UID_KEYRING;
 
 #ifdef KEY_DEBUGGING
        key->magic = KEY_DEBUG_MAGIC;

diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 262ed2a6b360eedd4414b096426ae20d54fa2593..0c8dd4fbe130c8797b149e9c4f90647089a17c10 100644 (file)
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -961,15 +961,15 @@ found:
 /*
  * Find a keyring with the specified name.
  *
- * All named keyrings in the current user namespace are searched, provided they
- * grant Search permission directly to the caller (unless this check is
- * skipped).  Keyrings whose usage points have reached zero or who have been
- * revoked are skipped.
+ * Only keyrings that have nonzero refcount, are not revoked, and are owned by a
+ * user in the current user namespace are considered.  If @uid_keyring is %true,
+ * the keyring additionally must have been allocated as a user or user session
+ * keyring; otherwise, it must grant Search permission directly to the caller.
  *
  * Returns a pointer to the keyring with the keyring's refcount having being
  * incremented on success.  -ENOKEY is returned if a key could not be found.
  */
-struct key *find_keyring_by_name(const char *name, bool skip_perm_check)
+struct key *find_keyring_by_name(const char *name, bool uid_keyring)
 {
        struct key *keyring;
        int bucket;
@@ -997,10 +997,15 @@ struct key *find_keyring_by_name(const char *name, bool skip_perm_check)
                        if (strcmp(keyring->description, name) != 0)
                                continue;
 
-                       if (!skip_perm_check &&
-                           key_permission(make_key_ref(keyring, 0),
-                                          KEY_NEED_SEARCH) < 0)
-                               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;
+                       }
 
                        /* we've got a match but we might end up racing with
                         * key_cleanup() if the keyring is currently 'dead'

diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 4ed909142956980d23375a45edd974d866dbccd0..7dd050f2426122f703f223669e7ac660f4d32871 100644 (file)
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -76,7 +76,9 @@ int install_user_keyrings(void)
                if (IS_ERR(uid_keyring)) {
                        uid_keyring = keyring_alloc(buf, user->uid, INVALID_GID,
                                                    cred, user_keyring_perm,
-                                                   KEY_ALLOC_IN_QUOTA, NULL);
+                                                   KEY_ALLOC_UID_KEYRING |
+                                                       KEY_ALLOC_IN_QUOTA,
+                                                   NULL);
                        if (IS_ERR(uid_keyring)) {
                                ret = PTR_ERR(uid_keyring);
                                goto error;
@@ -92,7 +94,9 @@ int install_user_keyrings(void)
                        session_keyring =
                                keyring_alloc(buf, user->uid, INVALID_GID,
                                              cred, user_keyring_perm,
-                                             KEY_ALLOC_IN_QUOTA, NULL);
+                                             KEY_ALLOC_UID_KEYRING |
+                                                 KEY_ALLOC_IN_QUOTA,
+                                             NULL);
                        if (IS_ERR(session_keyring)) {
                                ret = PTR_ERR(session_keyring);
                                goto error_release;