[Buildroot] [PATCH v7 0/4] Hardening Wrapper Updates and Test

[Buildroot] [PATCH v7 0/4] Hardening Wrapper Updates and Test

[Matt Weber]

This series looks to update the implementation and migration of
hardening related options to the compiler wrapper.

The series also provides runtime testing for RELRO/SSP/FORTIFY.

Overall test of features
-------------------------------------
support/testing/run-tests -k -o ~/runtime_test_tmp -d ~/dl_tmp \
 tests.core.test_hardening.TestFortifyConserv \
 tests.core.test_hardening.TestFortifyNone \
 tests.core.test_hardening.TestRelro \
 tests.core.test_hardening.TestRelroPartial \
 tests.core.test_hardening.TestSspNone \
 tests.core.test_hardening.TestSspStrong

Changes
--------------------------------------------------
v6 -> v7
 - Fixed a last minute endif rebase bug in the v6 series

v5 -> v6
 - Moved all RELRO/PIE handling to GCC frontend wrapper.
 - Updated PIE disable conditions and added comments
 - Updated comments in code and on patches to make design choices
   clear

v4 -> v5
 - RELRO patch updated to handle link time -r represented also as
   -wl,r

v3 -> v4
 - RELRO/PIE patch updated to solely use the wrapper.  I didn't
   understand how the specfiles where used and thought I needed to
   do something similar during the use of LD.  That is not the case.
   GCC compile wrapper has been updated to handle CC and LD options
   required for this feature.  Testing with verification using the
   checksec tool confirms the intended behavior is close to identical
   between the specfile approach and wrapper.  Wrapper actually is
   just slightly better since the specfile relied on FLAGS being
   correctly used.

v2 -> v3
 - Realized the complexity of having a link wrapper application vs
   using a combo of link specfile and GCC wrapper.  This patchset
   presents that hybrid approach and has updated comments on the
   patches implementing this concept to support the discussion.
 - Added additional detail to descriptions and test cases to this
   cover letter

v1 -> v2
 - There were issues when I started regression testing where
   packages where providing multiple pie/pic/shared args on
   a single call of gcc/ld.

Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
CC: Jan Kundr?t <jan.kundrat@cesnet.cz>
CC: Stefan S?rensen <stefan.sorensen@spectralink.com>

Matt Weber (4):
  toolchain/toolchain-wrapper: add BR2_RELRO_
  toolchain/toolchain-wrapper: add BR2_SSP_* support
  BR2_FORTIFY*: toolchain wrapper limitation note
  support/testing/tests/core: SSP & hardening flags

 .gitlab-ci.yml                               |   6 ++
 package/Makefile.in                          |  28 +++----
 support/testing/tests/core/test_hardening.py | 110 +++++++++++++++++++++++++++
 toolchain/toolchain-wrapper.c                |  91 +++++++++++++++++++++-
 toolchain/toolchain-wrapper.mk               |  14 ++++
 5 files changed, 228 insertions(+), 21 deletions(-)
 create mode 100644 support/testing/tests/core/test_hardening.py

-- 
1.9.1

[Buildroot] [PATCH v7 1/4] toolchain/toolchain-wrapper: add BR2_RELRO_

[Matt Weber]

The RELRO/PIE flags are currently passed via CFLAGS/LDFLAGS and this patch
proposes moving them to the toolchain wrapper.

 (1) The flags should _always_ be passed, without leaving the possibility
     for any package to ignore them. I.e, when BR2_RELRO_FULL=y is used
     in a build, all executables should be built PIE. Passing those
     options through the wrapper ensures they are used during the build
     of all packages.

 (2) Some options are incompatible with -fPIE. For example, when
     building object files for a shared libraries, -fPIC is used, and
     -fPIE shouldn't be used in combination with -fPIE. Similarly, -r
     or -static are directly incompatible as they are different link
     time behaviors then the intent of PIE. Passing those options
     through the wrapper allows to add some "smart" logic to only pass
     -fPIE/-pie when relevant.

 (3) Some toolchain, kernel and bootloader packages may want to
     explicitly disable PIE in a build where the rest of the userspace
     has intentionally enabled it. The wrapper provides an option
     to key on the -fno-pie/-no-pie and bypass the appending of RELRO
     flags.
     The current Kernel and U-boot source trees include this option.
     https://github.com/torvalds/linux/commit/8438ee76b004ef66d125ade64c91fc128047d244
     https://github.com/u-boot/u-boot/commit/6ace36e19a8cfdd16ce7c02625edf36864897bf5
     If using PIE with a older Kernel and/or U-boot version, a backport of these
     changes  might be required. However this patchset also uses the
     __KERNEL__ and __UBOOT__ defines as a way to disable PIE.

NOTE: The current implementation via CFLAGS/LDFLAGS is has caused some
build time failures as the conditional logic doesn't yet exist in
Buildroot.
https://bugs.busybox.net/show_bug.cgi?id=11206
https://bugs.busybox.net/show_bug.cgi?id=11321

Good summary of the most common build failures related to
enabling pie. https://wiki.ubuntu.com/SecurityTeam/PIE

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>

---
Changes
v6 -> v7
 - Fixed rebase error with a stray endif

v5 -> v6
[Thomas
 - Fixed commit logs description to describe the change
 - Moved relro partial options to wrapper and updated this commit
   title to make the patch generically capture the move of _RELRO_
 - Added comment to wrapper and commit about use of no-pie
 - Added a comment in the wrapper and above to cover conversation on
   Linux kernel and uboot no-pie fix-ups. Plus added a condition
   checking for the __KERNEL__ and __UBOOT__ defines
 - Add comment that -fPIE and -pie can interchangeably be used during
   link/compile and are correctly ignored when not used.

[Arnout
 - Collapsed the -pie conditional in the wrapper to be within the
   check for use of -fPIE. Also resolves missing -Wl,-r in the -fPIE loop
 - Removed specfile and consolidated into just the gcc frontend wrapper

v4 -> v5
 - Found -r can also be -wl,r. Wrapper has been updated to cover
   that case.
v3 -> v4
[Matt W
 - Added no-pie/pic options to allow per package tweaking of
   wrapper setting the option.  This lines up with other distros
[Arnout
 - After realizing that a LD invocation doesn't use the specfile,
   I agree with Arnout and have tested the solution can completely
   occur within the compiler wrapper. This version updates to just
   have modifications in the wrapper.

v2 -> v3
 - Fell back on a linker approach using a spec file and kept compiler
   flag tweaks in the wrapper

v1 -> v2
 - Reworked handling of pie/pic/shared to replace each time they
   occur with a dummy string and then insert the right combination
   when rebuilding the exec string.
 - Fixed mix of tabs and spaces
 - Swapped order of shared and pie.  Originally coded it backwards.
---
 package/Makefile.in            | 11 ------
 toolchain/toolchain-wrapper.c  | 82 ++++++++++++++++++++++++++++++++++++++++--
 toolchain/toolchain-wrapper.mk |  6 ++++
 3 files changed, 86 insertions(+), 13 deletions(-)

diff --git a/package/Makefile.in b/package/Makefile.in
index abfdb81..cd21482 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -141,9 +141,6 @@ ifeq ($(BR2_DEBUG_3),y)
 TARGET_DEBUGGING = -g3
 endif
 
-TARGET_CFLAGS_RELRO = -Wl,-z,relro
-TARGET_CFLAGS_RELRO_FULL = -Wl,-z,now $(TARGET_CFLAGS_RELRO)
-
 TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS))
 
 ifeq ($(BR2_SSP_REGULAR),y)
@@ -154,14 +151,6 @@ else ifeq ($(BR2_SSP_ALL),y)
 TARGET_HARDENED += -fstack-protector-all
 endif
 
-ifeq ($(BR2_RELRO_PARTIAL),y)
-TARGET_HARDENED += $(TARGET_CFLAGS_RELRO)
-TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO)
-else ifeq ($(BR2_RELRO_FULL),y)
-TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL)
-TARGET_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL)
-endif
-
 ifeq ($(BR2_FORTIFY_SOURCE_1),y)
 TARGET_HARDENED += -D_FORTIFY_SOURCE=1
 else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
index c5eb813..153ba87 100644
--- a/toolchain/toolchain-wrapper.c
+++ b/toolchain/toolchain-wrapper.c
@@ -49,8 +49,12 @@ static char _date_[sizeof("-D__DATE__=\"MMM DD YYYY\"")];
  * 	-D__TIME__=
  * 	-D__DATE__=
  * 	-Wno-builtin-macro-redefined
+ * 	-Wl,-z,now
+ * 	-Wl,-z,relro
+ * 	-fPIE
+ * 	-pie
  */
-#define EXCLUSIVE_ARGS	6
+#define EXCLUSIVE_ARGS	10
 
 static char *predef_args[] = {
 #ifdef BR_CCACHE
@@ -236,7 +240,8 @@ int main(int argc, char **argv)
 	char *env_debug;
 	char *paranoid_wrapper;
 	int paranoid;
-	int ret, i, count = 0, debug;
+	int ret, i, j, count = 0, debug;
+	unsigned int found_shared = 0;
 
 	/* Calculate the relative paths */
 	basename = strrchr(progpath, '/');
@@ -363,6 +368,79 @@ int main(int argc, char **argv)
 		*cur++ = "-Wno-builtin-macro-redefined";
 	}
 
+#ifdef BR2_RELRO_FULL
+	/* Patterned after Fedora/Gentoo hardening approaches.
+	 * https://fedoraproject.org/wiki/Changes/Harden_All_Packages
+	 * https://wiki.gentoo.org/wiki/Hardened/Toolchain#Position_Independent_Executables_.28PIEs.29
+	 *
+	 * A few checks are added to enable disabling of PIE
+	 * 1) -fno-pie and -no-pie are used by other distros to disable PIE in
+	 *    cases where the compiler enables it by default. The logic below
+	 *    maintains that behavior.
+	 *         Ref: https://wiki.ubuntu.com/SecurityTeam/PIE
+	 * 2) A check for -fno-PIE has been used in older Linux Kernel builds
+	 *    in a similar way to -fno-pie or -no-pie.
+	 * 3) A check is added for Kernel and U-boot defines
+	 *    (-D__KERNEL__ and -D__UBOOT__).
+	 */
+	for (i = 1; i < argc; i++) {
+		/* Apply all incompatible link flag and disable checks first */
+		if (!strcmp(argv[i], "-r") ||
+		    !strcmp(argv[i], "-Wl,-r") ||
+		    !strcmp(argv[i], "-static") ||
+		    !strcmp(argv[i], "-D__KERNEL__") ||
+		    !strcmp(argv[i], "-D__UBOOT__") ||
+		    !strcmp(argv[i], "-fno-pie") ||
+		    !strcmp(argv[i], "-fno-PIE") ||
+		    !strcmp(argv[i], "-no-pie"))
+			break;
+		/* Record that shared was present which disables -pie but don't
+		 * break out of loop as a check needs to occur that possibly
+		 * still allows -fPIE to be set
+		 */
+		if (!strcmp(argv[i], "-shared"))
+			found_shared = 1;
+	}
+	if (i == argc) {
+		/* Compile and link condition checking have been kept split
+		 * between these two loops, as there maybe already valid
+		 * compile flags set for position independence. In that case
+		 * the wrapper just adds the -pie for link.
+		 */
+		for (j = 1; j < argc; j++) {
+			if (!strcmp(argv[j], "-fpie") ||
+			    !strcmp(argv[j], "-fPIE") ||
+			    !strcmp(argv[j], "-fpic") ||
+			    !strcmp(argv[j], "-fPIC"))
+				break;
+		}
+		/* Both args below can be set@compile/link time
+                 * and are ignored correctly when not used
+                 */
+		if(j == argc)
+			*cur++ = "-fPIE";
+
+		if (!found_shared)
+			*cur++ = "-pie";
+	}
+#endif
+	/* Are we building the Linux Kernel or U-Boot? */
+	for (i = 1; i < argc; i++) {
+		if (!strcmp(argv[i], "-D__KERNEL__") ||
+		    !strcmp(argv[i], "-D__UBOOT__"))
+			break;
+	}
+	if (i == argc) {
+		/* https://wiki.gentoo.org/wiki/Hardened/Toolchain#Mark_Read-Only_Appropriate_Sections */
+#ifdef BR2_RELRO_PARTIAL
+		*cur++ = "-Wl,-z,relro";
+#endif
+#ifdef BR2_RELRO_FULL
+		*cur++ = "-Wl,-z,now";
+		*cur++ = "-Wl,-z,relro";
+#endif
+	}
+	
 	paranoid_wrapper = getenv("BR_COMPILER_PARANOID_UNSAFE_PATH");
 	if (paranoid_wrapper && strlen(paranoid_wrapper) > 0)
 		paranoid = 1;
diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
index b8074ef..99d3039 100644
--- a/toolchain/toolchain-wrapper.mk
+++ b/toolchain/toolchain-wrapper.mk
@@ -45,6 +45,12 @@ ifeq ($(BR2_CCACHE_USE_BASEDIR),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR_CCACHE_BASEDIR='"$(BASE_DIR)"'
 endif
 
+ifeq ($(BR2_RELRO_PARTIAL),y)
+TOOLCHAIN_WRAPPER_ARGS += -DBR2_RELRO_PARTIAL
+else ifeq ($(BR2_RELRO_FULL),y)
+TOOLCHAIN_WRAPPER_ARGS += -DBR2_RELRO_FULL
+endif
+
 define TOOLCHAIN_WRAPPER_BUILD
 	$(HOSTCC) $(HOST_CFLAGS) $(TOOLCHAIN_WRAPPER_ARGS) \
 		-s -Wl,--hash-style=$(TOOLCHAIN_WRAPPER_HASH_STYLE) \
-- 
1.9.1

[Buildroot] [PATCH v7 1/4] toolchain/toolchain-wrapper: add BR2_RELRO_

[Peter Korsgaard]

>>>>> "Matt" == Matt Weber <matthew.weber@rockwellcollins.com> writes:

 > The RELRO/PIE flags are currently passed via CFLAGS/LDFLAGS and this patch
 > proposes moving them to the toolchain wrapper.

 >  (1) The flags should _always_ be passed, without leaving the possibility
 >      for any package to ignore them. I.e, when BR2_RELRO_FULL=y is used
 >      in a build, all executables should be built PIE. Passing those
 >      options through the wrapper ensures they are used during the build
 >      of all packages.

 >  (2) Some options are incompatible with -fPIE. For example, when
 >      building object files for a shared libraries, -fPIC is used, and
 >      -fPIE shouldn't be used in combination with -fPIE. Similarly, -r
 >      or -static are directly incompatible as they are different link
 >      time behaviors then the intent of PIE. Passing those options
 >      through the wrapper allows to add some "smart" logic to only pass
 >      -fPIE/-pie when relevant.

 >  (3) Some toolchain, kernel and bootloader packages may want to
 >      explicitly disable PIE in a build where the rest of the userspace
 >      has intentionally enabled it. The wrapper provides an option
 >      to key on the -fno-pie/-no-pie and bypass the appending of RELRO
 >      flags.
 >      The current Kernel and U-boot source trees include this option.
 >      https://github.com/torvalds/linux/commit/8438ee76b004ef66d125ade64c91fc128047d244
 >      https://github.com/u-boot/u-boot/commit/6ace36e19a8cfdd16ce7c02625edf36864897bf5
 >      If using PIE with a older Kernel and/or U-boot version, a backport of these
 >      changes  might be required. However this patchset also uses the
 >      __KERNEL__ and __UBOOT__ defines as a way to disable PIE.

 > NOTE: The current implementation via CFLAGS/LDFLAGS is has caused some
 > build time failures as the conditional logic doesn't yet exist in
 > Buildroot.
 > https://bugs.busybox.net/show_bug.cgi?id=11206
 > https://bugs.busybox.net/show_bug.cgi?id=11321

 > Good summary of the most common build failures related to
 > enabling pie. https://wiki.ubuntu.com/SecurityTeam/PIE

 > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>

 > ---
 > Changes
 > v6 -> v7
 >  - Fixed rebase error with a stray endif

 > v5 -> v6
 > [Thomas
 >  - Fixed commit logs description to describe the change
 >  - Moved relro partial options to wrapper and updated this commit
 >    title to make the patch generically capture the move of _RELRO_
 >  - Added comment to wrapper and commit about use of no-pie
 >  - Added a comment in the wrapper and above to cover conversation on
 >    Linux kernel and uboot no-pie fix-ups. Plus added a condition
 >    checking for the __KERNEL__ and __UBOOT__ defines
 >  - Add comment that -fPIE and -pie can interchangeably be used during
 >    link/compile and are correctly ignored when not used.

 > [Arnout
 >  - Collapsed the -pie conditional in the wrapper to be within the
 >    check for use of -fPIE. Also resolves missing -Wl,-r in the -fPIE loop
 >  - Removed specfile and consolidated into just the gcc frontend wrapper

 > v4 -> v5
 >  - Found -r can also be -wl,r. Wrapper has been updated to cover
 >    that case.
 > v3 -> v4
 > [Matt W
 >  - Added no-pie/pic options to allow per package tweaking of
 >    wrapper setting the option.  This lines up with other distros
 > [Arnout
 >  - After realizing that a LD invocation doesn't use the specfile,
 >    I agree with Arnout and have tested the solution can completely
 >    occur within the compiler wrapper. This version updates to just
 >    have modifications in the wrapper.

 > v2 -> v3
 >  - Fell back on a linker approach using a spec file and kept compiler
 >    flag tweaks in the wrapper

 > v1 -> v2
 >  - Reworked handling of pie/pic/shared to replace each time they
 >    occur with a dummy string and then insert the right combination
 >    when rebuilding the exec string.
 >  - Fixed mix of tabs and spaces
 >  - Swapped order of shared and pie.  Originally coded it backwards.
 > ---
 >  package/Makefile.in            | 11 ------
 >  toolchain/toolchain-wrapper.c  | 82 ++++++++++++++++++++++++++++++++++++++++--
 >  toolchain/toolchain-wrapper.mk |  6 ++++
 >  3 files changed, 86 insertions(+), 13 deletions(-)

 > diff --git a/package/Makefile.in b/package/Makefile.in
 > index abfdb81..cd21482 100644
 > --- a/package/Makefile.in
 > +++ b/package/Makefile.in
 > @@ -141,9 +141,6 @@ ifeq ($(BR2_DEBUG_3),y)
 >  TARGET_DEBUGGING = -g3
 >  endif
 
 > -TARGET_CFLAGS_RELRO = -Wl,-z,relro
 > -TARGET_CFLAGS_RELRO_FULL = -Wl,-z,now $(TARGET_CFLAGS_RELRO)
 > -
 >  TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS))
 
 >  ifeq ($(BR2_SSP_REGULAR),y)
 > @@ -154,14 +151,6 @@ else ifeq ($(BR2_SSP_ALL),y)
 >  TARGET_HARDENED += -fstack-protector-all
 >  endif
 
 > -ifeq ($(BR2_RELRO_PARTIAL),y)
 > -TARGET_HARDENED += $(TARGET_CFLAGS_RELRO)
 > -TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO)
 > -else ifeq ($(BR2_RELRO_FULL),y)
 > -TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL)
 > -TARGET_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL)
 > -endif
 > -
 >  ifeq ($(BR2_FORTIFY_SOURCE_1),y)
 >  TARGET_HARDENED += -D_FORTIFY_SOURCE=1
 >  else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
 > diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
 > index c5eb813..153ba87 100644
 > --- a/toolchain/toolchain-wrapper.c
 > +++ b/toolchain/toolchain-wrapper.c
 > @@ -49,8 +49,12 @@ static char _date_[sizeof("-D__DATE__=\"MMM DD YYYY\"")];
 >   * 	-D__TIME__=
 >   * 	-D__DATE__=
 >   * 	-Wno-builtin-macro-redefined
 > + * 	-Wl,-z,now
 > + * 	-Wl,-z,relro
 > + * 	-fPIE
 > + * 	-pie
 >   */
 > -#define EXCLUSIVE_ARGS	6
 > +#define EXCLUSIVE_ARGS	10
 
 >  static char *predef_args[] = {
 >  #ifdef BR_CCACHE
 > @@ -236,7 +240,8 @@ int main(int argc, char **argv)
 >  	char *env_debug;
 >  	char *paranoid_wrapper;
 >  	int paranoid;
 > -	int ret, i, count = 0, debug;
 > +	int ret, i, j, count = 0, debug;
 > +	unsigned int found_shared = 0;

There is not really any specific reason why found_shared is unsigned, so
I've dropped that.


 >  	/* Calculate the relative paths */
 >  	basename = strrchr(progpath, '/');
 > @@ -363,6 +368,79 @@ int main(int argc, char **argv)
 >  		*cur++ = "-Wno-builtin-macro-redefined";
 >  	}
 
 > +#ifdef BR2_RELRO_FULL
 > +	/* Patterned after Fedora/Gentoo hardening approaches.
 > +	 * https://fedoraproject.org/wiki/Changes/Harden_All_Packages
 > +	 * https://wiki.gentoo.org/wiki/Hardened/Toolchain#Position_Independent_Executables_.28PIEs.29
 > +	 *
 > +	 * A few checks are added to enable disabling of PIE

enable disabling sounds quite confusing to me, so I've changed it to
"to allow disabling of PIE"

> +	 * 1) -fno-pie and -no-pie are used by other distros to disable PIE in
 > +	 *    cases where the compiler enables it by default. The logic below
 > +	 *    maintains that behavior.
 > +	 *         Ref: https://wiki.ubuntu.com/SecurityTeam/PIE
 > +	 * 2) A check for -fno-PIE has been used in older Linux Kernel builds
 > +	 *    in a similar way to -fno-pie or -no-pie.
 > +	 * 3) A check is added for Kernel and U-boot defines
 > +	 *    (-D__KERNEL__ and -D__UBOOT__).
 > +	 */
 > +	for (i = 1; i < argc; i++) {
 > +		/* Apply all incompatible link flag and disable checks first */
 > +		if (!strcmp(argv[i], "-r") ||
 > +		    !strcmp(argv[i], "-Wl,-r") ||
 > +		    !strcmp(argv[i], "-static") ||
 > +		    !strcmp(argv[i], "-D__KERNEL__") ||
 > +		    !strcmp(argv[i], "-D__UBOOT__") ||
 > +		    !strcmp(argv[i], "-fno-pie") ||
 > +		    !strcmp(argv[i], "-fno-PIE") ||
 > +		    !strcmp(argv[i], "-no-pie"))
 > +			break;
 > +		/* Record that shared was present which disables -pie but don't
 > +		 * break out of loop as a check needs to occur that possibly
 > +		 * still allows -fPIE to be set
 > +		 */
 > +		if (!strcmp(argv[i], "-shared"))
 > +			found_shared = 1;
 > +	}
 > +	if (i == argc) {

It would IMHO have been a bit clearer to add the found_shared / -fpie
logic here right after the check, but OK.


 > +		/* Compile and link condition checking have been kept split
 > +		 * between these two loops, as there maybe already valid

This also sounds a bit odd to me. I've added "are", E.G. "as there maybe
already are valid"


 > +		 * compile flags set for position independence. In that case
 > +		 * the wrapper just adds the -pie for link.
 > +		 */
 > +		for (j = 1; j < argc; j++) {

There is not really any reason to use a different loop iteration
variable than 'i' here, so I've dropped the 'j' variable.


> +			if (!strcmp(argv[j], "-fpie") ||
 > +			    !strcmp(argv[j], "-fPIE") ||
 > +			    !strcmp(argv[j], "-fpic") ||
 > +			    !strcmp(argv[j], "-fPIC"))
 > +				break;
 > +		}
 > +		/* Both args below can be set at compile/link time
 > +                 * and are ignored correctly when not used
 > +                 */

Indentation looks a bit odd here, fixed.

 > +#ifdef BR2_RELRO_FULL
 > +		*cur++ = "-Wl,-z,now";
 > +		*cur++ = "-Wl,-z,relro";
 > +#endif
 > +	}
 > +	

