Revert "Merge tag 'keys-acl-20190703' of git://git.kernel.org/pub/scm…

…/linux/kernel/git/dhowells/linux-fs" This reverts merge 0f75ef6 (and thus effectively commits 7a1ade8 ("keys: Provide KEYCTL_GRANT_PERMISSION") 2e12256 ("keys: Replace uid/gid/perm permissions checking with an ACL") that the merge brought in). It turns out that it breaks booting with an encrypted volume, and Eric biggers reports that it also breaks the fscrypt tests [1] and loading of in-kernel X.509 certificates [2]. The root cause of all the breakage is likely the same, but David Howells is off email so rather than try to work it out it's getting reverted in order to not impact the rest of the merge window. [1] https://lore.kernel.org/lkml/[email protected]/ [2] https://lore.kernel.org/lkml/[email protected]/ Link: https://lore.kernel.org/lkml/CAHk-=wjxoeMJfeBahnWH=9zShKp2bsVy527vo3_y8HfOdhwAAw@mail.gmail.com/ Reported-by: Eric Biggers <[email protected]> Cc: David Howells <[email protected]> Cc: James Morris <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
mrcsosa · Jul 11, 2019 · 028db3e · 028db3e
1 parent e9a83bd
commit 028db3e
Show file tree

Hide file tree

Showing 46 changed files with 325 additions and 992 deletions.
diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst
@@ -57,9 +57,9 @@ Each key has a number of attributes:
      type provides an operation to perform a match between the description on a
      key and a criterion string.
 
-  *  Each key has an owner user ID, a group ID and an ACL.  These are used to
-     control what a process may do to a key from userspace, and whether a
-     kernel service will be able to find the key.
+  *  Each key has an owner user ID, a group ID and a permissions mask. These
+     are used to control what a process may do to a key from userspace, and
+     whether a kernel service will be able to find the key.
 
   *  Each key can be set to expire at a specific time by the key type's
      instantiation function. Keys can also be immortal.
@@ -198,110 +198,43 @@ The key service provides a number of features besides keys:
 Key Access Permissions
 ======================
 
-Keys have an owner user ID, a group ID and an ACL.  The ACL is made up of a
-sequence of ACEs that each contain three elements:
+Keys have an owner user ID, a group access ID, and a permissions mask. The mask
+has up to eight bits each for possessor, user, group and other access. Only
+six of each set of eight bits are defined. These permissions granted are:
 
-  * The type of subject.
-  * The subject.
+  *  View
 
-    These two together indicate the subject to whom the permits are granted.
-    The type can be one of:
+     This permits a key or keyring's attributes to be viewed - including key
+     type and description.
 
-     * ``KEY_ACE_SUBJ_STANDARD``
+  *  Read
 
-       The subject is a standard 'macro' type.  The subject can be one of:
-
-        * ``KEY_ACE_EVERYONE``
-
-	  The permits are granted to everyone.  It replaces the old 'other'
-	  type on the assumption that you wouldn't grant a permission to other
-	  that you you wouldn't grant to everyone else.
-
-	* ``KEY_ACE_OWNER``
-
-	  The permits are granted to the owner of the key (key->uid).
-
-	* ``KEY_ACE_GROUP``
-
-	  The permits are granted to the key's group (key->gid).
-
-	* ``KEY_ACE_POSSESSOR``
-
-	  The permits are granted to anyone who possesses the key.
-
-  * The set of permits granted to the subject.  These include:
-
-     * ``KEY_ACE_VIEW``
-
-       This permits a key or keyring's attributes to be viewed - including the
-       key type and description.
-
-     * ``KEY_ACE_READ``
-
-       This permits a key's payload to be viewed or a keyring's list of linked
-       keys.
-
-     * ``KEY_ACE_WRITE``
-
-       This permits a key's payload to be instantiated or updated, or it allows
-       a link to be added to or removed from a keyring.
-
-     * ``KEY_ACE_SEARCH``
-
-       This permits keyrings to be searched and keys to be found. Searches can
-       only recurse into nested keyrings that have search permission set.
-
-     * ``KEY_ACE_LINK``
-
-       This permits a key or keyring to be linked to. To create a link from a
-       keyring to a key, a process must have Write permission on the keyring
-       and Link permission on the key.
-
-     * ``KEY_ACE_SET_SECURITY``
-
-       This permits a key's UID, GID and permissions mask to be changed.
+     This permits a key's payload to be viewed or a keyring's list of linked
+     keys.
 
-     * ``KEY_ACE_INVAL``
+  *  Write
 
-       This permits a key to be invalidated with KEYCTL_INVALIDATE.
+     This permits a key's payload to be instantiated or updated, or it allows a
+     link to be added to or removed from a keyring.
 
-     * ``KEY_ACE_REVOKE``
+  *  Search
 
-       This permits a key to be revoked with KEYCTL_REVOKE.
+     This permits keyrings to be searched and keys to be found. Searches can
+     only recurse into nested keyrings that have search permission set.
 
-     * ``KEY_ACE_JOIN``
+  *  Link
 
