2016-04-20 22:47:36

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 0/6] LSM: LoadPin for kernel file loading restrictions

This provides the mini-LSM "loadpin" that intercepts the now consolidated
kernel_file_read LSM hook so that a system can keep all loads coming from
a single trusted filesystem. This is what Chrome OS uses to pin kernel
module and firmware loading to the read-only crypto-verified dm-verity
partition so that kernel module signing is not needed.

-Kees

v5:
- replace enum-to-str code, mimi
v4:
- add missing "const" to char * src, joe
v3:
- changed module parameter to "loadpin.enabled"
- add sysctl docs, akpm
- add general use function for enum, zohar
- add gfp_t, joe
- clean up loops, andriy.shevchenko
- reduce BUG_ON to WARN_ON, joe
v2:
- break out utility helpers into separate functions
- have Yama use new helpers too


2016-04-20 22:46:40

by Kees Cook

[permalink] [raw]
Subject: [PATCH 3/6] string_helpers: add kstrdup_quotable_file

Allocate a NULL-terminated file path with special characters escaped,
safe for logging.

Signed-off-by: Kees Cook <[email protected]>
---
include/linux/string_helpers.h | 3 +++
lib/string_helpers.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+)

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index 684d2695fc36..5ce9538f290e 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -3,6 +3,8 @@

#include <linux/types.h>

+struct file;
+
/* Descriptions of the types of units to
* print in */
enum string_size_units {
@@ -70,5 +72,6 @@ static inline int string_escape_str_any_np(const char *src, char *dst,

char *kstrdup_quotable(const char *src, gfp_t gfp);
char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp);
+char *kstrdup_quotable_file(struct file *file, gfp_t gfp);

#endif
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index b16ee85aaf87..ecaac2c0526f 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -10,6 +10,8 @@
#include <linux/export.h>
#include <linux/ctype.h>
#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/limits.h>
#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/string.h>
@@ -596,3 +598,31 @@ char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp)
return quoted;
}
EXPORT_SYMBOL_GPL(kstrdup_quotable_cmdline);
+
+/*
+ * Returns allocated NULL-terminated string containing pathname,
+ * with special characters escaped, able to be safely logged. If
+ * there is an error, the leading character will be "<".
+ */
+char *kstrdup_quotable_file(struct file *file, gfp_t gfp)
+{
+ char *temp, *pathname;
+
+ if (!file)
+ return kstrdup("<unknown>", gfp);
+
+ /* We add 11 spaces for ' (deleted)' to be appended */
+ temp = kmalloc(PATH_MAX + 11, GFP_TEMPORARY);
+ if (!temp)
+ return kstrdup("<no_memory>", gfp);
+
+ pathname = file_path(file, temp, PATH_MAX + 11);
+ if (IS_ERR(pathname))
+ pathname = kstrdup("<too_long>", gfp);
+ else
+ pathname = kstrdup_quotable(pathname, gfp);
+
+ kfree(temp);
+ return pathname;
+}
+EXPORT_SYMBOL_GPL(kstrdup_quotable_file);
--
2.6.3

2016-04-20 22:46:38

by Kees Cook

[permalink] [raw]
Subject: [PATCH 1/6] string_helpers: add kstrdup_quotable

Handle allocating and escaping a string safe for logging.

Signed-off-by: Kees Cook <[email protected]>
---
include/linux/string_helpers.h | 2 ++
lib/string_helpers.c | 28 ++++++++++++++++++++++++++++
2 files changed, 30 insertions(+)

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index dabe643eb5fa..9de228af00c1 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -68,4 +68,6 @@ static inline int string_escape_str_any_np(const char *src, char *dst,
return string_escape_str(src, dst, sz, ESCAPE_ANY_NP, only);
}

+char *kstrdup_quotable(const char *src, gfp_t gfp);
+
#endif
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 5c88204b6f1f..aa00c9f989ee 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -10,6 +10,7 @@
#include <linux/export.h>
#include <linux/ctype.h>
#include <linux/errno.h>
+#include <linux/slab.h>
#include <linux/string.h>
#include <linux/string_helpers.h>

@@ -534,3 +535,30 @@ int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
return p - dst;
}
EXPORT_SYMBOL(string_escape_mem);
+
+/*
+ * Return an allocated string that has been escaped of special characters
+ * and double quotes, making it safe to log in quotes.
+ */
+char *kstrdup_quotable(const char *src, gfp_t gfp)
+{
+ size_t slen, dlen;
+ char *dst;
+ const int flags = ESCAPE_HEX;
+ const char esc[] = "\f\n\r\t\v\a\e\\\"";
+
+ if (!src)
+ return NULL;
+ slen = strlen(src);
+
+ dlen = string_escape_mem(src, slen, NULL, 0, flags, esc);
+ dst = kmalloc(dlen + 1, gfp);
+ if (!dst)
+ return NULL;
+
+ WARN_ON(string_escape_mem(src, slen, dst, dlen, flags, esc) != dlen);
+ dst[dlen] = '\0';
+
+ return dst;
+}
+EXPORT_SYMBOL_GPL(kstrdup_quotable);
--
2.6.3

2016-04-20 22:47:01

by Kees Cook

[permalink] [raw]
Subject: [PATCH 6/6] LSM: LoadPin for kernel file loading restrictions

This LSM enforces that kernel-loaded files (modules, firmware, etc)
must all come from the same filesystem, with the expectation that
such a filesystem is backed by a read-only device such as dm-verity
or CDROM. This allows systems that have a verified and/or unchangeable
filesystem to enforce module and firmware loading restrictions without
needing to sign the files individually.

Signed-off-by: Kees Cook <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
Documentation/security/LoadPin.txt | 17 ++++
MAINTAINERS | 6 ++
include/linux/lsm_hooks.h | 5 +
security/Kconfig | 1 +
security/Makefile | 2 +
security/loadpin/Kconfig | 10 ++
security/loadpin/Makefile | 1 +
security/loadpin/loadpin.c | 190 +++++++++++++++++++++++++++++++++++++
security/security.c | 1 +
9 files changed, 233 insertions(+)
create mode 100644 Documentation/security/LoadPin.txt
create mode 100644 security/loadpin/Kconfig
create mode 100644 security/loadpin/Makefile
create mode 100644 security/loadpin/loadpin.c

diff --git a/Documentation/security/LoadPin.txt b/Documentation/security/LoadPin.txt
new file mode 100644
index 000000000000..e11877f5d3d4
--- /dev/null
+++ b/Documentation/security/LoadPin.txt
@@ -0,0 +1,17 @@
+LoadPin is a Linux Security Module that ensures all kernel-loaded files
+(modules, firmware, etc) all originate from the same filesystem, with
+the expectation that such a filesystem is backed by a read-only device
+such as dm-verity or CDROM. This allows systems that have a verified
+and/or unchangeable filesystem to enforce module and firmware loading
+restrictions without needing to sign the files individually.
+
+The LSM is selectable at build-time with CONFIG_SECURITY_LOADPIN, and
+can be controlled at boot-time with the kernel command line option
+"loadpin.enabled". By default, it is enabled, but can be disabled at
+boot ("loadpin.enabled=0").
+
+LoadPin starts pinning when it sees the first file loaded. If the
+block device backing the filesystem is not read-only, a sysctl is
+created to toggle pinning: /proc/sys/kernel/loadpin/enabled. (Having
+a mutable filesystem means pinning is mutable too, but having the
+sysctl allows for easy testing on systems with a mutable filesystem.)
diff --git a/MAINTAINERS b/MAINTAINERS
index 1d5b4becab6f..6042f6318c8f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9971,6 +9971,12 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/jj/apparmor-dev.git
S: Supported
F: security/apparmor/

+LOADPIN SECURITY MODULE
+M: Kees Cook <[email protected]>
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git lsm/loadpin
+S: Supported
+F: security/loadpin/
+
YAMA SECURITY MODULE
M: Kees Cook <[email protected]>
T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git yama/tip
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index cdee11cbcdf1..f3402aab1927 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1893,5 +1893,10 @@ extern void __init yama_add_hooks(void);
#else
static inline void __init yama_add_hooks(void) { }
#endif
+#ifdef CONFIG_SECURITY_LOADPIN
+void __init loadpin_add_hooks(void);
+#else
+static inline void loadpin_add_hooks(void) { };
+#endif

#endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/Kconfig b/security/Kconfig
index e45237897b43..176758cdfa57 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -122,6 +122,7 @@ source security/selinux/Kconfig
source security/smack/Kconfig
source security/tomoyo/Kconfig
source security/apparmor/Kconfig
+source security/loadpin/Kconfig
source security/yama/Kconfig

source security/integrity/Kconfig
diff --git a/security/Makefile b/security/Makefile
index c9bfbc84ff50..f2d71cdb8e19 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -8,6 +8,7 @@ subdir-$(CONFIG_SECURITY_SMACK) += smack
subdir-$(CONFIG_SECURITY_TOMOYO) += tomoyo
subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
subdir-$(CONFIG_SECURITY_YAMA) += yama
+subdir-$(CONFIG_SECURITY_LOADPIN) += loadpin

# always enable default capabilities
obj-y += commoncap.o
@@ -22,6 +23,7 @@ obj-$(CONFIG_AUDIT) += lsm_audit.o
obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/
obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/
obj-$(CONFIG_SECURITY_YAMA) += yama/
+obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/
obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o

# Object integrity file lists
diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
new file mode 100644
index 000000000000..c668ac4eda65
--- /dev/null
+++ b/security/loadpin/Kconfig
@@ -0,0 +1,10 @@
+config SECURITY_LOADPIN
+ bool "Pin load of kernel files (modules, fw, etc) to one filesystem"
+ depends on SECURITY && BLOCK
+ help
+ Any files read through the kernel file reading interface
+ (kernel modules, firmware, kexec images, security policy) will
+ be pinned to the first filesystem used for loading. Any files
+ that come from other filesystems will be rejected. This is best
+ used on systems without an initrd that have a root filesystem
+ backed by a read-only device such as dm-verity or a CDROM.
diff --git a/security/loadpin/Makefile b/security/loadpin/Makefile
new file mode 100644
index 000000000000..c2d77f83037b
--- /dev/null
+++ b/security/loadpin/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SECURITY_LOADPIN) += loadpin.o
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
new file mode 100644
index 000000000000..e4debae3c4d6
--- /dev/null
+++ b/security/loadpin/loadpin.c
@@ -0,0 +1,190 @@
+/*
+ * Module and Firmware Pinning Security Module
+ *
+ * Copyright 2011-2016 Google Inc.
+ *
+ * Author: Kees Cook <[email protected]>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt) "LoadPin: " fmt
+
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/fs_struct.h>
+#include <linux/lsm_hooks.h>
+#include <linux/mount.h>
+#include <linux/path.h>
+#include <linux/sched.h> /* current */
+#include <linux/string_helpers.h>
+
+static void report_load(const char *origin, struct file *file, char *operation)
+{
+ char *cmdline, *pathname;
+
+ pathname = kstrdup_quotable_file(file, GFP_KERNEL);
+ cmdline = kstrdup_quotable_cmdline(current, GFP_KERNEL);
+
+ pr_notice("%s %s obj=%s%s%s pid=%d cmdline=%s%s%s\n",
+ origin, operation,
+ (pathname && pathname[0] != '<') ? "\"" : "",
+ pathname,
+ (pathname && pathname[0] != '<') ? "\"" : "",
+ task_pid_nr(current),
+ cmdline ? "\"" : "", cmdline, cmdline ? "\"" : "");
+
+ kfree(cmdline);
+ kfree(pathname);
+}
+
+static int enabled = 1;
+static struct super_block *pinned_root;
+static DEFINE_SPINLOCK(pinned_root_spinlock);
+
+#ifdef CONFIG_SYSCTL
+static int zero;
+static int one = 1;
+
+static struct ctl_path loadpin_sysctl_path[] = {
+ { .procname = "kernel", },
+ { .procname = "loadpin", },
+ { }
+};
+
+static struct ctl_table loadpin_sysctl_table[] = {
+ {
+ .procname = "enabled",
+ .data = &enabled,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &zero,
+ .extra2 = &one,
+ },
+ { }
+};
+
+/*
+ * This must be called after early kernel init, since then the rootdev
+ * is available.
+ */
+static void check_pinning_enforcement(struct super_block *mnt_sb)
+{
+ bool ro = false;
+
+ /*
+ * If load pinning is not enforced via a read-only block
+ * device, allow sysctl to change modes for testing.
+ */
+ if (mnt_sb->s_bdev) {
+ ro = bdev_read_only(mnt_sb->s_bdev);
+ pr_info("dev(%u,%u): %s\n",
+ MAJOR(mnt_sb->s_bdev->bd_dev),
+ MINOR(mnt_sb->s_bdev->bd_dev),
+ ro ? "read-only" : "writable");
+ } else
+ pr_info("mnt_sb lacks block device, treating as: writable\n");
+
+ if (!ro) {
+ if (!register_sysctl_paths(loadpin_sysctl_path,
+ loadpin_sysctl_table))
+ pr_notice("sysctl registration failed!\n");
+ else
+ pr_info("load pinning can be disabled.\n");
+ } else
+ pr_info("load pinning engaged.\n");
+}
+#else
+static void check_pinning_enforcement(struct super_block *mnt_sb)
+{
+ pr_info("load pinning engaged.\n");
+}
+#endif
+
+static void loadpin_sb_free_security(struct super_block *mnt_sb)
+{
+ /*
+ * When unmounting the filesystem we were using for load
+ * pinning, we acknowledge the superblock release, but make sure
+ * no other modules or firmware can be loaded.
+ */
+ if (!IS_ERR_OR_NULL(pinned_root) && mnt_sb == pinned_root) {
+ pinned_root = ERR_PTR(-EIO);
+ pr_info("umount pinned fs: refusing further loads\n");
+ }
+}
+
+static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
+{
+ struct super_block *load_root;
+ const char *origin = kernel_read_file_id_str(id);
+
+ /* This handles the older init_module API that has a NULL file. */
+ if (!file) {
+ if (!enabled) {
+ report_load(origin, NULL, "old-api-pinning-ignored");
+ return 0;
+ }
+
+ report_load(origin, NULL, "old-api-denied");
+ return -EPERM;
+ }
+
+ load_root = file->f_path.mnt->mnt_sb;
+
+ /* First loaded module/firmware defines the root for all others. */
+ spin_lock(&pinned_root_spinlock);
+ /*
+ * pinned_root is only NULL at startup. Otherwise, it is either
+ * a valid reference, or an ERR_PTR.
+ */
+ if (!pinned_root) {
+ pinned_root = load_root;
+ /*
+ * Unlock now since it's only pinned_root we care about.
+ * In the worst case, we will (correctly) report pinning
+ * failures before we have announced that pinning is
+ * enabled. This would be purely cosmetic.
+ */
+ spin_unlock(&pinned_root_spinlock);
+ check_pinning_enforcement(pinned_root);
+ report_load(origin, file, "pinned");
+ } else {
+ spin_unlock(&pinned_root_spinlock);
+ }
+
+ if (IS_ERR_OR_NULL(pinned_root) || load_root != pinned_root) {
+ if (unlikely(!enabled)) {
+ report_load(origin, file, "pinning-ignored");
+ return 0;
+ }
+
+ report_load(origin, file, "denied");
+ return -EPERM;
+ }
+
+ return 0;
+}
+
+static struct security_hook_list loadpin_hooks[] = {
+ LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
+ LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
+};
+
+void __init loadpin_add_hooks(void)
+{
+ pr_info("ready to pin (currently %sabled)", enabled ? "en" : "dis");
+ security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks));
+}
+
+/* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
+module_param(enabled, int, 0);
+MODULE_PARM_DESC(enabled, "Pin module/firmware loading (default: true)");
diff --git a/security/security.c b/security/security.c
index 3644b0344d29..ce02178c892f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -60,6 +60,7 @@ int __init security_init(void)
*/
capability_add_hooks();
yama_add_hooks();
+ loadpin_add_hooks();

/*
* Load all the remaining security modules.
--
2.6.3

2016-04-20 22:47:04

by Kees Cook

[permalink] [raw]
Subject: [PATCH 5/6] fs: define a string representation of the kernel_read_file_id enumeration

From: Mimi Zohar <[email protected]>

A string representation of the kernel_read_file_id enumeration is
needed for displaying messages (eg. pr_info, auditing) that can be
used by multiple LSMs and the integrity subsystem. To simplify
keeping the list of strings up to date with the enumeration, this
patch defines two new preprocessing macros named __fid_enumify and
__fid_stringify to create the enumeration and an array of strings.
kernel_read_file_id_str() returns a string based on the enumeration.

Signed-off-by: Mimi Zohar <[email protected]>
[kees: removed removal of my old version, constified pointer values]
Signed-off-by: Kees Cook <[email protected]>
---
include/linux/fs.h | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 70e61b58baaf..518716b4834e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2590,15 +2590,34 @@ static inline void i_readcount_inc(struct inode *inode)
#endif
extern int do_pipe_flags(int *, int);

+#define __kernel_read_file_id(id) \
+ id(UNKNOWN, unknown) \
+ id(FIRMWARE, firmware) \
+ id(MODULE, kernel-module) \
+ id(KEXEC_IMAGE, kexec-image) \
+ id(KEXEC_INITRAMFS, kexec-initramfs) \
+ id(POLICY, security-policy) \
+ id(MAX_ID, )
+
+#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
+#define __fid_stringify(dummy, str) #str,
+
enum kernel_read_file_id {
- READING_FIRMWARE = 1,
- READING_MODULE,
- READING_KEXEC_IMAGE,
- READING_KEXEC_INITRAMFS,
- READING_POLICY,
- READING_MAX_ID
+ __kernel_read_file_id(__fid_enumify)
+};
+
+static const char * const kernel_read_file_str[] = {
+ __kernel_read_file_id(__fid_stringify)
};

+static inline const char * const kernel_read_file_id_str(enum kernel_read_file_id id)
+{
+ if (id < 0 || id >= READING_MAX_ID)
+ return kernel_read_file_str[READING_UNKNOWN];
+
+ return kernel_read_file_str[id];
+}
+
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
enum kernel_read_file_id);
--
2.6.3

2016-04-20 22:48:05

by Kees Cook

[permalink] [raw]
Subject: [PATCH 4/6] Yama: consolidate error reporting

Use a common error reporting function for Yama violation reports, and give
more detail into the process command lines.

Signed-off-by: Kees Cook <[email protected]>
---
security/yama/yama_lsm.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index cb6ed10816d4..c19f6e5df9a3 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -18,6 +18,7 @@
#include <linux/prctl.h>
#include <linux/ratelimit.h>
#include <linux/workqueue.h>
+#include <linux/string_helpers.h>

#define YAMA_SCOPE_DISABLED 0
#define YAMA_SCOPE_RELATIONAL 1
@@ -41,6 +42,22 @@ static DEFINE_SPINLOCK(ptracer_relations_lock);
static void yama_relation_cleanup(struct work_struct *work);
static DECLARE_WORK(yama_relation_work, yama_relation_cleanup);

+static void report_access(const char *access, struct task_struct *target,
+ struct task_struct *agent)
+{
+ char *target_cmd, *agent_cmd;
+
+ target_cmd = kstrdup_quotable_cmdline(target, GFP_KERNEL);
+ agent_cmd = kstrdup_quotable_cmdline(agent, GFP_KERNEL);
+
+ pr_notice_ratelimited(
+ "ptrace %s of \"%s\"[%d] was attempted by \"%s\"[%d]\n",
+ access, target_cmd, target->pid, agent_cmd, agent->pid);
+
+ kfree(agent_cmd);
+ kfree(target_cmd);
+}
+
/**
* yama_relation_cleanup - remove invalid entries from the relation list
*
@@ -307,11 +324,8 @@ static int yama_ptrace_access_check(struct task_struct *child,
}
}

- if (rc && (mode & PTRACE_MODE_NOAUDIT) == 0) {
- printk_ratelimited(KERN_NOTICE
- "ptrace of pid %d was attempted by: %s (pid %d)\n",
- child->pid, current->comm, current->pid);
- }
+ if (rc && (mode & PTRACE_MODE_NOAUDIT) == 0)
+ report_access("attach", child, current);

return rc;
}
@@ -337,11 +351,8 @@ int yama_ptrace_traceme(struct task_struct *parent)
break;
}

- if (rc) {
- printk_ratelimited(KERN_NOTICE
- "ptraceme of pid %d was attempted by: %s (pid %d)\n",
- current->pid, parent->comm, parent->pid);
- }
+ if (rc)
+ report_access("traceme", current, parent);

return rc;
}
--
2.6.3

2016-04-20 22:48:02

by Kees Cook

[permalink] [raw]
Subject: [PATCH 2/6] string_helpers: add kstrdup_quotable_cmdline

Provide an escaped (but readable: no inter-argument NULLs) commandline
safe for logging.

Signed-off-by: Kees Cook <[email protected]>
---
include/linux/string_helpers.h | 1 +
lib/string_helpers.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index 9de228af00c1..684d2695fc36 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -69,5 +69,6 @@ static inline int string_escape_str_any_np(const char *src, char *dst,
}

char *kstrdup_quotable(const char *src, gfp_t gfp);
+char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp);

#endif
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index aa00c9f989ee..b16ee85aaf87 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -10,6 +10,7 @@
#include <linux/export.h>
#include <linux/ctype.h>
#include <linux/errno.h>
+#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/string_helpers.h>
@@ -562,3 +563,36 @@ char *kstrdup_quotable(const char *src, gfp_t gfp)
return dst;
}
EXPORT_SYMBOL_GPL(kstrdup_quotable);
+
+/*
+ * Returns allocated NULL-terminated string containing process
+ * command line, with inter-argument NULLs replaced with spaces,
+ * and other special characters escaped.
+ */
+char *kstrdup_quotable_cmdline(struct task_struct *task, gfp_t gfp)
+{
+ char *buffer, *quoted;
+ int i, res;
+
+ buffer = kmalloc(PAGE_SIZE, GFP_TEMPORARY);
+ if (!buffer)
+ return NULL;
+
+ res = get_cmdline(task, buffer, PAGE_SIZE - 1);
+ buffer[res] = '\0';
+
+ /* Collapse trailing NULLs, leave res pointing to last non-NULL. */
+ while (--res >= 0 && buffer[res] == '\0')
+ ;
+
+ /* Replace inter-argument NULLs. */
+ for (i = 0; i <= res; i++)
+ if (buffer[i] == '\0')
+ buffer[i] = ' ';
+
+ /* Make sure result is printable. */
+ quoted = kstrdup_quotable(buffer, gfp);
+ kfree(buffer);
+ return quoted;
+}
+EXPORT_SYMBOL_GPL(kstrdup_quotable_cmdline);
--
2.6.3

