On Thu, Dec 29, 2011 at 04:14:47AM +0100, Guillem Jover wrote:
> On Wed, 2011-12-28 at 15:28:45 -0800, Kees Cook wrote:
> > On Sun, Dec 18, 2011 at 09:42:50AM +0100, Guillem Jover wrote:
> > > On Fri, 2011-12-16 at 16:39:25 -0800, Kees Cook wrote:
> > > > Fresh patch attached! :)
> > >
> > > Thanks! Could you split the refactoring/cleaning into its own patch
> > > (actually something that already crossed my mind when first seeing
> > > the original buildflags code), and the new functionality into another
> > > one?
> >
> > Sure! Hopefully I chose the right line for this split. I also split out the
> > documentation adjustment that was unrelated to these changes.
>
> Heh, I guess I should have been a bit more explicit. See below.
Ah! Heh, I misunderstood. :) Fixed now.
> > Signed-off-by: Kees Cook <k...@debian.org>
>
> I guess you might want the From (Author) to match the Signed-off-by?
Good catch, fixed.
> I meant for example something like:
>
> my $para_shown = 0;
> foreach ... {
> if ($para_shown) {
> print "\n";
> } else {
> $para_shown = 1;
> }
> printf "Feature: %s\n", $feature;
> printf "Enabled: %s\n", $features{$feature} ? "yes" : "no";
> }
Adjusted. Refreshed patches attached...
Thanks!
-Kees
--
Kees Cook @debian.org
>From a553c3e3968ed328b341d32e7c627608c6fbb2c6 Mon Sep 17 00:00:00 2001
From: Kees Cook <k...@debian.org>
Date: Wed, 28 Dec 2011 15:22:55 -0800
Subject: [PATCH 1/3] docs: clarify the relationship between relro/bindnow
Clarify the documentation about how bindnow will be forced off if relro
is not enabled or available.
Signed-off-by: Kees Cook <k...@debian.org>
---
man/dpkg-buildflags.1 | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/man/dpkg-buildflags.1 b/man/dpkg-buildflags.1
index a018edb..b86ae0d 100644
--- a/man/dpkg-buildflags.1
+++ b/man/dpkg-buildflags.1
@@ -231,7 +231,8 @@ This setting (enabled by default) adds
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.
+notably this prevents GOT overwrite attacks. If this option is disabled,
+\fBbindnow\fP will become disabled as well.
.
.TP
.B bindnow
@@ -239,7 +240,7 @@ This setting (disabled by default) adds
.B \-Wl,\-z,now
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).
+above). The option cannot become enabled if \fBrelro\fP is not enabled.
.
.TP
.B pie
--
1.7.5.4
>From b531f0516c2a12b2a1dce2af612abf1ab3577804 Mon Sep 17 00:00:00 2001
From: Kees Cook <k...@debian.org>
Date: Wed, 28 Dec 2011 15:03:44 -0800
Subject: [PATCH 2/3] Refactor compiler hardening logic
Refactor the hardened compiler flag logic so the "use_feature" variable ends
up controlling the final stance of each given hardening feature.
Signed-off-by: Kees Cook <k...@debian.org>
---
scripts/Dpkg/Vendor/Debian.pm | 61 +++++++++++++++++++++++++++-------------
1 files changed, 41 insertions(+), 20 deletions(-)
diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm
index e824d0e..f363fee 100644
--- a/scripts/Dpkg/Vendor/Debian.pm
+++ b/scripts/Dpkg/Vendor/Debian.pm
@@ -83,7 +83,7 @@ sub add_hardening_flags {
my $arch = get_host_arch();
my ($abi, $os, $cpu) = debarch_to_debtriplet($arch);
- # Decide what's enabled
+ # Features enabled by default for all builds.
my %use_feature = (
"pie" => 0,
"stackprotector" => 1,
@@ -92,6 +92,8 @@ sub add_hardening_flags {
"relro" => 1,
"bindnow" => 0
);
+
+ # Adjust features based on Maintainer's desires.
my $opts = Dpkg::BuildOptions->new(envvar => "DEB_BUILD_MAINT_OPTIONS");
foreach my $feature (split(",", $opts->get("hardening") // "")) {
$feature = lc($feature);
@@ -112,43 +114,62 @@ sub add_hardening_flags {
}
}
- # 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)
+ # Mask features that are not available on certain architectures.
+ if ($os !~ /^(linux|knetbsd|hurd)$/ or
+ $cpu =~ /^(hppa|m68k|mips|mipsel|avr32)$/) {
+ # Disabled on non-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");
+ # (#574716).
+ $use_feature{"pie"} = 0;
}
- # Stack protector
- if ($use_feature{"stackprotector"} and
- $cpu !~ /^(ia64|alpha|mips|mipsel|hppa)$/ and $arch ne "arm") {
+ if ($cpu =~ /^(ia64|alpha|mips|mipsel|hppa)$/ or $arch eq "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)
+ $use_feature{"stackprotector"} = 0;
+ }
+ if ($cpu =~ /^(ia64|hppa|avr32)$/) {
+ # relro not implemented on ia64, hppa, avr32.
+ $use_feature{"relro"} = 0;
+ }
+
+ # Handle logical feature interactions.
+ if ($use_feature{"relro"} == 0) {
+ # Disable bindnow if relro is not enabled, since it has no
+ # hardening ability without relro and may incur load penalties.
+ $use_feature{"bindnow"} = 0;
+ }
+
+ # PIE
+ if ($use_feature{"pie"}) {
+ $flags->append("CFLAGS", "-fPIE");
+ $flags->append("CXXFLAGS", "-fPIE");
+ $flags->append("LDFLAGS", "-fPIE -pie");
+ }
+
+ # Stack protector
+ if ($use_feature{"stackprotector"}) {
$flags->append("CFLAGS", "-fstack-protector --param=ssp-buffer-size=4");
$flags->append("CXXFLAGS", "-fstack-protector --param=ssp-buffer-size=4");
}
- # Fortify
+
+ # Fortify Source
if ($use_feature{"fortify"}) {
$flags->append("CPPFLAGS", "-D_FORTIFY_SOURCE=2");
}
- # Format
+
+ # Format Security
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)$/) {
+
+ # Read-only Relocations
+ if ($use_feature{"relro"}) {
$flags->append("LDFLAGS", "-Wl,-z,relro");
- } else {
- # Disable full relro if relro is not enabled.
- $use_feature{"bindnow"} = 0;
}
+
# Bindnow
if ($use_feature{"bindnow"}) {
$flags->append("LDFLAGS", "-Wl,-z,now");
--
1.7.5.4
>From 01b601f66122935ab38a88b99ed17d0472888d06 Mon Sep 17 00:00:00 2001
From: Kees Cook <k...@debian.org>
Date: Thu, 8 Dec 2011 15:53:14 -0800
Subject: [PATCH 3/3] dpkg-buildflags: provide feature query ability
Since the logic for having a hardening flag enabled or disabled depends
on the architecture, and since the flags may change over time for each
hardening feature, there needs to be a way to externally query the state
of the hardening features. Specifically, lintian needs this to be able
to figure out if a binary package is missing expected hardening features.
Instead of maintaining multiple hard-coded lists of expected hardening
features, this makes dpkg-buildflags the canonical location of the
information, which can be queried by externally. (See bug 650536.)
Signed-off-by: Kees Cook <k...@debian.org>
---
man/dpkg-buildflags.1 | 16 ++++++++++++++++
scripts/Dpkg/BuildFlags.pm | 40 +++++++++++++++++++++++++++++++++++++++-
scripts/Dpkg/Vendor/Debian.pm | 5 +++++
scripts/dpkg-buildflags.pl | 19 ++++++++++++++++++-
4 files changed, 78 insertions(+), 2 deletions(-)
diff --git a/man/dpkg-buildflags.1 b/man/dpkg-buildflags.1
index b86ae0d..2b5fb2e 100644
--- a/man/dpkg-buildflags.1
+++ b/man/dpkg-buildflags.1
@@ -87,6 +87,22 @@ the flag is set/modified by a user-specific configuration;
the flag is set/modified by an environment-specific configuration.
.RE
.TP
+.BI \-\-query\-features " area"
+Print the features enabled for a given area. The only currently recognized
+area is \fBhardening\fP. Exits with 0 if the area is known otherwise exits
+with 1.
+.IP
+The output format is RFC822 header-style, with one section per feature.
+For example:
+.IP
+.nf
+ Feature: pie
+ Enabled: no
+
+ Feature: stackprotector
+ Enabled: yes
+.fi
+.TP
.B \-\-help
Show the usage message and exit.
.TP
diff --git a/scripts/Dpkg/BuildFlags.pm b/scripts/Dpkg/BuildFlags.pm
index 6112a9f..e832b39 100644
--- a/scripts/Dpkg/BuildFlags.pm
+++ b/scripts/Dpkg/BuildFlags.pm
@@ -18,7 +18,7 @@ package Dpkg::BuildFlags;
use strict;
use warnings;
-our $VERSION = "1.01";
+our $VERSION = "1.02";
use Dpkg::Gettext;
use Dpkg::BuildOptions;
@@ -68,6 +68,7 @@ sub load_vendor_defaults {
my ($self) = @_;
$self->{'options'} = {};
$self->{'source'} = {};
+ $self->{'features'} = {};
my $build_opts = Dpkg::BuildOptions->new();
my $default_flags = $build_opts->has("noopt") ? "-g -O0" : "-g -O2";
$self->{flags} = {
@@ -202,6 +203,19 @@ sub set {
$self->{origin}->{$flag} = $src if defined $src;
}
+=item $bf->set_feature($area, $feature, $enabled)
+
+Update the boolean state of whether a specific feature within a known
+feature area has been enabled. The only currently known feature area is
+"hardening".
+
+=cut
+
+sub set_feature {
+ my ($self, $area, $feature, $enabled) = @_;
+ $self->{features}->{$area}->{$feature} = !!$enabled;
+}
+
=item $bf->strip($flag, $value, $source)
Update the build flag $flag by stripping the flags listed in $value and
@@ -306,6 +320,18 @@ sub get {
return $self->{'flags'}{$key};
}
+=item $bf->get_features($area)
+
+Return, for the given area, a hash with keys as feature names, and values
+as booleans indicating whether the feature is enabled or not.
+
+=cut
+
+sub get_features {
+ my ($self, $area) = @_;
+ return %{$self->{'features'}{$area}};
+}
+
=item $bf->get_origin($flag)
Return the origin associated to the flag. It might be undef if the
@@ -318,6 +344,18 @@ sub get_origin {
return $self->{'origin'}{$key};
}
+=item $bf->has_features($area)
+
+Returns true if the given area of features is known, and false otherwise.
+The only currently recognized area is "hardening".
+
+=cut
+
+sub has_features {
+ my ($self, $area) = @_;
+ return exists $self->{'features'}{$area};
+}
+
=item $bf->has($option)
Returns a boolean indicating whether the flags exists in the object.
diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm
index f363fee..07935d9 100644
--- a/scripts/Dpkg/Vendor/Debian.pm
+++ b/scripts/Dpkg/Vendor/Debian.pm
@@ -174,6 +174,11 @@ sub add_hardening_flags {
if ($use_feature{"bindnow"}) {
$flags->append("LDFLAGS", "-Wl,-z,now");
}
+
+ # Store the feature usage.
+ for (keys %use_feature) {
+ $flags->set_feature("hardening", $_, $use_feature{$_})
+ }
}
1;
diff --git a/scripts/dpkg-buildflags.pl b/scripts/dpkg-buildflags.pl
index ee33961..e80cb97 100755
--- a/scripts/dpkg-buildflags.pl
+++ b/scripts/dpkg-buildflags.pl
@@ -47,6 +47,8 @@ Actions:
--get <flag> output the requested flag to stdout.
--origin <flag> output the origin of the flag to stdout:
value is one of vendor, system, user, env.
+ --query-features <area>
+ output the status of features for the given area.
--list output a list of the flags supported by the current vendor.
--export=(sh|make|configure)
output something convenient to import the
@@ -62,7 +64,7 @@ my ($param, $action);
while (@ARGV) {
$_ = shift(@ARGV);
- if (m/^--(get|origin)$/) {
+ if (m/^--(get|origin|query-features)$/) {
usageerr(_g("two commands specified: --%s and --%s"), $1, $action)
if defined($action);
$action = $1;
@@ -115,6 +117,21 @@ if ($action eq "get") {
print $build_flags->get_origin($param) . "\n";
exit(0);
}
+} elsif ($action eq "query-features") {
+ if ($build_flags->has_features($param)) {
+ my %features = $build_flags->get_features($param);
+ my $para_shown = 0;
+ foreach my $feature (sort keys %features) {
+ if ($para_shown) {
+ print "\n";
+ } else {
+ $para_shown = 1;
+ }
+ printf "Feature: %s\n", $feature;
+ printf "Enabled: %s\n", $features{$feature} ? "yes" : "no";
+ }
+ exit(0);
+ }
} elsif ($action =~ m/^export-(.*)$/) {
my $export_type = $1;
foreach my $flag ($build_flags->list()) {
--
1.7.5.4
