[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

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



On Apr 1, 2012 17:42 "Kees Cook" <kees@debian.org> 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 <kees@debian.org>
> > """
> > 
> > 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




Reply to: