diff mbox series

[v3,2/8] security hardening: add RELFO, FORTIFY options

Message ID 1515557739-6027-2-git-send-email-matthew.weber@rockwellcollins.com
State Superseded
Headers show
Series [v3,1/8] stack protector: moved option out of adv menu | expand

Commit Message

Matt Weber Jan. 10, 2018, 4:15 a.m. UTC 
This enables a user to build a complete system using these
options.  It is important to note that not all packages will
build correctly to start with.

A good testing tool to check a target's elf files for compliance
to an array of hardening techniques can be found here:
https://github.com/slimm609/checksec.sh

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

v2 -> v3
 - Consolidated the way flags were set using CPPFLAGS (Arnout)
 - Removed fortran flag as not relevant for this feature (Arnout)
 - Added BR2_TOOLCHAIN_USES_GLIBC and optimization level dependency

v1 -> v2
 - Cosmetic caps on titles
---
 Config.in           | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 package/Makefile.in | 46 +++++++++++++++++++++++------------
 2 files changed, 100 insertions(+), 15 deletions(-)

Comments

Nicolas Cavallari Jan. 10, 2018, 8:22 a.m. UTC | #1 
On 10/01/2018 05:15, Matt Weber wrote:
> [...]
> +config BR2_RELRO_NONE
> +	bool "None"
> +	help
> +	  Enables Relocation link-time protections.

Disables ?

> +config BR2_FORTIFY_SOURCE_NONE
> +	bool "None"
> +	help
> +	  Enables additional checks to detect buffer-overflows.

