[RFC PATCH] dpkg-buildflags: Switch to -fstack-protector-strong

2014-06-24 Thread Romain Francoise
Hi,

GCC 4.9 supports a new stack protector implementation, enabled via the
-fstack-protector-strong flag, which provides a better balance between
security and performance than the default implementation that we're
currently using. This new flag is already used by Fedora 20 and
ChromeOS. See the following for more information:

 https://lwn.net/Articles/584225/
 http://www.outflux.net/blog/archives/2014/01/27/fstack-protector-strong/
 https://fedorahosted.org/fesco/ticket/1128

The Security Team has expressed interest in switching dpkg-buildflags
over to this new flag in Debian for jessie, now that GCC 4.9 is the
default compiler on all release architectures. In order to see the
impact on the archive, David Suárez did a full rebuild on EC2 with a
patched dpkg-dev which emits the new flag.

There are only 16 new failures, which can be categorized as follows:

* explicitly build-depends on and uses gcc/g++ 4.8, which doesn't
  understand -fstack-protector-strong:
  - ccbuild 2.0.6-2.1
  - chromium-browser 35.0.1916.153-2
  - contextfree 3.0.5+dfsg1-2.1
  - flexc++ 2.01.00-1
  - gpg-remailer 3.00.02-1
  - higan 094-4
  - llvm-toolchain-snapshot 1:3.5~svn209039-2
  - openimageio 1.4.9~dfsg0-1 (already fixed in -2)
  - oxref 1.00.01-1
  - spek 0.8.2-3.1
  - webkitgtk 2.4.3-2

