Bug#1022994: Bug#1023778: TMPDIR behaviour in maintainer scripts [was: Re: Bug#1023778: mysql-server-8.0: fails to restart on upgrade with libpam-tmpdir]

2022-11-13 Thread Robie Basak
On Sun, Nov 13, 2022 at 05:46:00PM +0100, Marco d'Itri wrote:
> On Nov 13, Robie Basak  wrote:
> 
> > This seems inconsistent to me. Where is the expectation that TMPDIR must
> > be unset if dropping privileges coming from? Obviously for users of
> Where is the expectation that $TMPDIR is writable by any user but the 
> current one?
> I do not believe that it is expected that if a user creates a directory 
> and points $TMPDIR to it then they also have to make it sticky, so this 
> has nothing to do with libpam-tmpdir.

I understand the traditional semantics of TMPDIR to be exactly the same
as /tmp. So that includes the sticky bit, or at least behaviour that is
equivalent under all circumstances. Or, alternatively, that someone who
sets TMPDIR without setting the sticky bit is certain that it will be
used in a way that does not rely on that.

libpam-tmpdir breaks those semantics in a way that breaks in edge cases
like the situation raised in this (and other) bug reports. So on the
contrary, I think it has everything to do with libpam-tmpdir, and
anything else that sets TMPDIR to something that doesn't match the
traditional semantics.

To be clear, if it's better that we change the semantics to improve the
system as a whole, then I don't have a particular objection to that.

But I'd prefer that it be done deliberately, with consensus across all
developers, and be well-defined and documented. Rather than have a
change of semantics exist in a multitude of individual fixes for
individual bug reports that potentially end up solving the issue
differently, inconsistently or incompletely, and without regard for the
bigger picture (eg. if the conclusion is that it should be done
somewhere other than maintainer scripts or in common tooling). There's
also the risk that we swap one problem for another - for example if
there are use cases which rely on maintainer scripts honouring TMPDIR,
including when they drop privileges.


signature.asc
Description: PGP signature


Bug#1022994: Bug#1023778: TMPDIR behaviour in maintainer scripts [was: Re: Bug#1023778: mysql-server-8.0: fails to restart on upgrade with libpam-tmpdir]

2022-11-13 Thread Marco d'Itri
On Nov 13, Robie Basak  wrote:

> This seems inconsistent to me. Where is the expectation that TMPDIR must
> be unset if dropping privileges coming from? Obviously for users of
Where is the expectation that $TMPDIR is writable by any user but the 
current one?
I do not believe that it is expected that if a user creates a directory 
and points $TMPDIR to it then they also have to make it sticky, so this 
has nothing to do with libpam-tmpdir.

-- 
ciao,
Marco


signature.asc
Description: PGP signature


Bug#1022994: Bug#1023778: TMPDIR behaviour in maintainer scripts [was: Re: Bug#1023778: mysql-server-8.0: fails to restart on upgrade with libpam-tmpdir]

2022-11-13 Thread Robie Basak
On Sun, Nov 13, 2022 at 04:16:29PM +0100, Marco d'Itri wrote:
> And I think that it would be wrong to have dpkg generally unset $TMPDIR, 
> because if root sets it then it would be reasonable to expect that also
> dpkg and the maintainer scripts use it (as long as they are not dropping 
> privileges).

This seems inconsistent to me. Where is the expectation that TMPDIR must
be unset if dropping privileges coming from? Obviously for users of
libpam-tmpdir that's a problem. But in the default case, it's something
that would be entirely reasonable to inherit through a drop of
privileges, for the same reason that I think you find that setting
TMPDIR for maintainer scripts to use would be useful.


signature.asc
Description: PGP signature


Bug#1022994: Bug#1023778: TMPDIR behaviour in maintainer scripts [was: Re: Bug#1023778: mysql-server-8.0: fails to restart on upgrade with libpam-tmpdir]

2022-11-13 Thread Robie Basak
On Sun, Nov 13, 2022 at 02:58:47PM +, Simon McVittie wrote:
> If the maintainer script is *dropping* privileges from root down to a
> system user, then I think the maintainer script is/should be responsible
> for doing that privilege drop in a way that works...

Agreed, but amongst various other things, I don't think it's at all
clear whether TMPDIR is one of the things that the maintainer script
should be expected to unset in this case.

For libpam-tmpdir's use of setting TMPDIR, it's obviously the required
behaviour.

But what about in general? What's the "environment variable" interface
to maintainer scripts? Clearly they are expected to honor _some_
environment variables. Is TMPDIR one of them? Where's the complete list
defined, so that we can be sure that the semantics are reasonable for
all the use cases we can think of, and so that we can all write
maintainer scripts correctly?

Why is it that dropping privileges requires the unsetting of environment
variables in the case of maintainer script invocations anyway? They are
always run from a trusted environment and unsetting variables removes
the ability to pass configuration through. Sure, I can't rely on some
expectations holding (eg. HOME), but if I don't rely on this, there's no
problem. So then, what about TMPDIR? What are its actual semantics?
Tying TMPDIR to the uid that uses it is not the default, nor the
tradition. This entanglement is something that libpam-tmpdir adds. Maybe
that means that we need to consider TMPDIR's semantics changed, because
people find this kind of behaviour more useful. But that's a discussion
that hasn't yet happened.

For example, is there a user who expects to be able to use TMPDIR to
tell a maintainer script where there is enough space for its task, only
to find that it doesn't have any effect because the maintainer script
drops privileges and unsets it before doing its task?


signature.asc
Description: PGP signature


