Wednesday, 19 February 2020

Re: apport permission error

On Fri, Feb 14, 2020 at 09:31:55AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > > > It's worth noting whoopsie-upload-all is run as root. I've discovered
> > > > that changing /proc/sys/fs/protected_regular from 1 to 0 allows
> > > > whoopsie-upload-all to write to the file. I imagine that apport is not
> > > > the only application affected by this setting.
> > > >
> > > > What is the practical benefit of having protected_regular set to 1?

The Ubuntu Security Team would like to have these exploit mitigations
enabled if we can.

Application authors have been forgetting to add O_EXCL to their
open(O_CREAT) calls for decades; leaving it off is almost always a
mistake. protected_regular helps protect against a common exploit

However, among the systems I have easy access to, it appears
protected_regular wasn't enabled until focal[1].

I'm worried that turning this flag on for the first time in an LTS release
may be breaking too many expectations.

Adapting applications may be too much effort; because I don't know what
exactly apport is doing here it is hard to estimate how much effort it
will take to adapt it. Possibly the user-launched apport instances need
to create their own directory on launch. Possibly apport needs a
more invasive redesign.

> > Is there a system in place for tracking the number of breaks / programs
> > that have needed to adapt? As a follow on is there some way we could
> > search the archive for programs that will break because of this change?

Source code searching is not practical. The combination of working
with files in a world-writable sticky directory and not already using
O_EXCL with O_CREAT is not feasible to search for.

On failure, auditd will log entries that include "sticky_create_regular".
(I don't see any on my one focal system, but it is far from representative.)

> Any calls to fopen(..., "w") or fopen(..., "a") will use O_CREAT. But we are
> talking about opening a file for writing on /tmp/, where the usual pattern is
> using mktemp(3), then opening the file for writing. In this case, the file
> should not exist, and note that using mktemp is not advised, because of the
> races entailed. mkstemp is the recommended solution, where O_EXCL is also used.

This is an excellent demonstration of the problem; often times
applications are using an abstraction layer that prevents easily adding
O_EXCL to the open(2) call.

> Now, once files are created there, they can be read from, moved, but not opened
> for writing anymore, as lots of programs will end up using O_CREAT. Now, I
> can't answer if this is a pattern or not. You found one that is used for
> /var/crash/. Now, should /var/crash/ be world-writable? What does write there?

Debian Code Search suggests it's used without specific purpose:

Apport, kernel crash dumps configured via the makedumpfile package, a tool
to clean up home directories, Intel openpath fabric tools, systemtap,

Interestingly enough, systemd-coredump(8) will write to
/var/lib/systemd/coredump instead of /var/crash.

> It has been like this since November. Though eoan would have been a better way
> to introduce it and see what breaks. As for the other options, at first, I
> would lean towards enabling them, but for regular files, it seems to be
> something that demands more thought.

Yes, eoan would have been the better time to enable this.

What will we use to determine when to enable this mitigation by default in
Ubuntu? We cannot expect all software to be adapted -- afterall, if all
software were adapted, then we would no longer need the mitigation. This
mitigation exists because a lot of software is potentially unsafe.

Addressing apport is probably part of the minimum. (I also wonder if we
could use systemd-coredump(8) to replace some of the more bug-prone
portions of apport.)

A Galera tool was recently fixed for protected_regular:

Is there a third example of an application which trips over this
mitigation in a common usage?

Thanks again for bringing this up. We want this mitigation but if the
ecosystem is plainly not ready for it yet, we'd probably do more harm to
roll it out prematurely.


16.04 LTS:
$ sudo grep -R . /proc/sys/fs/protected_*

18.04 LTS:
$ sudo grep -R . /proc/sys/fs/protected_*

$ sudo grep -R . /proc/sys/fs/protected_*

$ sudo grep -R . /proc/sys/fs/protected_*