Hi,
I attached a new version of the patch here.
On 03/01/16 03:23, Greg Troxel wrote:
> Do you have a plan for how this will be extended to more
> compilers/platforms? I'd like to understand that before we start.
Nope. I cannot have a plan for compilers or platforms that I do not know
and actively use. That's why I'm starting by communicating and showing
my work, and as you can see, with input from others and jperkin@ in
particular here, the plan builds itself and we all learn.
> I am curious what kind of testing this has received. Have you done bulk
> builds with this? On which platforms? Have the programs had their
> tests run ('make test'), and are there regressions? What compilers
> have been tested with this besides gcc? Certainly clang matters. But
> if you are saying that no behavior will change except for netbsd/gcc,
> that's probably ok.
I have been running pkgsrc-2015Q2 with these patches for almost a year
(I started around the 2015Q1 release). In the current state, the
behavior only affects NetBSD/gcc on specific platforms, and on SunOS
where tested by jperkin@.
> This adds things to wrappers, but what about cwrappers? We are nearing
> flipping to cwrappers by default, which I think means that we are in a
> period where changes have to apply to both. Also, the new wrapper seems
> to open-code flags, making it harder to maintain this and to port to
> clang.
I have never had the chance to look at cwrappers yet. I do not know
if/what changes there in this context. I have integrated the changes
reported in this thread though.
> I would like to see this separated into adding mechanisms that don't
> behave differently by default and later changing the defaults, so that
> people can easily experiment locally, rather than having the new
> behavior hit everyone right away.
That is my intention too.
> +#PKGSRC_MKPIE?= yes
>
> +.if ${PKGSRC_MKPIE:Uyes} != "no"
>
> I find this style confusing (moving the default into the test, [...]
I tried to reproduce and stick the idioms already in place. I updated
the patch with the suggestions from jperkin@.
The rationale is that I don't want to give false promises and let the
user set "yes" and then have the feature(s) be silently ignored because
the platform used is not actually supported. Misplaced impression of
security is worse than no security at all. I still have some concerns
there, but also better is the enemy of good - sometimes it is best to do
something imperfect, than not do it at all. Failing to build is more
secure than a bad package. But so is a computer without power. *mindcrash*
> So all I all I'd like:
>
> to hear the long-term plan (other platforms, other compilers)
Listen to suggestions and complaints, and consider and address them.
> to avoid the yes=no make-optimization
I think this looks better now; let me know.
> to figure out about cwrappers
I believe the behavior with cwrappers is not affected at the moment,
except where tested by jperkin@. I welcome help here.
> to have this be just adding features that can be turned on, not
> changing defaults
That is fine for me: the actual import to pkgsrc will be easier for
everyone with these new options disabled initially. The patch attached
here is still a work in progress, in the context of the EdgeBSD project.
HTH,
--
khorben
commit ecf90a7e852c79decd6cc5637e3e4877c9e7d931
Author: Pierre Pronchery <khorben%EdgeBSD.org@localhost>
Date: Tue Mar 1 00:09:26 2016 +0100
Add support for fortify, PIE, RELRO, SSP
This is currently applied where assumed supported. It still requires
adjustments to reflect reality.
Support for PIE is known to still break building a number of
packages at the moment.
diff --git a/mk/bsd.prefs.mk b/mk/bsd.prefs.mk
index a20e48f..020bb1e 100644
--- a/mk/bsd.prefs.mk
+++ b/mk/bsd.prefs.mk
@@ -810,6 +810,35 @@ _USE_CWRAPPERS= yes
_USE_CWRAPPERS= no
.endif
+_PKGSRC_USE_FORTIFY= no
+.if (${PKGSRC_USE_FORTIFY:tl} == "yes") && \
+ (${_OPSYS_SUPPORTS_FORTIFY:Uno} == "yes")
+_PKGSRC_USE_FORTIFY= yes
+_GCC_CFLAGS+= ${_FORTIFY_CFLAGS.gcc}
+.endif
+
+_PKGSRC_USE_SSP= no
+.if (${PKGSRC_USE_SSP:tl} == "yes") && \
+ (${_OPSYS_SUPPORTS_SSP:Uno} == "yes")
+_PKGSRC_USE_SSP= yes
+_GCC_CFLAGS+= ${_SSP_CFLAGS.gcc}
+.endif
+
+_PKGSRC_MKPIE= no
+.if (${PKGSRC_MKPIE:tl} == "yes") && \
+ (${_OPSYS_SUPPORTS_MKPIE:Uno} == "yes")
+_PKGSRC_MKPIE= yes
+_GCC_CFLAGS+= ${_MKPIE_CFLAGS.gcc}
+_GCC_LDFLAGS+= ${_MKPIE_LDFLAGS.gcc}
+.endif
+
+_PKGSRC_USE_RELRO= no
+.if (${PKGSRC_USE_RELRO:tl} == "yes") && \
+ (${_OPSYS_SUPPORTS_RELRO:Uno} == "yes")
+_PKGSRC_USE_RELRO= yes
+_GCC_LDFLAGS+= ${_RELRO_LDFLAGS.gcc}
+.endif
+
# Wrapper framework definitions
.include "wrapper/wrapper-defs.mk"
diff --git a/mk/compiler/gcc.mk b/mk/compiler/gcc.mk
index 3fb8532..32e88f5 100644
--- a/mk/compiler/gcc.mk
+++ b/mk/compiler/gcc.mk
@@ -338,6 +338,14 @@ _WRAP_EXTRA_ARGS.CC+= -std=gnu99
CWRAPPERS_APPEND.cc+= -std=gnu99
.endif
+.if ${_PKGSRC_USE_FORTIFY} == "yes"
+CWRAPPERS_APPEND.cc+= -D_FORTIFY_SOURCE=2
+.endif
+
+.if ${_PKGSRC_USE_SSP} == "yes"
+CWRAPPERS_APPEND.cc+= -fstack-protector-all
+.endif
+
# GCC has this annoying behaviour where it advocates in a multi-line
# banner the use of "#include" over "#import" when including headers.
# This generates a huge number of warnings when building practically all
@@ -713,9 +721,10 @@ _GCC_LDFLAGS= # empty
. for _dir_ in ${_GCC_LIBDIRS:N*not_found*}
_GCC_LDFLAGS+= -L${_dir_} ${COMPILER_RPATH_FLAG}${_dir_}
. endfor
-LDFLAGS+= ${_GCC_LDFLAGS}
.endif
+LDFLAGS+= ${_GCC_LDFLAGS}
+
# Point the variables that specify the compiler to the installed
# GCC executables.
#
diff --git a/mk/defaults/mk.conf b/mk/defaults/mk.conf
index a8977ae..8ecbdf1 100644
--- a/mk/defaults/mk.conf
+++ b/mk/defaults/mk.conf
@@ -215,20 +215,30 @@ PKGSRC_RUN_TEST?= no
# Possible: yes, no
# Default: no
-PKGSRC_USE_FORT?= no
+PKGSRC_USE_FORTIFY?= yes
# Turns on substitute wrappers for commonly used functions that do not bounds
-# checking regularly, but could in some cases (with GCC for instance).
+# checking regularly, but could in some cases. This is effectively in use only
+# when supported.
# Possible: yes, no
-# Default: no
+# Default: yes
-.if ${PKGSRC_USE_FORT:Uno} != "no"
PKGSRC_USE_SSP?= yes
-.else
-PKGSRC_USE_SSP?= no
-.endif
-# Set this to YES to enable stack-smashing protection (on supported platforms).
+# Set this to yes to enable stack-smashing protection (on supported platforms).
# Possible: yes, no
-# Default: no, except if PKGSRC_USE_FORT is set to "yes".
+# Default: yes
+
+PKGSRC_MKPIE?= no
+# If no, create regular executables. Otherwise create PIE (Position Independent
+# Executables, on supported platforms). This option is necessary to fully
+# leverage ASLR as a mitigation for security vulnerabilities.
+# Possible: yes, no
+# Default: no
+
+PKGSRC_USE_RELRO?= yes
+# Link with RELRO by default (on supported platforms). This makes the
+# exploitation of some security vulnerabilities more difficult in some cases.
+# Possible: yes, no
+# Default: yes
# The default PREFER_PKGSRC should be empty, but due to historical reasons we have the list below.
# Please add your platform here once you have confirmed it is correct
diff --git a/mk/platform/NetBSD.mk b/mk/platform/NetBSD.mk
index 79d9713..fc5e987 100644
--- a/mk/platform/NetBSD.mk
+++ b/mk/platform/NetBSD.mk
@@ -124,19 +124,34 @@ FFLAGS+= -mieee
PKG_HAVE_KQUEUE= # defined
.endif
-.if ${PKGSRC_USE_FORT:Uno} != "no"
-# build with fortify
-_GCC_CFLAGS+= -D_FORTIFY_SOURCE=2
-.endif
-
-.if ${PKGSRC_USE_SSP:Uno} != "no"
-. if (${MACHINE_ARCH} != "alpha") && \
+_OPSYS_SUPPORTS_FORTIFY=yes
+_FORTIFY_CFLAGS.gcc= -D_FORTIFY_SOURCE=2
+.if (${MACHINE_ARCH} != "alpha") && \
(${MACHINE_ARCH} != "hppa") && \
(${MACHINE_ARCH} != "ia64") && \
(${MACHINE_ARCH} != "mips")
-# build with stack protection (with GCC)
-_GCC_CFLAGS+= -fstack-protector
-. endif
+_OPSYS_SUPPORTS_SSP= yes
+_SSP_CFLAGS.gcc= -fstack-protector-all
+.endif
+
+.if (${MACHINE_ARCH} == "i386") || \
+ (${MACHINE_ARCH} == "sparc") || \
+ (${MACHINE_ARCH} == "sparc64") || \
+ (${MACHINE_ARCH} == "x86_64")
+_OPSYS_SUPPORTS_MKPIE= yes
+_MKPIE_CFLAGS.gcc= -fPIC
+#XXX for executables it should be:
+#_MKPIE_CFLAGS.gcc= -fPIE
+#XXX for libraries a sink wrapper around gcc is required and used instead
+#_MKPIE_LDFLAGS.gcc= -Wl,-pie
+.endif
+
+.if (${MACHINE_ARCH} == "i386") || \
+ (${MACHINE_ARCH} == "sparc") || \
+ (${MACHINE_ARCH} == "sparc64") || \
+ (${MACHINE_ARCH} == "x86_64")
+_OPSYS_SUPPORTS_RELRO= yes
+_RELRO_LDFLAGS.gcc= -Wl,-z,relro -Wl,-z,now
.endif
_OPSYS_CAN_CHECK_SHLIBS= yes # use readelf in check/bsd.check-vars.mk
diff --git a/mk/platform/SunOS.mk b/mk/platform/SunOS.mk
index c8a2f39..de2b08d 100644
--- a/mk/platform/SunOS.mk
+++ b/mk/platform/SunOS.mk
@@ -115,9 +115,17 @@ _OPSYS_SYSTEM_RPATH?= /lib${LIBABISUFFIX}:/usr/lib${LIBABISUFFIX}
_OPSYS_LIB_DIRS?= /lib${LIBABISUFFIX} /usr/lib${LIBABISUFFIX}
_OPSYS_INCLUDE_DIRS?= /usr/include
+_OPSYS_SUPPORTS_FORTIFY= yes
+_OPSYS_SUPPORTS_SSP= yes
+
_OPSYS_CAN_CHECK_SHLIBS= yes # requires readelf
# check for maximum command line length and set it in configure's environment,
# to avoid a test required by the libtool script that takes forever.
# FIXME: Adjust to work on this system and enable the lines below.
#_OPSYS_MAX_CMDLEN_CMD= /sbin/sysctl -n kern.argmax
+
+.if ${PKGSRC_USE_SSP:Uno} != "no"
+# build with stack protection (with GCC)
+_GCC_CFLAGS+= -fstack-protector
+.endif
diff --git a/mk/wrapper/arg-source b/mk/wrapper/arg-source
index 9336414..6810240 100644
--- a/mk/wrapper/arg-source
+++ b/mk/wrapper/arg-source
@@ -161,6 +161,12 @@ while $test $# -gt 0; do
##############################################################
-c|-S|-E)
dont_link=yes
+ dont_link_binary=yes
+ append_queue argbuf "$arg"
+ $debug_log $wrapperlog " (arg-source) push: $arg"
+ ;;
+ -shared)
+ dont_link_binary=yes
append_queue argbuf "$arg"
$debug_log $wrapperlog " (arg-source) push: $arg"
;;
diff --git a/mk/wrapper/bsd.wrapper.mk b/mk/wrapper/bsd.wrapper.mk
index 8f79a36..21c53e6 100644
--- a/mk/wrapper/bsd.wrapper.mk
+++ b/mk/wrapper/bsd.wrapper.mk
@@ -311,6 +311,11 @@ _WRAP_TRANSFORM.CXX= ${_WRAP_TRANSFORM.CC}
.if !empty(PKGSRC_COMPILER:Mgcc)
_WRAP_TRANSFORM.CC= ${WRAPPER_TMPDIR}/transform-gcc
_WRAP_TRANSFORM.CXX= ${_WRAP_TRANSFORM.CC}
+. if ${OPSYS} == "NetBSD"
+. if ${_PKGSRC_MKPIE} != "no"
+_WRAP_CMD_SINK.CC= ${WRAPPER_TMPDIR}/cmd-sink-netbsd-gcc
+. endif
+. endif
.endif
_WRAP_CMD_SINK.LD= ${WRAPPER_TMPDIR}/cmd-sink-ld
@@ -513,6 +518,7 @@ generate-wrappers: ${_target_}
cmd-sink-irix-cc \
cmd-sink-irix-ld \
cmd-sink-interix-gcc \
+ cmd-sink-netbsd-gcc \
cmd-sink-ld \
cmd-sink-osf1-cc \
cmd-sink-osf1-ld \
diff --git a/mk/wrapper/cmd-sink-netbsd-gcc b/mk/wrapper/cmd-sink-netbsd-gcc
new file mode 100644
index 0000000..89c19cf
--- /dev/null
+++ b/mk/wrapper/cmd-sink-netbsd-gcc
@@ -0,0 +1,54 @@
+# $NetBSD$
+#
+# Copyright (c) 2015 The NetBSD Foundation, Inc.
+# All rights reserved.
+#
+# This code is derived from software contributed to The NetBSD Foundation
+# by Pierre Pronchery.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+# 3. All advertising materials mentioning features or use of this software
+# must display the following acknowledgement:
+# This product includes software developed by the NetBSD
+# Foundation, Inc. and its contributors.
+# 4. Neither the name of The NetBSD Foundation nor the names of its
+# contributors may be used to endorse or promote products derived
+# from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
+# ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+# TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
+# BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+# POSSIBILITY OF SUCH DAMAGE.
+
+while ! queue_is_empty cmdbuf; do
+ pop_queue cmdbuf arg
+ $debug_log $wrapperlog " (cmd-sink-netbsd-gcc) pop: $arg"
+ case $arg in
+ *)
+ . $buildcmd
+ ;;
+ esac
+done
+
+# Append any optional flags required when linking binaries.
+if $test "$dont_link_binary" != "yes"; then
+ # XXX obtain these flags from PIE_LDFLAGS
+ for arg in -Wl,-pie -shared-libgcc; do
+ $debug_log $wrapperlog " (cmd-sink-netbsd-gcc) pop: $arg"
+ . $buildcmd
+ done
+fi
diff --git a/mk/wrapper/transform-gcc b/mk/wrapper/transform-gcc
index 3dd6624..75a04b9 100644
--- a/mk/wrapper/transform-gcc
+++ b/mk/wrapper/transform-gcc
@@ -43,10 +43,14 @@ case $arg in
-fomit-frame-pointer |\
-fPIC |\
-fpic |\
+-fPIE |\
+-fpie |\
-fpcc-struct-return |\
-freg-struct-return |\
-frename-registers |\
-fsigned-char |\
+-fstack-protector |\
+-fstack-protector-all |\
-funroll-loops |\
-funsigned-char |\
-fweb |\
@@ -74,12 +78,14 @@ case $arg in
-print-search-dirs |\
-S |\
-shared |\
+-shared-libgcc |\
-static |\
-std=c99 |\
-std=gnu89 |\
-std=gnu99 |\
-W |\
-W[cLlS],* |\
+-Waggregate-return |\
-Wall |\
-Wbounded |\
-Wcast-align |\
@@ -90,6 +96,7 @@ case $arg in
-Werror |\
-Werror-implicit-function-declaration |\
-Wformat* |\
+-Winline |\
-Wmissing-declarations |\
-Wmissing-format-attribute |\
-Wmissing-prototypes |\
@@ -110,12 +117,14 @@ case $arg in
-Wno-write-strings |\
-Wparentheses |\
-Wpointer-arith |\
+-Wredundant-decls |\
-Wreturn-type |\
-Wshadow |\
-Wsign-compare |\
-Wstrict-aliasing |\
-Wstrict-prototypes |\
-Wswitch |\
+-Wtrigraphs |\
-Wunused |\
-Wundef |\
-Wwrite-strings ) transform_pass ;;
Attachment:
signature.asc
Description: OpenPGP digital signature