Bug#650536: ITM: Please review hardening-support branch to fix #650536 (Was: Re: Bug#650536: update!)

2012-04-04 Thread Kees Cook 
On Wed, Apr 04, 2012 at 11:45:38PM +0200, Niels Thykier wrote:
>  * Remove bindnow and nopie tags
>- It was not possible to trigger them (not enabled).

I guess this is okay since we'd need to rebuild lintian to get the new
dpkg-buildflags defaults if pie was enabled for an arch.

-Kees

-- 
Kees Cook@debian.org



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: ITM: Please review hardening-support branch to fix #650536 (Was: Re: Bug#650536: update!)

2012-04-04 Thread Niels Thykier 
On 2012-04-01 17:16, Niels Thykier wrote:
> [...]
> 
> I have rebased the branch and it is now available from [1] and I
> intend to merge it into master before we do the 2.5.7 release.
> As mentioned, I have added a new test suite hook[0], which some
> may (or may not) find controversial.
> 
> Assuming no comments/objections, I intend to merge the branch
> into master before the end of Easter.
> 
> ~Niels
> 
> [0] 
> http://anonscm.debian.org/gitweb/?p=users/nthykier/lintian.git;a=commit;h=0ce4b89f515afac59358090174c5dd794e887e1e
> 
> [1] 
> http://anonscm.debian.org/gitweb/?p=users/nthykier/lintian.git;a=shortlog;h=refs/heads/hardening-support-rebased-ee869db
> 
> The "pre-rebase" variant is available as:
> 
> http://anonscm.debian.org/gitweb/?p=users/nthykier/lintian.git;a=shortlog;h=refs/heads/hardening-support
> 

Merged with some changes on the way:
 * Split Kees's second patch, merged + re-ordered some commits.
   - I only merged "minor" things into Kees commits to keep
 authorship fairly consistent.
   - Despite my changes, commits d44806c and e2544b2 are broken to
 some extend (first completely, second only on certain archs).
 - Note d44806c was broken due to infrastrucal changes done on
   my part interleaved with the given patch and not due to Kees.
 * Remove bindnow and nopie tags
   - It was not possible to trigger them (not enabled).
 * test calibration fix
   - I noticed that my original patch fails if the calibration
 removes a "Test-For" tag.

~Niels




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: ITM: Please review hardening-support branch to fix #650536 (Was: Re: Bug#650536: update!)

2012-04-02 Thread Niels Thykier 
On 2012-04-02 18:28, Kees Cook wrote:
> On Mon, Apr 02, 2012 at 11:25:26AM +0200, Niels Thykier wrote:
>> No, At least the "hardening-no-stackprotector" can be triggered in a
>> perfectly safe program where the stack protector is not needed.  We
>> worked around this in the test suite by ensuring there was a stack
>> that needed protection, but I would be very sad to see people do
>> the same in real programs.
> 
> Well, I'd expect people to add an overrides file instead.
> 

Yes, but the tag will only be emitted on architectures that have
stackprotector enabled - hench the override ought to be architecture
specific.  At the moment the user/packagers will have to keep that
architecture list up to date in their override (or ignore it), and I
feel Lintian would do a better job given we have the information available.

>> If I understood you correctly in comment 44, then the protected
>> functions could have the same issue:
>>
>> """
>> For example, if the only function used was gethostname(), it's possible to
>> do compile-time verification to see that the _chk() version isn't needed,
>> so even though the source was built with -D_FORTIFY_SOURCE=2, the
>> unprotected function will be used.
>> """
>>
>> Actually, come to think of it - hardening-no-stackprotector smells a bit
>> like a "wild-guess" since we can only say if it is safe, not if it might
>> be vulnerable.  Though "possible" -> "wild-guess" would change it from
>> a "W" to "I" tag (and therefore not visible by default).
>>   The fortify functions code (in hardening-check) at least makes an
>> educated guess and marks it safe if there are no uses of "possible
>> vulnerable" functions (or if there are mixed uses).  It may still be
>> wrong, but we will not get a false-positive if the binary do not use
>> any of the vulnerable functions.
>>
>> Do anyone have comments on dropping the stack protector to wild-guess?
> 
> Based entirely on the language, it seems like the stack protector warning
> really is "possible". It's not "certain", for sure, but it doesn't seem
> like what I'd think of as a "wild-guess". In practice, if its behavior
> is more like the "wild-guess" checks, then it would make sense to drop
> it to that level.
> 


Maybe - I do not code C if I can avoid it, so I do not know how common
stack arrays are.  This is partly why I tried to invovle others in the
choice.  :)

> Perhaps we should examine some subset of the archive to figure out how
> much noise these checks will add?
> 
> -Kees
> 

Perhaps - unfortunately, the Lintian infrastructure cannot do it for us,
yet (see #660733).

~Niels




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: ITM: Please review hardening-support branch to fix #650536 (Was: Re: Bug#650536: update!)

2012-04-02 Thread Kees Cook 
On Mon, Apr 02, 2012 at 11:25:26AM +0200, Niels Thykier wrote:
> No, At least the "hardening-no-stackprotector" can be triggered in a
> perfectly safe program where the stack protector is not needed.  We
> worked around this in the test suite by ensuring there was a stack
> that needed protection, but I would be very sad to see people do
> the same in real programs.

Well, I'd expect people to add an overrides file instead.

> If I understood you correctly in comment 44, then the protected
> functions could have the same issue:
> 
> """
> For example, if the only function used was gethostname(), it's possible to
> do compile-time verification to see that the _chk() version isn't needed,
> so even though the source was built with -D_FORTIFY_SOURCE=2, the
> unprotected function will be used.
> """
> 
> Actually, come to think of it - hardening-no-stackprotector smells a bit
> like a "wild-guess" since we can only say if it is safe, not if it might
> be vulnerable.  Though "possible" -> "wild-guess" would change it from
> a "W" to "I" tag (and therefore not visible by default).
>   The fortify functions code (in hardening-check) at least makes an
> educated guess and marks it safe if there are no uses of "possible
> vulnerable" functions (or if there are mixed uses).  It may still be
> wrong, but we will not get a false-positive if the binary do not use
> any of the vulnerable functions.
> 
> Do anyone have comments on dropping the stack protector to wild-guess?

Based entirely on the language, it seems like the stack protector warning
really is "possible". It's not "certain", for sure, but it doesn't seem
like what I'd think of as a "wild-guess". In practice, if its behavior
is more like the "wild-guess" checks, then it would make sense to drop
it to that level.

Perhaps we should examine some subset of the archive to figure out how
much noise these checks will add?

-Kees

-- 
Kees Cook@debian.org



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: ITM: Please review hardening-support branch to fix #650536 (Was: Re: Bug#650536: update!)

2012-04-02 Thread Niels Thykier 
On Apr 1, 2012 17:42 "Kees Cook"  wrote:
> On Sun, Apr 01, 2012 at 05:16:38PM +0200, Niels Thykier wrote:
> [...]
> > Kees, btw, are you certain of the copyright statements in
> > collection/hardening-info?
> > 
> > """
> > # The original shell script version of this script is
> > # Copyright (C) 1998 Christian Schwarz
> > #
> > # The objdump version, including support for etch's binutils, is
> > # Copyright (C) 2008 Adam D. Barratt
> > #
> > # This version, a trimmed-down wrapper for hardening-check, is
> > # Copyright (C) 2012 Kees Cook 
> > """
> > 
> > I suspect some of it is copy-waste from collection/objdump-info...
> 
> Yeah, when collection/hardening-info started its life, it was largely
> copied from objdump-info. I wasn't sure if I should drop the copy
> rights
> from there, so I left them. If they shouldn't be there, then we can
> drop them (patch attached).
>

Honestly, I am not too sure about Copyright and all.  I just guessed
it was copy-paste of the file header (I have been known to do that
myself) because I did not see a lot of resemblance with objdump-info.

Maybe some of the others know what the correct thing to do here.  :)

> > > After this patch, the TODO's single remaining item is:
> > > 
> > > + revise tag certainty and description:
> > >   - overrides (we can't do much about FP etc.)
> > > 
> > > What is needed for this? Should I expand the descriptions more? Or
> > > was
> > > there something else?
> > >
> > 
> > It was mostly a reminder to myself to review them and maybe add a
> > "disclaimer" on the false-positives.
> 
> Yeah, I tried to do this in the descriptions already, but maybe it
> could be even more verbose. I wasn't sure to what level to get into
> the details of the potential false positives. I am, of course, open to
> any improvements! :)
> 
> [...]
> > Optimally, Lintian would handle the architecture specific part of
> > these tags better in terms of overrides so people do not have to
> > maintain the archlist for their overrides.  However, that can come
> > in Lintian 2.5.8 or some later time (if at all).
> 
> Wouldn't overrides only be a problem if a maintainer disabled a
> hardening
> feature on a subset of the archs that expected it to be enabled? This
> seems unlikely, or maybe I've misunderstood.
> 

No, At least the "hardening-no-stackprotector" can be triggered in a
perfectly safe program where the stack protector is not needed.  We
worked around this in the test suite by ensuring there was a stack
that needed protection, but I would be very sad to see people do
the same in real programs.

If I understood you correctly in comment 44, then the protected
functions could have the same issue:

"""
For example, if the only function used was gethostname(), it's possible to
do compile-time verification to see that the _chk() version isn't needed,
so even though the source was built with -D_FORTIFY_SOURCE=2, the
unprotected function will be used.
"""

Actually, come to think of it - hardening-no-stackprotector smells a bit
like a "wild-guess" since we can only say if it is safe, not if it might
be vulnerable.  Though "possible" -> "wild-guess" would change it from
a "W" to "I" tag (and therefore not visible by default).
  The fortify functions code (in hardening-check) at least makes an
educated guess and marks it safe if there are no uses of "possible
vulnerable" functions (or if there are mixed uses).  It may still be
wrong, but we will not get a false-positive if the binary do not use
any of the vulnerable functions.

Do anyone have comments on dropping the stack protector to wild-guess?

> > I have rebased the branch and it is now available from [1] and I
> > intend to merge it into master before we do the 2.5.7 release.
> > As mentioned, I have added a new test suite hook[0], which some
> > may (or may not) find controversial.
> > 
> > Assuming no comments/objections, I intend to merge the branch
> > into master before the end of Easter.
> 
> Great! Thank you so much for all the help with this. :)
> 
> -Kees
> 

You are welcome.  :)

~Niels




--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: ITM: Please review hardening-support branch to fix #650536 (Was: Re: Bug#650536: update!)

2012-04-01 Thread Kees Cook 
On Sun, Apr 01, 2012 at 05:16:38PM +0200, Niels Thykier wrote:
> Thanks, I have pushed it to my branch (with a minor change to also update
> the Depends of lintian in d/control).

Great!

> Kees, btw, are you certain of the copyright statements in
> collection/hardening-info?
> 
> """
> # The original shell script version of this script is
> # Copyright (C) 1998 Christian Schwarz
> #
> # The objdump version, including support for etch's binutils, is
> # Copyright (C) 2008 Adam D. Barratt
> #
> # This version, a trimmed-down wrapper for hardening-check, is
> # Copyright (C) 2012 Kees Cook 
> """
> 
> I suspect some of it is copy-waste from collection/objdump-info...

