| Message ID | 1508936397-33651-2-git-send-email-matthew.weber@rockwellcollins.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/2] stack protector: moved option out of adv menu | expand |
Hi Matt, In the subject: s/RELFO/RELRO/ On 25-10-17 14:59, Matt Weber wrote: > 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. So perhaps you could make another patch that extends the autobuilders to test this option. Just modify utils/genrandconfig and in the gen_config function randomly enable these options. > Additional initial patches > which update linker ordering changes, etc will be upstreamed > and then submitted to buildroot as a patch or bump. > > 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 > v1 -> v2 > - Cosmetic caps on titles > --- > Config.in | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > package/Makefile.in | 25 +++++++++++++++++++++++ > 2 files changed, 83 insertions(+) > > diff --git a/Config.in b/Config.in > index 61f6aa6..68eff24 100644 > --- a/Config.in > +++ b/Config.in > @@ -730,6 +730,64 @@ endchoice > comment "Stack Smashing Protection needs a toolchain w/ SSP" > depends on !BR2_TOOLCHAIN_HAS_SSP > > +choice > + bool "RELRO Protection" > + 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. Do these options make sense at all in static linking? Isn't it implicit then (text is always readonly I think)? -pie is certainly invalid in static linking (at least with uClibc). Also, does it make sense on NOMMU, even shared? I mean, "readonly" doesn't mean much when there is no MMU I think... > + > +endchoice > + > +choice > + bool "Buffer-overflow Detection (FORTIFY_SOURCE)" > + 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. > + > +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) Do you know how these behave in uClibc and musl? Waldemar, any idea? Obviously the gcc part will still be activated, which covers about half of the functionality. > + > +endchoice > + > endmenu > > source "toolchain/Config.in" > diff --git a/package/Makefile.in b/package/Makefile.in > index a1a5316..c99361f 100644 > --- a/package/Makefile.in > +++ b/package/Makefile.in > @@ -144,6 +144,9 @@ TARGET_CXXFLAGS = $(TARGET_CFLAGS) > TARGET_FCFLAGS = $(TARGET_ABI) $(TARGET_OPTIMIZATION) $(TARGET_DEBUGGING) > TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS)) > > +TARGET_CFLAGS_RELRO = -Wl,-z,relro > +TARGET_CFLAGS_RELRO_FULL = -Wl,-z,now $(TARGET_CFLAGS_RELRO) > + > ifeq ($(BR2_BINFMT_FLAT),y) > TARGET_CFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-Wl$(comma)-elf2flt=-s$($(PKG)_FLAT_STACKSIZE),\ > -Wl$(comma)-elf2flt) > @@ -181,6 +184,28 @@ TARGET_CXXFLAGS += -fstack-protector-all > TARGET_FCFLAGS += -fstack-protector-all > endif > > +ifeq ($(BR2_RELRO_PARTIAL),y) > +TARGET_CFLAGS += $(TARGET_CFLAGS_RELRO) > +TARGET_CXXFLAGS += $(TARGET_CFLAGS_RELRO) > +TARGET_FCFLAGS += $(TARGET_CFLAGS_RELRO) Since these are linker flags, it _should_ be sufficient to add them to LDFLAGS. There may be some packages that don't listen to LDFLAGS so in that sense it could be a good idea to add it to CFLAGS as well, but I tend to prefer to fix the packages. Only, there is no easy way to detect that LDFLAGS are ignored. > +TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO) > +else ifeq ($(BR2_RELRO_FULL),y) > +TARGET_CFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) > +TARGET_CXXFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) > +TARGET_FCFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) > +TARGET_LDFLAGS += -pie > +endif > + > +ifeq ($(BR2_FORTIFY_SOURCE_1),y) > +TARGET_CFLAGS += -D_FORTIFY_SOURCE=1 > +TARGET_CXXFLAGS += -D_FORTIFY_SOURCE=1 It should go into CPPFLAGS (which automatically goes into CFLAGS and CXXFLAGS). > +TARGET_FCFLAGS += -D_FORTIFY_SOURCE=1 FCFLAGS indeed needs to be handled separately. Except: is FORTIFY valid/useful for Fortran? Regards, Arnout > +else ifeq ($(BR2_FORTIFY_SOURCE_2),y) > +TARGET_CFLAGS += -D_FORTIFY_SOURCE=2 > +TARGET_CXXFLAGS += -D_FORTIFY_SOURCE=2 > +TARGET_FCFLAGS += -D_FORTIFY_SOURCE=2 > +endif > + > ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y) > TARGET_CROSS = $(HOST_DIR)/bin/$(GNU_TARGET_NAME)- > else >
Arnout, On Mon, Nov 6, 2017 at 3:14 PM, Arnout Vandecappelle <arnout@mind.be> wrote: > Hi Matt, > > In the subject: s/RELFO/RELRO/ > > On 25-10-17 14:59, Matt Weber wrote: >> 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. > > So perhaps you could make another patch that extends the autobuilders to test > this option. Just modify utils/genrandconfig and in the gen_config function > randomly enable these options. Sure :) I've got an initial patchset of half dozen packages we've fixed but I'm working upstreaming on some of them first before submitting here. Another thing to note.... As things still build with various hardening flags possibly taking affect our not, we validated the configurations took affect by using the checksec.sh tool against all target binaries post- build then went through fixing packages. I've been thinking about how to do this checking..... I think at a minimum, I should add the test tool to this series and at least one buildroot test case to cover a static set of packages we've fixed so far. The other option would be a more active check post build that validates all target elfs are compliant. >> + >> +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. > > Do these options make sense at all in static linking? Isn't it implicit then > (text is always readonly I think)? -pie is certainly invalid in static linking > (at least with uClibc). > > Also, does it make sense on NOMMU, even shared? I mean, "readonly" doesn't mean > much when there is no MMU I think... > For both of those, I agree it doesn't make sense for static or NOMMU but I have asked my developer just to be sure. >> + >> +endchoice >> + >> +choice >> + bool "Buffer-overflow Detection (FORTIFY_SOURCE)" >> + 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. >> + >> +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) > > Do you know how these behave in uClibc and musl? Waldemar, any idea? Obviously > the gcc part will still be activated, which covers about half of the functionality. > Checking on the answer, but we ran through the complete test-pkg build list. I'll see which were skipped. We didn't see specific failures. >> + >> +endchoice >> + >> endmenu >> >> source "toolchain/Config.in" >> diff --git a/package/Makefile.in b/package/Makefile.in >> index a1a5316..c99361f 100644 >> --- a/package/Makefile.in >> +++ b/package/Makefile.in >> @@ -144,6 +144,9 @@ TARGET_CXXFLAGS = $(TARGET_CFLAGS) >> TARGET_FCFLAGS = $(TARGET_ABI) $(TARGET_OPTIMIZATION) $(TARGET_DEBUGGING) >> TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS)) >> >> +TARGET_CFLAGS_RELRO = -Wl,-z,relro >> +TARGET_CFLAGS_RELRO_FULL = -Wl,-z,now $(TARGET_CFLAGS_RELRO) >> + >> ifeq ($(BR2_BINFMT_FLAT),y) >> TARGET_CFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-Wl$(comma)-elf2flt=-s$($(PKG)_FLAT_STACKSIZE),\ >> -Wl$(comma)-elf2flt) >> @@ -181,6 +184,28 @@ TARGET_CXXFLAGS += -fstack-protector-all >> TARGET_FCFLAGS += -fstack-protector-all >> endif >> >> +ifeq ($(BR2_RELRO_PARTIAL),y) >> +TARGET_CFLAGS += $(TARGET_CFLAGS_RELRO) >> +TARGET_CXXFLAGS += $(TARGET_CFLAGS_RELRO) >> +TARGET_FCFLAGS += $(TARGET_CFLAGS_RELRO) > > Since these are linker flags, it _should_ be sufficient to add them to LDFLAGS. > There may be some packages that don't listen to LDFLAGS so in that sense it > could be a good idea to add it to CFLAGS as well, but I tend to prefer to fix > the packages. Only, there is no easy way to detect that LDFLAGS are ignored. What we ended up with here worked for a large test build ( lots of packages in a glibc embedded router configuration) and passed a minimal test-pkg cfg. That being said, we could go back and try flag combos but I know a few of them can't be in the linker vs just cflags (and vice versa). > >> +TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO) >> +else ifeq ($(BR2_RELRO_FULL),y) >> +TARGET_CFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) >> +TARGET_CXXFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) >> +TARGET_FCFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) >> +TARGET_LDFLAGS += -pie >> +endif >> + >> +ifeq ($(BR2_FORTIFY_SOURCE_1),y) >> +TARGET_CFLAGS += -D_FORTIFY_SOURCE=1 >> +TARGET_CXXFLAGS += -D_FORTIFY_SOURCE=1 > > It should go into CPPFLAGS (which automatically goes into CFLAGS and CXXFLAGS). Nice that cleans it up > >> +TARGET_FCFLAGS += -D_FORTIFY_SOURCE=1 > > FCFLAGS indeed needs to be handled separately. Except: is FORTIFY valid/useful > for Fortran? > It is not, shoot we got that feedback on the RFC. I'll remove it this time. Matt <div dir="auto">Arnout,<br><br>On Mon, Nov 6, 2017 at 3:14 PM, Arnout Vandecappelle <<a href="mailto:arnout@mind.be">arnout@mind.be</a>> wrote:<br>> Hi Matt,<br>><br>> In the subject: s/RELFO/RELRO/<br>><br>> On 25-10-17 14:59, Matt Weber wrote:<br>>> This enables a user to build a complete system using these<br>>> options. It is important to note that not all packages will<br>>> build correctly to start with.<br>><br>> So perhaps you could make another patch that extends the autobuilders to test<br>> this option. Just modify utils/genrandconfig and in the gen_config function<br>> randomly enable these options.<br><br>Sure :) I've got an initial patchset of half dozen packages we've fixed but I'm working upstreaming on some of them first before submitting here.<br><br><br><span style="font-family:sans-serif">Another thing to note.... As things still build with various hardening flags possibly taking affect our not, we validated the configurations took affect by using the checksec.sh tool against all target binaries post- build then went through fixing packages. I've been thinking about how to do this checking..... I think at a minimum, I should add the test tool to this series and at least one buildroot test case to cover a static set of packages we've fixed so far. The other option would be a more active check post build that validates all target elfs are compliant.</span><div dir="auto"><font face="sans-serif"><br></font></div><div dir="auto"><font face="sans-serif"><br></font>>> +<br>>> +config BR2_RELRO_FULL<br>>> + bool "Full"<br>>> + help<br>>> + This option includes the partial configuration, but also<br>>> + marks the GOT as read-only at the cost of initialization time<br>>> + during program loading, i.e every time an executable is started.<br>><br>> Do these options make sense at all in static linking? Isn't it implicit then<br>> (text is always readonly I think)? -pie is certainly invalid in static linking<br>> (at least with uClibc).<br>><br>> Also, does it make sense on NOMMU, even shared? I mean, "readonly" doesn't mean<br>> much when there is no MMU I think...<br>><br><br>For both of those, I agree it doesn't make sense for static or NOMMU but I have asked my developer just to be sure.<br><br>>> +<br>>> +endchoice<br>>> +<br>>> +choice<br>>> + bool "Buffer-overflow Detection (FORTIFY_SOURCE)"<br>>> + help<br>>> + Enable the _FORTIFY_SOURCE macro which introduces additional<br>>> + checks to detect buffer-overflows in the following standard library<br>>> + functions: memcpy, mempcpy, memmove, memset, strcpy, stpcpy,<br>>> + strncpy, strcat, strncat, sprintf, vsprintf, snprintf, vsnprintf,<br>>> + gets.<br>>> +<br>>> +config BR2_FORTIFY_SOURCE_NONE<br>>> + bool "None"<br>>> + help<br>>> + Enables additional checks to detect buffer-overflows.<br>>> +<br>>> +config BR2_FORTIFY_SOURCE_1<br>>> + bool "Conservative"<br>>> + help<br>>> + This option sets _FORTIFY_SOURCE set to 1 and only introduces<br>>> + checks that shouldn't change the behavior of conforming programs.<br>>> + Adds checks at compile-time only.<br>>> +<br>>> +config BR2_FORTIFY_SOURCE_2<br>>> + bool "Aggressive"<br>>> + help<br>>> + This option sets _FORTIFY_SOURCES set to 2 and some more checking<br>>> + is added, but some conforming programs might fail.<br>>> + Also adds checks at run-time (detected buffer overflow terminates<br>>> + the program)<br>><br>> Do you know how these behave in uClibc and musl? Waldemar, any idea? Obviously<br>> the gcc part will still be activated, which covers about half of the functionality.<br>><br><br>Checking on the answer, but we ran through the complete test-pkg build list. I'll see which were skipped. We didn't see specific failures.<br><br>>> +<br>>> +endchoice<br>>> +<br>>> endmenu<br>>><br>>> source "toolchain/Config.in"<br>>> diff --git a/package/Makefile.in b/package/Makefile.in<br>>> index a1a5316..c99361f 100644<br>>> --- a/package/Makefile.in<br>>> +++ b/package/Makefile.in<br>>> @@ -144,6 +144,9 @@ TARGET_CXXFLAGS = $(TARGET_CFLAGS)<br>>> TARGET_FCFLAGS = $(TARGET_ABI) $(TARGET_OPTIMIZATION) $(TARGET_DEBUGGING)<br>>> TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS))<br>>><br>>> +TARGET_CFLAGS_RELRO = -Wl,-z,relro<br>>> +TARGET_CFLAGS_RELRO_FULL = -Wl,-z,now $(TARGET_CFLAGS_RELRO)<br>>> +<br>>> ifeq ($(BR2_BINFMT_FLAT),y)<br>>> TARGET_CFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-Wl$(comma)-elf2flt=-s$($(PKG)_FLAT_STACKSIZE),\<br>>> -Wl$(comma)-elf2flt)<br>>> @@ -181,6 +184,28 @@ TARGET_CXXFLAGS += -fstack-protector-all<br>>> TARGET_FCFLAGS += -fstack-protector-all<br>>> endif<br>>><br>>> +ifeq ($(BR2_RELRO_PARTIAL),y)<br>>> +TARGET_CFLAGS += $(TARGET_CFLAGS_RELRO)<br>>> +TARGET_CXXFLAGS += $(TARGET_CFLAGS_RELRO)<br>>> +TARGET_FCFLAGS += $(TARGET_CFLAGS_RELRO)<br>><br>> Since these are linker flags, it _should_ be sufficient to add them to LDFLAGS.<br>> There may be some packages that don't listen to LDFLAGS so in that sense it<br>> could be a good idea to add it to CFLAGS as well, but I tend to prefer to fix<br>> the packages. Only, there is no easy way to detect that LDFLAGS are ignored.<div dir="auto"><br></div><div dir="auto">What we ended up with here worked for a large test build ( lots of packages in a glibc embedded router configuration) and passed a minimal test-pkg cfg. That being said, we could go back and try flag combos but I know a few of them can't be in the linker vs just cflags (and vice versa). </div><div dir="auto"><br>><br>>> +TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO)<br>>> +else ifeq ($(BR2_RELRO_FULL),y)<br>>> +TARGET_CFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL)<br>>> +TARGET_CXXFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL)<br>>> +TARGET_FCFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL)<br>>> +TARGET_LDFLAGS += -pie<br>>> +endif<br>>> +<br>>> +ifeq ($(BR2_FORTIFY_SOURCE_1),y)<br>>> +TARGET_CFLAGS += -D_FORTIFY_SOURCE=1<br>>> +TARGET_CXXFLAGS += -D_FORTIFY_SOURCE=1<br>><br>> It should go into CPPFLAGS (which automatically goes into CFLAGS and CXXFLAGS).<div dir="auto"><br></div><div dir="auto">Nice that cleans it up</div><div dir="auto"><br>><br>>> +TARGET_FCFLAGS += -D_FORTIFY_SOURCE=1<br>><br>> FCFLAGS indeed needs to be handled separately. Except: is FORTIFY valid/useful<br>> for Fortran?<br>><br><br><div dir="auto">It is not, shoot we got that feedback on the RFC. I'll remove it this time.</div><div dir="auto"><br></div><div dir="auto">Matt</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"> <br></div></div></div></div></div>
Arnout, On Mon, Nov 6, 2017 at 6:08 PM, Matthew Weber <matthew.weber@rockwellcollins.com> wrote: > Arnout, > > On Mon, Nov 6, 2017 at 3:14 PM, Arnout Vandecappelle <arnout@mind.be> wrote: >> Hi Matt, >> >> In the subject: s/RELFO/RELRO/ >> >> On 25-10-17 14:59, Matt Weber wrote: >>> 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. >> >> So perhaps you could make another patch that extends the autobuilders to >> test >> this option. Just modify utils/genrandconfig and in the gen_config >> function >> randomly enable these options. > > Sure :) I've got an initial patchset of half dozen packages we've fixed but > I'm working upstreaming on some of them first before submitting here. > > > Another thing to note.... As things still build with various hardening flags > possibly taking affect our not, we validated the configurations took affect > by using the checksec.sh tool against all target binaries post- build then > went through fixing packages. I've been thinking about how to do this > checking..... I think at a minimum, I should add the test tool to this > series and at least one buildroot test case to cover a static set of > packages we've fixed so far. The other option would be a more active check > post build that validates all target elfs are compliant. > > >>> + >>> +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. >> >> Do these options make sense at all in static linking? Isn't it implicit >> then >> (text is always readonly I think)? -pie is certainly invalid in static >> linking >> (at least with uClibc). >> >> Also, does it make sense on NOMMU, even shared? I mean, "readonly" doesn't >> mean >> much when there is no MMU I think... >> > > For both of those, I agree it doesn't make sense for static or NOMMU but I > have asked my developer just to be sure. > > >>> + >>> +endchoice >>> + >>> +choice >>> + bool "Buffer-overflow Detection (FORTIFY_SOURCE)" >>> + 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. >>> + >>> +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) >> >> Do you know how these behave in uClibc and musl? Waldemar, any idea? >> Obviously >> the gcc part will still be activated, which covers about half of the >> functionality. >> > > Checking on the answer, but we ran through the complete test-pkg build list. > I'll see which were skipped. We didn't see specific failures. > The set of test packages I used ended up forcing a glibc only test-pkg build. I'll rerun with a basic busybox scenario. > >>> + >>> +endchoice >>> + >>> endmenu >>> >>> source "toolchain/Config.in" >>> diff --git a/package/Makefile.in b/package/Makefile.in >>> index a1a5316..c99361f 100644 >>> --- a/package/Makefile.in >>> +++ b/package/Makefile.in >>> @@ -144,6 +144,9 @@ TARGET_CXXFLAGS = $(TARGET_CFLAGS) >>> TARGET_FCFLAGS = $(TARGET_ABI) $(TARGET_OPTIMIZATION) $(TARGET_DEBUGGING) >>> TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS)) >>> >>> +TARGET_CFLAGS_RELRO = -Wl,-z,relro >>> +TARGET_CFLAGS_RELRO_FULL = -Wl,-z,now $(TARGET_CFLAGS_RELRO) >>> + >>> ifeq ($(BR2_BINFMT_FLAT),y) >>> TARGET_CFLAGS += $(if >>> $($(PKG)_FLAT_STACKSIZE),-Wl$(comma)-elf2flt=-s$($(PKG)_FLAT_STACKSIZE),\ >>> -Wl$(comma)-elf2flt) >>> @@ -181,6 +184,28 @@ TARGET_CXXFLAGS += -fstack-protector-all >>> TARGET_FCFLAGS += -fstack-protector-all >>> endif >>> >>> +ifeq ($(BR2_RELRO_PARTIAL),y) >>> +TARGET_CFLAGS += $(TARGET_CFLAGS_RELRO) >>> +TARGET_CXXFLAGS += $(TARGET_CFLAGS_RELRO) >>> +TARGET_FCFLAGS += $(TARGET_CFLAGS_RELRO) >> >> Since these are linker flags, it _should_ be sufficient to add them to >> LDFLAGS. >> There may be some packages that don't listen to LDFLAGS so in that sense >> it >> could be a good idea to add it to CFLAGS as well, but I tend to prefer to >> fix >> the packages. Only, there is no easy way to detect that LDFLAGS are >> ignored. > > What we ended up with here worked for a large test build ( lots of packages > in a glibc embedded router configuration) and passed a minimal test-pkg cfg. > That being said, we could go back and try flag combos but I know a few of > them can't be in the linker vs just cflags (and vice versa). > >> >>> +TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO) >>> +else ifeq ($(BR2_RELRO_FULL),y) >>> +TARGET_CFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) >>> +TARGET_CXXFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) >>> +TARGET_FCFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) >>> +TARGET_LDFLAGS += -pie >>> +endif >>> + >>> +ifeq ($(BR2_FORTIFY_SOURCE_1),y) >>> +TARGET_CFLAGS += -D_FORTIFY_SOURCE=1 >>> +TARGET_CXXFLAGS += -D_FORTIFY_SOURCE=1 >> >> It should go into CPPFLAGS (which automatically goes into CFLAGS and >> CXXFLAGS). > > Nice that cleans it up > >> >>> +TARGET_FCFLAGS += -D_FORTIFY_SOURCE=1 >> >> FCFLAGS indeed needs to be handled separately. Except: is FORTIFY >> valid/useful >> for Fortran? >> > > It is not, shoot we got that feedback on the RFC. I'll remove it this time. > > Matt > > >
Matt, please snip away some text when replying to a long mail, otherwise it's difficult to find back your answer in the middle of the long quote. On 07-11-17 04:25, Matthew Weber wrote: > Arnout, > > On Mon, Nov 6, 2017 at 6:08 PM, Matthew Weber > <matthew.weber@rockwellcollins.com> wrote: >> Arnout, >> >> On Mon, Nov 6, 2017 at 3:14 PM, Arnout Vandecappelle <arnout@mind.be> wrote: [snip] >>> Do you know how these behave in uClibc and musl? Waldemar, any idea? >>> Obviously >>> the gcc part will still be activated, which covers about half of the >>> functionality. >>> >> >> Checking on the answer, but we ran through the complete test-pkg build list. >> I'll see which were skipped. We didn't see specific failures. >> > > The set of test packages I used ended up forcing a glibc only test-pkg > build. I'll rerun with a basic busybox scenario. It will build, that's for sure. My question is: will it actually do anything useful? The effect of fortify is shared a bit between GCC and glibc. E.g. 'memset' has a GCC implementation (used when it can be inlined) and a glibc implementation (used when it's too big or unpredictable). As far as I can see, neither uClibc nor musl have support for FORTIFY. So only the GCC part will take effect. But I think that that is so little that it's hardly worth it. Regards, Arnout
Hi Arnout, Arnout Vandecappelle wrote, > >>> Do you know how these behave in uClibc and musl? Waldemar, any idea? > >>> Obviously > >>> the gcc part will still be activated, which covers about half of the > >>> functionality. > >>> > >> > >> Checking on the answer, but we ran through the complete test-pkg build list. > >> I'll see which were skipped. We didn't see specific failures. > >> > > > > The set of test packages I used ended up forcing a glibc only test-pkg > > build. I'll rerun with a basic busybox scenario. > > It will build, that's for sure. My question is: will it actually do anything > useful? The effect of fortify is shared a bit between GCC and glibc. E.g. > 'memset' has a GCC implementation (used when it can be inlined) and a glibc > implementation (used when it's too big or unpredictable). As far as I can see, > neither uClibc nor musl have support for FORTIFY. So only the GCC part will take > effect. But I think that that is so little that it's hardly worth it. I can confirm uClibc-ng does not support FORTIFY security mechanisms. There is some old unused code since 82098ab9b853c33ee8ade61c9510b295cc696de1, but I am considering removing it, because it is unused and incomplete. best regards Waldemar
On 07-11-17 20:31, Waldemar Brodkorb wrote: > Hi Arnout, > Arnout Vandecappelle wrote, > >>>>> Do you know how these behave in uClibc and musl? Waldemar, any idea? >>>>> Obviously >>>>> the gcc part will still be activated, which covers about half of the >>>>> functionality. >>>>> >>>> >>>> Checking on the answer, but we ran through the complete test-pkg build list. >>>> I'll see which were skipped. We didn't see specific failures. >>>> >>> >>> The set of test packages I used ended up forcing a glibc only test-pkg >>> build. I'll rerun with a basic busybox scenario. >> >> It will build, that's for sure. My question is: will it actually do anything >> useful? The effect of fortify is shared a bit between GCC and glibc. E.g. >> 'memset' has a GCC implementation (used when it can be inlined) and a glibc >> implementation (used when it's too big or unpredictable). As far as I can see, >> neither uClibc nor musl have support for FORTIFY. So only the GCC part will take >> effect. But I think that that is so little that it's hardly worth it. > > I can confirm uClibc-ng does not support FORTIFY security > mechanisms. > There is some old unused code since > 82098ab9b853c33ee8ade61c9510b295cc696de1, but I am considering > removing it, because it is unused and incomplete. So I did a little bit more research [1], and it looks like a significant part of the FORTIFY support comes from gcc. However, the gcc support is only activated if the appropriate header is included that replaces memcpy with __builtin___memcpy_chk etc. In addition, GCC's check will call __fortify_fail if the check fails. So you can easily find out if it's supported: does the libc define __fortify_fail or not? Neither uClibc nor musl does. In short, the fortify options should depend on glibc. Regards, Arnout [1] https://access.redhat.com/blogs/766093/posts/1976213
6.11.2017, 23:14, Arnout Vandecappelle kirjoitti: > @@ -181,6 +184,28 @@ TARGET_CXXFLAGS += -fstack-protector-all > TARGET_FCFLAGS += -fstack-protector-all > endif > > +ifeq ($(BR2_RELRO_PARTIAL),y) > +TARGET_CFLAGS += $(TARGET_CFLAGS_RELRO) > +TARGET_CXXFLAGS += $(TARGET_CFLAGS_RELRO) > +TARGET_FCFLAGS += $(TARGET_CFLAGS_RELRO) > Since these are linker flags, it _should_ be sufficient to add them to LDFLAGS. > There may be some packages that don't listen to LDFLAGS so in that sense it > could be a good idea to add it to CFLAGS as well, but I tend to prefer to fix > the packages. Only, there is no easy way to detect that LDFLAGS are ignored. > There could be a way to tell if package shows middle finger to CFLAGS/CXXFLAGS/LDFLAGS and just ignores the hardening options. There's a little perl script called hardening-check that could be used to do post installation checking of what packages actually respected the flags. http://manpages.ubuntu.com/manpages/trusty/man1/hardening-check.1.html I have a copy of that perl script here: https://www.orwell1984.today/hardening-check I also did the following little test: 1. First compiled turbovnc against i686-uclibc without any hardening and then running "hardening-check -c output/target/usr/bin/Xvnc" with following results: output/target/usr/bin/Xvnc: Position Independent Executable: no, normal executable! Stack protected: no, not found! Fortify Source functions: no, only unprotected functions found! Read-only relocations: yes Immediate binding: no, not found! 2. Then forced the gcc compiler to use hardening features by using GCC Spec File, so that if turbovnc did ignore CFLAGS/CXXFLAGS/LDFLAGS it would still be forcefeed the right hardening options, like this: - Dump the built-in specs file "$(HOST_CC) -dumpspecs > specs" and then edit it to enable all the hardening stuff (here's mine for i686-uclibc, forgot to enable stack-protection: https://www.orwell1984.today/specs) - Find location where gcc looks for specs file "dirname $($(HOST_CC) --print-libgcc-file-name)" and move the edited specs file there - Rebuild turbovnc - And finally, check "hardening-check -c output/target/usr/bin/Xvnc" with following result: output/target/usr/bin/Xvnc: Position Independent Executable: yes Stack protected: no, not found! Fortify Source functions: no, only unprotected functions found! Read-only relocations: yes Immediate binding: yes Here turbovnc built with pie, relro,now and if I would have remember to enable stack protection in toolchain, also with stack protection. So that's a one way to force & check hardening afterwards. But have to admit, not very elegant way. Maybe there could be hardened directory with some premade "profiles" (gcc spec files) for various arch-lib combos which could be selected from menu and then the buildroot cross-compiler would have it's `dirname $($HOST_CC) --print-libgcc-file-name`/specs be a just symlink to that arch-lib combos like this: output/host/lib/gcc/i686-buildroot-linux-uclibc/6.4.0/specs --> ../../../../../../hardened/i686/uclibc/specs If selecting vanilla, non-hardened toolchain from menu, it would just remove the symlink. And maybe there could be an option to run hardening-check script at the end of installation. Just throwing thoughts around -S-
On 08-11-17 03:01, Stefan Fröberg wrote: > > > 6.11.2017, 23:14, Arnout Vandecappelle kirjoitti: >> @@ -181,6 +184,28 @@ TARGET_CXXFLAGS += -fstack-protector-all >> TARGET_FCFLAGS += -fstack-protector-all >> endif >> +ifeq ($(BR2_RELRO_PARTIAL),y) >> +TARGET_CFLAGS += $(TARGET_CFLAGS_RELRO) >> +TARGET_CXXFLAGS += $(TARGET_CFLAGS_RELRO) >> +TARGET_FCFLAGS += $(TARGET_CFLAGS_RELRO) >> Since these are linker flags, it _should_ be sufficient to add them to LDFLAGS. >> There may be some packages that don't listen to LDFLAGS so in that sense it >> could be a good idea to add it to CFLAGS as well, but I tend to prefer to fix >> the packages. Only, there is no easy way to detect that LDFLAGS are ignored. >> > > There could be a way to tell if package shows middle finger to > CFLAGS/CXXFLAGS/LDFLAGS > and just ignores the hardening options. > > There's a little perl script called hardening-check that could be used to do > post installation checking > of what packages actually respected the flags. > > http://manpages.ubuntu.com/manpages/trusty/man1/hardening-check.1.html > > I have a copy of that perl script here: > https://www.orwell1984.today/hardening-check Yeah, Matthew already proposed to include a (different) hardening check script. I think that that's a good idea. [snip] > Maybe there could be hardened directory with some premade "profiles" (gcc spec > files) for various arch-lib combos > which could be selected from menu and then the buildroot cross-compiler would have > it's `dirname $($HOST_CC) --print-libgcc-file-name`/specs be a just symlink to > that arch-lib combos like this: > > output/host/lib/gcc/i686-buildroot-linux-uclibc/6.4.0/specs --> > ../../../../../../hardened/i686/uclibc/specs > > If selecting vanilla, non-hardened toolchain from menu, it would just remove the > symlink. Hm, I think messing with the specs file is making things complicated... However, we do have a toolchain wrapper and we could add the hardening options in there instead of in CFLAGS. One small caveat, however: some packages may not build at all with some of the hardening options (-pie for example), in particular bootloaders and kernels are prone to be problematic. Otherwise it sounds like a viable option though. > And maybe there could be an option to run hardening-check script at the end of > installation. Yep, certainly. Regards, Arnout > > Just throwing thoughts around > -S- > > > > > > > > > >
diff --git a/Config.in b/Config.in index 61f6aa6..68eff24 100644 --- a/Config.in +++ b/Config.in @@ -730,6 +730,64 @@ endchoice comment "Stack Smashing Protection needs a toolchain w/ SSP" depends on !BR2_TOOLCHAIN_HAS_SSP +choice + bool "RELRO Protection" + 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 + +choice + bool "Buffer-overflow Detection (FORTIFY_SOURCE)" + 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. + +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 + endmenu source "toolchain/Config.in" diff --git a/package/Makefile.in b/package/Makefile.in index a1a5316..c99361f 100644 --- a/package/Makefile.in +++ b/package/Makefile.in @@ -144,6 +144,9 @@ TARGET_CXXFLAGS = $(TARGET_CFLAGS) TARGET_FCFLAGS = $(TARGET_ABI) $(TARGET_OPTIMIZATION) $(TARGET_DEBUGGING) TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS)) +TARGET_CFLAGS_RELRO = -Wl,-z,relro +TARGET_CFLAGS_RELRO_FULL = -Wl,-z,now $(TARGET_CFLAGS_RELRO) + ifeq ($(BR2_BINFMT_FLAT),y) TARGET_CFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-Wl$(comma)-elf2flt=-s$($(PKG)_FLAT_STACKSIZE),\ -Wl$(comma)-elf2flt) @@ -181,6 +184,28 @@ TARGET_CXXFLAGS += -fstack-protector-all TARGET_FCFLAGS += -fstack-protector-all endif +ifeq ($(BR2_RELRO_PARTIAL),y) +TARGET_CFLAGS += $(TARGET_CFLAGS_RELRO) +TARGET_CXXFLAGS += $(TARGET_CFLAGS_RELRO) +TARGET_FCFLAGS += $(TARGET_CFLAGS_RELRO) +TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO) +else ifeq ($(BR2_RELRO_FULL),y) +TARGET_CFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) +TARGET_CXXFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) +TARGET_FCFLAGS += -fPIE $(TARGET_CFLAGS_RELRO_FULL) +TARGET_LDFLAGS += -pie +endif + +ifeq ($(BR2_FORTIFY_SOURCE_1),y) +TARGET_CFLAGS += -D_FORTIFY_SOURCE=1 +TARGET_CXXFLAGS += -D_FORTIFY_SOURCE=1 +TARGET_FCFLAGS += -D_FORTIFY_SOURCE=1 +else ifeq ($(BR2_FORTIFY_SOURCE_2),y) +TARGET_CFLAGS += -D_FORTIFY_SOURCE=2 +TARGET_CXXFLAGS += -D_FORTIFY_SOURCE=2 +TARGET_FCFLAGS += -D_FORTIFY_SOURCE=2 +endif + ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y) TARGET_CROSS = $(HOST_DIR)/bin/$(GNU_TARGET_NAME)- else
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. Additional initial patches which update linker ordering changes, etc will be upstreamed and then submitted to buildroot as a patch or bump. 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 v1 -> v2 - Cosmetic caps on titles --- Config.in | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++ package/Makefile.in | 25 +++++++++++++++++++++++ 2 files changed, 83 insertions(+)