Bug 1065951

Summary: VUL-0: systemd: Add additional checks to prevent unprivileged users from killing privileged processes in certain configurations
Product: [Novell Products] SUSE Security Incidents Reporter: Peter Wullinger <wullinger>
Component: IncidentsAssignee: systemd maintainers <systemd-maintainers>
Status: RESOLVED FIXED QA Contact: Security Team bot <security-team>
Severity: Minor    
Priority: P3 - Medium CC: astieger, fbui, jsegitz, karol, meissner, security-team
Version: unspecified   
Target Milestone: unspecified   
Hardware: All   
OS: openSUSE 42.3   
Whiteboard: CVSSv2:NVD:CVE-2018-16888:1.9:(AV:L/AC:M/Au:N/C:N/I:N/A:P) CVSSv3:NVD:CVE-2018-16888:4.7:(AV:L/AC:H/PR:L/UI:N/S:U/C:N/I:N/A:H) CVSSv3:RedHat:CVE-2018-16888:4.4:(AV:L/AC:H/PR:L/UI:R/S:U/C:N/I:N/A:H) CVSSv3:SUSE:CVE-2018-16888:5.0:(AV:L/AC:L/PR:L/UI:R/S:U/C:N/I:N/A:H)
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Attachments: Example service code

Description Peter Wullinger 2017-11-01 08:16:20 UTC
Created attachment 746593 [details]
Example service code

When creating a systemd service with Type=forking and PIDFile=, there seems to be a privilege escalation that makes it possible to have the service manager kill an arbitrary (maybe privileged) process with information from a non-privileged process.

Example:

Define a service as follows, where $TARGET_PID is the PID of a root owned process. /opt/test/escalator forks and writes the specified PID into a file.

# /etc/systemd/system/escalator.service
[Service]
Type=forking
PIDFile=/run/escalator/pid
ExecStart=/opt/test/escalator $TARGET_PID
RuntimeDirectory=escalator
User=nobody
Group=nogroup

If one starts such a unit, systemd will notice if the specified target PID is not a direct descendent of the service manager:

systemd[1]: escalator.service: Supervising process $TARGET_PID which is not our child. We'll most likely not notice when it exits.

It will, however, kill the target PID on when the unprivileged unit is stopped. 

The target PID may be running with higher privileges than the stopped unit, it seems possible to have systemd kill arbitrary processes with information from a non-privileged process.
Comment 1 Peter Wullinger 2017-11-01 08:26:52 UTC
This seems to be the same problem as upstream issue 6632 (https://github.com/systemd/systemd/issues/6632), but upstream has not (yet) classified this as a security issue ...
Comment 2 Johannes Segitz 2017-11-02 11:50:01 UTC
Thank you for your report. I can't recreate the issue. When I try to
kill a privileged process with my user service I get
escalator.service: Failed to kill main process 14514 (top), ignoring: Operation not permitted
which is what I would expect to see. Under which user did you create this scenario?
Comment 3 Peter Wullinger 2017-11-02 13:16:44 UTC
> escalator.service: Failed to kill main process 14514 (top), ignoring: Operation not permitted
> which is what I would expect to see. Under which user did you create this scenario

Let me rephrase that: You did create a user service, not a system service running as a non-privileged user (note location of service file in original posting)?

The scenario that I can reproduce is as follows

- have a system service running as User=nobody, type=Forking, PidFile=/run/escalator/pid
- systemd unconditionally trusts the contents of /run/escalator/pid, written by a non-root service. 
- stop the unit, systemd kills the PID named in the PID file. 

Systemd doesn't check the command name, (like e.g. killproc did previously), nor if the process is owned by the unit's user, nor if the process is in the service's cgroup. It assumes that the contents of the PIDfile are trustworthy.

The escalation is from "write to a file as user" to "kill any process on the system when a unit restarts". I think it is troublesome that setting PIDFile makes it possible for a service to break out of its cgroup this way.

This doesn't work for user units, since here the process manager itself doesn't run as root.
Comment 4 Johannes Segitz 2017-11-03 12:17:04 UTC
(In reply to Peter Wullinger from comment #3)
I got confused by initial description and the high rating. Now I can recreate your report, but this is more of a hardening measure. 

@systemd maintainers: It's a reasonable hardening, please include it in Factory.
Comment 5 Peter Wullinger 2017-11-03 13:39:23 UTC
That rating may have been overeager, possible stemming from the annoyance of upstream ignoring existing hardening practices.

Note that if I read the documentation correctly, setting

KillMode=control-group
ExecStop=/usr/bin/kill -TERM $MAINPID

seems to be a useable workaround, since (unless PermissionsStartOnly=yes), ExecStop= are executed with the privileges of the unit's user.

Additional hardening would be quite appreciated, since currently, we'll have to audit every non-root system unit.
Comment 6 Franck Bui 2017-11-06 14:33:52 UTC
(In reply to Johannes Segitz from comment #4)
> @systemd maintainers: It's a reasonable hardening, please include it in
> Factory.

Sorry but what are we supposed to include in Factory exactly ?
Comment 7 Franck Bui 2017-11-06 14:48:48 UTC
(In reply to Peter Wullinger from comment #3)
> 
> This doesn't work for user units, since here the process manager itself
> doesn't run as root.

Since this happens with system units I'm not sure to see the prob here. If root decides to start any untrusted services then there many ways to compromise the system much more severely...

The fact that the system service runs as a different user doesn't seem to matter here: the point if that it's a system service started by root.

Finally wouldn't it better to discuss this with upstream directly since you seem to request an enhancement ?
Comment 8 Peter Wullinger 2017-11-06 15:37:07 UTC
> Since this happens with system units I'm not sure to see the prob here. 
> If root decides to start any untrusted services then there many ways to 
> compromise the system much more severely...

A service that was explicitly configured to run as non-root being able to mislead the system to kill a root process is not a problem?

I'd like to repeat: killproc has additional (albeit racy) hardening against such problems, i.e. 

/sbin/killproc -p /run/escalator/pid foobar

doesn't work, especially when it is not run root nor as the user as which the escalator is running. This hardening goes away when using systemd.

And to be frank: I filed this so that you can take note of this (potential) security issue. If you label it a "request for enhancement" as upstream did, do so.
Comment 9 Franck Bui 2017-11-06 16:43:04 UTC
(In reply to Peter Wullinger from comment #8)
> > Since this happens with system units I'm not sure to see the prob here. 
> > If root decides to start any untrusted services then there many ways to 
> > compromise the system much more severely...
> 
> A service that was explicitly configured to run as non-root being able to
> mislead the system to kill a root process is not a problem?

Sorry for the my bad wording, I didn't mean there's no issues, I was just trying to say that the security issue doesn't seem as bad as you said.

And again the fact that the service is run as non-root doesn't seem to be relevant as the service is managed by root.

> 
> And to be frank: I filed this so that you can take note of this (potential)
> security issue. If you label it a "request for enhancement" as upstream did,
> do so.

Thanks for making us aware of this but I think it's better if it's discussed on github. BTW the discussion on github seems to be mainly about MAINPID= sent by sd_notify() currently.
Comment 10 Peter Wullinger 2017-11-06 17:29:35 UTC
> Sorry for the my bad wording, I didn't mean there's no issues, I was just 
> trying to say that the security issue doesn't seem as bad as you said.
> And again the fact that the service is run as non-root doesn't seem to 
> be relevant as the service is managed by root.

Hmm, let me explain this from my position:

Given e.g. that ExecStop= by default runs with the privileges of the unit's user, I think it is quite reasonable to expect systemd to do something similar to

   su -u $UNITUSER -c "/usr/sbin/killproc -p '$PIDFILE' -- '$MAINCMD'"

while it actually does

   kill "`head -1 "$PIDFILE"`"

So yes, obviously, you are dealing with a root making assumptions. But I do think this particular assumption is quite reasonable.

It's not "that bad" an issue in that it cannot be remotely exploited without help, but it's also a regression from a well written init script (however few there may actually be).

Quite frankly, having to go through the effort of explaining this in detail, with the quite strong expectation have it waved away as some irrelevant problem, is what makes me so reluctant to interact with upstream, especially on bugs already marked as "RFE" yet with an incomplete problem analysis.
Comment 11 Franck Bui 2017-11-07 13:16:32 UTC
(In reply to Peter Wullinger from comment #10)
> 
> Quite frankly, having to go through the effort of explaining this in detail,

hmm so you're basically asking us to do the effort instead ? ;)

> with the quite strong expectation have it waved away as some irrelevant
> problem, is what makes me so reluctant to interact with upstream, especially
> on bugs already marked as "RFE" yet with an incomplete problem analysis.

That doesn't sound fair.

I can't see where you actually tried to bring your arguments to upstream. And AFAICS, Lennart is open for discussion and had even proposed a solution...

So I would strongly suggest you to give your arguments there and see where it leads before blaming upstream.

Thanks.
Comment 12 Peter Wullinger 2017-11-07 14:26:59 UTC
(In reply to Franck Bui from comment #11)
> (In reply to Peter Wullinger from comment #10)
> > 
> > Quite frankly, having to go through the effort of explaining this in detail,
> 
> hmm so you're basically asking us to do the effort instead ? ;)
> 

Hmm, this now seems to have become a discussion on what to expect from a maintainer with regard to the details in which a problem needs to be explained to warrant actual attention from the maintainer. I may have even started it. If so, I'm sorry for that, it does belong elsewhere.

Nonetheless, this by now has nothing to do with the initial privilege escalation reported by this bug. My intentions for this bug report have been fulfilled: SuSE knows that this is a potential problem to look out for and that upstream has yet to react and where the reaction is likely to take place.
Comment 18 Johannes Segitz 2018-11-30 10:55:53 UTC
fixed in Factory, not going to backport this one because risk of regressions is higher than benefit of this hardening
Comment 19 Karol Babioch 2019-01-10 09:37:20 UTC
*** Bug 1120625 has been marked as a duplicate of this bug. ***