Bug#650536: [new check] test for missing hardening build flags
On Sat, Dec 03, 2011 at 11:20:05AM +0100, Niels Thykier wrote:
> On 2011-12-02 01:33, Kees Cook wrote:
> > 1) With these build tests added, all the other internal lintian tests
> > need to either:
> > a) add the new warnings to their "tags" file, or
> > b) have all their builds adjusted to bring in the dpkg-buildflag
> > defaults.
> > It looks like a pretty large change, but it should be relatively
> > mechanical to accomplish. I would think that "b" above is the better
> > of the two options.
> >
>
> I suspect this is mostly about updating the template rules file +
> correct a few extra tests. As an added benefit it should be fairly
> trivial (if we ignore architecture specificness for a moment).
Okay, sounds good. I'll look at what it'll take to clear all the tests from
these warnings, though I suspect this may come after solving the #2 problem
below...
> > 2) In reality, the tests are arch-dependent. For example, "relro" doesn't
> > exist at all on ia64 hppa avr32, and "stackprotector" doesn't exist on
> > ia64 alpha mips mipsel hppa arm. I think this expectation needs to be
> > built into lintian's invocation of "hardening-check", but that means
> > that the "tags" file in the internal lintian tests suddenly needs to
> > be generated instead of being static. (i.e. on ia64 and hppa, "no-relro"
> > shouldn't show up because it can never happen.)
> >
>
> I proposed the use of the "post_test" sed script here, which hopefully
> should do for the actual test.
> The question is if we will need it only for the hardening test or we
> need it for all the tests with C-binaries. The latter would be very
> inconvient.
>
> Alternatively we can mark the tests architecture dependent. Though in
> that case I would prefer being able to determine the architecture list
> at test time. If we have to manually update the architecture list of
> these tests, they will most likely end up being outdated and even
> inconsistent.
Right, a manual list is exactly what I want to avoid. I think what is
needed is a new feature added to dpkg-buildflags to be able to query the
list of hardening features. Both lintian and hardening-check could use it.
> Accuracy of the checks:
> =======================
>
> In the previous versions of hardening-check, the hardening function
> check were prone to false-positives. If a binary was not using
> protectable-functions at all it would have been tagged as unproected
> (because there no protected functions in the binary). I am very pleased
> to see that appears to have been fixed in the new version.
> As I understand the code, this check should no longer have
> false-positives. However it may have some false-negatives - particuarly
> if protected and unprotected functions are mixed, it will assume the
> binary is protected (in its Lintian output at least).
> This check is not architecture dependent and should be fairly trivial
> to include.
Right. It's better than it was, but it can still produce false-positives.
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. Not all functions are compile-time
checkable, and trying to figure out which are which seems a bit out of
scope at the moment. And because of this, we're forced into the
false-negatives problem. This is why I left it as "possible".
> The stack-protector is architecture dependent (as mentioned above).
> Protected binaries will have a certain symbol (__stack_chk_fail), but
> not all binaries need it[1]. In this case the symbol will be absent
> without the binary is vulnerable, which currently leads to false-positives.
> As I understand [1], the protection is only needed if the binaries
> have an array on the stack. Can we detect the presence (or absence) of
> stack arrays (beyond relying on __stack_chk_fail or analysing the binary
> code)? If we can, we should be able to reduce the false-positives.
We cannot, unfortunately. Or rather, it requires heavy static analysis
of arch-dep assembly for the function preamble (which may change between
compiler versions, optimizations, etc) and use of alloca(), effectively
reverse engineering the asm back to its C form. Additionally, at that
level it may be very hard to distinguish between a character array and
other kinds of arrays (which would not trigger stack-protector). I'm open
to ideas on this, but am currently coming up empty on easy solutions. This
is, again, why I left it as "possible".
> Finally the relro. This is also architecture dependent, but I do not
> know anything about false-positives or false-negatives here. Kees, your
> patch marks it "certain" so I presume we have a low false-positive
> rating here (if we ignore architecture for a moment)?
Correct, relro is a program header on the ELF binary (GNU_RELRO) that
helps direct the dynamic linker. It's either there or it isn't, and if
it isn't, the build flags were not applied. However, it is arch-specific,
so really it's certain if the feature is enabled. This gets me back to
adding some logic to dpkg-buildflags. I'll link to that bug once I've
gotten that written.
> Backporting concerns and output stability:
> ==========================================
>
> Both the FTP-masters and Lintian.d.o needs everything in stable (or
> stable-backports). The hardening-wrapper package appears to be small
> and trivial enough to backport in itself. But there might be a concern
> in terms of the hardening-includes that (if changed) may affect backport
> builds.
> If we can get a reliable backporter for hardening-wrapper as well,
> most of my concerns here covered. On the lintian.d.o side, it means we
> may have to nag DSA for an upgrade of hardening-wrapper every now and then.
Given that dpkg-buildflags won't be backported, perhaps just having lintian
detect the lack of the "what are the hardening features?" query ability in
dpkg-buildflags would be enough to disable the hardening tests in the
backport?
> Jakub Wilk also expressed some concerns with the output of Lintian
> would/could (?) vary with the version of hardening-wrapper. Personally
> I am not too concerned here and I do not believe we have a conflict with
> our "deterministic-output"-clause[2].
> Aside from possible regressions in hardening-check, I suspect the
> accuracy of it will only increase if anything. My greatest concern in
> this area is more that it may break our test-suite (i.e. make Lintian
> FTBFS).
That is my feeling as well. The tests in hardening-check should only
improve.
-Kees
--
Kees Cook @debian.org
Reply to: