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>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

diff --git a/include/linux/key.h b/include/linux/key.h
index 183a6af7715d341bb5ffa9c8ebb3428d44f93cc8..2c631b4c559e8bdc91d801ebe9550eb1f078df52 100644 (file)
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -155,6 +155,7 @@ struct key {
 #define KEY_FLAG_IN_QUOTA      3       /* set if key consumes quota */
 #define KEY_FLAG_USER_CONSTRUCT        4       /* set if key is being constructed in userspace */
 #define KEY_FLAG_NEGATIVE      5       /* set if key is negative */
+#define KEY_FLAG_UID_KEYRING   11      /* set if key is a user or user session keyring */
 
        /* the description string
         * - this is used to match a key against search criteria
@@ -196,6 +197,7 @@ extern struct key *key_alloc(struct key_type *type,
 #define KEY_ALLOC_IN_QUOTA     0x0000  /* add to quota, reject if would overrun */
 #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_UID_KEYRING  0x0010  /* allocating a user or user session keyring */
 
 extern void key_revoke(struct key *key);
 extern void key_put(struct key *key);

diff --git a/security/keys/internal.h b/security/keys/internal.h
index c7a7caec4830b33e414ac7fa3b665633b853a080..1f4b104e79e2f2377ead1a34df7e72cc4505816b 100644 (file)
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -124,7 +124,7 @@ extern key_ref_t search_process_keyrings(struct key_type *type,
                                         key_match_func_t match,
                                         const struct cred *cred);
 
-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 04c809fb0add3b418f7cc34de36c536fb9842fe5..1f5c2c53fa9d1011435aca60eadbdbd8f77fdc00 100644 (file)
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -305,6 +305,8 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 
        if (!(flags & KEY_ALLOC_NOT_IN_QUOTA))
                key->flags |= 1 << KEY_FLAG_IN_QUOTA;
+       if (flags & KEY_ALLOC_UID_KEYRING)
+               key->flags |= 1 << KEY_FLAG_UID_KEYRING;
 
        memset(&key->type_data, 0, sizeof(key->type_data));
 

diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 8f31d5f6cda560443f446e91f9ebe3d231df7324..4d01452b27eb03b48442e32907c8efbba452d2cf 100644 (file)
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -550,15 +550,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;
@@ -586,10 +586,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_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_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 fe5719f793daa0d70b745f42edbc735beb71a36c..7777cb8a93abb2316c900b71e97d748f49cc9b87 100644 (file)
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -72,7 +72,9 @@ int install_user_keyrings(void)
                uid_keyring = find_keyring_by_name(buf, true);
                if (IS_ERR(uid_keyring)) {
                        uid_keyring = keyring_alloc(buf, user->uid, (gid_t) -1,
-                                                   cred, KEY_ALLOC_IN_QUOTA,
+                                                   cred,
+                                                   KEY_ALLOC_UID_KEYRING |
+                                                       KEY_ALLOC_IN_QUOTA,
                                                    NULL);
                        if (IS_ERR(uid_keyring)) {
                                ret = PTR_ERR(uid_keyring);
@@ -88,7 +90,10 @@ int install_user_keyrings(void)
                if (IS_ERR(session_keyring)) {
                        session_keyring =
                                keyring_alloc(buf, user->uid, (gid_t) -1,
-                                             cred, KEY_ALLOC_IN_QUOTA, NULL);
+                                             cred,
+                                             KEY_ALLOC_UID_KEYRING |
+                                                 KEY_ALLOC_IN_QUOTA,
+                                             NULL);
                        if (IS_ERR(session_keyring)) {
                                ret = PTR_ERR(session_keyring);
                                goto error_release;