And here too ?
Nicolas Cavallari Jan. 10, 2018, 9:41 a.m. UTC | #2 
On 10/01/2018 05:15, Matt Weber wrote:
> +ifneq ($(BR2_OPTIMIZE_S)$(BR2_OPTIMIZE_0)$(BR2_OPTIMIZE_1)$(BR2_OPTIMIZE_2)$(BR2_OPTIMIZE_G),)
> +ifeq ($(BR2_FORTIFY_SOURCE_1),y)
> +TARGET_CPPFLAGS += -D_FORTIFY_SOURCE=1
> +else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
> +TARGET_CPPFLAGS += -D_FORTIFY_SOURCE=2
> +endif
> +else
> +$(error BR2_FORTIFY_SOURCE_# requires optimization level s/1/2/3/g)
> +endif

This seems to test if the optimization level is s/0/1/2/g instead of
s/1/2/3/g

Shouldn't this actually be expressed as a dependency in Config.in
instead ? (i.e. make BR2_FORTIFY_SOURCE_{1,2} depends on !BR2_OPTIMIZE_0)
Matt Weber Jan. 10, 2018, 12:20 p.m. UTC | #3 
Nicolas,

On Wed, Jan 10, 2018 at 2:22 AM, Nicolas Cavallari
<Nicolas.Cavallari@green-communications.fr> wrote:
> On 10/01/2018 05:15, Matt Weber wrote:
>> [...]
>> +config BR2_RELRO_NONE
>> +     bool "None"
>> +     help
>> +       Enables Relocation link-time protections.
>
> Disables ?
>
>> +config BR2_FORTIFY_SOURCE_NONE
>> +     bool "None"
>> +     help
>> +       Enables additional checks to detect buffer-overflows.
>
> And here too ?

Correct in both cases, oops.  I'll update in v4.

Thanks for the review!
Matt
Matt Weber Jan. 10, 2018, 12:25 p.m. UTC | #4 
Nicolas,

On Wed, Jan 10, 2018 at 3:41 AM, Nicolas Cavallari
<Nicolas.Cavallari@green-communications.fr> wrote:
> On 10/01/2018 05:15, Matt Weber wrote:
>> +ifneq ($(BR2_OPTIMIZE_S)$(BR2_OPTIMIZE_0)$(BR2_OPTIMIZE_1)$(BR2_OPTIMIZE_2)$(BR2_OPTIMIZE_G),)
>> +ifeq ($(BR2_FORTIFY_SOURCE_1),y)
>> +TARGET_CPPFLAGS += -D_FORTIFY_SOURCE=1
>> +else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
>> +TARGET_CPPFLAGS += -D_FORTIFY_SOURCE=2
>> +endif
>> +else
>> +$(error BR2_FORTIFY_SOURCE_# requires optimization level s/1/2/3/g)
>> +endif
>
> This seems to test if the optimization level is s/0/1/2/g instead of
> s/1/2/3/g

Good catch, will update in v4

>
> Shouldn't this actually be expressed as a dependency in Config.in
> instead ? (i.e. make BR2_FORTIFY_SOURCE_{1,2} depends on !BR2_OPTIMIZE_0)

I looked at a few ways of doing this and since those optimize
variables are from a choice, I couldn't depend on them in Kconfig.  I
think I could change how that choice works to evaluate a value which
is set to the choice, however that seemed more complex then just
documenting and errorring.  I'm definitely open to ideas on this.

Matt
Nicolas Cavallari Jan. 10, 2018, 4:30 p.m. UTC | #5 
On 10/01/2018 13:25, Matthew Weber wrote:
> Nicolas,
> 
> On Wed, Jan 10, 2018 at 3:41 AM, Nicolas Cavallari
> <Nicolas.Cavallari@green-communications.fr> wrote:
>> On 10/01/2018 05:15, Matt Weber wrote:
>>> +ifneq ($(BR2_OPTIMIZE_S)$(BR2_OPTIMIZE_0)$(BR2_OPTIMIZE_1)$(BR2_OPTIMIZE_2)$(BR2_OPTIMIZE_G),)
>>> +ifeq ($(BR2_FORTIFY_SOURCE_1),y)
>>> +TARGET_CPPFLAGS += -D_FORTIFY_SOURCE=1
>>> +else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
>>> +TARGET_CPPFLAGS += -D_FORTIFY_SOURCE=2
>>> +endif
>>> +else
>>> +$(error BR2_FORTIFY_SOURCE_# requires optimization level s/1/2/3/g)
>>> +endif
>>
>> This seems to test if the optimization level is s/0/1/2/g instead of
>> s/1/2/3/g
> 
> Good catch, will update in v4
> 
>>
>> Shouldn't this actually be expressed as a dependency in Config.in
>> instead ? (i.e. make BR2_FORTIFY_SOURCE_{1,2} depends on !BR2_OPTIMIZE_0)
> 
> I looked at a few ways of doing this and since those optimize
> variables are from a choice, I couldn't depend on them in Kconfig.

It should work fine.  What issue do you have ?

just adding "depends on !BR2_OPTIMIZE_0"
on the BR2_FORTIFY_SOURCE_1 and BR2_FORTIFY_SOURCE_2 definitions
should work.

if BR2_OPTIMIZE_0 is selected, then the choice will only have one
possible value.
diff mbox series

Patch

diff --git a/Config.in b/Config.in
index e7e5c2d..f57e2b6 100644
--- a/Config.in
+++ b/Config.in
@@ -734,6 +734,75 @@  endchoice
 comment "Stack Smashing Protection needs a toolchain w/ SSP"
 	depends on !BR2_TOOLCHAIN_HAS_SSP
 
+choice
+	bool "RELRO Protection"
+	depends on BR2_SHARED_LIBS
+	help
+	  Enable a link-time protection know as RELRO (RELocation Read Only)
+	  which helps to protect from certain type of exploitation techniques
+	  altering the content of some ELF sections.
+
+config BR2_RELRO_NONE
+	bool "None"
+	help
+	  Enables Relocation link-time protections.
+
+config BR2_RELRO_PARTIAL
+	bool "Partial"
+	help
+	  This option makes the dynamic section not writeable after
+	  initialization (with almost no performance penalty).
+
+config BR2_RELRO_FULL
+	bool "Full"
+	help
+	  This option includes the partial configuration, but also
+	  marks the GOT as read-only at the cost of initialization time
+	  during program loading, i.e every time an executable is started.
+
+endchoice
+
+comment "RELocation Read Only (RELRO) needs shared libraries"
+	depends on !BR2_SHARED_LIBS
+
+choice
+	bool "Buffer-overflow Detection (FORTIFY_SOURCE)"
+	depends on BR2_TOOLCHAIN_USES_GLIBC
+	help
+	  Enable the _FORTIFY_SOURCE macro which introduces additional
+	  checks to detect buffer-overflows in the following standard library
+	  functions: memcpy, mempcpy, memmove, memset, strcpy, stpcpy,
+	  strncpy, strcat, strncat, sprintf, vsprintf, snprintf, vsnprintf,
+	  gets.
+
+	  NOTE: This feature requires an optimization level of s/1/2/3/g
+
+	  Support for this feature has been present since GCC 4.x.
+
+config BR2_FORTIFY_SOURCE_NONE
+	bool "None"
+	help
+	  Enables additional checks to detect buffer-overflows.
+
+config BR2_FORTIFY_SOURCE_1
+	bool "Conservative"
+	help
+	  This option sets _FORTIFY_SOURCE set to 1 and only introduces
+	  checks that shouldn't change the behavior of conforming programs.
+	  Adds checks at compile-time only.
+
+config BR2_FORTIFY_SOURCE_2
+	bool "Aggressive"
+	help
+	  This option sets _FORTIFY_SOURCES set to 2 and some more checking
+	  is added, but some conforming programs might fail.
+	  Also adds checks at run-time (detected buffer overflow terminates
+	  the program)
+
+endchoice
+
+comment "Fortify Source needs a GLIBC toolchain"
+	depends on !BR2_TOOLCHAIN_USES_GLIBC
 endmenu
 
 source "toolchain/Config.in"
diff --git a/package/Makefile.in b/package/Makefile.in
index a1a5316..84d4f0c 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -138,11 +138,41 @@  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)
+TARGET_CPPFLAGS += -fstack-protector
+else ifeq ($(BR2_SSP_STRONG),y)
+TARGET_CPPFLAGS += -fstack-protector-strong
+else ifeq ($(BR2_SSP_ALL),y)
+TARGET_CPPFLAGS += -fstack-protector-all
+endif
+
+ifeq ($(BR2_RELRO_PARTIAL),y)
+TARGET_CPPFLAGS += $(TARGET_CFLAGS_RELRO)
+TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO)
+else ifeq ($(BR2_RELRO_FULL),y)
+TARGET_CPPFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL)
+TARGET_LDFLAGS += -pie
+endif
+
+ifneq ($(BR2_OPTIMIZE_S)$(BR2_OPTIMIZE_0)$(BR2_OPTIMIZE_1)$(BR2_OPTIMIZE_2)$(BR2_OPTIMIZE_G),)
+ifeq ($(BR2_FORTIFY_SOURCE_1),y)
+TARGET_CPPFLAGS += -D_FORTIFY_SOURCE=1
+else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
+TARGET_CPPFLAGS += -D_FORTIFY_SOURCE=2
+endif
+else
+$(error BR2_FORTIFY_SOURCE_# requires optimization level s/1/2/3/g)
+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_CXXFLAGS = $(TARGET_CFLAGS)
 TARGET_FCFLAGS = $(TARGET_ABI) $(TARGET_OPTIMIZATION) $(TARGET_DEBUGGING)
-TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS))
 
 ifeq ($(BR2_BINFMT_FLAT),y)
 TARGET_CFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-Wl$(comma)-elf2flt=-s$($(PKG)_FLAT_STACKSIZE),\
@@ -167,20 +197,6 @@  TARGET_FCFLAGS += -msep-data
 TARGET_CXXFLAGS += -msep-data
 endif
 
-ifeq ($(BR2_SSP_REGULAR),y)
-TARGET_CFLAGS += -fstack-protector
-TARGET_CXXFLAGS += -fstack-protector
-TARGET_FCFLAGS += -fstack-protector
-else ifeq ($(BR2_SSP_STRONG),y)
-TARGET_CFLAGS += -fstack-protector-strong
-TARGET_CXXFLAGS += -fstack-protector-strong
-TARGET_FCFLAGS += -fstack-protector-strong
-else ifeq ($(BR2_SSP_ALL),y)
-TARGET_CFLAGS += -fstack-protector-all
-TARGET_CXXFLAGS += -fstack-protector-all
-TARGET_FCFLAGS += -fstack-protector-all
-endif
-
 ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)
 TARGET_CROSS = $(HOST_DIR)/bin/$(GNU_TARGET_NAME)-
 else