Re: Hardening patch
Hi,
On Wed, 07 Sep 2011, Guillem Jover wrote:
> > --- a/scripts/Dpkg/BuildFlags.pm
> > +++ b/scripts/Dpkg/BuildFlags.pm
> > @@ -84,9 +89,93 @@ sub load_vendor_defaults {
> > FFLAGS => 'vendor',
> > LDFLAGS => 'vendor',
> > };
> > + $self->add_hardening_flags();
> > run_vendor_hook("update-buildflags", $self);
> > }
>
> This seems too compiler, system and distribution specific to be set by
> default for all BuildFlags users, I think it would make more sense to
> split it into a different module, and enable it from the Debian vendor
> hook, so that Ubuntu inherits it too.
Good remark, creating a new module seemed overkill but I moved the logic
to Dpkg::Vendor::Debian (and adjusted Dpkg::Vendor::Ubuntu to properly
inherit from it).
If we ever end up supporting multiple compiler/libc each with their own
hardening build flags, then a new module might be a good idea. For now,
this will do.
> > Document the various hardening options that can be enabled/disabled
> > via DEB_BUILD_MAINT_OPTIONS.
>
> All minus signs that can end up being copy&pasted into a runnable
> command, etc. need to be escaped as in \- so that man does not turn
> them into hyphens.
Yeah, I noticed that yesterday too, they were already fixed in an
updated copy of pu/build-flags as well as a few other details.
New patches attached.
Cheers,
--
Raphaël Hertzog ◈ Debian Developer
Follow my Debian News ▶ http://RaphaelHertzog.com (English)
▶ http://RaphaelHertzog.fr (Français)
>From 449bbede7028ce55937bce670226b0af9e7c663b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hertzog@debian.org>
Date: Tue, 2 Aug 2011 14:15:17 +0200
Subject: [PATCH 1/3] Dpkg::BuildOptions: enable usage of alternative variable
names
---
scripts/Dpkg/BuildOptions.pm | 26 ++++++++++++++++++--------
scripts/t/300_Dpkg_BuildOptions.t | 6 +++++-
2 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/scripts/Dpkg/BuildOptions.pm b/scripts/Dpkg/BuildOptions.pm
index 2638017..1accdec 100644
--- a/scripts/Dpkg/BuildOptions.pm
+++ b/scripts/Dpkg/BuildOptions.pm
@@ -19,7 +19,7 @@ package Dpkg::BuildOptions;
use strict;
use warnings;
-our $VERSION = "1.00";
+our $VERSION = "1.01";
use Dpkg::Gettext;
use Dpkg::ErrorHandling;
@@ -33,16 +33,18 @@ Dpkg::BuildOptions - parse and update build options
=head1 DESCRIPTION
The Dpkg::BuildOptions object can be used to manipulate options stored
-in the DEB_BUILD_OPTIONS environment variable.
+in environment variables like DEB_BUILD_OPTIONS and
+DEB_BUILD_MAINT_OPTIONS.
=head1 FUNCTIONS
=over 4
-=item my $bo = Dpkg::BuildOptions->new()
+=item my $bo = Dpkg::BuildOptions->new(%opts)
Create a new Dpkg::BuildOptions object. It will be initialized based
-on the value of the DEB_BUILD_OPTIONS environment variable.
+on the value of the environment variable named $opts{'envvar'} (or
+DEB_BUILD_OPTIONS if that option is not set).
=cut
@@ -53,9 +55,10 @@ sub new {
my $self = {
options => {},
source => {},
+ envvar => $opts{'envvar'} // "DEB_BUILD_OPTIONS",
};
bless $self, $class;
- $self->merge($ENV{DEB_BUILD_OPTIONS}, "DEB_BUILD_OPTIONS");
+ $self->merge($ENV{$self->{'envvar'}}, $self->{'envvar'});
return $self;
}
@@ -169,14 +172,14 @@ sub output {
=item $bo->export([$var])
Export the build options to the given environment variable. If omitted,
-DEB_BUILD_OPTIONS is assumed. The value set to the variable is also
-returned.
+the environment variable defined at creation time is assumed. The value
+set to the variable is also returned.
=cut
sub export {
my ($self, $var) = @_;
- $var = "DEB_BUILD_OPTIONS" unless defined $var;
+ $var = $self->{'envvar'} unless defined $var;
my $content = $self->output();
$ENV{$var} = $content;
return $content;
@@ -184,6 +187,13 @@ sub export {
=back
+=head1 CHANGES
+
+=head2 Version 1.01
+
+Enable to use another environment variable instead of DEB_BUILD_OPTIONS.
+Thus add support for the "envvar" option at creation time.
+
=head1 AUTHOR
Raphaël Hertzog <hertzog@debian.org>
diff --git a/scripts/t/300_Dpkg_BuildOptions.t b/scripts/t/300_Dpkg_BuildOptions.t
index d5979e4..a715549 100644
--- a/scripts/t/300_Dpkg_BuildOptions.t
+++ b/scripts/t/300_Dpkg_BuildOptions.t
@@ -13,7 +13,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-use Test::More tests => 22;
+use Test::More tests => 24;
use Dpkg::ErrorHandling;
use strict;
@@ -66,3 +66,7 @@ $ENV{DEB_BUILD_OPTIONS} = 'foobar';
$dbo = Dpkg::BuildOptions->new();
$dbo->set("noopt", 1);
is($dbo->output(), "foobar noopt", "output");
+
+$dbo = Dpkg::BuildOptions->new(envvar => "OTHER_VARIABLE");
+is($dbo->get("parallel"), 5, "import from other variable, check parallel");
+ok($dbo->has("noopt"), "import from other variable, check noopt");
--
1.7.5.4
>From 8ea91d6285f490d583f85e1b1621a67ccb33e64a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hertzog@debian.org>
Date: Wed, 27 Jul 2011 22:10:49 +0200
Subject: [PATCH 2/3] dpkg-buildflags: emit hardening build flags by default
All the hardening build flags supported by hardening-includes
are supported except that PIE is not enabled by default (just like
the corresponding gcc patch doesn't enable it by default).
Inspired by the work of Kees Cook <kees@debian.org>.
---
debian/changelog | 3 +
scripts/Dpkg/BuildFlags.pm | 1 +
scripts/Dpkg/Vendor/Debian.pm | 88 ++++++++++++++++++++++++++++++++++++++++-
scripts/Dpkg/Vendor/Ubuntu.pm | 4 ++
4 files changed, 95 insertions(+), 1 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index 06d7dbb..977d27d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -102,6 +102,9 @@ dpkg (1.16.1) UNRELEASED; urgency=low
* Fix dpkg's handling of a hardlink pointing to a conffile. Closes: #638291
* Add example of extend-diff-ignore's usage in dpkg-source(1).
Closes: #640198
+ * dpkg-buildflags now returns hardening flags by default. Closes: #489771
+ They can be individually enabled/disabled via DEB_BUILD_MAINT_OPTIONS,
+ see dpkg-buildflags(1). Thanks to Kees Cook for his help.
[ Guillem Jover ]
* Install deb-src-control(5) man pages in dpkg-dev. Closes: #620520
diff --git a/scripts/Dpkg/BuildFlags.pm b/scripts/Dpkg/BuildFlags.pm
index 9bc473a..6112a9f 100644
--- a/scripts/Dpkg/BuildFlags.pm
+++ b/scripts/Dpkg/BuildFlags.pm
@@ -84,6 +84,7 @@ sub load_vendor_defaults {
FFLAGS => 'vendor',
LDFLAGS => 'vendor',
};
+ # The Debian vendor hook will add hardening build flags
run_vendor_hook("update-buildflags", $self);
}
diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm
index 2cc2c78..54f406c 100644
--- a/scripts/Dpkg/Vendor/Debian.pm
+++ b/scripts/Dpkg/Vendor/Debian.pm
@@ -1,4 +1,8 @@
-# Copyright © 2009 Raphaël Hertzog <hertzog@debian.org>
+# Copyright © 2009-2011 Raphaël Hertzog <hertzog@debian.org>
+#
+# Hardening build flags handling derived from work of:
+# Copyright © 2009-2011 Kees Cook <kees@debian.org>
+# Copyright © 2007-2008 Canonical, Ltd.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -21,8 +25,13 @@ use warnings;
our $VERSION = "0.01";
use base qw(Dpkg::Vendor::Default);
+
+use Dpkg::Gettext;
+use Dpkg::ErrorHandling;
use Dpkg::Control::Types;
use Dpkg::Vendor::Ubuntu;
+use Dpkg::BuildOptions;
+use Dpkg::Arch qw(get_host_arch debarch_to_debtriplet);
=encoding utf8
@@ -62,9 +71,86 @@ sub run_hook {
foreach my $bug (@$b) {
$$textref .= "Bug-Ubuntu: https://bugs.launchpad.net/bugs/$bug\n";;
}
+ } elsif ($hook eq "update-buildflags") {
+ $self->add_hardening_flags(@params);
} else {
return $self->SUPER::run_hook($hook, @params);
}
}
+sub add_hardening_flags {
+ my ($self, $flags) = @_;
+ my $arch = get_host_arch();
+ my ($abi, $os, $cpu) = debarch_to_debtriplet($arch);
+
+ # Decide what's enabled
+ my %use_feature = (
+ "pie" => 0,
+ "stackprotector" => 1,
+ "fortify" => 1,
+ "format" => 1,
+ "relro" => 1,
+ "bindnow" => 1
+ );
+ my $opts = Dpkg::BuildOptions->new(envvar => "DEB_BUILD_MAINT_OPTIONS");
+ foreach my $feature (split(",", $opts->get("hardening") // "")) {
+ $feature = lc($feature);
+ if ($feature =~ s/^([+-])//) {
+ my $value = ($1 eq "+") ? 1 : 0;
+ if ($feature eq "all") {
+ $use_feature{$_} = $value foreach keys %use_feature;
+ } else {
+ if (exists $use_feature{$feature}) {
+ $use_feature{$feature} = $value;
+ } else {
+ warning(_g("unknown hardening feature: %s"), $feature);
+ }
+ }
+ } else {
+ warning(_g("incorrect value in hardening option of " .
+ "DEB_BUILD_MAINT_OPTIONS: %s"), $feature);
+ }
+ }
+
+ # PIE
+ if ($use_feature{"pie"} and
+ $os =~ /^(linux|knetbsd|hurd)$/ and
+ $cpu !~ /^(hppa|m68k|mips|mipsel|avr32)$/) {
+ # Only on linux/knetbsd/hurd (see #430455 and #586215)
+ # Disabled on hppa, m68k (#451192), mips/mipsel (#532821), avr32
+ # (#574716)
+ $flags->append("CFLAGS", "-fPIE");
+ $flags->append("CXXFLAGS", "-fPIE");
+ $flags->append("LDFLAGS", "-fPIE -pie");
+ }
+ # Stack protector
+ if ($use_feature{"stackprotector"} and
+ $cpu !~ /^(ia64|alpha|mips|mipsel|hppa)$/ and $arch ne "arm") {
+ # Stack protector disabled on ia64, alpha, mips, mipsel, hppa.
+ # "warning: -fstack-protector not supported for this target"
+ # Stack protector disabled on arm (ok on armel).
+ # compiler supports it incorrectly (leads to SEGV)
+ $flags->append("CFLAGS", "-fstack-protector --param=ssp-buffer-size=4");
+ $flags->append("CXXFLAGS", "-fstack-protector --param=ssp-buffer-size=4");
+ }
+ # Fortify
+ if ($use_feature{"fortify"}) {
+ $flags->append("CFLAGS", "-D_FORTIFY_SOURCE=2");
+ $flags->append("CXXFLAGS", "-D_FORTIFY_SOURCE=2");
+ }
+ # Format
+ if ($use_feature{"format"}) {
+ $flags->append("CFLAGS", "-Wformat -Wformat-security -Werror=format-security");
+ $flags->append("CXXFLAGS", "-Wformat -Wformat-security -Werror=format-security");
+ }
+ # Relro
+ if ($use_feature{"relro"} and $cpu !~ /^(ia64|hppa|avr32)$/) {
+ $flags->append("LDFLAGS", "-Wl,-z,relro");
+ }
+ # Bindnow
+ if ($use_feature{"bindnow"}) {
+ $flags->append("LDFLAGS", "-Wl,-z,now");
+ }
+}
+
1;
diff --git a/scripts/Dpkg/Vendor/Ubuntu.pm b/scripts/Dpkg/Vendor/Ubuntu.pm
index a07aa53..f3ead0a 100644
--- a/scripts/Dpkg/Vendor/Ubuntu.pm
+++ b/scripts/Dpkg/Vendor/Ubuntu.pm
@@ -94,6 +94,7 @@ sub run_hook {
} elsif ($hook eq "update-buildflags") {
my $flags = shift @params;
+
if (debarch_eq(get_host_arch(), 'ppc64')) {
for my $flag (qw(CFLAGS CXXFLAGS FFLAGS)) {
$flags->set($flag, '-g -O3', 'vendor');
@@ -102,6 +103,9 @@ sub run_hook {
# Per https://wiki.ubuntu.com/DistCompilerFlags
$flags->set('LDFLAGS', '-Wl,-Bsymbolic-functions', 'vendor');
+ # Run the Debian hook to add hardening flags
+ $self->SUPER::run_hook($hook, $flags);
+
# Allow control of hardening-wrapper via dpkg-buildpackage DEB_BUILD_OPTIONS
my $build_opts = Dpkg::BuildOptions->new();
my $hardening;
--
1.7.5.4
>From fc86184089b2d9c9f11a991a1dc000a1d2bbe001 Mon Sep 17 00:00:00 2001
From: Kees Cook <kees@debian.org>
Date: Mon, 5 Sep 2011 23:34:49 -0700
Subject: [PATCH 3/3] dpkg-buildflags(1): add initial hardening documentation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Document the various hardening options that can be enabled/disabled
via DEB_BUILD_MAINT_OPTIONS.
Improved-by: Raphaël Hertzog <hertzog@debian.org>
Signed-off-by: Kees Cook <kees@debian.org>
Signed-off-by: Raphaël Hertzog <hertzog@debian.org>
---
man/dpkg-buildflags.1 | 109 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 108 insertions(+), 1 deletions(-)
diff --git a/man/dpkg-buildflags.1 b/man/dpkg-buildflags.1
index b8dcd43..f559b83 100644
--- a/man/dpkg-buildflags.1
+++ b/man/dpkg-buildflags.1
@@ -1,4 +1,4 @@
-.TH dpkg\-buildflags 1 "2011-08-14" "Debian Project" "dpkg suite"
+.TH dpkg\-buildflags 1 "2011-09-07" "Debian Project" "dpkg suite"
.SH NAME
dpkg\-buildflags \- returns build flags to use during package build
.
@@ -157,10 +157,117 @@ returned for the given \fIflag\fP.
.BI DEB_ flag _MAINT_PREPEND
This variable can be used to prepend supplementary options to the value
returned for the given \fIflag\fP.
+.TP
+.B DEB_BUILD_MAINT_OPTIONS
+This variable can be used to disable/enable various hardening build
+flags through the \fBhardening\fP option. See the \fBHARDENING\fP section
+for details.
+.
+.SH HARDENING
+Several compile-time options (detailed below) can be used to help harden
+a resulting binary against memory corruption attacks, or provide
+additional warning messages during compilation. Except as noted below,
+these are enabled by default for architectures that support them.
+.P
+Each class of hardening flags can be enabled and disabled in the
+\fBDEB_BUILD_MAINT_OPTIONS\fP environment variable's \fBhardening\fP
+value with the "+" and "\-" modifier. For example, to enable the
+"pie" class and disable the "fortify" class you can do this
+in \fBdebian/rules\fP:
+.P
+ export DEB_BUILD_MAINT_OPTIONS="hardening=+pie,\-fortify"
+.P
+The special class \fBall\fP can be used to enable or disable all classes
+of hardening flag at the same time. Thus disabling everything and enabling
+only "format" and "fortify" can be achieved with:
+.P
+ export DEB_BUILD_MAINT_OPTIONS="hardening=-all,+format,+fortify"
+.
+.TP
+.B format
+This setting (enabled by default) adds
+.B \-Wformat \-Wformat\-security \-Werror=format\-security
+to \fBCFLAGS\fP and \fBCXXFLAGS\fP. This will warn about improper format
+string uses, and will fail when format functions are used in a way that
+that represent possible security problems. At present, this warns about
+calls to \fBprintf\fP and \fBscanf\fP functions where the format string is
+not a string literal and there are no format arguments, as in
+\fBprintf(foo);\fP instead of \fPprintf("%s", foo);\fP
+This may be a security hole if the format string came from untrusted
+input and contains "%n".
+.
+.TP
+.B fortify
+This setting (enabled by default) adds
+.B \-D_FORTIFY_SOURCE=2
+to \fBCFLAGS\fP and \fBCXXFLAGS\fP. During code generation the compiler
+knows a great deal of information about buffer sizes (where possible), and
+attempts to replace insecure unlimited length buffer function calls with
+length-limited ones. This is especially useful for old, crufty code.
+Additionally, format strings in writable memory that contain '%n' are
+blocked. If an application depends on such a format string, it will need
+to be worked around.
+
+Note that for this option to have any effect, the source must also
+be compiled with \fB\-O1\fP or higher.
+.TP
+.B stackprotector
+This setting (enabled by default) adds
+.B \-fstack-protector \-\-param=ssp\-buffer\-size=4
+to \fBCFLAGS\fP and \fBCXXFLAGS\fP. This adds safety checks against stack
+overwrites. This renders many potential code injection attacks into
+aborting situations. In the best case this turns code injection
+vulnerabilities into denial of service or into non-issues (depending on
+the application).
+
+This feature requires linking against glibc (or another provider of
+\fB__stack_chk_fail\fP), so needs to be disabled when building with
+\fB\-nostdlib\fP or \fB\-ffreestanding\fP or similar.
+.
+.TP
+.B relro
+This setting (enabled by default) adds
+.B \-Wl,\-z,relro
+to \fBLDFLAGS\fP. During program load, several ELF memory sections need
+to be written to by the linker. This flags the loader to turn these
+sections read-only before turning over control to the program. Most
+notably this prevents GOT overwrite attacks.
+.
+.TP
+.B bindnow
+This setting (enabled by default) adds
+.B \-Wl,\-z,bindnow
+to \fBLDFLAGS\fP. During program load, all dynamic symbols are resolved,
+allowing for the entire PLT to be marked read-only (due to \fBrelro\fP
+above).
+.
+.TP
+.B pie
+This setting (disabled by default) adds \fB\-fPIE\fP to \fBCFLAGS\fP and
+\fBCXXFLAGS\fP, and \fB\-fPIE \-pie\fP to \fBLDFLAGS\fP. Position Independent
+Executable are needed to take advantage of Address Space Layout
+Randomization, supported by some kernel versions. While ASLR can already
+be enforced for data areas in the stack and heap (brk and mmap), the code
+areas must be compiled as position-independent. Shared libraries already
+do this (\-fPIC), so they gain ASLR automatically, but binary .text
+regions need to be build PIE to gain ASLR. When this happens, ROP (Return
+Oriented Programming) attacks are much harder since there are no static
+locations to bounce off of during a memory corruption attack.
+
+This is not compatible with \fB\-fPIC\fP so care must be taken when
+building shared objects.
+
+Additionally, since PIE is implemented via a general register, some
+architectures (most notably i386) can see performance losses of up to
+15% in very text-segment-heavy application workloads; most workloads
+see less than 1%. Architectures with more general registers (e.g. amd64)
+do not see as high a worst-case penalty.
.
.SH AUTHOR
Copyright \(co 2010-2011 Rapha\[:e]l Hertzog
.sp
+Copyright \(co 2011 Kees Cook
+.sp
This is free software; see the GNU General Public Licence version 2 or
later for copying conditions. There is NO WARRANTY.
--
1.7.5.4
Reply to: