[RFC PATCH 0/5] Fix some early boot audit problems

[RFC PATCH 0/5] Fix some early boot audit problems

[Paul Moore]

Unfortunately it turns out that we are not properly enabling audit
early enough in the boot process to tag PID 1 (init/systemd/etc.)
with the special audit magic necessary to cause PID 1 events to
be audited.  This patch set fixes this problem (look at patch 1/5,
that should be the only fix that is strictly necessary) and makes
a few other improvements to make the early enable/initializaton
code a bit more robust.

---

Paul Moore (5):
      audit: ensure that 'audit=1' actually enables audit for PID 1
      audit: initialize the audit subsystem as early as possible
      audit: don't use simple_strtol() anymore
      audit: convert audit_ever_enabled to a boolean
      audit: use audit_set_enabled() in audit_enable()


 kernel/audit.c |   21 +++++++++++++--------
 kernel/audit.h |    2 +-
 2 files changed, 14 insertions(+), 9 deletions(-)

[RFC PATCH 1/5] audit: ensure that 'audit=1' actually enables audit for PID 1

[Paul Moore]

From: Paul Moore <paul@paul-moore.com>

Prior to this patch we enabled audit in audit_init(), which is too
late for PID 1 as the standard initcalls are run after the PID 1 task
is forked.  This means that we never allocate an audit_context (see
audit_alloc()) for PID 1 and therefore miss a lot of audit events
generated by PID 1.

This patch enables audit as early as possible to help ensure that when
PID 1 is forked it can allocate an audit_context if required.

Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 kernel/audit.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index cb744085ea8d..33b00ec2157f 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -85,13 +85,13 @@ static int	audit_initialized;
 #define AUDIT_OFF	0
 #define AUDIT_ON	1
 #define AUDIT_LOCKED	2
-u32		audit_enabled;
-u32		audit_ever_enabled;
+u32		audit_enabled = AUDIT_OFF;
+u32		audit_ever_enabled = !!AUDIT_OFF;
 
 EXPORT_SYMBOL_GPL(audit_enabled);
 
 /* Default state when kernel boots without any parameters. */
-static u32	audit_default;
+static u32	audit_default = AUDIT_OFF;
 
 /* If auditing cannot proceed, audit_failure selects what happens. */
 static u32	audit_failure = AUDIT_FAIL_PRINTK;
@@ -1548,8 +1548,6 @@ static int __init audit_init(void)
 	register_pernet_subsys(&audit_net_ops);
 
 	audit_initialized = AUDIT_INITIALIZED;
-	audit_enabled = audit_default;
-	audit_ever_enabled |= !!audit_default;
 
 	kauditd_task = kthread_run(kauditd_thread, NULL, "kauditd");
 	if (IS_ERR(kauditd_task)) {
@@ -1571,6 +1569,8 @@ static int __init audit_enable(char *str)
 	audit_default = !!simple_strtol(str, NULL, 0);
 	if (!audit_default)
 		audit_initialized = AUDIT_DISABLED;
+	audit_enabled = audit_default;
+	audit_ever_enabled = !!audit_enabled;
 
 	pr_info("%s\n", audit_default ?
 		"enabled (after initialization)" : "disabled (until reboot)");

[RFC PATCH 2/5] audit: initialize the audit subsystem as early as possible

[Paul Moore]

From: Paul Moore <paul@paul-moore.com>

We can't initialize the audit subsystem until after the network layer
is initialized (core_initcall), but do it soon after.

Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 kernel/audit.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 33b00ec2157f..de8a9b8465ae 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1561,7 +1561,7 @@ static int __init audit_init(void)
 
 	return 0;
 }
-__initcall(audit_init);
+postcore_initcall(audit_init);
 
 /* Process kernel command-line parameter at boot time.  audit=0 or audit=1. */
 static int __init audit_enable(char *str)

[RFC PATCH 3/5] audit: don't use simple_strtol() anymore

[Paul Moore]

From: Paul Moore <paul@paul-moore.com>

The simple_strtol() function is deprecated, use kstrtol() instead.

Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 kernel/audit.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index de8a9b8465ae..9df2ef4d3e53 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1566,8 +1566,13 @@ postcore_initcall(audit_init);
 /* Process kernel command-line parameter at boot time.  audit=0 or audit=1. */
 static int __init audit_enable(char *str)
 {
-	audit_default = !!simple_strtol(str, NULL, 0);
-	if (!audit_default)
+	long val;
+
+	if (kstrtol(str, 0, &val))
+		panic("audit: invalid 'audit' parameter value (%s)\n", str);
+	audit_default = (val ? AUDIT_ON : AUDIT_OFF);
+
+	if (audit_default == AUDIT_OFF)
 		audit_initialized = AUDIT_DISABLED;
 	audit_enabled = audit_default;
 	audit_ever_enabled = !!audit_enabled;

[RFC PATCH 4/5] audit: convert audit_ever_enabled to a boolean

[Paul Moore]

From: Paul Moore <paul@paul-moore.com>

We were treating it as a boolean, let's make it a boolean to help
avoid future mistakes.

Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 kernel/audit.c |    2 +-
 kernel/audit.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 9df2ef4d3e53..01bf1e479a8c 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -86,7 +86,7 @@ static int	audit_initialized;
 #define AUDIT_ON	1
 #define AUDIT_LOCKED	2
 u32		audit_enabled = AUDIT_OFF;
-u32		audit_ever_enabled = !!AUDIT_OFF;
+bool		audit_ever_enabled = !!AUDIT_OFF;
 
 EXPORT_SYMBOL_GPL(audit_enabled);
 
diff --git a/kernel/audit.h b/kernel/audit.h
index b331d9b83f63..6bdaf6bd377e 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -208,7 +208,7 @@ struct audit_context {
 	struct audit_proctitle proctitle;
 };
 
-extern u32 audit_ever_enabled;
+extern bool audit_ever_enabled;
 
 extern void audit_copy_inode(struct audit_names *name,
 			     const struct dentry *dentry,

[RFC PATCH 5/5] audit: use audit_set_enabled() in audit_enable()

[Paul Moore]

From: Paul Moore <paul@paul-moore.com>

Use audit_set_enabled() to enable auditing during early boot.  This
obviously won't emit an audit change record, but it will work anyway
and should help prevent in future problems by consolidating the
enable/disable code in one function.

Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 kernel/audit.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 01bf1e479a8c..842237f5182b 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1574,8 +1574,8 @@ static int __init audit_enable(char *str)
 
 	if (audit_default == AUDIT_OFF)
 		audit_initialized = AUDIT_DISABLED;
-	audit_enabled = audit_default;
-	audit_ever_enabled = !!audit_enabled;
+	if (audit_set_enabled(audit_default))
+		panic("audit: error setting audit state (%d)\n", audit_default);
 
 	pr_info("%s\n", audit_default ?
 		"enabled (after initialization)" : "disabled (until reboot)");

Re: [RFC PATCH 1/5] audit: ensure that 'audit=1' actually enables audit for PID 1

[Richard Guy Briggs]

On 2017-09-01 09:44, Paul Moore wrote:
> From: Paul Moore <paul@paul-moore.com>
> 
> Prior to this patch we enabled audit in audit_init(), which is too
> late for PID 1 as the standard initcalls are run after the PID 1 task
> is forked.  This means that we never allocate an audit_context (see
> audit_alloc()) for PID 1 and therefore miss a lot of audit events
> generated by PID 1.
> 
> This patch enables audit as early as possible to help ensure that when
> PID 1 is forked it can allocate an audit_context if required.

Ok, since I was certain this was working properly at some point, I
started to dig to find out why.  It appears this patch restores previous
behaviour and that this wasn't all useless code that was removed in this
previous commit:
	d3ca0344b21f04786219bf0f49647f24e4e17323 gaofeng 2013-10-31 
	("audit: remove useless code in audit_enable")

Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  kernel/audit.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index cb744085ea8d..33b00ec2157f 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -85,13 +85,13 @@ static int	audit_initialized;
>  #define AUDIT_OFF	0
>  #define AUDIT_ON	1
>  #define AUDIT_LOCKED	2
> -u32		audit_enabled;
> -u32		audit_ever_enabled;
> +u32		audit_enabled = AUDIT_OFF;
> +u32		audit_ever_enabled = !!AUDIT_OFF;
>  
>  EXPORT_SYMBOL_GPL(audit_enabled);
>  
>  /* Default state when kernel boots without any parameters. */
> -static u32	audit_default;
> +static u32	audit_default = AUDIT_OFF;
>  
>  /* If auditing cannot proceed, audit_failure selects what happens. */
>  static u32	audit_failure = AUDIT_FAIL_PRINTK;
> @@ -1548,8 +1548,6 @@ static int __init audit_init(void)
>  	register_pernet_subsys(&audit_net_ops);
>  
>  	audit_initialized = AUDIT_INITIALIZED;
> -	audit_enabled = audit_default;
> -	audit_ever_enabled |= !!audit_default;
>  
>  	kauditd_task = kthread_run(kauditd_thread, NULL, "kauditd");
>  	if (IS_ERR(kauditd_task)) {
> @@ -1571,6 +1569,8 @@ static int __init audit_enable(char *str)
>  	audit_default = !!simple_strtol(str, NULL, 0);
>  	if (!audit_default)
>  		audit_initialized = AUDIT_DISABLED;
> +	audit_enabled = audit_default;
> +	audit_ever_enabled = !!audit_enabled;
>  
>  	pr_info("%s\n", audit_default ?
>  		"enabled (after initialization)" : "disabled (until reboot)");
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH 2/5] audit: initialize the audit subsystem as early as possible

[Richard Guy Briggs]

On 2017-09-01 09:44, Paul Moore wrote:
> From: Paul Moore <paul@paul-moore.com>
> 
> We can't initialize the audit subsystem until after the network layer
> is initialized (core_initcall), but do it soon after.

I had run into a similar problem when trying to log the creation of
initial namespaces and had to move this around in V4 to V7 of the
namespace ID and namespace serial number patchsets:

2014-08-20 21:09 To linux-audit@redha (  26) ├─>[PATCH V4 8/8] audit: initialize at subsystem time rather than device time
https://www.redhat.com/archives/linux-audit/2014-August/msg00044.html
2014-10-06 01:08 To linux-audit@redha (3.0K) ├─>[PATCH V5 05/13] audit: initialize at subsystem time rather than device time
https://www.redhat.com/archives/linux-audit/2014-October/msg00035.html
2015-04-17 03:35 To containers@lists. (3.0K) ├─>[PATCH V6 04/10] audit: initialize at subsystem time rather than device time
https://www.redhat.com/archives/linux-audit/2015-April/msg00018.html
2015-05-12 16:02 To linux-audit@redha (3.0K) ├─>[PATCH V7 05/10] audit: initialize at subsystem time rather than device time
https://www.redhat.com/archives/linux-audit/2015-May/msg00023.html

> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

> ---
>  kernel/audit.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 33b00ec2157f..de8a9b8465ae 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1561,7 +1561,7 @@ static int __init audit_init(void)
>  
>  	return 0;
>  }
> -__initcall(audit_init);
> +postcore_initcall(audit_init);
>  
>  /* Process kernel command-line parameter at boot time.  audit=0 or audit=1. */
>  static int __init audit_enable(char *str)
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Re: [RFC PATCH 4/5] audit: convert audit_ever_enabled to a boolean

[Richard Guy Briggs]

On 2017-09-01 09:44, Paul Moore wrote:
> From: Paul Moore <paul@paul-moore.com>
> 
> We were treating it as a boolean, let's make it a boolean to help
> avoid future mistakes.
> 
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

> ---
>  kernel/audit.c |    2 +-
>  kernel/audit.h |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 9df2ef4d3e53..01bf1e479a8c 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -86,7 +86,7 @@ static int	audit_initialized;
>  #define AUDIT_ON	1
>  #define AUDIT_LOCKED	2
>  u32		audit_enabled = AUDIT_OFF;
> -u32		audit_ever_enabled = !!AUDIT_OFF;
> +bool		audit_ever_enabled = !!AUDIT_OFF;
>  
>  EXPORT_SYMBOL_GPL(audit_enabled);
>  
> diff --git a/kernel/audit.h b/kernel/audit.h
> index b331d9b83f63..6bdaf6bd377e 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -208,7 +208,7 @@ struct audit_context {
>  	struct audit_proctitle proctitle;
>  };
>  
> -extern u32 audit_ever_enabled;
> +extern bool audit_ever_enabled;
>  
>  extern void audit_copy_inode(struct audit_names *name,
>  			     const struct dentry *dentry,
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH 3/5] audit: don't use simple_strtol() anymore

[Richard Guy Briggs]

On 2017-09-01 09:44, Paul Moore wrote:
> From: Paul Moore <paul@paul-moore.com>
> 
> The simple_strtol() function is deprecated, use kstrtol() instead.
> 
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

> ---
>  kernel/audit.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index de8a9b8465ae..9df2ef4d3e53 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1566,8 +1566,13 @@ postcore_initcall(audit_init);
>  /* Process kernel command-line parameter at boot time.  audit=0 or audit=1. */
>  static int __init audit_enable(char *str)
>  {
> -	audit_default = !!simple_strtol(str, NULL, 0);
> -	if (!audit_default)
> +	long val;
> +
> +	if (kstrtol(str, 0, &val))
> +		panic("audit: invalid 'audit' parameter value (%s)\n", str);
> +	audit_default = (val ? AUDIT_ON : AUDIT_OFF);
> +
> +	if (audit_default == AUDIT_OFF)
>  		audit_initialized = AUDIT_DISABLED;
>  	audit_enabled = audit_default;
>  	audit_ever_enabled = !!audit_enabled;
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH 5/5] audit: use audit_set_enabled() in audit_enable()

[Richard Guy Briggs]

On 2017-09-01 09:45, Paul Moore wrote:
> From: Paul Moore <paul@paul-moore.com>
> 
> Use audit_set_enabled() to enable auditing during early boot.  This
> obviously won't emit an audit change record, but it will work anyway
> and should help prevent in future problems by consolidating the
> enable/disable code in one function.
> 
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

> ---
>  kernel/audit.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 01bf1e479a8c..842237f5182b 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1574,8 +1574,8 @@ static int __init audit_enable(char *str)
>  
>  	if (audit_default == AUDIT_OFF)
>  		audit_initialized = AUDIT_DISABLED;
> -	audit_enabled = audit_default;
> -	audit_ever_enabled = !!audit_enabled;
> +	if (audit_set_enabled(audit_default))
> +		panic("audit: error setting audit state (%d)\n", audit_default);
>  
>  	pr_info("%s\n", audit_default ?
>  		"enabled (after initialization)" : "disabled (until reboot)");
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH 0/5] Fix some early boot audit problems

[Paul Moore]

On Fri, Sep 1, 2017 at 9:44 AM, Paul Moore <paul@paul-moore.com> wrote:
> Unfortunately it turns out that we are not properly enabling audit
> early enough in the boot process to tag PID 1 (init/systemd/etc.)
> with the special audit magic necessary to cause PID 1 events to
> be audited.  This patch set fixes this problem (look at patch 1/5,
> that should be the only fix that is strictly necessary) and makes
> a few other improvements to make the early enable/initializaton
> code a bit more robust.
>
> ---
>
> Paul Moore (5):
>       audit: ensure that 'audit=1' actually enables audit for PID 1
>       audit: initialize the audit subsystem as early as possible
>       audit: don't use simple_strtol() anymore
>       audit: convert audit_ever_enabled to a boolean
>       audit: use audit_set_enabled() in audit_enable()
>
>
>  kernel/audit.c |   21 +++++++++++++--------
>  kernel/audit.h |    2 +-
>  2 files changed, 14 insertions(+), 9 deletions(-)

FYI, I just merged all five patches into audit/next.

-- 
paul moore
www.paul-moore.com