Yeah, when collection/hardening-info started its life, it was largely
copied from objdump-info. I wasn't sure if I should drop the copy rights
from there, so I left them. If they shouldn't be there, then we can
drop them (patch attached).

> > After this patch, the TODO's single remaining item is:
> > 
> > + revise tag certainty and description:
> >   - overrides (we can't do much about FP etc.)
> > 
> > What is needed for this? Should I expand the descriptions more? Or was
> > there something else?
> >
> 
> It was mostly a reminder to myself to review them and maybe add a
> "disclaimer" on the false-positives.

Yeah, I tried to do this in the descriptions already, but maybe it
could be even more verbose. I wasn't sure to what level to get into
the details of the potential false positives. I am, of course, open to
any improvements! :)

> There was also something else, namely making the test suite able to
> handle the architecture specific nature of the hardening tags.  I
> have committed some code to handle this.[0]  Assuming no one objects
> to the approach, I think we are more or less good to go.

Ah! Yeah, very cool. That had totally slipped my mind.

> Optimally, Lintian would handle the architecture specific part of
> these tags better in terms of overrides so people do not have to
> maintain the archlist for their overrides.  However, that can come
> in Lintian 2.5.8 or some later time (if at all).

Wouldn't overrides only be a problem if a maintainer disabled a hardening
feature on a subset of the archs that expected it to be enabled? This
seems unlikely, or maybe I've misunderstood.

> I have rebased the branch and it is now available from [1] and I
> intend to merge it into master before we do the 2.5.7 release.
> As mentioned, I have added a new test suite hook[0], which some
> may (or may not) find controversial.
> 
> Assuming no comments/objections, I intend to merge the branch
> into master before the end of Easter.

Great! Thank you so much for all the help with this. :)

-Kees

-- 
Kees Cook@debian.org
diff --git a/collection/hardening-info b/collection/hardening-info
index b7408be..3e9c6fa 100755
--- a/collection/hardening-info
+++ b/collection/hardening-info
@@ -1,13 +1,7 @@
 #!/usr/bin/perl -w
 # hardening-info -- lintian collection script
 
-# The original shell script version of this script is
-# Copyright (C) 1998 Christian Schwarz
-#
-# The objdump version, including support for etch's binutils, is
-# Copyright (C) 2008 Adam D. Barratt
-#
-# This version, a trimmed-down wrapper for hardening-check, is
+# This trimmed-down wrapper for hardening-check is
 # Copyright (C) 2012 Kees Cook 
 #
 # This program is free software; you can redistribute it and/or modify

Bug#650536: ITM: Please review hardening-support branch to fix #650536 (Was: Re: Bug#650536: update!)

2012-04-01 Thread Niels Thykier 
On Apr 1, 2012 09:21 "Kees Cook"  wrote:
> Hi Niels,
> 
> On Sun, Mar 11, 2012 at 12:16:09AM +0100, Niels Thykier wrote:
> > I have started an unofficial branch[1] to get something more
> > concrete on
> > this.  I decided to rename the tags so they had a common prefix (it
> > simplified the updated to t/scripts/implemented-tags.t).
> 
> Attached is a patch to clean up the remaining tests that still needed
> stack protector and fortify to show up in their binaries.
> 

Hi,

Thanks, I have pushed it to my branch (with a minor change to also update
the Depends of lintian in d/control).

Kees, btw, are you certain of the copyright statements in
collection/hardening-info?

"""
# The original shell script version of this script is
# Copyright (C) 1998 Christian Schwarz
#
# The objdump version, including support for etch's binutils, is
# Copyright (C) 2008 Adam D. Barratt
#
# This version, a trimmed-down wrapper for hardening-check, is
# Copyright (C) 2012 Kees Cook 
"""

I suspect some of it is copy-waste from collection/objdump-info...

> > Last I checked we still have an "outstanding issue" hardening-check
> > using ldd, which I am not certain will work with "foreign" binaries
> > (see
> > comment #39).  I suspect it will mostly affect people who do
> > cross-builds and lintian.d.o[2].
> 
> And this should be taken care of now in hardening-includes 2.0, which
> uses a hard-coded list of libc functions instead of trying to build
> the list at runtime.
> 

Awesome, :)

