selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring
@ 2020-07-16 20:34 David Howells
  2020-07-16 20:34 ` [RFC PATCH 1/5] keys: Move permissions checking decisions into the checking code David Howells
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: David Howells @ 2020-07-16 20:34 UTC (permalink / raw)
  To: Stephen Smalley, Casey Schaufler
  Cc: keyrings, Jarkko Sakkinen, Paul Moore, selinux, dhowells,
	Jarkko Sakkinen, Eric Biederman, jlayton, christian, selinux,
	linux-afs, linux-nfs, linux-cifs, linux-api, linux-fsdevel,
	linux-security-module, linux-kernel, containers


Here are some patches to provide some security changes and some container
support:

 (1) The mapping from KEY_NEED_* symbols to what is permitted is pushed
     further down into the general permissions checking routines and the
     LSMs.

     More KEY_NEED_* symbols are provided to give finer grained control in
     the mapping.

 (2) The permissions mask is replaced internally with an ACL that contains
     a list of ACEs, each with a specific subject and granted permissions
     mask.  Potted default ACLs are available for new keys and keyrings.

     ACE subjects include:

	- The owner of the key (uid)
	- The key's group (gid)
	- Anyone who possesses the key
	- Everyone
	- Containers (ie. user_namespaces)

 (3) The ACE permissions are split to give finer control.  Examples include
     splitting the revocation permit from the change-attributes permit,
     thereby allowing someone to be granted permission to revoke a key
     without allowing them to change the owner; also the ability to join a
     keyring is split from the ability to link to it, thereby stopping a
     process accessing a keyring by joining it and thus acquiring use of
     possessor permits.

     This is only accessible through the ACL interface; the old setperm
     interface concocts an ACL from what it is given.

 (4) A keyctl is provided to grant or remove a grant of one or more permits
     to a specific subject.  Direct access to the ACL is not permitted, and
     the ACL cannot be viewed.

 (5) A container keyring is made available on the user-namespace and
     created when needed.  This is searched by request_key() after it has
     searched the normal thread/process/session keyrings, but it can't
     normally be accessed from inside the container.  The container
     manager, however, *can* manipulate the contents of the keyring.

     This allows the manager to push keys into the container to allow the
     use of authenticated network filesystems without any need for the
     denizens inside the container to do anything as the manager can add
     and refresh the keys.

The patches can be found on the following branch:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-acl

David
---
David Howells (1):
      keys: Implement a 'container' keyring


 Documentation/security/keys/core.rst               | 128 ++++-
 Documentation/security/keys/request-key.rst        |   9 +-
 certs/blacklist.c                                  |   9 +-
 certs/system_keyring.c                             |  12 +-
 crypto/asymmetric_keys/asymmetric_type.c           |   2 +-
 drivers/md/dm-crypt.c                              |   2 +-
 drivers/md/dm-verity-verify-sig.c                  |   3 +-
 drivers/nvdimm/security.c                          |   2 +-
 fs/afs/security.c                                  |   2 +-
 fs/cifs/cifs_spnego.c                              |  25 +-
 fs/cifs/cifsacl.c                                  |  28 +-
 fs/cifs/connect.c                                  |   4 +-
 fs/crypto/keyring.c                                |  29 +-
 fs/crypto/keysetup_v1.c                            |   2 +-
 fs/ecryptfs/ecryptfs_kernel.h                      |   2 +-
 fs/ecryptfs/keystore.c                             |   2 +-
 fs/fscache/object-list.c                           |   2 +-
 fs/nfs/nfs4idmap.c                                 |  30 +-
 fs/ubifs/auth.c                                    |   2 +-
 fs/verity/signature.c                              |  14 +-
 include/linux/key.h                                | 142 +++--
 include/linux/lsm_hook_defs.h                      |   2 +-
 include/linux/lsm_hooks.h                          |   8 +
 include/linux/security.h                           |   9 +-
 include/linux/user_namespace.h                     |   7 +
 include/uapi/linux/keyctl.h                        |  69 +++
 lib/digsig.c                                       |   2 +-
 net/ceph/ceph_common.c                             |   2 +-
 net/dns_resolver/dns_key.c                         |  12 +-
 net/dns_resolver/dns_query.c                       |  15 +-
 net/rxrpc/key.c                                    |  19 +-
 net/rxrpc/security.c                               |   2 +-
 net/wireless/reg.c                                 |   6 +-
 security/integrity/digsig.c                        |  31 +-
 security/integrity/digsig_asymmetric.c             |   2 +-
 security/integrity/evm/evm_crypto.c                |   2 +-
 security/integrity/ima/ima_mok.c                   |  13 +-
 security/integrity/integrity.h                     |   6 +-
 .../integrity/platform_certs/platform_keyring.c    |  14 +-
 security/keys/Kconfig                              |  11 +
 security/keys/compat.c                             |   5 +
 security/keys/dh.c                                 |   7 +-
 security/keys/encrypted-keys/encrypted.c           |   2 +-
 security/keys/encrypted-keys/masterkey_trusted.c   |   2 +-
 security/keys/gc.c                                 |   2 +-
 security/keys/internal.h                           |  41 +-
 security/keys/key.c                                |  98 ++--
 security/keys/keyctl.c                             | 580 +++++++++++---------
 security/keys/keyctl_pkey.c                        |  17 +-
 security/keys/keyring.c                            |  47 +-
 security/keys/permission.c                         | 600 +++++++++++++++++++--
 security/keys/persistent.c                         |  29 +-
 security/keys/proc.c                               |  27 +-
 security/keys/process_keys.c                       | 137 +++--
 security/keys/request_key.c                        |  49 +-
 security/keys/request_key_auth.c                   |  23 +-
 security/security.c                                |   4 +-
 security/selinux/hooks.c                           | 163 ++++--
 security/smack/smack_lsm.c                         |  90 +++-
 59 files changed, 1885 insertions(+), 721 deletions(-)



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH 1/5] keys: Move permissions checking decisions into the checking code
  2020-07-16 20:34 [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring David Howells
@ 2020-07-16 20:34 ` David Howells
  2020-07-16 20:35 ` [RFC PATCH 3/5] keys: Provide KEYCTL_GRANT_PERMISSION David Howells
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: David Howells @ 2020-07-16 20:34 UTC (permalink / raw)
  To: Stephen Smalley, Casey Schaufler
  Cc: Jarkko Sakkinen, Paul Moore, keyrings, selinux, dhowells,
	Jarkko Sakkinen, Eric Biederman, jlayton, christian, selinux,
	linux-afs, linux-nfs, linux-cifs, linux-api, linux-fsdevel,
	linux-security-module, linux-kernel, containers

Overhaul the permissions checking, moving the decisions of which permits to
request for what operation and what overrides to allow into the permissions
checking functions in keyrings, SELinux and Smack.

To this end, the KEY_NEED_* constants are turned into an enum and expanded
in number to cover operation types individually.

Note that some more tweaking is probably needed to separate kernel uses,
e.g. AFS using rxrpc keys, from direct userspace users.

Some overrides are available and this needs to be handled specially:

 (1) KEY_FLAG_KEEP in key->flags - The key may not be deleted and/or things
     may not be removed from the keyring.  This can only be set inside the
     kernel.  It's used to protect the blacklist keyring and the keys in
     it, for example.

 (2) KEY_FLAG_ROOT_CAN_CLEAR in key->flags - The keyring can be cleared by
     CAP_SYS_ADMIN.  This can only be set by the kernel and is used to
     allow the system admin to manually clear a keyring that is used for
     caching network filesystem information (eg. DNS) upcall results.

 (3) KEY_FLAG_ROOT_CAN_INVAL in key->flags - The key can be invalidated by
     CAP_SYS_ADMIN.  This can only be set by the kernel and is used to
     allow the system admin to manually invalidate a key that is caching
     the result of a netfs information upcall (eg. DNS).

 (4) An appropriate auth token being set in cred->request_key_auth that
     gives a process transient permission to view and instantiate a key.
     This is used by the kernel to delegate instantiation to userspace.  It
     can only be automatically generated by request_key() to allow an
     upcalled instantiator program access to the key to be instantiated and
     the keyrings of the process that called request_key().

     Possibly an additional permission check should be imposed to allow the
     upcall process to actually access the calling process's keyrings.

Note that this requires some tweaks to the testsuite as some of the error
codes change:

 (*) KEYCTL_DH_COMPUTE now passes the error from lookup_user_key() back
     rather than replacing it unconditionally with ENOKEY.  This means that
     permission and other errors are now correctly seen (including now
     getting EINVAL for a key ID of 0 - which is never valid).

 (*) KEYCTL_READ now sees EINVAL rather than ENOKEY for a key ID of 0.
     Internally, this is because the code trying to work out whether a key
     can be read can now be moved into the permissions checking code - where
     it's easier to evaluate - and lookup_user_key() can be called without
     deferral of the permission check.

Signed-off-by: David Howells <dhowells@redhat.com>
Reported-by: Stephen Smalley <stephen.smalley.work@gmail.com>
cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
cc: Paul Moore <paul@paul-moore.com>
cc: Stephen Smalley <stephen.smalley.work@gmail.com>
cc: Casey Schaufler <casey@schaufler-ca.com>
cc: keyrings@vger.kernel.org
cc: selinux@vger.kernel.org
---

 include/linux/key.h              |   33 ++-
 include/linux/lsm_hook_defs.h    |    2 
 include/linux/lsm_hooks.h        |    8 +
 include/linux/security.h         |    9 +
 security/keys/dh.c               |    7 -
 security/keys/internal.h         |   13 +
 security/keys/key.c              |   66 ++++---
 security/keys/keyctl.c           |  373 ++++++++++++--------------------------
 security/keys/keyctl_pkey.c      |   17 +-
 security/keys/keyring.c          |    2 
 security/keys/permission.c       |  143 ++++++++++++---
 security/keys/persistent.c       |    2 
 security/keys/proc.c             |    2 
 security/keys/process_keys.c     |   21 +-
 security/keys/request_key.c      |   10 +
 security/keys/request_key_auth.c |    7 +
 security/security.c              |    4 
 security/selinux/hooks.c         |  163 +++++++++++++----
 security/smack/smack_lsm.c       |   90 +++++++--
 19 files changed, 562 insertions(+), 410 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 0f2e24f13c2b..5ab146cdeb08 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -75,17 +75,28 @@ struct net;
  * The permissions required on a key that we're looking up.
  */
 enum key_need_perm {
-	KEY_NEED_UNSPECIFIED,	/* Needed permission unspecified */
-	KEY_NEED_VIEW,		/* Require permission to view attributes */
-	KEY_NEED_READ,		/* Require permission to read content */
-	KEY_NEED_WRITE,		/* Require permission to update / modify */
-	KEY_NEED_SEARCH,	/* Require permission to search (keyring) or find (key) */
-	KEY_NEED_LINK,		/* Require permission to link */
-	KEY_NEED_SETATTR,	/* Require permission to change attributes */
-	KEY_NEED_UNLINK,	/* Require permission to unlink key */
-	KEY_SYSADMIN_OVERRIDE,	/* Special: override by CAP_SYS_ADMIN */
-	KEY_AUTHTOKEN_OVERRIDE,	/* Special: override by possession of auth token */
-	KEY_DEFER_PERM_CHECK,	/* Special: permission check is deferred */
+	KEY_NEED_UNSPECIFIED,		/* Needed permission unspecified */
+	KEY_NEED_ASSUME_AUTHORITY,	/* Want to assume instantiation authority */
+	KEY_NEED_CHOWN,			/* Want to change key's ownership/group */
+	KEY_NEED_DESCRIBE,		/* Want to get a key's attributes */
+	KEY_NEED_GET_SECURITY,		/* Want to get a key's security label */
+	KEY_NEED_INSTANTIATE,		/* Want to instantiate a key */
+	KEY_NEED_INVALIDATE,		/* Want to invalidate key */
+	KEY_NEED_JOIN,			/* Want to set a keyring as the session keyring */
+	KEY_NEED_KEYRING_ADD,		/* Want to add a link to a keyring */
+	KEY_NEED_KEYRING_CLEAR,		/* Want to clear a keyring */
+	KEY_NEED_KEYRING_DELETE,	/* Want to remove a link from a keyring */
+	KEY_NEED_LINK,			/* Want to create a link to a key */
+	KEY_NEED_READ,			/* Want to read content to userspace */
+	KEY_NEED_REVOKE,		/* Want to revoke a key */
+	KEY_NEED_SEARCH,		/* Want to find a key in a search */
+	KEY_NEED_SETPERM,		/* Want to set the permissions mask */
+	KEY_NEED_SET_RESTRICTION,	/* Want to set a restriction on a keyring */
+	KEY_NEED_SET_TIMEOUT,		/* Want to set the expiration time on a key */
+	KEY_NEED_UNLINK,		/* Want to remove a link from a key */
+	KEY_NEED_UPDATE,		/* Want to update a key's payload */
+	KEY_NEED_USE,			/* Want to use a key (in kernel) */
+	KEY_NEED_WATCH,			/* Want to watch a key for events */
 };
 
 struct seq_file;
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index af998f93d256..000f8db4706d 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -360,7 +360,7 @@ LSM_HOOK(int, 0, key_alloc, struct key *key, const struct cred *cred,
 	 unsigned long flags)
 LSM_HOOK(void, LSM_RET_VOID, key_free, struct key *key)
 LSM_HOOK(int, 0, key_permission, key_ref_t key_ref, const struct cred *cred,
-	 enum key_need_perm need_perm)
+	 enum key_need_perm need_perm, unsigned int flags)
 LSM_HOOK(int, 0, key_getsecurity, struct key *key, char **_buffer)
 #endif /* CONFIG_KEYS */
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 95b7c1d32062..f1130bd699bc 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1119,6 +1119,14 @@
  *	@cred points to the credentials to provide the context against which to
  *	evaluate the security data on the key.
  *	@perm describes the combination of permissions required of this key.
+ *	@flags indicates any special conditions set in the normal checks, such
+ *	as:
+ *		KEY_PERMISSION_USED_AUTH_OVERRIDE - A lack of permission was
+ *		overridden by the presence of an instantiation authorisation
+ *		token.
+ *		KEY_PERMISSION_USED_SYSADMIN_OVERRIDE - A lack of permission was
+ *		overridden by the presence of a KEY_FLAG_ROOT_CAN_xxx flag on
+ *		the key an the success of a CAP_SYS_ADMIN check.
  *	Return 0 if permission is granted, -ve error otherwise.
  * @key_getsecurity:
  *	Get a textual representation of the security context attached to a key
diff --git a/include/linux/security.h b/include/linux/security.h
index 0a0a03b36a3b..4f51e3aa2440 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1778,13 +1778,17 @@ static inline int security_path_chroot(const struct path *path)
 }
 #endif	/* CONFIG_SECURITY_PATH */
 
+/* Flags for security_key_permission() */
+#define KEY_PERMISSION_USED_AUTH_OVERRIDE	0x01 /* Auth token overrode lack of permission */
+#define KEY_PERMISSION_USED_SYSADMIN_OVERRIDE	0x02 /* Sysadmin overrode lack of permission */
+
 #ifdef CONFIG_KEYS
 #ifdef CONFIG_SECURITY
 
 int security_key_alloc(struct key *key, const struct cred *cred, unsigned long flags);
 void security_key_free(struct key *key);
 int security_key_permission(key_ref_t key_ref, const struct cred *cred,
-			    enum key_need_perm need_perm);
+			    enum key_need_perm need_perm, unsigned int flags);
 int security_key_getsecurity(struct key *key, char **_buffer);
 
 #else
@@ -1802,7 +1806,8 @@ static inline void security_key_free(struct key *key)
 
 static inline int security_key_permission(key_ref_t key_ref,
 					  const struct cred *cred,
-					  enum key_need_perm need_perm)
+					  enum key_need_perm need_perm,
+					  unsigned int flags)
 {
 	return 0;
 }
diff --git a/security/keys/dh.c b/security/keys/dh.c
index c4c629bb1c03..e43731d22310 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -22,10 +22,8 @@ static ssize_t dh_data_from_key(key_serial_t keyid, void **data)
 	ssize_t ret;
 
 	key_ref = lookup_user_key(keyid, 0, KEY_NEED_READ);
-	if (IS_ERR(key_ref)) {
-		ret = -ENOKEY;
-		goto error;
-	}
+	if (IS_ERR(key_ref))
+		return PTR_ERR(key_ref);
 
 	key = key_ref_to_ptr(key_ref);
 
@@ -52,7 +50,6 @@ static ssize_t dh_data_from_key(key_serial_t keyid, void **data)
 	}
 
 	key_put(key);
-error:
 	return ret;
 }
 
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 338a526cbfa5..68faf0f0bda8 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -108,6 +108,14 @@ extern void __key_link_end(struct key *keyring,
 
 extern key_ref_t find_key_to_update(key_ref_t keyring_ref,
 				    const struct keyring_index_key *index_key);
+extern key_ref_t key_create_or_update_perm_checked(key_ref_t keyring_ref,
+						   const char *type,
+						   const char *description,
+						   const void *payload,
+						   size_t plen,
+						   key_perm_t perm,
+						   unsigned long flags);
+extern int key_update_perm_checked(key_ref_t key_ref, const void *payload, size_t plen);
 
 extern struct key *keyring_search_instkey(struct key *keyring,
 					  key_serial_t target_id);
@@ -165,8 +173,9 @@ extern struct key *request_key_and_link(struct key_type *type,
 
 extern bool lookup_user_key_possessed(const struct key *key,
 				      const struct key_match_data *match_data);
-#define KEY_LOOKUP_CREATE	0x01
-#define KEY_LOOKUP_PARTIAL	0x02
+#define KEY_LOOKUP_CREATE		0x01
+#define KEY_LOOKUP_PARTIAL		0x02
+#define KEY_LOOKUP_AUTH_OVERRIDE	0x04
 
 extern long join_session_keyring(const char *name);
 extern void key_change_session_keyring(struct callback_head *twork);
diff --git a/security/keys/key.c b/security/keys/key.c
index e282c6179b21..6b12eae4e612 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -755,7 +755,7 @@ static inline key_ref_t __key_update(key_ref_t key_ref,
 	int ret;
 
 	/* need write permission on the key to update it */
-	ret = key_permission(key_ref, KEY_NEED_WRITE);
+	ret = key_permission(key_ref, KEY_NEED_UPDATE);
 	if (ret < 0)
 		goto error;
 
@@ -810,13 +810,13 @@ static inline key_ref_t __key_update(key_ref_t key_ref,
  * On success, the possession flag from the keyring ref will be tacked on to
  * the key ref before it is returned.
  */
-key_ref_t key_create_or_update(key_ref_t keyring_ref,
-			       const char *type,
-			       const char *description,
-			       const void *payload,
-			       size_t plen,
-			       key_perm_t perm,
-			       unsigned long flags)
+key_ref_t key_create_or_update_perm_checked(key_ref_t keyring_ref,
+					    const char *type,
+					    const char *description,
+					    const void *payload,
+					    size_t plen,
+					    key_perm_t perm,
+					    unsigned long flags)
 {
 	struct keyring_index_key index_key = {
 		.description	= description,
@@ -894,14 +894,6 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
 		}
 	}
 
-	/* if we're going to allocate a new key, we're going to have
-	 * to modify the keyring */
-	ret = key_permission(keyring_ref, KEY_NEED_WRITE);
-	if (ret < 0) {
-		key_ref = ERR_PTR(ret);
-		goto error_link_end;
-	}
-
 	/* if it's possible to update this type of key, search for an existing
 	 * key of the same type and description in the destination keyring and
 	 * update that instead if possible
@@ -981,6 +973,25 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
 
 	goto error_free_prep;
 }
+
+key_ref_t key_create_or_update(key_ref_t keyring_ref,
+			       const char *type,
+			       const char *description,
+			       const void *payload,
+			       size_t plen,
+			       key_perm_t perm,
+			       unsigned long flags)
+{
+	int ret;
+
+	ret = key_permission(keyring_ref, KEY_NEED_KEYRING_ADD);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	return key_create_or_update_perm_checked(keyring_ref, type,
+						 description, payload,
+						 plen, perm, flags);
+}
 EXPORT_SYMBOL(key_create_or_update);
 
 /**
@@ -996,19 +1007,12 @@ EXPORT_SYMBOL(key_create_or_update);
  * Returns 0 on success, -EACCES if not permitted and -EOPNOTSUPP if the key
  * type does not support updating.  The key type may return other errors.
  */
-int key_update(key_ref_t key_ref, const void *payload, size_t plen)
+int key_update_perm_checked(key_ref_t key_ref, const void *payload, size_t plen)
 {
 	struct key_preparsed_payload prep;
 	struct key *key = key_ref_to_ptr(key_ref);
 	int ret;
 
-	key_check(key);
-
-	/* the key must be writable */
-	ret = key_permission(key_ref, KEY_NEED_WRITE);
-	if (ret < 0)
-		return ret;
-
 	/* attempt to update it if supported */
 	if (!key->type->update)
 		return -EOPNOTSUPP;
@@ -1040,6 +1044,20 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen)
 		key->type->free_preparse(&prep);
 	return ret;
 }
+
+int key_update(key_ref_t key_ref, const void *payload, size_t plen)
+{
+	int ret;
+
+	key_check(key_ref_to_ptr(key_ref));
+
+	/* the key must be writable */
+	ret = key_permission(key_ref, KEY_NEED_UPDATE);
+	if (ret < 0)
+		return ret;
+
+	return key_update_perm_checked(key_ref, payload, plen);
+}
 EXPORT_SYMBOL(key_update);
 
 /**
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 9febd37a168f..b75326e15a96 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -123,7 +123,8 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type,
 	}
 
 	/* find the target keyring (which must be writable) */
-	keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE, KEY_NEED_WRITE);
+	keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE,
+				      KEY_NEED_KEYRING_ADD);
 	if (IS_ERR(keyring_ref)) {
 		ret = PTR_ERR(keyring_ref);
 		goto error3;
@@ -131,9 +132,9 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type,
 
 	/* create or update the requested key and add it to the target
 	 * keyring */
-	key_ref = key_create_or_update(keyring_ref, type, description,
-				       payload, plen, KEY_PERM_UNDEF,
-				       KEY_ALLOC_IN_QUOTA);
+	key_ref = key_create_or_update_perm_checked(keyring_ref, type, description,
+						    payload, plen, KEY_PERM_UNDEF,
+						    KEY_ALLOC_IN_QUOTA);
 	if (!IS_ERR(key_ref)) {
 		ret = key_ref_to_ptr(key_ref)->serial;
 		key_ref_put(key_ref);
@@ -204,7 +205,7 @@ SYSCALL_DEFINE4(request_key, const char __user *, _type,
 	dest_ref = NULL;
 	if (destringid) {
 		dest_ref = lookup_user_key(destringid, KEY_LOOKUP_CREATE,
-					   KEY_NEED_WRITE);
+					   KEY_NEED_KEYRING_ADD);
 		if (IS_ERR(dest_ref)) {
 			ret = PTR_ERR(dest_ref);
 			goto error3;
@@ -348,14 +349,14 @@ long keyctl_update_key(key_serial_t id,
 	}
 
 	/* find the target key (which must be writable) */
-	key_ref = lookup_user_key(id, 0, KEY_NEED_WRITE);
+	key_ref = lookup_user_key(id, 0, KEY_NEED_UPDATE);
 	if (IS_ERR(key_ref)) {
 		ret = PTR_ERR(key_ref);
 		goto error2;
 	}
 
 	/* update the key */
-	ret = key_update(key_ref, payload, plen);
+	ret = key_update_perm_checked(key_ref, payload, plen);
 
 	key_ref_put(key_ref);
 error2:
@@ -379,31 +380,14 @@ long keyctl_update_key(key_serial_t id,
 long keyctl_revoke_key(key_serial_t id)
 {
 	key_ref_t key_ref;
-	struct key *key;
-	long ret;
-
-	key_ref = lookup_user_key(id, 0, KEY_NEED_WRITE);
-	if (IS_ERR(key_ref)) {
-		ret = PTR_ERR(key_ref);
-		if (ret != -EACCES)
-			goto error;
-		key_ref = lookup_user_key(id, 0, KEY_NEED_SETATTR);
-		if (IS_ERR(key_ref)) {
-			ret = PTR_ERR(key_ref);
-			goto error;
-		}
-	}
 
-	key = key_ref_to_ptr(key_ref);
-	ret = 0;
-	if (test_bit(KEY_FLAG_KEEP, &key->flags))
-		ret = -EPERM;
-	else
-		key_revoke(key);
+	key_ref = lookup_user_key(id, 0, KEY_NEED_REVOKE);
+	if (IS_ERR(key_ref))
+		return PTR_ERR(key_ref);
 
+	key_revoke(key_ref_to_ptr(key_ref));
 	key_ref_put(key_ref);
-error:
-	return ret;
+	return 0;
 }
 
 /*
@@ -420,41 +404,16 @@ long keyctl_revoke_key(key_serial_t id)
 long keyctl_invalidate_key(key_serial_t id)
 {
 	key_ref_t key_ref;
-	struct key *key;
-	long ret;
 
 	kenter("%d", id);
 
-	key_ref = lookup_user_key(id, 0, KEY_NEED_SEARCH);
-	if (IS_ERR(key_ref)) {
-		ret = PTR_ERR(key_ref);
-
-		/* Root is permitted to invalidate certain special keys */
-		if (capable(CAP_SYS_ADMIN)) {
-			key_ref = lookup_user_key(id, 0, KEY_SYSADMIN_OVERRIDE);
-			if (IS_ERR(key_ref))
-				goto error;
-			if (test_bit(KEY_FLAG_ROOT_CAN_INVAL,
-				     &key_ref_to_ptr(key_ref)->flags))
-				goto invalidate;
-			goto error_put;
-		}
-
-		goto error;
-	}
+	key_ref = lookup_user_key(id, 0, KEY_NEED_INVALIDATE);
+	if (IS_ERR(key_ref))
+		return PTR_ERR(key_ref);
 
-invalidate:
-	key = key_ref_to_ptr(key_ref);
-	ret = 0;
-	if (test_bit(KEY_FLAG_KEEP, &key->flags))
-		ret = -EPERM;
-	else
-		key_invalidate(key);
-error_put:
+	key_invalidate(key_ref_to_ptr(key_ref));
 	key_ref_put(key_ref);
-error:
-	kleave(" = %ld", ret);
-	return ret;
+	return 0;
 }
 
 /*
@@ -467,37 +426,15 @@ long keyctl_invalidate_key(key_serial_t id)
 long keyctl_keyring_clear(key_serial_t ringid)
 {
 	key_ref_t keyring_ref;
-	struct key *keyring;
 	long ret;
 
-	keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE, KEY_NEED_WRITE);
-	if (IS_ERR(keyring_ref)) {
-		ret = PTR_ERR(keyring_ref);
+	keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE,
+				      KEY_NEED_KEYRING_CLEAR);
+	if (IS_ERR(keyring_ref))
+		return PTR_ERR(keyring_ref);
 
-		/* Root is permitted to invalidate certain special keyrings */
-		if (capable(CAP_SYS_ADMIN)) {
-			keyring_ref = lookup_user_key(ringid, 0,
-						      KEY_SYSADMIN_OVERRIDE);
-			if (IS_ERR(keyring_ref))
-				goto error;
-			if (test_bit(KEY_FLAG_ROOT_CAN_CLEAR,
-				     &key_ref_to_ptr(keyring_ref)->flags))
-				goto clear;
-			goto error_put;
-		}
-
-		goto error;
-	}
-
-clear:
-	keyring = key_ref_to_ptr(keyring_ref);
-	if (test_bit(KEY_FLAG_KEEP, &keyring->flags))
-		ret = -EPERM;
-	else
-		ret = keyring_clear(keyring);
-error_put:
+	ret = keyring_clear(key_ref_to_ptr(keyring_ref));
 	key_ref_put(keyring_ref);
-error:
 	return ret;
 }
 
@@ -517,7 +454,8 @@ long keyctl_keyring_link(key_serial_t id, key_serial_t ringid)
 	key_ref_t keyring_ref, key_ref;
 	long ret;
 
-	keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE, KEY_NEED_WRITE);
+	keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE,
+				      KEY_NEED_KEYRING_ADD);
 	if (IS_ERR(keyring_ref)) {
 		ret = PTR_ERR(keyring_ref);
 		goto error;
@@ -552,10 +490,9 @@ long keyctl_keyring_link(key_serial_t id, key_serial_t ringid)
 long keyctl_keyring_unlink(key_serial_t id, key_serial_t ringid)
 {
 	key_ref_t keyring_ref, key_ref;
-	struct key *keyring, *key;
 	long ret;
 
-	keyring_ref = lookup_user_key(ringid, 0, KEY_NEED_WRITE);
+	keyring_ref = lookup_user_key(ringid, 0, KEY_NEED_KEYRING_DELETE);
 	if (IS_ERR(keyring_ref)) {
 		ret = PTR_ERR(keyring_ref);
 		goto error;
@@ -567,13 +504,7 @@ long keyctl_keyring_unlink(key_serial_t id, key_serial_t ringid)
 		goto error2;
 	}
 
-	keyring = key_ref_to_ptr(keyring_ref);
-	key = key_ref_to_ptr(key_ref);
-	if (test_bit(KEY_FLAG_KEEP, &keyring->flags) &&
-	    test_bit(KEY_FLAG_KEEP, &key->flags))
-		ret = -EPERM;
-	else
-		ret = key_unlink(keyring, key);
+	ret = key_unlink(key_ref_to_ptr(keyring_ref), key_ref_to_ptr(key_ref));
 
 	key_ref_put(key_ref);
 error2:
@@ -605,13 +536,13 @@ long keyctl_keyring_move(key_serial_t id, key_serial_t from_ringid,
 	if (IS_ERR(key_ref))
 		return PTR_ERR(key_ref);
 
-	from_ref = lookup_user_key(from_ringid, 0, KEY_NEED_WRITE);
+	from_ref = lookup_user_key(from_ringid, 0, KEY_NEED_KEYRING_DELETE);
 	if (IS_ERR(from_ref)) {
 		ret = PTR_ERR(from_ref);
 		goto error2;
 	}
 
-	to_ref = lookup_user_key(to_ringid, KEY_LOOKUP_CREATE, KEY_NEED_WRITE);
+	to_ref = lookup_user_key(to_ringid, KEY_LOOKUP_CREATE, KEY_NEED_KEYRING_ADD);
 	if (IS_ERR(to_ref)) {
 		ret = PTR_ERR(to_ref);
 		goto error3;
@@ -645,33 +576,21 @@ long keyctl_describe_key(key_serial_t keyid,
 			 char __user *buffer,
 			 size_t buflen)
 {
-	struct key *key, *instkey;
+	struct key *key;
 	key_ref_t key_ref;
 	char *infobuf;
 	long ret;
 	int desclen, infolen;
 
-	key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL, KEY_NEED_VIEW);
-	if (IS_ERR(key_ref)) {
-		/* viewing a key under construction is permitted if we have the
-		 * authorisation token handy */
-		if (PTR_ERR(key_ref) == -EACCES) {
-			instkey = key_get_instantiation_authkey(keyid);
-			if (!IS_ERR(instkey)) {
-				key_put(instkey);
-				key_ref = lookup_user_key(keyid,
-							  KEY_LOOKUP_PARTIAL,
-							  KEY_AUTHTOKEN_OVERRIDE);
-				if (!IS_ERR(key_ref))
-					goto okay;
-			}
-		}
-
-		ret = PTR_ERR(key_ref);
-		goto error;
-	}
+	/* Viewing a key under construction is permitted if we have the
+	 * authorisation token handy.
+	 */
+	key_ref = lookup_user_key(keyid,
+				  KEY_LOOKUP_PARTIAL | KEY_LOOKUP_AUTH_OVERRIDE,
+				  KEY_NEED_DESCRIBE);
+	if (IS_ERR(key_ref))
+		return PTR_ERR(key_ref);
 
-okay:
 	key = key_ref_to_ptr(key_ref);
 	desclen = strlen(key->description);
 
@@ -683,23 +602,21 @@ long keyctl_describe_key(key_serial_t keyid,
 			    from_kuid_munged(current_user_ns(), key->uid),
 			    from_kgid_munged(current_user_ns(), key->gid),
 			    key->perm);
-	if (!infobuf)
-		goto error2;
-	infolen = strlen(infobuf);
-	ret = infolen + desclen + 1;
-
-	/* consider returning the data */
-	if (buffer && buflen >= ret) {
-		if (copy_to_user(buffer, infobuf, infolen) != 0 ||
-		    copy_to_user(buffer + infolen, key->description,
-				 desclen + 1) != 0)
-			ret = -EFAULT;
-	}
+	if (infobuf) {
+		infolen = strlen(infobuf);
+		ret = infolen + desclen + 1;
+
+		/* consider returning the data */
+		if (buffer && buflen >= ret) {
+			if (copy_to_user(buffer, infobuf, infolen) != 0 ||
+			    copy_to_user(buffer + infolen, key->description,
+					 desclen + 1) != 0)
+				ret = -EFAULT;
+		}
 
-	kfree(infobuf);
-error2:
+		kfree(infobuf);
+	}
 	key_ref_put(key_ref);
-error:
 	return ret;
 }
 
@@ -745,7 +662,7 @@ long keyctl_keyring_search(key_serial_t ringid,
 	dest_ref = NULL;
 	if (destringid) {
 		dest_ref = lookup_user_key(destringid, KEY_LOOKUP_CREATE,
-					   KEY_NEED_WRITE);
+					   KEY_NEED_KEYRING_ADD);
 		if (IS_ERR(dest_ref)) {
 			ret = PTR_ERR(dest_ref);
 			goto error3;
@@ -815,9 +732,6 @@ static long __keyctl_read_key(struct key *key, char *buffer, size_t buflen)
 /*
  * Read a key's payload.
  *
- * The key must either grant the caller Read permission, or it must grant the
- * caller Search permission when searched for from the process keyrings.
- *
  * If successful, we place up to buflen bytes of data into the buffer, if one
  * is provided, and return the amount of data that is available in the key,
  * irrespective of how much we copied into the buffer.
@@ -831,36 +745,11 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen)
 	size_t key_data_len;
 
 	/* find the key first */
-	key_ref = lookup_user_key(keyid, 0, KEY_DEFER_PERM_CHECK);
-	if (IS_ERR(key_ref)) {
-		ret = -ENOKEY;
-		goto out;
-	}
+	key_ref = lookup_user_key(keyid, 0, KEY_NEED_READ);
+	if (IS_ERR(key_ref))
+		return PTR_ERR(key_ref);
 
 	key = key_ref_to_ptr(key_ref);
-
-	ret = key_read_state(key);
-	if (ret < 0)
-		goto key_put_out; /* Negatively instantiated */
-
-	/* see if we can read it directly */
-	ret = key_permission(key_ref, KEY_NEED_READ);
-	if (ret == 0)
-		goto can_read_key;
-	if (ret != -EACCES)
-		goto key_put_out;
-
-	/* we can't; see if it's searchable from this process's keyrings
-	 * - we automatically take account of the fact that it may be
-	 *   dangling off an instantiation key
-	 */
-	if (!is_key_possessed(key_ref)) {
-		ret = -EACCES;
-		goto key_put_out;
-	}
-
-	/* the key is probably readable - now try to read it */
-can_read_key:
 	if (!key->type->read) {
 		ret = -EOPNOTSUPP;
 		goto key_put_out;
@@ -927,18 +816,16 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen)
 
 key_put_out:
 	key_put(key);
-out:
 	return ret;
 }
 
 /*
  * Change the ownership of a key
  *
- * The key must grant the caller Setattr permission for this to work, though
- * the key need not be fully instantiated yet.  For the UID to be changed, or
- * for the GID to be changed to a group the caller is not a member of, the
- * caller must have sysadmin capability.  If either uid or gid is -1 then that
- * attribute is not changed.
+ * The key need not be fully instantiated for this operation to be applied.
+ * For the UID to be changed, or for the GID to be changed to a group the
+ * caller is not a member of, the caller must have sysadmin capability.  If
+ * either uid or gid is -1 then that attribute is not changed.
  *
  * If the UID is to be changed, the new user must have sufficient quota to
  * accept the key.  The quota deduction will be removed from the old user to
@@ -968,7 +855,7 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group)
 		goto error;
 
 	key_ref = lookup_user_key(id, KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL,
-				  KEY_NEED_SETATTR);
+				  KEY_NEED_CHOWN);
 	if (IS_ERR(key_ref)) {
 		ret = PTR_ERR(key_ref);
 		goto error;
@@ -1060,9 +947,9 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group)
 /*
  * Change the permission mask on a key.
  *
- * The key must grant the caller Setattr permission for this to work, though
- * the key need not be fully instantiated yet.  If the caller does not have
- * sysadmin capability, it may only change the permission on keys that it owns.
+ * The key doesn't have to be fully instantiated yet for this to work.  If the
+ * caller does not have sysadmin capability, it may only change the permission
+ * on keys that it owns.
  */
 long keyctl_setperm_key(key_serial_t id, key_perm_t perm)
 {
@@ -1075,7 +962,7 @@ long keyctl_setperm_key(key_serial_t id, key_perm_t perm)
 		goto error;
 
 	key_ref = lookup_user_key(id, KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL,
-				  KEY_NEED_SETATTR);
+				  KEY_NEED_SETPERM);
 	if (IS_ERR(key_ref)) {
 		ret = PTR_ERR(key_ref);
 		goto error;
@@ -1102,7 +989,7 @@ long keyctl_setperm_key(key_serial_t id, key_perm_t perm)
 
 /*
  * Get the destination keyring for instantiation and check that the caller has
- * Write permission on it.
+ * permission to add a key to it.
  */
 static long get_instantiation_keyring(key_serial_t ringid,
 				      struct request_key_auth *rka,
@@ -1118,7 +1005,8 @@ static long get_instantiation_keyring(key_serial_t ringid,
 
 	/* if a specific keyring is nominated by ID, then use that */
 	if (ringid > 0) {
-		dkref = lookup_user_key(ringid, KEY_LOOKUP_CREATE, KEY_NEED_WRITE);
+		dkref = lookup_user_key(ringid, KEY_LOOKUP_CREATE,
+					KEY_NEED_KEYRING_ADD);
 		if (IS_ERR(dkref))
 			return PTR_ERR(dkref);
 		*_dest_keyring = key_ref_to_ptr(dkref);
@@ -1159,7 +1047,7 @@ static int keyctl_change_reqkey_auth(struct key *key)
  * Instantiate a key with the specified payload and link the key into the
  * destination keyring if one is given.
  *
- * The caller must have the appropriate instantiation permit set for this to
+ * The caller must have the appropriate instantiation token set for this to
  * work (see keyctl_assume_authority).  No other permissions are required.
  *
  * If successful, 0 will be returned.
@@ -1170,29 +1058,34 @@ long keyctl_instantiate_key_common(key_serial_t id,
 {
 	const struct cred *cred = current_cred();
 	struct request_key_auth *rka;
-	struct key *instkey, *dest_keyring;
+	struct key *key, *instkey, *dest_keyring;
+	key_ref_t kref;
 	size_t plen = from ? iov_iter_count(from) : 0;
 	void *payload;
 	long ret;
 
 	kenter("%d,,%zu,%d", id, plen, ringid);
 
+	if (plen > 1024 * 1024 - 1)
+		return -EINVAL;
+
 	if (!plen)
 		from = NULL;
 
-	ret = -EINVAL;
-	if (plen > 1024 * 1024 - 1)
-		goto error;
-
 	/* the appropriate instantiation authorisation key must have been
 	 * assumed before calling this */
-	ret = -EPERM;
 	instkey = cred->request_key_auth;
 	if (!instkey)
-		goto error;
+		return -EPERM;
+
+	kref = lookup_user_key(id, KEY_LOOKUP_PARTIAL, KEY_NEED_INSTANTIATE);
+	if (IS_ERR(kref))
+		return PTR_ERR(kref);
+	key = key_ref_to_ptr(kref);
 
+	ret = -EPERM;
 	rka = instkey->payload.data[0];
-	if (rka->target_key->serial != id)
+	if (rka->target_key != key)
 		goto error;
 
 	/* pull the payload in if one was supplied */
@@ -1216,7 +1109,7 @@ long keyctl_instantiate_key_common(key_serial_t id,
 		goto error2;
 
 	/* instantiate the key and link it into a keyring */
-	ret = key_instantiate_and_link(rka->target_key, payload, plen,
+	ret = key_instantiate_and_link(key, payload, plen,
 				       dest_keyring, instkey);
 
 	key_put(dest_keyring);
@@ -1229,6 +1122,7 @@ long keyctl_instantiate_key_common(key_serial_t id,
 error2:
 	kvfree_sensitive(payload, plen);
 error:
+	key_put(key);
 	return ret;
 }
 
@@ -1332,7 +1226,8 @@ long keyctl_reject_key(key_serial_t id, unsigned timeout, unsigned error,
 {
 	const struct cred *cred = current_cred();
 	struct request_key_auth *rka;
-	struct key *instkey, *dest_keyring;
+	struct key *key, *instkey, *dest_keyring;
+	key_ref_t kref;
 	long ret;
 
 	kenter("%d,%u,%u,%d", id, timeout, error, ringid);
@@ -1348,13 +1243,18 @@ long keyctl_reject_key(key_serial_t id, unsigned timeout, unsigned error,
 
 	/* the appropriate instantiation authorisation key must have been
 	 * assumed before calling this */
-	ret = -EPERM;
 	instkey = cred->request_key_auth;
 	if (!instkey)
-		goto error;
+		return -EPERM;
+
+	kref = lookup_user_key(id, KEY_LOOKUP_PARTIAL, KEY_NEED_INSTANTIATE);
+	if (IS_ERR(kref))
+		return PTR_ERR(kref);
+	key = key_ref_to_ptr(kref);
 
+	ret = -EPERM;
 	rka = instkey->payload.data[0];
-	if (rka->target_key->serial != id)
+	if (rka->target_key != key)
 		goto error;
 
 	/* find the destination keyring if present (which must also be
@@ -1375,6 +1275,7 @@ long keyctl_reject_key(key_serial_t id, unsigned timeout, unsigned error,
 		keyctl_change_reqkey_auth(NULL);
 
 error:
+	key_put(key);
 	return ret;
 }
 
@@ -1438,8 +1339,8 @@ long keyctl_set_reqkey_keyring(int reqkey_defl)
 /*
  * Set or clear the timeout on a key.
  *
- * Either the key must grant the caller Setattr permission or else the caller
- * must hold an instantiation authorisation token for the key.
+ * Either the key must grant the caller permission or else the caller must hold
+ * an instantiation authorisation token for the key.
  *
  * The timeout is either 0 to clear the timeout, or a number of seconds from
  * the current time.  The key and any links to the key will be automatically
@@ -1451,44 +1352,25 @@ long keyctl_set_reqkey_keyring(int reqkey_defl)
  */
 long keyctl_set_timeout(key_serial_t id, unsigned timeout)
 {
-	struct key *key, *instkey;
+	struct key *key;
 	key_ref_t key_ref;
-	long ret;
-
-	key_ref = lookup_user_key(id, KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL,
-				  KEY_NEED_SETATTR);
-	if (IS_ERR(key_ref)) {
-		/* setting the timeout on a key under construction is permitted
-		 * if we have the authorisation token handy */
-		if (PTR_ERR(key_ref) == -EACCES) {
-			instkey = key_get_instantiation_authkey(id);
-			if (!IS_ERR(instkey)) {
-				key_put(instkey);
-				key_ref = lookup_user_key(id,
-							  KEY_LOOKUP_PARTIAL,
-							  KEY_AUTHTOKEN_OVERRIDE);
-				if (!IS_ERR(key_ref))
-					goto okay;
-			}
-		}
 
-		ret = PTR_ERR(key_ref);
-		goto error;
-	}
+	/* Setting the timeout on a key under construction is permitted if we
+	 * have the authorisation token handy
+	 */
+	key_ref = lookup_user_key(id,
+				  KEY_LOOKUP_CREATE |
+				  KEY_LOOKUP_PARTIAL |
+				  KEY_LOOKUP_AUTH_OVERRIDE,
+				  KEY_NEED_SET_TIMEOUT);
+	if (IS_ERR(key_ref))
+		return PTR_ERR(key_ref);
 
-okay:
 	key = key_ref_to_ptr(key_ref);
-	ret = 0;
-	if (test_bit(KEY_FLAG_KEEP, &key->flags)) {
-		ret = -EPERM;
-	} else {
-		key_set_timeout(key, timeout);
-		notify_key(key, NOTIFY_KEY_SETATTR, 0);
-	}
+	key_set_timeout(key, timeout);
+	notify_key(key, NOTIFY_KEY_SETATTR, 0);
 	key_put(key);
-
-error:
-	return ret;
+	return 0;
 }
 
 /*
@@ -1557,28 +1439,17 @@ long keyctl_get_security(key_serial_t keyid,
 			 char __user *buffer,
 			 size_t buflen)
 {
-	struct key *key, *instkey;
+	struct key *key;
 	key_ref_t key_ref;
 	char *context;
 	long ret;
 
-	key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL, KEY_NEED_VIEW);
-	if (IS_ERR(key_ref)) {
-		if (PTR_ERR(key_ref) != -EACCES)
-			return PTR_ERR(key_ref);
-
-		/* viewing a key under construction is also permitted if we
-		 * have the authorisation token handy */
-		instkey = key_get_instantiation_authkey(keyid);
-		if (IS_ERR(instkey))
-			return PTR_ERR(instkey);
-		key_put(instkey);
-
-		key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL,
-					  KEY_AUTHTOKEN_OVERRIDE);
-		if (IS_ERR(key_ref))
-			return PTR_ERR(key_ref);
-	}
+	key_ref = lookup_user_key(keyid,
+				  KEY_LOOKUP_PARTIAL |
+				  KEY_LOOKUP_AUTH_OVERRIDE,
+				  KEY_NEED_GET_SECURITY);
+	if (IS_ERR(key_ref))
+		return PTR_ERR(key_ref);
 
 	key = key_ref_to_ptr(key_ref);
 	ret = security_key_getsecurity(key, &context);
@@ -1610,8 +1481,8 @@ long keyctl_get_security(key_serial_t keyid,
  * Attempt to install the calling process's session keyring on the process's
  * parent process.
  *
- * The keyring must exist and must grant the caller LINK permission, and the
- * parent process must be single-threaded and must have the same effective
+ * The keyring must exist and must grant the caller permission to join it, and
+ * the parent process must be single-threaded and must have the same effective
  * ownership as this process and mustn't be SUID/SGID.
  *
  * The keyring will be emplaced on the parent when it next resumes userspace.
@@ -1627,7 +1498,7 @@ long keyctl_session_to_parent(void)
 	struct cred *cred;
 	int ret;
 
-	keyring_r = lookup_user_key(KEY_SPEC_SESSION_KEYRING, 0, KEY_NEED_LINK);
+	keyring_r = lookup_user_key(KEY_SPEC_SESSION_KEYRING, 0, KEY_NEED_JOIN);
 	if (IS_ERR(keyring_r))
 		return PTR_ERR(keyring_r);
 
@@ -1729,7 +1600,7 @@ long keyctl_restrict_keyring(key_serial_t id, const char __user *_type,
 	char *restriction = NULL;
 	long ret;
 
-	key_ref = lookup_user_key(id, 0, KEY_NEED_SETATTR);
+	key_ref = lookup_user_key(id, 0, KEY_NEED_SET_RESTRICTION);
 	if (IS_ERR(key_ref))
 		return PTR_ERR(key_ref);
 
@@ -1777,7 +1648,7 @@ long keyctl_watch_key(key_serial_t id, int watch_queue_fd, int watch_id)
 	if (watch_id < -1 || watch_id > 0xff)
 		return -EINVAL;
 
-	key_ref = lookup_user_key(id, KEY_LOOKUP_CREATE, KEY_NEED_VIEW);
+	key_ref = lookup_user_key(id, KEY_LOOKUP_CREATE, KEY_NEED_WATCH);
 	if (IS_ERR(key_ref))
 		return PTR_ERR(key_ref);
 	key = key_ref_to_ptr(key_ref);
diff --git a/security/keys/keyctl_pkey.c b/security/keys/keyctl_pkey.c
index 931d8dfb4a7f..aece0651eeae 100644
--- a/security/keys/keyctl_pkey.c
+++ b/security/keys/keyctl_pkey.c
@@ -77,7 +77,8 @@ static int keyctl_pkey_params_parse(struct kernel_pkey_params *params)
  */
 static int keyctl_pkey_params_get(key_serial_t id,
 				  const char __user *_info,
-				  struct kernel_pkey_params *params)
+				  struct kernel_pkey_params *params,
+				  enum key_need_perm need_perm)
 {
 	key_ref_t key_ref;
 	void *p;
@@ -95,7 +96,7 @@ static int keyctl_pkey_params_get(key_serial_t id,
 	if (ret < 0)
 		return ret;
 
-	key_ref = lookup_user_key(id, 0, KEY_NEED_SEARCH);
+	key_ref = lookup_user_key(id, 0, need_perm);
 	if (IS_ERR(key_ref))
 		return PTR_ERR(key_ref);
 	params->key = key_ref_to_ptr(key_ref);
@@ -113,7 +114,8 @@ static int keyctl_pkey_params_get(key_serial_t id,
 static int keyctl_pkey_params_get_2(const struct keyctl_pkey_params __user *_params,
 				    const char __user *_info,
 				    int op,
-				    struct kernel_pkey_params *params)
+				    struct kernel_pkey_params *params,
+				    enum key_need_perm need_perm)
 {
 	struct keyctl_pkey_params uparams;
 	struct kernel_pkey_query info;
@@ -125,7 +127,7 @@ static int keyctl_pkey_params_get_2(const struct keyctl_pkey_params __user *_par
 	if (copy_from_user(&uparams, _params, sizeof(uparams)) != 0)
 		return -EFAULT;
 
-	ret = keyctl_pkey_params_get(uparams.key_id, _info, params);
+	ret = keyctl_pkey_params_get(uparams.key_id, _info, params, need_perm);
 	if (ret < 0)
 		return ret;
 
@@ -168,7 +170,7 @@ long keyctl_pkey_query(key_serial_t id,
 
 	memset(&params, 0, sizeof(params));
 
-	ret = keyctl_pkey_params_get(id, _info, &params);
+	ret = keyctl_pkey_params_get(id, _info, &params, KEY_NEED_DESCRIBE);
 	if (ret < 0)
 		goto error;
 
@@ -213,7 +215,8 @@ long keyctl_pkey_e_d_s(int op,
 	void *in, *out;
 	long ret;
 
-	ret = keyctl_pkey_params_get_2(_params, _info, op, &params);
+	ret = keyctl_pkey_params_get_2(_params, _info, op, &params,
+				       KEY_NEED_USE);
 	if (ret < 0)
 		goto error_params;
 
@@ -289,7 +292,7 @@ long keyctl_pkey_verify(const struct keyctl_pkey_params __user *_params,
 	long ret;
 
 	ret = keyctl_pkey_params_get_2(_params, _info, KEYCTL_PKEY_VERIFY,
-				       &params);
+				       &params, KEY_NEED_USE);
 	if (ret < 0)
 		goto error_params;
 
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 14abfe765b7e..6199efbe19b4 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1167,7 +1167,7 @@ struct key *find_keyring_by_name(const char *name, bool uid_keyring)
 				continue;
 		} else {
 			if (key_permission(make_key_ref(keyring, 0),
-					   KEY_NEED_SEARCH) < 0)
+					   KEY_NEED_JOIN) < 0)
 				continue;
 		}
 
diff --git a/security/keys/permission.c b/security/keys/permission.c
index 4a61f804e80f..2c06fb92d9bd 100644
--- a/security/keys/permission.c
+++ b/security/keys/permission.c
@@ -7,8 +7,118 @@
 
 #include <linux/export.h>
 #include <linux/security.h>
+#include <keys/request_key_auth-type.h>
 #include "internal.h"
 
+/*
+ * Determine if we have sufficient permission to perform an operation.
+ */
+static int check_key_permission(struct key *key, const struct cred *cred,
+				key_perm_t perms, enum key_need_perm need_perm,
+				unsigned int *_notes)
+{
+	struct request_key_auth *rka;
+
+	switch (need_perm) {
+	case KEY_NEED_ASSUME_AUTHORITY:
+		return 0;
+
+	case KEY_NEED_DESCRIBE:
+	case KEY_NEED_GET_SECURITY:
+		if (perms & KEY_OTH_VIEW)
+			return 0;
+		goto check_auth_override;
+
+	case KEY_NEED_CHOWN:
+	case KEY_NEED_SETPERM:
+	case KEY_NEED_SET_RESTRICTION:
+		return perms & KEY_OTH_SETATTR ? 0 : -EACCES;
+
+	case KEY_NEED_INSTANTIATE:
+		goto check_auth_override;
+
+	case KEY_NEED_INVALIDATE:
+		if (test_bit(KEY_FLAG_KEEP, &key->flags))
+			return -EPERM;
+		if (perms & KEY_OTH_SEARCH)
+			return 0;
+		if (test_bit(KEY_FLAG_ROOT_CAN_INVAL, &key->flags))
+			goto check_sysadmin_override;
+		return -EACCES;
+
+	case KEY_NEED_JOIN:
+	case KEY_NEED_LINK:
+		return perms & KEY_OTH_LINK ? 0 : -EACCES;
+
+	case KEY_NEED_KEYRING_DELETE:
+		if (test_bit(KEY_FLAG_KEEP, &key->flags))
+			return -EPERM;
+		/* Fall through. */
+	case KEY_NEED_KEYRING_ADD:
+		return perms & KEY_OTH_WRITE ? 0 : -EACCES;
+
+	case KEY_NEED_KEYRING_CLEAR:
+		if (test_bit(KEY_FLAG_KEEP, &key->flags))
+			return -EPERM;
+		if (perms & KEY_OTH_WRITE)
+			return 0;
+		if (test_bit(KEY_FLAG_ROOT_CAN_CLEAR, &key->flags))
+			goto check_sysadmin_override;
+		return -EACCES;
+
+	case KEY_NEED_READ:
+		return perms & (KEY_OTH_READ | KEY_OTH_SEARCH) ? 0 : -EACCES;
+
+	case KEY_NEED_REVOKE:
+		if (test_bit(KEY_FLAG_KEEP, &key->flags))
+			return -EPERM;
+		return perms & (KEY_OTH_WRITE | KEY_OTH_SETATTR) ? 0 : -EACCES;
+
+	case KEY_NEED_SEARCH:
+		return perms & KEY_OTH_SEARCH ? 0 : -EACCES;
+
+	case KEY_NEED_SET_TIMEOUT:
+		if (test_bit(KEY_FLAG_KEEP, &key->flags))
+			return -EPERM;
+		if (perms & KEY_OTH_SETATTR)
+			return 0;
+		goto check_auth_override;
+
+	case KEY_NEED_UNLINK:
+		if (test_bit(KEY_FLAG_KEEP, &key->flags))
+			return -EPERM;
+		return 0;
+
+	case KEY_NEED_UPDATE:
+		return perms & KEY_OTH_WRITE ? 0 : -EACCES;
+
+	case KEY_NEED_USE:
+		return perms & (KEY_OTH_READ | KEY_OTH_SEARCH) ? 0 : -EACCES;
+
+	case KEY_NEED_WATCH:
+		return perms & KEY_OTH_VIEW ? 0 : -EACCES;
+
+	default:
+		WARN_ON(1);
+		return -EACCES;
+	}
+
+check_auth_override:
+	if (!cred->request_key_auth)
+		return -EACCES;
+	rka = cred->request_key_auth->payload.data[0];
+	if (rka->target_key != key)
+		return -EACCES;
+	*_notes |= KEY_PERMISSION_USED_AUTH_OVERRIDE;
+	return 0;
+
+check_sysadmin_override:
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+	*_notes |= KEY_PERMISSION_USED_SYSADMIN_OVERRIDE;
+	return 0;
+}
+
 /**
  * key_task_permission - Check a key can be used
  * @key_ref: The key to check.
@@ -27,27 +137,10 @@ int key_task_permission(const key_ref_t key_ref, const struct cred *cred,
 			enum key_need_perm need_perm)
 {
 	struct key *key;
-	key_perm_t kperm, mask;
+	unsigned int notes = 0;
+	key_perm_t kperm;
 	int ret;
 
-	switch (need_perm) {
-	default:
-		WARN_ON(1);
-		return -EACCES;
-	case KEY_NEED_UNLINK:
-	case KEY_SYSADMIN_OVERRIDE:
-	case KEY_AUTHTOKEN_OVERRIDE:
-	case KEY_DEFER_PERM_CHECK:
-		goto lsm;
-
-	case KEY_NEED_VIEW:	mask = KEY_OTH_VIEW;	break;
-	case KEY_NEED_READ:	mask = KEY_OTH_READ;	break;
-	case KEY_NEED_WRITE:	mask = KEY_OTH_WRITE;	break;
-	case KEY_NEED_SEARCH:	mask = KEY_OTH_SEARCH;	break;
-	case KEY_NEED_LINK:	mask = KEY_OTH_LINK;	break;
-	case KEY_NEED_SETATTR:	mask = KEY_OTH_SETATTR;	break;
-	}
-
 	key = key_ref_to_ptr(key_ref);
 
 	/* use the second 8-bits of permissions for keys the caller owns */
@@ -75,19 +168,19 @@ int key_task_permission(const key_ref_t key_ref, const struct cred *cred,
 	kperm = key->perm;
 
 use_these_perms:
-
 	/* use the top 8-bits of permissions for keys the caller possesses
 	 * - possessor permissions are additive with other permissions
 	 */
 	if (is_key_possessed(key_ref))
 		kperm |= key->perm >> 24;
 
-	if ((kperm & mask) != mask)
-		return -EACCES;
+	ret = check_key_permission(key, cred, kperm & KEY_OTH_ALL, need_perm,
+				   &notes);
+	if (ret < 0)
+		return ret;
 
-	/* let LSM be the final arbiter */
-lsm:
-	return security_key_permission(key_ref, cred, need_perm);
+	/* Let the LSMs be the final arbiter */
+	return security_key_permission(key_ref, cred, need_perm, notes);
 }
 EXPORT_SYMBOL(key_task_permission);
 
diff --git a/security/keys/persistent.c b/security/keys/persistent.c
index 97af230aa4b2..6131a1528680 100644
--- a/security/keys/persistent.c
+++ b/security/keys/persistent.c
@@ -151,7 +151,7 @@ long keyctl_get_persistent(uid_t _uid, key_serial_t destid)
 	}
 
 	/* There must be a destination keyring */
-	dest_ref = lookup_user_key(destid, KEY_LOOKUP_CREATE, KEY_NEED_WRITE);
+	dest_ref = lookup_user_key(destid, KEY_LOOKUP_CREATE, KEY_NEED_KEYRING_ADD);
 	if (IS_ERR(dest_ref))
 		return PTR_ERR(dest_ref);
 	if (key_ref_to_ptr(dest_ref)->type != &key_type_keyring) {
diff --git a/security/keys/proc.c b/security/keys/proc.c
index d0cde6685627..373e62556fa5 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -188,7 +188,7 @@ static int proc_keys_show(struct seq_file *m, void *v)
 	}
 
 	/* check whether the current task is allowed to view the key */
-	rc = key_task_permission(key_ref, ctx.cred, KEY_NEED_VIEW);
+	rc = key_task_permission(key_ref, ctx.cred, KEY_NEED_DESCRIBE);
 	if (rc < 0)
 		return 0;
 
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 7e0232db1707..e39d9033c34c 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -776,26 +776,17 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 	if (need_perm != KEY_NEED_UNLINK) {
 		if (!(lflags & KEY_LOOKUP_PARTIAL)) {
 			ret = wait_for_key_construction(key, true);
-			switch (ret) {
-			case -ERESTARTSYS:
+			if (ret < 0)
 				goto invalid_key;
-			default:
-				if (need_perm != KEY_AUTHTOKEN_OVERRIDE &&
-				    need_perm != KEY_DEFER_PERM_CHECK)
-					goto invalid_key;
-			case 0:
-				break;
-			}
-		} else if (need_perm != KEY_DEFER_PERM_CHECK) {
+
+			ret = -EIO;
+			if (key_read_state(key) == KEY_IS_UNINSTANTIATED)
+				goto invalid_key;
+		} else {
 			ret = key_validate(key);
 			if (ret < 0)
 				goto invalid_key;
 		}
-
-		ret = -EIO;
-		if (!(lflags & KEY_LOOKUP_PARTIAL) &&
-		    key_read_state(key) == KEY_IS_UNINSTANTIATED)
-			goto invalid_key;
 	}
 
 	/* check the permissions */
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index e1b9f1a80676..c835b7407a5f 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -332,10 +332,10 @@ static int construct_get_dest_keyring(struct key **_dest_keyring)
 			BUG();
 		}
 
-		/*
-		 * Require Write permission on the keyring.  This is essential
-		 * because the default keyring may be the session keyring, and
-		 * joining a keyring only requires Search permission.
+		/* Require permission to add a link to the keyring.  This is
+		 * essential because the default keyring may be the session
+		 * keyring, and joining a keyring only requires Search
+		 * permission.
 		 *
 		 * However, this check is skipped for the "requestor keyring" so
 		 * that /sbin/request-key can itself use request_key() to add
@@ -343,7 +343,7 @@ static int construct_get_dest_keyring(struct key **_dest_keyring)
 		 */
 		if (dest_keyring && do_perm_check) {
 			ret = key_permission(make_key_ref(dest_keyring, 1),
-					     KEY_NEED_WRITE);
+					     KEY_NEED_KEYRING_ADD);
 			if (ret) {
 				key_put(dest_keyring);
 				return ret;
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 41e9735006d0..588130b631b8 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -258,6 +258,7 @@ struct key *key_get_instantiation_authkey(key_serial_t target_id)
 	};
 	struct key *authkey;
 	key_ref_t authkey_ref;
+	int ret;
 
 	ctx.index_key.desc_len = sprintf(description, "%x", target_id);
 
@@ -272,6 +273,12 @@ struct key *key_get_instantiation_authkey(key_serial_t target_id)
 		goto error;
 	}
 
+	ret = key_permission(authkey_ref, KEY_NEED_ASSUME_AUTHORITY);
+	if (ret < 0) {
+		key_ref_put(authkey_ref);
+		authkey = ERR_PTR(ret);
+	}
+
 	authkey = key_ref_to_ptr(authkey_ref);
 	if (test_bit(KEY_FLAG_REVOKED, &authkey->flags)) {
 		key_put(authkey);
diff --git a/security/security.c b/security/security.c
index 70a7ad357bc6..4989f8963b89 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2443,9 +2443,9 @@ void security_key_free(struct key *key)
 }
 
 int security_key_permission(key_ref_t key_ref, const struct cred *cred,
-			    enum key_need_perm need_perm)
+			    enum key_need_perm need_perm, unsigned int flags)
 {
-	return call_int_hook(key_permission, 0, key_ref, cred, need_perm);
+	return call_int_hook(key_permission, 0, key_ref, cred, need_perm, flags);
 }
 
 int security_key_getsecurity(struct key *key, char **_buffer)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index efa6108b1ce9..3bed89539160 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -91,6 +91,7 @@
 #include <uapi/linux/mount.h>
 #include <linux/fsnotify.h>
 #include <linux/fanotify.h>
+#include <keys/request_key_auth-type.h>
 
 #include "avc.h"
 #include "objsec.h"
@@ -6529,6 +6530,121 @@ static int selinux_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 }
 #ifdef CONFIG_KEYS
 
+/*
+ * Convert the requested KEY_NEED_* permit into an SELinux KEY__* permission.
+ *
+ * flags may also convey override flags such as
+ * KEY_PERMISSION_USED_AUTH/SYSADMIN_OVERRIDE to indicate when the main
+ * permission check overrode the permissions on the key.
+ *
+ * Returns the perms to check for in *_perm and *_perm2.  If either perm is
+ * present, then the operation is allowed.
+ */
+static int selinux_keyperm_to_av(struct key *key, const struct cred *cred,
+				 unsigned int need_perm, unsigned int flags,
+				 u32 *_perm, u32 *_perm2)
+{
+	bool auth_can_override = false; /* See KEYCTL_ASSUME_AUTHORITY */
+	bool sysadmin_can_override = false;
+
+	switch (need_perm) {
+	case KEY_NEED_ASSUME_AUTHORITY:
+		return 0;
+
+	case KEY_NEED_DESCRIBE:
+	case KEY_NEED_GET_SECURITY:
+		*_perm = KEY__VIEW;
+		auth_can_override = true;
+		break;
+
+	case KEY_NEED_CHOWN:
+	case KEY_NEED_SETPERM:
+	case KEY_NEED_SET_RESTRICTION:
+		*_perm = KEY__SETATTR;
+		break;
+
+	case KEY_NEED_INSTANTIATE:
+		auth_can_override = true;
+		break;
+
+	case KEY_NEED_INVALIDATE:
+		*_perm = KEY__SEARCH;
+		if (test_bit(KEY_FLAG_ROOT_CAN_INVAL, &key->flags))
+			sysadmin_can_override = true;
+		break;
+
+	case KEY_NEED_JOIN:
+	case KEY_NEED_LINK:
+		*_perm = KEY__LINK;
+		break;
+
+	case KEY_NEED_KEYRING_ADD:
+	case KEY_NEED_KEYRING_DELETE:
+		*_perm = KEY__WRITE;
+		break;
+
+	case KEY_NEED_KEYRING_CLEAR:
+		*_perm = KEY__WRITE;
+		if (test_bit(KEY_FLAG_ROOT_CAN_CLEAR, &key->flags))
+			sysadmin_can_override = true;
+		break;
+
+	case KEY_NEED_READ:
+		*_perm = KEY__READ;
+		break;
+
+	case KEY_NEED_REVOKE:
+		*_perm = KEY__SETATTR;
+		*_perm2 = KEY__WRITE;
+		break;
+
+	case KEY_NEED_SEARCH:
+		*_perm = KEY__SEARCH;
+		break;
+
+	case KEY_NEED_SET_TIMEOUT:
+		*_perm = KEY__SETATTR;
+		auth_can_override = true;
+		break;
+
+	case KEY_NEED_UNLINK:
+		return 0; /* Mustn't prevent this; KEY_FLAG_KEEP is already
+			   * dealt with. */
+
+	case KEY_NEED_UPDATE:
+		*_perm = KEY__WRITE;
+		break;
+
+	case KEY_NEED_USE:
+		*_perm = KEY__READ;
+		*_perm2 = KEY__SEARCH;
+		break;
+
+	case KEY_NEED_WATCH:
+		*_perm = KEY__VIEW;
+		break;
+
+	default:
+		WARN_ON(1);
+		return -EPERM;
+	}
+
+	/* Just allow the operation if the process has an authorisation token.
+	 * The presence of the token means that the kernel delegated
+	 * instantiation of a key to the process - which is problematic if we
+	 * then say that the process isn't allowed to get the description of
+	 * the key or actually instantiate it.
+	 */
+	if (auth_can_override && cred->request_key_auth) {
+		struct request_key_auth *rka =
+			cred->request_key_auth->payload.data[0];
+		if (rka->target_key == key)
+			*_perm = 0;
+	}
+
+	return 0;
+}
+
 static int selinux_key_alloc(struct key *k, const struct cred *cred,
 			     unsigned long flags)
 {
@@ -6559,48 +6675,29 @@ static void selinux_key_free(struct key *k)
 
 static int selinux_key_permission(key_ref_t key_ref,
 				  const struct cred *cred,
-				  enum key_need_perm need_perm)
+				  enum key_need_perm need_perm,
+				  unsigned int flags)
 {
 	struct key *key;
 	struct key_security_struct *ksec;
-	u32 perm, sid;
-
-	switch (need_perm) {
-	case KEY_NEED_VIEW:
-		perm = KEY__VIEW;
-		break;
-	case KEY_NEED_READ:
-		perm = KEY__READ;
-		break;
-	case KEY_NEED_WRITE:
-		perm = KEY__WRITE;
-		break;
-	case KEY_NEED_SEARCH:
-		perm = KEY__SEARCH;
-		break;
-	case KEY_NEED_LINK:
-		perm = KEY__LINK;
-		break;
-	case KEY_NEED_SETATTR:
-		perm = KEY__SETATTR;
-		break;
-	case KEY_NEED_UNLINK:
-	case KEY_SYSADMIN_OVERRIDE:
-	case KEY_AUTHTOKEN_OVERRIDE:
-	case KEY_DEFER_PERM_CHECK:
-		return 0;
-	default:
-		WARN_ON(1);
-		return -EPERM;
-
-	}
+	u32 sid, perm = 0, perm2 = 0;
+	int ret;
 
 	sid = cred_sid(cred);
 	key = key_ref_to_ptr(key_ref);
 	ksec = key->security;
 
+	ret = selinux_keyperm_to_av(key, cred, need_perm, flags, &perm, &perm2);
+	if (ret < 0 || !perm)
+		return ret;
+
+	ret = avc_has_perm(&selinux_state,
+			   sid, ksec->sid, SECCLASS_KEY, perm, NULL);
+	if (ret == 0 || !perm2)
+		return ret;
+
 	return avc_has_perm(&selinux_state,
-			    sid, ksec->sid, SECCLASS_KEY, perm, NULL);
+			    sid, ksec->sid, SECCLASS_KEY, perm2, NULL);
 }
 
 static int selinux_key_getsecurity(struct key *key, char **_buffer)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 8ffbf951b7ed..2246b4dc99ab 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4221,7 +4221,8 @@ static void smack_key_free(struct key *key)
  */
 static int smack_key_permission(key_ref_t key_ref,
 				const struct cred *cred,
-				enum key_need_perm need_perm)
+				enum key_need_perm need_perm,
+				unsigned int flags)
 {
 	struct key *keyp;
 	struct smk_audit_info ad;
@@ -4229,46 +4230,87 @@ static int smack_key_permission(key_ref_t key_ref,
 	int request = 0;
 	int rc;
 
+	keyp = key_ref_to_ptr(key_ref);
+	if (keyp == NULL)
+		return -EINVAL;
+	/*
+	 * If the key hasn't been initialized give it access so that
+	 * it may do so.
+	 */
+	if (keyp->security == NULL)
+		return 0;
+	/*
+	 * This should not occur
+	 */
+	if (tkp == NULL)
+		return -EACCES;
+
 	/*
 	 * Validate requested permissions
 	 */
 	switch (need_perm) {
+	case KEY_NEED_ASSUME_AUTHORITY:
+		return 0;
+
+	case KEY_NEED_DESCRIBE:
+	case KEY_NEED_GET_SECURITY:
+		request |= MAY_READ;
+		auth_can_override = true;
+		break;
+
+	case KEY_NEED_CHOWN:
+	case KEY_NEED_INVALIDATE:
+	case KEY_NEED_JOIN:
+	case KEY_NEED_LINK:
+	case KEY_NEED_KEYRING_ADD:
+	case KEY_NEED_KEYRING_CLEAR:
+	case KEY_NEED_KEYRING_DELETE:
+	case KEY_NEED_REVOKE:
+	case KEY_NEED_SETPERM:
+	case KEY_NEED_SET_RESTRICTION:
+	case KEY_NEED_UPDATE:
+		request |= MAY_WRITE;
+		break;
+
+	case KEY_NEED_INSTANTIATE:
+		auth_can_override = true;
+		break;
+
 	case KEY_NEED_READ:
 	case KEY_NEED_SEARCH:
-	case KEY_NEED_VIEW:
+	case KEY_NEED_USE:
+	case KEY_NEED_WATCH:
 		request |= MAY_READ;
 		break;
-	case KEY_NEED_WRITE:
-	case KEY_NEED_LINK:
-	case KEY_NEED_SETATTR:
+
+	case KEY_NEED_SET_TIMEOUT:
 		request |= MAY_WRITE;
+		auth_can_override = true;
 		break;
-	case KEY_NEED_UNSPECIFIED:
+
 	case KEY_NEED_UNLINK:
-	case KEY_SYSADMIN_OVERRIDE:
-	case KEY_AUTHTOKEN_OVERRIDE:
-	case KEY_DEFER_PERM_CHECK:
-		return 0;
+		return 0; /* Mustn't prevent this; KEY_FLAG_KEEP is already
+			   * dealt with. */
+
 	default:
+		WARN_ON(1);
 		return -EINVAL;
 	}
 
-	keyp = key_ref_to_ptr(key_ref);
-	if (keyp == NULL)
-		return -EINVAL;
-	/*
-	 * If the key hasn't been initialized give it access so that
-	 * it may do so.
+	/* Just allow the operation if the process has an authorisation token.
+	 * The presence of the token means that the kernel delegated
+	 * instantiation of a key to the process - which is problematic if we
+	 * then say that the process isn't allowed to get the description of
+	 * the key or actually instantiate it.
 	 */
-	if (keyp->security == NULL)
-		return 0;
-	/*
-	 * This should not occur
-	 */
-	if (tkp == NULL)
-		return -EACCES;
+	if (auth_can_override && cred->request_key_auth) {
+		struct request_key_auth *rka =
+			cred->request_key_auth->payload.data[0];
+		if (rka->target_key == key)
+			*_perm = 0;
+	}
 
-	if (smack_privileged(CAP_MAC_OVERRIDE))
+	if (smack_privileged_cred(CAP_MAC_OVERRIDE, cred))
 		return 0;
 
 #ifdef CONFIG_AUDIT



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RFC PATCH 3/5] keys: Provide KEYCTL_GRANT_PERMISSION
  2020-07-16 20:34 [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring David Howells
  2020-07-16 20:34 ` [RFC PATCH 1/5] keys: Move permissions checking decisions into the checking code David Howells
