Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1296624 > unrolled thread

[PATCH V3 1/2] audit: stop an old auditd being starved out by a new auditd

Back to article view | Back to linux.kernel

Contents

  [PATCH V3 1/2] audit: stop an old auditd being starved out by a new auditd Richard Guy Briggs <rgb@redhat.com> - 2015-12-22 10:10 +0100
    [PATCH V3 2/2] audit: log failed attempts to change audit_pid configuration Richard Guy Briggs <rgb@redhat.com> - 2015-12-22 10:10 +0100
      Re: [PATCH V3 2/2] audit: log failed attempts to change audit_pid configuration Paul Moore <pmoore@redhat.com> - 2015-12-23 00:50 +0100
    Re: [PATCH V3 1/2] audit: stop an old auditd being starved out by a new auditd Paul Moore <pmoore@redhat.com> - 2015-12-22 15:30 +0100
      Re: [PATCH V3 1/2] audit: stop an old auditd being starved out by a new auditd Steve Grubb <sgrubb@redhat.com> - 2015-12-22 16:00 +0100
        Re: [PATCH V3 1/2] audit: stop an old auditd being starved out by a  new auditd Richard Guy Briggs <rgb@redhat.com> - 2015-12-22 16:50 +0100
    Re: [PATCH V3 1/2] audit: stop an old auditd being starved out by a new auditd Paul Moore <pmoore@redhat.com> - 2015-12-23 00:50 +0100

#1296624 — [PATCH V3 1/2] audit: stop an old auditd being starved out by a new auditd

Nothing prevents a new auditd starting up and replacing a valid
audit_pid when an old auditd is still running, effectively starving out
the old auditd since audit_pid no longer points to the old valid auditd.

If no message to auditd has been attempted since auditd died unnaturally
or got killed, audit_pid will still indicate it is alive.  There isn't
an easy way to detect if an old auditd is still running on the existing
audit_pid other than attempting to send a message to see if it fails.
An -ECONNREFUSED almost certainly means it disappeared and can be
replaced.  Other errors are not so straightforward and may indicate
transient problems that will resolve themselves and the old auditd will
recover.  Yet others will likely need manual intervention for which a
new auditd will not solve the problem.

Send a new message type (AUDIT_REPLACE) to the old auditd containing a
u32 with the PID of the new auditd.  If the audit replace message
succeeds (or doesn't fail with certainty), fail to register the new
auditd and return an error (-EEXIST).

This is expected to make the patch preventing an old auditd orphaning a
new auditd redundant.

V3: Switch audit message type from 1000 to 1300 block.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/uapi/linux/audit.h |    1 +
 kernel/audit.c             |   16 +++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 843540c..d820aa9 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -110,6 +110,7 @@
 #define AUDIT_SECCOMP		1326	/* Secure Computing event */
 #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
 #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes */
+#define AUDIT_REPLACE		1329	/* Replace auditd if this packet unanswerd */
 
 #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
 #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
diff --git a/kernel/audit.c b/kernel/audit.c
index 36989a1..0368be2 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -809,6 +809,16 @@ static int audit_set_feature(struct sk_buff *skb)
 	return 0;
 }
 
+static int audit_replace(pid_t pid)
+{
+	struct sk_buff *skb = audit_make_reply(0, 0, AUDIT_REPLACE, 0, 0,
+					       &pid, sizeof(pid));
+
+	if (!skb)
+		return -ENOMEM;
+	return netlink_unicast(audit_sock, skb, audit_nlk_portid, 0);
+}
+
 static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
 	u32			seq;
