Bug#568313: Suggestion: forbid the use of dpkg-statoverride in postinst scripts, except for --list
Brandon
Version: 3.8.4.0
Severity: wishlist
Regarding Debian Policy 10.9.
First, I suggest that Debian Policy require, or at least recommend,
that packages not use dpkg-statoverride to set permissions for files
with static uids and gids. In other words, if it is possible for the
maintainer to set the permissions in the package binary itself, then he
should.
As for setting permissions for files with dynamic ids, debian policy
says that dpkg-statoverride is necessary. This is not true, or at least
misleading. Certainly the post install script should check to make sure
that there isn't any override in place before setting file permissions,
but I think it would be better to set permissions with chown and chmod
than dpkg-statoverride.
In summary, I have 2 similar suggestions:
1) require that permissions be set in the package binary for files with
static uid and gids (this is easier anyway)
2) forbid the use of dpkg-statoverride, except for getting a list of the
preferences already set by the administrator.
-Brandon
Don Armstrong
> First, I suggest that Debian Policy require, or at least recommend,
> that packages not use dpkg-statoverride to set permissions for files
> with static uids and gids. In other words, if it is possible for the
> maintainer to set the permissions in the package binary itself, then
> he should.
What is the rationale for this? What set of packages currently
existing would be instantly buggy if this were the case?
> As for setting permissions for files with dynamic ids, debian policy
> says that dpkg-statoverride is necessary. This is not true, or at
> least misleading. Certainly the post install script should check to
> make sure that there isn't any override in place before setting file
> permissions, but I think it would be better to set permissions with
> chown and chmod than dpkg-statoverride.
This is a bad idea. There's no advantage to using chown and chmod over
dpkg-statoverride. In fact, you have to do more work, because you have
to check all of the things that dpkg-statoverride gets you for free,
like making sure that dpkg-statoverride hasn't previously been set.
It also means that there will be a relatively long time when the
package has been unpacked with the wrong permissions set until the
postinst is called to fix them up.
Don Armstrong
--
Who is thinking this?
I am.
-- Greg Egan _Diaspora_ p38
http://www.donarmstrong.com http://rzlab.ucr.edu
--
To UNSUBSCRIBE, email to debian-bugs-...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listm...@lists.debian.org
Brandon
> > that packages not use dpkg-statoverride to set permissions for files
> > with static uids and gids. In other words, if it is possible for the
> > maintainer to set the permissions in the package binary itself, then
> > he should.
>
> What is the rationale for this?
The whole point of using dpkg-statoverride is to override maintainer
specified file permissions. There is no reason that a package
maintainer would need to "override" the permissions that they
specified. When a maintainer uses dpkg-statoverride, the permissions
are specified in the file attributes and in /var/lib/dpkg/statoverride,
but they only need to specified in one location.
It's ugly to have stuff that I did not specify in my statoverride list.
> What set of packages currently existing would be instantly buggy if
> this were the case?
I bet there will be several. On my system currently, xcdroast and
xsendmail use stat overrides unnecessarily.
> > As for setting permissions for files with dynamic ids, debian policy
> > says that dpkg-statoverride is necessary. This is not true, or at
> > least misleading. Certainly the post install script should check to
> > make sure that there isn't any override in place before setting file
> > permissions, but I think it would be better to set permissions with
> > chown and chmod than dpkg-statoverride.
>
> This is a bad idea. There's no advantage to using chown and chmod over
> dpkg-statoverride.
There are a few advantages. One, is that is less for the system to keep
track of. The statoverride file won't have so much cruft. Two, it is
actually easier for the maintainer. The only reason that any maintainer
would set overrides is because they don't know any better. chown/chmod
works just as well, it can usually be done in the rules file, so some
packages can get rid of their postinst, and you never have to undo it
in postrm (since the file gets removed on purge anyway).
Basically, dpkg-statoverride --add does more than chown/chmod, but
that extra stuff is unneeded cruft that needs to be ignored by the
adminstrator and eventually undone by the maintainer.
> In fact, you have to do more work,
No, less work. See above.
> because you have to check all of the things that dpkg-statoverride
> gets you for free, like making sure that dpkg-statoverride hasn't
> previously been set.
dpkg-statoverride --list <filename> makes sure that dpkg-statoverride
has not been set. And like I said, "except for --list". It's in the bug
title.
> It also means that there will be a relatively long time when the
> package has been unpacked with the wrong permissions set until the
> postinst is called to fix them up.
That doesn't make any sense. In most cases, the permissions would be
set before they would with dpkg-statoverride, because you set it in the
rules file and it is included in the package binary. In the case that
you can't set it in the rules file, it would be set in the postinst
file at the exact same time that the maintainer would otherwise be
using dpkg-statoverride. In other words, there is no situation where
this would be true.
-Brandon
Brandon
that don't use dpkg-statoverride to set permissions, here is the example
from the policy document:
postinst:
for i in /usr/bin/foo /usr/sbin/bar
do
# only do something when no setting exists
if ! dpkg-statoverride --list $i >/dev/null 2>&1
then
#include: debconf processing, question about foo and bar
if [ "$RET" = "true" ] ; then
dpkg-statoverride --update --add sysuser root 4755 $i
fi
fi
done
The corresponding code to remove the override when the package is
purged would be:
for i in /usr/bin/foo /usr/sbin/bar
do
if dpkg-statoverride --list $i >/dev/null 2>&1
then
dpkg-statoverride --remove $i
fi
done
And here is what it would be with chown/chmod instead:
postinst:
for i in /usr/bin/foo /usr/sbin/bar
do
# only do something when no setting exists
if ! dpkg-statoverride --list $i >/dev/null 2>&1
then
#include: debconf processing, question about foo and bar
if [ "$RET" = "true" ] ; then
chown sysuser:root $i
chmod 4755 $i
fi
fi
done
The corresponding code to remove the override when the package is
purged would be: nothing.
-Brandon
Russ Allbery
> As for setting permissions for files with dynamic ids, debian policy
> says that dpkg-statoverride is necessary. This is not true, or at least
> misleading. Certainly the post install script should check to make sure
> that there isn't any override in place before setting file permissions,
> but I think it would be better to set permissions with chown and chmod
> than dpkg-statoverride.
If you set the permissions with chown, aren't they overwritten every time
the package is upgraded and then have to be reset again, leaving windows
on every upgrade when they have the wrong permissions?
--
Russ Allbery (r...@debian.org) <http://www.eyrie.org/~eagle/>
Brandon
> > says that dpkg-statoverride is necessary. This is not true, or at
> > least misleading. Certainly the post install script should check to
> > make sure that there isn't any override in place before setting
> > file permissions, but I think it would be better to set permissions
> > with chown and chmod than dpkg-statoverride.
>
> If you set the permissions with chown, aren't they overwritten every
> time the package is upgraded and then have to be reset again
No. You have to check for overrides first, and only chown/chmod if
there aren't any in place. You have to do this regardless of which
method you use.
> leaving windows on every upgrade when they have the wrong permissions?
I don't know what this means.
-Brandon
Brandon
Oh. I know what this means now. I was thinking of glass windows. Or
maybe MS Windows. There is no point in time where the user would have
the wrong permissions, as long as the package scripts are done
correctly.
-Brandon
martin f krafft
> If you set the permissions with chown, aren't they overwritten every time
> the package is upgraded and then have to be reset again, leaving windows
> on every upgrade when they have the wrong permissions?
Maybe dpkg could be taught to preserve permissions on files that
already exist (i.e. on upgrades)?
--
.''`. martin f. krafft <madduck@d.o> Related projects:
: :' : proud Debian developer http://debiansystem.info
`. `'` http://people.debian.org/~madduck http://vcs-pkg.org
`- Debian - when you have better things to do than fixing systems
Brandon
martin f krafft <mad...@debian.org> wrote:
> also sprach Russ Allbery <r...@debian.org> [2010.02.04.1222 +1300]:
> > If you set the permissions with chown, aren't they overwritten
> > every time the package is upgraded and then have to be reset again,
> > leaving windows on every upgrade when they have the wrong
> > permissions?
>
> Maybe dpkg could be taught to preserve permissions on files that
> already exist (i.e. on upgrades)?
Actually, that is exactly what dpkg-statoverride is for. Administrators
can set overrides, which prevents dpkg from overwriting permissions.
If dpkg just didn't overwrite permissions, then when package
maintainers actually do need to change permissions on files that they
had set with previous packages, they couldn't. gpg, for example, was
set 4755 some time ago, to prevent paging your passwords to disk. Then
some time later, that no longer required root permissions, so gpg was
set to 755. If dpkg never overwrote permissions, then gpg would not
have been able to update these permissions on upgrade.
-Brandon
Russ Allbery
> also sprach Russ Allbery <r...@debian.org> [2010.02.04.1222 +1300]:
>> If you set the permissions with chown, aren't they overwritten every
>> time the package is upgraded and then have to be reset again, leaving
>> windows on every upgrade when they have the wrong permissions?
> Maybe dpkg could be taught to preserve permissions on files that
> already exist (i.e. on upgrades)?
I suppose it's a question of what the best defaults are, but I prefer the
behavior of assuming the ownership and permissions shipped in the package
are correct and the installed files should be modified to match. The case
where I want to preserve the existing permissions is much rarer than the
case where the new version of the package needs to change the permissions
for some reason, I think.
Russ Allbery
>> If you set the permissions with chown, aren't they overwritten every
>> time the package is upgraded and then have to be reset again
> No. You have to check for overrides first, and only chown/chmod if there
> aren't any in place. You have to do this regardless of which method you
> use.
Your second sentence doesn't, so far as I can tell, address my point, so
maybe we're talking past each other. I'm saying that I believe dpkg, when
unpacking the package, will reset the ownership and permissions of any
files contained in that package to match what's in the package, changing
the effect of the chown in the postinst script, unless dpkg-statoverride
was used.
>> leaving windows on every upgrade when they have the wrong permissions?
> I don't know what this means.
If I'm correct about how this currently works, during an upgrade, a file
that's been changed with chown will have its ownership revert to the
ownership specified in the *.deb file during the unpack phase, and then
will have to be changed back to the owner the maintainer desires in the
postinst phase, just as would be the case for the initial install.
If dpkg-statoverride is used instead, I believe that this only happens
during the original install and upgrades then carry over the same
ownership and permissions through the unpack phase.
Stephen Gran
> As for setting permissions for files with dynamic ids, debian policy
> says that dpkg-statoverride is necessary. This is not true, or at least
> misleading.
How do you expect packages to maintain correct ownerships and
permissions on files owned by dynamically created ids in the time
period between unpack and when the postinstall is run without the use
of dpkg-statoverride?
Why do you think dpkg-statoverride was designed solely for a local admin
and not for package use? What gave you that impression and where did
you read it?
Cheers,
--
-----------------------------------------------------------------
| ,''`. Stephen Gran |
| : :' : sg...@debian.org |
| `. `' Debian user, admin, and developer |
| `- http://www.debian.org |
-----------------------------------------------------------------
Brandon
thanks
You're right Russ. In that scenario, there would a be a short period of
time where the permissions would not be set correctly.
I still think that dpkg-statoverride should be forbidden in the case
where the uid and gid are static.
I still don't like the idea of using statoverride for the case where
uid or gid are dynamic, though. If the owner and permissions in the
package binary were set to sane defaults, then that would alleviate
security concerns for that window, but the permissions would still be
wrong. I don't know of another way around that. Using dpkg-statoverride
in postinst is a messy solution, but the only one I can think of that
keeps the permissions correct during that window when the uid or gid
are dynamic.
-Brandon
Russ Allbery
> I still think that dpkg-statoverride should be forbidden in the case
> where the uid and gid are static.
Policy basically already does that, doesn't it? It's not normative (maybe
it should be), but that's how I always read 10.9.1:
Given the above, dpkg-statoverride is essentially a tool for system
administrators and would not normally be needed in the maintainer
scripts. There is one type of situation, though, where calls to
dpkg-statoverride would be needed in the maintainer scripts, and that
involves packages which use dynamically allocated user or group ids.
That's a fair bit short of forbidden in strength, but that's the general
gist of it.
I'd personally be happy to strengthen that to say that you should only use
dpkg-statoverride in maintainer scripts if you're handling dynamic UID/GID
file ownership. I don't see any immediate downside to doing so; other
changes should be handled directly by the permissions set in the *.deb.
> I still don't like the idea of using statoverride for the case where uid
> or gid are dynamic, though. If the owner and permissions in the package
> binary were set to sane defaults, then that would alleviate security
> concerns for that window, but the permissions would still be wrong. I
> don't know of another way around that. Using dpkg-statoverride in
> postinst is a messy solution, but the only one I can think of that keeps
> the permissions correct during that window when the uid or gid are
> dynamic.
Thankfully, this is a relatively rare case, since normally binaries don't
need to be owned by dynamic UIDs or GIDs. It's mostly limited to a small
handful of special setuid or setgid situations.
On my local system, for instance, I have:
windlord:~> dpkg-statoverride --list
root postdrop 2555 /usr/sbin/postdrop
root postdrop 2555 /usr/sbin/postqueue
root mlocate 2755 /usr/bin/mlocate
postfix postdrop 2710 /var/spool/postfix/public
root ssl-cert 710 /etc/ssl/private
root video 2755 /usr/lib/w3m/w3mimgdisplay
which is not horribly long. (And that last one looks incorrect to me on
first glance, but I may be missing some subtlety.)
In retrospect, I suspect we would have been better off if ssl-cert were a
statically-allocated group, but oh well.
--
Russ Allbery (r...@debian.org) <http://www.eyrie.org/~eagle/>
--
martin f krafft
> I suppose it's a question of what the best defaults are, but I prefer the
> behavior of assuming the ownership and permissions shipped in the package
> are correct and the installed files should be modified to match. The case
> where I want to preserve the existing permissions is much rarer than the
> case where the new version of the package needs to change the permissions
> for some reason, I think.
I tend to agree. With that in mind, though, why would one ever
want/need to chmod/chown files under control by dpkg (which are the
only ones for which dpkg-statoverride applies to) *from postinst*?
I can perfectly understand admins wanting to override package
defaults, but not why the package maintainer needs to use
dpkg-statoverride.
In short, I am in favour of forbidding use of dpkg-statoverride by
package maintainers, unless I missed something in the above.
--
.''`. martin f. krafft <madduck@d.o> Related projects:
: :' : proud Debian developer http://debiansystem.info
`. `'` http://people.debian.org/~madduck http://vcs-pkg.org
`- Debian - when you have better things to do than fixing systems
seminars, n.:
from "semi" and "arse", hence, any half-assed discussion.
Russ Allbery
> I tend to agree. With that in mind, though, why would one ever want/need
> to chmod/chown files under control by dpkg (which are the only ones for
> which dpkg-statoverride applies to) *from postinst*? I can perfectly
> understand admins wanting to override package defaults, but not why the
> package maintainer needs to use dpkg-statoverride.
It's for setting the ownership of files to dynamically allocated UIDs or
GIDs which are created in the postinst script. Since you don't know what
UID or GID they'll have, and since they don't exist during the package
unpack phase, there's no way to express that ownership in the *.deb file.
martin f krafft
> In short, I am in favour of forbidding use of dpkg-statoverride by
> package maintainers, unless I missed something in the above.
Further information from IRC:
< madduck> sgran: feel free to slam me down on my latest
reply to #568313 wrt the dynamic uids. ;)
< sgran> madduck: it fails for things like
/var/run/<package> where <package> runs as a dynamically created
user
< sgran> or similar things
< madduck> sgran: /var/run/<package> must not be shipped
< madduck> but /var/spool/postfix might be an example
< sgran> pick a better directory
< madduck> or /var/lib/squid
< sgran> /var/lib/clamav
< madduck> basically /var/*/*
< sgran> etc
< madduck> yeah
< madduck> otoh, I don't see a risk, really…
< madduck> i mean, the permissions are in the package, so
they won't be changed
< madduck> so all that is happening is that the new file is
root.root instead of clamav.clamav
< madduck> but between unpacking and postinst, the daemon
isn't running anyway…
< madduck> well, in most cases.
< sgran> unless of course there are multiple daemons that
all need access to a directory or something
< madduck> so there's actually a window of tightening of
security, not a window of elevated access
< sgran> unless of course the statoverride is to change
perms to 0700 or something
< madduck> sgran: the risk is that one of those daemons
won't be able to access files during that window. i think this is an
acceptable downside of an upgrade, not?
< sgran> no
< sgran> why should an upgrade break a working system?
< madduck> sgran: then the directory should be in the .deb
with 0700, no?
< sgran> have you suddnenly become Md?
< madduck> not break, but briefly suspend services.
< madduck> of course, if this windows causes breakage,
that's a different store
< madduck> but i feel that's one that would have to be
addressed in the daemon, no?
< sgran> I would guess that most daemons don't cope well
with permissions/ownerships being changed out from under them
< sgran> and it's a silly thing to do, since we don't have
to do it
< madduck> for the few seconds it takes between unpack and
postinst, that's okay if it doesn't cause permanent, damage, no?
< sgran> explain again why it's ok to do the wrong thing when it's easy to do the right thing?
< madduck> note how the proposal is about static uids
< madduck> and there it's quite simply the case that we don't need to do it.
< sgran> of course
I misunderstood the original intent and thought it was about static
uids/gids only. Helps to read the message again on the day of
replying.
--
.''`. martin f. krafft <madduck@d.o> Related projects:
: :' : proud Debian developer http://debiansystem.info
`. `'` http://people.debian.org/~madduck http://vcs-pkg.org
`- Debian - when you have better things to do than fixing systems
god is real, unless declared integer.
(dedicated to gabriel gómez)
Don Armstrong
> I can perfectly understand admins wanting to override package
> defaults, but not why the package maintainer needs to use
> dpkg-statoverride.
You need to use dpkg-statoverride when you are using a dynamic uid/gid
and files that you ship need to be setuid/setgid to that uid/gid, or
when you've got a sensible default of not setgid (or similar) but you
want that setting to be easily configurable via debconf.
Don Armstrong
--
"There's no problem so large it can't be solved by killing the user
off, deleting their files, closing their account and reporting their
REAL earnings to the IRS."
-- The B.O.F.H..
http://www.donarmstrong.com http://rzlab.ucr.edu
Don Armstrong
I think this is a holdover from when xcdroast asked a debconf
question; it's probably a bug that that code is still there... file
it!
> xsendmail use stat overrides unnecessarily.
I don't know about an xsendmail package; sendmail itself doesn't do
this. (Or at least, it doesn't any more.)
The reason why I'm asking this question is because before policy is
changed into a must requirement, someone should have found out which
packages will be instantly RC buggy.
Don Armstrong
--
"People selling drug paraphernalia ... are as much a part of drug
trafficking as silencers are a part of criminal homicide."
-- John Brown, DEA Chief