[Buildroot] [PATCH 1/4] package/Makefile.in: Do not use CPPFLAGS for hardening options

[Buildroot] [PATCH 1/4] package/Makefile.in: Do not use CPPFLAGS for hardening options

[Stefan Sørensen]

The hardening options are compiler flags, not pure pre-processor flags, so
put them in CFLAGS, not CPPFLAGS.

This fixes build errors where -D_FORTIFY_SOURCE=2 whas put in CPPFLAGS and
then applied to configure tests which could fail since the required -O2 is
only in CFLAGS.

Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>
---
 package/Makefile.in | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/package/Makefile.in b/package/Makefile.in
index 4325f7b3a9..d72a5494ab 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -147,29 +147,29 @@ 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
+TARGET_HARDENED += -fstack-protector
 else ifeq ($(BR2_SSP_STRONG),y)
-TARGET_CPPFLAGS += -fstack-protector-strong
+TARGET_HARDENED += -fstack-protector-strong
 else ifeq ($(BR2_SSP_ALL),y)
-TARGET_CPPFLAGS += -fstack-protector-all
+TARGET_HARDENED += -fstack-protector-all
 endif
 
 ifeq ($(BR2_RELRO_PARTIAL),y)
-TARGET_CPPFLAGS += $(TARGET_CFLAGS_RELRO)
+TARGET_HARDENED += $(TARGET_CFLAGS_RELRO)
 TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO)
 else ifeq ($(BR2_RELRO_FULL),y)
-TARGET_CPPFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL)
+TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL)
 TARGET_LDFLAGS += -pie
 endif
 
 ifeq ($(BR2_FORTIFY_SOURCE_1),y)
-TARGET_CPPFLAGS += -D_FORTIFY_SOURCE=1
+TARGET_HARDENED += -D_FORTIFY_SOURCE=1
 else ifeq ($(BR2_FORTIFY_SOURCE_2),y)
-TARGET_CPPFLAGS += -D_FORTIFY_SOURCE=2
+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_CFLAGS = $(TARGET_CPPFLAGS) $(TARGET_ABI) $(TARGET_OPTIMIZATION) $(TARGET_DEBUGGING) $(TARGET_HARDENED)
 TARGET_CXXFLAGS = $(TARGET_CFLAGS)
 TARGET_FCFLAGS = $(TARGET_ABI) $(TARGET_OPTIMIZATION) $(TARGET_DEBUGGING)
 
-- 
2.14.3

[Buildroot] [PATCH 2/4] package/Makefile.in: Add missing options to LDFLAGS for full RELRO build

[Stefan Sørensen]

The options for a full RELRO build should also be added to LDFLAGS.

Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>
---
 package/Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/Makefile.in b/package/Makefile.in
index d72a5494ab..1cbf34e0df 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -159,7 +159,7 @@ 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_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL)
 endif
 
 ifeq ($(BR2_FORTIFY_SOURCE_1),y)
-- 
2.14.3

[Buildroot] [PATCH 2/4] package/Makefile.in: Add missing options to LDFLAGS for full RELRO build

[Matthew Weber]

Stefan,

On Wed, Apr 25, 2018 at 1:45 AM, Stefan S?rensen
<stefan.sorensen@spectralink.com> wrote:
> The options for a full RELRO build should also be added to LDFLAGS.
>
> Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>



Tested-by: Matt Weber <matthew.weber@rockwellcollins.com>

[Buildroot] [PATCH 3/4] package/Makefile.in: Remove RELRO linker flags from CFLAGS

[Stefan Sørensen]

The RELRO related flags are for the linker, not the compiler, so they belong
in LDFLAGS.

Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>
---
 package/Makefile.in | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/package/Makefile.in b/package/Makefile.in
index 1cbf34e0df..4525b8e79f 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)
@@ -155,11 +152,10 @@ TARGET_HARDENED += -fstack-protector-all
 endif
 
 ifeq ($(BR2_RELRO_PARTIAL),y)
-TARGET_HARDENED += $(TARGET_CFLAGS_RELRO)
-TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO)
+TARGET_LDFLAGS += -Wl,-z,relro
 else ifeq ($(BR2_RELRO_FULL),y)
-TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL)
-TARGET_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL)
+TARGET_HARDENED += -fPIE
+TARGET_LDFLAGS += -pie -Wl,-z,now -Wl,-z,relro
 endif
 
 ifeq ($(BR2_FORTIFY_SOURCE_1),y)
-- 
2.14.3

[Buildroot] [PATCH 3/4] package/Makefile.in: Remove RELRO linker flags from CFLAGS

[Matthew Weber]

Stefan,

On Wed, Apr 25, 2018 at 1:45 AM, Stefan S?rensen
<stefan.sorensen@spectralink.com> wrote:
> The RELRO related flags are for the linker, not the compiler, so they belong
> in LDFLAGS.
>
> Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>
> ---
>  package/Makefile.in | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/package/Makefile.in b/package/Makefile.in
> index 1cbf34e0df..4525b8e79f 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)
> -

If you remove these flags from CFLAGS then packages that link with gcc
vs explicitly ld won't get the flag.

[Buildroot] [PATCH 3/4] package/Makefile.in: Remove RELRO linker flags from CFLAGS

[Matthew Weber]

Stefan,

On Mon, Apr 30, 2018 at 12:57 PM, Matthew Weber
<matthew.weber@rockwellcollins.com> wrote:
> Stefan,
>
> On Wed, Apr 25, 2018 at 1:45 AM, Stefan S?rensen
> <stefan.sorensen@spectralink.com> wrote:
>> The RELRO related flags are for the linker, not the compiler, so they belong
>> in LDFLAGS.
>>
>> Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>
>> ---
>>  package/Makefile.in | 10 +++-------
>>  1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/package/Makefile.in b/package/Makefile.in
>> index 1cbf34e0df..4525b8e79f 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)
>> -
>
> If you remove these flags from CFLAGS then packages that link with gcc
> vs explicitly ld won't get the flag.

Suggest dropping this patch as we want to maintain the ability for gcc
to use flags when linking an executable directly.

Matt

[Buildroot] [PATCH 3/4] package/Makefile.in: Remove RELRO linker flags from CFLAGS

[Arnout Vandecappelle]

On 30-04-18 19:57, Matthew Weber wrote:
> Stefan,
> 
> On Wed, Apr 25, 2018 at 1:45 AM, Stefan S?rensen
> <stefan.sorensen@spectralink.com> wrote:
>> The RELRO related flags are for the linker, not the compiler, so they belong
>> in LDFLAGS.
>>
>> Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>
>> ---
>>  package/Makefile.in | 10 +++-------
>>  1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/package/Makefile.in b/package/Makefile.in
>> index 1cbf34e0df..4525b8e79f 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)
>> -
> 
> If you remove these flags from CFLAGS then packages that link with gcc
> vs explicitly ld won't get the flag.

 The intention is that LDFLAGS is used when linking with $CC/$CXX. Linking with
ld should ideally never be done, except for some special snowflakes like the kernel.

 The intention is that TARGET_LDFLAGS is used in all linking invocation.
However, we're pretty sure that for a large number of packages, this isn't
really the case. That's why we want to move more things into the wrapper.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 3/4] package/Makefile.in: Remove RELRO linker flags from CFLAGS

[Matthew Weber]

Arnout,

On Wed, May 2, 2018 at 4:54 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
>
> On 30-04-18 19:57, Matthew Weber wrote:
> > Stefan,
> >
> > On Wed, Apr 25, 2018 at 1:45 AM, Stefan S?rensen
> > <stefan.sorensen@spectralink.com> wrote:
> >> The RELRO related flags are for the linker, not the compiler, so they belong
> >> in LDFLAGS.
> >>
> >> Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>
> >> ---
> >>  package/Makefile.in | 10 +++-------
> >>  1 file changed, 3 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/package/Makefile.in b/package/Makefile.in
> >> index 1cbf34e0df..4525b8e79f 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)
> >> -
> >
> > If you remove these flags from CFLAGS then packages that link with gcc
> > vs explicitly ld won't get the flag.
>
>  The intention is that LDFLAGS is used when linking with $CC/$CXX. Linking with
> ld should ideally never be done, except for some special snowflakes like the kernel.
>
>  The intention is that TARGET_LDFLAGS is used in all linking invocation.
> However, we're pretty sure that for a large number of packages, this isn't
> really the case. That's why we want to move more things into the wrapper.
>

