- Package:
- debian-policy
- Source:
- debian-policy
- Submitter:
- LinuxChix SysAdmin
- Date:
- 2017-12-24 15:03:03 UTC
- Severity:
- normal
The use of "su - " in postinst is breaking package installs/upgrades only when there are file(s) in /etc/profile.d. Details are in the two bug reports: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=884955 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=875958 I do not know what other packages may use "su - " but would suggest that this issue be documented in debconf-doc so maintainers/devs can avoid this. If I should file this some place else, please let me know. Thank you.
Control: reassign -1 debian-policy This doesn't seem to have anything much to do with debconf, which is a specific piece of technology commonly used in maintainer scripts rather than being documentation of how to write maintainer scripts in general. While it does seem worth writing maintainer scripts in a way that avoids this kind of problem, I think that probably belongs in the policy manual; I've reassigned your bug there. Thanks,
control: retitle -1 Discourage 'su -' in maintscripts, in favour of plain 'su' control: user debian-policy@packages.debian.org control: usertags normative proposal Hello, The proposal to discourage the use of 'su -' seems uncontroversial, so now we need a patch to the Policy Manual. I suggest we use "should not" rather than "must not" as there may be cases unknown to us where 'su -' is needed.
Plain 'su' is also problematic, because it inherits the entire
environment, including variables like HOME, XDG_RUNTIME_DIR, XDG_*_HOME,
DBUS_SESSION_BUS_ADDRESS that make little sense to inherit across
privilege raising or dropping. A typical failure mode is to create files
in user A's directory owned by user B, which either causes a denial of
service for user A, or makes user A accidentally trust user B, neither
of which is good. That failure mode can be mitigated by software that
checks ownership of these files and directories before using them
(like dbus does when using XDG_RUNTIME_DIR, specifically to defend
against being used under su), but it seems unrealistic to assume that
everyone will always remember to include such checks.
runuser(1) might be a better thing to recommend for non-interactive use,
although that doesn't scrub the environment by default (runuser -l does),
so variables like XDG_RUNTIME_DIR can still be a problem.
If we had a reliable way to arrange for a script to be run in a
known/predictable environment without inheriting arbitrary state from
its parent, that would probably be a better solution to this than ad-hoc
privilege-dropping elsewhere in the process inheritance tree. If systemd
was mandatory, then one obvious answer would be to use systemd-run(1)
to ask systemd to run it as a child of process 1; but other init systems
are supported, so we can't rely on that.
The su(1) man page also says
The su command is used to become another user during a login session
which isn't really what's happening here (maintainer scripts aren't
necessarily run within login sessions), so perhaps this is su not being
used as intended?
Terry Roy wrote:
Arguably the bug here is that there was a file in /etc/profile.d that
assumed bash syntax, rather than guarding bashisms with a check for
$BASH_VERSION or similar. I would recommend changing those profile
snippets so the problematic parts only run on shells that are known to
support them (bash and maybe zsh).
For many uses of profile snippets, it would make sense to wrap them in a
guard against non-interactive shells, which also dodges any issues that
those snippets might cause in non-interactive shells.
smcv
Simon,
Thank you for your comments.
> Arguably the bug here is that there was a file in /etc/profile.d that
> assumed bash syntax, rather than guarding bashisms with a check for
> $BASH_VERSION or similar. I would recommend changing those profile
> snippets so the problematic parts only run on shells that are known to
> support them (bash and maybe zsh).
>
> For many uses of profile snippets, it would make sense to wrap them in
> a guard against non-interactive shells, which also dodges any issues
> that those snippets might cause in non-interactive shells.
>
> smcv
>
I've done quite a bit more testing on this. I've had three packages with
this issue - spamassassin, sa-compile and tuptime.
The user for spamassassin and sa-compile is debian-spamd whose shell is
set to /bin/sh. The user for tuptime is tuptime whose shell is also set
to /bin/sh.
Changing the shell to /bin/bash for tuptime for example, eliminates the
error with using 'su -'.
I did note that the package postgresql-common calls the Bourne shell in
postinst but without a login and this poses no problem during installation:
su -s /bin/sh postgres -c "test -O /var/lib/postgresql &&
test -G /var/lib/postgresql" || \
chown postgres:postgres /var/lib/postgresql
The postgres user's shell is /bin/bash but I don't think this matters
because login isn't being used so /etc/profile.d files aren't being read.
Aliases present no problem. Functions, depending on how they are
written, do.
function somefunction () {....} causes a problem.
function somefunction {....} causes a problem.
somefunction () {....} does not. So there's the bashism - using the
reserved word 'function'.
Interestingly, when using the reserved word function, the presence of ()
changes the error message.
With the use of ():
-sh: 5: /etc/profile.d/test.sh: Syntax error: "(" unexpected (expecting
"fi")
Without the use of ():
-sh: 5: /etc/profile.d/test.sh: function: not found
Definitely would have been much easier to spot the error had we not used ().
Wrapping the function with this worked.
if [ "${BASH-}" ] && [ "$BASH" != "/bin/sh" ]; then
function testfunction
{
echo 'Function from test.sh in /etc/profile.d'
}
fi
Doing the same thing but with the () included did not work. That has me
scratching my head. It should have bypassed those lines entirely.
I'll update the bug reports I've filed in the packages with this
information. I'm not sure there's a good solution for this. It seems to
be such a specific issue. Had I not created a user with a /bin/sh shell
and tried to recreate the function for that user, I'm not sure I would
have twigged that the reserved word function was causing the problem
since the error kept coming back as the ( causing the issue.
Anyway, thanks again for your comments. It spurred me to investigate
further.
An update.
We experienced the same problem with the package tuptime and I filed a
bug report there[1]. I had some comments from the other bug reports
which led me to do some more testing. Here's what I found.
Three packages have had this issue - spamassassin, sa-compile and tuptime.
The user for spamassassin and sa-compile is debian-spamd whose shell is
set to /bin/sh. The user for tuptime is tuptime whose shell is also set
to /bin/sh. No other users on my system use /bin/sh.
Changing the shell to /bin/bash for tuptime for example, eliminates the
error with using 'su -'.
The aliases in the /etc/profile.d files present no problem. Functions,
depending on how they are written, do.
function somefunction () {....} causes a problem.
function somefunction {....} causes a problem.
somefunction () {....} does not. Using the reserved word 'function'
causes /bin/sh to throw an error. Interestingly, when using the reserved
word function, the presence of () changes the error message.
With the use of ():
-sh: 5: /etc/profile.d/test.sh: Syntax error: "(" unexpected (expecting
"fi")
Without the use of ():
-sh: 5: /etc/profile.d/test.sh: function: not found
Definitely would have been much easier to spot the error had we not used ().
So while calling a login shell caused a problem, ultimately, it's
because /bin/sh does not recognise the reserved word 'function'. I'm not
sure there's a good solution for this. It seems to be such a specific
issue. Had I not created a user with a /bin/sh shell and tried to
recreate the function for that user, I'm not sure I would have twigged
that the reserved word function was causing the problem since the error
kept coming back as the "(" causing the issue. At least you're aware if
this is ever reported again.
I hope this provides some clarification and allows you to reproduce the
issue.
In testing, four things fixed the issue:
1. Setting a user's shell to /bin/bash.
2. Using su without "-"
3. removing the reserved word function from the function
4. Wrapping the function with a test for bash but it behaved strangely.
It worked if no "()" was present but if it was present, we had an error.
Unexpected. We're still testing.
We've chosen to remove the reserved word function so now our functions
are simply somefunction () which solves the issue for us.
Thank you.
bash as their login shell. profile.d snippets need to be written to be
at least minimally compatible with the POSIX shell language, if only
via a guard like
if [ -n "$BASH_VERSION" ] || [ -n "$ZSH_VERSION" ]; then
... your bashisms here ...
fi
Debian's default /bin/sh (dash) implements the POSIX shell language and
hardly any more than that.
POSIX shell:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
The superset of POSIX shell that a Debian /bin/sh can be assumed to have:
https://www.debian.org/doc/debian-policy/#scripts
(I think dash implements *slightly* more than what Policy §10.4 requires
it to.)
Yes. Aliases are part of the POSIX shell language, and so are functions defined
with the somefunction () {...} syntax, but the function reserved word in
bash is not a reserved word in the POSIX shell language.
It's unfortunate that the POSIX shell language has so many traps and
pitfalls, but we're several decades too late to influence how it works.
() is a special token in POSIX /bin/sh, so it can affect tokenization
even inside a check for bash. If you're doing something tricky, you can
use a pattern like
if [ -n "$BASH_VERSION" ]; then
. /usr/local/share/my-bashisms
fi
to dodge that, and put the parts that have weird tokenization in
/usr/local/share/my-bashisms.
Welcome to shell scripting. :-(
smcv
Thank you again, Simon, for your concise explanations. We do have a policy on shell usage and the snippets were tested against those shells, but not against /bin/sh. My fault, I'm afraid, since I hadn't considered system users. We'll be adding that to our policy. I've sent the info to the two packages I filed bug reports for. Hopefully, the next time someone runs into the issue, they'll be able to come across these discussions and resolve it quickly.