Trailing spaces.

Committed with these minor fixes, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v7 2/4] toolchain/toolchain-wrapper: add BR2_SSP_* support

[Matt Weber]

Migrate the stack protection flag management into the wrapper.

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>

---
Changes

v2 -> v7
 - Rebased, nothing functional

v1 -> v2
 - None.
---
 package/Makefile.in            | 8 --------
 toolchain/toolchain-wrapper.c  | 9 +++++++++
 toolchain/toolchain-wrapper.mk | 8 ++++++++
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/package/Makefile.in b/package/Makefile.in
index cd21482..dc0eecf 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -143,14 +143,6 @@ endif
 
 TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS))
 
-ifeq ($(BR2_SSP_REGULAR),y)
-TARGET_HARDENED += -fstack-protector
-else ifeq ($(BR2_SSP_STRONG),y)
-TARGET_HARDENED += -fstack-protector-strong
-else ifeq ($(BR2_SSP_ALL),y)
-TARGET_HARDENED += -fstack-protector-all
-endif
-
 ifeq ($(BR2_FORTIFY_SOURCE_1),y)
 TARGET_HARDENED += -D_FORTIFY_SOURCE=1
 else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
index 153ba87..81d4672 100644
--- a/toolchain/toolchain-wrapper.c
+++ b/toolchain/toolchain-wrapper.c
@@ -98,6 +98,15 @@ static char *predef_args[] = {
 #if defined(BR_MIPS_TARGET_BIG_ENDIAN) || defined(BR_ARC_TARGET_BIG_ENDIAN)
 	"-EB",
 #endif
+#ifdef BR_SSP_REGULAR
+	"-fstack-protector",
+#endif
+#ifdef BR_SSP_STRONG
+	"-fstack-protector-strong",
+#endif
+#ifdef BR_SSP_ALL
+	"-fstack-protector-all",
+#endif
 #ifdef BR_ADDITIONAL_CFLAGS
 	BR_ADDITIONAL_CFLAGS
 #endif
diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
index 99d3039..613f5f6 100644
--- a/toolchain/toolchain-wrapper.mk
+++ b/toolchain/toolchain-wrapper.mk
@@ -51,6 +51,14 @@ else ifeq ($(BR2_RELRO_FULL),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR2_RELRO_FULL
 endif
 
+ifeq ($(BR2_SSP_REGULAR),y)
+TOOLCHAIN_WRAPPER_ARGS += -DBR_SSP_REGULAR
+else ifeq ($(BR2_SSP_STRONG),y)
+TOOLCHAIN_WRAPPER_ARGS += -DBR_SSP_STRONG
+else ifeq ($(BR2_SSP_ALL),y)
+TOOLCHAIN_WRAPPER_ARGS += -DBR_SSP_ALL
+endif
+
 define TOOLCHAIN_WRAPPER_BUILD
 	$(HOSTCC) $(HOST_CFLAGS) $(TOOLCHAIN_WRAPPER_ARGS) \
 		-s -Wl,--hash-style=$(TOOLCHAIN_WRAPPER_HASH_STYLE) \
-- 
1.9.1

[Buildroot] [PATCH v7 2/4] toolchain/toolchain-wrapper: add BR2_SSP_* support

[Peter Korsgaard]

>>>>> "Matt" == Matt Weber <matthew.weber@rockwellcollins.com> writes:

 > Migrate the stack protection flag management into the wrapper.
 > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>

 > ---
 > Changes

 > v2 -> v7
 >  - Rebased, nothing functional

 > v1 -> v2
 >  - None.

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v7 3/4] BR2_FORTIFY*: toolchain wrapper limitation note

[Matt Weber]

A note is added to tie off the discussion on why moving _FORTIFY_SOURCE
related flags into the toolchain wrapper doesn't currently work.

 - Currently -D_FORTIFY_SOURCE and optimizations are passed through
   CFLAGS

 - Packages like linux-tools ignore CFLAGS entirely and some
   autotools toolchain testing cases dependent on not using
   CFLAGS.

 - If FORTIFY_SOURCE is passed through the wrapper, then linux-tools
   will no longer be able to ignore it, because it's enforced at a
   lower-level and since the optimization -Os/g/1/2/3 are via CFLAGS,
   there is no optimization flag set.  Therefore linux-tools will do
   all its configuration tests with FORTIFY_SOURCE forcefully enabled
   at the wrapper level, but no optimization enabled, and consequently
   tests will fail.

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
---

Changes
v6 -> v7
 - No changes

v5 -> v6
[Thomas P
 - Updated makefile comment and description in the patch to clearly
   state the example case that warrants the change.

v4 -> v5
 - No changes

v3
 - New patch
---
 package/Makefile.in | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/package/Makefile.in b/package/Makefile.in
index dc0eecf..44761b7 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -143,6 +143,15 @@ endif
 
 TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS))
 
