Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH 35/38] vfs: Convert securityfs to fs_context
From: David Howells @ 2019-03-14 16:13 UTC (permalink / raw)
  To: viro; +Cc: linux-security-module, dhowells, linux-fsdevel, linux-kernel
In-Reply-To: <155257972443.13720.11743171471060355965.stgit@warthog.procyon.org.uk>

Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-security-module@vger.kernel.org
---

 security/inode.c |   21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/security/inode.c b/security/inode.c
index b7772a9b315e..4fecbc8b072a 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -16,6 +16,7 @@
 #include <linux/sysfs.h>
 #include <linux/kobject.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
 #include <linux/mount.h>
 #include <linux/pagemap.h>
 #include <linux/init.h>
@@ -40,7 +41,7 @@ static const struct super_operations securityfs_super_operations = {
 	.evict_inode	= securityfs_evict_inode,
 };
 
-static int fill_super(struct super_block *sb, void *data, int silent)
+static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	static const struct tree_descr files[] = {{""}};
 	int error;
@@ -54,17 +55,25 @@ static int fill_super(struct super_block *sb, void *data, int silent)
 	return 0;
 }
 
-static struct dentry *get_sb(struct file_system_type *fs_type,
-		  int flags, const char *dev_name,
-		  void *data)
+static int securityfs_get_tree(struct fs_context *fc)
 {
-	return mount_single(fs_type, flags, data, fill_super);
+	return vfs_get_super(fc, vfs_get_single_super, securityfs_fill_super);
+}
+
+static const struct fs_context_operations securityfs_context_ops = {
+	.get_tree	= securityfs_get_tree,
+};
+
+static int securityfs_init_fs_context(struct fs_context *fc)
+{
+	fc->ops = &securityfs_context_ops;
+	return 0;
 }
 
 static struct file_system_type fs_type = {
 	.owner =	THIS_MODULE,
 	.name =		"securityfs",
-	.mount =	get_sb,
+	.init_fs_context = securityfs_init_fs_context,
 	.kill_sb =	kill_litter_super,
 };
 


^ permalink raw reply related

* [PATCH 36/38] vfs: Convert selinuxfs to fs_context
From: David Howells @ 2019-03-14 16:13 UTC (permalink / raw)
  To: viro
  Cc: Paul Moore, Stephen Smalley, Eric Paris, selinux,
	linux-security-module, dhowells, linux-fsdevel, linux-kernel
In-Reply-To: <155257972443.13720.11743171471060355965.stgit@warthog.procyon.org.uk>

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Paul Moore <paul@paul-moore.com>
cc: Stephen Smalley <sds@tycho.nsa.gov>
cc: Eric Paris <eparis@parisplace.org>
cc: selinux@vger.kernel.org
cc: linux-security-module@vger.kernel.org
---

 security/selinux/selinuxfs.c |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 145ee62f205a..ccfe86dfa784 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -19,6 +19,7 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
 #include <linux/mount.h>
 #include <linux/mutex.h>
 #include <linux/init.h>
@@ -1893,7 +1894,7 @@ static struct dentry *sel_make_dir(struct dentry *dir, const char *name,
 
 #define NULL_FILE_NAME "null"
 
-static int sel_fill_super(struct super_block *sb, void *data, int silent)
+static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct selinux_fs_info *fsi;
 	int ret;
@@ -2009,10 +2010,19 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
 	return ret;
 }
 
-static struct dentry *sel_mount(struct file_system_type *fs_type,
-		      int flags, const char *dev_name, void *data)
+static int sel_get_tree(struct fs_context *fc)
 {
-	return mount_single(fs_type, flags, data, sel_fill_super);
+	return vfs_get_super(fc, vfs_get_single_super, sel_fill_super);
+}
+
+static const struct fs_context_operations sel_context_ops = {
+	.get_tree	= sel_get_tree,
+};
+
+static int sel_init_fs_context(struct fs_context *fc)
+{
+	fc->ops = &sel_context_ops;
+	return 0;
 }
 
 static void sel_kill_sb(struct super_block *sb)
@@ -2023,7 +2033,7 @@ static void sel_kill_sb(struct super_block *sb)
 
 static struct file_system_type sel_fs_type = {
 	.name		= "selinuxfs",
-	.mount		= sel_mount,
+	.init_fs_context = sel_init_fs_context,
 	.kill_sb	= sel_kill_sb,
 };
 


^ permalink raw reply related

* [PATCH 37/38] vfs: Convert smackfs to fs_context
From: David Howells @ 2019-03-14 16:13 UTC (permalink / raw)
  To: viro
  Cc: Casey Schaufler, linux-security-module, dhowells, linux-fsdevel,
	linux-kernel
In-Reply-To: <155257972443.13720.11743171471060355965.stgit@warthog.procyon.org.uk>

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Casey Schaufler <casey@schaufler-ca.com>
cc: linux-security-module@vger.kernel.org
---

 security/smack/smackfs.c |   34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index faf2ea3968b3..7e3a81d05136 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -27,6 +27,7 @@
 #include <linux/ctype.h>
 #include <linux/audit.h>
 #include <linux/magic.h>
+#include <linux/fs_context.h>
 #include "smack.h"
 
 #define BEBITS	(sizeof(__be32) * 8)
@@ -2843,14 +2844,13 @@ static const struct file_operations smk_ptrace_ops = {
 /**
  * smk_fill_super - fill the smackfs superblock
  * @sb: the empty superblock
- * @data: unused
- * @silent: unused
+ * @fc: unused
  *
  * Fill in the well known entries for the smack filesystem
  *
  * Returns 0 on success, an error code on failure
  */
-static int smk_fill_super(struct super_block *sb, void *data, int silent)
+static int smk_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	int rc;
 
@@ -2920,25 +2920,35 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
 }
 
 /**
- * smk_mount - get the smackfs superblock
- * @fs_type: passed along without comment
- * @flags: passed along without comment
- * @dev_name: passed along without comment
- * @data: passed along without comment
+ * smk_get_tree - get the smackfs superblock
+ * @fc: The mount context, including any options
  *
  * Just passes everything along.
  *
  * Returns what the lower level code does.
  */
-static struct dentry *smk_mount(struct file_system_type *fs_type,
-		      int flags, const char *dev_name, void *data)
+static int smk_get_tree(struct fs_context *fc)
 {
-	return mount_single(fs_type, flags, data, smk_fill_super);
+	return vfs_get_super(fc, vfs_get_single_super, smk_fill_super);
+}
+
+static const struct fs_context_operations smk_context_ops = {
+	.get_tree	= smk_get_tree,
+};
+
+/**
+ * smk_init_fs_context - Initialise a filesystem context for smackfs
+ * @fc: The blank mount context
+ */
+static int smk_init_fs_context(struct fs_context *fc)
+{
+	fc->ops = &smk_context_ops;
+	return 0;
 }
 
 static struct file_system_type smk_fs_type = {
 	.name		= "smackfs",
-	.mount		= smk_mount,
+	.init_fs_context = smk_init_fs_context,
 	.kill_sb	= kill_litter_super,
 };
 


^ permalink raw reply related

* [PATCH 34/38] vfs: Convert apparmorfs to fs_context
From: David Howells @ 2019-03-14 16:13 UTC (permalink / raw)
  To: viro
  Cc: John Johansen, apparmor, linux-security-module, dhowells,
	linux-fsdevel, linux-kernel
In-Reply-To: <155257972443.13720.11743171471060355965.stgit@warthog.procyon.org.uk>

Signed-off-by: David Howells <dhowells@redhat.com>
cc: John Johansen <john.johansen@canonical.com>
cc: apparmor@lists.ubuntu.com
cc: linux-security-module@vger.kernel.org
---

 security/apparmor/apparmorfs.c |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index fefee040bf79..75f7da0e5ec1 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -23,6 +23,7 @@
 #include <linux/capability.h>
 #include <linux/rcupdate.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
 #include <linux/poll.h>
 #include <uapi/linux/major.h>
 #include <uapi/linux/magic.h>
@@ -137,7 +138,7 @@ static const struct super_operations aafs_super_ops = {
 	.show_path = aafs_show_path,
 };
 
-static int fill_super(struct super_block *sb, void *data, int silent)
+static int apparmorfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	static struct tree_descr files[] = { {""} };
 	int error;
@@ -150,16 +151,25 @@ static int fill_super(struct super_block *sb, void *data, int silent)
 	return 0;
 }
 
-static struct dentry *aafs_mount(struct file_system_type *fs_type,
-				 int flags, const char *dev_name, void *data)
+static int apparmorfs_get_tree(struct fs_context *fc)
 {
-	return mount_single(fs_type, flags, data, fill_super);
+	return vfs_get_super(fc, vfs_get_single_super, apparmorfs_fill_super);
+}
+
+static const struct fs_context_operations apparmorfs_context_ops = {
+	.get_tree	= apparmorfs_get_tree,
+};
+
+static int apparmorfs_init_fs_context(struct fs_context *fc)
+{
+	fc->ops = &apparmorfs_context_ops;
+	return 0;
 }
 
 static struct file_system_type aafs_ops = {
 	.owner = THIS_MODULE,
 	.name = AAFS_NAME,
-	.mount = aafs_mount,
+	.init_fs_context = apparmorfs_init_fs_context,
 	.kill_sb = kill_anon_super,
 };
 


^ permalink raw reply related

* Re: Add support for TCG2 log format on UEFI systems
From: Jarkko Sakkinen @ 2019-03-14  9:35 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, peterhuewe, jgg, roberto.sassu, linux-efi,
	linux-security-module, linux-kernel, tweek
In-Reply-To: <20190227202658.197113-1-matthewgarrett@google.com>

On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> Identical to V4, but based on tpmdd-next

This is not found /sys/kernel/security/tpm0/ascii_bios_measurements

But still

[    0.000000] efi:  ACPI 2.0=0x69ca2000  ACPI=0x69ca2000  TPMFinalLog=0x69ce4000  SMBIOS=0x69f63000  SMBIOS 3.0=0x69f62000  ESRT=0x69f3e818  MEMATTR=0x63448018

Tried this with too machines now.

I wonder if anyone else has had success...

/Jarkko

^ permalink raw reply

* [PATCH 1/1] Smack: Create smack_rule cache to optimize memory usage
From: Vishal Goel @ 2019-03-14  9:06 UTC (permalink / raw)
  To: casey, linux-security-module, linux-kernel
  Cc: pankaj.m, a.sahrawat, Vishal Goel
In-Reply-To: <CGME20190314091717epcas5p34ec02dc48347e76c2ba7a9d9c6fa9abe@epcas5p3.samsung.com>

This patch allows for small memory optimization by creating the
kmem cache for "struct smack_rule" instead of using kzalloc.
For adding new smack rule, kzalloc is used to allocate the memory
for "struct smack_rule". kzalloc will always allocate 32 or 64 bytes
for 1 structure depending upon the kzalloc cache sizes available in
system. Although the size of structure is 20 bytes only, resulting
in memory wastage per object in the default pool.