@ 2020-07-16 20:35 ` David Howells
  2020-07-16 20:35 ` [RFC PATCH 4/5] keys: Split the search perms between KEY_NEED_USE and KEY_NEED_SEARCH David Howells
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: David Howells @ 2020-07-16 20:35 UTC (permalink / raw)
  To: Stephen Smalley, Casey Schaufler
  Cc: dhowells, Jarkko Sakkinen, Eric Biederman, jlayton, christian,
	selinux, linux-afs, linux-nfs, linux-cifs, linux-api,
	linux-fsdevel, linux-security-module, linux-kernel, containers

Provide a keyctl() operation to grant/remove permissions.  The grant
operation, wrapped by libkeyutils, looks like:

	int ret = keyctl_grant_permission(key_serial_t key,
					  enum key_ace_subject_type type,
					  unsigned int subject,
					  unsigned int perm);

Where key is the key to be modified, type and subject represent the subject
to which permission is to be granted (or removed) and perm is the set of
permissions to be granted.  0 is returned on success.  SETSEC permission is
required for this.

The subject type currently must be KEY_ACE_SUBJ_STANDARD for the moment
(other subject types will come along later).

For subject type KEY_ACE_SUBJ_STANDARD, the following subject values are
available:

	KEY_ACE_POSSESSOR	The possessor of the key
	KEY_ACE_OWNER		The owner of the key
	KEY_ACE_GROUP		The key's group
	KEY_ACE_EVERYONE	Everyone

perm lists the permissions to be granted:

	KEY_ACE_VIEW		Can view the key metadata
	KEY_ACE_READ		Can read the key content
	KEY_ACE_WRITE		Can update/modify the key content
	KEY_ACE_SEARCH		Can find the key by searching/requesting
	KEY_ACE_LINK		Can make a link to the key
	KEY_ACE_SETSEC		Can set security
	KEY_ACE_INVAL		Can invalidate
	KEY_ACE_REVOKE		Can revoke
	KEY_ACE_JOIN		Can join this keyring
	KEY_ACE_CLEAR		Can clear this keyring

If an ACE already exists for the subject, then the permissions mask will be
overwritten; if perm is 0, it will be deleted.

Currently, the internal ACL is limited to a maximum of 16 entries.

For example:

	int ret = keyctl_grant_permission(key,
					  KEY_ACE_SUBJ_STANDARD,
					  KEY_ACE_OWNER,
					  KEY_ACE_VIEW | KEY_ACE_READ);

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/uapi/linux/keyctl.h |    2 +
 security/keys/compat.c      |    2 +
 security/keys/internal.h    |    5 ++
 security/keys/keyctl.c      |    8 +++
 security/keys/permission.c  |  120 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 136 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index 998d4e50bd41..a5938f2c3e66 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -133,6 +133,7 @@ enum key_ace_standard_subject {
 #define KEYCTL_MOVE			30	/* Move keys between keyrings */
 #define KEYCTL_CAPABILITIES		31	/* Find capabilities of keyrings subsystem */
 #define KEYCTL_WATCH_KEY		32	/* Watch a key or ring of keys for changes */
+#define KEYCTL_GRANT_PERMISSION		33	/* Grant a permit to a key */
 
 /* keyctl structures */
 struct keyctl_dh_params {
@@ -196,5 +197,6 @@ struct keyctl_pkey_params {
 #define KEYCTL_CAPS1_NS_KEY_TAG		0x02 /* Key indexing can include a namespace tag */
 #define KEYCTL_CAPS1_NOTIFICATIONS	0x04 /* Keys generate watchable notifications */
 #define KEYCTL_CAPS1_ACL		0x08 /* Keys have ACLs rather than a p-u-g-o bitmask */
+#define KEYCTL_CAPS1_GRANT_PERMISSION	0x10 /* KEYCTL_GRANT_PERMISSION is supported */
 
 #endif /*  _LINUX_KEYCTL_H */
diff --git a/security/keys/compat.c b/security/keys/compat.c
index 6ee9d8f6a4a5..2b675f9a6162 100644
--- a/security/keys/compat.c
+++ b/security/keys/compat.c
@@ -152,6 +152,8 @@ COMPAT_SYSCALL_DEFINE5(keyctl, u32, option,
 
 	case KEYCTL_MOVE:
 		return keyctl_keyring_move(arg2, arg3, arg4, arg5);
+	case KEYCTL_GRANT_PERMISSION:
+		return keyctl_grant_permission(arg2, arg3, arg4, arg5);
 
 	case KEYCTL_CAPABILITIES:
 		return keyctl_capabilities(compat_ptr(arg2), arg3);
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 3b2114d00d5c..af2c9531c435 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -377,6 +377,11 @@ static inline long keyctl_watch_key(key_serial_t key_id, int watch_fd, int watch
 }
 #endif
 
+extern long keyctl_grant_permission(key_serial_t keyid,
+				    enum key_ace_subject_type type,
+				    unsigned int subject,
+				    unsigned int perm);
+
 /*
  * Debugging key validation
  */
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 8689d4331285..fae2df676e30 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -39,7 +39,8 @@ static const unsigned char keyrings_capabilities[2] = {
 	[1] = (KEYCTL_CAPS1_NS_KEYRING_NAME |
 	       KEYCTL_CAPS1_NS_KEY_TAG |
 	       (IS_ENABLED(CONFIG_KEY_NOTIFICATIONS)	? KEYCTL_CAPS1_NOTIFICATIONS : 0) |
-	       KEYCTL_CAPS1_ACL
+	       KEYCTL_CAPS1_ACL |
+	       KEYCTL_CAPS1_GRANT_PERMISSION
 	       ),
 };
 
@@ -1920,6 +1921,11 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
 					   (key_serial_t)arg3,
 					   (key_serial_t)arg4,
 					   (unsigned int)arg5);
+	case KEYCTL_GRANT_PERMISSION:
+		return keyctl_grant_permission((key_serial_t)arg2,
+					       (enum key_ace_subject_type)arg3,
+					       (unsigned int)arg4,
+					       (unsigned int)arg5);
 
 	case KEYCTL_CAPABILITIES:
 		return keyctl_capabilities((unsigned char __user *)arg2, (size_t)arg3);
diff --git a/security/keys/permission.c b/security/keys/permission.c
index 37bad810bc16..0bb7f6b695f4 100644
--- a/security/keys/permission.c
+++ b/security/keys/permission.c
@@ -380,3 +380,123 @@ long key_set_acl(struct key *key, struct key_acl *acl)
 	key_put_acl(acl);
 	return 0;
 }
+
+/*
+ * Allocate a new ACL with an extra ACE slot.
+ */
+static struct key_acl *key_alloc_acl(const struct key_acl *old_acl, int nr, int skip)
+{
+	struct key_acl *acl;
+	int nr_ace, i, j = 0;
+
+	nr_ace = old_acl->nr_ace + nr;
+	if (nr_ace > 16)
+		return ERR_PTR(-EINVAL);
+
+	acl = kzalloc(struct_size(acl, aces, nr_ace), GFP_KERNEL);
+	if (!acl)
+		return ERR_PTR(-ENOMEM);
+
+	refcount_set(&acl->usage, 1);
+	acl->nr_ace = nr_ace;
+	for (i = 0; i < old_acl->nr_ace; i++) {
+		if (i == skip)
+			continue;
+		acl->aces[j] = old_acl->aces[i];
+		j++;
+	}
+	return acl;
+}
+
+/*
+ * Generate the revised ACL.
+ */
+static long key_change_acl(struct key *key, struct key_ace *new_ace)
+{
+	struct key_acl *acl, *old;
+	int i;
+
+	old = rcu_dereference_protected(key->acl, lockdep_is_held(&key->sem));
+
+	for (i = 0; i < old->nr_ace; i++)
+		if (old->aces[i].type == new_ace->type &&
+		    old->aces[i].subject_id == new_ace->subject_id)
+			goto found_match;
+
+	if (new_ace->perm == 0)
+		return 0; /* No permissions to remove.  Add deny record? */
+
+	acl = key_alloc_acl(old, 1, -1);
+	if (IS_ERR(acl))
+		return PTR_ERR(acl);
+	acl->aces[i] = *new_ace;
+	goto change;
+
+found_match:
+	if (new_ace->perm == 0)
+		goto delete_ace;
+	if (new_ace->perm == old->aces[i].perm)
+		return 0;
+	acl = key_alloc_acl(old, 0, -1);
+	if (IS_ERR(acl))
+		return PTR_ERR(acl);
+	acl->aces[i].perm = new_ace->perm;
+	goto change;
+
+delete_ace:
+	acl = key_alloc_acl(old, -1, i);
+	if (IS_ERR(acl))
+		return PTR_ERR(acl);
+	goto change;
+
+change:
+	return key_set_acl(key, acl);
+}
+
+/*
+ * Add, alter or remove (if perm == 0) an ACE in a key's ACL.
+ */
+long keyctl_grant_permission(key_serial_t keyid,
+			     enum key_ace_subject_type type,
+			     unsigned int subject,
+			     unsigned int perm)
+{
+	struct key_ace new_ace;
+	struct key *key;
+	key_ref_t key_ref;
+	long ret;
+
+	new_ace.type = type;
+	new_ace.perm = perm;
+
+	switch (type) {
+	case KEY_ACE_SUBJ_STANDARD:
+		if (subject >= nr__key_ace_standard_subject)
+			return -ENOENT;
+		new_ace.subject_id = subject;
+		break;
+
+	default:
+		return -ENOENT;
+	}
+
+	key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL, KEY_NEED_CHANGE_ACL);
+	if (IS_ERR(key_ref)) {
+		ret = PTR_ERR(key_ref);
+		goto error;
+	}
+
+	key = key_ref_to_ptr(key_ref);
+
+	down_write(&key->sem);
+
+	/* If we're not the sysadmin, we can only change a key that we own */
+	ret = -EACCES;
+	if (capable(CAP_SYS_ADMIN) || uid_eq(key->uid, current_fsuid()))
+		ret = key_change_acl(key, &new_ace);
+
+	up_write(&key->sem);
+	key_put(key);
+error:
+	return ret;
+}



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RFC PATCH 4/5] keys: Split the search perms between KEY_NEED_USE and KEY_NEED_SEARCH
  2020-07-16 20:34 [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring David Howells
  2020-07-16 20:34 ` [RFC PATCH 1/5] keys: Move permissions checking decisions into the checking code David Howells
  2020-07-16 20:35 ` [RFC PATCH 3/5] keys: Provide KEYCTL_GRANT_PERMISSION David Howells
@ 2020-07-16 20:35 ` David Howells
  2020-07-16 20:35 ` [RFC PATCH 5/5] keys: Implement a 'container' keyring David Howells
  2020-07-19 18:10 ` [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring Eric W. Biederman
  4 siblings, 0 replies; 6+ messages in thread
From: David Howells @ 2020-07-16 20:35 UTC (permalink / raw)
  To: Stephen Smalley, Casey Schaufler
  Cc: dhowells, Jarkko Sakkinen, Eric Biederman, jlayton, christian,
	selinux, linux-afs, linux-nfs, linux-cifs, linux-api,
	linux-fsdevel, linux-security-module, linux-kernel, containers

Allow the permission needed for a keyring search to be specified and split
the permissions between KEY_NEED_USE (the kernel wants to do something with
the key) and KEY_NEED_SEARCH (userspace wants to do something with the
key).

This primarily affects how request_key() works, differentiating implicit
calls (e.g. from filesystems) from userspace calling the request_key()
system call.

This will allow the kernel to find keys in a hidden container keyring, but
not the denizens of the container.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 certs/blacklist.c                        |    2 +-
 crypto/asymmetric_keys/asymmetric_type.c |    2 +-
 include/linux/key.h                      |    2 ++
 net/rxrpc/security.c                     |    2 +-
 security/keys/internal.h                 |    4 ++++
 security/keys/keyctl.c                   |    6 ++++--
 security/keys/keyring.c                  |   13 ++++++++-----
 security/keys/permission.c               |   31 ++++++++++++++++++++++++++++++
 security/keys/proc.c                     |    1 +
 security/keys/process_keys.c             |    8 ++++++--
 security/keys/request_key.c              |    5 +++++
 security/keys/request_key_auth.c         |    1 +
 12 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/certs/blacklist.c b/certs/blacklist.c
index aff83e3a9f49..29c3cb6254d9 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -123,7 +123,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, false);
+			      &key_type_blacklist, buffer, KEY_NEED_USE, 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 6e5fc8e31f01..4559ac2f0bb7 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -83,7 +83,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, true);
+			     &key_type_asymmetric, req, KEY_NEED_USE, 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 94a6d51464b5..0db5539366e7 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -296,6 +296,7 @@ extern struct key *key_alloc(struct key_type *type,
 #define KEY_ALLOC_BUILT_IN		0x0004	/* Key is built into kernel */
 #define KEY_ALLOC_BYPASS_RESTRICTION	0x0008	/* Override the check on restricted keyrings */
 #define KEY_ALLOC_UID_KEYRING		0x0010	/* allocating a user or user session keyring */
+#define KEY_ALLOC_USERSPACE_REQUEST	0x0020	/* Userspace requested the key */
 
 extern void key_revoke(struct key *key);
 extern void key_invalidate(struct key *key);
@@ -432,6 +433,7 @@ extern int keyring_clear(struct key *keyring);
 extern key_ref_t keyring_search(key_ref_t keyring,
 				struct key_type *type,
 				const char *description,
+				enum key_need_perm need_perm,
 				bool recurse);
 
 extern int keyring_add_key(struct key *keyring,
diff --git a/net/rxrpc/security.c b/net/rxrpc/security.c
index 9b1fb9ed0717..23077cfe3d44 100644
--- a/net/rxrpc/security.c
+++ b/net/rxrpc/security.c
@@ -141,7 +141,7 @@ bool rxrpc_look_up_server_security(struct rxrpc_local *local, struct rxrpc_sock
 
 	/* look through the service's keyring */
 	kref = keyring_search(make_key_ref(rx->securities, 1UL),
-			      &key_type_rxrpc_s, kdesc, true);
+			      &key_type_rxrpc_s, kdesc, KEY_NEED_USE, true);
 	if (IS_ERR(kref)) {
 		trace_rxrpc_abort(0, "SVK",
 				  sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
diff --git a/security/keys/internal.h b/security/keys/internal.h
index af2c9531c435..d0d1bce95674 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -131,6 +131,7 @@ struct keyring_search_context {
 	struct keyring_index_key index_key;
 	const struct cred	*cred;
 	struct key_match_data	match_data;
+	enum key_need_perm	need_perm;	/* Permission required for search */
 	unsigned		flags;
 #define KEYRING_SEARCH_NO_STATE_CHECK	0x0001	/* Skip state checks */
 #define KEYRING_SEARCH_DO_STATE_CHECK	0x0002	/* Override NO_STATE_CHECK */
@@ -196,6 +197,9 @@ extern void key_gc_keytype(struct key_type *ktype);
 extern int key_task_permission(const key_ref_t key_ref,
 			       const struct cred *cred,
 			       enum key_need_perm need_perm);
+extern int key_search_permission(const key_ref_t key_ref,
+				 struct keyring_search_context *ctx,
+				 enum key_need_perm need_perm);
 extern unsigned int key_acl_to_perm(const struct key_acl *acl);
 extern long key_set_acl(struct key *key, struct key_acl *acl);
 extern void key_put_acl(struct key_acl *acl);
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index fae2df676e30..54a2bfff9af2 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -225,7 +225,8 @@ SYSCALL_DEFINE4(request_key, const char __user *, _type,
 	key = request_key_and_link(ktype, description, NULL, callout_info,
 				   callout_len, NULL, NULL,
 				   key_ref_to_ptr(dest_ref),
-				   KEY_ALLOC_IN_QUOTA);
+				   KEY_ALLOC_IN_QUOTA |
+				   KEY_ALLOC_USERSPACE_REQUEST);
 	if (IS_ERR(key)) {
 		ret = PTR_ERR(key);
 		goto error5;
@@ -685,7 +686,8 @@ long keyctl_keyring_search(key_serial_t ringid,
 	}
 
 	/* do the search */
-	key_ref = keyring_search(keyring_ref, ktype, description, true);
+	key_ref = keyring_search(keyring_ref, ktype, description,
+				 KEY_NEED_SEARCH, true);
 	if (IS_ERR(key_ref)) {
 		ret = PTR_ERR(key_ref);
 
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index f14aabf27a51..1779c95b428c 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -621,8 +621,8 @@ static int keyring_search_iterator(const void *object, void *iterator_data)
 
 	/* key must have search permissions */
 	if (!(ctx->flags & KEYRING_SEARCH_NO_CHECK_PERM) &&
-	    key_task_permission(make_key_ref(key, ctx->possessed),
-				ctx->cred, KEY_NEED_SEARCH) < 0) {
+	    key_search_permission(make_key_ref(key, ctx->possessed),
+				  ctx, ctx->need_perm) < 0) {
 		ctx->result = ERR_PTR(-EACCES);
 		kleave(" = %d [!perm]", ctx->skipped_ret);
 		goto skipped;
@@ -798,8 +798,8 @@ static bool search_nested_keyrings(struct key *keyring,
 
 		/* Search a nested keyring */
 		if (!(ctx->flags & KEYRING_SEARCH_NO_CHECK_PERM) &&
-		    key_task_permission(make_key_ref(key, ctx->possessed),
-					ctx->cred, KEY_NEED_SEARCH) < 0)
+		    key_search_permission(make_key_ref(key, ctx->possessed),
+					  ctx, KEY_NEED_SEARCH) < 0)
 			continue;
 
 		/* stack the current position */
@@ -921,7 +921,7 @@ key_ref_t keyring_search_rcu(key_ref_t keyring_ref,
 		return ERR_PTR(-ENOTDIR);
 
 	if (!(ctx->flags & KEYRING_SEARCH_NO_CHECK_PERM)) {
-		err = key_task_permission(keyring_ref, ctx->cred, KEY_NEED_SEARCH);
+		err = key_search_permission(keyring_ref, ctx, ctx->need_perm);
 		if (err < 0)
 			return ERR_PTR(err);
 	}
@@ -937,6 +937,7 @@ 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.
+ * @need_perm: The permission required of the target key.
  * @recurse: True to search the children of @keyring also
  *
  * As keyring_search_rcu() above, but using the current task's credentials and
@@ -945,6 +946,7 @@ key_ref_t keyring_search_rcu(key_ref_t keyring_ref,
 key_ref_t keyring_search(key_ref_t keyring,
 			 struct key_type *type,
 			 const char *description,
+			 enum key_need_perm need_perm,
 			 bool recurse)
 {
 	struct keyring_search_context ctx = {
@@ -955,6 +957,7 @@ key_ref_t keyring_search(key_ref_t keyring,
 		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.need_perm		= need_perm,
 		.flags			= KEYRING_SEARCH_DO_STATE_CHECK,
 	};
 	key_ref_t key;
diff --git a/security/keys/permission.c b/security/keys/permission.c
index 0bb7f6b695f4..3ae4d9aedc3a 100644
--- a/security/keys/permission.c
+++ b/security/keys/permission.c
@@ -253,6 +253,37 @@ int key_task_permission(const key_ref_t key_ref, const struct cred *cred,
 	return security_key_permission(key_ref, cred, need_perm, notes);
 }
 
+/**
+ * key_search_permission - Check a key can be searched for
+ * @key_ref: The key to check.
+ * @cred: The credentials to use.
+ * @need_perm: The permission required.
+ *
+ * Check to see whether permission is granted to use a key in the desired way,
+ * but permit the security modules to override.
+ *
+ * The caller must hold the RCU readlock.
+ *
+ * Returns 0 if successful, -EACCES if access is denied based on the
+ * permissions bits or the LSM check.
+ */
+int key_search_permission(const key_ref_t key_ref,
+			  struct keyring_search_context *ctx,
+			  enum key_need_perm need_perm)
+{
+	unsigned int allow, notes = 0;
+	int ret;
+
+	allow = key_resolve_acl(key_ref, ctx->cred);
+
+	ret = check_key_permission(key_ref, ctx->cred, allow, need_perm, &notes);
+	if (ret < 0)
+		return ret;
+
+	/* Let the LSMs be the final arbiter */
+	return security_key_permission(key_ref, ctx->cred, need_perm, notes);
+}
+
 /**
  * key_validate - Validate a key.
  * @key: The key to be validated.
diff --git a/security/keys/proc.c b/security/keys/proc.c
index c68ec5f98659..a6b349ee1759 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -174,6 +174,7 @@ 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,
+		.need_perm		= KEY_NEED_SEARCH,
 		.flags			= (KEYRING_SEARCH_NO_STATE_CHECK |
 					   KEYRING_SEARCH_RECURSE),
 	};
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 11227101bea0..3721f96dd6fb 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -135,7 +135,8 @@ int look_up_user_keyrings(struct key **_user_keyring,
 	 */
 	snprintf(buf, sizeof(buf), "_uid.%u", uid);
 	uid_keyring_r = keyring_search(make_key_ref(reg_keyring, true),
-				       &key_type_keyring, buf, false);
+				       &key_type_keyring, buf, KEY_NEED_SEARCH,
+				       false);
 	kdebug("_uid %p", uid_keyring_r);
 	if (uid_keyring_r == ERR_PTR(-EAGAIN)) {
 		uid_keyring = keyring_alloc(buf, cred->user->uid, INVALID_GID,
@@ -157,7 +158,8 @@ int look_up_user_keyrings(struct key **_user_keyring,
 	/* 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);
+					   &key_type_keyring, buf, KEY_NEED_SEARCH,
+					   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,
@@ -230,6 +232,7 @@ struct key *get_user_session_keyring_rcu(const struct cred *cred)
 		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= buf,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.need_perm		= KEY_NEED_SEARCH,
 		.flags			= KEYRING_SEARCH_DO_STATE_CHECK,
 	};
 
@@ -648,6 +651,7 @@ 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,
+		.need_perm		= KEY_NEED_SEARCH,
 		.flags			= (KEYRING_SEARCH_NO_STATE_CHECK |
 					   KEYRING_SEARCH_RECURSE),
 	};
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 2b84efb420cb..479ae0573d1e 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -567,6 +567,7 @@ struct key *request_key_and_link(struct key_type *type,
 		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.need_perm		= KEY_NEED_USE,
 		.flags			= (KEYRING_SEARCH_DO_STATE_CHECK |
 					   KEYRING_SEARCH_SKIP_EXPIRED |
 					   KEYRING_SEARCH_RECURSE),
@@ -579,6 +580,9 @@ struct key *request_key_and_link(struct key_type *type,
 	       ctx.index_key.type->name, ctx.index_key.description,
 	       callout_info, callout_len, aux, dest_keyring, flags);
 
+	if (flags & KEY_ALLOC_USERSPACE_REQUEST)
+		ctx.need_perm = KEY_NEED_SEARCH;
+
 	if (type->match_preparse) {
 		ret = type->match_preparse(&ctx.match_data);
 		if (ret < 0) {
@@ -774,6 +778,7 @@ struct key *request_key_rcu(struct key_type *type,
 		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.need_perm		= KEY_NEED_USE,
 		.flags			= (KEYRING_SEARCH_DO_STATE_CHECK |
 					   KEYRING_SEARCH_SKIP_EXPIRED),
 	};
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index ee8c5fe6ed61..f8f77af152de 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -264,6 +264,7 @@ 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,
+		.need_perm		= KEY_NEED_USE,
 		.flags			= (KEYRING_SEARCH_DO_STATE_CHECK |
 					   KEYRING_SEARCH_RECURSE),
 	};



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RFC PATCH 5/5] keys: Implement a 'container' keyring
  2020-07-16 20:34 [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring David Howells
                   ` (2 preceding siblings ...)
  2020-07-16 20:35 ` [RFC PATCH 4/5] keys: Split the search perms between KEY_NEED_USE and KEY_NEED_SEARCH David Howells
@ 2020-07-16 20:35 ` David Howells
  2020-07-19 18:10 ` [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring Eric W. Biederman
  4 siblings, 0 replies; 6+ messages in thread
From: David Howells @ 2020-07-16 20:35 UTC (permalink / raw)
  To: Stephen Smalley, Casey Schaufler
  Cc: dhowells, Jarkko Sakkinen, Eric Biederman, jlayton, christian,
	selinux, linux-afs, linux-nfs, linux-cifs, linux-api,
	linux-fsdevel, linux-security-module, linux-kernel, containers

Implement a per-container keyring, dangling it off of a user_namespace.
The properties of this keyring are such that it's searched by request_key()
selectively before or after the other keyrings have been searched, but the
keys in it don't grant possession to the denizens of the container, and so
the denizens can't see such keys unless those keys grant direct access
through the ACL.  The kernel recurses up the user_namespace stack looking
for keyrings.

The container manager, however, can access the keyring, and can add, update
and remove keys therein.

This allows the container manager to push filesystem authentication keys,
for example, into the container and to keep them refreshed without the
denizens of the container needing to know anything about it.

To this end, the following pieces are also added:

 (1) A new keyctl function, KEYCTL_GET_CONTAINER_KEYRING, to get the
     container keyring from a user namespace:

	keyring = keyctl_get_userns_keyring(int userns_fd,
					    key_serial_t dest_keyring);

     Get the container keyring attached to a user namespace, creating it if
     it doesn't exist.  A file descriptor pointing to the user namespace
     must be supplied.  The keyring will be linked into the destination
     keyring if one is supplied (ie. not 0).  The keyring will be owned by
     the user_namespace's owner and will grant various permissions to the
     possessor.

 (2) An ACL ACE type that allows access to a key by a container:

	keyctl_grant_acl(key_serial_t key,
			 KEY_ACE_SUBJ_CONTAINER,
			 int userns_fd,
			 KEY_ACE_SEARCH);

     This grants the kernel the ability to use a key on behalf of the
     denizens of a container, but doesn't grant any other rights, including
     the ability of the denizens see the key even exists.

This can then be tested with something like the following from the command
line:

 (1) Get the container keyring for a user namespace and link it to the
     session keyring.  The container is referenced as file descriptor 5.

	# keyctl get_container 5 @s 5</proc/self/ns/user
	197321290

 (2) Get a key that should be placed into the container, e.g.:

	# kinit foo@EXAMPLE.COM
	# aklog-kafs example.com

     This, say, adds key 748104263 to the session keyring.

 (3) Grant permission to the container to use the key:

	# keyctl grant 748104263 cont:5 s 5</proc/self/ns/user

 (4) Move (or link) the key into the container keyring:

	# keyctl move 748104263 @s 197321290

 (5) View the resultant keyrings:

	# keyctl show
	Session Keyring
	 711486290 --alswrv      0     0  keyring: _ses
	 468790230 ---lswrv      0 65534   \_ keyring: _uid.0
	 197321290 ----swrv      0 65534   \_ keyring: .container
	 748104263 --alswrv      0     0       \_ rxrpc: afs@example.com

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/key.h            |   13 +++-
 include/linux/user_namespace.h |    7 ++
 include/uapi/linux/keyctl.h    |    3 +
 security/keys/Kconfig          |   11 +++
 security/keys/compat.c         |    3 +
 security/keys/internal.h       |   10 +++
 security/keys/key.c            |    2 -
 security/keys/keyctl.c         |  128 ++++++++++++++++++++++++++++++++++++++++
 security/keys/keyring.c        |    7 ++
 security/keys/permission.c     |   94 +++++++++++++++++++++++++++--
 security/keys/proc.c           |    2 -
 security/keys/process_keys.c   |   36 +++++++++++
 12 files changed, 302 insertions(+), 14 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 0db5539366e7..810ea1ce01f4 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -112,7 +112,8 @@ struct key_ace {
 	union {
 		kuid_t		uid;
 		kgid_t		gid;
-		unsigned int	subject_id;
+		unsigned long	subject_id;
+		struct key_tag	*subject_tag;
 	};
 };
 
@@ -124,13 +125,13 @@ struct key_acl {
 	struct key_ace		aces[];
 };
 
-#define KEY_POSSESSOR_ACE(perms) {			\
+#define KEY_POSSESSOR_ACE(perms) (struct key_ace){	\
 		.type = KEY_ACE_SUBJ_STANDARD,		\
 		.perm = perms,				\
 		.subject_id = KEY_ACE_POSSESSOR		\
 	}
 
-#define KEY_OWNER_ACE(perms) {				\
+#define KEY_OWNER_ACE(perms) (struct key_ace){		\
 		.type = KEY_ACE_SUBJ_STANDARD,		\
 		.perm = perms,				\
 		.subject_id = KEY_ACE_OWNER		\
@@ -320,6 +321,12 @@ static inline void key_ref_put(key_ref_t key_ref)
 	key_put(key_ref_to_ptr(key_ref));
 }
 
+static inline struct key_tag *key_get_tag(struct key_tag *tag)
+{
+	refcount_inc(&tag->usage);
+	return tag;
+}
+
 extern struct key *request_key_tag(struct key_type *type,
 				   const char *description,
 				   struct key_tag *domain_tag,
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 6ef1c7109fc4..f007258bd3d9 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -79,6 +79,13 @@ struct user_namespace {
 	/* Register of per-UID persistent keyrings for this namespace */
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	struct key		*persistent_keyring_register;
+#endif
+	/* Ring of keys that the namespace owner can insert into the
+	 * namespace for transparent access by the denizens.
+	 */
+#ifdef CONFIG_CONTAINER_KEYRINGS
+	struct key		*container_keyring;
+	struct key_tag		*container_subj;	/* The ACE subject to match */
 #endif
 	struct work_struct	work;
 #ifdef CONFIG_SYSCTL
diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index a5938f2c3e66..2579fe75b93f 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -20,6 +20,7 @@
  */
 enum key_ace_subject_type {
 	KEY_ACE_SUBJ_STANDARD	= 0,	/* subject is one of key_ace_standard_subject */
+	KEY_ACE_SUBJ_CONTAINER	= 1,	/* Subject is an fd referring to a container (eg. userns) */
 	nr__key_ace_subject_type
 };
 
@@ -134,6 +135,7 @@ enum key_ace_standard_subject {
 #define KEYCTL_CAPABILITIES		31	/* Find capabilities of keyrings subsystem */
 #define KEYCTL_WATCH_KEY		32	/* Watch a key or ring of keys for changes */
 #define KEYCTL_GRANT_PERMISSION		33	/* Grant a permit to a key */
+#define KEYCTL_GET_CONTAINER_KEYRING	34	/* Get a container keyring */
 
 /* keyctl structures */
 struct keyctl_dh_params {
@@ -198,5 +200,6 @@ struct keyctl_pkey_params {
 #define KEYCTL_CAPS1_NOTIFICATIONS	0x04 /* Keys generate watchable notifications */
 #define KEYCTL_CAPS1_ACL		0x08 /* Keys have ACLs rather than a p-u-g-o bitmask */
 #define KEYCTL_CAPS1_GRANT_PERMISSION	0x10 /* KEYCTL_GRANT_PERMISSION is supported */
+#define KEYCTL_CAPS1_CONTAINER_KEYRINGS	0x20 /* Container keyrings are supported */
 
 #endif /*  _LINUX_KEYCTL_H */
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index 83bc23409164..df138027942e 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -123,3 +123,14 @@ config KEY_NOTIFICATIONS
 	  and keyrings on which the caller has View permission.  This makes use
 	  of the /dev/watch_queue misc device to handle the notification
 	  buffer and provides KEYCTL_WATCH_KEY to enable/disable watches.
+
+config CONTAINER_KEYRINGS
+	bool "Provide per-container keyrings"
+	depends on KEYS && USER_NS
+	help
+	  This option provides a keyring on each user_namespace that is
+	  searched by request_key() after it has searched the normal process
+	  keyrings.  This is a place that the container manager can insert
+	  filesystem authentication keys into a container so that the denizens
+	  can use authenticated storage without having to do anything for
+	  themselves - the manager can take care of that.
diff --git a/security/keys/compat.c b/security/keys/compat.c
index 2b675f9a6162..3c9eecb43bba 100644
--- a/security/keys/compat.c
+++ b/security/keys/compat.c
@@ -161,6 +161,9 @@ COMPAT_SYSCALL_DEFINE5(keyctl, u32, option,
 	case KEYCTL_WATCH_KEY:
 		return keyctl_watch_key(arg2, arg3, arg4);
 
+	case KEYCTL_GET_CONTAINER_KEYRING:
+		return keyctl_get_container_keyring(arg2, arg3);
+
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/security/keys/internal.h b/security/keys/internal.h
index d0d1bce95674..e3218aa72d4c 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -144,6 +144,7 @@ struct keyring_search_context {
 	int (*iterator)(const void *object, void *iterator_data);
 
 	/* Internal stuff */
+	const struct key_tag	*container_subj; /* The ACE container subject or NULL */
 	int			skipped_ret;
 	bool			possessed;
 	key_ref_t		result;
@@ -386,6 +387,15 @@ extern long keyctl_grant_permission(key_serial_t keyid,
 				    unsigned int subject,
 				    unsigned int perm);
 
+#ifdef CONFIG_KEY_NOTIFICATIONS
+extern long keyctl_get_container_keyring(int container_fd, key_serial_t destringid);
+#else
+static inline long keyctl_get_container_keyring(int container_fd, key_serial_t destringid)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
 /*
  * Debugging key validation
  */
diff --git a/security/keys/key.c b/security/keys/key.c
index 51491c07d7b9..17da784de9e8 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -318,7 +318,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 		goto security_error;
 
 	/* publish the key by giving it a serial number */
-	refcount_inc(&key->domain_tag->usage);
+	key_get_tag(key->domain_tag);
 	atomic_inc(&user->nkeys);
 	key_alloc_serial(key);
 
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 54a2bfff9af2..786da7382b7a 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -21,6 +21,10 @@
 #include <linux/security.h>
 #include <linux/uio.h>
 #include <linux/uaccess.h>
+#include <linux/file.h>
+#include <linux/proc_fs.h>
+#include <linux/proc_ns.h>
+#include <linux/user_namespace.h>
 #include <keys/request_key_auth-type.h>
 #include "internal.h"
 
@@ -40,7 +44,8 @@ static const unsigned char keyrings_capabilities[2] = {
 	       KEYCTL_CAPS1_NS_KEY_TAG |
 	       (IS_ENABLED(CONFIG_KEY_NOTIFICATIONS)	? KEYCTL_CAPS1_NOTIFICATIONS : 0) |
 	       KEYCTL_CAPS1_ACL |
-	       KEYCTL_CAPS1_GRANT_PERMISSION
+	       KEYCTL_CAPS1_GRANT_PERMISSION |
+	       (IS_ENABLED(CONFIG_CONTAINER_KEYRINGS)	? KEYCTL_CAPS1_CONTAINER_KEYRINGS : 0)
 	       ),
 };
 
@@ -1758,6 +1763,124 @@ long keyctl_watch_key(key_serial_t id, int watch_queue_fd, int watch_id)
 }
 #endif /* CONFIG_KEY_NOTIFICATIONS */
 
+#ifdef CONFIG_CONTAINER_KEYRINGS
+/*
+ * Create a container keyring for a user namespace and add it.
+ */
+static struct key *key_create_container_keyring(struct user_namespace *user_ns)
+{
+	struct key_tag *tag;
+	struct key_acl *acl;
+	struct key *keyring;
+
+	keyring = key_get(user_ns->container_keyring);
+	if (keyring)
+		return keyring;
+
+	/* We're going to need a subject tag... */
+	tag = user_ns->container_subj;
+	if (!tag) {
+		tag = kzalloc(sizeof(struct key_tag), GFP_KERNEL);
+		if (!tag)
+			return ERR_PTR(-ENOMEM);
+		refcount_set(&tag->usage, 1);
+		user_ns->container_subj = tag;
+	}
+
+	/* ...  so that we can grant the container denizens search permission
+	 * on the keyring.
+	 */
+	acl = kzalloc(struct_size(acl, aces, 3), GFP_KERNEL);
+	if (!acl)
+		return ERR_PTR(-ENOMEM);
+
+	refcount_set(&acl->usage, 1);
+	acl->possessor_viewable = true;
+	acl->nr_ace		= 3;
+
+	acl->aces[0] = KEY_POSSESSOR_ACE(KEY_ACE_VIEW | KEY_ACE_READ | KEY_ACE_WRITE |
+					 KEY_ACE_CLEAR | KEY_ACE_SEARCH);
+	acl->aces[1] = KEY_OWNER_ACE(KEY_ACE_VIEW | KEY_ACE_READ);
+
+	acl->aces[2].type = KEY_ACE_SUBJ_CONTAINER;
+	acl->aces[2].subject_tag = key_get_tag(tag);
+	acl->aces[2].perm = KEY_ACE_SEARCH;
+
+	keyring = keyring_alloc(".container", user_ns->owner, INVALID_GID,
+				current_cred(), acl, 0, NULL, NULL);
+	key_put_acl(acl);
+	if (IS_ERR(keyring))
+		return keyring;
+
+	smp_store_release(&user_ns->container_keyring, key_get(keyring));
+	return keyring;
+}
+
+/*
+ * Get the container keyring attached to a container.  The container is
+ * referenced by a file descriptor referring to, say, a user_namespace.
+ */
+long keyctl_get_container_keyring(int container_fd, key_serial_t destringid)
+{
+	struct user_namespace *user_ns;
+	struct ns_common *ns;
+	struct file *f;
+	struct key *keyring;
+	key_ref_t dest_ref;
+	int ret = -EINVAL;
+
+	f = fget(container_fd);
+	if (!f)
+		return -EBADF;
+
+	if (!proc_ns_file(f))
+		goto error_file;
+	ns = get_proc_ns(file_inode(f));
+	if (ns->ops->type != CLONE_NEWUSER)
+		goto error_file;
+	user_ns = container_of(ns, struct user_namespace, ns);
+
+	keyring = key_get(READ_ONCE(user_ns->container_keyring));
+	if (!keyring) {
+		down_write(&user_ns->keyring_sem);
+		keyring = key_create_container_keyring(user_ns);
+		up_write(&user_ns->keyring_sem);
+		if (IS_ERR(keyring)) {
+			ret = PTR_ERR(keyring);
+			goto error_file;
+		}
+	}
+
+	/* Get the destination keyring if specified.  We don't need LINK
+	 * permission on the container keyring as having the container fd is
+	 * sufficient to grant us that.
+	 */
+	dest_ref = NULL;
+	if (destringid) {
+		dest_ref = lookup_user_key(destringid, KEY_LOOKUP_CREATE,
+					   KEY_NEED_KEYRING_ADD);
+		if (IS_ERR(dest_ref)) {
+			ret = PTR_ERR(dest_ref);
+			goto error_keyring;
+		}
+
+		ret = key_link(key_ref_to_ptr(dest_ref), keyring);
+		if (ret < 0)
+			goto error_dest;
+	}
+
+	ret = key_serial(keyring);
+
+error_dest:
+	key_ref_put(dest_ref);
+error_keyring:
+	key_put(keyring);
+error_file:
+	fput(f);
+	return ret;
+}
+#endif
+
 /*
  * Get keyrings subsystem capabilities.
  */
@@ -1935,6 +2058,9 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case KEYCTL_WATCH_KEY:
 		return keyctl_watch_key((key_serial_t)arg2, (int)arg3, (int)arg4);
 
+	case KEYCTL_GET_CONTAINER_KEYRING:
+		return keyctl_get_container_keyring((int)arg2, (key_serial_t)arg3);
+
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 1779c95b428c..eb311987bb9b 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -64,6 +64,13 @@ void key_free_user_ns(struct user_namespace *ns)
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	key_put(ns->persistent_keyring_register);
 #endif
+#ifdef CONFIG_CONTAINER_KEYRINGS
+	if (ns->container_subj) {
+		ns->container_subj->removed = true;
+		key_put_tag(ns->container_subj);
+	}
+	key_put(ns->container_keyring);
+#endif
 }
 
 /*
diff --git a/security/keys/permission.c b/security/keys/permission.c
index 3ae4d9aedc3a..b984e1ce7e5d 100644
--- a/security/keys/permission.c
+++ b/security/keys/permission.c
@@ -7,6 +7,10 @@
 
 #include <linux/export.h>
 #include <linux/security.h>
+#include <linux/file.h>
+#include <linux/proc_fs.h>
+#include <linux/proc_ns.h>
+#include <linux/user_namespace.h>
 #include <keys/request_key_auth-type.h>
 #include "internal.h"
 
@@ -177,7 +181,9 @@ static int check_key_permission(const key_ref_t key_ref, const struct cred *cred
 /*
  * Resolve an ACL to a mask.
  */
-static unsigned int key_resolve_acl(const key_ref_t key_ref, const struct cred *cred)
+static unsigned int key_resolve_acl(const key_ref_t key_ref,
+				    const struct cred *cred,
+				    const struct key_tag *tag)
 {
 	const struct key *key = key_ref_to_ptr(key_ref);
 	const struct key_acl *acl;
@@ -215,6 +221,11 @@ static unsigned int key_resolve_acl(const key_ref_t key_ref, const struct cred *
 				break;
 			}
 			break;
+
+		case KEY_ACE_SUBJ_CONTAINER:
+			if (ace->subject_tag == tag)
+				allow |= ace->perm;
+			break;
 		}
 	}
 
@@ -242,7 +253,7 @@ int key_task_permission(const key_ref_t key_ref, const struct cred *cred,
 	int ret;
 
 	rcu_read_lock();
-	allow = key_resolve_acl(key_ref, cred);
+	allow = key_resolve_acl(key_ref, cred, NULL);
 	rcu_read_unlock();
 
 	ret = check_key_permission(key_ref, cred, allow, need_perm, &notes);
@@ -274,7 +285,7 @@ int key_search_permission(const key_ref_t key_ref,
 	unsigned int allow, notes = 0;
 	int ret;
 
-	allow = key_resolve_acl(key_ref, ctx->cred);
+	allow = key_resolve_acl(key_ref, ctx->cred, ctx->container_subj);
 
 	ret = check_key_permission(key_ref, ctx->cred, allow, need_perm, &notes);
 	if (ret < 0)
@@ -374,13 +385,24 @@ unsigned int key_acl_to_perm(const struct key_acl *acl)
 	return perm;
 }
 
+static void key_free_acl(struct rcu_head *rcu)
+{
+	struct key_acl *acl = container_of(rcu, struct key_acl, rcu);
+	unsigned int i;
+
+	for (i = 0; i < acl->nr_ace; i++)
+		if (acl->aces[i].type == KEY_ACE_SUBJ_CONTAINER)
+			key_put_tag(acl->aces[i].subject_tag);
+	kfree(acl);
+}
+
 /*
  * Destroy a key's ACL.
  */
 void key_put_acl(struct key_acl *acl)
 {
 	if (acl && refcount_dec_and_test(&acl->usage))
-		kfree_rcu(acl, rcu);
+		call_rcu(&acl->rcu, key_free_acl);
 }
 
 /*
@@ -440,7 +462,8 @@ static struct key_acl *key_alloc_acl(const struct key_acl *old_acl, int nr, int
 }
 
 /*
- * Generate the revised ACL.
+ * Generate the revised ACL.  If the new ACE contains a key_tag and we don't
+ * have the tag in ACL yet, we steal the tag and clear the caller's pointer.
  */
 static long key_change_acl(struct key *key, struct key_ace *new_ace)
 {
@@ -461,6 +484,7 @@ static long key_change_acl(struct key *key, struct key_ace *new_ace)
 	if (IS_ERR(acl))
 		return PTR_ERR(acl);
 	acl->aces[i] = *new_ace;
+	new_ace->subject_tag = NULL; /* Stole the tag */
 	goto change;
 
 found_match:
@@ -484,6 +508,49 @@ static long key_change_acl(struct key *key, struct key_ace *new_ace)
 	return key_set_acl(key, acl);
 }
 
+/*
+ * Look up the user namespace tag associated with a fd.
+ */
+static struct key_tag *key_get_ns_tag(int fd)
+{
+#ifdef CONFIG_CONTAINER_KEYRINGS
+	struct user_namespace *userns;
+	struct ns_common *ns;
+	struct key_tag *tag = ERR_PTR(-EINVAL), *candidate;
+	struct file *f;
+
+	f = fget(fd);
+	if (!f)
+		return ERR_PTR(-EBADF);
+
+	if (!proc_ns_file(f))
+		goto error;
+	ns = get_proc_ns(file_inode(f));
+	if (ns->ops->type != CLONE_NEWUSER)
+		goto error;
+
+	userns = container_of(ns, struct user_namespace, ns);
+	if (!userns->container_subj) {
+		candidate = kzalloc(sizeof(struct key_tag), GFP_KERNEL);
+		refcount_set(&candidate->usage, 1);
+		down_write(&userns->keyring_sem);
+		if (!userns->container_subj) {
+			userns->container_subj = candidate;
+			candidate = NULL;
+		}
+		up_write(&userns->keyring_sem);
+		kfree(candidate);
+	}
+
+	tag = key_get_tag(userns->container_subj);
+error:
+	fput(f);
+	return tag;
+#else
+	return ERR_PTR(-EOPNOTSUPP);
+#endif
+}
+
 /*
  * Add, alter or remove (if perm == 0) an ACE in a key's ACL.
  */
@@ -492,13 +559,15 @@ long keyctl_grant_permission(key_serial_t keyid,
 			     unsigned int subject,
 			     unsigned int perm)
 {
-	struct key_ace new_ace;
+	struct key_tag *tag;
 	struct key *key;
 	key_ref_t key_ref;
 	long ret;
 
-	new_ace.type = type;
-	new_ace.perm = perm;
+	struct key_ace new_ace = {
+		.type = type,
+		.perm = perm,
+	};
 
 	switch (type) {
 	case KEY_ACE_SUBJ_STANDARD:
@@ -507,6 +576,13 @@ long keyctl_grant_permission(key_serial_t keyid,
 		new_ace.subject_id = subject;
 		break;
 
+	case KEY_ACE_SUBJ_CONTAINER:
+		tag = key_get_ns_tag(subject);
+		if (IS_ERR(tag))
+			return PTR_ERR(tag);
+		new_ace.subject_tag = tag;
+		break;
+
 	default:
 		return -ENOENT;
 	}
@@ -529,5 +605,7 @@ long keyctl_grant_permission(key_serial_t keyid,
 	up_write(&key->sem);
 	key_put(key);
 error:
+	if (new_ace.type == KEY_ACE_SUBJ_CONTAINER && new_ace.subject_tag)
+		key_put_tag(new_ace.subject_tag);
 	return ret;
 }
diff --git a/security/keys/proc.c b/security/keys/proc.c
index a6b349ee1759..ba3be0e9c97d 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -183,7 +183,7 @@ static int proc_keys_show(struct seq_file *m, void *v)
 	check_pos = acl->possessor_viewable;
 
 	/* determine if the key is possessed by this process (a test we can
-	 * skip if the key does not indicate the possessor can view it
+	 * skip if the key does not indicate the possessor can view it)
 	 */
 	key_ref = make_key_ref(key, 0);
 	if (check_pos) {
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 3721f96dd6fb..af09db1e5984 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -458,6 +458,7 @@ void key_fsgid_changed(struct cred *new_cred)
  */
 key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx)
 {
+	struct user_namespace *userns;
 	struct key *user_session;
 	key_ref_t key_ref, ret, err;
 	const struct cred *cred = ctx->cred;
@@ -556,6 +557,41 @@ key_ref_t search_cred_keyrings_rcu(struct keyring_search_context *ctx)
 		}
 	}
 
+#ifdef CONFIG_CONTAINER_KEYRINGS
+	for (userns = cred->user_ns; userns; userns = userns->parent) {
+		if (!userns->container_keyring || !userns->container_subj)
+			continue;
+
+		/* The denizens of the container don't possess the key
+		 * and have a special subject to match.
+		 */
+		ctx->container_subj = userns->container_subj;
+		key_ref = keyring_search_rcu(make_key_ref(userns->container_keyring,
+							  false), ctx);
+		ctx->container_subj = NULL;
+
+		if (!IS_ERR(key_ref))
+			goto found;
+
+		switch (PTR_ERR(key_ref)) {
+		case -EAGAIN: /* no key */
+			if (ret)
+				break;
+			/* fall through */
+		case -ENOKEY: /* negative key */
+			ret = key_ref;
+			break;
+		default:
+			/* Hmmm...  should we admit to the denizens of
+			 * the container that a key exists?
+			 */
+			err = key_ref;
+			break;
+		}
+	}
+
+#endif
+
 	/* no key - decide on the error we're going to go for */
 	key_ref = ret ? ret : err;
 



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring
  2020-07-16 20:34 [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring David Howells
                   ` (3 preceding siblings ...)
  2020-07-16 20:35 ` [RFC PATCH 5/5] keys: Implement a 'container' keyring David Howells
@ 2020-07-19 18:10 ` Eric W. Biederman
  4 siblings, 0 replies; 6+ messages in thread
From: Eric W. Biederman @ 2020-07-19 18:10 UTC (permalink / raw)
  To: David Howells
  Cc: Stephen Smalley, Casey Schaufler, keyrings, Jarkko Sakkinen,
	Paul Moore, selinux, jlayton, christian, linux-afs, linux-nfs,
	linux-cifs, linux-api, linux-fsdevel, linux-security-module,
	linux-kernel, containers, Linus Torvalds

David Howells <dhowells@redhat.com> writes:

> Here are some patches to provide some security changes and some container
> support:

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

There remain unfixed security issues in the new mount api.   Those need
to get fixed before it is even worth anyones time reviewing new code.

Those issues came up in the review.  I successfully demonstrated how to
address the security issues in the new mount api before the code was
merged.  Yet the code was merged with the security issues present,
and I have not seem those issues addressed.

So far I have had to rewrite two filesystems because of bugs in the
mount API.

Enough is enough.  Let's get the what has already been merged sorted
out before we had more.

Eric

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-07-19 18:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16 20:34 [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring David Howells
2020-07-16 20:34 ` [RFC PATCH 1/5] keys: Move permissions checking decisions into the checking code David Howells
2020-07-16 20:35 ` [RFC PATCH 3/5] keys: Provide KEYCTL_GRANT_PERMISSION David Howells
2020-07-16 20:35 ` [RFC PATCH 4/5] keys: Split the search perms between KEY_NEED_USE and KEY_NEED_SEARCH David Howells
2020-07-16 20:35 ` [RFC PATCH 5/5] keys: Implement a 'container' keyring David Howells
2020-07-19 18:10 ` [RFC PATCH 0/5] keys: Security changes, ACLs and Container keyring Eric W. Biederman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).