| Message ID | 20180425064518.31797-1-stefan.sorensen@spectralink.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/4] package/Makefile.in: Do not use CPPFLAGS for hardening options | expand |
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
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
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
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
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
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
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>
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)
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(-)