[PATCH audit] reconfigure: Apply log_group change

[PATCH audit] reconfigure: Apply log_group change

[Lubomir Rintel]

Link: https://bugzilla.redhat.com/show_bug.cgi?id=1118313
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1118262
---
 src/auditd-event.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/auditd-event.c b/src/auditd-event.c
index 4fa266e..66dff34 100644
--- a/src/auditd-event.c
+++ b/src/auditd-event.c
@@ -1130,6 +1130,12 @@ static void reconfigure(struct auditd_consumer_data *data)
 	// log format
 	oconf->log_format = nconf->log_format;
 
+	// log group
+	if (oconf->log_group != nconf->log_group) {
+		oconf->log_group = nconf->log_group;
+		need_reopen = 1;
+	}
+
 	// action_mail_acct
 	if (strcmp(oconf->action_mail_acct, nconf->action_mail_acct)) {
 		free((void *)oconf->action_mail_acct);
-- 
1.8.3.1

Re: [PATCH audit] reconfigure: Apply log_group change

[Steve Grubb]

Hello Lubomir,

Thanks for the patch...but I think that why this is not currently addressed in 
the code is something like this. Let's assume the system has 5 audit logs with 
600 root.

If an admin decides to allow a group to read the audit logs, they will have 
to:

1) create the group
2) add users to the group
3) change the auditd.conf file
4) chgrp -R group /var/log/audit
5) chmod 0750 /var/log/audit
6) chmod 0640 /var/log/audit/*
7) restart the audit daemon

What this patch does is part of step 4 and 6. It would change audit.log to be 
readable, but would leave audit.log.1 -> audit.log.4 untouched. Because 
allowing a group requires so many steps, it's always been left as an admin 
exercise...just like revoking group access would.

-Steve


On Friday, July 25, 2014 01:59:04 PM Lubomir Rintel wrote:
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1118313
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1118262
> ---
>  src/auditd-event.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/auditd-event.c b/src/auditd-event.c
> index 4fa266e..66dff34 100644
> --- a/src/auditd-event.c
> +++ b/src/auditd-event.c
> @@ -1130,6 +1130,12 @@ static void reconfigure(struct auditd_consumer_data
> *data) // log format
>  	oconf->log_format = nconf->log_format;
> 
> +	// log group
> +	if (oconf->log_group != nconf->log_group) {
> +		oconf->log_group = nconf->log_group;
> +		need_reopen = 1;
> +	}
> +
>  	// action_mail_acct
>  	if (strcmp(oconf->action_mail_acct, nconf->action_mail_acct)) {
>  		free((void *)oconf->action_mail_acct);

Re: [PATCH audit] reconfigure: Apply log_group change

[Lubomir Rintel]

Hi Steve,

On Fri, 2014-07-25 at 09:27 -0400, Steve Grubb wrote:
> Hello Lubomir,
> 
> Thanks for the patch...but I think that why this is not currently addressed in 
> the code is something like this. Let's assume the system has 5 audit logs with 
> 600 root.
> 
> If an admin decides to allow a group to read the audit logs, they will have 
> to:
> 
> 1) create the group
> 2) add users to the group
> 3) change the auditd.conf file
> 4) chgrp -R group /var/log/audit
> 5) chmod 0750 /var/log/audit
> 6) chmod 0640 /var/log/audit/*
> 7) restart the audit daemon
> 
> What this patch does is part of step 4 and 6. It would change audit.log to be 
> readable, but would leave audit.log.1 -> audit.log.4 untouched. Because 
> allowing a group requires so many steps, it's always been left as an admin 
> exercise...just like revoking group access would.

I believe that not touching already rotated files would be an expected
behavior; as well as requirement for the operator to take care of parent
directory permissions. (At least I'd expect that.) However, in our setup
the configuration is changes upon machine setup, with a very low change
there would be rotated log files around.

The last step ("restart the audit daemon") is precisely what I'm trying
to avoid, as it is no longer allowed in el7 (the service file
specifically disables ability to stop the service, likely to protect
loss of any audit events).

> -Steve

Regards
Lubo

> 
> 
> On Friday, July 25, 2014 01:59:04 PM Lubomir Rintel wrote:
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1118313
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1118262
> > ---
> >  src/auditd-event.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/src/auditd-event.c b/src/auditd-event.c
> > index 4fa266e..66dff34 100644
> > --- a/src/auditd-event.c
> > +++ b/src/auditd-event.c
> > @@ -1130,6 +1130,12 @@ static void reconfigure(struct auditd_consumer_data
> > *data) // log format
> >  	oconf->log_format = nconf->log_format;
> > 
> > +	// log group
> > +	if (oconf->log_group != nconf->log_group) {
> > +		oconf->log_group = nconf->log_group;
> > +		need_reopen = 1;
> > +	}
> > +
> >  	// action_mail_acct
> >  	if (strcmp(oconf->action_mail_acct, nconf->action_mail_acct)) {
> >  		free((void *)oconf->action_mail_acct);
> 

Re: [PATCH audit] reconfigure: Apply log_group change

[Steve Grubb]

Hello,

On Friday, July 25, 2014 04:04:03 PM Lubomir Rintel wrote:
> On Fri, 2014-07-25 at 09:27 -0400, Steve Grubb wrote:
> > Hello Lubomir,
> > 
> > Thanks for the patch...but I think that why this is not currently
> > addressed in the code is something like this. Let's assume the system has
> > 5 audit logs with 600 root.
> > 
> > If an admin decides to allow a group to read the audit logs, they will
> > have
> > to:
> > 
> > 1) create the group
> > 2) add users to the group
> > 3) change the auditd.conf file
> > 4) chgrp -R group /var/log/audit
> > 5) chmod 0750 /var/log/audit
> > 6) chmod 0640 /var/log/audit/*
> > 7) restart the audit daemon
> > 
> > What this patch does is part of step 4 and 6. It would change audit.log to
> > be readable, but would leave audit.log.1 -> audit.log.4 untouched.
> > Because allowing a group requires so many steps, it's always been left as
> > an admin exercise...just like revoking group access would.
> 
> I believe that not touching already rotated files would be an expected
> behavior; as well as requirement for the operator to take care of parent
> directory permissions. (At least I'd expect that.) However, in our setup
> the configuration is changes upon machine setup, with a very low change
> there would be rotated log files around.
> 
> The last step ("restart the audit daemon") is precisely what I'm trying
> to avoid, as it is no longer allowed in el7 (the service file
> specifically disables ability to stop the service, likely to protect
> loss of any audit events).

If you are using systemctl to manage auditd, then yes that happens. The 
problem is that security requirements dictate that the person issuing the 
shutdown/reload has to be recorded. This is so that if there is a gap in the 
audit records you know who did it. When done by systemctl, it talks to dbus 
which talks to systemd which sends the signal. The loginuid is recorded in the 
kernel and the audit daemon asks for the sender of the signal so it can be 
recorded. The loginuid for systemd is -1, so that is useless. 

Instead, you have to use the service command just like always. It runs in the 
user context and sends the signal itself. Therefore the loginuid is correctly 
recorded. Special support files have been added so that the service command 
supports: resume, rotate, stop, and restart - just like it always did.

I think the issue of attribution can be solved in the future when we have a 
kernel dbus + impersonation. At that point systemctl/systemd will work as 
required and we can drop the current method.

Thanks,
-Steve


> > On Friday, July 25, 2014 01:59:04 PM Lubomir Rintel wrote:
> > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1118313
> > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1118262
> > > ---
> > > 
> > >  src/auditd-event.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/src/auditd-event.c b/src/auditd-event.c
> > > index 4fa266e..66dff34 100644
> > > --- a/src/auditd-event.c
> > > +++ b/src/auditd-event.c
> > > @@ -1130,6 +1130,12 @@ static void reconfigure(struct
> > > auditd_consumer_data
> > > *data) // log format
> > > 
> > >  	oconf->log_format = nconf->log_format;
> > > 
> > > +	// log group
> > > +	if (oconf->log_group != nconf->log_group) {
> > > +		oconf->log_group = nconf->log_group;
> > > +		need_reopen = 1;
> > > +	}
> > > +
> > > 
> > >  	// action_mail_acct
> > >  	if (strcmp(oconf->action_mail_acct, nconf->action_mail_acct)) {
> > >  	
> > >  		free((void *)oconf->action_mail_acct);