[PATCH v3 1/2] KEYS: Return user session keyring on lookup

[PATCH v3 1/2] KEYS: Return user session keyring on lookup

[Jarkko Sakkinen]

A process without a session keyring looking up KEY_SPEC_SESSION_KEYRING
without KEY_LOOKUP_CREATE mutates the credentials. This causes struct creds
instances shared with other subsystems to become stale.

Address this by returning the resolved user session keyring directly
without installing it into credentials.

Cc: keyrings@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Cc: Eric Biggers <ebiggers@kernel.org>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: stable@vger.kernel.org # v5.10+
Fixes: d84f4f992cbd ("CRED: Inaugurate COW credentials")
Suggested-by: Jann Horn <jannh@google.com>
Closes: https://lore.kernel.org/keyrings/CAG48ez0XjVSR=M--UBauTm8sJuc+ZbhKzaPa3a6wrSVHyoKevw@mail.gmail.com/
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v3:
In keyctl_session_to_parent(), check the resolved cred->session_keyring
instead of mycred->session_keyring.
v2:
- Added mandatory tags.
- No code changes.
- Link: https://lore.kernel.org/keyrings/20261019145624.309790-1-jarkko@kernel.org/
 Documentation/security/keys/core.rst |  6 ++--
 security/keys/keyctl.c               |  5 ++--
 security/keys/process_keys.c         | 44 +++++++---------------------
 3 files changed, 16 insertions(+), 39 deletions(-)
---
 Documentation/security/keys/core.rst |  6 ++--
 security/keys/keyctl.c               |  5 ++--
 security/keys/process_keys.c         | 44 +++++++---------------------
 3 files changed, 16 insertions(+), 39 deletions(-)

diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst
index 37ed42382f16..e3a215e55f81 100644
--- a/Documentation/security/keys/core.rst
+++ b/Documentation/security/keys/core.rst
@@ -165,8 +165,10 @@ The key service provides a number of features besides keys:
      When a process changes its real UID, if it used to have no session key, it
      will be subscribed to the default session key for the new UID.
 
-     If a process attempts to access its session key when it doesn't have one,
-     it will be subscribed to the default for its current UID.
+     If a process attempts to access its session keyring when it doesn't have
+     one, the default user session keyring for its current UID is returned
+     without being installed into its credentials.  If creation is requested,
+     an anonymous session keyring is installed.
 
   *  Each user has two quotas against which the keys they own are tracked. One
      limits the total number of keys and keyrings, the other limits the total
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index d6b10fd67627..b11716f88ded 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1682,8 +1682,7 @@ long keyctl_session_to_parent(void)
 	 * there's no point */
 	mycred = current_cred();
 	pcred = __task_cred(parent);
-	if (mycred == pcred ||
-	    mycred->session_keyring == pcred->session_keyring) {
+	if (cred->session_keyring == pcred->session_keyring) {
 		ret = 0;
 		goto unlock;
 	}
@@ -1701,7 +1700,7 @@ long keyctl_session_to_parent(void)
 	/* the keyrings must have the same UID */
 	if ((pcred->session_keyring &&
 	     !uid_eq(pcred->session_keyring->uid, mycred->euid)) ||
-	    !uid_eq(mycred->session_keyring->uid, mycred->euid))
+	    !uid_eq(cred->session_keyring->uid, mycred->euid))
 		goto unlock;
 
 	/* cancel an already pending keyring replacement */
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 3af71c6322af..cdf03b8124d0 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -356,31 +356,6 @@ int install_session_keyring_to_cred(struct cred *cred, struct key *keyring)
 	return 0;
 }
 
-/*
- * Install the given keyring as the session keyring of the current task,
- * replacing the existing one if any.  If the given keyring is NULL, then
- * install a new anonymous session keyring.
- *
- * Return: 0 on success; -errno on failure.
- */
-static int install_session_keyring(struct key *keyring)
-{
-	struct cred *new;
-	int ret;
-
-	new = prepare_creds();
-	if (!new)
-		return -ENOMEM;
-
-	ret = install_session_keyring_to_cred(new, keyring);
-	if (ret < 0) {
-		abort_creds(new);
-		return ret;
-	}
-
-	return commit_creds(new);
-}
-
 /*
  * Handle the fsuid changing.
  */