Arnout, in the near term, do you agree we could merge this
patchset(minus patch 3) to fix existing bugs?

[Buildroot] [PATCH 4/4] package/Makefile.in: Use gcc spec files for PIE build flags

[Stefan Sørensen]

The PIE build flags are only intended for building executables and can not be
used in relocateable links (-r), static builds and shared library build -
including the flags here causes build errors.

So instead of parsing the PIE flags directly on the command line to gcc,
include them in a gcc spec file where it is possible to only apply the flags
when other incompatible flags are not set.

This method and the spec files are from the Fedora build system.

Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>
---
 package/Makefile.in         | 4 ++--
 toolchain/gcc-specs-pie-cc1 | 2 ++
 toolchain/gcc-specs-pie-ld  | 2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)
 create mode 100644 toolchain/gcc-specs-pie-cc1
 create mode 100644 toolchain/gcc-specs-pie-ld

diff --git a/package/Makefile.in b/package/Makefile.in
index 4525b8e79f..141eb83946 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -154,8 +154,8 @@ endif
 ifeq ($(BR2_RELRO_PARTIAL),y)
 TARGET_LDFLAGS += -Wl,-z,relro
 else ifeq ($(BR2_RELRO_FULL),y)
-TARGET_HARDENED += -fPIE
-TARGET_LDFLAGS += -pie -Wl,-z,now -Wl,-z,relro
+TARGET_HARDENED += -specs=$(TOPDIR)/toolchain/gcc-specs-pie-cc1
+TARGET_LDFLAGS += -Wl,-z,relro -Wl,-z,now -specs=$(TOPDIR)/toolchain/gcc-specs-pie-ld
 endif
 
 ifeq ($(BR2_FORTIFY_SOURCE_1),y)
diff --git a/toolchain/gcc-specs-pie-cc1 b/toolchain/gcc-specs-pie-cc1
new file mode 100644
index 0000000000..fc54bcb510
--- /dev/null
+++ b/toolchain/gcc-specs-pie-cc1
@@ -0,0 +1,2 @@
+*cc1_options:
++ %{!r:%{!fpie:%{!fPIE:%{!fpic:%{!fPIC:%{!fno-pic:-fPIE}}}}}}
diff --git a/toolchain/gcc-specs-pie-ld b/toolchain/gcc-specs-pie-ld
new file mode 100644
index 0000000000..bd6b9071ff
--- /dev/null
+++ b/toolchain/gcc-specs-pie-ld
@@ -0,0 +1,2 @@
+*self_spec:
++ %{!static:%{!shared:%{!r:-pie}}}
-- 
2.14.3

[Buildroot] [PATCH 4/4] package/Makefile.in: Use gcc spec files for PIE build flags

[Matthew Weber]

Stefan,

On Wed, Apr 25, 2018 at 1:45 AM, Stefan S?rensen
<stefan.sorensen@spectralink.com> wrote:
> The PIE build flags are only intended for building executables and can not be
> used in relocateable links (-r), static builds and shared library build -
> including the flags here causes build errors.
>
> So instead of parsing the PIE flags directly on the command line to gcc,
> include them in a gcc spec file where it is possible to only apply the flags
> when other incompatible flags are not set.
>
> This method and the spec files are from the Fedora build system.
>
> Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>

Thank you for this approach, I had a patchset reworking packages to
deal with cases of "-r", static and shared.  This eliminates that and
makes a wrapper approach much easier as the next step.

Tested-by: Matt Weber <matthew.weber@rockwellcollins.com>

[Buildroot] [PATCH 4/4] package/Makefile.in: Use gcc spec files for PIE build flags

[Matthew Weber]

Stefan,

On Tue, May 1, 2018 at 10:13 AM, Matthew Weber
<matthew.weber@rockwellcollins.com> wrote:
> Stefan,
>
> On Wed, Apr 25, 2018 at 1:45 AM, Stefan S?rensen
> <stefan.sorensen@spectralink.com> wrote:
>> The PIE build flags are only intended for building executables and can not be
>> used in relocateable links (-r), static builds and shared library build -
>> including the flags here causes build errors.
>>
>> So instead of parsing the PIE flags directly on the command line to gcc,
>> include them in a gcc spec file where it is possible to only apply the flags
>> when other incompatible flags are not set.
>>
>> This method and the spec files are from the Fedora build system.
>>
>> Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>
>
> Thank you for this approach, I had a patchset reworking packages to
> deal with cases of "-r", static and shared.  This eliminates that and
> makes a wrapper approach much easier as the next step.
>
> Tested-by: Matt Weber <matthew.weber@rockwellcollins.com>

Just realized when I was cleaning things up this one needs rework with
the drop of patch 3.  Here's a v2 of this patch.
http://patchwork.ozlabs.org/patch/907093/

[Buildroot] [PATCH 4/4] package/Makefile.in: Use gcc spec files for PIE build flags

[Arnout Vandecappelle]

On 25-04-18 08:45, Stefan S?rensen wrote:
> The PIE build flags are only intended for building executables and can not be
> used in relocateable links (-r), static builds and shared library build -
> including the flags here causes build errors.
> 
> So instead of parsing the PIE flags directly on the command line to gcc,
> include them in a gcc spec file where it is possible to only apply the flags
> when other incompatible flags are not set.

 This is a very elegant solution! In fact, a lot of things we now do in the
wrapper could perhaps be moved to the spec file. The idea would be that in
addition to generating the wrapper, we also generate a spec file with all the
options that are currently passed either into the wrapper or into CFLAGS/LDFLAGS.

 Thomas, perhaps a spec file is a good approach to force flags, what do you
think? This would mean:

- there is a way to distinguish between compile and link invocations;
- there is a way to take care of conflicting options;
- it is possible to hack the spec file after the build (mainly useful for
developers :-).

 Disadvantage is that the spec file syntax is not intuitive or familiar.


> This method and the spec files are from the Fedora build system.
> 
> Signed-off-by: Stefan S?rensen <stefan.sorensen@spectralink.com>
> ---
>  package/Makefile.in         | 4 ++--
>  toolchain/gcc-specs-pie-cc1 | 2 ++
>  toolchain/gcc-specs-pie-ld  | 2 ++
>  3 files changed, 6 insertions(+), 2 deletions(-)
>  create mode 100644 toolchain/gcc-specs-pie-cc1
>  create mode 100644 toolchain/gcc-specs-pie-ld
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index 4525b8e79f..141eb83946 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -154,8 +154,8 @@ endif
>  ifeq ($(BR2_RELRO_PARTIAL),y)
>  TARGET_LDFLAGS += -Wl,-z,relro
>  else ifeq ($(BR2_RELRO_FULL),y)
> -TARGET_HARDENED += -fPIE
> -TARGET_LDFLAGS += -pie -Wl,-z,now -Wl,-z,relro
> +TARGET_HARDENED += -specs=$(TOPDIR)/toolchain/gcc-specs-pie-cc1
> +TARGET_LDFLAGS += -Wl,-z,relro -Wl,-z,now -specs=$(TOPDIR)/toolchain/gcc-specs-pie-ld

 Why not move the -z options into the spec file as well?

 There is no reason to have separate -cc1 and -ld spec files, right?

 The -specs option should be passed as part of the toolchain wrapper, so we're
sure it's really always used.

>  endif
>  
>  ifeq ($(BR2_FORTIFY_SOURCE_1),y)
> diff --git a/toolchain/gcc-specs-pie-cc1 b/toolchain/gcc-specs-pie-cc1
> new file mode 100644
> index 0000000000..fc54bcb510
> --- /dev/null
> +++ b/toolchain/gcc-specs-pie-cc1
> @@ -0,0 +1,2 @@
> +*cc1_options:
> ++ %{!r:%{!fpie:%{!fPIE:%{!fpic:%{!fPIC:%{!fno-pic:-fPIE}}}}}}
> diff --git a/toolchain/gcc-specs-pie-ld b/toolchain/gcc-specs-pie-ld
> new file mode 100644
> index 0000000000..bd6b9071ff
> --- /dev/null
> +++ b/toolchain/gcc-specs-pie-ld
> @@ -0,0 +1,2 @@
> +*self_spec:

 self_spec only exists since gcc 4.7, while in theory we support gcc versions as
