Hardening Wrapper Updates and Test

PATCH Superseded Matt Weber 2 participants 8 messages Aug 30, 2018

Share & tools

Matt Weber

author

2018-08-30 03:34 UTC #0 lore raw

v2 cover Hardening Wrapper Updates and Test

This series looks to complete the implementation and migration of hardening related options to the compiler and (new) linker wrapper.

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

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 <[email protected]>

CC Jan Kundr?t <[email protected]>

CC Stefan S?rensen <[email protected]>

Matt Weber (5):
toolchain/toolchain-wrapper: add link wrapper toolchain/toolchain-wrapper: add BR2_RELRO_FULL support toolchain/toolchain-wrapper: add BR2_SSP_* support toolchain/toolchain-wrapper: add BR2_FORTIFY_SOURCE_* support support/testing/tests/core: SSP & hardening flags

 .gitlab-ci.yml                                     |   6 +
 package/Makefile.in                                |  19 +-
 package/gcc/gcc.mk                                 |  12 +-
 support/testing/tests/core/test_hardening.py       | 110 +++++++++
 .../toolchain-external/pkg-toolchain-external.mk   |  12 +-
 toolchain/toolchain-wrapper-linker.c               | 273 +++++++++++++++++++++
 toolchain/toolchain-wrapper.c                      |  58 ++++-
 toolchain/toolchain-wrapper.mk                     |  24 ++
 8 files changed, 495 insertions(+), 19 deletions(-)
 create mode 100644 support/testing/tests/core/test_hardening.py
 create mode 100644 toolchain/toolchain-wrapper-linker.c

Signature · 1 line

1.9.1

Matt Weber

author

2018-08-30 03:34 UTC #1 lore raw

PATCH v2 1/5 toolchain/toolchain-wrapper: add link wrapper

- Updates symlink creation int/ext toolchain
- Adds new source file for link wrapper with build/install

Signed-off-by Matthew Weber <[email protected]>

---
 package/gcc/gcc.mk                                 |  12 +-
 .../toolchain-external/pkg-toolchain-external.mk   |  12 +-
 toolchain/toolchain-wrapper-linker.c               | 248 +++++++++++++++++++++
 toolchain/toolchain-wrapper.mk                     |   6 +
 4 files changed, 276 insertions(+), 2 deletions(-)
 create mode 100644 toolchain/toolchain-wrapper-linker.c

package/gcc/gcc.mk +11 -1

index 1ae9b7e..2b4ce33 100644
@@ -262,7 +262,9 @@ HOST_GCC_COMMON_CONF_OPTS += \
 	--with-long-double-128
 endif
 
-HOST_GCC_COMMON_TOOLCHAIN_WRAPPER_ARGS += -DBR_CROSS_PATH_SUFFIX='".br_real"'
+HOST_GCC_COMMON_TOOLCHAIN_WRAPPER_ARGS += \
+	-DBR_CROSS_PATH_SUFFIX='".br_real"' \
+	-DBR_CROSS_PATH_LD_SUFFIX='".br_real"'
 
 # For gcc-initial, we need to tell gcc that the C library will be
 # providing the ssp support, as it can't guess it since the C library

@@ -334,6 +336,14 @@ define HOST_GCC_INSTALL_WRAPPER_AND_SIMPLE_SYMLINKS
 			ln -sf toolchain-wrapper $(ARCH)-linux$${i##$(GNU_TARGET_NAME)}; \
 			ln -snf $$i.br_real $(ARCH)-linux$${i##$(GNU_TARGET_NAME)}.br_real; \
 			;; \
+		*ld) \
+			if [ ! -e $$i.br_real ] ; then \
+				mv $$i $$i.br_real; \
+				ln -sf toolchain-wrapper-linker $$i; \
+				ln -sf toolchain-wrapper-linker $(ARCH)-linux$${i##$(GNU_TARGET_NAME)}; \
+				ln -snf $$i.br_real $(ARCH)-linux$${i##$(GNU_TARGET_NAME)}.br_real; \
+			fi; \
+			;; \
 		*) \
 			ln -snf $$i $(ARCH)-linux$${i##$(GNU_TARGET_NAME)}; \
 			;; \

toolchain/toolchain-external/pkg-toolchain-external.mk +11 -1

index 8b2c283..99eea68 100644
@@ -90,10 +90,16 @@ endif
 TOOLCHAIN_EXTERNAL_SUFFIX = \
 	$(if $(wildcard $(TOOLCHAIN_EXTERNAL_BIN)/*.br_real),.br_real)
 
+# Handle the case where there are bootlin toolchain's without ld wrappers which
+# would not have a .br_real defined.
+TOOLCHAIN_EXTERNAL_LD_SUFFIX = \
+	$(if $(wildcard $(TOOLCHAIN_EXTERNAL_BIN)/*ld.br_real),.br_real)
+
 TOOLCHAIN_EXTERNAL_CROSS = $(TOOLCHAIN_EXTERNAL_BIN)/$(TOOLCHAIN_EXTERNAL_PREFIX)-
 TOOLCHAIN_EXTERNAL_CC = $(TOOLCHAIN_EXTERNAL_CROSS)gcc$(TOOLCHAIN_EXTERNAL_SUFFIX)
 TOOLCHAIN_EXTERNAL_CXX = $(TOOLCHAIN_EXTERNAL_CROSS)g++$(TOOLCHAIN_EXTERNAL_SUFFIX)
 TOOLCHAIN_EXTERNAL_FC = $(TOOLCHAIN_EXTERNAL_CROSS)gfortran$(TOOLCHAIN_EXTERNAL_SUFFIX)
+TOOLCHAIN_EXTERNAL_LD = $(TOOLCHAIN_EXTERNAL_CROSS)ld$(TOOLCHAIN_EXTERNAL_LD_SUFFIX)
 TOOLCHAIN_EXTERNAL_READELF = $(TOOLCHAIN_EXTERNAL_CROSS)readelf
 
 # Normal handling of downloaded toolchain tarball extraction.

@@ -229,7 +235,8 @@ TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += -DBR_SOFTFLOAT=1
 endif
 
 TOOLCHAIN_EXTERNAL_TOOLCHAIN_WRAPPER_ARGS += \
-	-DBR_CROSS_PATH_SUFFIX='"$(TOOLCHAIN_EXTERNAL_SUFFIX)"'
+	-DBR_CROSS_PATH_SUFFIX='"$(TOOLCHAIN_EXTERNAL_SUFFIX)"' \
+	-DBR_CROSS_PATH_LD_SUFFIX='"$(TOOLCHAIN_EXTERNAL_LD_SUFFIX)"' \
 
 ifeq ($(filter $(HOST_DIR)/%,$(TOOLCHAIN_EXTERNAL_BIN)),)
 # TOOLCHAIN_EXTERNAL_BIN points outside HOST_DIR => absolute path

@@ -274,6 +281,9 @@ define TOOLCHAIN_EXTERNAL_INSTALL_WRAPPER
 				ln -sf $$(echo $$i | sed 's%^$(HOST_DIR)%..%') .; \
 			fi \
 			;; \
+		*ld) \
+			ln -sf toolchain-wrapper-linker $$base; \
+			;; \
 		*) \
 			ln -sf $$(echo $$i | sed 's%^$(HOST_DIR)%..%') .; \
 			;; \

toolchain/toolchain-wrapper-linker.c new file +248 -0

248 lines not on this page · raw message

toolchain/toolchain-wrapper.mk +6 -0

index b8074ef..3a4cbcd 100644
@@ -50,9 +50,15 @@ define TOOLCHAIN_WRAPPER_BUILD
 		-s -Wl,--hash-style=$(TOOLCHAIN_WRAPPER_HASH_STYLE) \
 		toolchain/toolchain-wrapper.c \
 		-o $(@D)/toolchain-wrapper
+	$(HOSTCC) $(HOST_CFLAGS) $(TOOLCHAIN_WRAPPER_ARGS) \
+		-s -Wl,--hash-style=$(TOOLCHAIN_WRAPPER_HASH_STYLE) \
+		toolchain/toolchain-wrapper-linker.c \
+		-o $(@D)/toolchain-wrapper-linker
 endef
 
 define TOOLCHAIN_WRAPPER_INSTALL
 	$(INSTALL) -D -m 0755 $(@D)/toolchain-wrapper \
 		$(HOST_DIR)/bin/toolchain-wrapper
+	$(INSTALL) -D -m 0755 $(@D)/toolchain-wrapper-linker \
+		$(HOST_DIR)/bin/toolchain-wrapper-linker
 endef

Signature · 1 line

1.9.1

Thomas Petazzoni

2018-08-30 07:53 UTC #2 lore raw

PATCH v2 1/5 toolchain/toolchain-wrapper: add link wrapper

Hello Matt,

Quoted 4 lines · Matt Weber

- Updates symlink creation int/ext toolchain
- Adds new source file for link wrapper with build/install
Signed-off-by Matthew Weber <[email protected]>

I think this commit log should contain a detailed explanation of why wrapping the linker is necessary. Could you provide an example of a package that absolutely needs the linker to be wrapped ? I'd like to do a bit of testing, because I'm still not entirely convinced that we want to wrap the linker. Perhaps we need to do this, but I'd like to spend a bit of time looking at a specific example of a package that makes this necessary. Could you provide such an example ?

Thanks a lot!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Matthew Weber

author

2018-08-30 12:48 UTC #3 lore raw

PATCH v2 1/5 toolchain/toolchain-wrapper: add link wrapper

Thomas,

Quoted 9 lines · Thomas Petazzoni

Hello Matt,
Quoted 4 lines · Matt Weber
- Updates symlink creation int/ext toolchain
- Adds new source file for link wrapper with build/install
Signed-off-by Matthew Weber <[email protected]>

Quoted 8 lines

I think this commit log should contain a detailed explanation of why wrapping the linker is necessary. Could you provide an example of a package that absolutely needs the linker to be wrapped ? I'd like to do a bit of testing, because I'm still not entirely convinced that we want to wrap the linker. Perhaps we need to do this, but I'd like to spend a bit of time looking at a specific example of a package that makes this necessary. Could you provide such an example ?

Definitely. I can put together a few scenarios.

Right now the v2 series has an issue found by my night run of tests and I'll have to respin. I'll include something in the v3 series.

All, if anyone build tests this, the v2 change to preserve -fPIC has broken busybox individual binaries mode.

Matt

Matt Weber

author

2018-08-30 03:34 UTC #4 lore raw

PATCH v2 2/5 toolchain/toolchain-wrapper: add BR2_RELRO_FULL support

In reply to Matt Weber

Signed-off-by Matthew Weber <[email protected]>

---
Changes
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. Coded it backwards.

---
 package/Makefile.in                  |  5 +++--
 toolchain/toolchain-wrapper-linker.c | 27 +++++++++++++++++++++-
 toolchain/toolchain-wrapper.c        | 43 +++++++++++++++++++++++++++++++++++-
 toolchain/toolchain-wrapper.mk       |  4 ++++
 4 files changed, 75 insertions(+), 4 deletions(-)

package/Makefile.in +3 -2

index 14b3bbd..2e885bf 100644
@@ -158,8 +158,9 @@ 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)
+TARGET_HARDENED += $(TARGET_CFLAGS_RELRO_FULL)
+# -DBR_SET_PIE is used by the GCC wrapper to tell when linking
+TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO_FULL) -DBR_SET_PIE
 endif
 
 ifeq ($(BR2_FORTIFY_SOURCE_1),y)

toolchain/toolchain-wrapper-linker.c +26 -1

index b587fea..32e8156 100644
@@ -33,8 +33,10 @@ static char path[PATH_MAX];
  * one to the real compiler if the inverse option isn't in the argument list.
  * This specifies the worst case number of extra arguments we might pass
  * Currently, we may have:
+ * 	-pie
+ * 	-shared
  */
-#define EXCLUSIVE_ARGS	0
+#define EXCLUSIVE_ARGS	2
 
 static char *predef_args[] = {
 	path

@@ -126,6 +128,7 @@ int main(int argc, char **argv)
 	char *paranoid_wrapper;
 	int paranoid;
 	int ret, i, count = 0, debug;
+	unsigned int found_shared = 0;
 
 	/* Calculate the relative paths */
 	basename = strrchr(progpath, '/');

@@ -185,6 +188,28 @@ int main(int argc, char **argv)
 	memcpy(cur, predef_args, sizeof(predef_args));
 	cur += sizeof(predef_args) / sizeof(predef_args[0]);
 
+#ifdef BR2_RELRO_FULL
+	for (i = 1; i < argc; i++) {
+		if (!strcmp(argv[i], "-static") ||
+		    !strcmp(argv[i], "-r"))
+			break;
+
+		if( !strcmp(argv[i], "-shared") ) {
+		    /* Setting the value to something so that the compiler
+		       doesn't error on a empty '' when -share is removed */
+		    argv[i] = "-DBR_REMOVED_-shared";
+		    found_shared = 1;
+		}
+	}
+
+	if (i == argc) {
+		*cur++ = "-pie";
+
+		if( found_shared )
+		    *cur++ = "-shared";
+	}
+#endif
+
 	paranoid_wrapper = getenv("BR_COMPILER_PARANOID_UNSAFE_PATH");
 	if (paranoid_wrapper && strlen(paranoid_wrapper) > 0)
 		paranoid = 1;

toolchain/toolchain-wrapper.c +42 -1

62 lines not on this page · raw message

toolchain/toolchain-wrapper.mk +4 -0

index 3a4cbcd..b0e44b3 100644
@@ -45,6 +45,10 @@ ifeq ($(BR2_CCACHE_USE_BASEDIR),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR_CCACHE_BASEDIR='"$(BASE_DIR)"'
 endif
 
+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) \

Signature · 1 line

1.9.1

Matt Weber

author

2018-08-30 03:34 UTC #5 lore raw

PATCH v2 3/5 toolchain/toolchain-wrapper: add BR2_SSP_* support

In reply to Matt Weber

Migrate the stack protection flag management into the wrapper.

Signed-off-by Matthew Weber <[email protected]>

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

package/Makefile.in +0 -8

index 2e885bf..ed6fd0d 100644
@@ -146,14 +146,6 @@ TARGET_CFLAGS_RELRO_FULL = -Wl,-z,now $(TARGET_CFLAGS_RELRO)
 
 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_RELRO_PARTIAL),y)
 TARGET_HARDENED += $(TARGET_CFLAGS_RELRO)
 TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO)

toolchain/toolchain-wrapper.c +9 -0

index 4649091..6c5ea1a 100644
@@ -97,6 +97,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

toolchain/toolchain-wrapper.mk +8 -0

index b0e44b3..15f1a74 100644
@@ -49,6 +49,14 @@ 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) \

Signature · 1 line

1.9.1

Matt Weber

author

2018-08-30 03:34 UTC #6 lore raw

PATCH v2 4/5 toolchain/toolchain-wrapper: add BR2_FORTIFY_SOURCE_* support

In reply to Matt Weber

Migrate the fortify flag management into the wrapper.

Signed-off-by Matthew Weber <[email protected]>

---
 package/Makefile.in            | 6 ------
 toolchain/toolchain-wrapper.c  | 6 ++++++
 toolchain/toolchain-wrapper.mk | 6 ++++++
 3 files changed, 12 insertions(+), 6 deletions(-)

package/Makefile.in +0 -6

index ed6fd0d..d1f9789 100644
@@ -155,12 +155,6 @@ TARGET_HARDENED += $(TARGET_CFLAGS_RELRO_FULL)
 TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO_FULL) -DBR_SET_PIE
 endif
 
-ifeq ($(BR2_FORTIFY_SOURCE_1),y)
-TARGET_HARDENED += -D_FORTIFY_SOURCE=1
-else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
-TARGET_HARDENED += -D_FORTIFY_SOURCE=2
-endif
-
 TARGET_CPPFLAGS += -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
 TARGET_CFLAGS = $(TARGET_CPPFLAGS) $(TARGET_ABI) $(TARGET_OPTIMIZATION) $(TARGET_DEBUGGING) $(TARGET_HARDENED)
 TARGET_CXXFLAGS = $(TARGET_CFLAGS)

toolchain/toolchain-wrapper.c +6 -0

index 6c5ea1a..a2d5f1d 100644
@@ -106,6 +106,12 @@ static char *predef_args[] = {
 #ifdef BR_SSP_ALL
 	"-fstack-protector-all",
 #endif
+#ifdef BR_FORTIFY_SOURCE_1
+	"-D_FORTIFY_SOURCE=1",
+#endif
+#ifdef BR_FORTIFY_SOURCE_2
+	"-D_FORTIFY_SOURCE=2",
+#endif
 #ifdef BR_ADDITIONAL_CFLAGS
 	BR_ADDITIONAL_CFLAGS
 #endif

toolchain/toolchain-wrapper.mk +6 -0

index 15f1a74..8b8db8b 100644
@@ -57,6 +57,12 @@ else ifeq ($(BR2_SSP_ALL),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR_SSP_ALL
 endif
 
+ifeq ($(BR2_FORTIFY_SOURCE_1),y)
+TOOLCHAIN_WRAPPER_ARGS += -DBR_FORTIFY_SOURCE_1
+else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
+TOOLCHAIN_WRAPPER_ARGS += -DBR_FORTIFY_SOURCE_2
+endif
+
 define TOOLCHAIN_WRAPPER_BUILD
 	$(HOSTCC) $(HOST_CFLAGS) $(TOOLCHAIN_WRAPPER_ARGS) \
 		-s -Wl,--hash-style=$(TOOLCHAIN_WRAPPER_HASH_STYLE) \

Signature · 1 line

1.9.1

Matt Weber

author

2018-08-30 03:34 UTC #7 lore raw

PATCH v2 5/5 support/testing/tests/core: SSP & hardening flags

In reply to Matt Weber

Catch the commonly used options of SSP, Relro, and fortify.

Signed-off-by Matthew Weber <[email protected]>

---
Changes
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

.gitlab-ci.yml +6 -0

index 3fc9b06..c271c05 100644
@@ -261,6 +261,12 @@ zynqmp_zcu106_defconfig: *defconfig
 tests.boot.test_atf.TestATFAllwinner: *runtime_test
 tests.boot.test_atf.TestATFMarvell: *runtime_test
 tests.boot.test_atf.TestATFVexpress: *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

support/testing/tests/core/test_hardening.py new file +110 -0

110 lines not on this page · raw message

Signature · 1 line

1.9.1