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 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