@@ -870,9 +880,13 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		}
 		if (s.mask & AUDIT_STATUS_PID) {
 			int new_pid = s.pid;
+			pid_t requesting_pid = task_tgid_vnr(current);
 
-			if ((!new_pid) && (task_tgid_vnr(current) != audit_pid))
+			if ((!new_pid) && (requesting_pid != audit_pid))
 				return -EACCES;
+			if (audit_pid && new_pid &&
+			    audit_replace(requesting_pid) != -ECONNREFUSED)
+				return -EEXIST;
 			if (audit_enabled != AUDIT_OFF)
 				audit_log_config_change("audit_pid", new_pid, audit_pid, 1);
 			audit_pid = new_pid;
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1296627 — [PATCH V3 2/2] audit: log failed attempts to change audit_pid configuration

Failed attempts to change the audit_pid configuration are not presently
logged.  One case is an attempt to starve an old auditd by starting up a
new auditd when the old one is still alive and active.  The other case
is an attempt to orphan a new auditd when an old auditd shuts down.

Log both as AUDIT_CONFIG_CHANGE messages with failure result.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 0368be2..9000c6f 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -882,11 +882,15 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 			int new_pid = s.pid;
 			pid_t requesting_pid = task_tgid_vnr(current);
 
-			if ((!new_pid) && (requesting_pid != audit_pid))
+			if ((!new_pid) && (requesting_pid != audit_pid)) {
+				audit_log_config_change("audit_pid", new_pid, audit_pid, 0);
 				return -EACCES;
+			}
 			if (audit_pid && new_pid &&
-			    audit_replace(requesting_pid) != -ECONNREFUSED)
+			    audit_replace(requesting_pid) != -ECONNREFUSED) {
+				audit_log_config_change("audit_pid", new_pid, audit_pid, 0);
 				return -EEXIST;
+			}
 			if (audit_enabled != AUDIT_OFF)
 				audit_log_config_change("audit_pid", new_pid, audit_pid, 1);
 			audit_pid = new_pid;
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1297146 — Re: [PATCH V3 2/2] audit: log failed attempts to change audit_pid configuration

On Tuesday, December 22, 2015 04:03:07 AM Richard Guy Briggs wrote:
> Failed attempts to change the audit_pid configuration are not presently
> logged.  One case is an attempt to starve an old auditd by starting up a
> new auditd when the old one is still alive and active.  The other case
> is an attempt to orphan a new auditd when an old auditd shuts down.
> 
> Log both as AUDIT_CONFIG_CHANGE messages with failure result.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)

Same as 1/2, applied to the audit next queue.

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 0368be2..9000c6f 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -882,11 +882,15 @@ static int audit_receive_msg(struct sk_buff *skb,
> struct nlmsghdr *nlh) int new_pid = s.pid;
>  			pid_t requesting_pid = task_tgid_vnr(current);
> 
> -			if ((!new_pid) && (requesting_pid != audit_pid))
> +			if ((!new_pid) && (requesting_pid != audit_pid)) {
> +				audit_log_config_change("audit_pid", new_pid, audit_pid, 0);
>  				return -EACCES;
> +			}
>  			if (audit_pid && new_pid &&
> -			    audit_replace(requesting_pid) != -ECONNREFUSED)
> +			    audit_replace(requesting_pid) != -ECONNREFUSED) {
> +				audit_log_config_change("audit_pid", new_pid, audit_pid, 0);
>  				return -EEXIST;
> +			}
>  			if (audit_enabled != AUDIT_OFF)
>  				audit_log_config_change("audit_pid", new_pid, audit_pid, 1);
>  			audit_pid = new_pid;

-- 
paul moore
security @ redhat

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296810

