Bug#889066: lintian should warn if the maintainer scripts include "chown -R" or "chmod -R"

2018-02-06 Thread Daniel Kahn Gillmor
On Mon 2018-02-05 17:55:27 +0100, Raphael Hertzog wrote:
> I'm not quite sure of what colord is vulnerable. #889060 assumes the
> attacker can create arbitrary hardlinks as the "colord" user in
> /var/lib/colord. I don't know colord enough to know if that's the case
> and why that would be the case.
>
> In general, when you have a dedicated user it's because you want to run a
> daemon under that user to restrict its accesses. The interfaces of most
> daemons do not allow end users to create hardlinks/symlinks in the data
> directories of the daemon... hence this chown -R vulnerability is only
> exploitable after having found another vulnerability in the daemon to
> create the hardlinks and/or symlinks.
>
> That makes it much less important as a vulnerability.

The goal here is defense in depth.  If a compromise of colord results in
scrambled color profiles, meh, i can accept it as the risk of running
colord.  If a compromise of colord results in the adversary getting root
on my machine, i'll be pretty unhappy.

   --dkg


signature.asc
Description: PGP signature


Bug#889066: lintian should warn if the maintainer scripts include "chown -R" or "chmod -R"

2018-02-05 Thread Raphael Hertzog
Hi,

On Fri, 02 Feb 2018, Chris Lamb wrote:
> > In my case, I remember having touched many packages with dedicated
> > users created and I expect this tag to have a very high false positive
> > ratio
> 
> Can you make this more concrete? (Or, perhaps, why is colord
> vulnerable but your particular package is not..?)

I'm not quite sure of what colord is vulnerable. #889060 assumes the
attacker can create arbitrary hardlinks as the "colord" user in
/var/lib/colord. I don't know colord enough to know if that's the case
and why that would be the case.

In general, when you have a dedicated user it's because you want to run a
daemon under that user to restrict its accesses. The interfaces of most
daemons do not allow end users to create hardlinks/symlinks in the data
directories of the daemon... hence this chown -R vulnerability is only
exploitable after having found another vulnerability in the daemon to
create the hardlinks and/or symlinks.

That makes it much less important as a vulnerability.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/



Bug#889066: lintian should warn if the maintainer scripts include "chown -R" or "chmod -R"

2018-02-02 Thread Chris Lamb
Hi Raphael,

> Consensus? Has there been a broader discussion on this topic that I
> missed?

Chatter on #debian-devel mostly.

> You could have a checklist

I follow a checklist internally but, as I implied in my previous mail,
using this particular tag is a poor example/representation. :)

A quick grep of "git log -p checks/*.desc" for "+ Please" will show
that tags I add invariably some kind of actionable advice. :)

> Sorry, what CVE are you referring to?

This is via https://bugs.debian.org/889060#5.


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-



Bug#889066: lintian should warn if the maintainer scripts include "chown -R" or "chmod -R"

2018-02-02 Thread Raphael Hertzog
Hi,

On Fri, 02 Feb 2018, Chris Lamb wrote:
> > you do not suggest any alternative (how do I fix change
> > permissions/ownership securely?)
> 
> Indeed, as the consensus is still not clear at this point. Do you
> have any suggestions for such a text?

Consensus? Has there been a broader discussion on this topic that I
missed?

In any case, maybe we could encourage the use of "-h / --no-dereference"
on such calls?

Of if there is no consensus, but multiple suggestions have been made,
then it's probably best to list all the possible solutions that have been
pointed out (maybe usage of systemd's dynamic user feature).

> > Please try to be a bit more restrictive in what new tags you are
> > accepting.
> 
> You seem to be implying this is a pattern. If so, please could you
> provide some other examples so I could understand better?

Well, it seems to me that you could put a bit more thought up-front
when a new tag is added... it seems to me that tags are added and
that sub-sequesent versions often provide a longer explanation
with more context and/or with new ways to not trigger the tag (i.e. that 
do not require adding an override).

That was the case with new-package-should-not-package-python2-module
and dependency-on-python-version-marked-for-end-of-life.

In any case, it's not a big deal, I largely prefer having lintian very
actively maintained with a few mistakes quickly fixed than having no new
checks... but you are still the gatekeeper, Debian developers have lots
of (sometimes weird) desires/wishlists for a tool like lintian and you
should help them better define their checks before merging them.

You could have a checklist:

- Does the long description tell the maintainer how to fix the problem?
  Can it include a reference te some relevant documentation?
- Does the long description gives the rationale why this is a problem
  in the first place?
- Can we have a mechanism to not trigger the tag when the maintainer
  knows that it's a false positive (without adding an explicit override
  tag)?
- Did someone do an estimation of the false positive ratio? Is it
  reasonable?

> This was a judgement call based on the severity of the problem (it,
> after all, had a CVE). Personally I'd rather have a check for such
> an issue that had an incomplete long description than not have the
> check at all. Clearly, this would not apply to a trivial or even a
> normal issue..

Sorry, what CVE are you referring to?

In my case, I remember having touched many packages with dedicated
users created and I expect this tag to have a very high false positive
ratio. If you know this, you might want to acknowledge it in the long
description explaining that you accept the false positives because
of the security impact of any case where nobody took the time to
analyze the security implications (but then again you should help the
maintainer to do his own assessment, what is safe and what is not safe?).

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/



Bug#889066: lintian should warn if the maintainer scripts include "chown -R" or "chmod -R"

2018-02-02 Thread Chris Lamb
Raphael,

> you do not suggest any alternative (how do I fix change
> permissions/ownership securely?)

Indeed, as the consensus is still not clear at this point. Do you
have any suggestions for such a text?

> Please try to be a bit more restrictive in what new tags you are
> accepting.

You seem to be implying this is a pattern. If so, please could you
provide some other examples so I could understand better?

This was a judgement call based on the severity of the problem (it,
after all, had a CVE). Personally I'd rather have a check for such
an issue that had an incomplete long description than not have the
check at all. Clearly, this would not apply to a trivial or even a
normal issue..


Best wishes,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-



Bug#889066: lintian should warn if the maintainer scripts include "chown -R" or "chmod -R"

2018-02-02 Thread Raphael Hertzog
Hi,

On Thu, 01 Feb 2018, Daniel Kahn Gillmor wrote:
> "chown -R" and "chmod -R" are very hard to use safely

Why ?

> some debian maintainer scripts might be tempted to use them to adjust
> file ownership to specific users.  however, those scripts are
> vulnerable to attack on kernels that do not have
> fs.protected_hardlinks=1.

Only if someone has write access to the directories where chown/chmod
are called... which is generally not the cases for directories that
are modified by maintainer scripts (/var/log/foo, /var/lib/foo).

I'm sorry but this tag is going to generate lots of noise and
unhappiness among maintainers because:
1/ you do not suggest any alternative (how do I fix change
   permissions/ownership securely?)
2/ you do not tell them how to ensure that their case is safe or not and
   whether they should just override the tag or not.
3/ I expect the false-positive ratio to be very high

Chris, as a lintian maintainer, I would expect you to ensure that
any tag has actionable data and looking at the commit, clearly this
one doesn't have any. There's no indication on how to go forward
to fix this tag.

Please try to be a bit more restrictive in what new tags you are
accepting.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/



Bug#889066: lintian should warn if the maintainer scripts include "chown -R" or "chmod -R"

2018-02-02 Thread Chris Lamb
tags 889066 + pending
thanks

Fixed in Git, pending upload:

  
https://anonscm.debian.org/git/lintian/lintian.git/commit/?id=e46b47690c6018847c48e05d2162562f16bb87e6


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-



Bug#889066: lintian should warn if the maintainer scripts include "chown -R" or "chmod -R"

2018-02-01 Thread Daniel Kahn Gillmor
Package: lintian
Version: 2.5.72
Severity: wishlist

"chown -R" and "chmod -R" are very hard to use safely, and very
tempting as a sledgehammer to "just make the permissions be what i
want them to be".

some debian maintainer scripts might be tempted to use them to adjust
file ownership to specific users.  however, those scripts are
vulnerable to attack on kernels that do not have
fs.protected_hardlinks=1.

while debian defaults to fs.protected_hardlinks=1, we also want to
safely support people who run:

 * non-debian kernels
 * with some kind of fiddly settings in /etc/sysctl*

And, debian maintscripts are often used as a basis for other distros
or other packaging that doesn't necessarily inherit the protections
that the debian kernel ships with, so making sure our maintscripts are
safe in these other contexts is a worthwhile task.

 --dkg

-- System Information:
Debian Release: buster/sid
  APT prefers testing-debug
  APT policy: (500, 'testing-debug'), (500, 'testing'), (500, 'oldstable'), 
(200, 'unstable-debug'), (200, 'unstable'), (1, 'experimental-debug'), (1, 
'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.14.0-3-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages lintian depends on:
ii  binutils  2.29.1-13
ii  bzip2 1.0.6-8.1
ii  diffstat  1.61-1+b1
ii  dpkg  1.19.0.5
ii  file  1:5.32-1
ii  gettext   0.19.8.1-4
ii  intltool-debian   0.35.0+20060710.4
ii  libapt-pkg-perl   0.1.33
ii  libarchive-zip-perl   1.60-1
ii  libclass-accessor-perl0.51-1
ii  libclone-perl 0.39-1
ii  libdpkg-perl  1.19.0.5
ii  libemail-valid-perl   1.202-1
ii  libfile-basedir-perl  0.07-1
ii  libipc-run-perl   0.96-1
ii  liblist-moreutils-perl0.416-1+b3
ii  libparse-debianchangelog-perl 1.2.0-12
ii  libperl5.24 [libdigest-sha-perl]  5.24.1-7
ii  libperl5.26 [libdigest-sha-perl]  5.26.1-4
ii  libtext-levenshtein-perl  0.13-1
ii  libtimedate-perl  2.3000-2
ii  liburi-perl   1.73-1
ii  libxml-simple-perl2.24-1
ii  libyaml-libyaml-perl  0.69+repack-1
ii  man-db2.7.6.1-4
ii  patchutils0.3.4-2
ii  perl  5.26.1-4
ii  t1utils   1.41-2
ii  xz-utils  5.2.2-1.3

Versions of packages lintian recommends:
pn  libperlio-gzip-perl  

Versions of packages lintian suggests:
pn  binutils-multiarch 
ii  dpkg-dev   1.19.0.5
ii  libhtml-parser-perl3.72-3+b2
ii  libtext-template-perl  1.47-1

-- no debconf information