For e.g., if there are 20000 rules, then it will save 240KB(20000*12)
which is crucial for small memory targets.

Signed-off-by: Vishal Goel <vishal.goel@samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
---
 security/smack/smack.h     |  1 +
 security/smack/smack_lsm.c | 12 ++++++++++--
 security/smack/smackfs.c   |  2 +-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/security/smack/smack.h b/security/smack/smack.h
index 6a71fc7..a5d7461 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -354,6 +354,7 @@ int smk_tskacc(struct task_smack *, struct smack_known *,
 
 #define SMACK_HASH_SLOTS 16
 extern struct hlist_head smack_known_hash[SMACK_HASH_SLOTS];
+extern struct kmem_cache *smack_rule_cache;
 
 /*
  * Is the directory transmuting?
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 319add3..16b6cf5 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -56,6 +56,7 @@
 static LIST_HEAD(smk_ipv6_port_list);
 #endif
 static struct kmem_cache *smack_inode_cache;
+struct kmem_cache *smack_rule_cache;
 int smack_enabled;
 
 static const match_table_t smk_mount_tokens = {
@@ -349,7 +350,7 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead,
 	int rc = 0;
 
 	list_for_each_entry_rcu(orp, ohead, list) {
-		nrp = kzalloc(sizeof(struct smack_rule), gfp);
+		nrp = kmem_cache_zalloc(smack_rule_cache, gfp);
 		if (nrp == NULL) {
 			rc = -ENOMEM;
 			break;
@@ -1995,7 +1996,7 @@ static void smack_cred_free(struct cred *cred)
 	list_for_each_safe(l, n, &tsp->smk_rules) {
 		rp = list_entry(l, struct smack_rule, list);
 		list_del(&rp->list);
-		kfree(rp);
+		kmem_cache_free(smack_rule_cache, rp);
 	}
 	kfree(tsp);
 }
@@ -4788,10 +4789,17 @@ static __init int smack_init(void)
 	if (!smack_inode_cache)
 		return -ENOMEM;
 
+	smack_rule_cache = KMEM_CACHE(smack_rule, 0);
+	if (!smack_rule_cache) {
+		kmem_cache_destroy(smack_inode_cache);
+		return -ENOMEM;
+	}
+
 	tsp = new_task_smack(&smack_known_floor, &smack_known_floor,
 				GFP_KERNEL);
 	if (tsp == NULL) {
 		kmem_cache_destroy(smack_inode_cache);
+		kmem_cache_destroy(smack_rule_cache);
 		return -ENOMEM;
 	}
 
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 2a8a1f5..d8a0e25 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -236,7 +236,7 @@ static int smk_set_access(struct smack_parsed_rule *srp,
 	}
 
 	if (found == 0) {
-		sp = kzalloc(sizeof(*sp), GFP_KERNEL);
+		sp = kmem_cache_zalloc(smack_rule_cache, GFP_KERNEL);
 		if (sp == NULL) {
 			rc = -ENOMEM;
 			goto out;
-- 
1.9.1


^ permalink raw reply related

* Replacing IPv6 port labeling with CALIPSO in Smack
From: Casey Schaufler @ 2019-03-13 22:55 UTC (permalink / raw)
  To: Piotr Sawicki, LSM, SMACK-discuss@lists.01.org; +Cc: casey
In-Reply-To: <60d493f6-633f-e577-9947-cd41e1c762dc@schaufler-ca.com>

I am looking at CALIPSO support for Smack. CALIPSO provides
the same sort of network packet labeling for IPv6 that CIPSO
provides for IPv4. Because most of the details are buried in
the Netlabel code this should be reasonably straight forward.
The complication is that Smack has two mechanisms in place
for labeling IPv6 already, and neither uses anything like
CALIPSO packet labeling. If CONFIG_SECURITY_SMACK_NETFILTER
is defined Smack secids are sent via the netfilter secmark.
Otherwise, the Smack label of the process creating a socket
is maintained in a table indexed by the port number.

My proposed change would make the IPv6 labeling match the IPv4
labeling. The entire port number scheme would be abandoned.
The current secmark scheme would continue to be used if it
is configured. Whereas today IPv6 labeling is only supported
locally, the new code would support labeling remote systems as
well.

Systems that use CONFIG_SECURITY_SMACK_NETFILTER should be
unaffected for local use. The host address labeling scheme
would be retained, so any system configured to use IPv6
externally shouldn't see a difference. Systems that don't
use the option should also work the same as they do today.

Are there any users of Smack that use IPv6 but do not use
CONFIG_SECURITY_SMACK_NETFILTER? Does anyone have, know of
or imagine a use case where CALIPSO labeling would not be
a viable replacement for the hackish "port labeling"?

Thank you.


^ permalink raw reply

* RE: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: James Morris @ 2019-03-13 20:00 UTC (permalink / raw)
  To: Edwin Zimmerman
  Cc: 'Tetsuo Handa', 'Stephen Smalley',
	linux-security-module
In-Reply-To: <000001d4d91e$68837940$398a6bc0$@211mainstreet.net>

On Tue, 12 Mar 2019, Edwin Zimmerman wrote:

> On March 12, 2019 5:15, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote
> > >> Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
> > >> utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.
> > >
> > > What do you mean cannot afford ?
> > >
> > 
> > Some people have to set SELINUX=disabled in /etc/selinux/config or pass security=none from
> > the kernel command line.
> 
> If you specifically don't want in-kernel LSMs, and you specifically do want an out-of-tree LSM,
> there are other options. For example, you could just livepatch the security_* hooks you need, 
> since you already would using an LKM-based LSM.  That would give you your
> out-of-tree module and would also disable selinux on the hooks that got livepatched.
> 

Ahh, ok, this is about out of tree LSMs.

This has been discussed many times over the years and the answer is always 
the same: we will not add infrastructure to the kernel to support out of 
tree code.  This is a long-standing tenet of the Linux kernel.



-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply

* [PATCH AUTOSEL 4.20 13/60] keys: Fix dependency loop between construction record and auth key
From: Sasha Levin @ 2019-03-13 19:09 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: David Howells, James Morris, Sasha Levin, linux-nfs, keyrings,
	linux-security-module
In-Reply-To: <20190313191021.158171-1-sashal@kernel.org>

From: David Howells <dhowells@redhat.com>

[ Upstream commit 822ad64d7e46a8e2c8b8a796738d7b657cbb146d ]

In the request_key() upcall mechanism there's a dependency loop by which if
a key type driver overrides the ->request_key hook and the userspace side
manages to lose the authorisation key, the auth key and the internal
construction record (struct key_construction) can keep each other pinned.

Fix this by the following changes:

 (1) Killing off the construction record and using the auth key instead.

 (2) Including the operation name in the auth key payload and making the
     payload available outside of security/keys/.

 (3) The ->request_key hook is given the authkey instead of the cons
     record and operation name.

Changes (2) and (3) allow the auth key to naturally be cleaned up if the
keyring it is in is destroyed or cleared or the auth key is unlinked.

Fixes: 7ee02a316600 ("keys: Fix dependency loop between construction record and auth key")
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.morris@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/nfs/nfs4idmap.c                   | 31 ++++++------
 include/keys/request_key_auth-type.h | 36 ++++++++++++++
 include/linux/key-type.h             | 22 +++------
 security/keys/internal.h             | 13 +----
 security/keys/keyctl.c               |  1 +
 security/keys/process_keys.c         |  1 +
 security/keys/request_key.c          | 72 +++++++++++-----------------
 security/keys/request_key_auth.c     | 16 ++++---
 8 files changed, 100 insertions(+), 92 deletions(-)
 create mode 100644 include/keys/request_key_auth-type.h

diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index 3f23b6840547..bf34ddaa2ad7 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -44,6 +44,7 @@
 #include <linux/keyctl.h>
 #include <linux/key-type.h>
 #include <keys/user-type.h>
+#include <keys/request_key_auth-type.h>
 #include <linux/module.h>
 
 #include "internal.h"
@@ -59,7 +60,7 @@ static struct key_type key_type_id_resolver_legacy;
 struct idmap_legacy_upcalldata {
 	struct rpc_pipe_msg pipe_msg;
 	struct idmap_msg idmap_msg;
-	struct key_construction	*key_cons;
+	struct key	*authkey;
 	struct idmap *idmap;
 };
 
@@ -384,7 +385,7 @@ static const match_table_t nfs_idmap_tokens = {
 	{ Opt_find_err, NULL }
 };
 
-static int nfs_idmap_legacy_upcall(struct key_construction *, const char *, void *);
+static int nfs_idmap_legacy_upcall(struct key *, void *);
 static ssize_t idmap_pipe_downcall(struct file *, const char __user *,
 				   size_t);
 static void idmap_release_pipe(struct inode *);
@@ -549,11 +550,12 @@ nfs_idmap_prepare_pipe_upcall(struct idmap *idmap,
 static void
 nfs_idmap_complete_pipe_upcall_locked(struct idmap *idmap, int ret)
 {
-	struct key_construction *cons = idmap->idmap_upcall_data->key_cons;
+	struct key *authkey = idmap->idmap_upcall_data->authkey;
 
 	kfree(idmap->idmap_upcall_data);
 	idmap->idmap_upcall_data = NULL;
-	complete_request_key(cons, ret);
+	complete_request_key(authkey, ret);
+	key_put(authkey);
 }
 
 static void
@@ -563,15 +565,14 @@ nfs_idmap_abort_pipe_upcall(struct idmap *idmap, int ret)
 		nfs_idmap_complete_pipe_upcall_locked(idmap, ret);
 }
 
-static int nfs_idmap_legacy_upcall(struct key_construction *cons,
-				   const char *op,
-				   void *aux)
+static int nfs_idmap_legacy_upcall(struct key *authkey, void *aux)
 {
 	struct idmap_legacy_upcalldata *data;
+	struct request_key_auth *rka = get_request_key_auth(authkey);
 	struct rpc_pipe_msg *msg;
 	struct idmap_msg *im;
 	struct idmap *idmap = (struct idmap *)aux;
-	struct key *key = cons->key;
+	struct key *key = rka->target_key;
 	int ret = -ENOKEY;
 
 	if (!aux)
@@ -586,7 +587,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
 	msg = &data->pipe_msg;
 	im = &data->idmap_msg;
 	data->idmap = idmap;
-	data->key_cons = cons;
+	data->authkey = key_get(authkey);
 
 	ret = nfs_idmap_prepare_message(key->description, idmap, im, msg);
 	if (ret < 0)
@@ -604,7 +605,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
 out2:
 	kfree(data);
 out1:
-	complete_request_key(cons, ret);
+	complete_request_key(authkey, ret);
 	return ret;
 }
 
@@ -651,9 +652,10 @@ out:
 static ssize_t
 idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 {
+	struct request_key_auth *rka;
 	struct rpc_inode *rpci = RPC_I(file_inode(filp));
 	struct idmap *idmap = (struct idmap *)rpci->private;
-	struct key_construction *cons;
+	struct key *authkey;
 	struct idmap_msg im;
 	size_t namelen_in;
 	int ret = -ENOKEY;
@@ -665,7 +667,8 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 	if (idmap->idmap_upcall_data == NULL)
 		goto out_noupcall;
 
-	cons = idmap->idmap_upcall_data->key_cons;
+	authkey = idmap->idmap_upcall_data->authkey;
+	rka = get_request_key_auth(authkey);
 
 	if (mlen != sizeof(im)) {
 		ret = -ENOSPC;
@@ -690,9 +693,9 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 
 	ret = nfs_idmap_read_and_verify_message(&im,
 			&idmap->idmap_upcall_data->idmap_msg,
-			cons->key, cons->authkey);
+			rka->target_key, authkey);
 	if (ret >= 0) {
-		key_set_timeout(cons->key, nfs_idmap_cache_timeout);
+		key_set_timeout(rka->target_key, nfs_idmap_cache_timeout);
 		ret = mlen;
 	}
 
diff --git a/include/keys/request_key_auth-type.h b/include/keys/request_key_auth-type.h
new file mode 100644
index 000000000000..a726dd3f1dc6
--- /dev/null
+++ b/include/keys/request_key_auth-type.h
@@ -0,0 +1,36 @@
+/* request_key authorisation token key type
+ *
+ * Copyright (C) 2005 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#ifndef _KEYS_REQUEST_KEY_AUTH_TYPE_H
+#define _KEYS_REQUEST_KEY_AUTH_TYPE_H
+
+#include <linux/key.h>
+
+/*
+ * Authorisation record for request_key().
+ */
+struct request_key_auth {
+	struct key		*target_key;
+	struct key		*dest_keyring;
+	const struct cred	*cred;
+	void			*callout_info;
+	size_t			callout_len;
+	pid_t			pid;
+	char			op[8];
+} __randomize_layout;
+
+static inline struct request_key_auth *get_request_key_auth(const struct key *key)
+{
+	return key->payload.data[0];
+}
+
+
+#endif /* _KEYS_REQUEST_KEY_AUTH_TYPE_H */
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index bc9af551fc83..e49d1de0614e 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -20,15 +20,6 @@
 struct kernel_pkey_query;
 struct kernel_pkey_params;
 
-/*
- * key under-construction record
- * - passed to the request_key actor if supplied
- */
-struct key_construction {
-	struct key	*key;	/* key being constructed */
-	struct key	*authkey;/* authorisation for key being constructed */
-};
-
 /*
  * Pre-parsed payload, used by key add, update and instantiate.
  *
@@ -50,8 +41,7 @@ struct key_preparsed_payload {
 	time64_t	expiry;		/* Expiry time of key */
 } __randomize_layout;
 
-typedef int (*request_key_actor_t)(struct key_construction *key,
-				   const char *op, void *aux);
+typedef int (*request_key_actor_t)(struct key *auth_key, void *aux);
 
 /*
  * Preparsed matching criterion.
@@ -181,20 +171,20 @@ extern int key_instantiate_and_link(struct key *key,
 				    const void *data,
 				    size_t datalen,
 				    struct key *keyring,
-				    struct key *instkey);
+				    struct key *authkey);
 extern int key_reject_and_link(struct key *key,
 			       unsigned timeout,
 			       unsigned error,
 			       struct key *keyring,
-			       struct key *instkey);
-extern void complete_request_key(struct key_construction *cons, int error);
+			       struct key *authkey);
+extern void complete_request_key(struct key *authkey, int error);
 
 static inline int key_negate_and_link(struct key *key,
 				      unsigned timeout,
 				      struct key *keyring,
-				      struct key *instkey)
+				      struct key *authkey)
 {
-	return key_reject_and_link(key, timeout, ENOKEY, keyring, instkey);
+	return key_reject_and_link(key, timeout, ENOKEY, keyring, authkey);
 }
 
 extern int generic_key_instantiate(struct key *key, struct key_preparsed_payload *prep);
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 74cb0ff42fed..321aea3911f5 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -188,20 +188,9 @@ static inline int key_permission(const key_ref_t key_ref, unsigned perm)
 	return key_task_permission(key_ref, current_cred(), perm);
 }
 
-/*
- * Authorisation record for request_key().
- */
-struct request_key_auth {
-	struct key		*target_key;
-	struct key		*dest_keyring;
-	const struct cred	*cred;
-	void			*callout_info;
-	size_t			callout_len;
-	pid_t			pid;
-} __randomize_layout;
-
 extern struct key_type key_type_request_key_auth;
 extern struct key *request_key_auth_new(struct key *target,
+					const char *op,
 					const void *callout_info,
 					size_t callout_len,
 					struct key *dest_keyring);
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 18619690ce77..2d39606e6472 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -26,6 +26,7 @@
 #include <linux/security.h>
 #include <linux/uio.h>
 #include <linux/uaccess.h>
+#include <keys/request_key_auth-type.h>
 #include "internal.h"
 
 #define KEY_MAX_DESC_SIZE 4096
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index d5b25e535d3a..d776d2114651 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -20,6 +20,7 @@
 #include <linux/security.h>
 #include <linux/user_namespace.h>
 #include <linux/uaccess.h>
+#include <keys/request_key_auth-type.h>
 #include "internal.h"
 
 /* Session keyring create vs join semaphore */
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 738553698649..1a547cec8385 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -18,31 +18,30 @@
 #include <linux/keyctl.h>
 #include <linux/slab.h>
 #include "internal.h"
+#include <keys/request_key_auth-type.h>
 
 #define key_negative_timeout	60	/* default timeout on a negative key's existence */
 
 /**
  * complete_request_key - Complete the construction of a key.
- * @cons: The key construction record.
+ * @auth_key: The authorisation key.
  * @error: The success or failute of the construction.
  *
  * Complete the attempt to construct a key.  The key will be negated
  * if an error is indicated.  The authorisation key will be revoked
  * unconditionally.
  */
-void complete_request_key(struct key_construction *cons, int error)
+void complete_request_key(struct key *authkey, int error)
 {
-	kenter("{%d,%d},%d", cons->key->serial, cons->authkey->serial, error);
+	struct request_key_auth *rka = get_request_key_auth(authkey);
+	struct key *key = rka->target_key;
+
+	kenter("%d{%d},%d", authkey->serial, key->serial, error);
 
 	if (error < 0)
-		key_negate_and_link(cons->key, key_negative_timeout, NULL,
-				    cons->authkey);
+		key_negate_and_link(key, key_negative_timeout, NULL, authkey);
 	else
-		key_revoke(cons->authkey);
-
-	key_put(cons->key);
-	key_put(cons->authkey);
-	kfree(cons);
+		key_revoke(authkey);
 }
 EXPORT_SYMBOL(complete_request_key);
 
