Bug 1191614 - AUDIT-FIND: auditd: Split systemd services to allow for stricter hardenings
AUDIT-FIND: auditd: Split systemd services to allow for stricter hardenings
Status: RESOLVED FIXED
Classification: Novell Products
Product: SUSE Security Incidents
Classification: Novell Products
Component: Audits
unspecified
Other Other
: P5 - None : Minor
: ---
Assigned To: Enzo Matsumiya
Security Team bot
:
Depends on:
Blocks: 1181400
  Show dependency treegraph
 
Reported: 2021-10-13 12:20 UTC by Johannes Segitz
Modified: 2022-02-15 14:58 UTC (History)
3 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---
ematsumiya: needinfo? (fvogt)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Johannes Segitz 2021-10-13 12:20:30 UTC
+++ This bug was initially created as a clone of Bug #1181400 +++

As Ludwig suggested in https://bugzilla.suse.com/show_bug.cgi?id=1181400#c29 we could split the services to ensure that the main daemon is run without additional permissions
Comment 1 Enzo Matsumiya 2021-10-13 23:15:10 UTC
Submitted https://build.opensuse.org/request/show/925196

Please share your thoughts.
Comment 2 Fabian Vogt 2021-11-29 10:47:27 UTC
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?
Comment 3 Ludwig Nussel 2021-11-29 11:33:47 UTC
Requires/Wants auditd.service to active augenrules should suffice without touching presets right?
Comment 4 Ludwig Nussel 2021-11-29 11:37:35 UTC
Requires/Wants _in_ auditd.service to pull in augenrules.service I mean :)
Comment 5 Fabian Vogt 2021-11-29 11:43:42 UTC
(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.
Comment 6 Enzo Matsumiya 2021-11-29 13:35:10 UTC
(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...
Comment 7 Enzo Matsumiya 2021-11-29 13:39:51 UTC
(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.
Comment 8 Fabian Vogt 2021-11-29 13:55:29 UTC
(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.
Comment 9 Fabian Vogt 2021-11-29 13:56:21 UTC
(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?
Comment 10 Enzo Matsumiya 2021-11-29 14:12:54 UTC
(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?
Comment 11 Enzo Matsumiya 2021-11-29 14:14:06 UTC
(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.
Comment 12 Fabian Vogt 2021-11-29 14:23:44 UTC
(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.
Comment 13 Enzo Matsumiya 2021-11-29 14:46:05 UTC
(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
Comment 14 Enzo Matsumiya 2022-01-06 16:10:15 UTC
(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?
Comment 16 Enzo Matsumiya 2022-02-03 00:10:35 UTC
(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?
Comment 17 Johannes Segitz 2022-02-03 08:18:53 UTC
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)
Comment 18 Enzo Matsumiya 2022-02-15 14:58:03 UTC
(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.