On Tuesday, December 22, 2015 04:03:06 AM Richard Guy Briggs wrote:
> Nothing prevents a new auditd starting up and replacing a valid
> audit_pid when an old auditd is still running, effectively starving out
> the old auditd since audit_pid no longer points to the old valid auditd.
> 
> If no message to auditd has been attempted since auditd died unnaturally
> or got killed, audit_pid will still indicate it is alive.  There isn't
> an easy way to detect if an old auditd is still running on the existing
> audit_pid other than attempting to send a message to see if it fails.
> An -ECONNREFUSED almost certainly means it disappeared and can be
> replaced.  Other errors are not so straightforward and may indicate
> transient problems that will resolve themselves and the old auditd will
> recover.  Yet others will likely need manual intervention for which a
> new auditd will not solve the problem.
> 
> Send a new message type (AUDIT_REPLACE) to the old auditd containing a
> u32 with the PID of the new auditd.  If the audit replace message
> succeeds (or doesn't fail with certainty), fail to register the new
> auditd and return an error (-EEXIST).
> 
> This is expected to make the patch preventing an old auditd orphaning a
> new auditd redundant.
> 
> V3: Switch audit message type from 1000 to 1300 block.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  include/uapi/linux/audit.h |    1 +
>  kernel/audit.c             |   16 +++++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletions(-)
> 
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 843540c..d820aa9 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -110,6 +110,7 @@
>  #define AUDIT_SECCOMP		1326	/* Secure Computing event */
>  #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
>  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes 
*/
> +#define AUDIT_REPLACE		1329	/* Replace auditd if this packet...

Steve, are you okay with this record number?

-- 
paul moore
security @ redhat

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296822

On Tuesday, December 22, 2015 09:24:56 AM Paul Moore wrote:
> On Tuesday, December 22, 2015 04:03:06 AM Richard Guy Briggs wrote:
> > Nothing prevents a new auditd starting up and replacing a valid
> > audit_pid when an old auditd is still running, effectively starving out
> > the old auditd since audit_pid no longer points to the old valid auditd.
> > 
> > If no message to auditd has been attempted since auditd died unnaturally
> > or got killed, audit_pid will still indicate it is alive.  There isn't
> > an easy way to detect if an old auditd is still running on the existing
> > audit_pid other than attempting to send a message to see if it fails.
> > An -ECONNREFUSED almost certainly means it disappeared and can be
> > replaced.  Other errors are not so straightforward and may indicate
> > transient problems that will resolve themselves and the old auditd will
> > recover.  Yet others will likely need manual intervention for which a
> > new auditd will not solve the problem.
> > 
> > Send a new message type (AUDIT_REPLACE) to the old auditd containing a
> > u32 with the PID of the new auditd.  If the audit replace message
> > succeeds (or doesn't fail with certainty), fail to register the new
> > auditd and return an error (-EEXIST).
> > 
> > This is expected to make the patch preventing an old auditd orphaning a
> > new auditd redundant.
> > 
> > V3: Switch audit message type from 1000 to 1300 block.
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > 
> >  include/uapi/linux/audit.h |    1 +
> >  kernel/audit.c             |   16 +++++++++++++++-
> >  2 files changed, 16 insertions(+), 1 deletions(-)
> > 
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 843540c..d820aa9 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -110,6 +110,7 @@
> > 
> >  #define AUDIT_SECCOMP		1326	/* Secure Computing event */
> >  #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
> >  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes
> 
> */
> 
> > +#define AUDIT_REPLACE		1329	/* Replace auditd if this packet...
> 
> Steve, are you okay with this record number?

Yes. Just wondering what to do with the event.

-Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296848 — Re: [PATCH V3 1/2] audit: stop an old auditd being starved out by a new auditd

On 15/12/22, Steve Grubb wrote:
> On Tuesday, December 22, 2015 09:24:56 AM Paul Moore wrote:
> > On Tuesday, December 22, 2015 04:03:06 AM Richard Guy Briggs wrote:
> > > Nothing prevents a new auditd starting up and replacing a valid
> > > audit_pid when an old auditd is still running, effectively starving out
> > > the old auditd since audit_pid no longer points to the old valid auditd.
> > > 
> > > If no message to auditd has been attempted since auditd died unnaturally
> > > or got killed, audit_pid will still indicate it is alive.  There isn't
> > > an easy way to detect if an old auditd is still running on the existing
> > > audit_pid other than attempting to send a message to see if it fails.
> > > An -ECONNREFUSED almost certainly means it disappeared and can be
> > > replaced.  Other errors are not so straightforward and may indicate
> > > transient problems that will resolve themselves and the old auditd will
> > > recover.  Yet others will likely need manual intervention for which a
> > > new auditd will not solve the problem.
> > > 
> > > Send a new message type (AUDIT_REPLACE) to the old auditd containing a
> > > u32 with the PID of the new auditd.  If the audit replace message
> > > succeeds (or doesn't fail with certainty), fail to register the new
> > > auditd and return an error (-EEXIST).
> > > 
> > > This is expected to make the patch preventing an old auditd orphaning a
> > > new auditd redundant.
> > > 
> > > V3: Switch audit message type from 1000 to 1300 block.
> > > 
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > > 
> > >  include/uapi/linux/audit.h |    1 +
> > >  kernel/audit.c             |   16 +++++++++++++++-
> > >  2 files changed, 16 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > index 843540c..d820aa9 100644
> > > --- a/include/uapi/linux/audit.h
> > > +++ b/include/uapi/linux/audit.h
> > > @@ -110,6 +110,7 @@
> > > 
> > >  #define AUDIT_SECCOMP		1326	/* Secure Computing event */
> > >  #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
> > >  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes
> > 
> > */
> > 
> > > +#define AUDIT_REPLACE		1329	/* Replace auditd if this packet...
> > 
> > Steve, are you okay with this record number?
> 
> Yes. Just wondering what to do with the event.

There's a u32 in it with the pid of the process trying to hijack/replace
the one the kernel thinks still exists, so it is worth recording that
information, I think, if the old auditd actually receives this message.

> -Steve

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1297150

On Tuesday, December 22, 2015 04:03:06 AM Richard Guy Briggs wrote:
> Nothing prevents a new auditd starting up and replacing a valid
> audit_pid when an old auditd is still running, effectively starving out
> the old auditd since audit_pid no longer points to the old valid auditd.
> 
> If no message to auditd has been attempted since auditd died unnaturally
> or got killed, audit_pid will still indicate it is alive.  There isn't
> an easy way to detect if an old auditd is still running on the existing
> audit_pid other than attempting to send a message to see if it fails.
> An -ECONNREFUSED almost certainly means it disappeared and can be
> replaced.  Other errors are not so straightforward and may indicate
> transient problems that will resolve themselves and the old auditd will
> recover.  Yet others will likely need manual intervention for which a
> new auditd will not solve the problem.
> 
> Send a new message type (AUDIT_REPLACE) to the old auditd containing a
> u32 with the PID of the new auditd.  If the audit replace message
> succeeds (or doesn't fail with certainty), fail to register the new
> auditd and return an error (-EEXIST).
> 
> This is expected to make the patch preventing an old auditd orphaning a
> new auditd redundant.
> 
> V3: Switch audit message type from 1000 to 1300 block.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  include/uapi/linux/audit.h |    1 +
>  kernel/audit.c             |   16 +++++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletions(-)

Applied to my audit next queue for after the merge window, thanks.

> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 843540c..d820aa9 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -110,6 +110,7 @@
>  #define AUDIT_SECCOMP		1326	/* Secure Computing event */
>  #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
>  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes 
*/
> +#define AUDIT_REPLACE		1329	/* Replace auditd if this packet 
unanswerd */
> 
>  #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
>  #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 36989a1..0368be2 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -809,6 +809,16 @@ static int audit_set_feature(struct sk_buff *skb)
>  	return 0;
>  }
> 
> +static int audit_replace(pid_t pid)
> +{
> +	struct sk_buff *skb = audit_make_reply(0, 0, AUDIT_REPLACE, 0, 0,
> +					       &pid, sizeof(pid));
> +
> +	if (!skb)
> +		return -ENOMEM;
> +	return netlink_unicast(audit_sock, skb, audit_nlk_portid, 0);
> +}
> +
>  static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>  {
>  	u32			seq;
> @@ -870,9 +880,13 @@ static int audit_receive_msg(struct sk_buff *skb,
> struct nlmsghdr *nlh) }
>  		if (s.mask & AUDIT_STATUS_PID) {
>  			int new_pid = s.pid;
> +			pid_t requesting_pid = task_tgid_vnr(current);
> 
> -			if ((!new_pid) && (task_tgid_vnr(current) != audit_pid))
> +			if ((!new_pid) && (requesting_pid != audit_pid))
>  				return -EACCES;
> +			if (audit_pid && new_pid &&
> +			    audit_replace(requesting_pid) != -ECONNREFUSED)
> +				return -EEXIST;
>  			if (audit_enabled != AUDIT_OFF)
>  				audit_log_config_change("audit_pid", new_pid, audit_pid, 1);
>  			audit_pid = new_pid;

-- 
paul moore
security @ redhat

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web