> After this patch, the TODO's single remaining item is:
> 
> + revise tag certainty and description:
>   - overrides (we can't do much about FP etc.)
> 
> What is needed for this? Should I expand the descriptions more? Or was
> there something else?
>

It was mostly a reminder to myself to review them and maybe add a
"disclaimer" on the false-positives.

There was also something else, namely making the test suite able to
handle the architecture specific nature of the hardening tags.  I
have committed some code to handle this.[0]  Assuming no one objects
to the approach, I think we are more or less good to go.

Optimally, Lintian would handle the architecture specific part of
these tags better in terms of overrides so people do not have to
maintain the archlist for their overrides.  However, that can come
in Lintian 2.5.8 or some later time (if at all).

> 
> Thanks!
> 
> -Kees
> 

I have rebased the branch and it is now available from [1] and I
intend to merge it into master before we do the 2.5.7 release.
As mentioned, I have added a new test suite hook[0], which some
may (or may not) find controversial.

Assuming no comments/objections, I intend to merge the branch
into master before the end of Easter.

~Niels

[0] 
http://anonscm.debian.org/gitweb/?p=users/nthykier/lintian.git;a=commit;h=0ce4b89f515afac59358090174c5dd794e887e1e

[1] 
http://anonscm.debian.org/gitweb/?p=users/nthykier/lintian.git;a=shortlog;h=refs/heads/hardening-support-rebased-ee869db

The "pre-rebase" variant is available as:

http://anonscm.debian.org/gitweb/?p=users/nthykier/lintian.git;a=shortlog;h=refs/heads/hardening-support




--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-04-01 Thread Kees Cook 
Hi Niels,

On Sun, Mar 11, 2012 at 12:16:09AM +0100, Niels Thykier wrote:
> I have started an unofficial branch[1] to get something more concrete on
> this.  I decided to rename the tags so they had a common prefix (it
> simplified the updated to t/scripts/implemented-tags.t).

Attached is a patch to clean up the remaining tests that still needed
stack protector and fortify to show up in their binaries.

> Last I checked we still have an "outstanding issue" hardening-check
> using ldd, which I am not certain will work with "foreign" binaries (see
> comment #39).  I suspect it will mostly affect people who do
> cross-builds and lintian.d.o[2].

And this should be taken care of now in hardening-includes 2.0, which
uses a hard-coded list of libc functions instead of trying to build the
list at runtime.

After this patch, the TODO's single remaining item is:

+ revise tag certainty and description:
  - overrides (we can't do much about FP etc.)

What is needed for this? Should I expand the descriptions more? Or was
there something else?

Thanks!

-Kees

-- 
Kees Cook@debian.org
>From 44917dcc8af48043cb22b104398cfc494b74fbf6 Mon Sep 17 00:00:00 2001
From: Kees Cook 
Date: Sat, 31 Mar 2012 23:59:28 -0700
Subject: [PATCH] Update for latest hardening-check

Additionally, clean up remaining hardening warnings in the tests.

Signed-off-by: Kees Cook 
---
 collection/hardening-info  |7 ---
 debian/changelog   |7 +--
 debian/control |2 +-
 .../debian/hardening-trigger.h |6 ++
 t/tests/binaries-embedded-libs/debian/libbz2.c |1 +
 t/tests/binaries-embedded-libs/debian/libexpat.c   |1 +
 t/tests/binaries-embedded-libs/debian/libjpeg.c|1 +
 t/tests/binaries-embedded-libs/debian/libm.c   |1 +
 t/tests/binaries-embedded-libs/debian/libmagic.c   |1 +
 .../binaries-embedded-libs/debian/libopenjpeg.c|1 +
 t/tests/binaries-embedded-libs/debian/libpcre3.c   |1 +
 t/tests/binaries-embedded-libs/debian/libpng.c |1 +
 t/tests/binaries-embedded-libs/debian/libsqlite.c  |1 +
 t/tests/binaries-embedded-libs/debian/libtiff.c|1 +
 t/tests/binaries-embedded-libs/debian/libxml2.c|1 +
 t/tests/binaries-embedded-libs/debian/zlib.c   |1 +
 .../debian/basic.c |   10 ++
 .../debian/basic.c |   10 ++
 .../debian/basic.c |   10 ++
 t/tests/binaries-missing-depends/debian/basic.c|   10 ++
 t/tests/binaries-multiarch-same/debian/basic.c |   10 ++
 .../binaries-multiarch-wrong-dir/debian/basic.c|   10 ++
 t/tests/binaries-multiarch/debian/basic.c  |   10 ++
 t/tests/binaries-spelling/debian/basic.c   |   10 ++
 t/tests/binaries-unsafe-open/debian/dummy.c|   10 ++
 t/tests/strings-elf-detection/debian/Makefile  |7 +++
 t/tests/strings-elf-detection/debian/debian/rules  |3 +--
 t/tests/strings-elf-detection/debian/true.c|   17 +
 28 files changed, 135 insertions(+), 16 deletions(-)
 create mode 100644 t/tests/binaries-embedded-libs/debian/hardening-trigger.h
 create mode 100644 t/tests/strings-elf-detection/debian/Makefile
 create mode 100644 t/tests/strings-elf-detection/debian/true.c

diff --git a/collection/hardening-info b/collection/hardening-info
index 6692c96..b7408be 100755
--- a/collection/hardening-info
+++ b/collection/hardening-info
@@ -44,13 +44,6 @@ if ( -e "$dir/hardening-info" ) {
 open OUT, '>', "$dir/hardening-info"
 or fail("cannot open hardening-info: $!");
 
-# If we're running inside the Lintian test suite itself, we need to avoid
-# all the tests except the "binaries-hardening" test.
-exit 0
-if (defined $ENV{'LINTIAN_INTERNAL_TESTSUITE'} and
-$ENV{'LINTIAN_INTERNAL_TESTSUITE'} eq "1" and
-$dir !~ m|/binaries-hardening/binaries-hardening_1.0_.*_binary$|);
-
 # Prepare to examine the file tree.
 chdir ("$dir/unpacked")
 or fail("unable to chdir to unpacked: $!");
diff --git a/debian/changelog b/debian/changelog
index 1a71129..42224a0 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,13 +1,8 @@
 lintian (2.5.6) UNRELEASED; urgency=low
 
   * BRANCH TODO:
-+ handle checking of binaries from foreign architectures:
-  - hardening-check uses ldd
 + revise tag certainty and description:
   - overrides (we can't do much about FP etc.)
-+ test suite clean up:
-  - remove test-suite check in coll/hardening-info
-  - fix broken tests
 
   * checks/*:
 + [NT] Simplified some bit operations done on file permissions.
@@ -58,7 +53,7 @@ lintian (2.5.6) UNRELEASED; urgency=low
   * checks/binaries, collector/hardening-info*:
 + Add ELF hardening checks.  (Closes: 650

Bug#650536: update!

2012-03-11 Thread Niels Thykier 
On 2012-03-11 13:37, Kees Cook wrote:
> On Sun, Mar 11, 2012 at 12:16:09AM +0100, Niels Thykier wrote:
>> I have bumped the debhelper standard test suite to use compat 9 by
>> default.  I doubt it will fix all the failures we saw, but at least the
>> standard flags are enabled by default.
> 
> When I was playing with it, this solved a lot but not all of them. Doesn't
> this pose an unbackportable change though? I didn't think compat 9
> existed in Squeeze.
> 

It does not, but debhelper 9 has been backported already, so we can rely
on it.

In fact, we already needed debhelper 9 due to t/tests/debhelper-dh-exec,
but I apparently forgot to bump the depends back then...

> [...]
>>> - build internal hardening test for all archs (hook to generate tags file)
>>> - fix other lintian internal tests to work with hardening check
>>
>> This part still needs some work though.
>>
>> I suspect it might be a good idea to try the test suite on some
>> different architectures at some point.  These
> 
> Cool, I'll spend some time on the branch getting any stragglers building
> correctly.
> 

Much appreciated.

>> Last I checked we still have an "outstanding issue" hardening-check
>> using ldd, which I am not certain will work with "foreign" binaries (see
>> comment #39).  I suspect it will mostly affect people who do
>> cross-builds and lintian.d.o[2].
> 
> Yeah, I was just starting to notice this. Inspired by the data file idea, I
> think I might do the same for hardening-check and have it build the list of
> functions at build-time. I can check if a binary is using libc without
> running ldd, and I only needed ldd to generate the function list dynamically.
> If it's static, things are faster and more portable. It'll just need updating
> from time to time when anything major happens with eglibc.
> 
> -Kees
> 

Sounds good.  :)

~Niels




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-11 Thread Kees Cook 
On Sun, Mar 11, 2012 at 12:16:09AM +0100, Niels Thykier wrote:
> I have bumped the debhelper standard test suite to use compat 9 by
> default.  I doubt it will fix all the failures we saw, but at least the
> standard flags are enabled by default.

When I was playing with it, this solved a lot but not all of them. Doesn't
this pose an unbackportable change though? I didn't think compat 9
existed in Squeeze.

> > - make lintian work for wheezy (but disable internal tests for hardening)
> > - backport hardening check to work on squeeze
> 
> I just finished a patch that solves these two.  I made a data file that
> is trivial to regenerate/update using private/refresh-archs (requiring
> dpkg-dev from experimental or newer).  It also removes the need for
> dpkg-dev at runtime.  :)
>   I know you had some concerns about using data files, but I honestly
> think they will be the easiest way to solve the backportability problem.
>  Since we can use dpkg-buildflags to regenerate it automatically, it
> should be trivial to keep it in sync.

I think you've convinced me about the data files. The options shouldn't be
changing terribly frequently.

> It also solves one of the test errors as dpkg-buildflags does not handle
> invalid architectures too well.

True.

> > - build internal hardening test for all archs (hook to generate tags file)
> > - fix other lintian internal tests to work with hardening check
> 
> This part still needs some work though.
> 
> I suspect it might be a good idea to try the test suite on some
> different architectures at some point.  These

Cool, I'll spend some time on the branch getting any stragglers building
correctly.

> Last I checked we still have an "outstanding issue" hardening-check
> using ldd, which I am not certain will work with "foreign" binaries (see
> comment #39).  I suspect it will mostly affect people who do
> cross-builds and lintian.d.o[2].

Yeah, I was just starting to notice this. Inspired by the data file idea, I
think I might do the same for hardening-check and have it build the list of
functions at build-time. I can check if a binary is using libc without
running ldd, and I only needed ldd to generate the function list dynamically.
If it's static, things are faster and more portable. It'll just need updating
from time to time when anything major happens with eglibc.

-Kees

-- 
Kees Cook@debian.org



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-10 Thread Niels Thykier 
On 2012-03-06 20:26, Kees Cook wrote:
> Hi Russ,
> 
> On Tue, Mar 06, 2012 at 10:08:31AM -0800, Russ Allbery wrote:
>> Kees Cook  writes:
>>

Hi,

I have started an unofficial branch[1] to get something more concrete on
this.  I decided to rename the tags so they had a common prefix (it
simplified the updated to t/scripts/implemented-tags.t).

>>> This was the big problem. I spent a lot of time trying to see how bad it
>>> would be to fix every build in the testsuite to DTRT with respect to
>>> dpkg-buildflags, but it was a losing battle. Or, at least, a tedious
>>> battle.  Ultimately I decided it was better to just have the hardening
>>> checker disable itself in the face of the other tests.
>>
>>> I'm open to ideas for this part, but a lot of the test builds don't pass
>>> all the needed flags, or hard code flags, etc etc. Changing the compat
>>> level worked for many of the failures, but not all and left about 30
>>> that still needed to be changed by hand. If it's important to do this
>>> strictly correct, I can, it'll just take me a while.
>>
>> The general intent of the Lintian test suite is that the packages it
>> produces should be Lintian-clean except for the tags that the package is
>> specifically testing (or others that are unavoidable for some reason).  So
>> when new requirements for Debian packages are added, as a general rule of
>> thumb we want to update the test suite so that it meets those requirements
>> except for those tests that are testing Lintian's tags for those
>> requirements.
>>

I have bumped the debhelper standard test suite to use compat 9 by
default.  I doubt it will fix all the failures we saw, but at least the
standard flags are enabled by default.

>> So, this is work that does need to be done eventually, I think.  That
>> doesn't mean it has to be a blocker for getting the tag into Lintian,
>> though.
> 
> Okay. In that case, I think the work needs to be broken into several pieces:
> 
> - make lintian work for wheezy (but disable internal tests for hardening)
> - backport hardening check to work on squeeze

I just finished a patch that solves these two.  I made a data file that
is trivial to regenerate/update using private/refresh-archs (requiring
dpkg-dev from experimental or newer).  It also removes the need for
dpkg-dev at runtime.  :)
  I know you had some concerns about using data files, but I honestly
think they will be the easiest way to solve the backportability problem.
 Since we can use dpkg-buildflags to regenerate it automatically, it
should be trivial to keep it in sync.

It also solves one of the test errors as dpkg-buildflags does not handle
invalid architectures too well.

> - build internal hardening test for all archs (hook to generate tags file)
> - fix other lintian internal tests to work with hardening check

This part still needs some work though.

I suspect it might be a good idea to try the test suite on some
different architectures at some point.  These

> 
> -Kees
> 

Last I checked we still have an "outstanding issue" hardening-check
using ldd, which I am not certain will work with "foreign" binaries (see
comment #39).  I suspect it will mostly affect people who do
cross-builds and lintian.d.o[2].
  If I recall, hardening-check falls back to assuming the binary is
"safe" for the checks needing ldd data, so we should get false-negatives
rather than false-positives in this case.


~Niels

[1]
http://anonscm.debian.org/gitweb/?p=users/nthykier/lintian.git;a=shortlog;h=refs/heads/hardening-support

[2] lintian.d.o is amd64, but it checks i386 packages.




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-06 Thread Kees Cook 
On Tue, Mar 06, 2012 at 11:36:42AM -0800, Russ Allbery wrote:
> Kees Cook  writes:
> 
> > Okay. In that case, I think the work needs to be broken into several pieces:
> 
> > - make lintian work for wheezy (but disable internal tests for hardening)
> 
> A better way than disabling it might be to just list the expected tags
> until the test cases have been revised to not issue those tags.

It changes based on architecture. ;) Wheee

-- 
Kees Cook@debian.org



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-06 Thread Russ Allbery 
Kees Cook  writes:

> Okay. In that case, I think the work needs to be broken into several pieces:

> - make lintian work for wheezy (but disable internal tests for hardening)

A better way than disabling it might be to just list the expected tags
until the test cases have been revised to not issue those tags.

-- 
Russ Allbery (r...@debian.org)   



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-06 Thread Kees Cook 
Hi Russ,

On Tue, Mar 06, 2012 at 10:08:31AM -0800, Russ Allbery wrote:
> Kees Cook  writes:
> 
> > This was the big problem. I spent a lot of time trying to see how bad it
> > would be to fix every build in the testsuite to DTRT with respect to
> > dpkg-buildflags, but it was a losing battle. Or, at least, a tedious
> > battle.  Ultimately I decided it was better to just have the hardening
> > checker disable itself in the face of the other tests.
> 
> > I'm open to ideas for this part, but a lot of the test builds don't pass
> > all the needed flags, or hard code flags, etc etc. Changing the compat
> > level worked for many of the failures, but not all and left about 30
> > that still needed to be changed by hand. If it's important to do this
> > strictly correct, I can, it'll just take me a while.
> 
> The general intent of the Lintian test suite is that the packages it
> produces should be Lintian-clean except for the tags that the package is
> specifically testing (or others that are unavoidable for some reason).  So
> when new requirements for Debian packages are added, as a general rule of
> thumb we want to update the test suite so that it meets those requirements
> except for those tests that are testing Lintian's tags for those
> requirements.
> 
> So, this is work that does need to be done eventually, I think.  That
> doesn't mean it has to be a blocker for getting the tag into Lintian,
> though.

Okay. In that case, I think the work needs to be broken into several pieces:

- make lintian work for wheezy (but disable internal tests for hardening)
- build internal hardening test for all archs (hook to generate tags file)
- fix other lintian internal tests to work with hardening check
- backport hardening check to work on squeeze

-Kees

-- 
Kees Cook@debian.org



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-06 Thread Russ Allbery 
Kees Cook  writes:
> On Tue, Mar 06, 2012 at 06:36:07PM +0100, Niels Thykier wrote:

>> Lintian.d.o, ftp-master.d.o and potentionally a lot of developers run
>> Lintian on a Debian/Squeeze.  I suspect a static data file is better
>> than disabling it for Squeeze.

> Oh, you mean they'll run a squeeze lintian against a wheezy deb?

They will run the most recent Lintian (possibly backported, possibly -- in
the case of lintian.d.o -- run from Git) on a squeeze system against a
wheezy deb.  That's how the entirety of lintian.d.o works.

-- 
Russ Allbery (r...@debian.org)   



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-06 Thread Kees Cook 
On Tue, Mar 06, 2012 at 06:36:07PM +0100, Niels Thykier wrote:
> On 2012-03-06 01:58, Kees Cook wrote:
> > Right -- though I have no way around this. All the pieces needed for
> > these checks come from the new dpkg-buildflags. Perhaps the hardening
> > check can be disabled for the backport, since it's rather meaningless
> > for stable anyway?
> 
> Lintian.d.o, ftp-master.d.o and potentionally a lot of developers run
> Lintian on a Debian/Squeeze.  I suspect a static data file is better
> than disabling it for Squeeze.

Oh, you mean they'll run a squeeze lintian against a wheezy deb?

If so, yeah, that is troublesome.

-- 
Kees Cook@debian.org



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-06 Thread Russ Allbery 
Kees Cook  writes:

> This was the big problem. I spent a lot of time trying to see how bad it
> would be to fix every build in the testsuite to DTRT with respect to
> dpkg-buildflags, but it was a losing battle. Or, at least, a tedious
> battle.  Ultimately I decided it was better to just have the hardening
> checker disable itself in the face of the other tests.

> I'm open to ideas for this part, but a lot of the test builds don't pass
> all the needed flags, or hard code flags, etc etc. Changing the compat
> level worked for many of the failures, but not all and left about 30
> that still needed to be changed by hand. If it's important to do this
> strictly correct, I can, it'll just take me a while.

The general intent of the Lintian test suite is that the packages it
produces should be Lintian-clean except for the tags that the package is
specifically testing (or others that are unavoidable for some reason).  So
when new requirements for Debian packages are added, as a general rule of
thumb we want to update the test suite so that it meets those requirements
except for those tests that are testing Lintian's tags for those
requirements.

So, this is work that does need to be done eventually, I think.  That
doesn't mean it has to be a blocker for getting the tag into Lintian,
though.

-- 
Russ Allbery (r...@debian.org)   



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-06 Thread Niels Thykier 
On 2012-03-06 01:58, Kees Cook wrote:
> On Mon, Mar 05, 2012 at 11:29:46AM +0100, Niels Thykier wrote:
>> On 2012-03-05 04:47, Kees Cook wrote:
>>> - It requires the lastest dpkg-dev (still in experimental) to get
>>>   the dpkg-buildflags that supports --query-features.
>>
> [...]
>> The second problem is that the given version of dpkg-dev is not in
>> stable[1] and (as I recall) the backport FTP masters were not too happy
>> with the last backport.
>>
>> [1] It is not in unstable either, but at this point I am more concerned
>> with getting it in stable.
>>
> 
> Right -- though I have no way around this. All the pieces needed for
> these checks come from the new dpkg-buildflags. Perhaps the hardening
> check can be disabled for the backport, since it's rather meaningless
> for stable anyway?
> 

Lintian.d.o, ftp-master.d.o and potentionally a lot of developers run
Lintian on a Debian/Squeeze.  I suspect a static data file is better
than disabling it for Squeeze.

> [...]
> 
> -Kees
> 

Skipping the "testing issue" for now.  :)

~Niels




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-05 Thread Kees Cook 
On Mon, Mar 05, 2012 at 11:29:46AM +0100, Niels Thykier wrote:
> On 2012-03-05 04:47, Kees Cook wrote:
> > - It requires the lastest dpkg-dev (still in experimental) to get
> >   the dpkg-buildflags that supports --query-features.
> 
> Unfortunately I see two issues here.  First, we have been asked to avoid
> the unconditional dpkg-dev dependency (see #626476).  Perhaps we can use
> libdpkg-perl as a fall-back in this case (like we do in
> collection/unpacked).

Hrm, well, as long as dpkg-buildflags is the right one, I don't care
what the Depends say. ;)

> The second problem is that the given version of dpkg-dev is not in
> stable[1] and (as I recall) the backport FTP masters were not too happy
> with the last backport.
> 
> [1] It is not in unstable either, but at this point I am more concerned
> with getting it in stable.
> 

Right -- though I have no way around this. All the pieces needed for
these checks come from the new dpkg-buildflags. Perhaps the hardening
check can be disabled for the backport, since it's rather meaningless
for stable anyway?

> > - The hardening checker checks if it is running as part of the
> >   internal test suite, so that it is disabled for all tests except
> >   its own, since the bulk of the internal tests do not build with
> >   hardening flags, and only for i386 and amd64 since there isn't
> >   a sane way to generate the "tags" file on the fly for a test.
> > 
> 
> To be honest I do not like the idea of Lintian checks/collections
> behaving differently during tests.
>   I suppose we could a make """sane way to generate the "tags" file""".
>  We already have several hooks in the test suite, adding another one
> should not be a great issue.

I could write a hook the generate the tags file on the fly, but that
only handles the per-arch limitation of the internal test for the
hardening checker.

> Though, we only want hardening tags emitted in a selected few tests...

This was the big problem. I spent a lot of time trying to see how bad
it would be to fix every build in the testsuite to DTRT with respect
to dpkg-buildflags, but it was a losing battle. Or, at least, a tedious
battle.  Ultimately I decided it was better to just have the hardening
checker disable itself in the face of the other tests.

I'm open to ideas for this part, but a lot of the test builds don't pass
all the needed flags, or hard code flags, etc etc. Changing the compat
level worked for many of the failures, but not all and left about 30
that still needed to be changed by hand. If it's important to do this
strictly correct, I can, it'll just take me a while.

-Kees

-- 
Kees Cook@debian.org



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-05 Thread Niels Thykier 
On 2012-03-05 04:47, Kees Cook wrote:
> Okay, here's the latest version. Some notes:
> 

Hi,

Thanks for the update.

> - It requires the lastest dpkg-dev (still in experimental) to get
>   the dpkg-buildflags that supports --query-features.
> 

Unfortunately I see two issues here.  First, we have been asked to avoid
the unconditional dpkg-dev dependency (see #626476).  Perhaps we can use
libdpkg-perl as a fall-back in this case (like we do in
collection/unpacked).

The second problem is that the given version of dpkg-dev is not in
stable[1] and (as I recall) the backport FTP masters were not too happy
with the last backport.

[1] It is not in unstable either, but at this point I am more concerned
with getting it in stable.

> - The hardening checker only expects the hardened features that are
>   defaulted on for the architecture of the package it is examining.
> 

Good :)

> - The hardening checker checks if it is running as part of the
>   internal test suite, so that it is disabled for all tests except
>   its own, since the bulk of the internal tests do not build with
>   hardening flags, and only for i386 and amd64 since there isn't
>   a sane way to generate the "tags" file on the fly for a test.
> 

To be honest I do not like the idea of Lintian checks/collections
behaving differently during tests.
  I suppose we could a make """sane way to generate the "tags" file""".
 We already have several hooks in the test suite, adding another one
should not be a great issue.

Though, we only want hardening tags emitted in a selected few tests...

> Doing manual testing shows that building, for example, the "hello"
> package as-is triggers appropriate warnings, and when I fix the "hello"
> package to import the dpkg-buildflags correctly, the lintian warnings
> go away. :)
> 
> -Kees
> 

~Niels




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#650536: update!

2012-03-04 Thread Kees Cook 
Okay, here's the latest version. Some notes:

- It requires the lastest dpkg-dev (still in experimental) to get
  the dpkg-buildflags that supports --query-features.

- The hardening checker only expects the hardened features that are
  defaulted on for the architecture of the package it is examining.

- The hardening checker checks if it is running as part of the
  internal test suite, so that it is disabled for all tests except
  its own, since the bulk of the internal tests do not build with
  hardening flags, and only for i386 and amd64 since there isn't
  a sane way to generate the "tags" file on the fly for a test.

Doing manual testing shows that building, for example, the "hello"
package as-is triggers appropriate warnings, and when I fix the "hello"
package to import the dpkg-buildflags correctly, the lintian warnings
go away. :)

-Kees

-- 
Kees Cook@debian.org
>From 550f6f0ef62a965118dc510f59329e66d73668d7 Mon Sep 17 00:00:00 2001
From: Kees Cook 
Date: Thu, 1 Dec 2011 16:04:00 -0800
Subject: [PATCH] Add initial lintian checks for ELF hardening

This adds the "hardening-info" collector which depends on the
new "--lintian" mode of the "hardening-check" tool found in the
"hardening-includes" package.

Signed-off-by: Kees Cook 
---
 checks/binaries|   10 +++
 checks/binaries.desc   |   53 +++-
 collection/hardening-info  |   98 
 collection/hardening-info.desc |7 ++
 debian/changelog   |5 +-
 debian/control |5 +-
 lib/Lintian/Collect/Binary.pm  |   27 
 t/tests/binaries-hardening/debian/Makefile |   18 +
 t/tests/binaries-hardening/debian/hello.c  |   17 +
 t/tests/binaries-hardening/desc|9 +++
 t/tests/binaries-hardening/tags|5 ++
 11 files changed, 251 insertions(+), 3 deletions(-)
 create mode 100755 collection/hardening-info
 create mode 100644 collection/hardening-info.desc
 create mode 100644 t/tests/binaries-hardening/debian/Makefile
 create mode 100644 t/tests/binaries-hardening/debian/hello.c
 create mode 100644 t/tests/binaries-hardening/desc
 create mode 100644 t/tests/binaries-hardening/tags

diff --git a/checks/binaries b/checks/binaries
index 0e3c956..f3d01fd 100644
--- a/checks/binaries
+++ b/checks/binaries
@@ -1,6 +1,7 @@
 # binaries -- lintian check script -*- perl -*-
 
 # Copyright (C) 1998 Christian Schwarz and Richard Braakman
+# Copyright (C) 2012 Kees Cook
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -437,6 +438,15 @@ foreach my $file (@{$info->sorted_file_info}) {
 tag 'program-not-linked-against-libc', $file;
 }
 }
+
+# Check for missing hardening characteristics. This currently
+# handles the following checks:
+# no-relro no-fortify-functions no-stackprotector no-bindnow no-pie
+if (exists($info->hardening_info->{$file})) {
+foreach my $tag (@{$info->hardening_info->{$file}}) {
+tag $tag, $file;
+}
+}
 }
 }
 
diff --git a/checks/binaries.desc b/checks/binaries.desc
index 4e0ad1e..f269c70 100644
--- a/checks/binaries.desc
+++ b/checks/binaries.desc
@@ -2,7 +2,7 @@ Check-Script: binaries
 Author: Christian Schwarz 
 Abbrev: bin
 Type: binary, udeb
-Needs-Info: objdump-info, file-info, strings, index
+Needs-Info: hardening-info, objdump-info, file-info, strings, index
 Info: This script checks binaries and object files for bugs.
 
 Tag: arch-independent-package-contains-binary-or-object
@@ -299,3 +299,54 @@ Info: This package provides an OCaml bytecode executable linked with a
  custom runtime. Such executables cannot be stripped and require
  special care. Their usage is deprecated in favour of shared libraries
  for C stubs (dll*.so).
+
+Tag: no-stackprotector
+Severity: normal
+Certainty: possible
+Info: This package provides an ELF binary that lacks the stack protector
+ function __stack_chk_fail. Either there are no character arrays used
+ on the stack of any routines, or the package was not built with the
+ default Debian compiler flags defined by dpkg-buildflags. If built
+ using dpkg-buildflags directly, be sure to import CFLAGS
+ and/or CXXFLAGS.
+Ref: http://wiki.debian.org/Hardening
+
+Tag: no-fortify-functions
+Severity: normal
+Certainty: possible
+Info: This package provides an ELF binary that lacks the use of fortified
+ libc functions. Either there are no potentially unfortified functions
+ called by any routines, all unfortified calls have already been fully
+ validated at compile-time, or the package was not built with the default
+ Debian compiler flags defined by dpkg-buildflags. If built using
+ dpkg-buildflags directly, be sure to import CPPFLAGS.
+Ref: