[PATCH RFC UEK5 2/7] bpf: Restrict bpf when kernel lockdown is in confidentiality mode

[PATCH RFC UEK5 2/7] bpf: Restrict bpf when kernel lockdown is in confidentiality mode

[Konrad Rzeszutek Wilk]

bpf_read() and bpf_read_str() could potentially be abused to (eg) allow
private keys in kernel memory to be leaked. Disable them if the kernel
has been locked down in confidentiality mode.

Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
cc: netdev@vger.kernel.org
cc: Chun-Yi Lee <jlee@suse.com>
cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: James Morris <jmorris@namei.org>

[Backport notes:
 The upstream version is using enums, and all that fancy code.
 We are just retroffiting UEK5 a bit and just checking to
 see if integrity mode has been enabled and if so then
 allow it. If the default lockdown mode (confidentiality) is on
 then we don't allow it.]

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 security/lock_down.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/security/lock_down.c b/security/lock_down.c
index 96ff1badfac0b..1b913f855d48d 100644
--- a/security/lock_down.c
+++ b/security/lock_down.c
@@ -57,9 +57,16 @@ void __init init_lockdown(void)
  */
 bool __kernel_is_locked_down(const char *what, bool first)
 {
-	if (what && first && kernel_locked_down)
+	if (what && first && kernel_locked_down) {
+		/* If we are in integrity mode we allow certain callsites */
+		if (!lockdown_confidentiality) {
+			if ((strcmp(what, "BPF") == 0)) {
+				return 0;
+			}
+		}
 		pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
 			  what);
+	}
 	return kernel_locked_down;
 }
 EXPORT_SYMBOL(__kernel_is_locked_down);
-- 
2.13.6

[PATCH RFC UEK5 5/7] debugfs: Restrict debugfs when the kernel is locked down

[Konrad Rzeszutek Wilk]