low as 4.3. So this would mean that RELRO_FULL should depend on
BR2_TOOLCHAIN_GCC_AT_LEAST_4_7.

 Regards,
 Arnout

> ++ %{!static:%{!shared:%{!r:-pie}}}
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/4] package/Makefile.in: Do not use CPPFLAGS for hardening options

[Matthew Weber]

Stefan,

On Wed, Apr 25, 2018 at 1:45 AM, Stefan S?rensen
<stefan.sorensen@spectralink.com> wrote:
> The hardening options are compiler flags, not pure pre-processor flags, so
> put them in CFLAGS, not CPPFLAGS.
>
> This fixes build errors where -D_FORTIFY_SOURCE=2 whas put in CPPFLAGS and
> then applied to configure tests which could fail since the required -O2 is
> only in CFLAGS.
>

Thanks for sending this series.  When we added the initial support we
debated on doing a few things differently at some point with how this
is implemented.  First, Buildroot uses a toolchain wrapper where it
could inject these flags vs appending like the current design does.
This would allow all the packages with flag ordering issues and no
formal releases, to not carry a patch in buildroot for the long term.
The second was to add support for the autobuilders to start enabling a
safe configuration of these options on random builds to build up the
maturity of the packages. Lastly there was discussion at the late
developer days on integrating the checksec scripting so there was a
way to do some validation of settings taking affect as part of new
Buildroot test cases.  All of these are covered in more detail in the
referenced slides link below.

Refs (patchwork links and design discussion):
https://docs.google.com/presentation/d/1IyrflpslZ6Gnsl-deR5G3sODfuICe-UkBeD44Edudhk/edit?usp=sharing

For this series, I'll work on some test builds (today/tomorrow) and
get you some feedback.

Matt

[Buildroot] [PATCH 1/4] package/Makefile.in: Do not use CPPFLAGS for hardening options

[Sørensen, Stefan]

On Wed, 2018-04-25 at 07:50 -0500, Matthew Weber wrote:

> Thanks for sending this series.  When we added the initial support we
> debated on doing a few things differently at some point with how this
> is implemented.  First, Buildroot uses a toolchain wrapper where it
> could inject these flags vs appending like the current design does.

Personally I prefer that flags are appended - when injecting them
through the wrapper, they are invisible in the build logs. 

> Lastly there was discussion at the late developer days on integrating
> the checksec scripting so there was a way to do some validation of
> settings taking affect as part of new Buildroot test cases.

I am working on integrating support for the annobin gcc plugin and
adding a check step for hardening. I hope to have a RFC patch ready
next week.

Stefan

[Buildroot] [PATCH 1/4] package/Makefile.in: Do not use CPPFLAGS for hardening options

[Matthew Weber]

Stefan,

On Wed, Apr 25, 2018 at 8:08 AM, S?rensen, Stefan
<Stefan.Sorensen@spectralink.com> wrote:
> On Wed, 2018-04-25 at 07:50 -0500, Matthew Weber wrote:
>
>> Thanks for sending this series.  When we added the initial support we
>> debated on doing a few things differently at some point with how this
>> is implemented.  First, Buildroot uses a toolchain wrapper where it
>> could inject these flags vs appending like the current design does.
>
> Personally I prefer that flags are appended - when injecting them
> through the wrapper, they are invisible in the build logs.
>

Agree.  Also the parsing problem was fairly complex for
reordering+add/subtracting conflicting flags.  You may have solved
that though with the spec.

>> Lastly there was discussion at the late developer days on integrating
>> the checksec scripting so there was a way to do some validation of
>> settings taking affect as part of new Buildroot test cases.
>
> I am working on integrating support for the annobin gcc plugin and
> adding a check step for hardening. I hope to have a RFC patch ready
> next week.
>

I'm not familiar with that plugin, I'd be curious if you're approach
would allow us to define a few test scenarios to add to the Buildroot
test suite.

Would the plugin require a internal toolchain build or just specific
external prebuilt requirements?

Matt

[Buildroot] [PATCH 1/4] package/Makefile.in: Do not use CPPFLAGS for hardening options

[Sørensen, Stefan]

On Wed, 2018-04-25 at 08:12 -0500, Matthew Weber wrote:
> 
> I'm not familiar with that plugin, I'd be curious if you're approach
> would allow us to define a few test scenarios to add to the Buildroot
> test suite.

Annobin also records some other hardening flags and the optimization
level used in the build. This has so far revealed a long list of
packages where the Buildroot flags are not propagated to the build.
> 
> Would the plugin require a internal toolchain build or just specific
> external prebuilt requirements?

I just requires a gcc built with plugin support - I have testet with
both internal and external.

Reading the generated data however requires at lest binutils 2.30 I
believe.

Stefan

[Buildroot] [PATCH 1/4] package/Makefile.in: Do not use CPPFLAGS for hardening options

[Thomas Petazzoni]

Hello Stefan,

On Wed, 25 Apr 2018 13:08:18 +0000, S?rensen, Stefan wrote:
> On Wed, 2018-04-25 at 07:50 -0500, Matthew Weber wrote:
> 
> > Thanks for sending this series.  When we added the initial support we
> > debated on doing a few things differently at some point with how this
> > is implemented.  First, Buildroot uses a toolchain wrapper where it
> > could inject these flags vs appending like the current design does.  
> 
> Personally I prefer that flags are appended - when injecting them
> through the wrapper, they are invisible in the build logs. 

The problem with appended flags is that you are never sure they will be
passed. Indeed, some packages ignore the CFLAGS/LDFLAGS passed on the
command line. Having such flags in the wrapper ensures they are
*always* passed.

In addition, having such flags passed in the wrapper ensures that they
are passed even if you build something with the Buildroot toolchain, but
outside of Buildroot itself.

As part of the latest Buildroot hackathon, Arnout (added in Cc) and I
reviewed the usage of our flags, we concluded that hardening flags
should be passed through the wrapper. I have some notes about our
discussion, but haven't cleaned them up yet so they haven't been posted
so far.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/4] package/Makefile.in: Do not use CPPFLAGS for hardening options

[Thomas Petazzoni]

Hello,

On Wed, 25 Apr 2018 07:50:37 -0500, Matthew Weber wrote:

> Thanks for sending this series.  When we added the initial support we
> debated on doing a few things differently at some point with how this
> is implemented.  First, Buildroot uses a toolchain wrapper where it
> could inject these flags vs appending like the current design does.
> This would allow all the packages with flag ordering issues and no
> formal releases, to not carry a patch in buildroot for the long term.

For the record: there is no flag ordering issue with PIE, contrary to
what we discussed in Brussels.

I think it is something I discussed further with my colleague Antoine
Tenart (in Cc). Basically, the issue is not that there is an ordering
requirement between -pie and -shared. The issue is that -pie and
-shared are incompatible with each other.

Passing -pie before -shared just papers over the problem, and basically
-shared "wins". Indeed, there is no point for a shared library to be
compiled PIE. PIE only makes sense for executables. Shared libraries
already need to be compiled as PIC, regardless of whether PIE is used
or not for executables.

The issue is of course that we hardly have control over when PIE is
used vs. PIC. But I think it's important to make it clear what the
exact problem is: it's not a flag ordering problem.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/4] package/Makefile.in: Do not use CPPFLAGS for hardening options

[Matthew Weber]

Stefan,

On Wed, Apr 25, 2018 at 1:45 AM, Stefan S?rensen
<stefan.sorensen@spectralink.com> wrote:
> The hardening options are compiler flags, not pure pre-processor flags, so
> put them in CFLAGS, not CPPFLAGS.
>
> This fixes build errors where -D_FORTIFY_SOURCE=2 whas put in CPPFLAGS and
> then applied to configure tests which could fail since the required -O2 is
> only in CFLAGS.
>

Confirmed that the current upstream has a bug where you can't get full
RELRO.  This patchset fixes that.

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