Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 853283

Summary: sys-apps/portage uses /var/tmp insecurely
Product: Gentoo Security Reporter: herypt
Component: VulnerabilitiesAssignee: Gentoo Security <security>
Status: IN_PROGRESS ---    
Severity: normal CC: 1i5t5.duncan, ago, dev-portage, floppym, mgorny, mjo, sam, toralf
Priority: Normal Keywords: PullRequest
Version: unspecified   
Hardware: All   
OS: Linux   
See Also: https://github.com/gentoo/gentoo/pull/27871
https://github.com/gentoo/portage/pull/926
https://bugs.gentoo.org/show_bug.cgi?id=891863
https://bugs.gentoo.org/show_bug.cgi?id=603222
Whiteboard:
Package list:
Runtime testing required: ---

Description herypt 2022-06-21 07:15:52 UTC
If /var/tmp/portage exists and is owned by the wrong user/group, portage will change the owner and reuse the directory.
Steps to reproduce:
$ sudo rm -rf /var/tmp/portage (systemd-tmpfiles will do this automatically if the directory isn't used for 30 days: https://systemd.io/TEMPORARY_DIRECTORIES/)
$ mkdir /var/tmp/portage
$ cp /usr/bin/id /var/tmp/portage/app-editors
$ chmod g+s /var/tmp/portage/app-editors (setuid is cleared but setgid isn't)
$ sudo emerge vim (will crash)
$ /var/tmp/portage/app-editors
egid=250(portage)
Comment 1 John Helmert III archtester Gentoo Infrastructure gentoo-dev Security 2022-10-19 23:31:34 UTC
Adding another Portage maintainer
Comment 2 Mike Gilbert gentoo-dev 2022-10-20 01:41:46 UTC
I think this behavior is pretty well known, and I see no reason to treat this bug as "confidential".
Comment 3 Mike Gilbert gentoo-dev 2022-10-20 02:04:54 UTC
The permissions/owner get reset whenever portage.util.ensure_dirs() is called with PORTAGE_TMPDIR as an argument.

For example, this happens in _emerge.EbuildBuild.async_lock().

Example call trace:

(Pdb) bt
  /home/floppym/src/portage/bin/emerge(60)<module>()
-> retval = emerge_main()
  /home/floppym/src/portage/lib/_emerge/main.py(1294)emerge_main()
-> return run_action(emerge_config)
  /home/floppym/src/portage/lib/_emerge/actions.py(4065)run_action()
-> retval = action_build(emerge_config, spinner=spinner)
  /home/floppym/src/portage/lib/_emerge/actions.py(677)action_build()
-> retval = mergetask.merge()
  /home/floppym/src/portage/lib/_emerge/Scheduler.py(1149)merge()
-> rval = self._merge()
  /home/floppym/src/portage/lib/_emerge/Scheduler.py(1599)_merge()
-> self._main_loop()
  /home/floppym/src/portage/lib/_emerge/Scheduler.py(1551)_main_loop()
-> self._event_loop.run_until_complete(self._main_exit)
  /home/floppym/src/portage/lib/portage/util/_eventloop/asyncio_event_loop.py(129)_run_until_complete()
-> return self._loop.run_until_complete(future)
  /usr/lib/python3.11/asyncio/base_events.py(637)run_until_complete()
-> self.run_forever()
  /usr/lib/python3.11/asyncio/base_events.py(604)run_forever()
-> self._run_once()
  /usr/lib/python3.11/asyncio/base_events.py(1909)_run_once()
-> handle._run()
  /usr/lib/python3.11/asyncio/events.py(80)_run()
-> self._context.run(self._callback, *self._args)
  /home/floppym/src/portage/lib/_emerge/AsynchronousTask.py(210)_exit_listener_cb()
-> listener(self)
  /home/floppym/src/portage/lib/_emerge/EbuildBuild.py(122)_start_with_metadata()
-> self._prefetch_exit(prefetcher)
  /home/floppym/src/portage/lib/_emerge/EbuildBuild.py(210)_prefetch_exit()
-> AsyncTaskFuture(future=self._build_dir.async_lock()), self._start_pre_clean
  /home/floppym/src/portage/lib/_emerge/EbuildBuildDir.py(107)async_lock()
-> portage.util.ensure_dirs(
  /home/floppym/src/portage/lib/portage/util/__init__.py(1616)ensure_dirs()
-> perms_modified = apply_permissions(dir_path, **kwargs)
> /home/floppym/src/portage/lib/portage/util/__init__.py(1221)apply_permissions()
-> modified = False
(Pdb) p filename
'/x/portage'

We would probably need to make some adjustment to ensure_dirs() or apply_permissions().
Comment 4 John Helmert III archtester Gentoo Infrastructure gentoo-dev Security 2022-10-20 15:22:24 UTC
(In reply to Mike Gilbert from comment #2)
> I think this behavior is pretty well known, and I see no reason to treat
> this bug as "confidential".

Unchecking this bug as private then.
Comment 5 Mike Gilbert gentoo-dev 2022-10-20 15:24:51 UTC
An easy workaround for this would be to install a tmpfiles snippet to create /var/tmp/portage on boot and prevent it from being cleaned.
Comment 6 Michael Orlitzky gentoo-dev 2022-10-21 02:57:21 UTC
(In reply to Mike Gilbert from comment #5)
> An easy workaround for this would be to install a tmpfiles snippet to create
> /var/tmp/portage on boot and prevent it from being cleaned.

I don't think the PR is enough. The directory /var/tmp/portage is still subject to being cleaned by systemd, or anything else really -- the FHS explicitly allows it. And as soon as the directory goes away, the exploit is available again.

Note also that running tmpfiles_process() during (re)installs might not be safe here. I don't remember any of the details, but it's working in a directory with unusual security properties, and the systemd docs specifically warn about it:

  (You can get away with using guessable names, if you pre-create subdirectories
  below /tmp/ for them, like X11 does with /tmp/.X11-unix/ through tmpfiles.d/
  drop-ins. However this is not recommended, as it is fully safe only if these 
  directories are pre-created during early boot, and thus problematic if package 
  installation during runtime is permitted.)
Comment 7 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2022-10-21 02:59:49 UTC
(being cheeky and CCing you as I want your input on the bug generally ;))
Comment 8 Mike Gilbert gentoo-dev 2022-10-21 12:56:53 UTC
(In reply to Michael Orlitzky from comment #6)
> (In reply to Mike Gilbert from comment #5)
> > An easy workaround for this would be to install a tmpfiles snippet to create
> > /var/tmp/portage on boot and prevent it from being cleaned.
> 
> I don't think the PR is enough. The directory /var/tmp/portage is still
> subject to being cleaned by systemd

It's not subject to cleaning by systemd if we install a tmpfiles snippet that disables said cleaning.

> 
> Note also that running tmpfiles_process() during (re)installs might not be
> safe here.

If the user is currently running emerge to install sys-apps/portage, and /var/tmp/portage has already been compromised, they are screwed anyway and running tmpfiles_process will not hurt them any further.
Comment 9 Mike Gilbert gentoo-dev 2022-10-21 13:37:34 UTC
(In reply to Mike Gilbert from comment #8)
> (In reply to Michael Orlitzky from comment #6)
> > Note also that running tmpfiles_process() during (re)installs might not be
> > safe here.
> 
> If the user is currently running emerge to install sys-apps/portage, and
> /var/tmp/portage has already been compromised, they are screwed anyway and
> running tmpfiles_process will not hurt them any further.

That said, running tmpfiles_process is also rather pointless, since /var/tmp/portage must already exist if we are executing ebuild code. I will adjust the PR to remove it.
Comment 10 Michael Orlitzky gentoo-dev 2022-10-21 14:21:14 UTC
(In reply to Mike Gilbert from comment #8)
> 
> It's not subject to cleaning by systemd if we install a tmpfiles snippet
> that disables said cleaning.

Yep, I didn't read the tmpfiles spec closely enough.

But now I'm realizing another problem: this directory isn't fixed, it's controlled by $PORTAGE_TMPDIR at runtime. So, for example, the people who put it under /tmp won't be helped by hard-coding a fix for /var/tmp/portage.

Obviously we can't fix all security issues that result from a poor (user) choice of PORTAGE_TMPDIR, but we should probably aim to make it safe in any situation that is not already inherently insecure.

At the simple-but-disruptive end of the spectrum, we could give up on the idea of using a single hierarchical directory for all builds. If instead we had one temporary directory for each build, portage could use the python facilities for creating that temporary directory on the fly, passing dir=PORTAGE_TMPDIR. That should be secure anywhere that anything is secure.

I'm not quite sure what exists at the other end. With a great deal of effort, it's possible in C and on Linux (systemd-tmpfiles does it) to create/chmod/chown things safely in a hostile directory. But I don't think it can be done in python, and (if my knowledge is not outdated) it relies on the particulars of the Linux /proc implementation as well as several non-default kernel options.
Comment 11 Mike Gilbert gentoo-dev 2022-10-21 14:23:24 UTC
I think it might make sense to update the ensure_dirs function to raise an error if the target directory already exists and the current owner is neither root (uid 0) nor the uid passed to the function via kwargs.
Comment 12 Larry the Git Cow gentoo-dev 2022-10-21 14:32:58 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=d9eeba71d686e10711cba09e547ed5e20d8c6bfb

commit d9eeba71d686e10711cba09e547ed5e20d8c6bfb
Author:     Mike Gilbert <floppym@gentoo.org>
AuthorDate: 2022-10-20 15:45:51 +0000
Commit:     Mike Gilbert <floppym@gentoo.org>
CommitDate: 2022-10-21 14:32:08 +0000

    sys-apps/portage: install tmpfiles.d to create /var/tmp/portage
    
    This ensures the directory is created at boot, and prevents other users
    from creating it before emerge is run.
    
    Bug: https://bugs.gentoo.org/853283
    Closes: https://github.com/gentoo/gentoo/pull/27871
    Signed-off-by: Mike Gilbert <floppym@gentoo.org>

 sys-apps/portage/files/portage-tmpdir.conf                              | 1 +
 .../portage/{portage-3.0.38.1.ebuild => portage-3.0.38.1-r1.ebuild}     | 2 +-
 sys-apps/portage/portage-9999.ebuild                                    | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)
Comment 13 Michael Orlitzky gentoo-dev 2022-10-21 15:15:52 UTC
(In reply to Mike Gilbert from comment #11)
> I think it might make sense to update the ensure_dirs function to raise an
> error if the target directory already exists and the current owner is
> neither root (uid 0) nor the uid passed to the function via kwargs.

That's part of what I'm talking about, but you have to be careful not to e.g. follow symlinks because I can create a symlink that makes /var/tmp/portage look good for a split second, and then delete it and recreate it as directory that I own. Enumerating badness is hard to get right, and relies on the cleverest hacker being on your team.

Another thing to think about is, when portage crashes, what are you going to tell people to do to fix the issue? Is *that* safe?
Comment 14 Mike Gilbert gentoo-dev 2022-10-21 17:30:55 UTC
(In reply to Michael Orlitzky from comment #13)
> Another thing to think about is, when portage crashes, what are you going to
> tell people to do to fix the issue? Is *that* safe?

If a random user owns PORTAGE_TMPDIR and the sysadmin doesn't know how that happened, they probably need to quarantine and audit the system.

If they created the directory with the wrong owner by accident, it should be safe to just remove it.
Comment 15 Mike Gilbert gentoo-dev 2022-10-22 03:34:55 UTC
I'm not sure it's really feasible make ensure_dirs() "safe" while still allowing random unprivileged users to run commands like ebuild(1). It makes my head hurt.

Short of revamping PORTAGE_TMPDIR entirely, perhaps we might document that changing PORTAGE_TMPDIR carries security implications and advise the user to take care.
Comment 16 Michael Orlitzky gentoo-dev 2022-10-22 17:04:17 UTC
(In reply to Mike Gilbert from comment #15)
> 
> Short of revamping PORTAGE_TMPDIR entirely

How annoying would this really be? It's easy, "obviously correct," and would simplify portage a bit.

When I need to inspect a failed build, it's usually interactively, so personally I wouldn't mind portage telling me to look in /var/tmp/portage-wwgpae9g or whatever. And PORTAGE_LOGDIR let's you put the build logs, at least, somewhere predictable.

Mainly I'd be worried about annoying people like Agostino and Toralf who (in an automated fashion) need to retrieve things like config.log. But I also don't think it would be too huge of a hurdle to parse the output from portage to figure out where to look for them.


> perhaps we might document that changing PORTAGE_TMPDIR carries security implications and advise the user to take care.

Even the default is unsafe if something other than systemd is doing the clean up, such as app-admin/tmpreaper.
Comment 17 Mike Gilbert gentoo-dev 2022-10-22 22:03:43 UTC
Using a random temp directory for every portage invocation makes resuming builds impossible.

Another possible solution would be to move the default PORTAGE_TMPDIR out of /var/tmp and into a location where random users don't have write access.
Comment 18 Mike Gilbert gentoo-dev 2022-10-22 22:25:31 UTC
(In reply to Mike Gilbert from comment #17)
> Using a random temp directory for every portage invocation makes resuming
> builds impossible.

I think we would need to provide some additional command line option to manually specify the actual build directory to support resuming.
Comment 19 Michael Orlitzky gentoo-dev 2022-10-22 22:28:52 UTC
(In reply to Mike Gilbert from comment #17)
> Using a random temp directory for every portage invocation makes resuming
> builds impossible.

This could be worked around by (say) saving a package -> directory map outside of /var/tmp, but raises a good point: how long do we expect these directories to stick around? For ebuild(1) to work as promised, we likely shouldn't be treating them as temporary in the first place...

> Another possible solution would be to move the default PORTAGE_TMPDIR out of
> /var/tmp and into a location where random users don't have write access.

...making this a good idea.

Specifically, we will run into trouble in any situation where ebuild(1) wants to reuse a directory but enough time has passed that the directory could have been cleaned.
Comment 20 Mike Gilbert gentoo-dev 2022-10-22 23:51:19 UTC
Ok, so where exactly to put it? :)

Somewhere under /var makes sense. Nothing in FHS really fits perfectly thoguh.

https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch05.html
Comment 21 Michael Orlitzky gentoo-dev 2022-10-24 14:04:00 UTC
(In reply to Mike Gilbert from comment #20)
> Ok, so where exactly to put it? :)
> 
> Somewhere under /var makes sense. Nothing in FHS really fits perfectly
> thoguh.
> 
> https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch05.html

/var/lib/portage/tmp would be my first instinct.

But that's the easy part. Are we going to remove PORTAGE_TMPDIR, or will we still allow people to set it to, say, /tmp where it will be insecure? The docs allude to that in various places, and there are good reasons to do it.

There's ultimately a conflict between two goals:

  1. Being able to re-access the temporary build directory at a later time based on the package name alone.
  2. Being able (and having a good reason to) put PORTAGE_TMPDIR under /tmp or /var/tmp.

While (1) is important, I think it's only developers who use it regularly. And those developers would be relatively well equipped to jump through some hoops if we have to break things to fix this. On the other hand (2) is an end-user feature, likely used all over the place, and is the security-sensitive bit.

The goals are mutually exclusive: you can't re-access a path by name under /tmp after a "significant" amount of time has passed, even if the name was originally chosen randomly. So, for example, we can't allow developers to put PORTAGE_TMPDIR=/tmp and somehow still use `ebuild compile` safely. Should we make people choose between (1) and (2)?

One option that comes to mind is to have the default be PORTAGE_TMPDIR=<unset>, which would use /var/lib/portage/tmp with a predictable naming scheme. Then if PORTAGE_TMPDIR is set, we'd use a secure naming scheme, breaking `ebuild` in the process. In effect we'd be making developers use the default if they want `ebuild` to be able to pick up where it left off, but making sure that end-users get security out of the box with the new version, regardless of their existing configuration.

I'm just thinking out loud, this will probably have to rattle around in the back of my head for a while longer before it all makes sense.
Comment 22 Mike Gilbert gentoo-dev 2022-10-24 15:22:25 UTC
(In reply to Michael Orlitzky from comment #21)
> The goals are mutually exclusive: you can't re-access a path by name under
> /tmp after a "significant" amount of time has passed, even if the name was
> originally chosen randomly. So, for example, we can't allow developers to
> put PORTAGE_TMPDIR=/tmp and somehow still use `ebuild compile` safely.
> Should we make people choose between (1) and (2)?

If the goal is to use a tmpfs for building, users could mount a separate tmpfs with a more secure mode. For example, I have PORTAGE_TMPDIR="/x" and this entry in fstab on my main Gentoo system:

> tmpfs /x tmpfs mode=0770,uid=root,gid=portage,size=20g 0 0
Comment 23 Michael Orlitzky gentoo-dev 2022-10-25 22:17:13 UTC
(In reply to Mike Gilbert from comment #22)
> 
> If the goal is to use a tmpfs for building, users could mount a separate
> tmpfs with a more secure mode. For example, I have PORTAGE_TMPDIR="/x" and
> this entry in fstab on my main Gentoo system:

In concert with a free-form PORTAGE_TMPDIR, I feel like this is a cop-out, because what tempfile security issues can't be avoided by giving each program its own dedicated tmpfs? We're still placing the pitfall at users' feet and expecting them to know that (a) there is a problem and (b) how to avoid it.

But, as a response to "I want to use tmpfs instead of the secure default," I think it's great. Could we get away with deprecating PORTAGE_TMPDIR, switching the default to /var/lib/portage/tmp, and documenting how to mount a tmpfs at that location for the people who want a tmpfs? The same approach should be able to handle network drives, and with bind mounts, almost anything else.

There are still some questions,

* How and when does /var/lib/portage/tmp get cleaned?
* Is this flexible enough for ccache, prefix, and everything else that PORTAGE_TMPDIR gets used for?

But I think I like the idea. It's secure out of the box, and applies retroactively to people who have PORTAGE_TMPDIR already set. We could emit a warning if PORTAGE_TMPDIR is defined, so that no one is left with a silently broken config. The code changes should be easy enough and actually simplify portage. And if you really want to, the door is still open to shoot yourself in the foot with bind mounts or symlinks (or simply by hacking the source).
Comment 24 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2022-10-25 22:20:55 UTC
I hate that I'm even commenting this, but it's worth keeping in mind that /var/lib/portage/tmp is longer than /var/tmp/portage, so we'll hit even more of those annoying path limit length bugs in test suites with socket names, etc.
Comment 25 Michael Orlitzky gentoo-dev 2022-10-25 22:30:52 UTC
(In reply to Sam James from comment #24)
> I hate that I'm even commenting this, but it's worth keeping in mind that
> /var/lib/portage/tmp is longer than /var/tmp/portage, so we'll hit even more
> of those annoying path limit length bugs in test suites with socket names,
> etc.

As payment for this, you would receive consistency: the path will always be /var/lib/portage/tmp, and not whatever monster the user has entered in PORTAGE_TMPDIR.
Comment 26 Mike Gilbert gentoo-dev 2022-10-25 23:50:11 UTC
I'm not on board with removing the ability for users to override PORTAGE_TMPDIR. We can document the security implication, and if they choose to shoot themselves in the foot that's on them.
Comment 27 Mike Gilbert gentoo-dev 2022-10-26 00:09:33 UTC
Anyway, I'm getting kind of tired of going back and forth on this bug.

The issue originally reported has been addressed via the tmpfiles.d drop-in. sys-apps/portage is no longer vulnerable in its default configuration.

Patches to make Portage more secure are always welcome, provided they don't remove existing flexibility.

My suggestion to the security team for any potential GLSA would be:

1. Advise users not to not remove /var/tmp/portage via automated cleanup scripts like tmpreaper.
2. Advise users take care when overriding PORTAGE_TMPDIR.
Comment 28 Larry the Git Cow gentoo-dev 2022-10-26 00:23:26 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=c046ee45bcc9d2e6164b70f73acf7074e03d31f1

commit c046ee45bcc9d2e6164b70f73acf7074e03d31f1
Author:     Mike Gilbert <floppym@gentoo.org>
AuthorDate: 2022-10-26 00:18:33 +0000
Commit:     Mike Gilbert <floppym@gentoo.org>
CommitDate: 2022-10-26 00:20:24 +0000

    sys-apps/portage: update tmpfiles.d to create /tmp/portage
    
    This is common alternate location for PORTAGE_TMPDIR.
    
    Bug: https://bugs.gentoo.org/853283
    Signed-off-by: Mike Gilbert <floppym@gentoo.org>

 sys-apps/portage/files/portage-tmpdir.conf                               | 1 +
 .../portage/{portage-3.0.38.1-r1.ebuild => portage-3.0.38.1-r2.ebuild}   | 0
 2 files changed, 1 insertion(+)
Comment 29 Michael Orlitzky gentoo-dev 2022-10-26 01:22:24 UTC
(In reply to Mike Gilbert from comment #27)
> Anyway, I'm getting kind of tired of going back and forth on this bug.
> 
> The issue originally reported has been addressed via the tmpfiles.d drop-in.
> sys-apps/portage is no longer vulnerable in its default configuration.
> 

It's still vulnerable when something other than systemd is doing the cleanup.
Comment 30 Mike Gilbert gentoo-dev 2022-10-26 01:48:28 UTC
(In reply to Michael Orlitzky from comment #29)
> It's still vulnerable when something other than systemd is doing the cleanup.

We can't control the actions of users.
Comment 31 Duncan 2023-01-23 22:46:34 UTC
(In reply to Mike Gilbert from comment #22)
> If the goal is to use a tmpfs for building, users could mount a separate
> tmpfs with a more secure mode. For example, I have PORTAGE_TMPDIR="/x" and
> this entry in fstab on my main Gentoo system:
> 
> > tmpfs /x tmpfs mode=0770,uid=root,gid=portage,size=20g 0 0

Doesn't that just open up a possible memory-exhaustion DoS due to multiple tmpfs with a total capacity greater than RAM (likely plus swap), that's arguably way less obscure and way easier to trigger, even inadvertently on a single-human-user system?

Or I can hope there's a total-tmpfs-limit kernel knob I missed, that can limit say 20 gig /tmp tmpfs plus 20 gig portage tmpfs plus misc other tmpfs to say 25 gig total?
Comment 32 Mike Gilbert gentoo-dev 2023-01-23 22:59:49 UTC
(In reply to Duncan from comment #31)

The sysadmin can override the size of any tmpfs mounts on the system by specifying them in fstab.
Comment 33 Duncan 2023-01-24 03:07:23 UTC Comment hidden (obsolete)
Comment 34 Mike Gilbert gentoo-dev 2023-01-24 19:14:51 UTC
Sorry to be rude, but I have marked comment 33 as "obsolete" to hide it from view by default. It's a long-winded explanation of how an admin could hypothetically misconfigure their system. I don't see how it is relevant to this bug.
Comment 35 John Helmert III archtester Gentoo Infrastructure gentoo-dev Security 2023-01-25 21:20:39 UTC
(In reply to Mike Gilbert from comment #34)
> Sorry to be rude, but I have marked comment 33 as "obsolete" to hide it from
> view by default. It's a long-winded explanation of how an admin could
> hypothetically misconfigure their system. I don't see how it is relevant to
> this bug.

Agreed.