Bug#650536: ITM: Please review hardening-support branch to fix #650536 (Was: Re: Bug#650536: update!)
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!)
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!)
On Apr 1, 2012 17:42 Kees Cook k...@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 k...@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 -- 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!)
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!)
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!)
On Apr 1, 2012 09:21 Kees Cook k...@debian.org 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 k...@debian.org 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: ITM: Please review hardening-support branch to fix #650536 (Was: Re: Bug#650536: update!)
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 k...@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). 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 k...@debian.org # # This program is free software; you can redistribute it and/or modify