2016-04-21 00:22:42

by James Morris

[permalink] [raw]
Subject: Re: [PATCH v5 0/6] LSM: LoadPin for kernel file loading restrictions

On Wed, 20 Apr 2016, Kees Cook wrote:

> This provides the mini-LSM "loadpin" that intercepts the now consolidated
> kernel_file_read LSM hook so that a system can keep all loads coming from
> a single trusted filesystem. This is what Chrome OS uses to pin kernel
> module and firmware loading to the read-only crypto-verified dm-verity
> partition so that kernel module signing is not needed.
>

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next


--
James Morris
<[email protected]>

2016-04-21 13:25:33

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 5/6] fs: define a string representation of the kernel_read_file_id enumeration

On Wed, 2016-04-20 at 15:46 -0700, Kees Cook wrote:
> From: Mimi Zohar <[email protected]>
>
> A string representation of the kernel_read_file_id enumeration is
> needed for displaying messages (eg. pr_info, auditing) that can be
> used by multiple LSMs and the integrity subsystem.  To simplify
> keeping the list of strings up to date with the enumeration, this
> patch defines two new preprocessing macros named __fid_enumify and
> __fid_stringify to create the enumeration and an array of strings.
> kernel_read_file_id_str() returns a string based on the enumeration.
>
> Signed-off-by: Mimi Zohar <[email protected]>
> [kees: removed removal of my old version, constified pointer values]
> Signed-off-by: Kees Cook <[email protected]>


I got this

In file included from /home/andy/prj/linux-
topic/include/linux/seq_file.h:10:0,
                 from /home/andy/prj/linux-
topic/include/linux/pinctrl/consumer.h:17,
                 from /home/andy/prj/linux-
topic/include/linux/pinctrl/devinfo.h:21,
                 from /home/andy/prj/linux-
topic/include/linux/device.h:24,
                 from /home/andy/prj/linux-
topic/include/linux/dmaengine.h:20,
                 from /home/andy/prj/linux-
topic/drivers/dma/dw/core.c:15:
/home/andy/prj/linux-topic/include/linux/fs.h:2627:74: warning: type
qualifiers ignored on function return type [-Wignored-qualifiers]
 static inline const char * const kernel_read_file_id_str(enum
kernel_read_file_id id)

> ---
>  include/linux/fs.h | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 70e61b58baaf..518716b4834e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2590,15 +2590,34 @@ static inline void i_readcount_inc(struct
> inode *inode)
>  #endif
>  extern int do_pipe_flags(int *, int);
>  
> +#define __kernel_read_file_id(id) \
> + id(UNKNOWN, unknown) \
> + id(FIRMWARE, firmware) \
> + id(MODULE, kernel-module) \
> + id(KEXEC_IMAGE, kexec-image) \
> + id(KEXEC_INITRAMFS, kexec-initramfs) \
> + id(POLICY, security-policy) \
> + id(MAX_ID, )
> +
> +#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
> +#define __fid_stringify(dummy, str) #str,
> +
>  enum kernel_read_file_id {
> - READING_FIRMWARE = 1,
> - READING_MODULE,
> - READING_KEXEC_IMAGE,
> - READING_KEXEC_INITRAMFS,
> - READING_POLICY,
> - READING_MAX_ID
> + __kernel_read_file_id(__fid_enumify)
> +};
> +
> +static const char * const kernel_read_file_str[] = {
> + __kernel_read_file_id(__fid_stringify)
>  };
>  
> +static inline const char * const kernel_read_file_id_str(enum
> kernel_read_file_id id)
> +{
> + if (id < 0 || id >= READING_MAX_ID)
> + return kernel_read_file_str[READING_UNKNOWN];
> +
> + return kernel_read_file_str[id];
> +}
> +
>  extern int kernel_read(struct file *, loff_t, char *, unsigned long);
>  extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
>       enum kernel_read_file_id);

--
Andy Shevchenko <[email protected]>
Intel Finland Oy

2016-04-21 16:47:51

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 5/6] fs: define a string representation of the kernel_read_file_id enumeration

On Thu, Apr 21, 2016 at 6:26 AM, Andy Shevchenko
<[email protected]> wrote:
> On Wed, 2016-04-20 at 15:46 -0700, Kees Cook wrote:
>> From: Mimi Zohar <[email protected]>
>>
>> A string representation of the kernel_read_file_id enumeration is
>> needed for displaying messages (eg. pr_info, auditing) that can be
>> used by multiple LSMs and the integrity subsystem. To simplify
>> keeping the list of strings up to date with the enumeration, this
>> patch defines two new preprocessing macros named __fid_enumify and
>> __fid_stringify to create the enumeration and an array of strings.
>> kernel_read_file_id_str() returns a string based on the enumeration.
>>
>> Signed-off-by: Mimi Zohar <[email protected]>
>> [kees: removed removal of my old version, constified pointer values]
>> Signed-off-by: Kees Cook <[email protected]>
>
>
> I got this
>
> In file included from /home/andy/prj/linux-
> topic/include/linux/seq_file.h:10:0,
> from /home/andy/prj/linux-
> topic/include/linux/pinctrl/consumer.h:17,
> from /home/andy/prj/linux-
> topic/include/linux/pinctrl/devinfo.h:21,
> from /home/andy/prj/linux-
> topic/include/linux/device.h:24,
> from /home/andy/prj/linux-
> topic/include/linux/dmaengine.h:20,
> from /home/andy/prj/linux-
> topic/drivers/dma/dw/core.c:15:
> /home/andy/prj/linux-topic/include/linux/fs.h:2627:74: warning: type
> qualifiers ignored on function return type [-Wignored-qualifiers]
> static inline const char * const kernel_read_file_id_str(enum
> kernel_read_file_id id)

What gcc version are you using? Seems like I need to drop the added
"const" on the return?

-Kees

>
>> ---
>> include/linux/fs.h | 31 +++++++++++++++++++++++++------
>> 1 file changed, 25 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 70e61b58baaf..518716b4834e 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -2590,15 +2590,34 @@ static inline void i_readcount_inc(struct
>> inode *inode)
>> #endif
>> extern int do_pipe_flags(int *, int);
>>
>> +#define __kernel_read_file_id(id) \
>> + id(UNKNOWN, unknown) \
>> + id(FIRMWARE, firmware) \
>> + id(MODULE, kernel-module) \
>> + id(KEXEC_IMAGE, kexec-image) \
>> + id(KEXEC_INITRAMFS, kexec-initramfs) \
>> + id(POLICY, security-policy) \
>> + id(MAX_ID, )
>> +
>> +#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
>> +#define __fid_stringify(dummy, str) #str,
>> +
>> enum kernel_read_file_id {
>> - READING_FIRMWARE = 1,
>> - READING_MODULE,
>> - READING_KEXEC_IMAGE,
>> - READING_KEXEC_INITRAMFS,
>> - READING_POLICY,
>> - READING_MAX_ID
>> + __kernel_read_file_id(__fid_enumify)
>> +};
>> +
>> +static const char * const kernel_read_file_str[] = {
>> + __kernel_read_file_id(__fid_stringify)
>> };
>>
>> +static inline const char * const kernel_read_file_id_str(enum
>> kernel_read_file_id id)
>> +{
>> + if (id < 0 || id >= READING_MAX_ID)
>> + return kernel_read_file_str[READING_UNKNOWN];
>> +
>> + return kernel_read_file_str[id];
>> +}
>> +
>> extern int kernel_read(struct file *, loff_t, char *, unsigned long);
>> extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
>> enum kernel_read_file_id);
>
> --
> Andy Shevchenko <[email protected]>
> Intel Finland Oy
>



--
Kees Cook
Chrome OS & Brillo Security

2016-04-21 17:12:48

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 5/6] fs: define a string representation of the kernel_read_file_id enumeration

On Thu, 2016-04-21 at 09:47 -0700, Kees Cook wrote:
> On Thu, Apr 21, 2016 at 6:26 AM, Andy Shevchenko
> <[email protected]> wrote:
> >
> > On Wed, 2016-04-20 at 15:46 -0700, Kees Cook wrote:
> > >
> > > From: Mimi Zohar <[email protected]>
> > >
> > > A string representation of the kernel_read_file_id enumeration is
> > > needed for displaying messages (eg. pr_info, auditing) that can be
> > > used by multiple LSMs and the integrity subsystem.  To simplify
> > > keeping the list of strings up to date with the enumeration, this
> > > patch defines two new preprocessing macros named __fid_enumify and
> > > __fid_stringify to create the enumeration and an array of strings.
> > > kernel_read_file_id_str() returns a string based on the
> > > enumeration.


> > I got this
> >
> > In file included from /home/andy/prj/linux-
> > topic/include/linux/seq_file.h:10:0,
> >                  from /home/andy/prj/linux-
> > topic/include/linux/pinctrl/consumer.h:17,
> >                  from /home/andy/prj/linux-
> > topic/include/linux/pinctrl/devinfo.h:21,
> >                  from /home/andy/prj/linux-
> > topic/include/linux/device.h:24,
> >                  from /home/andy/prj/linux-
> > topic/include/linux/dmaengine.h:20,
> >                  from /home/andy/prj/linux-
> > topic/drivers/dma/dw/core.c:15:
> > /home/andy/prj/linux-topic/include/linux/fs.h:2627:74: warning: type
> > qualifiers ignored on function return type [-Wignored-qualifiers]
> >  static inline const char * const kernel_read_file_id_str(enum
> > kernel_read_file_id id)
> What gcc version are you using? Seems like I need to drop the added
> "const" on the return?


$ gcc --version
gcc (Debian 5.3.1-14) 5.3.1 20160409


Build was done with 
$ make W=1


--
Andy Shevchenko <[email protected]>
Intel Finland Oy