+# By design, _FORTIFY_SOURCE requires gcc optimization to be enabled.
+# Therefore, we need to pass _FORTIFY_SOURCE and the optimization level
+# through the same mechanism, i.e currently through CFLAGS. Passing
+# _FORTIFY_SOURCE through the wrapper and the optimization level
+# through CFLAGS would not work, because CFLAGS are sometimes
+# ignored/overridden by packages, but the flags passed by the wrapper
+# are enforced: this would cause _FORTIFY_SOURCE to be used without any
+# optimization level, leading to a build / configure failure. So we keep
+# passing _FORTIFY_SOURCE and the optimization level both through CFLAGS.
 ifeq ($(BR2_FORTIFY_SOURCE_1),y)
 TARGET_HARDENED += -D_FORTIFY_SOURCE=1
 else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
-- 
1.9.1

[Buildroot] [PATCH v7 3/4] BR2_FORTIFY*: toolchain wrapper limitation note

[Peter Korsgaard]

>>>>> "Matt" == Matt Weber <matthew.weber@rockwellcollins.com> writes:

 > A note is added to tie off the discussion on why moving _FORTIFY_SOURCE
 > related flags into the toolchain wrapper doesn't currently work.

 >  - Currently -D_FORTIFY_SOURCE and optimizations are passed through
 >    CFLAGS

 >  - Packages like linux-tools ignore CFLAGS entirely and some
 >    autotools toolchain testing cases dependent on not using
 >    CFLAGS.

 >  - If FORTIFY_SOURCE is passed through the wrapper, then linux-tools
 >    will no longer be able to ignore it, because it's enforced at a
 >    lower-level and since the optimization -Os/g/1/2/3 are via CFLAGS,
 >    there is no optimization flag set.  Therefore linux-tools will do
 >    all its configuration tests with FORTIFY_SOURCE forcefully enabled
 >    at the wrapper level, but no optimization enabled, and consequently
 >    tests will fail.

 > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v7 4/4] support/testing/tests/core: SSP & hardening flags

[Matt Weber]

Catch the commonly used options of SSP, Relro, and fortify.
Using the package targets of busybox and lighttpd.  This
can easily be expanded to a larger list.

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>

---
Changes
v1 -> v7 (link wrapper patchset)
 - None

Original v4 -> v1 of link wrapper patchset
 - Added back in busybox test case

v3 -> v4
 - Removed commented out lines I missed when I removed busybox
 - Removed duplicate fortify assertion test

v2 -> v3
[Matt
 - Removed the busybox target as without the link time
   wrapper/specfile being merged the build will fail.
   Link time conflict between use of 'r' and pie.

[Thomas
 - Add clarificaion of what checksec can test
 - Reworked using inheritance
 - Relocated json load (removed duplication)

v1 -> v2
[Ricardo
 - Fix flake8 warnings
 - Added missing busyfox pie assertions
 - Updated the yml to include new test cases
---
 .gitlab-ci.yml                               |   6 ++
 support/testing/tests/core/test_hardening.py | 110 +++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)
 create mode 100644 support/testing/tests/core/test_hardening.py

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index d3d544b..8f325cd 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -263,6 +263,12 @@ tests.boot.test_atf.TestATFAllwinner: *runtime_test
 tests.boot.test_atf.TestATFMarvell: *runtime_test
 tests.boot.test_atf.TestATFVexpress: *runtime_test
 tests.core.test_file_capabilities.TestFileCapabilities: *runtime_test
+tests.core.test_hardening.TestFortifyConserv: *runtime_test
+tests.core.test_hardening.TestFortifyNone: *runtime_test
+tests.core.test_hardening.TestRelro: *runtime_test
+tests.core.test_hardening.TestRelroPartial: *runtime_test
+tests.core.test_hardening.TestSspNone: *runtime_test
+tests.core.test_hardening.TestSspStrong: *runtime_test
 tests.core.test_post_scripts.TestPostScripts: *runtime_test
 tests.core.test_rootfs_overlay.TestRootfsOverlay: *runtime_test
 tests.core.test_timezone.TestGlibcAllTimezone: *runtime_test
diff --git a/support/testing/tests/core/test_hardening.py b/support/testing/tests/core/test_hardening.py
new file mode 100644
index 0000000..9f26962
--- /dev/null
+++ b/support/testing/tests/core/test_hardening.py
@@ -0,0 +1,110 @@
+import os
+import subprocess
+import json
+
+import infra.basetest
+
+
+class TestHardeningBase(infra.basetest.BRTest):
+    config = \
+        """
+        BR2_powerpc64=y
+        BR2_powerpc_e5500=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
+        BR2_TOOLCHAIN_EXTERNAL_URL="https://toolchains.bootlin.com/downloads/releases/toolchains/powerpc64-e5500/tarballs/powerpc64-e5500--glibc--stable-2018.02-2.tar.bz2"
+        BR2_TOOLCHAIN_EXTERNAL_GCC_6=y
+        BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_1=y
+        BR2_TOOLCHAIN_EXTERNAL_CUSTOM_GLIBC=y
+        BR2_TOOLCHAIN_EXTERNAL_CXX=y
+        BR2_PACKAGE_LIGHTTPD=y
+        BR2_PACKAGE_HOST_CHECKSEC=y
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
+
+    checksec_files = ["usr/sbin/lighttpd","bin/busybox"]
+
+    def checksec_run(self, target_file):
+        filepath = os.path.join(self.builddir, "target", target_file)
+        cmd = ["host/bin/checksec", "--output", "json", "--file", filepath]
+        # Checksec is being used for elf file analysis only.  There are no
+        # assumptions of target/run-time checks as part of this testing.
+        ret = subprocess.check_output(cmd,
+                                      stderr=open(os.devnull, "w"),
+                                      cwd=self.builddir,
+                                      env={"LANG": "C"})
+        return json.loads(ret)
+
+
+class TestRelro(TestHardeningBase):
+    config = TestHardeningBase.config + \
+        """
+        BR2_RELRO_FULL=y
+        """
+
+    def test_run(self):
+        for f in self.checksec_files:
+            out = self.checksec_run(f)
+            self.assertEqual(out["file"]["relro"], "full")
+            self.assertEqual(out["file"]["pie"], "yes")
+
+
+class TestRelroPartial(TestHardeningBase):
+    config = TestHardeningBase.config + \
+        """
+        BR2_RELRO_PARTIAL=y
+        """
+
+    def test_run(self):
+        for f in self.checksec_files:
+            out = self.checksec_run(f)
+            self.assertEqual(out["file"]["relro"], "partial")
+            self.assertEqual(out["file"]["pie"], "no")
+
+
+class TestSspNone(TestHardeningBase):
+    config = TestHardeningBase.config + \
+        """
+        BR2_SSP_NONE=y
+        """
+
+    def test_run(self):
+        for f in self.checksec_files:
+            out = self.checksec_run(f)
+            self.assertEqual(out["file"]["canary"], "no")
+
+
+class TestSspStrong(TestHardeningBase):
+    config = TestHardeningBase.config + \
+        """
+        BR2_SSP_STRONG=y
+        """
+
+    def test_run(self):
+        for f in self.checksec_files:
+            out = self.checksec_run(f)
+            self.assertEqual(out["file"]["canary"], "yes")
+
+
+class TestFortifyNone(TestHardeningBase):
+    config = TestHardeningBase.config + \
+        """
+        BR2_FORTIFY_SOURCE_NONE=y
+        """
+
+    def test_run(self):
+        for f in self.checksec_files:
+            out = self.checksec_run(f)
+            self.assertEqual(out["file"]["fortified"], "0")
+
+
+class TestFortifyConserv(TestHardeningBase):
+    config = TestHardeningBase.config + \
+        """
+        BR2_FORTIFY_SOURCE_1=y
+        """
+
+    def test_run(self):
+        for f in self.checksec_files:
+            out = self.checksec_run(f)
+            self.assertNotEqual(out["file"]["fortified"], "0")
-- 
1.9.1

[Buildroot] [PATCH v7 4/4] support/testing/tests/core: SSP & hardening flags

[Peter Korsgaard]

>>>>> "Matt" == Matt Weber <matthew.weber@rockwellcollins.com> writes:

 > Catch the commonly used options of SSP, Relro, and fortify.
 > Using the package targets of busybox and lighttpd.  This
 > can easily be expanded to a larger list.

 > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>

 > ---
 > Changes
 > v1 -> v7 (link wrapper patchset)
 >  - None

Committed, thanks.

-- 
Bye, Peter Korsgaard