apport permission error

[Thadeu Lima de Souza Cascardo]

On Thu, Feb 13, 2020 at 01:11:59PM -0800, Brian Murray wrote:
> On Thu, Feb 13, 2020 at 08:47:03AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > On Wed, Feb 12, 2020 at 03:43:28PM -0800, Brian Murray wrote:
> > > I've recently been investigating a bug[1] where whoopsie-upload-all,
> > > provided by apport, is unable to add extra details to crash reports.
> > > This happens when the crash report has the following attributes:
> > > 
> > > -rw-r----- 1 bdmurray whoopsie 194K Feb  7 15:22 _usr_bin_seeded-in-ubuntu.1000.crash
> > > 
> > > 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?
> > > 
> > 
> > This doesn't allow O_CREAT open on files inside world-writable sticky
> > directories.
> > 
> > So, basically, we are preventing the root running program to open a file
> > that is controlled by a potential attacker.
> > 
> > It seems open(filename, "ab") is what whoopsie-upload-all is calling that
> > python3 translates into:
> > 
> > openat(AT_FDCWD, "filename", O_WRONLY|O_CREAT|O_APPEND|O_CLOEXEC, 0666) = 3
> > 
> > I wonder if using "+b", then seeking would do what is intended here, as I
> > see no easy way to still use python's open and not use O_CREAT with "a" or
> > "w". I see there is os.open, which allows one to give the flags to be used:
> > 
> > os.open("filename", os.O_WRONLY | os.O_APPEND)
> > 
> > That would assume the file exists and fail otherwise, but it's better than
> > what is happening right now.
> > 
> > One could ask what could an attacker do with whoopsie-upload-all anyway
> > that it wouldn't be able to do with protected_regular = 1, but we are
> > protecting from a class of attacks, and other programs will need to adapt,
> > unfortunately, unless we start seeing so many breaks that we need to step
> > back.
> 
> 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?
> 

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.

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?

> Additionally, when did /proc/sys/fs/protected_regular get set to 1? I
> ask as the crash in the Error Tracker shows systems running 19.10 being
> affected but a physical machine and virtual machine I spot checked did
> not have it set to 1.
> 

This has been caused by an upgrade to systemd. So, it seems systemd 241
upstream is doing it, and Ubuntu systemd 242-7ubuntu1 dropped it, leaving it
for us in the kernel team to decide about it, which we never did, AFAIK.

Then, for some odd reason, during a merge with Debian, the change was dropped,
possibly by accident. See Ubuntu systemd commit
e33e05451e3c13589a71cde603d0279a6608ab8b, which was released first as
243-2ubuntu1.

I am adding ubuntu-devel, xnox and rbalint to the discussion.

We will probably drop this again from systemd, but we should continue the
discussion as to what to do as a distro.

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.

Cascardo.

> Thanks!
> --
> Brian Murray