* [PATCH v4 1/6] string_helpers: add kstrdup_quotable
2016-04-12 16:54 [PATCH v4 0/6] LSM: LoadPin for kernel file loading restrictions Kees Cook
@ 2016-04-12 16:54 ` Kees Cook
2016-04-12 21:13 ` Serge E. Hallyn
2016-04-12 16:54 ` [PATCH v4 2/6] string_helpers: add kstrdup_quotable_cmdline Kees Cook
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Kees Cook @ 2016-04-12 16:54 UTC (permalink / raw)
To: James Morris
Cc: Kees Cook, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Handle allocating and escaping a string safe for logging.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 1/6] string_helpers: add kstrdup_quotable
2016-04-12 16:54 ` [PATCH v4 1/6] string_helpers: add kstrdup_quotable Kees Cook
@ 2016-04-12 21:13 ` Serge E. Hallyn
0 siblings, 0 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2016-04-12 21:13 UTC (permalink / raw)
To: Kees Cook
Cc: James Morris, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Quoting Kees Cook (keescook@chromium.org):
> Handle allocating and escaping a string safe for logging.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 2/6] string_helpers: add kstrdup_quotable_cmdline
2016-04-12 16:54 [PATCH v4 0/6] LSM: LoadPin for kernel file loading restrictions Kees Cook
2016-04-12 16:54 ` [PATCH v4 1/6] string_helpers: add kstrdup_quotable Kees Cook
@ 2016-04-12 16:54 ` Kees Cook
2016-04-12 21:19 ` Serge E. Hallyn
2016-04-12 16:54 ` [PATCH v4 3/6] string_helpers: add kstrdup_quotable_file Kees Cook
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Kees Cook @ 2016-04-12 16:54 UTC (permalink / raw)
To: James Morris
Cc: Kees Cook, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Provide an escaped (but readable: no inter-argument NULLs) commandline
safe for logging.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 2/6] string_helpers: add kstrdup_quotable_cmdline
2016-04-12 16:54 ` [PATCH v4 2/6] string_helpers: add kstrdup_quotable_cmdline Kees Cook
@ 2016-04-12 21:19 ` Serge E. Hallyn
2016-04-13 11:53 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Serge E. Hallyn @ 2016-04-12 21:19 UTC (permalink / raw)
To: Kees Cook
Cc: James Morris, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Quoting Kees Cook (keescook@chromium.org):
> Provide an escaped (but readable: no inter-argument NULLs) commandline
> safe for logging.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 2/6] string_helpers: add kstrdup_quotable_cmdline
2016-04-12 21:19 ` Serge E. Hallyn
@ 2016-04-13 11:53 ` Andy Shevchenko
0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2016-04-13 11:53 UTC (permalink / raw)
To: Serge E. Hallyn, Kees Cook
Cc: James Morris, Joe Perches, Mimi Zohar, Andrew Morton,
Jonathan Corbet, Kalle Valo, Mauro Carvalho Chehab, Guenter Roeck,
Jiri Slaby, Paul Moore, Stephen Smalley, Casey Schaufler,
Andreas Gruenbacher, Rasmus Villemoes, Ulf Hansson,
Vitaly Kuznetsov, linux-security-module, linux-kernel, linux-doc
On Tue, 2016-04-12 at 16:19 -0500, Serge E. Hallyn wrote:
> Quoting Kees Cook (keescook@chromium.org):
> >
> > Provide an escaped (but readable: no inter-argument NULLs)
> > commandline
> > safe for logging.
> >
Sorry, have no access to the original mail right now.
>> +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')
> > + ;
Nitpick: perhaps leave comment that make more visible
/* nothing */ ;
(up to you)?
> > +
> > + /* Replace inter-argument NULLs. */
> > + for (i = 0; i <= res; i++)
But why do you need to check = res? It's already checked by previous
condition and undoubtfully it's non-'\0'.
Forgot to mention this earlier.
> > + if (buffer[i] == '\0')
> > + buffer[i] = ' ';
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 3/6] string_helpers: add kstrdup_quotable_file
2016-04-12 16:54 [PATCH v4 0/6] LSM: LoadPin for kernel file loading restrictions Kees Cook
2016-04-12 16:54 ` [PATCH v4 1/6] string_helpers: add kstrdup_quotable Kees Cook
2016-04-12 16:54 ` [PATCH v4 2/6] string_helpers: add kstrdup_quotable_cmdline Kees Cook
@ 2016-04-12 16:54 ` Kees Cook
2016-04-12 21:24 ` Serge E. Hallyn
2016-04-12 16:54 ` [PATCH v4 4/6] Yama: consolidate error reporting Kees Cook
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Kees Cook @ 2016-04-12 16:54 UTC (permalink / raw)
To: James Morris
Cc: Kees Cook, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Allocate a NULL-terminated file path with special characters escaped,
safe for logging.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 3/6] string_helpers: add kstrdup_quotable_file
2016-04-12 16:54 ` [PATCH v4 3/6] string_helpers: add kstrdup_quotable_file Kees Cook
@ 2016-04-12 21:24 ` Serge E. Hallyn
0 siblings, 0 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2016-04-12 21:24 UTC (permalink / raw)
To: Kees Cook
Cc: James Morris, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Quoting Kees Cook (keescook@chromium.org):
> Allocate a NULL-terminated file path with special characters escaped,
> safe for logging.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 4/6] Yama: consolidate error reporting
2016-04-12 16:54 [PATCH v4 0/6] LSM: LoadPin for kernel file loading restrictions Kees Cook
` (2 preceding siblings ...)
2016-04-12 16:54 ` [PATCH v4 3/6] string_helpers: add kstrdup_quotable_file Kees Cook
@ 2016-04-12 16:54 ` Kees Cook
2016-04-12 21:26 ` Serge E. Hallyn
2016-04-12 16:54 ` [PATCH v4 5/6] fs: provide function to report enum strings Kees Cook
2016-04-12 16:54 ` [PATCH v4 6/6] LSM: LoadPin for kernel file loading restrictions Kees Cook
5 siblings, 1 reply; 17+ messages in thread
From: Kees Cook @ 2016-04-12 16:54 UTC (permalink / raw)
To: James Morris
Cc: Kees Cook, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Use a common error reporting function for Yama violation reports, and give
more detail into the process command lines.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 4/6] Yama: consolidate error reporting
2016-04-12 16:54 ` [PATCH v4 4/6] Yama: consolidate error reporting Kees Cook
@ 2016-04-12 21:26 ` Serge E. Hallyn
0 siblings, 0 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2016-04-12 21:26 UTC (permalink / raw)
To: Kees Cook
Cc: James Morris, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Quoting Kees Cook (keescook@chromium.org):
> Use a common error reporting function for Yama violation reports, and give
> more detail into the process command lines.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 5/6] fs: provide function to report enum strings
2016-04-12 16:54 [PATCH v4 0/6] LSM: LoadPin for kernel file loading restrictions Kees Cook
` (3 preceding siblings ...)
2016-04-12 16:54 ` [PATCH v4 4/6] Yama: consolidate error reporting Kees Cook
@ 2016-04-12 16:54 ` Kees Cook
2016-04-12 21:30 ` Serge E. Hallyn
2016-04-12 22:31 ` Al Viro
2016-04-12 16:54 ` [PATCH v4 6/6] LSM: LoadPin for kernel file loading restrictions Kees Cook
5 siblings, 2 replies; 17+ messages in thread
From: Kees Cook @ 2016-04-12 16:54 UTC (permalink / raw)
To: James Morris
Cc: Kees Cook, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Providing human-readable (and audit-parsable) strings for the READING_*
enums is needed by some LSMs.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
fs/exec.c | 19 +++++++++++++++++++
include/linux/fs.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/fs/exec.c b/fs/exec.c
index c4010b8207a1..05e71b6c0ef0 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -819,6 +819,25 @@ struct file *open_exec(const char *name)
}
EXPORT_SYMBOL(open_exec);
+const char *kernel_read_file_id_str(enum kernel_read_file_id id)
+{
+ switch (id) {
+ case READING_FIRMWARE:
+ return "firmware";
+ case READING_MODULE:
+ return "kernel-module";
+ case READING_KEXEC_IMAGE:
+ return "kexec-image";
+ case READING_KEXEC_INITRAMFS:
+ return "kexec-initramfs";
+ case READING_POLICY:
+ return "security-policy";
+ default:
+ return "unknown";
+ }
+}
+EXPORT_SYMBOL(kernel_read_file_id_str);
+
int kernel_read(struct file *file, loff_t offset,
char *addr, unsigned long count)
{
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 304991a80e23..596b403d5a28 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2589,6 +2589,7 @@ enum kernel_read_file_id {
READING_MAX_ID
};
+extern const char *kernel_read_file_id_str(enum kernel_read_file_id 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 5/6] fs: provide function to report enum strings
2016-04-12 16:54 ` [PATCH v4 5/6] fs: provide function to report enum strings Kees Cook
@ 2016-04-12 21:30 ` Serge E. Hallyn
2016-04-12 22:31 ` Al Viro
1 sibling, 0 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2016-04-12 21:30 UTC (permalink / raw)
To: Kees Cook
Cc: James Morris, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Quoting Kees Cook (keescook@chromium.org):
> Providing human-readable (and audit-parsable) strings for the READING_*
> enums is needed by some LSMs.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> fs/exec.c | 19 +++++++++++++++++++
> include/linux/fs.h | 1 +
> 2 files changed, 20 insertions(+)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index c4010b8207a1..05e71b6c0ef0 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -819,6 +819,25 @@ struct file *open_exec(const char *name)
> }
> EXPORT_SYMBOL(open_exec);
>
> +const char *kernel_read_file_id_str(enum kernel_read_file_id id)
> +{
> + switch (id) {
> + case READING_FIRMWARE:
> + return "firmware";
> + case READING_MODULE:
> + return "kernel-module";
> + case READING_KEXEC_IMAGE:
> + return "kexec-image";
> + case READING_KEXEC_INITRAMFS:
> + return "kexec-initramfs";
> + case READING_POLICY:
> + return "security-policy";
> + default:
> + return "unknown";
> + }
> +}
> +EXPORT_SYMBOL(kernel_read_file_id_str);
> +
> int kernel_read(struct file *file, loff_t offset,
> char *addr, unsigned long count)
> {
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 304991a80e23..596b403d5a28 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2589,6 +2589,7 @@ enum kernel_read_file_id {
> READING_MAX_ID
> };
>
> +extern const char *kernel_read_file_id_str(enum kernel_read_file_id 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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 5/6] fs: provide function to report enum strings
2016-04-12 16:54 ` [PATCH v4 5/6] fs: provide function to report enum strings Kees Cook
2016-04-12 21:30 ` Serge E. Hallyn
@ 2016-04-12 22:31 ` Al Viro
2016-04-12 22:38 ` Kees Cook
1 sibling, 1 reply; 17+ messages in thread
From: Al Viro @ 2016-04-12 22:31 UTC (permalink / raw)
To: Kees Cook
Cc: James Morris, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
On Tue, Apr 12, 2016 at 09:54:44AM -0700, Kees Cook wrote:
> Providing human-readable (and audit-parsable) strings for the READING_*
> enums is needed by some LSMs.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> fs/exec.c | 19 +++++++++++++++++++
> include/linux/fs.h | 1 +
> 2 files changed, 20 insertions(+)
What the devil is that doing in fs/exec.c, of all places?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 5/6] fs: provide function to report enum strings
2016-04-12 22:31 ` Al Viro
@ 2016-04-12 22:38 ` Kees Cook
2016-04-13 11:53 ` Mimi Zohar
0 siblings, 1 reply; 17+ messages in thread
From: Kees Cook @ 2016-04-12 22:38 UTC (permalink / raw)
To: Al Viro
Cc: James Morris, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, LKML, linux-doc@vger.kernel.org
On Tue, Apr 12, 2016 at 3:31 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Tue, Apr 12, 2016 at 09:54:44AM -0700, Kees Cook wrote:
>> Providing human-readable (and audit-parsable) strings for the READING_*
>> enums is needed by some LSMs.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>> fs/exec.c | 19 +++++++++++++++++++
>> include/linux/fs.h | 1 +
>> 2 files changed, 20 insertions(+)
>
> What the devil is that doing in fs/exec.c, of all places?
Since that's where the kernel_read* functions that use the enum live,
it seemed like the right place to put the string function too. I'm
happy to move it where ever folks think it's best to live.
-Kees
--
Kees Cook
Chrome OS & Brillo Security
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 5/6] fs: provide function to report enum strings
2016-04-12 22:38 ` Kees Cook
@ 2016-04-13 11:53 ` Mimi Zohar
0 siblings, 0 replies; 17+ messages in thread
From: Mimi Zohar @ 2016-04-13 11:53 UTC (permalink / raw)
To: Kees Cook
Cc: Al Viro, James Morris, Joe Perches, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, LKML, linux-doc@vger.kernel.org
On Tue, 2016-04-12 at 15:38 -0700, Kees Cook wrote:
> On Tue, Apr 12, 2016 at 3:31 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > On Tue, Apr 12, 2016 at 09:54:44AM -0700, Kees Cook wrote:
> >> Providing human-readable (and audit-parsable) strings for the READING_*
> >> enums is needed by some LSMs.
> >>
> >> Signed-off-by: Kees Cook <keescook@chromium.org>
> >> ---
> >> fs/exec.c | 19 +++++++++++++++++++
> >> include/linux/fs.h | 1 +
> >> 2 files changed, 20 insertions(+)
> >
> > What the devil is that doing in fs/exec.c, of all places?
>
> Since that's where the kernel_read* functions that use the enum live,
> it seemed like the right place to put the string function too. I'm
> happy to move it where ever folks think it's best to live.
Al,
The problem is keeping the enum and corresponding string in sync. As
soon as the enum definition is separated from the string definition, it
will become a problem. I've tried to use _stringify() to initialize
both the enum and the string, but it is ugly. Perhaps someone else has
a better, prettier method.
Mimi
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 6/6] LSM: LoadPin for kernel file loading restrictions
2016-04-12 16:54 [PATCH v4 0/6] LSM: LoadPin for kernel file loading restrictions Kees Cook
` (4 preceding siblings ...)
2016-04-12 16:54 ` [PATCH v4 5/6] fs: provide function to report enum strings Kees Cook
@ 2016-04-12 16:54 ` Kees Cook
2016-04-12 21:44 ` Serge E. Hallyn
5 siblings, 1 reply; 17+ messages in thread
From: Kees Cook @ 2016-04-12 16:54 UTC (permalink / raw)
To: James Morris
Cc: Kees Cook, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
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 <keescook@chromium.org>
---
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 40eb1dbe2ae5..de4cf8e9247e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9964,6 +9964,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 <keescook@chromium.org>
+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 <keescook@chromium.org>
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 <keescook@chromium.org>
+ *
+ * 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 6/6] LSM: LoadPin for kernel file loading restrictions
2016-04-12 16:54 ` [PATCH v4 6/6] LSM: LoadPin for kernel file loading restrictions Kees Cook
@ 2016-04-12 21:44 ` Serge E. Hallyn
0 siblings, 0 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2016-04-12 21:44 UTC (permalink / raw)
To: Kees Cook
Cc: James Morris, Joe Perches, Mimi Zohar, Andy Shevchenko,
Andrew Morton, Serge E. Hallyn, Jonathan Corbet, Kalle Valo,
Mauro Carvalho Chehab, Guenter Roeck, Jiri Slaby, Paul Moore,
Stephen Smalley, Casey Schaufler, Andreas Gruenbacher,
Rasmus Villemoes, Ulf Hansson, Vitaly Kuznetsov,
linux-security-module, linux-kernel, linux-doc
Quoting Kees Cook (keescook@chromium.org):
> 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 <keescook@chromium.org>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> 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 40eb1dbe2ae5..de4cf8e9247e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9964,6 +9964,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 <keescook@chromium.org>
> +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 <keescook@chromium.org>
> 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 <keescook@chromium.org>
> + *
> + * 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
^ permalink raw reply [flat|nested] 17+ messages in thread