Disallow opening of debugfs files that might be used to muck around when
the kernel is locked down as various drivers give raw access to hardware
through debugfs.  Given the effort of auditing all 2000 or so files and
manually fixing each one as necessary, I've chosen to apply a heuristic
instead.  The following changes are made:

 (1) chmod and chown are disallowed on debugfs objects (though the root dir
     can be modified by mount and remount, but I'm not worried about that).

 (2) When the kernel is locked down, only files with the following criteria
     are permitted to be opened:

        - The file must have mode 00444
        - The file must not have ioctl methods
        - The file must not have mmap

 (3) When the kernel is locked down, files may only be opened for reading.

Normal device interaction should be done through configfs, sysfs or a
miscdev, not debugfs.

Note that this makes it unnecessary to specifically lock down show_dsts(),
show_devs() and show_call() in the asus-wmi driver.

I would actually prefer to lock down all files by default and have the
the files unlocked by the creator.  This is tricky to manage correctly,
though, as there are 19 creation functions and ~1600 call sites (some of
them in loops scanning tables).

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Andy Shevchenko <andy.shevchenko@gmail.com>
cc: acpi4asus-user@lists.sourceforge.net
cc: platform-driver-x86@vger.kernel.org
cc: Matthew Garrett <mjg59@srcf.ucam.org>
cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Greg KH <greg@kroah.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
Signed-off-by: James Morris <jmorris@namei.org>

[Backport:
 Since UEK5 by default is confidentiality we have to outright
 disallow debugfs if the default mode is selected. Hence the
 call to __kernel_is_confidentiality_mode to help us.

 If we are in integrity lockdown mode, we can enable debugfs
 IF they match with the above 1-3 criteria]

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 fs/debugfs/file.c    | 36 +++++++++++++++++++++++++++++++++---
 fs/debugfs/inode.c   | 34 ++++++++++++++++++++++++++++++++--
 security/lock_down.c |  1 +
 3 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 32b5168a7e910..86c7235dfd57b 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -97,15 +97,35 @@ EXPORT_SYMBOL_GPL(debugfs_use_file_finish);
 
 #define F_DENTRY(filp) ((filp)->f_path.dentry)
 
+
+/*
+ * Only permit access to world-readable files when the kernel is locked down.
+ * We also need to exclude any file that has ways to write or alter it as root
+ * can bypass the permissions check.
+ */
+static bool debugfs_is_locked_down(struct inode *inode,
+				   struct file *filp,
+				   const struct file_operations *real_fops)
+{
+	if (__kernel_is_confidentiality_mode())
+		return true;
+
+	if ((inode->i_mode & 07777) == 0444 &&
+	    !(filp->f_mode & FMODE_WRITE) &&
+	    !real_fops->unlocked_ioctl &&
+	    !real_fops->compat_ioctl &&
+	    !real_fops->mmap)
+		return false;
+
+	return kernel_is_locked_down("debugfs");
+}
+
 static int open_proxy_open(struct inode *inode, struct file *filp)
 {
 	const struct dentry *dentry = F_DENTRY(filp);
 	const struct file_operations *real_fops = NULL;
 	int srcu_idx, r;
 
-	if (kernel_is_locked_down("debugfs"))
-		return -EPERM;
-
 	r = debugfs_use_file_start(dentry, &srcu_idx);
 	if (r) {
 		r = -ENOENT;
@@ -113,6 +133,11 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
 	}
 
 	real_fops = debugfs_real_fops(filp);
+
+	r = debugfs_locked_down(inode, filp, real_fops);
+	if (r)
+		goto out;
+
 	real_fops = fops_get(real_fops);
 	if (!real_fops) {
 		/* Huh? Module did not clean up after itself at exit? */
@@ -245,6 +270,11 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
 	}
 
 	real_fops = debugfs_real_fops(filp);
+
+	r = debugfs_is_locked_down(inode, filp, real_fops);
+	if (r)
+		goto out;
+
 	real_fops = fops_get(real_fops);
 	if (!real_fops) {
 		/* Huh? Module did not cleanup after itself at exit? */
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index f4df6feec2713..5a42b2387dd07 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -39,6 +39,35 @@ static struct vfsmount *debugfs_mount;
 static int debugfs_mount_count;
 static bool debugfs_registered;
 
+/*
+ * Don't allow access attributes to be changed whilst the kernel is locked down
+ * so that we can use the file mode as part of a heuristic to determine whether
+ * to lock down individual files.
+ */
+static int debugfs_setattr(struct dentry *dentry, struct iattr *ia)
+{
+	int ret;
+
+	if (kernel_is_locked_down("debugfs"))
+		ret = -EPERM;
+
+	if (ret && (ia->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)))
+		return ret;
+	return simple_setattr(dentry, ia);
+}
+
+static const struct inode_operations debugfs_file_inode_operations = {
+	.setattr	= debugfs_setattr,
+};
+static const struct inode_operations debugfs_dir_inode_operations = {
+	.lookup		= simple_lookup,
+	.setattr	= debugfs_setattr,
+};
+static const struct inode_operations debugfs_symlink_inode_operations = {
+	.get_link	= simple_get_link,
+	.setattr	= debugfs_setattr,
+};
+
 static struct inode *debugfs_get_inode(struct super_block *sb)
 {
 	struct inode *inode = new_inode(sb);
@@ -362,6 +391,7 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
 	inode->i_mode = mode;
 	inode->i_private = data;
 
+	inode->i_op = &debugfs_file_inode_operations;
 	inode->i_fop = proxy_fops;
 	dentry->d_fsdata = (void *)real_fops;
 
@@ -518,7 +548,7 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
 		return failed_creating(dentry);
 
 	inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
-	inode->i_op = &simple_dir_inode_operations;
+	inode->i_op = &debugfs_dir_inode_operations;
 	inode->i_fop = &simple_dir_operations;
 
 	/* directory inodes start off with i_nlink == 2 (for "." entry) */
@@ -613,7 +643,7 @@ struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent,
 		return failed_creating(dentry);
 	}
 	inode->i_mode = S_IFLNK | S_IRWXUGO;
-	inode->i_op = &simple_symlink_inode_operations;
+	inode->i_op = &debugfs_symlink_inode_operations;
 	inode->i_link = link;
 	d_instantiate(dentry, inode);
 	return end_creating(dentry);
diff --git a/security/lock_down.c b/security/lock_down.c
index 1301b25137127..c709c70701235 100644
--- a/security/lock_down.c
+++ b/security/lock_down.c
@@ -61,6 +61,7 @@ bool __kernel_is_locked_down(const char *what, bool first)
 		/* If we are in integrity mode we allow certain callsites */
 		if (!lockdown_confidentiality) {
 			if ((strcmp(what, "BPF") == 0) ||
+			    (strcmp(what, "debugfs") == 0) ||
 			    (strcmp(what, "DTRACE") == 0)) {
 				return 0;
 			}
-- 
2.13.6

[PATCH RFC UEK5 6/7] debugfs: Return -EPERM when locked down

[Konrad Rzeszutek Wilk]

From: Eric Snowberg <eric.snowberg@oracle.com>

When lockdown is enabled, debugfs_is_locked_down returns 1. It will then
trigger the following:

WARNING: CPU: 48 PID: 3747
CPU: 48 PID: 3743 Comm: bash Not tainted 5.4.0-1946.x86_64 #1
Hardware name: Oracle Corporation ORACLE SERVER X7-2/ASM, MB, X7-2, BIOS 41060400 05/20/2019
RIP: 0010:do_dentry_open+0x343/0x3a0
Code: 00 40 08 00 45 31 ff 48 c7 43 28 40 5b e7 89 e9 02 ff ff ff 48 8b 53 28 4c 8b 72 70 4d 85 f6 0f 84 10 fe ff ff e9 f5 fd ff ff <0f> 0b 41 bf ea ff ff ff e9 3b ff ff ff 41 bf e6 ff ff ff e9 b4 fe
RSP: 0018:ffffb8740dde7ca0 EFLAGS: 00010202
RAX: ffffffff89e88a40 RBX: ffff928c8e6b6f00 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff928dbfd97778 RDI: ffff9285cff685c0
RBP: ffffb8740dde7cc8 R08: 0000000000000821 R09: 0000000000000030
R10: 0000000000000057 R11: ffffb8740dde7a98 R12: ffff926ec781c900
R13: ffff928c8e6b6f10 R14: ffffffff8936e190 R15: 0000000000000001
FS:  00007f45f6777740(0000) GS:ffff928dbfd80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fff95e0d5d8 CR3: 0000001ece562006 CR4: 00000000007606e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
 vfs_open+0x2d/0x30
 path_openat+0x2d4/0x1680
 ? tty_mode_ioctl+0x298/0x4c0
 do_filp_open+0x93/0x100
 ? strncpy_from_user+0x57/0x1b0
 ? __alloc_fd+0x46/0x150
 do_sys_open+0x182/0x230
 __x64_sys_openat+0x20/0x30
 do_syscall_64+0x60/0x1b0
 entry_SYSCALL_64_after_hwframe+0x170/0x1d5
RIP: 0033:0x7f45f5e5ce02
Code: 25 00 00 41 00 3d 00 00 41 00 74 4c 48 8d 05 25 59 2d 00 8b 00 85 c0 75 6d 89 f2 b8 01 01 00 00 48 89 fe bf 9c ff ff ff 0f 05 <48> 3d 00 f0 ff ff 0f 87 a2 00 00 00 48 8b 4c 24 28 64 48 33 0c 25
RSP: 002b:00007fff95e0d2e0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 0000561178c069b0 RCX: 00007f45f5e5ce02
RDX: 0000000000000241 RSI: 0000561178c08800 RDI: 00000000ffffff9c
RBP: 00007fff95e0d3e0 R08: 0000000000000020 R09: 0000000000000005
R10: 00000000000001b6 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000003 R14: 0000000000000001 R15: 0000561178c08800

Change the return type to int and return -EPERM when lockdown is enabled
to remove the warning above. Also rename debugfs_is_locked_down to
debugfs_locked_down to make it sound less like it returns a boolean.

Fixes: 5496197f9b08 ("debugfs: Restrict debugfs when the kernel is locked down")
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: stable <stable@vger.kernel.org>
Acked-by: James Morris <jamorris@linux.microsoft.com>
Link: https://lore.kernel.org/r/20191207161603.35907-1-eric.snowberg@oracle.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

(cherry picked from commit a37f4958f7b63d2b3cd17a76151fdfc29ce1da5f)
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

[Backport:

 Mostly the same, but needed to add an extra return for the
 confidentiality mode]
---
 fs/debugfs/file.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 86c7235dfd57b..87cd56dc637d7 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -103,21 +103,24 @@ EXPORT_SYMBOL_GPL(debugfs_use_file_finish);
  * We also need to exclude any file that has ways to write or alter it as root
  * can bypass the permissions check.
  */
-static bool debugfs_is_locked_down(struct inode *inode,
-				   struct file *filp,
-				   const struct file_operations *real_fops)
+static int debugfs_locked_down(struct inode *inode,
+			       struct file *filp,
+			       const struct file_operations *real_fops)
 {
 	if (__kernel_is_confidentiality_mode())
-		return true;
+		return -EPERM;
 
 	if ((inode->i_mode & 07777) == 0444 &&
 	    !(filp->f_mode & FMODE_WRITE) &&
 	    !real_fops->unlocked_ioctl &&
 	    !real_fops->compat_ioctl &&
 	    !real_fops->mmap)
-		return false;
+		return 0;
+
+	if (kernel_is_locked_down("debugfs"))
+		return -EPERM;
 
-	return kernel_is_locked_down("debugfs");
+	return 0;
 }
 
 static int open_proxy_open(struct inode *inode, struct file *filp)
@@ -271,7 +274,7 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
 
 	real_fops = debugfs_real_fops(filp);
 
-	r = debugfs_is_locked_down(inode, filp, real_fops);
+	r = debugfs_locked_down(inode, filp, real_fops);
 	if (r)
 		goto out;
 
-- 
2.13.6

Re: [PATCH RFC UEK5 2/7] bpf: Restrict bpf when kernel lockdown is in confidentiality mode

[Konrad Rzeszutek Wilk]

On Tue, Oct 20, 2020 at 04:59:59PM -0400, Konrad Rzeszutek Wilk wrote:
> bpf_read() and bpf_read_str() could potentially be abused to (eg) allow
> private keys in kernel memory to be leaked. Disable them if the kernel
> has been locked down in confidentiality mode.
> 
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> cc: netdev@vger.kernel.org
> cc: Chun-Yi Lee <jlee@suse.com>
> cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: James Morris <jmorris@namei.org>
> 
> [Backport notes:
>  The upstream version is using enums, and all that fancy code.
>  We are just retroffiting UEK5 a bit and just checking to
>  see if integrity mode has been enabled and if so then
>  allow it. If the default lockdown mode (confidentiality) is on
>  then we don't allow it.]

<sigh>

And that is what I get for _not_ doing --suppress-cc=all

My apologies for spamming you all!

<goes to hide in the corner of shame>

Re: [PATCH RFC UEK5 5/7] debugfs: Restrict debugfs when the kernel is locked down

[Konrad Rzeszutek Wilk]

On Tue, Oct 20, 2020 at 05:00:02PM -0400, Konrad Rzeszutek Wilk wrote:
> Disallow opening of debugfs files that might be used to muck around when

..snip..

> [Backport:
>  Since UEK5 by default is confidentiality we have to outright
>  disallow debugfs if the default mode is selected. Hence the
>  call to __kernel_is_confidentiality_mode to help us.
> 
>  If we are in integrity lockdown mode, we can enable debugfs
>  IF they match with the above 1-3 criteria]

<sigh>

And that is what I get for _not_ doing --suppress-cc=all

My apologies for spamming you all!

<goes to hide in the corner of shame>

Re: [PATCH RFC UEK5 6/7] debugfs: Return -EPERM when locked down

[Konrad Rzeszutek Wilk]

On Tue, Oct 20, 2020 at 05:00:03PM -0400, Konrad Rzeszutek Wilk wrote:
> From: Eric Snowberg <eric.snowberg@oracle.com>

..monster snip..
> Cc: stable <stable@vger.kernel.org>

..stable tree folks please ignore this. 

<sigh>

My apologies for spamming you all!

<goes to hide in the corner of shame>

Re: [PATCH RFC UEK5 5/7] debugfs: Restrict debugfs when the kernel is locked down

[Hans de Goede]

Hi,

On 10/20/20 11:08 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, Oct 20, 2020 at 05:00:02PM -0400, Konrad Rzeszutek Wilk wrote:
>> Disallow opening of debugfs files that might be used to muck around when
> 
> ..snip..
> 
>> [Backport:
>>  Since UEK5 by default is confidentiality we have to outright
>>  disallow debugfs if the default mode is selected. Hence the
>>  call to __kernel_is_confidentiality_mode to help us.
>>
>>  If we are in integrity lockdown mode, we can enable debugfs
>>  IF they match with the above 1-3 criteria]
> 
> <sigh>
> 
> And that is what I get for _not_ doing --suppress-cc=all
> 
> My apologies for spamming you all!

Actually I find this a quite interesting patch, I think it would be
good to get something like this done upstream, rather then relying
on a downstream distro specific patch.

Are there any plans to submit this upstream ?

Regards,

Hans