-       This permits a keyring to be joined as a session by
-       KEYCTL_JOIN_SESSION_KEYRING or KEYCTL_SESSION_TO_PARENT.
+     This permits a key or keyring to be linked to. To create a link from a
+     keyring to a key, a process must have Write permission on the keyring and
+     Link permission on the key.
 
-     * ``KEY_ACE_CLEAR``
+  *  Set Attribute
 
-       This permits a keyring to be cleared.
+     This permits a key's UID, GID and permissions mask to be changed.
 
 For changing the ownership, group ID or permissions mask, being the owner of
 the key or having the sysadmin capability is sufficient.
 
-The legacy KEYCTL_SETPERM and KEYCTL_DESCRIBE functions can only see/generate
-View, Read, Write, Search, Link and SetAttr permits, and do this for each of
-possessor, user, group and other permission sets as a 32-bit flag mask.  These
-will be approximated/inferred:
-
-	SETPERM Permit	Implied ACE Permit
-	===============	=======================
-	Search		Inval, Join
-	Write		Revoke, Clear
-	Setattr		Set Security, Revoke
-
-	ACE Permit	Described as
-	===============	=======================
-	Inval		Search
-	Join		Search
-	Revoke		Write (unless Setattr)
-	Clear		write
-	Set Security	Setattr
-
-'Other' will be approximated as/inferred from the 'Everyone' subject.
-
 
 SELinux Support
 ===============
@@ -1151,8 +1084,7 @@ payload contents" for more information.
 
 	struct key *request_key(const struct key_type *type,
 				const char *description,
-				const char *callout_info,
-				struct key_acl *acl);
+				const char *callout_info);
 
     This is used to request a key or keyring with a description that matches
     the description specified according to the key type's match_preparse()
@@ -1167,8 +1099,6 @@ payload contents" for more information.
     If successful, the key will have been attached to the default keyring for
     implicitly obtained request-key keys, as set by KEYCTL_SET_REQKEY_KEYRING.
 
-    If a key is created, it will be given the specified ACL.
-
     See also Documentation/security/keys/request-key.rst.
 
 
@@ -1177,8 +1107,7 @@ payload contents" for more information.
 	struct key *request_key_tag(const struct key_type *type,
 				    const char *description,
 				    struct key_tag *domain_tag,
-				    const char *callout_info,
-				    struct key_acl *acl);
+				    const char *callout_info);
 
     This is identical to request_key(), except that a domain tag may be
     specifies that causes search algorithm to only match keys matching that
@@ -1193,8 +1122,7 @@ payload contents" for more information.
 					     struct key_tag *domain_tag,
 					     const void *callout_info,
 					     size_t callout_len,
-					     void *aux,
-					     struct key_acl *acl);
+					     void *aux);
 
     This is identical to request_key_tag(), except that the auxiliary data is
     passed to the key_type->request_key() op if it exists, and the
@@ -1267,7 +1195,7 @@ payload contents" for more information.
 
 	struct key *keyring_alloc(const char *description, uid_t uid, gid_t gid,
 				  const struct cred *cred,
-				  struct key_acl *acl,
+				  key_perm_t perm,
 				  struct key_restriction *restrict_link,
 				  unsigned long flags,
 				  struct key *dest);

diff --git a/Documentation/security/keys/request-key.rst b/Documentation/security/keys/request-key.rst
@@ -11,16 +11,14 @@ The process starts by either the kernel requesting a service by calling
 
 	struct key *request_key(const struct key_type *type,
 				const char *description,
-				const char *callout_info,
-				struct key_acl *acl);
+				const char *callout_info);
 
 or::
 
 	struct key *request_key_tag(const struct key_type *type,
 				    const char *description,
 				    const struct key_tag *domain_tag,
-				    const char *callout_info,
-				    struct key_acl *acl);
+				    const char *callout_info);
 
 or::
 
@@ -29,8 +27,7 @@ or::
 					     const struct key_tag *domain_tag,
 					     const char *callout_info,
 					     size_t callout_len,
-					     void *aux,
-					     struct key_acl *acl);
+					     void *aux);
 
 or::
 

diff --git a/certs/blacklist.c b/certs/blacklist.c
@@ -89,7 +89,8 @@ int mark_hash_blacklisted(const char *hash)
 				   hash,
 				   NULL,
 				   0,
-				   &internal_key_acl,
+				   ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+				    KEY_USR_VIEW),
 				   KEY_ALLOC_NOT_IN_QUOTA |
 				   KEY_ALLOC_BUILT_IN);
 	if (IS_ERR(key)) {
@@ -148,7 +149,9 @@ static int __init blacklist_init(void)
 		keyring_alloc(".blacklist",
 			      KUIDT_INIT(0), KGIDT_INIT(0),
 			      current_cred(),
-			      &internal_keyring_acl,
+			      (KEY_POS_ALL & ~KEY_POS_SETATTR) |
+			      KEY_USR_VIEW | KEY_USR_READ |
+			      KEY_USR_SEARCH,
 			      KEY_ALLOC_NOT_IN_QUOTA |
 			      KEY_FLAG_KEEP,
 			      NULL, NULL);

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
@@ -99,7 +99,9 @@ static __init int system_trusted_keyring_init(void)
 	builtin_trusted_keys =
 		keyring_alloc(".builtin_trusted_keys",
 			      KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
-			      &internal_key_acl, KEY_ALLOC_NOT_IN_QUOTA,
+			      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+			      KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH),
+			      KEY_ALLOC_NOT_IN_QUOTA,
 			      NULL, NULL);
 	if (IS_ERR(builtin_trusted_keys))
 		panic("Can't allocate builtin trusted keyring\n");
@@ -108,7 +110,10 @@ static __init int system_trusted_keyring_init(void)
 	secondary_trusted_keys =
 		keyring_alloc(".secondary_trusted_keys",
 			      KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
-			      &internal_writable_keyring_acl, KEY_ALLOC_NOT_IN_QUOTA,
+			      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+			       KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH |
+			       KEY_USR_WRITE),
+			      KEY_ALLOC_NOT_IN_QUOTA,
 			      get_builtin_and_secondary_restriction(),
 			      NULL);
 	if (IS_ERR(secondary_trusted_keys))
@@ -158,7 +163,8 @@ static __init int load_system_certificate_list(void)
 					   NULL,
 					   p,
 					   plen,
-					   &internal_key_acl,
+					   ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+					   KEY_USR_VIEW | KEY_USR_READ),
 					   KEY_ALLOC_NOT_IN_QUOTA |
 					   KEY_ALLOC_BUILT_IN |
 					   KEY_ALLOC_BYPASS_RESTRICTION);

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
@@ -2035,7 +2035,7 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string
 		return -ENOMEM;
 
 	key = request_key(key_string[0] == 'l' ? &key_type_logon : &key_type_user,
-			  key_desc + 1, NULL, NULL);
+			  key_desc + 1, NULL);
 	if (IS_ERR(key)) {
 		kzfree(new_key_string);
 		return PTR_ERR(key);

diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
@@ -55,7 +55,7 @@ static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
 	struct device *dev = &nvdimm->dev;
 
 	sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id);
-	key = request_key(&key_type_encrypted, desc, "", NULL);
+	key = request_key(&key_type_encrypted, desc, "");
 	if (IS_ERR(key)) {
 		if (PTR_ERR(key) == -ENOKEY)
 			dev_dbg(dev, "request_key() found no key\n");

diff --git a/fs/afs/security.c b/fs/afs/security.c
@@ -28,7 +28,7 @@ struct key *afs_request_key(struct afs_cell *cell)
 
 	_debug("key %s", cell->anonymous_key->description);
 	key = request_key(&key_type_rxrpc, cell->anonymous_key->description,
-			  NULL, NULL);
+			  NULL);
 	if (IS_ERR(key)) {
 		if (PTR_ERR(key) != -ENOKEY) {
 			_leave(" = %ld", PTR_ERR(key));

diff --git a/fs/cifs/cifs_spnego.c b/fs/cifs/cifs_spnego.c
@@ -32,25 +32,6 @@
 #include "cifsproto.h"
 static const struct cred *spnego_cred;
 
-static struct key_acl cifs_spnego_key_acl = {
-	.usage	= REFCOUNT_INIT(1),
-	.nr_ace	= 2,
-	.possessor_viewable = true,
-	.aces = {
-		KEY_POSSESSOR_ACE(KEY_ACE_VIEW | KEY_ACE_SEARCH | KEY_ACE_READ),
-		KEY_OWNER_ACE(KEY_ACE_VIEW),
-	}
-};
-
-static struct key_acl cifs_spnego_keyring_acl = {
-	.usage	= REFCOUNT_INIT(1),
-	.nr_ace	= 2,
-	.aces = {
-		KEY_POSSESSOR_ACE(KEY_ACE_SEARCH | KEY_ACE_WRITE),
-		KEY_OWNER_ACE(KEY_ACE_VIEW | KEY_ACE_READ | KEY_ACE_CLEAR),
-	}
-};
-
 /* create a new cifs key */
 static int
 cifs_spnego_key_instantiate(struct key *key, struct key_preparsed_payload *prep)
@@ -189,8 +170,7 @@ cifs_get_spnego_key(struct cifs_ses *sesInfo)
 
 	cifs_dbg(FYI, "key description = %s\n", description);
 	saved_cred = override_creds(spnego_cred);
-	spnego_key = request_key(&cifs_spnego_key_type, description, "",
-				 &cifs_spnego_key_acl);
+	spnego_key = request_key(&cifs_spnego_key_type, description, "");
 	revert_creds(saved_cred);
 
 #ifdef CONFIG_CIFS_DEBUG2
@@ -227,7 +207,8 @@ init_cifs_spnego(void)
 
 	keyring = keyring_alloc(".cifs_spnego",
 				GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred,
-				&cifs_spnego_keyring_acl,
+				(KEY_POS_ALL & ~KEY_POS_SETATTR) |
+				KEY_USR_VIEW | KEY_USR_READ,
 				KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
 	if (IS_ERR(keyring)) {
 		ret = PTR_ERR(keyring);