Bug 1191614

Submitted https://build.opensuse.org/request/show/925196

Please share your thoughts.

This split broke audit. augenrules.service is not enabled by default, so audit rules are never loaded. Found while looking at https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/13761.

In theory,

[Install]
Also=augenrules.service

in auditd.service would suffice, but the systemd preset counteracts that. After auditd.service is installed, augenrules.service is disabled again through the preset.

BTW: What does upstream have to say about the unit hardening and split?

Requires/Wants auditd.service to active augenrules should suffice without touching presets right?

Requires/Wants _in_ auditd.service to pull in augenrules.service I mean :)

(In reply to Ludwig Nussel from comment #3)
> Requires/Wants auditd.service to active augenrules should suffice without
> touching presets right?

(In reply to Ludwig Nussel from comment #4)
> Requires/Wants _in_ auditd.service to pull in augenrules.service I mean :)

Yep, should work.

(In reply to Ludwig Nussel from comment #4)
> Requires/Wants _in_ auditd.service to pull in augenrules.service I mean :)

Bummer. I had this in my dev repo, but forgot to sync to OBS. Resubmitting...

(In reply to Fabian Vogt from comment #2)
> BTW: What does upstream have to say about the unit hardening and split?

AFAIK *.spec files and most files in audit/init.d/ are kind of templates for packagers/distributors to adapt to their own environments, and are in a best-effort support mode.

(In reply to Enzo Matsumiya from comment #6)
> (In reply to Ludwig Nussel from comment #4)
> > Requires/Wants _in_ auditd.service to pull in augenrules.service I mean :)
> 
> Bummer. I had this in my dev repo, but forgot to sync to OBS. Resubmitting...

And I submitted my own version with some additional fixes meanwhile, but while mentioning the SR here and clicking on "Save changes" I ended up with a "Mid-air collision" but didn't notice...

Anyway, https://build.opensuse.org/request/show/934558.

(In reply to Enzo Matsumiya from comment #7)
> (In reply to Fabian Vogt from comment #2)
> > BTW: What does upstream have to say about the unit hardening and split?
> 
> AFAIK *.spec files and most files in audit/init.d/ are kind of templates for
> packagers/distributors to adapt to their own environments, and are in a
> best-effort support mode.

Sounds like the split might get accepted upstream then?

(In reply to Fabian Vogt from comment #8)
> Anyway, https://build.opensuse.org/request/show/934558.

Yeah, you beat me to it :)

Your submission seems ok, just one nit though:

> +PropagatesStopTo=augenrules.service

Do we really need this even if augenrules.service is defined as oneshot?

(In reply to Fabian Vogt from comment #9)
> Sounds like the split might get accepted upstream then?

I think so. I'll open a PR.

(In reply to Enzo Matsumiya from comment #10)
> (In reply to Fabian Vogt from comment #8)
> > Anyway, https://build.opensuse.org/request/show/934558.
> 
> Yeah, you beat me to it :)
> 
> Your submission seems ok, just one nit though:
> 
> > +PropagatesStopTo=augenrules.service
> 
> Do we really need this even if augenrules.service is defined as oneshot?

Yes. Otherwise "systemctl restart auditd.service" results in no rules being loaded. It stops auditd, which clears all rules, and then starts auditd again. augenrules.service stays running the whole time, so it doesn't load rules again. So propagation to augenrules is necessary.

(In reply to Fabian Vogt from comment #12)
> Yes. Otherwise "systemctl restart auditd.service" results in no rules being
> loaded. It stops auditd, which clears all rules, and then starts auditd
> again. augenrules.service stays running the whole time, so it doesn't load
> rules again. So propagation to augenrules is necessary.

Got it, thanks.

(In reply to Enzo Matsumiya from comment #11)
> I think so. I'll open a PR.

Let's see what they have to say.
https://github.com/linux-audit/audit-userspace/pull/228

(In reply to Fabian Vogt from comment #12)
> (In reply to Enzo Matsumiya from comment #10)
> > (In reply to Fabian Vogt from comment #8)
> > > Anyway, https://build.opensuse.org/request/show/934558.
> > 
> > Yeah, you beat me to it :)
> > 
> > Your submission seems ok, just one nit though:
> > 
> > > +PropagatesStopTo=augenrules.service
> > 
> > Do we really need this even if augenrules.service is defined as oneshot?
> 
> Yes. Otherwise "systemctl restart auditd.service" results in no rules being
> loaded. It stops auditd, which clears all rules, and then starts auditd
> again. augenrules.service stays running the whole time, so it doesn't load
> rules again. So propagation to augenrules is necessary.

@Fabian

It looks like that this only works for stop, but not restart.

For restart I had to add "PartOf=auditd.service" to augenrules.service.

Can you confirm please?

(In reply to Enzo Matsumiya from comment #13)
> Let's see what they have to say.
> https://github.com/linux-audit/audit-userspace/pull/228

Upstream didn't like the hardening part because it could break third party plugins.

IMHO we could stay these modifications (unit hardening + separate augenrules service) as SUSE-only for now.

Thoughts?

That would be fine for me. I can't really judge how high the chance for breakage for third party plugins is, I've never used them and I would hope that they don't need more access than given here. Let's give it a try in Factory and see if people run into issues (hopefully not)

(In reply to Johannes Segitz from comment #17)
> Let's give it a try in
> Factory and see if people run into issues (hopefully not)

Ok, it's already there, so I'm closing this one.