On Sun, Sep 16, 2012 at 08:10:12PM +0930, Ron wrote: > On Sat, Sep 15, 2012 at 12:53:53PM +0200, Simon Ruderich wrote: >> Some hardening flags (format flags and relro on some archs) are >> still missing because they are not set in debian/rules. > > Do you have some actual evidence of that?
Hello Ron,
Of course. Either look at the build logs on your own, or use a
tool like blhc to check the build logs. The following flag is
missing:
-Werror=format-security
I don't want to paste the result here because it's just a long
list of compiler commands each with the flag missing.
I made a mistake about the relro flags, that was a false
positive, they are applied correctly. Sorry for that.
> Since the patch you attached _removes_ the lines that set these
> things from rules - I'd have thought the obvious incorrectness
> of that statement would be obvious.
The patch removes the manually added flags, but uses
dpkg-buildflags which automatically applies all the default
hardening flags.
> [snip]
>
> The attached patch would appear to do nothing whatsoever except
> make it entirely opaque as to what hardening flags will be applied.
>
> (and if applied to some of my hardened packages, would actually
> _remove_ some hardening flags that are presently applied too)
The general consensus (see [1], and the links in my original
post) is to use dpkg-buildflags as global institution which
decides which flags to use by default (and to handle platform
specific flags in one place, and not in each package), that
includes hardening flags.
You're right, I've missed one flag, sorry for that. I forgot to
re-add -Wformat=2. But all other flags are applied just as
before.
>> This automatically takes care of old versions of dpkg-buildpackage
>> setting different flags, handling noopt and architectures which
>> don't support certain hardening flags (e.g. relro). -g and -O2
>> are also added by default (-O0 with noopt).
>
> Things which were already handled by the code your patch removes ...
True, but dpkg-buildflags handles that for you automatically (man
dpkg-buildflags | less -p noopt), no need to duplicate that in
each package.
> [snip]
>
> If there is something really missing or broken (aside from a false
> positive from hardening-check), then please do explain clearly what
> that is. Otherwise I see no bug here at all, and if there is one,
> the patch that was attached wouldn't appear to fix it anyway.
I admit there was a false positive in blhc (it didn't accept
-Wformat=2 as -Wformat) - which I used to check the logs, and I
failed to look close enough. But the -Werror=format-security flag
is missing and should be added.
Using the flags from dpkg-buildflags makes automatic checks
easier to spot packages with missing flags - and reduces the
burden of you as maintainer to maintain the platform specific
checks.
And it allows other users to rebuild the package with different
flags without modifying it, they can just use dpkg-buildflags to
change the build flags (man dpkg-buildflags | less -p
'^ENVIRONMENT').
> Ron
>
> [We could possibly make speex{enc,dec} pie, but it's not clear that
> they have an attack surface which really makes that worth the effort
> of patching upstream - and dpkg-buildflags isn't helpful for that
> job either ... If we're going to worry about that, then these flags
> should all be supplied by the upstream build itself and it dropped
> from the rules file entirely.]
Regards,
Simon
[1]: https://wiki.debian.org/HardeningWalkthrough
--
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
pgpoXiUADOg1O.pgp
Description: PGP signature