Bug#1022994: Bug#1023778: TMPDIR behaviour in maintainer scripts [was: Re: Bug#1023778: mysql-server-8.0: fails to restart on upgrade with libpam-tmpdir]

2022-11-13 Thread Marco d'Itri
On Nov 13, Simon McVittie  wrote:

> I think you can both be right. The symptom here is a maintainer script
> failing, but if I'm understanding Marco's argument correctly, he's
> saying that the root cause is that when you switch between execution
> environments, not all of the environment variables inherited from the old
> execution environment are appropriate for the new one. (A similar thing
> can happen for other inheritable process parameters like resource limits.)
Yes, but this is a general issue and not relevant for this specific 
case.

Because let's consider an environment in which:
- there is no relevant elevation of privileges, so discussions about 
  what should libpam-something, sudo, etc... do are not applicable
- the root environmente legitimately has TMPDIR=/tmp/root-only/ mode 700
- a maintainer script does something as an unprivileged user which needs
  to access $TMPDIR

so you correctly noted that:

> If the maintainer script is *dropping* privileges from root down to a
> system user, then I think the maintainer script is/should be responsible
> for doing that privilege drop in a way that works - but ideally the
> maintainer script should be delegating that responsibility to a common
> tool rather than open-coding it itself, either by launching a one-shot
> system service that the init infrastructure can run in a predictable
> execution environment, or using a common privilege-level-switching tool
> like runuser, su or sudo, or using an IPC-based "run this command in a
> more controllable enviroment" tool like systemd-run.

And I think that it would be wrong to have dpkg generally unset $TMPDIR, 
because if root sets it then it would be reasonable to expect that also
dpkg and the maintainer scripts use it (as long as they are not dropping 
privileges).

-- 
ciao,
Marco


signature.asc
Description: PGP signature


Bug#1022994: Bug#1023778: TMPDIR behaviour in maintainer scripts [was: Re: Bug#1023778: mysql-server-8.0: fails to restart on upgrade with libpam-tmpdir]

2022-11-13 Thread Simon McVittie
On Sun, 13 Nov 2022 at 11:38:08 +, Robie Basak wrote:
> On Sun, Nov 13, 2022 at 02:21:58AM +0100, Marco d'Itri wrote:
> > On Nov 12, Otto Kekäläinen  wrote:
> > > Instead of manually trying to manage TMPDIR env variable in various
> > > places, we should have a standardized way to run maintainer scripts in
> > > clean shell sessions that have all env variables set automatically
> > > correctly.
> >
> > This is not about maintainer scripts, but about programs which change 
> > the UID without sanitizing the environment.
> 
> No, it's absolutely about maintainer scripts, since that's the bug
> reported, and the specific fix suggested implies that all relevant
> maintainer scripts need changing.

I think you can both be right. The symptom here is a maintainer script
failing, but if I'm understanding Marco's argument correctly, he's
saying that the root cause is that when you switch between execution
environments, not all of the environment variables inherited from the old
execution environment are appropriate for the new one. (A similar thing
can happen for other inheritable process parameters like resource limits.)

The specific bug we're discussing in mysql-server-8.0 seems like it
potentially has two separate privilege transitions: one elevating
privileges from a sysadmin up to root (via `sudo apt upgrade` or
equivalent), and one dropping privileges from root down to a system user
to run a preparation step in the maintainer script. I think some of the
people replying to the thread on -devel might be conflating those two
privilege transitions, which means we end up talking at cross purposes
because a valid point made about one transition can be invalid for
the other.

For the privilege elevation from a sysadmin up to root, this could in
principle be addressed anywhere within the inheritance chain:

your shell -> sudo/su/pkexec -> apt -> dpkg -> maintainer script -> ...

but I think it's reasonable to say that it's sudo/su/pkexec/... that
is doing an unusual change to the execution environment, so it should
also be sudo/su/pkexec/... that is responsible for making sure the new
execution environment is internally consistent. Making each maintainer
script responsible for solving this problem for itself would scale poorly,
so if someone's suggested fix involves changing all maintainer scripts,
I'd very much prefer to look at whether that can be avoided.

If the maintainer script is *dropping* privileges from root down to a
system user, then I think the maintainer script is/should be responsible
for doing that privilege drop in a way that works - but ideally the
maintainer script should be delegating that responsibility to a common
tool rather than open-coding it itself, either by launching a one-shot
system service that the init infrastructure can run in a predictable
execution environment, or using a common privilege-level-switching tool
like runuser, su or sudo, or using an IPC-based "run this command in a
more controllable enviroment" tool like systemd-run.

smcv



Bug#1022994: Bug#1023778: TMPDIR behaviour in maintainer scripts [was: Re: Bug#1023778: mysql-server-8.0: fails to restart on upgrade with libpam-tmpdir]

2022-11-13 Thread Robie Basak
On Sun, Nov 13, 2022 at 02:21:58AM +0100, Marco d'Itri wrote:
> On Nov 12, Otto Kekäläinen  wrote:
> 
> > Instead of manually trying to manage TMPDIR env variable in various
> > places, we should have a standardized way to run maintainer scripts in
> > clean shell sessions that have all env variables set automatically
> > correctly.
> This is not about maintainer scripts, but about programs which change 
> the UID without sanitizing the environment.

No, it's absolutely about maintainer scripts, since that's the bug
reported, and the specific fix suggested implies that all relevant
maintainer scripts need changing.

You have generalised this, but I don't think it's at all clear that such
a broad generalisation is appropriate here.


signature.asc
Description: PGP signature