@@ -720,20 +695,21 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 
 	case KEY_SPEC_SESSION_KEYRING:
 		if (!ctx.cred->session_keyring) {
-			/* always install a session keyring upon access if one
-			 * doesn't exist yet */
 			ret = look_up_user_keyrings(NULL, &user_session);
 			if (ret < 0)
 				goto error;
-			if (lflags & KEY_LOOKUP_CREATE)
+
+			if (lflags & KEY_LOOKUP_CREATE) {
+				key_put(user_session);
 				ret = join_session_keyring(NULL);
-			else
-				ret = install_session_keyring(user_session);
+				if (ret < 0)
+					goto error;
+				goto reget_creds;
+			}
 
-			key_put(user_session);
-			if (ret < 0)
-				goto error;
-			goto reget_creds;
+			key = user_session;
+			key_ref = make_key_ref(key, 1);
+			break;
 		} else if (test_bit(KEY_FLAG_UID_KEYRING,
 				    &ctx.cred->session_keyring->flags) &&
 			   lflags & KEY_LOOKUP_CREATE) {
-- 
2.47.3

[PATCH v3 2/2] KEYS: Reject keyring creation with overridden credentials

[Jarkko Sakkinen]

If the current task is running with overridden credentials calling
commit_creds() is forbidden and triggers a BUG_ON().

Reject keyring creation with -EPERM when credentials are overridden.

Cc: keyrings@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Cc: Eric Biggers <ebiggers@kernel.org>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: stable@vger.kernel.org # v5.10+
Cc: stable@vger.kernel.org # v5.10+
Fixes: 3b11a1decef0 ("CRED: Differentiate objective and effective subjective credentials on a task")
Suggested-by: Jann Horn <jannh@google.com>
Closes: https://lore.kernel.org/keyrings/CAG48ez0XjVSR=M--UBauTm8sJuc+ZbhKzaPa3a6wrSVHyoKevw@mail.gmail.com/
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v3:
Collapse and open-code cred_overridden() to the entry point of lookup_user_key() (David).
v2:
- Added mandatory tags.
- No code changes.
- Link: https://lore.kernel.org/keyrings/20261019145624.309790-2-jarkko@kernel.org/
 Documentation/security/keys/core.rst | 3 ++-
 security/keys/process_keys.c         | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)
---
 Documentation/security/keys/core.rst | 3 ++-
 security/keys/process_keys.c         | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst
index e3a215e55f81..a60b2f725a0b 100644
--- a/Documentation/security/keys/core.rst
+++ b/Documentation/security/keys/core.rst
@@ -168,7 +168,8 @@ The key service provides a number of features besides keys:
      If a process attempts to access its session keyring when it doesn't have
      one, the default user session keyring for its current UID is returned
      without being installed into its credentials.  If creation is requested,
-     an anonymous session keyring is installed.
+     an anonymous session keyring is installed, failing with -EPERM if
+     credentials are overridden.

   *  Each user has two quotas against which the keys they own are tracked. One
      limits the total number of keys and keyrings, the other limits the total
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index cdf03b8124d0..c878a5f8d47c 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -633,7 +633,8 @@ void key_register_fs_verity_keyring(struct key *keyring)
  * to a key or the best found key was a negative key; -EKEYREVOKED or
  * -EKEYEXPIRED if the best found key was revoked or expired; -EACCES if the
  * found key doesn't grant the requested permit or the LSM denied access to it;
- * or -ENOMEM if a special keyring couldn't be created.
+ * -ENOMEM if a special keyring couldn't be created; or -EPERM if KEY_LOOKUP_CREATE
+ * is requested while credentials are overridden.
  *
  * In the case of a successful return, the possession attribute is set on the
  * returned key reference.
@@ -652,6 +653,9 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 	key_ref_t key_ref, skey_ref;
 	int ret;

+	if ((lflags & KEY_LOOKUP_CREATE) && current_cred() != current_real_cred())
+		return ERR_PTR(-EPERM);
+
 try_again:
 	ctx.cred = get_current_cred();
 	key_ref = ERR_PTR(-ENOKEY);
--
2.47.3