* explicitly build-depends on and uses gcc 4.6:
  - estic 1.61-20.1 (#747980)

* explicitly build-depends on and uses Clang 3.4:
  - feel++ 1:0.98.0-final-1

* false positives:
  - gcc-4.7 4.7.4-1 (checks that dpkg-dev is 'ii')
  - seqan 1.4.1-3 (attempts to disable the stack protector using sed)

* needs test suite upgrade for -fstack-protector-strong:
  - hardening-wrapper 2.5

See http://aws-logs.debian.net/ftbfs-logs/buildflags/ for the full
results and build logs.

As the number of build failures is low, I think it's safe to simply
switch the default flag emitted by dpkg-buildflags and file bugs against
the above packages to ask the maintainers to disable the stack protector
or filter out/replace the new flag if they really can't upgrade to GCC
4.9.

So here is a prospective patch which changes dpkg-buildflags to emit the
new flag for all architectures known to use GCC 4.9 as of today. Let me
know if this looks workable for you.


diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm
index c5020dc..4e19752 100644
--- a/scripts/Dpkg/Vendor/Debian.pm
+++ b/scripts/Dpkg/Vendor/Debian.pm
@@ -92,6 +92,7 @@ sub add_hardening_flags {
relro => 1,
bindnow => 0,
 );
+my $use_stackprotector_strong = 1;
 
 # Adjust features based on Maintainer's desires.
 my $opts = Dpkg::BuildOptions->new(envvar => 'DEB_BUILD_MAINT_OPTIONS');
@@ -129,6 +130,12 @@ sub add_hardening_flags {
#   compiler supports it incorrectly (leads to SEGV)
$use_feature{stackprotector} = 0;
 }
+if ($arch =~ /^(?:m68k|or1k|powerpcspe|sh4|x32)$/) {
+   # "Strong" stack protector disabled on m68k, or1k, powerpcspe, sh4, x32.
+   #   It requires GCC 4.9 and these archs are still using 4.8 as of
+   #   gcc-defaults 1.128.
+   $use_stackprotector_strong = 0;
+}
 if ($cpu =~ /^(?:ia64|hppa|avr32)$/) {
# relro not implemented on ia64, hppa, avr32.
$use_feature{relro} = 0;
@@ -161,13 +168,23 @@ sub add_hardening_flags {
 
 # Stack protector
 if ($use_feature{stackprotector}) {
-   $flags->append('CFLAGS', '-fstack-protector --param=ssp-buffer-size=4');
-   $flags->append('OBJCFLAGS', '-fstack-protector 
--param=ssp-buffer-size=4');
-   $flags->append('OBJCXXFLAGS', '-fstack-protector 
--param=ssp-buffer-size=4');
-   $flags->append('FFLAGS', '-fstack-protector --param=ssp-buffer-size=4');
-   $flags->append('FCFLAGS', '-fstack-protector 
--param=ssp-buffer-size=4');
-   $flags->append('CXXFLAGS', '-fstack-protector 
--param=ssp-buffer-size=4');
-   $flags->append('GCJFLAGS', '-fstack-protector 
--param=ssp-buffer-size=4');
+   if ($use_stackprotector_strong) {
+   $flags->append('CFLAGS', '-fstack-protector-strong');
+   $flags->append('OBJCFLAGS', '-fstack-protector-strong');
+   $flags->append('OBJCXXFLAGS', '-fstack-protector-strong');
+   $flags->append('FFLAGS', '-fstack-protector-strong');
+   $flags->append('FCFLAGS', '-fstack-protector-strong');
+   $flags->append('CXXFLAGS', '-fstack-protector-strong');
+   $flags->append('GCJFLAGS', '-fstack-protector-strong');
+   } else {
+   $flags->append('CFLAGS', '-fstack-protector 
--param=ssp-buffer-size=4');
+   $flags->append('OBJCFLAGS', '-fstack-protector 
--param=ssp-buffer-size=4');
+   $flags->append('OBJCXXFLAGS', '-fstack-protector 
--param=ssp-buffer-size=4');
+   $flags->append('FFLAGS', '-fstack-protector 
--param=ssp-buffer-size=4');
+   $flags->append('FCFLAGS', '-fstack-protector 
--param=ssp-buffer-size=4');
+   $flags->append('CXXFLAGS', '-fstack-protector 
-

Re: [RFC PATCH] dpkg-buildflags: Switch to -fstack-protector-strong

2014-06-24 Thread Guillem Jover
Hi!

On Tue, 2014-06-24 at 11:29:31 +0200, Romain Francoise wrote:
> GCC 4.9 supports a new stack protector implementation, enabled via the
> -fstack-protector-strong flag, which provides a better balance between
> security and performance than the default implementation that we're
> currently using. This new flag is already used by Fedora 20 and
> ChromeOS. See the following for more information:
> 
>  https://lwn.net/Articles/584225/
>  http://www.outflux.net/blog/archives/2014/01/27/fstack-protector-strong/
>  https://fedorahosted.org/fesco/ticket/1128
> 
> The Security Team has expressed interest in switching dpkg-buildflags
> over to this new flag in Debian for jessie, now that GCC 4.9 is the
> default compiler on all release architectures. In order to see the
> impact on the archive, David Suárez did a full rebuild on EC2 with a
> patched dpkg-dev which emits the new flag.

Thanks a lot for doing this!

> * false positives:
>   - gcc-4.7 4.7.4-1 (checks that dpkg-dev is 'ii')

For what purpose?

>   - seqan 1.4.1-3 (attempts to disable the stack protector using sed)

I guess it should be switched to use DEB_foo_STRIP build variables.

> As the number of build failures is low, I think it's safe to simply
> switch the default flag emitted by dpkg-buildflags and file bugs against
> the above packages to ask the maintainers to disable the stack protector
> or filter out/replace the new flag if they really can't upgrade to GCC
> 4.9.

Yeah, given the analysis and references this seems pretty safe, and
we could always disable it by default if we end up finding something
onerous going on. I'm tentatively merging this locally for either
dpkg 1.17.11 or 1.17.12. I'd appreciate if you could send a mail to
debian-devel for a heads-up and to look for input from other people
in case there's any possible known showstopper.

> So here is a prospective patch which changes dpkg-buildflags to emit the
> new flag for all architectures known to use GCC 4.9 as of today. Let me
> know if this looks workable for you.

> diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm
> index c5020dc..4e19752 100644
> --- a/scripts/Dpkg/Vendor/Debian.pm
> +++ b/scripts/Dpkg/Vendor/Debian.pm
> @@ -92,6 +92,7 @@ sub add_hardening_flags {
>   relro => 1,
>   bindnow => 0,
>  );
> +my $use_stackprotector_strong = 1;

I've changed this to be just one more %use_feature key.

>  # Adjust features based on Maintainer's desires.
>  my $opts = Dpkg::BuildOptions->new(envvar => 'DEB_BUILD_MAINT_OPTIONS');
> @@ -129,6 +130,12 @@ sub add_hardening_flags {
>   #   compiler supports it incorrectly (leads to SEGV)
>   $use_feature{stackprotector} = 0;
>  }
> +if ($arch =~ /^(?:m68k|or1k|powerpcspe|sh4|x32)$/) {
> + # "Strong" stack protector disabled on m68k, or1k, powerpcspe, sh4, x32.
> + #   It requires GCC 4.9 and these archs are still using 4.8 as of
> + #   gcc-defaults 1.128.
> + $use_stackprotector_strong = 0;
> +}

It would be nicer to detect the gcc version and deactivate based on
that, but I don't think that might be reliable, as CC/CXX might not
have been setup in the correct place for this to work, so we'll go
with this for now.

Thanks,
Guillem


-- 
To UNSUBSCRIBE, email to debian-dpkg-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/20140624102156.ga2...@gaara.hadrons.org



Re: [RFC PATCH] dpkg-buildflags: Switch to -fstack-protector-strong

2014-06-24 Thread Kees Cook
On Tue, Jun 24, 2014 at 11:29:31AM +0200, Romain Francoise wrote:
> Hi,
> 
> GCC 4.9 supports a new stack protector implementation, enabled via the
> -fstack-protector-strong flag, which provides a better balance between
> security and performance than the default implementation that we're
> currently using. This new flag is already used by Fedora 20 and
> ChromeOS. See the following for more information:

Thanks for testing this! I would love to see this change go into the
archive.

>  https://lwn.net/Articles/584225/
>  http://www.outflux.net/blog/archives/2014/01/27/fstack-protector-strong/
>  https://fedorahosted.org/fesco/ticket/1128
> 
> The Security Team has expressed interest in switching dpkg-buildflags
> over to this new flag in Debian for jessie, now that GCC 4.9 is the
> default compiler on all release architectures. In order to see the
> impact on the archive, David Suárez did a full rebuild on EC2 with a
> patched dpkg-dev which emits the new flag.
> 
> There are only 16 new failures, which can be categorized as follows:
> 
> * explicitly build-depends on and uses gcc/g++ 4.8, which doesn't
>   understand -fstack-protector-strong:
>   - ccbuild 2.0.6-2.1
>   - chromium-browser 35.0.1916.153-2
>   - contextfree 3.0.5+dfsg1-2.1
>   - flexc++ 2.01.00-1
>   - gpg-remailer 3.00.02-1
>   - higan 094-4
>   - llvm-toolchain-snapshot 1:3.5~svn209039-2
>   - openimageio 1.4.9~dfsg0-1 (already fixed in -2)
>   - oxref 1.00.01-1
>   - spek 0.8.2-3.1
>   - webkitgtk 2.4.3-2
> 
> * explicitly build-depends on and uses gcc 4.6:
>   - estic 1.61-20.1 (#747980)
> 
> * explicitly build-depends on and uses Clang 3.4:
>   - feel++ 1:0.98.0-final-1

I wonder if there is any sensible way for dpkg-buildflags to detect (or
maybe just be told) which compile will be used for a build? Perhaps it
could take a new argument that would allow it to select flags based on the
compiler name and version?

dpkg-buildflags --compiler=gcc-4.7

> * false positives:
>   - gcc-4.7 4.7.4-1 (checks that dpkg-dev is 'ii')
>   - seqan 1.4.1-3 (attempts to disable the stack protector using sed)
> 
> * needs test suite upgrade for -fstack-protector-strong:
>   - hardening-wrapper 2.5

I can get this fixed up. Though really hardening-wrapper should be
deprecated for Jessie.

> See http://aws-logs.debian.net/ftbfs-logs/buildflags/ for the full
> results and build logs.
> 
> As the number of build failures is low, I think it's safe to simply
> switch the default flag emitted by dpkg-buildflags and file bugs against
> the above packages to ask the maintainers to disable the stack protector
> or filter out/replace the new flag if they really can't upgrade to GCC
> 4.9.
> 
> So here is a prospective patch which changes dpkg-buildflags to emit the
> new flag for all architectures known to use GCC 4.9 as of today. Let me
> know if this looks workable for you.
> 
> 
> diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm
> index c5020dc..4e19752 100644
> --- a/scripts/Dpkg/Vendor/Debian.pm
> +++ b/scripts/Dpkg/Vendor/Debian.pm
> @@ -92,6 +92,7 @@ sub add_hardening_flags {
>   relro => 1,
>   bindnow => 0,
>  );
> +my $use_stackprotector_strong = 1;
>  
>  # Adjust features based on Maintainer's desires.
>  my $opts = Dpkg::BuildOptions->new(envvar => 'DEB_BUILD_MAINT_OPTIONS');
> @@ -129,6 +130,12 @@ sub add_hardening_flags {
>   #   compiler supports it incorrectly (leads to SEGV)
>   $use_feature{stackprotector} = 0;
>  }
> +if ($arch =~ /^(?:m68k|or1k|powerpcspe|sh4|x32)$/) {
> + # "Strong" stack protector disabled on m68k, or1k, powerpcspe, sh4, x32.
> + #   It requires GCC 4.9 and these archs are still using 4.8 as of
> + #   gcc-defaults 1.128.
> + $use_stackprotector_strong = 0;
> +}
>  if ($cpu =~ /^(?:ia64|hppa|avr32)$/) {
>   # relro not implemented on ia64, hppa, avr32.
>   $use_feature{relro} = 0;
> @@ -161,13 +168,23 @@ sub add_hardening_flags {
>  
>  # Stack protector
>  if ($use_feature{stackprotector}) {
> - $flags->append('CFLAGS', '-fstack-protector --param=ssp-buffer-size=4');
> - $flags->append('OBJCFLAGS', '-fstack-protector 
> --param=ssp-buffer-size=4');
> - $flags->append('OBJCXXFLAGS', '-fstack-protector 
> --param=ssp-buffer-size=4');
> - $flags->append('FFLAGS', '-fstack-protector --param=ssp-buffer-size=4');
> - $flags->append('FCFLAGS', '-fstack-protector 
> --param=ssp-buffer-size=4');
> - $flags->append('CXXFLAGS', '-fstack-protector 
> --param=ssp-buffer-size=4');
> - $flags->append('GCJFLAGS', '-fstack-protector 
> --param=ssp-buffer-size=4');
> + if ($use_stackprotector_strong) {
> + $flags->append('CFLAGS', '-fstack-protector-strong');
> + $flags->append('OBJCFLAGS', '-fstack-protector-strong');
> + $flags->append('OBJCXXFLAGS', '-fstack-protector-strong');
> + $flags->append('FFLAGS', '-fstack-protector-strong');
> + $flags->append('FCFLA

Re: [RFC PATCH] dpkg-buildflags: Switch to -fstack-protector-strong

2014-06-24 Thread Romain Francoise
On Tue, Jun 24, 2014 at 12:21:56PM +0200, Guillem Jover wrote:
>> * false positives:
>>   - gcc-4.7 4.7.4-1 (checks that dpkg-dev is 'ii')
>
> For what purpose?

By way of getting dpkg-dev's installed version. The changelog suggests
that the dependency cannot be expressed using Build-Depends for reasons
of cross-buildability... Anyway, this is just an artifact of the rebuild
configuration, not a problem with -fstack-protector-strong.

>>   - seqan 1.4.1-3 (attempts to disable the stack protector using sed)
>
> I guess it should be switched to use DEB_foo_STRIP build variables.

Ack, I filed #752558 about this. Not that it will make a huge difference
as the package doesn't build with GCC 4.9 anyway (#746911).

> Yeah, given the analysis and references this seems pretty safe, and
> we could always disable it by default if we end up finding something
> onerous going on. I'm tentatively merging this locally for either
> dpkg 1.17.11 or 1.17.12.

Awesome, thanks! If you need anything more from me on the dpkg side to
help drive this to completion, just let me know.

> I'd appreciate if you could send a mail to debian-devel for a heads-up
> and to look for input from other people in case there's any possible
> known showstopper.

Good idea, will do.

Cheers,
-- 
Romain Francoise 
http://people.debian.org/~rfrancoise/


-- 
To UNSUBSCRIBE, email to debian-dpkg-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/87pphy5lfz@kima.orebokech.com



Re: [RFC PATCH] dpkg-buildflags: Switch to -fstack-protector-strong

2014-06-24 Thread Romain Francoise
On Tue, Jun 24, 2014 at 07:11:58AM -0700, Kees Cook wrote:
> I wonder if there is any sensible way for dpkg-buildflags to detect (or
> maybe just be told) which compile will be used for a build? Perhaps it
> could take a new argument that would allow it to select flags based on the
> compiler name and version?
>
> dpkg-buildflags --compiler=gcc-4.7

Hmm. This could quickly become a huge headache, and in general I think
that we shouldn't encourage maintainers to use a non-standard/older
toolchain, it causes issues that go beyond hardening. So the cost of
doing so (like disabling incompatible flags) should be borne by the
package, not dpkg.

It would perhaps make more sense in terms of GCC vs. Clang, but in this
case -fstack-protector-strong is already supported by Clang 3.5.

>> * needs test suite upgrade for -fstack-protector-strong:
>>   - hardening-wrapper 2.5

> I can get this fixed up. Though really hardening-wrapper should be
> deprecated for Jessie.

I guess I should file a bug against hardening-wrapper in any case?

Thanks,
-- 
Romain Francoise 
http://people.debian.org/~rfrancoise/


-- 
To UNSUBSCRIBE, email to debian-dpkg-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/87lhsm5kte@kima.orebokech.com



Re: [RFC PATCH] dpkg-buildflags: Switch to -fstack-protector-strong

2014-06-24 Thread Kees Cook
On Tue, Jun 24, 2014 at 06:33:33PM +0200, Romain Francoise wrote:
> On Tue, Jun 24, 2014 at 07:11:58AM -0700, Kees Cook wrote:
> > I wonder if there is any sensible way for dpkg-buildflags to detect (or
> > maybe just be told) which compile will be used for a build? Perhaps it
> > could take a new argument that would allow it to select flags based on the
> > compiler name and version?
> >
> > dpkg-buildflags --compiler=gcc-4.7
> 
> Hmm. This could quickly become a huge headache, and in general I think
> that we shouldn't encourage maintainers to use a non-standard/older
> toolchain, it causes issues that go beyond hardening. So the cost of
> doing so (like disabling incompatible flags) should be borne by the
> package, not dpkg.
> 
> It would perhaps make more sense in terms of GCC vs. Clang, but in this
> case -fstack-protector-strong is already supported by Clang 3.5.

Sounds good to me! I would prefer the default just be the default,
honestly.

> >> * needs test suite upgrade for -fstack-protector-strong:
> >>   - hardening-wrapper 2.5
> 
> > I can get this fixed up. Though really hardening-wrapper should be
> > deprecated for Jessie.
> 
> I guess I should file a bug against hardening-wrapper in any case?

That would be helpful, thank you!

-Kees

-- 
Kees Cook@debian.org


-- 
To UNSUBSCRIBE, email to debian-dpkg-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/20140624164004.gs5...@outflux.net



Re: [RFC PATCH] dpkg-buildflags: Switch to -fstack-protector-strong

2014-06-24 Thread Moritz Muehlenhoff
On Tue, Jun 24, 2014 at 07:11:58AM -0700, Kees Cook wrote:
> Though really hardening-wrapper should be
> deprecated for Jessie.

I looked into it, but the number of packages using -wrapper or
-includes is too large to do this realistically. Rather for
jessie+1.

I think we need a lintian check to gain some traction.

Cheers,
Moritz


-- 
To UNSUBSCRIBE, email to debian-dpkg-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/20140624163727.ga32...@inutil.org



Re: [RFC PATCH] dpkg-buildflags: Switch to -fstack-protector-strong

2014-06-24 Thread Russ Allbery
Moritz Muehlenhoff  writes:
> On Tue, Jun 24, 2014 at 07:11:58AM -0700, Kees Cook wrote:

>> Though really hardening-wrapper should be deprecated for Jessie.

> I looked into it, but the number of packages using -wrapper or
> -includes is too large to do this realistically. Rather for
> jessie+1.

> I think we need a lintian check to gain some traction.

Speaking as one of the people maintaining a package that still uses
hardening-wrapper, some packages have build systems that are quite
difficult to patch to use packaging-provided compiler flags.  It's
definitely an upstream bug, but

-- 
Russ Allbery (r...@debian.org)   


-- 
To UNSUBSCRIBE, email to debian-dpkg-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/87wqc6ywl3@windlord.stanford.edu



Re: [RFC PATCH] dpkg-buildflags: Switch to -fstack-protector-strong

2014-06-24 Thread Niels Thykier
On 2014-06-24 18:37, Moritz Muehlenhoff wrote:
> On Tue, Jun 24, 2014 at 07:11:58AM -0700, Kees Cook wrote:
>> Though really hardening-wrapper should be
>> deprecated for Jessie.
> 
> I looked into it, but the number of packages using -wrapper or
> -includes is too large to do this realistically. Rather for
> jessie+1.
> 
> I think we need a lintian check to gain some traction.
> 
> Cheers,
> Moritz
> 
> 

Hi,

Already implemented in 2.5.23 (#711193)[1].

~Niels

[1]
http://anonscm.debian.org/gitweb/?p=lintian/lintian.git;a=blob;f=debian/changelog;h=12ed6ab22b51d347e2154b7c534b9d11f19b2f7d;hb=cf428ac97f55708d56c500d728e0c129e8327b53#l154


-- 
To UNSUBSCRIBE, email to debian-dpkg-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/53a9d3aa.6060...@thykier.net



Re: [RFC PATCH] dpkg-buildflags: Switch to -fstack-protector-strong

2014-06-24 Thread Romain Francoise
On Tue, Jun 24, 2014 at 11:46:32AM -0700, Russ Allbery wrote:
> Speaking as one of the people maintaining a package that still uses
> hardening-wrapper, some packages have build systems that are quite
> difficult to patch to use packaging-provided compiler flags.  It's
> definitely an upstream bug, but

So if the compiler diversion itself is valuable, we can just remove all
the logic from hardening-wrapper and use something conceptually like the
following as the wrapper:

#!/bin/sh
if [ "$DEB_BUILD_HARDENING" = "1" ]; then
export DEB_BUILD_MAINT_OPTIONS=hardening=+all
eval $(/usr/bin/dpkg-buildflags --export=sh)
fi
exec /usr/bin/cc $CPPFLAGS $CFLAGS "$@"

-- 
Romain Francoise 
http://people.debian.org/~rfrancoise/


-- 
To UNSUBSCRIBE, email to debian-dpkg-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/87tx7a3xfu@kima.orebokech.com