@@ -91,21 +90,19 @@ static int call_usermodehelper_keys(const char *path, char **argv, char **envp,
  * Request userspace finish the construction of a key
  * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"
  */
-static int call_sbin_request_key(struct key_construction *cons,
-				 const char *op,
-				 void *aux)
+static int call_sbin_request_key(struct key *authkey, void *aux)
 {
 	static char const request_key[] = "/sbin/request-key";
+	struct request_key_auth *rka = get_request_key_auth(authkey);
 	const struct cred *cred = current_cred();
 	key_serial_t prkey, sskey;
-	struct key *key = cons->key, *authkey = cons->authkey, *keyring,
-		*session;
+	struct key *key = rka->target_key, *keyring, *session;
 	char *argv[9], *envp[3], uid_str[12], gid_str[12];
 	char key_str[12], keyring_str[3][12];
 	char desc[20];
 	int ret, i;
 
-	kenter("{%d},{%d},%s", key->serial, authkey->serial, op);
+	kenter("{%d},{%d},%s", key->serial, authkey->serial, rka->op);
 
 	ret = install_user_keyrings();
 	if (ret < 0)
@@ -163,7 +160,7 @@ static int call_sbin_request_key(struct key_construction *cons,
 	/* set up the argument list */
 	i = 0;
 	argv[i++] = (char *)request_key;
-	argv[i++] = (char *) op;
+	argv[i++] = (char *)rka->op;
 	argv[i++] = key_str;
 	argv[i++] = uid_str;
 	argv[i++] = gid_str;
@@ -191,7 +188,7 @@ error_link:
 	key_put(keyring);
 
 error_alloc:
-	complete_request_key(cons, ret);
+	complete_request_key(authkey, ret);
 	kleave(" = %d", ret);
 	return ret;
 }
@@ -205,42 +202,31 @@ static int construct_key(struct key *key, const void *callout_info,
 			 size_t callout_len, void *aux,
 			 struct key *dest_keyring)
 {
-	struct key_construction *cons;
 	request_key_actor_t actor;
 	struct key *authkey;
 	int ret;
 
 	kenter("%d,%p,%zu,%p", key->serial, callout_info, callout_len, aux);
 
-	cons = kmalloc(sizeof(*cons), GFP_KERNEL);
-	if (!cons)
-		return -ENOMEM;
-
 	/* allocate an authorisation key */
-	authkey = request_key_auth_new(key, callout_info, callout_len,
+	authkey = request_key_auth_new(key, "create", callout_info, callout_len,
 				       dest_keyring);
-	if (IS_ERR(authkey)) {
-		kfree(cons);
-		ret = PTR_ERR(authkey);
-		authkey = NULL;
-	} else {
-		cons->authkey = key_get(authkey);
-		cons->key = key_get(key);
+	if (IS_ERR(authkey))
+		return PTR_ERR(authkey);
 
-		/* make the call */
-		actor = call_sbin_request_key;
-		if (key->type->request_key)
-			actor = key->type->request_key;
+	/* Make the call */
+	actor = call_sbin_request_key;
+	if (key->type->request_key)
+		actor = key->type->request_key;
 
-		ret = actor(cons, "create", aux);
+	ret = actor(authkey, aux);
 
-		/* check that the actor called complete_request_key() prior to
-		 * returning an error */
-		WARN_ON(ret < 0 &&
-			!test_bit(KEY_FLAG_REVOKED, &authkey->flags));
-		key_put(authkey);
-	}
+	/* check that the actor called complete_request_key() prior to
+	 * returning an error */
+	WARN_ON(ret < 0 &&
+		!test_bit(KEY_FLAG_REVOKED, &authkey->flags));
 
+	key_put(authkey);
 	kleave(" = %d", ret);
 	return ret;
 }
@@ -275,7 +261,7 @@ static int construct_get_dest_keyring(struct key **_dest_keyring)
 			if (cred->request_key_auth) {
 				authkey = cred->request_key_auth;
 				down_read(&authkey->sem);
-				rka = authkey->payload.data[0];
+				rka = get_request_key_auth(authkey);
 				if (!test_bit(KEY_FLAG_REVOKED,
 					      &authkey->flags))
 					dest_keyring =
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 6797843154f0..5e515791ccd1 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -18,7 +18,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include "internal.h"
-#include <keys/user-type.h>
+#include <keys/request_key_auth-type.h>
 
 static int request_key_auth_preparse(struct key_preparsed_payload *);
 static void request_key_auth_free_preparse(struct key_preparsed_payload *);
@@ -69,7 +69,7 @@ static int request_key_auth_instantiate(struct key *key,
 static void request_key_auth_describe(const struct key *key,
 				      struct seq_file *m)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 
 	seq_puts(m, "key:");
 	seq_puts(m, key->description);
@@ -84,7 +84,7 @@ static void request_key_auth_describe(const struct key *key,
 static long request_key_auth_read(const struct key *key,
 				  char __user *buffer, size_t buflen)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 	size_t datalen;
 	long ret;
 
@@ -110,7 +110,7 @@ static long request_key_auth_read(const struct key *key,
  */
 static void request_key_auth_revoke(struct key *key)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 
 	kenter("{%d}", key->serial);
 
@@ -137,7 +137,7 @@ static void free_request_key_auth(struct request_key_auth *rka)
  */
 static void request_key_auth_destroy(struct key *key)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 
 	kenter("{%d}", key->serial);
 
@@ -148,8 +148,9 @@ static void request_key_auth_destroy(struct key *key)
  * Create an authorisation token for /sbin/request-key or whoever to gain
  * access to the caller's security data.
  */
-struct key *request_key_auth_new(struct key *target, const void *callout_info,
-				 size_t callout_len, struct key *dest_keyring)
+struct key *request_key_auth_new(struct key *target, const char *op,
+				 const void *callout_info, size_t callout_len,
+				 struct key *dest_keyring)
 {
 	struct request_key_auth *rka, *irka;
 	const struct cred *cred = current->cred;
@@ -167,6 +168,7 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
 	if (!rka->callout_info)
 		goto error_free_rka;
 	rka->callout_len = callout_len;
+	strlcpy(rka->op, op, sizeof(rka->op));
 
 	/* see if the calling process is already servicing the key request of
 	 * another process */
-- 
2.19.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.19 11/48] keys: Fix dependency loop between construction record and auth key
From: Sasha Levin @ 2019-03-13 19:12 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: David Howells, James Morris, Sasha Levin, linux-nfs, keyrings,
	linux-security-module
In-Reply-To: <20190313191250.158955-1-sashal@kernel.org>

From: David Howells <dhowells@redhat.com>

[ Upstream commit 822ad64d7e46a8e2c8b8a796738d7b657cbb146d ]

In the request_key() upcall mechanism there's a dependency loop by which if
a key type driver overrides the ->request_key hook and the userspace side
manages to lose the authorisation key, the auth key and the internal
construction record (struct key_construction) can keep each other pinned.

Fix this by the following changes:

 (1) Killing off the construction record and using the auth key instead.

 (2) Including the operation name in the auth key payload and making the
     payload available outside of security/keys/.

 (3) The ->request_key hook is given the authkey instead of the cons
     record and operation name.

Changes (2) and (3) allow the auth key to naturally be cleaned up if the
keyring it is in is destroyed or cleared or the auth key is unlinked.

Fixes: 7ee02a316600 ("keys: Fix dependency loop between construction record and auth key")
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.morris@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/nfs/nfs4idmap.c                   | 31 ++++++------
 include/keys/request_key_auth-type.h | 36 ++++++++++++++
 include/linux/key-type.h             | 22 +++------
 security/keys/internal.h             | 13 +----
 security/keys/keyctl.c               |  1 +
 security/keys/process_keys.c         |  1 +
 security/keys/request_key.c          | 72 +++++++++++-----------------
 security/keys/request_key_auth.c     | 16 ++++---
 8 files changed, 100 insertions(+), 92 deletions(-)
 create mode 100644 include/keys/request_key_auth-type.h

diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index 3f23b6840547..bf34ddaa2ad7 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -44,6 +44,7 @@
 #include <linux/keyctl.h>
 #include <linux/key-type.h>
 #include <keys/user-type.h>
+#include <keys/request_key_auth-type.h>
 #include <linux/module.h>
 
 #include "internal.h"
@@ -59,7 +60,7 @@ static struct key_type key_type_id_resolver_legacy;
 struct idmap_legacy_upcalldata {
 	struct rpc_pipe_msg pipe_msg;
 	struct idmap_msg idmap_msg;
-	struct key_construction	*key_cons;
+	struct key	*authkey;
 	struct idmap *idmap;
 };
 
@@ -384,7 +385,7 @@ static const match_table_t nfs_idmap_tokens = {
 	{ Opt_find_err, NULL }
 };
 
-static int nfs_idmap_legacy_upcall(struct key_construction *, const char *, void *);
+static int nfs_idmap_legacy_upcall(struct key *, void *);
 static ssize_t idmap_pipe_downcall(struct file *, const char __user *,
 				   size_t);
 static void idmap_release_pipe(struct inode *);
@@ -549,11 +550,12 @@ nfs_idmap_prepare_pipe_upcall(struct idmap *idmap,
 static void
 nfs_idmap_complete_pipe_upcall_locked(struct idmap *idmap, int ret)
 {
-	struct key_construction *cons = idmap->idmap_upcall_data->key_cons;
+	struct key *authkey = idmap->idmap_upcall_data->authkey;
 
 	kfree(idmap->idmap_upcall_data);
 	idmap->idmap_upcall_data = NULL;
-	complete_request_key(cons, ret);
+	complete_request_key(authkey, ret);
+	key_put(authkey);
 }
 
 static void
@@ -563,15 +565,14 @@ nfs_idmap_abort_pipe_upcall(struct idmap *idmap, int ret)
 		nfs_idmap_complete_pipe_upcall_locked(idmap, ret);
 }
 
-static int nfs_idmap_legacy_upcall(struct key_construction *cons,
-				   const char *op,
-				   void *aux)
+static int nfs_idmap_legacy_upcall(struct key *authkey, void *aux)
 {
 	struct idmap_legacy_upcalldata *data;
+	struct request_key_auth *rka = get_request_key_auth(authkey);
 	struct rpc_pipe_msg *msg;
 	struct idmap_msg *im;
 	struct idmap *idmap = (struct idmap *)aux;
-	struct key *key = cons->key;
+	struct key *key = rka->target_key;
 	int ret = -ENOKEY;
 
 	if (!aux)
@@ -586,7 +587,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
 	msg = &data->pipe_msg;
 	im = &data->idmap_msg;
 	data->idmap = idmap;
-	data->key_cons = cons;
+	data->authkey = key_get(authkey);
 
 	ret = nfs_idmap_prepare_message(key->description, idmap, im, msg);
 	if (ret < 0)
@@ -604,7 +605,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
 out2:
 	kfree(data);
 out1:
-	complete_request_key(cons, ret);
+	complete_request_key(authkey, ret);
 	return ret;
 }
 
@@ -651,9 +652,10 @@ static int nfs_idmap_read_and_verify_message(struct idmap_msg *im,
 static ssize_t
 idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 {
+	struct request_key_auth *rka;
 	struct rpc_inode *rpci = RPC_I(file_inode(filp));
 	struct idmap *idmap = (struct idmap *)rpci->private;
-	struct key_construction *cons;
+	struct key *authkey;
 	struct idmap_msg im;
 	size_t namelen_in;
 	int ret = -ENOKEY;
@@ -665,7 +667,8 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 	if (idmap->idmap_upcall_data == NULL)
 		goto out_noupcall;
 
-	cons = idmap->idmap_upcall_data->key_cons;
+	authkey = idmap->idmap_upcall_data->authkey;
+	rka = get_request_key_auth(authkey);
 
 	if (mlen != sizeof(im)) {
 		ret = -ENOSPC;
@@ -690,9 +693,9 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 
 	ret = nfs_idmap_read_and_verify_message(&im,
 			&idmap->idmap_upcall_data->idmap_msg,
-			cons->key, cons->authkey);
+			rka->target_key, authkey);
 	if (ret >= 0) {
-		key_set_timeout(cons->key, nfs_idmap_cache_timeout);
+		key_set_timeout(rka->target_key, nfs_idmap_cache_timeout);
 		ret = mlen;
 	}
 
diff --git a/include/keys/request_key_auth-type.h b/include/keys/request_key_auth-type.h
new file mode 100644
index 000000000000..a726dd3f1dc6
--- /dev/null
+++ b/include/keys/request_key_auth-type.h
@@ -0,0 +1,36 @@
+/* request_key authorisation token key type
+ *
+ * Copyright (C) 2005 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#ifndef _KEYS_REQUEST_KEY_AUTH_TYPE_H
+#define _KEYS_REQUEST_KEY_AUTH_TYPE_H
+
+#include <linux/key.h>
+
+/*
+ * Authorisation record for request_key().
+ */
+struct request_key_auth {
+	struct key		*target_key;
+	struct key		*dest_keyring;
+	const struct cred	*cred;
+	void			*callout_info;
+	size_t			callout_len;
+	pid_t			pid;
+	char			op[8];
+} __randomize_layout;
+
+static inline struct request_key_auth *get_request_key_auth(const struct key *key)
+{
+	return key->payload.data[0];
+}
+
+
+#endif /* _KEYS_REQUEST_KEY_AUTH_TYPE_H */
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index 05d8fb5a06c4..d3c5ae8ad498 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -17,15 +17,6 @@
 
 #ifdef CONFIG_KEYS
 
-/*
- * key under-construction record
- * - passed to the request_key actor if supplied
- */
-struct key_construction {
-	struct key	*key;	/* key being constructed */
-	struct key	*authkey;/* authorisation for key being constructed */
-};
-
 /*
  * Pre-parsed payload, used by key add, update and instantiate.
  *
@@ -47,8 +38,7 @@ struct key_preparsed_payload {
 	time64_t	expiry;		/* Expiry time of key */
 } __randomize_layout;
 
-typedef int (*request_key_actor_t)(struct key_construction *key,
-				   const char *op, void *aux);
+typedef int (*request_key_actor_t)(struct key *auth_key, void *aux);
 
 /*
  * Preparsed matching criterion.
@@ -170,20 +160,20 @@ extern int key_instantiate_and_link(struct key *key,
 				    const void *data,
 				    size_t datalen,
 				    struct key *keyring,
-				    struct key *instkey);
+				    struct key *authkey);
 extern int key_reject_and_link(struct key *key,
 			       unsigned timeout,
 			       unsigned error,
 			       struct key *keyring,
-			       struct key *instkey);
-extern void complete_request_key(struct key_construction *cons, int error);
+			       struct key *authkey);
+extern void complete_request_key(struct key *authkey, int error);
 
 static inline int key_negate_and_link(struct key *key,
 				      unsigned timeout,
 				      struct key *keyring,
-				      struct key *instkey)
+				      struct key *authkey)
 {
-	return key_reject_and_link(key, timeout, ENOKEY, keyring, instkey);
+	return key_reject_and_link(key, timeout, ENOKEY, keyring, authkey);
 }
 
 extern int generic_key_instantiate(struct key *key, struct key_preparsed_payload *prep);
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 9f8208dc0e55..a02742621c8d 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -188,20 +188,9 @@ static inline int key_permission(const key_ref_t key_ref, unsigned perm)
 	return key_task_permission(key_ref, current_cred(), perm);
 }
 
-/*
- * Authorisation record for request_key().
- */
-struct request_key_auth {
-	struct key		*target_key;
-	struct key		*dest_keyring;
-	const struct cred	*cred;
-	void			*callout_info;
-	size_t			callout_len;
-	pid_t			pid;
-} __randomize_layout;
-
 extern struct key_type key_type_request_key_auth;
 extern struct key *request_key_auth_new(struct key *target,
+					const char *op,
 					const void *callout_info,
 					size_t callout_len,
 					struct key *dest_keyring);
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 1ffe60bb2845..ca31af186abd 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -26,6 +26,7 @@
 #include <linux/security.h>
 #include <linux/uio.h>
 #include <linux/uaccess.h>
+#include <keys/request_key_auth-type.h>
 #include "internal.h"
 
 #define KEY_MAX_DESC_SIZE 4096
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index d5b25e535d3a..d776d2114651 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -20,6 +20,7 @@
 #include <linux/security.h>
 #include <linux/user_namespace.h>
 #include <linux/uaccess.h>
+#include <keys/request_key_auth-type.h>
 #include "internal.h"
 
 /* Session keyring create vs join semaphore */
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 738553698649..1a547cec8385 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -18,31 +18,30 @@
 #include <linux/keyctl.h>
 #include <linux/slab.h>
 #include "internal.h"
+#include <keys/request_key_auth-type.h>
 
 #define key_negative_timeout	60	/* default timeout on a negative key's existence */
 
 /**
  * complete_request_key - Complete the construction of a key.
- * @cons: The key construction record.
+ * @auth_key: The authorisation key.
  * @error: The success or failute of the construction.
  *
  * Complete the attempt to construct a key.  The key will be negated
  * if an error is indicated.  The authorisation key will be revoked
  * unconditionally.
  */
-void complete_request_key(struct key_construction *cons, int error)
+void complete_request_key(struct key *authkey, int error)
 {
-	kenter("{%d,%d},%d", cons->key->serial, cons->authkey->serial, error);
+	struct request_key_auth *rka = get_request_key_auth(authkey);
+	struct key *key = rka->target_key;
+
+	kenter("%d{%d},%d", authkey->serial, key->serial, error);
 
 	if (error < 0)
-		key_negate_and_link(cons->key, key_negative_timeout, NULL,
-				    cons->authkey);
+		key_negate_and_link(key, key_negative_timeout, NULL, authkey);
 	else
-		key_revoke(cons->authkey);
-
-	key_put(cons->key);
-	key_put(cons->authkey);
-	kfree(cons);
+		key_revoke(authkey);
 }
 EXPORT_SYMBOL(complete_request_key);
 
@@ -91,21 +90,19 @@ static int call_usermodehelper_keys(const char *path, char **argv, char **envp,
  * Request userspace finish the construction of a key
  * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"
  */
-static int call_sbin_request_key(struct key_construction *cons,
-				 const char *op,
-				 void *aux)
+static int call_sbin_request_key(struct key *authkey, void *aux)
 {
 	static char const request_key[] = "/sbin/request-key";
+	struct request_key_auth *rka = get_request_key_auth(authkey);
 	const struct cred *cred = current_cred();
 	key_serial_t prkey, sskey;
-	struct key *key = cons->key, *authkey = cons->authkey, *keyring,
-		*session;
+	struct key *key = rka->target_key, *keyring, *session;
 	char *argv[9], *envp[3], uid_str[12], gid_str[12];
 	char key_str[12], keyring_str[3][12];
 	char desc[20];
 	int ret, i;
 
-	kenter("{%d},{%d},%s", key->serial, authkey->serial, op);
+	kenter("{%d},{%d},%s", key->serial, authkey->serial, rka->op);
 
 	ret = install_user_keyrings();
 	if (ret < 0)
@@ -163,7 +160,7 @@ static int call_sbin_request_key(struct key_construction *cons,
 	/* set up the argument list */
 	i = 0;
 	argv[i++] = (char *)request_key;
-	argv[i++] = (char *) op;
+	argv[i++] = (char *)rka->op;
 	argv[i++] = key_str;
 	argv[i++] = uid_str;
 	argv[i++] = gid_str;
@@ -191,7 +188,7 @@ static int call_sbin_request_key(struct key_construction *cons,
 	key_put(keyring);
 
 error_alloc:
-	complete_request_key(cons, ret);
+	complete_request_key(authkey, ret);
 	kleave(" = %d", ret);
 	return ret;
 }
@@ -205,42 +202,31 @@ static int construct_key(struct key *key, const void *callout_info,
 			 size_t callout_len, void *aux,
 			 struct key *dest_keyring)
 {
-	struct key_construction *cons;
 	request_key_actor_t actor;
 	struct key *authkey;
 	int ret;
 
 	kenter("%d,%p,%zu,%p", key->serial, callout_info, callout_len, aux);
 
-	cons = kmalloc(sizeof(*cons), GFP_KERNEL);
-	if (!cons)
-		return -ENOMEM;
-
 	/* allocate an authorisation key */
-	authkey = request_key_auth_new(key, callout_info, callout_len,
+	authkey = request_key_auth_new(key, "create", callout_info, callout_len,
 				       dest_keyring);
-	if (IS_ERR(authkey)) {
-		kfree(cons);
-		ret = PTR_ERR(authkey);
-		authkey = NULL;
-	} else {
-		cons->authkey = key_get(authkey);
-		cons->key = key_get(key);
+	if (IS_ERR(authkey))
+		return PTR_ERR(authkey);
 
-		/* make the call */
-		actor = call_sbin_request_key;
-		if (key->type->request_key)
-			actor = key->type->request_key;
+	/* Make the call */
+	actor = call_sbin_request_key;
+	if (key->type->request_key)
+		actor = key->type->request_key;
 
-		ret = actor(cons, "create", aux);
+	ret = actor(authkey, aux);
 
-		/* check that the actor called complete_request_key() prior to
-		 * returning an error */
-		WARN_ON(ret < 0 &&
-			!test_bit(KEY_FLAG_REVOKED, &authkey->flags));
-		key_put(authkey);
-	}
+	/* check that the actor called complete_request_key() prior to
+	 * returning an error */
+	WARN_ON(ret < 0 &&
+		!test_bit(KEY_FLAG_REVOKED, &authkey->flags));
 
+	key_put(authkey);
 	kleave(" = %d", ret);
 	return ret;
 }
@@ -275,7 +261,7 @@ static int construct_get_dest_keyring(struct key **_dest_keyring)
 			if (cred->request_key_auth) {
 				authkey = cred->request_key_auth;
 				down_read(&authkey->sem);
-				rka = authkey->payload.data[0];
+				rka = get_request_key_auth(authkey);
 				if (!test_bit(KEY_FLAG_REVOKED,
 					      &authkey->flags))
 					dest_keyring =
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 6797843154f0..5e515791ccd1 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -18,7 +18,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include "internal.h"
-#include <keys/user-type.h>
+#include <keys/request_key_auth-type.h>
 
 static int request_key_auth_preparse(struct key_preparsed_payload *);
 static void request_key_auth_free_preparse(struct key_preparsed_payload *);
@@ -69,7 +69,7 @@ static int request_key_auth_instantiate(struct key *key,
 static void request_key_auth_describe(const struct key *key,
 				      struct seq_file *m)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 
 	seq_puts(m, "key:");
 	seq_puts(m, key->description);
@@ -84,7 +84,7 @@ static void request_key_auth_describe(const struct key *key,
 static long request_key_auth_read(const struct key *key,
 				  char __user *buffer, size_t buflen)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 	size_t datalen;
 	long ret;
 
@@ -110,7 +110,7 @@ static long request_key_auth_read(const struct key *key,
  */
 static void request_key_auth_revoke(struct key *key)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 
 	kenter("{%d}", key->serial);
 
@@ -137,7 +137,7 @@ static void free_request_key_auth(struct request_key_auth *rka)
  */
 static void request_key_auth_destroy(struct key *key)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 
 	kenter("{%d}", key->serial);
 
@@ -148,8 +148,9 @@ static void request_key_auth_destroy(struct key *key)
  * Create an authorisation token for /sbin/request-key or whoever to gain
  * access to the caller's security data.
  */
-struct key *request_key_auth_new(struct key *target, const void *callout_info,
-				 size_t callout_len, struct key *dest_keyring)
+struct key *request_key_auth_new(struct key *target, const char *op,
+				 const void *callout_info, size_t callout_len,
+				 struct key *dest_keyring)
 {
 	struct request_key_auth *rka, *irka;
 	const struct cred *cred = current->cred;
@@ -167,6 +168,7 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
 	if (!rka->callout_info)
 		goto error_free_rka;
 	rka->callout_len = callout_len;
+	strlcpy(rka->op, op, sizeof(rka->op));
 
 	/* see if the calling process is already servicing the key request of
 	 * another process */
-- 
2.19.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.14 07/33] keys: Fix dependency loop between construction record and auth key
From: Sasha Levin @ 2019-03-13 19:14 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: David Howells, James Morris, Sasha Levin, linux-nfs, keyrings,
	linux-security-module
In-Reply-To: <20190313191506.159677-1-sashal@kernel.org>

From: David Howells <dhowells@redhat.com>

[ Upstream commit 822ad64d7e46a8e2c8b8a796738d7b657cbb146d ]

In the request_key() upcall mechanism there's a dependency loop by which if
a key type driver overrides the ->request_key hook and the userspace side
manages to lose the authorisation key, the auth key and the internal
construction record (struct key_construction) can keep each other pinned.

Fix this by the following changes:

 (1) Killing off the construction record and using the auth key instead.

 (2) Including the operation name in the auth key payload and making the
     payload available outside of security/keys/.

 (3) The ->request_key hook is given the authkey instead of the cons
     record and operation name.

Changes (2) and (3) allow the auth key to naturally be cleaned up if the
keyring it is in is destroyed or cleared or the auth key is unlinked.

Fixes: 7ee02a316600 ("keys: Fix dependency loop between construction record and auth key")
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.morris@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/nfs/nfs4idmap.c                   | 31 ++++++------
 include/keys/request_key_auth-type.h | 36 ++++++++++++++
 include/linux/key-type.h             | 22 +++------
 security/keys/internal.h             | 13 +----
 security/keys/keyctl.c               |  1 +
 security/keys/process_keys.c         |  1 +
 security/keys/request_key.c          | 72 +++++++++++-----------------
 security/keys/request_key_auth.c     | 16 ++++---
 8 files changed, 100 insertions(+), 92 deletions(-)
 create mode 100644 include/keys/request_key_auth-type.h

diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index b6f9d84ba19b..ae2d6f220627 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -44,6 +44,7 @@
 #include <linux/keyctl.h>
 #include <linux/key-type.h>
 #include <keys/user-type.h>
+#include <keys/request_key_auth-type.h>
 #include <linux/module.h>
 
 #include "internal.h"
@@ -59,7 +60,7 @@ static struct key_type key_type_id_resolver_legacy;
 struct idmap_legacy_upcalldata {
 	struct rpc_pipe_msg pipe_msg;
 	struct idmap_msg idmap_msg;
-	struct key_construction	*key_cons;
+	struct key	*authkey;
 	struct idmap *idmap;
 };
 
@@ -384,7 +385,7 @@ static const match_table_t nfs_idmap_tokens = {
 	{ Opt_find_err, NULL }
 };
 
-static int nfs_idmap_legacy_upcall(struct key_construction *, const char *, void *);
+static int nfs_idmap_legacy_upcall(struct key *, void *);
 static ssize_t idmap_pipe_downcall(struct file *, const char __user *,
 				   size_t);
 static void idmap_release_pipe(struct inode *);
@@ -545,11 +546,12 @@ nfs_idmap_prepare_pipe_upcall(struct idmap *idmap,
 static void
 nfs_idmap_complete_pipe_upcall_locked(struct idmap *idmap, int ret)
 {
-	struct key_construction *cons = idmap->idmap_upcall_data->key_cons;
+	struct key *authkey = idmap->idmap_upcall_data->authkey;
 
 	kfree(idmap->idmap_upcall_data);
 	idmap->idmap_upcall_data = NULL;
-	complete_request_key(cons, ret);
+	complete_request_key(authkey, ret);
+	key_put(authkey);
 }
 
 static void
@@ -559,15 +561,14 @@ nfs_idmap_abort_pipe_upcall(struct idmap *idmap, int ret)
 		nfs_idmap_complete_pipe_upcall_locked(idmap, ret);
 }
 
-static int nfs_idmap_legacy_upcall(struct key_construction *cons,
-				   const char *op,
-				   void *aux)
+static int nfs_idmap_legacy_upcall(struct key *authkey, void *aux)
 {
 	struct idmap_legacy_upcalldata *data;
+	struct request_key_auth *rka = get_request_key_auth(authkey);
 	struct rpc_pipe_msg *msg;
 	struct idmap_msg *im;
 	struct idmap *idmap = (struct idmap *)aux;
-	struct key *key = cons->key;
+	struct key *key = rka->target_key;
 	int ret = -ENOKEY;
 
 	if (!aux)
@@ -582,7 +583,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
 	msg = &data->pipe_msg;
 	im = &data->idmap_msg;
 	data->idmap = idmap;
-	data->key_cons = cons;
+	data->authkey = key_get(authkey);
 
 	ret = nfs_idmap_prepare_message(key->description, idmap, im, msg);
 	if (ret < 0)
@@ -600,7 +601,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
 out2:
 	kfree(data);
 out1:
-	complete_request_key(cons, ret);
+	complete_request_key(authkey, ret);
 	return ret;
 }
 
@@ -647,9 +648,10 @@ static int nfs_idmap_read_and_verify_message(struct idmap_msg *im,
 static ssize_t
 idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 {
+	struct request_key_auth *rka;
 	struct rpc_inode *rpci = RPC_I(file_inode(filp));
 	struct idmap *idmap = (struct idmap *)rpci->private;
-	struct key_construction *cons;
+	struct key *authkey;
 	struct idmap_msg im;
 	size_t namelen_in;
 	int ret = -ENOKEY;
@@ -661,7 +663,8 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 	if (idmap->idmap_upcall_data == NULL)
 		goto out_noupcall;
 
-	cons = idmap->idmap_upcall_data->key_cons;
+	authkey = idmap->idmap_upcall_data->authkey;
+	rka = get_request_key_auth(authkey);
 
 	if (mlen != sizeof(im)) {
 		ret = -ENOSPC;
@@ -686,9 +689,9 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
 
 	ret = nfs_idmap_read_and_verify_message(&im,
 			&idmap->idmap_upcall_data->idmap_msg,
-			cons->key, cons->authkey);
+			rka->target_key, authkey);
 	if (ret >= 0) {
-		key_set_timeout(cons->key, nfs_idmap_cache_timeout);
+		key_set_timeout(rka->target_key, nfs_idmap_cache_timeout);
 		ret = mlen;
 	}
 
diff --git a/include/keys/request_key_auth-type.h b/include/keys/request_key_auth-type.h
new file mode 100644
index 000000000000..a726dd3f1dc6
--- /dev/null
+++ b/include/keys/request_key_auth-type.h
@@ -0,0 +1,36 @@
+/* request_key authorisation token key type
+ *
+ * Copyright (C) 2005 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#ifndef _KEYS_REQUEST_KEY_AUTH_TYPE_H
+#define _KEYS_REQUEST_KEY_AUTH_TYPE_H
+
+#include <linux/key.h>
+
+/*
+ * Authorisation record for request_key().
+ */
+struct request_key_auth {
+	struct key		*target_key;
+	struct key		*dest_keyring;
+	const struct cred	*cred;
+	void			*callout_info;
+	size_t			callout_len;
+	pid_t			pid;
+	char			op[8];
+} __randomize_layout;
+
+static inline struct request_key_auth *get_request_key_auth(const struct key *key)
+{
+	return key->payload.data[0];
+}
+
+
+#endif /* _KEYS_REQUEST_KEY_AUTH_TYPE_H */
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index 9520fc3c3b9a..dfb3ba782d2c 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -17,15 +17,6 @@
 
 #ifdef CONFIG_KEYS
 
-/*
- * key under-construction record
- * - passed to the request_key actor if supplied
- */
-struct key_construction {
-	struct key	*key;	/* key being constructed */
-	struct key	*authkey;/* authorisation for key being constructed */
-};
-
 /*
  * Pre-parsed payload, used by key add, update and instantiate.
  *
@@ -47,8 +38,7 @@ struct key_preparsed_payload {
 	time_t		expiry;		/* Expiry time of key */
 } __randomize_layout;
 
-typedef int (*request_key_actor_t)(struct key_construction *key,
-				   const char *op, void *aux);
+typedef int (*request_key_actor_t)(struct key *auth_key, void *aux);
 
 /*
  * Preparsed matching criterion.
@@ -170,20 +160,20 @@ extern int key_instantiate_and_link(struct key *key,
 				    const void *data,
 				    size_t datalen,
 				    struct key *keyring,
-				    struct key *instkey);
+				    struct key *authkey);
 extern int key_reject_and_link(struct key *key,
 			       unsigned timeout,
 			       unsigned error,
 			       struct key *keyring,
-			       struct key *instkey);
-extern void complete_request_key(struct key_construction *cons, int error);
+			       struct key *authkey);
+extern void complete_request_key(struct key *authkey, int error);
 
 static inline int key_negate_and_link(struct key *key,
 				      unsigned timeout,
 				      struct key *keyring,
-				      struct key *instkey)
+				      struct key *authkey)
 {
-	return key_reject_and_link(key, timeout, ENOKEY, keyring, instkey);
+	return key_reject_and_link(key, timeout, ENOKEY, keyring, authkey);
 }
 
 extern int generic_key_instantiate(struct key *key, struct key_preparsed_payload *prep);
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 503adbae7b0d..e3a573840186 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -188,20 +188,9 @@ static inline int key_permission(const key_ref_t key_ref, unsigned perm)
 	return key_task_permission(key_ref, current_cred(), perm);
 }
 
-/*
- * Authorisation record for request_key().
- */
-struct request_key_auth {
-	struct key		*target_key;
-	struct key		*dest_keyring;
-	const struct cred	*cred;
-	void			*callout_info;
-	size_t			callout_len;
-	pid_t			pid;
-} __randomize_layout;
-
 extern struct key_type key_type_request_key_auth;
 extern struct key *request_key_auth_new(struct key *target,
+					const char *op,
 					const void *callout_info,
 					size_t callout_len,
 					struct key *dest_keyring);
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 1ffe60bb2845..ca31af186abd 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -26,6 +26,7 @@
 #include <linux/security.h>
 #include <linux/uio.h>
 #include <linux/uaccess.h>
+#include <keys/request_key_auth-type.h>
 #include "internal.h"
 
 #define KEY_MAX_DESC_SIZE 4096
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 740affd65ee9..5f2993ab2d50 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -20,6 +20,7 @@
 #include <linux/security.h>
 #include <linux/user_namespace.h>
 #include <linux/uaccess.h>
+#include <keys/request_key_auth-type.h>
 #include "internal.h"
 
 /* Session keyring create vs join semaphore */
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index c707fdbb3429..2ecd67221476 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -18,31 +18,30 @@
 #include <linux/keyctl.h>
 #include <linux/slab.h>
 #include "internal.h"
+#include <keys/request_key_auth-type.h>
 
 #define key_negative_timeout	60	/* default timeout on a negative key's existence */
 
 /**
  * complete_request_key - Complete the construction of a key.
- * @cons: The key construction record.
+ * @auth_key: The authorisation key.
  * @error: The success or failute of the construction.
  *
  * Complete the attempt to construct a key.  The key will be negated
  * if an error is indicated.  The authorisation key will be revoked
  * unconditionally.
  */
-void complete_request_key(struct key_construction *cons, int error)
+void complete_request_key(struct key *authkey, int error)
 {
-	kenter("{%d,%d},%d", cons->key->serial, cons->authkey->serial, error);
+	struct request_key_auth *rka = get_request_key_auth(authkey);
+	struct key *key = rka->target_key;
+
+	kenter("%d{%d},%d", authkey->serial, key->serial, error);
 
 	if (error < 0)
-		key_negate_and_link(cons->key, key_negative_timeout, NULL,
-				    cons->authkey);
+		key_negate_and_link(key, key_negative_timeout, NULL, authkey);
 	else
-		key_revoke(cons->authkey);
-
-	key_put(cons->key);
-	key_put(cons->authkey);
-	kfree(cons);
+		key_revoke(authkey);
 }
 EXPORT_SYMBOL(complete_request_key);
 
@@ -91,21 +90,19 @@ static int call_usermodehelper_keys(const char *path, char **argv, char **envp,
  * Request userspace finish the construction of a key
  * - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"
  */
-static int call_sbin_request_key(struct key_construction *cons,
-				 const char *op,
-				 void *aux)
+static int call_sbin_request_key(struct key *authkey, void *aux)
 {
 	static char const request_key[] = "/sbin/request-key";
+	struct request_key_auth *rka = get_request_key_auth(authkey);
 	const struct cred *cred = current_cred();
 	key_serial_t prkey, sskey;
-	struct key *key = cons->key, *authkey = cons->authkey, *keyring,
-		*session;
+	struct key *key = rka->target_key, *keyring, *session;
 	char *argv[9], *envp[3], uid_str[12], gid_str[12];
 	char key_str[12], keyring_str[3][12];
 	char desc[20];
 	int ret, i;
 
-	kenter("{%d},{%d},%s", key->serial, authkey->serial, op);
+	kenter("{%d},{%d},%s", key->serial, authkey->serial, rka->op);
 
 	ret = install_user_keyrings();
 	if (ret < 0)
@@ -163,7 +160,7 @@ static int call_sbin_request_key(struct key_construction *cons,
 	/* set up the argument list */
 	i = 0;
 	argv[i++] = (char *)request_key;
-	argv[i++] = (char *) op;
+	argv[i++] = (char *)rka->op;
 	argv[i++] = key_str;
 	argv[i++] = uid_str;
 	argv[i++] = gid_str;
@@ -191,7 +188,7 @@ static int call_sbin_request_key(struct key_construction *cons,
 	key_put(keyring);
 
 error_alloc:
-	complete_request_key(cons, ret);
+	complete_request_key(authkey, ret);
 	kleave(" = %d", ret);
 	return ret;
 }
@@ -205,42 +202,31 @@ static int construct_key(struct key *key, const void *callout_info,
 			 size_t callout_len, void *aux,
 			 struct key *dest_keyring)
 {
-	struct key_construction *cons;
 	request_key_actor_t actor;
 	struct key *authkey;
 	int ret;
 
 	kenter("%d,%p,%zu,%p", key->serial, callout_info, callout_len, aux);
 
-	cons = kmalloc(sizeof(*cons), GFP_KERNEL);
-	if (!cons)
-		return -ENOMEM;
-
 	/* allocate an authorisation key */
-	authkey = request_key_auth_new(key, callout_info, callout_len,
+	authkey = request_key_auth_new(key, "create", callout_info, callout_len,
 				       dest_keyring);
-	if (IS_ERR(authkey)) {
-		kfree(cons);
-		ret = PTR_ERR(authkey);
-		authkey = NULL;
-	} else {
-		cons->authkey = key_get(authkey);
-		cons->key = key_get(key);
+	if (IS_ERR(authkey))
+		return PTR_ERR(authkey);
 
-		/* make the call */
-		actor = call_sbin_request_key;
-		if (key->type->request_key)
-			actor = key->type->request_key;
+	/* Make the call */
+	actor = call_sbin_request_key;
+	if (key->type->request_key)
+		actor = key->type->request_key;
 
-		ret = actor(cons, "create", aux);
+	ret = actor(authkey, aux);
 
-		/* check that the actor called complete_request_key() prior to
-		 * returning an error */
-		WARN_ON(ret < 0 &&
-			!test_bit(KEY_FLAG_REVOKED, &authkey->flags));
-		key_put(authkey);
-	}
+	/* check that the actor called complete_request_key() prior to
+	 * returning an error */
+	WARN_ON(ret < 0 &&
+		!test_bit(KEY_FLAG_REVOKED, &authkey->flags));
 
+	key_put(authkey);
 	kleave(" = %d", ret);
 	return ret;
 }
@@ -275,7 +261,7 @@ static int construct_get_dest_keyring(struct key **_dest_keyring)
 			if (cred->request_key_auth) {
 				authkey = cred->request_key_auth;
 				down_read(&authkey->sem);
-				rka = authkey->payload.data[0];
+				rka = get_request_key_auth(authkey);
 				if (!test_bit(KEY_FLAG_REVOKED,
 					      &authkey->flags))
 					dest_keyring =
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 6797843154f0..5e515791ccd1 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -18,7 +18,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include "internal.h"
-#include <keys/user-type.h>
+#include <keys/request_key_auth-type.h>
 
 static int request_key_auth_preparse(struct key_preparsed_payload *);
 static void request_key_auth_free_preparse(struct key_preparsed_payload *);
@@ -69,7 +69,7 @@ static int request_key_auth_instantiate(struct key *key,
 static void request_key_auth_describe(const struct key *key,
 				      struct seq_file *m)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 
 	seq_puts(m, "key:");
 	seq_puts(m, key->description);
@@ -84,7 +84,7 @@ static void request_key_auth_describe(const struct key *key,
 static long request_key_auth_read(const struct key *key,
 				  char __user *buffer, size_t buflen)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 	size_t datalen;
 	long ret;
 
@@ -110,7 +110,7 @@ static long request_key_auth_read(const struct key *key,
  */
 static void request_key_auth_revoke(struct key *key)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 
 	kenter("{%d}", key->serial);
 
@@ -137,7 +137,7 @@ static void free_request_key_auth(struct request_key_auth *rka)
  */
 static void request_key_auth_destroy(struct key *key)
 {
-	struct request_key_auth *rka = key->payload.data[0];
+	struct request_key_auth *rka = get_request_key_auth(key);
 
 	kenter("{%d}", key->serial);
 
@@ -148,8 +148,9 @@ static void request_key_auth_destroy(struct key *key)
  * Create an authorisation token for /sbin/request-key or whoever to gain
  * access to the caller's security data.
  */
-struct key *request_key_auth_new(struct key *target, const void *callout_info,
-				 size_t callout_len, struct key *dest_keyring)
+struct key *request_key_auth_new(struct key *target, const char *op,
+				 const void *callout_info, size_t callout_len,
+				 struct key *dest_keyring)
 {
 	struct request_key_auth *rka, *irka;
 	const struct cred *cred = current->cred;
@@ -167,6 +168,7 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info,
 	if (!rka->callout_info)
 		goto error_free_rka;
 	rka->callout_len = callout_len;
+	strlcpy(rka->op, op, sizeof(rka->op));
 
 	/* see if the calling process is already servicing the key request of
 	 * another process */
-- 
2.19.1


^ permalink raw reply related

* Re: [GIT PULL] SELinux fixes for v5.1 (#1)
From: pr-tracker-bot @ 2019-03-13 18:40 UTC (permalink / raw)
  To: Paul Moore; +Cc: Linus Torvalds, selinux, linux-security-module, linux-kernel
In-Reply-To: <CAHC9VhTNWou2axPZ5SeCD0pML_r6=r1Wok1U=ggONUR14O+mQw@mail.gmail.com>

The pull request you sent on Tue, 12 Mar 2019 19:12:50 -0400:

> git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git tags/selinux-pr-20190312

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/fa3d493f7a573b4e4e2538486e912093a0161c1b

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

^ permalink raw reply

* Re: [GIT PULL] apparmor updates for v5.1
From: pr-tracker-bot @ 2019-03-13 18:40 UTC (permalink / raw)
  To: John Johansen; +Cc: Linus Torvalds, LKLM, open list:SECURITY SUBSYSTEM
In-Reply-To: <d695d181-eaa1-eb62-9d01-56e4e52178d7@canonical.com>

The pull request you sent on Tue, 12 Mar 2019 05:23:30 -0700:

> git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor tags/apparmor-pr-2019-03-12

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/8636b1dbce854363cd98922b3e4b49a603c4d5fd

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

^ permalink raw reply

* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: Paul Moore @ 2019-03-13 13:17 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: James Morris, linux-security-module, Dmitry Vyukov, syzbot,
	syzbot
In-Reply-To: <8c586ed0-e9ee-a450-df06-3257cdf657ad@i-love.sakura.ne.jp>

On Wed, Mar 13, 2019 at 6:29 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> On 2019/03/13 6:24, James Morris wrote:
> > On Wed, 13 Mar 2019, Tetsuo Handa wrote:
> >
> >>> I don't understand the logic here. If the cause of this is no policy
> >>> loaded combined with running out of memory, shouldn't the no-policy issue
> >>> be dealt with earlier?
> >>>
> >>
> >> This patch is for automatically loading minimal policy at boot time
> >> in order to address the no-policy issue. By applying this patch, syzbot
> >> can test TOMOYO module without modifying userspace to load TOMOYO's policy
> >> when /sbin/init starts.
> >
> > If syzbot is trying to test Tomoyo and this requires policy to be loaded,
> > shouldn't it do that?
>
> SELinux has disabled/permissive/enforcing modes.
> And syzbot is testing SELinux in permissive mode, isn't it?

I've lost track of what syzbot currently does, but in the beginning it
ran with SELinux enabled (probably in permissive mode, but that isn't
important here) without a policy loaded and that caused a handful of
problems which we have since fixed.  While it is not recommended, you
should be able to safely run a SELinux enabled system without a policy
loaded.

> TOMOYO has disabled/learning/permissive/enforcing modes.
> And syzbot will test TOMOYO in learning mode.
>
> This patch is required for telling TOMOYO to run in learning mode, by
> loading minimal policy, without asking userspace to run policy loader.
> This patch is easier than asking syzbot users to update their filesystem
> images in order to embed policy loader and minimal policy into their
> filesystem images.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: Tetsuo Handa @ 2019-03-13 10:29 UTC (permalink / raw)
  To: James Morris; +Cc: linux-security-module, Dmitry Vyukov, syzbot, syzbot
In-Reply-To: <alpine.LRH.2.21.1903130821250.2172@namei.org>

On 2019/03/13 6:24, James Morris wrote:
> On Wed, 13 Mar 2019, Tetsuo Handa wrote:
> 
>>> I don't understand the logic here. If the cause of this is no policy 
>>> loaded combined with running out of memory, shouldn't the no-policy issue 
>>> be dealt with earlier?
>>>
>>
>> This patch is for automatically loading minimal policy at boot time
>> in order to address the no-policy issue. By applying this patch, syzbot
>> can test TOMOYO module without modifying userspace to load TOMOYO's policy
>> when /sbin/init starts.
> 
> If syzbot is trying to test Tomoyo and this requires policy to be loaded, 
> shouldn't it do that?

SELinux has disabled/permissive/enforcing modes.
And syzbot is testing SELinux in permissive mode, isn't it?

TOMOYO has disabled/learning/permissive/enforcing modes.
And syzbot will test TOMOYO in learning mode.

This patch is required for telling TOMOYO to run in learning mode, by
loading minimal policy, without asking userspace to run policy loader.
This patch is easier than asking syzbot users to update their filesystem
images in order to embed policy loader and minimal policy into their
filesystem images.

> 
> And again, I think the no-policy situation needs to be detected before 
> you start trying to apply memory policies to running processes. Surely 
> there is some much earlier point during initialization that you will 
> detect that there is no policy?

TOMOYO is already detecting no-policy situation. TOMOYO is calling panic()
due to "memory allocation failure before loading minimal policy".

This patch avoids panic() by automatically loading minimal policy which is
embedded into the kernel.


^ permalink raw reply

* [GIT PULL] SELinux fixes for v5.1 (#1)
From: Paul Moore @ 2019-03-12 23:12 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: selinux, linux-security-module, linux-kernel

Hi Linus,

Two small fixes for SELinux in v5.1: one adds a buffer length check to
the SELinux SCTP code, the other ensures that the SELinux labeling for
a NFS mount is not disabled if the filesystem is mounted twice.
Please pull for v5.1-rc1.

Thanks,
-Paul

--
The following changes since commit 45189a1998e00f6375ebd49d1e18161acddd73de:

 selinux: fix avc audit messages (2019-02-05 12:34:33 -0500)

are available in the Git repository at:

 git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
   tags/selinux-pr-20190312

for you to fetch changes up to 3815a245b50124f0865415dcb606a034e97494d4:

 security/selinux: fix SECURITY_LSM_NATIVE_LABELS on reused superblock
   (2019-03-11 16:13:17 -0400)

----------------------------------------------------------------
selinux/stable-5.1 PR 20190312

----------------------------------------------------------------
J. Bruce Fields (1):
     security/selinux: fix SECURITY_LSM_NATIVE_LABELS on reused superblock

Xin Long (1):
     selinux: add the missing walk_size + len check in
              selinux_sctp_bind_connect

security/selinux/hooks.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* RE: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: Edwin Zimmerman @ 2019-03-12 21:56 UTC (permalink / raw)
  To: 'Tetsuo Handa', 'James Morris'
  Cc: 'Stephen Smalley', linux-security-module
In-Reply-To: <770d8413-bd57-c379-21a0-acbbdf82241e@i-love.sakura.ne.jp>

On March 12, 2019 5:15, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote
> On 2019/03/13 2:19, James Morris wrote:
> > On Mon, 11 Mar 2019, Tetsuo Handa wrote:
> >
> >> On 2019/03/05 12:32, James Morris wrote:
> >>> On Tue, 5 Mar 2019, Tetsuo Handa wrote:
> >>>
> >>>> I guess that majority of TOMOYO users are now using the upstream version. But
> >>>> pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
> >>>> officially supported
> >>>
> >>> You mean dynamically loadable LSMs?
> >>
> >> Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
> >> utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.
> >
> > What do you mean cannot afford ?
> >
> 
> Some people have to set SELINUX=disabled in /etc/selinux/config or pass security=none from
> the kernel command line.

If you specifically don't want in-kernel LSMs, and you specifically do want an out-of-tree LSM,
there are other options. For example, you could just livepatch the security_* hooks you need, 
since you already would using an LKM-based LSM.  That would give you your
out-of-tree module and would also disable selinux on the hooks that got livepatched.


^ permalink raw reply

* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: James Morris @ 2019-03-12 21:24 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: linux-security-module, Dmitry Vyukov, syzbot, syzbot
In-Reply-To: <5b69c819-eed4-1825-9e67-fff1206e9e3f@i-love.sakura.ne.jp>

On Wed, 13 Mar 2019, Tetsuo Handa wrote:

> > I don't understand the logic here. If the cause of this is no policy 
> > loaded combined with running out of memory, shouldn't the no-policy issue 
> > be dealt with earlier?
> > 
> 
> This patch is for automatically loading minimal policy at boot time
> in order to address the no-policy issue. By applying this patch, syzbot
> can test TOMOYO module without modifying userspace to load TOMOYO's policy
> when /sbin/init starts.

If syzbot is trying to test Tomoyo and this requires policy to be loaded, 
shouldn't it do that?

And again, I think the no-policy situation needs to be detected before 
you start trying to apply memory policies to running processes. Surely 
there is some much earlier point during initialization that you will 
detect that there is no policy?

-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply

* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: James Morris @ 2019-03-12 21:19 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: Stephen Smalley, linux-security-module
In-Reply-To: <770d8413-bd57-c379-21a0-acbbdf82241e@i-love.sakura.ne.jp>

On Wed, 13 Mar 2019, Tetsuo Handa wrote:

> >> Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
> >> utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.
> > 
> > What do you mean cannot afford ?
> > 
> 
> Some people have to set SELINUX=disabled in /etc/selinux/config or pass security=none from
> the kernel command line.

Why do they have to do this? 

-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply

* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: Tetsuo Handa @ 2019-03-12 21:15 UTC (permalink / raw)
  To: James Morris; +Cc: Stephen Smalley, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1903130418580.24386@namei.org>

On 2019/03/13 2:19, James Morris wrote:
> On Mon, 11 Mar 2019, Tetsuo Handa wrote:
> 
>> On 2019/03/05 12:32, James Morris wrote:
>>> On Tue, 5 Mar 2019, Tetsuo Handa wrote:
>>>
>>>> I guess that majority of TOMOYO users are now using the upstream version. But
>>>> pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
>>>> officially supported
>>>
>>> You mean dynamically loadable LSMs?
>>
>> Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
>> utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.
> 
> What do you mean cannot afford ?
> 

Some people have to set SELINUX=disabled in /etc/selinux/config or pass security=none from
the kernel command line.

^ permalink raw reply

* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: Tetsuo Handa @ 2019-03-12 20:56 UTC (permalink / raw)
  To: James Morris; +Cc: linux-security-module, Dmitry Vyukov, syzbot, syzbot
In-Reply-To: <alpine.LRH.2.21.1903130519440.27366@namei.org>

On 2019/03/13 3:21, James Morris wrote:
> On Thu, 28 Feb 2019, Tetsuo Handa wrote:
> 
>> syzbot is reporting kernel panic triggered by memory allocation fault
>> injection before loading TOMOYO's policy [1]. To make the fuzzing tests
>> useful, we need to assign a profile other than "disabled" (no-op) mode.
>> Therefore, let's allow syzbot to load TOMOYO's built-in policy for
>> "learning" mode using a kernel config option. This option must not be
>> enabled for kernels built for production system, for this option also
>> disables domain/program checks when modifying policy configuration via
>> /sys/kernel/security/tomoyo/ interface.
> 
> I don't understand the logic here. If the cause of this is no policy 
> loaded combined with running out of memory, shouldn't the no-policy issue 
> be dealt with earlier?
> 

This patch is for automatically loading minimal policy at boot time
in order to address the no-policy issue. By applying this patch, syzbot
can test TOMOYO module without modifying userspace to load TOMOYO's policy
when /sbin/init starts.

^ permalink raw reply

* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: James Morris @ 2019-03-12 18:21 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: linux-security-module, Dmitry Vyukov, syzbot, syzbot
In-Reply-To: <1551362770-8655-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>

On Thu, 28 Feb 2019, Tetsuo Handa wrote:

> syzbot is reporting kernel panic triggered by memory allocation fault
> injection before loading TOMOYO's policy [1]. To make the fuzzing tests
> useful, we need to assign a profile other than "disabled" (no-op) mode.
> Therefore, let's allow syzbot to load TOMOYO's built-in policy for
> "learning" mode using a kernel config option. This option must not be
> enabled for kernels built for production system, for this option also
> disables domain/program checks when modifying policy configuration via
> /sys/kernel/security/tomoyo/ interface.

I don't understand the logic here. If the cause of this is no policy 
loaded combined with running out of memory, shouldn't the no-policy issue 
be dealt with earlier?


-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply

* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: James Morris @ 2019-03-12 17:19 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: Stephen Smalley, linux-security-module
In-Reply-To: <72b3b8e1-fc34-f312-de12-cf42992bceb8@i-love.sakura.ne.jp>

On Mon, 11 Mar 2019, Tetsuo Handa wrote:

> On 2019/03/05 12:32, James Morris wrote:
> > On Tue, 5 Mar 2019, Tetsuo Handa wrote:
> > 
> >> I guess that majority of TOMOYO users are now using the upstream version. But
> >> pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
> >> officially supported
> > 
> > You mean dynamically loadable LSMs?
> 
> Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
> utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.

What do you mean cannot afford ?

-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply

* [GIT PULL] apparmor updates for v5.1
From: John Johansen @ 2019-03-12 12:23 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: LKLM, open list:SECURITY SUBSYSTEM

Hi,


Please pull these apparmor bugfixes for v5.1
Thanks!

- John


The following changes since commit 43aa09fee2f08c8d90a4f35d4c8c711362afcaee:

  apparmor: Fix warning about unused function apparmor_ipv6_postroute (2018-11-14 11:42:18 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor tags/apparmor-pr-2019-03-12

for you to fetch changes up to d8dbb581d4f86a2ac669c056fc71a28ebeb367f4:

  apparmor: fix double free when unpack of secmark rules fails (2019-03-12 03:48:02 -0700)

----------------------------------------------------------------
+ Bug Fixes
  - fix double when failing to unpack secmark rules in policy
  - fix leak of dentry when profile is removed

----------------------------------------------------------------
Chris Coulson (1):
      apparmor: delete the dentry in aafs_remove() to avoid a leak

John Johansen (1):
      apparmor: fix double free when unpack of secmark rules fails

 security/apparmor/apparmorfs.c    | 1 +
 security/apparmor/policy_unpack.c | 1 +
 2 files changed, 2 insertions(+)

^ permalink raw reply

* Re: [PULL REQUEST] Kernel lockdown patches for 5.2
From: Mimi Zohar @ 2019-03-12  1:52 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <CACdnJuszfo4gTUD88p+8jeHU5e3hSdp+GEr8_=F1AMThPb7yWg@mail.gmail.com>

On Mon, 2019-03-11 at 17:42 -0700, Matthew Garrett wrote:
> On Wed, Mar 6, 2019 at 8:24 PM Matthew Garrett <mjg59@google.com> wrote:
> >
> > On Wed, Mar 6, 2019 at 7:56 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > The kexec and kernel modules patches in this patch set continues to
> > > ignore IMA.  This patch set should up front either provide an
> > > alternative solution to coordinate the different signature
> > > verification methods or rely on the architecture specific policy for
> > > that coordination.
> >
> > Hi Mimi,
> >
> > I'm working on a patch for this at the moment which can then be added
> > to either patchset. Is there a tree that contains the proposed Power
> > architecture policy? I want to make sure I don't accidentally end up
> > depending on anything x86.
> 
> I've been digging into this some more, and want to ensure that I get
> the appropriate semantics. Are we happy with the x86 solution for
> module signing (ie, if the arch policy is enabled and the kernel
> supports module signatures, use module signatures rather than IMA
> signatures)? 

There's a slight nuance you're missing.  If the arch policy is enabled
and the kernel supports module signatures, do not add an IMA appraise
rule.  A custom policy could require an IMA signature, as well as the
module appended signature.

Saying only use the module signatures, even if the IMA custom policy
contains a kernel module rule, doesn't make sense.

> If so, that just leaves kexec. For platforms that support
> PE signing for kernels (x86 and arm), are we ok punting to that?

Similarly, if the custom policy has a kexec kernel image policy rule,
it shouldn't be ignored.

> If so
> then to maintain the semantics we have for lockdown in general (ie, no
> way for a user to modify ring 0 code) then I think that would mean
> allowing kexec_file() only when the following criteria are met:
> 
> 1) IMA is appraising kexec with digital signatures, either ima digital
> signatures or ima hashes with associated EVM digital signatures

The kernel image could be signed with an appended signature as well.

> 2) CONFIG_INTEGRITY_TRUSTED_KEYRING is set in order to prevent an
> attacker being able to add a key to the keyring

Agreed

> Does this sound reasonable? Are there any further criteria that are
> required for this?

With the caveats described above.

Mimi


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox