Re: RFC: Introduce -fhardened to enable security-related flags
> From: Qing Zhao > Date: Tue, 19 Sep 2023 14:19:09 + > > On Sep 17, 2023, at 12:36 PM, Hans-Peter Nilsson via Gcc-patches > > wrote: > >> From: Sam James > >> Date: Sun, 17 Sep 2023 05:00:37 +0100 > >> Did some bug ever get filed for this to see if we can do a bit > >> better here? > > > > Not that I know of; neither for systemd nor gcc. > > Then, is it convenient to file a bug on this? A fair request, but I can't commit to analyze it myself to the usual level, producing a self-contained test-case. I see Sam James was super fast and has already added you to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111523 (thanks!) > That will > be very helpful for us to locate the issue and fix it. > > Before I committing the -ftrivial-auto-var-init patch, I > have done some performance testing on CPU2017 for x86 and > aarch64, > The runtime overhead was quite limited. Perhaps it would also make sense to performance-test on network-facing software and system software such as systemd? > Which platform the 35% performance slowdown was on? arm-linux-eabi on ARM Cortex-A9. brgds, H-P
Re: RFC: Introduce -fhardened to enable security-related flags
I'd like to provide some data-points on hardening-related flags, as I've spent some time with Sam documenting their usage across various distributions here[1]. I also attached the relevant file to this email for archiving purposes. tl'dr: the suggested flag selection for `-fhardened` is not only sound but great. The only issue I see is `-fcf-protection=full`, since musl libc doesn't support it for now, so it might cause runtime issues. 1. https://github.com/jvoisin/compiler-flags-distro/blob/main/README.md -- Julien (jvoisin) Voisin GPG: 04D041E8171901CC dustri.org# Usage of enabled-by-default hardening-related compiler flags across Linux distributions |.| Alpine | Debian | Fedora| Gentoo Hardened | Ubuntu | OpenSUSE | ArchLinux | OpenBSD | Chimera Linux | Android | |-|||---|-||--|---|-|---|-| |`-D_FORTIFY_SOURCE=2`|[yes](https://gitlab.alpinelinux.org/alpine/tsc/-/issues/64)|[2011](https://github.com/guillemj/dpkg/commit/f3bb7d4939ae95cf44c89e8f599e7ed5da431e57)|[2007](https://listman.redhat.com/archives/fedora-devel-announce/2007-September/msg00015.html)|superseded|[2008](https://wiki.ubuntu.com/ToolChain/CompilerFlags#A-D_FORTIFY_SOURCE.3D2)|[2005](https://en.opensuse.org/openSUSE:Security_Features)|[2021](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/f409a72342bf37017f190021970efaaeac1bb619)|?|[yes](https://github.com/chimera-linux/cports/commit/9b78e55067f024b8dbf9fbceb472e8705f84ed5d)|[2017](https://android-developers.googleblog.com/2019/10/introducing-ndk-r21-our-first-long-term.html)| |`-D_FORTIFY_SOURCE=3`|no |[no](https://wiki.debian.org/Hardening)|[2023](https://fedoraproject.org/wiki/Changes/Add_FORTIFY_SOURCE%3D3_to_distribution_build_flags)|[2022](https://bugs.gentoo.org/876893)|[no](https://bugs.launchpad.net/ubuntu/+source/gcc-12/+bug/2012440)|[2023](https://en.opensuse.org/openSUSE:Security_Features)|[not](https://gitlab.archlinux.org/archlinux/rfcs/-/merge_requests/17) [yet](https://gitlab.archlinux.org/archlinux/devtools/-/merge_requests/191)|?|no|[no](https://android.googlesource.com/platform/bionic.git/+/HEAD/docs/status.md#fortify)| |`-D_GLIBCXX_ASSERTIONS` |[2023](https://gitlab.alpinelinux.org/alpine/abuild/-/commit/44c933da5d8e364d6cd755071f629c05444191df)|no|[2018](https://fedoraproject.org/wiki/Changes/HardeningFlags28)|[2022](https://bugs.gentoo.org/876895)|[no](https://bugs.launchpad.net/ubuntu/+source/gcc-12/+bug/2016042)|yes|[2021](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/f409a72342bf37017f190021970efaaeac1bb619)|no|no|no| |`-D_LIBCPP_ENABLE_HARDENED_MODE` (llvm17) |[not yet](https://gitlab.alpinelinux.org/alpine/abuild/-/commit/65b5d578b2d9e3f170bc9d31dcd23f0014cfc36e)[^1]|no|no|[2023](https://bugs.gentoo.org/85)|no|no|no|?|?|no| |`-D_LIBCXX_ENABLE_ASSERTIONS` (llvm16) |no|no|no|superseded|no|no|no|?|[yes](https://github.com/search?q=repo%3Achimera-linux%2Fcports+DLIBCXX_ENABLE_ASSERTIONS=code)|?| |`-Wformat -Wformat-security`/`-Wformat=2` |[2023](https://gitlab.alpinelinux.org/alpine/abuild/-/commit/ca8375f0e9d1715e38c14c918c675d6774f1eabc)|[2011](https://salsa.debian.org/toolchain-team/gcc/-/blob/master/debian/patches/gcc-distro-specs.diff)|[2013](https://fedoraproject.org/wiki/Changes/FormatSecurity)|[2009](https://bugs.gentoo.org/259417)|[2008](https://wiki.ubuntu.com/ToolChain/CompilerFlags)|yes|[2021](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/f409a72342bf37017f190021970efaaeac1bb619)|?|[2023](https://github.com/chimera-linux/cports/commit/ad898a6b645b11dee989f4504e89577f5395ba24)|[2010](https://source.android.com/docs/security/enhancements/enhancements41)| |`-Wl,-z,noexecstack` |yes|yes|yes|yes|yes|yes|yes|yes|yes|yes| |`-Wl,-z,relro`/`-Wl,-z,now` |[yes](https://gitlab.alpinelinux.org/alpine/tsc/-/issues/64)|[yes](https://salsa.debian.org/toolchain-team/gcc/-/blob/master/debian/patches/gcc-distro-specs.diff)|[2015](https://fedoraproject.org/wiki/Security_Features_Matrix#Built_as_PIE)|[yes](https://wiki.gentoo.org/wiki/Hardened/Toolchain)|[2008](https://wiki.ubuntu.com/ToolChain/CompilerFlags)|[2006](https://en.opensuse.org/openSUSE:Security_Features)|[2017](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/b4b2bb56174493ea2e60b1eecc0085db421908cc)|?|[yes](https://github.com/chimera-linux/cports/commit/9b78e55067f024b8dbf9fbceb472e8705f84ed5d)|[2013](https://source.android.com/docs/security/enhancements/enhancements43)| |`-fPIE`/`-fPIC`/…
Re: RFC: Introduce -fhardened to enable security-related flags
> On Sep 17, 2023, at 12:36 PM, Hans-Peter Nilsson via Gcc-patches > wrote: > >> From: Sam James >> Date: Sun, 17 Sep 2023 05:00:37 +0100 > >> Hans-Peter Nilsson via Gcc-patches writes: >> Date: Tue, 29 Aug 2023 15:42:27 -0400 From: Marek Polacek via Gcc-patches >>> Surely, there must be no ABI impact, the option cannot cause severe performance issues, >>> Currently, -fhardened enables: >>> ... -ftrivial-auto-var-init=zero >>> Thoughts? >>> >>> Regarding -ftrivial-auto-var-init=zero, I was consulted when >>> colleagues investigating a performance regression >>> pint-pointed it as *causing severe performance issues*; >>> cf. https://github.com/systemd/systemd.git commit 1a4e392760 >>> (TL;DR: adds "-ftrivial-auto-var-init=zero" to the systemd >>> build). >>> >>> The situation was described as "we noticed that some test >>> suites takes 35% percent longer time to finish. After >>> further investigation it was noticed that running systemctl >>> unmask x takes around 5s more time on [version including >>> patch vs. before that patch]" (timing out some tests). >>> Reverting that patch fixed the drop in performance. >> >> Did some bug ever get filed for this to see if we can do a bit >> better here? > > Not that I know of; neither for systemd nor gcc. Then, is it convenient to file a bug on this? That will be very helpful for us to locate the issue and fix it. Before I committing the -ftrivial-auto-var-init patch, I have done some performance testing on CPU2017 for x86 and aarch64, The runtime overhead was quite limited. Which platform the 35% performance slowdown was on? Thanks. Qing > >> Some slowdown doesn't mean it's of the expected magnitude. > > Can you please rephrase that? > >>> Just a data point, but I believe also exactly your intended >>> use. IMO including -ftrivial-auto-var-init is worth extra >>> consideration. >>> >>> Alternatively, strike the while "cannot cause severe >>> performance issues". >>> >>> brgds, H-P
Re: RFC: Introduce -fhardened to enable security-related flags
> From: Sam James > Date: Mon, 18 Sep 2023 08:21:45 +0100 > Hans-Peter Nilsson writes: > > >> From: Sam James > >> Date: Sun, 17 Sep 2023 05:00:37 +0100 > > > >> Hans-Peter Nilsson via Gcc-patches writes: > >> > The situation was described as "we noticed that some test > >> > suites takes 35% percent longer time to finish. After > >> > further investigation it was noticed that running systemctl > >> > unmask x takes around 5s more time on [version including > >> > patch vs. before that patch]" (timing out some tests). > >> > Reverting that patch fixed the drop in performance. > >> > >> Did some bug ever get filed for this to see if we can do a bit > >> better here? > > > > Not that I know of; neither for systemd nor gcc. > > > >> Some slowdown doesn't mean it's of the expected magnitude. > > > > Can you please rephrase that? > > Sorry, what I meant was: yes, I'd expect a bit of a slowdown > with this option that's unavoidable, but what you're describing > sounds far beyond the normal case. Thanks. My take is that it's something I more or less expect when indiscriminately using that option. IOW, for code consciously using that option, a step is necessary where the code is vetted and adjusted, for example to remove large structures and arrays allocated on the stack (just a guess). > (to the extent that it might be > we're either missing an optimisation in GCC for the relevant bits, > or the systemd parts being exercised here are pathological.) Sorry, I don't have anything more in terms of local observations from that investigation; no perf and flamegraphs. Again as above, the issue should be observable as a noticeable slowdown for "systemctl unmask x" for a systemd compiled with/without 1a4e392760. brgds, H-P
Re: RFC: Introduce -fhardened to enable security-related flags
Hans-Peter Nilsson writes: >> From: Sam James >> Date: Sun, 17 Sep 2023 05:00:37 +0100 > >> Hans-Peter Nilsson via Gcc-patches writes: >> >> >> Date: Tue, 29 Aug 2023 15:42:27 -0400 >> >> From: Marek Polacek via Gcc-patches >> > >> >> Surely, there must be no ABI impact, the option cannot cause >> >> severe performance issues, >> > >> >> Currently, -fhardened enables: >> > ... >> >> -ftrivial-auto-var-init=zero >> > >> >> Thoughts? >> > >> > Regarding -ftrivial-auto-var-init=zero, I was consulted when >> > colleagues investigating a performance regression >> > pint-pointed it as *causing severe performance issues*; >> > cf. https://github.com/systemd/systemd.git commit 1a4e392760 >> > (TL;DR: adds "-ftrivial-auto-var-init=zero" to the systemd >> > build). >> > >> > The situation was described as "we noticed that some test >> > suites takes 35% percent longer time to finish. After >> > further investigation it was noticed that running systemctl >> > unmask x takes around 5s more time on [version including >> > patch vs. before that patch]" (timing out some tests). >> > Reverting that patch fixed the drop in performance. >> >> Did some bug ever get filed for this to see if we can do a bit >> better here? > > Not that I know of; neither for systemd nor gcc. > >> Some slowdown doesn't mean it's of the expected magnitude. > > Can you please rephrase that? Sorry, what I meant was: yes, I'd expect a bit of a slowdown with this option that's unavoidable, but what you're describing sounds far beyond the normal case. (to the extent that it might be we're either missing an optimisation in GCC for the relevant bits, or the systemd parts being exercised here are pathological.)
Re: RFC: Introduce -fhardened to enable security-related flags
> From: Sam James > Date: Sun, 17 Sep 2023 05:00:37 +0100 > Hans-Peter Nilsson via Gcc-patches writes: > > >> Date: Tue, 29 Aug 2023 15:42:27 -0400 > >> From: Marek Polacek via Gcc-patches > > > >> Surely, there must be no ABI impact, the option cannot cause > >> severe performance issues, > > > >> Currently, -fhardened enables: > > ... > >> -ftrivial-auto-var-init=zero > > > >> Thoughts? > > > > Regarding -ftrivial-auto-var-init=zero, I was consulted when > > colleagues investigating a performance regression > > pint-pointed it as *causing severe performance issues*; > > cf. https://github.com/systemd/systemd.git commit 1a4e392760 > > (TL;DR: adds "-ftrivial-auto-var-init=zero" to the systemd > > build). > > > > The situation was described as "we noticed that some test > > suites takes 35% percent longer time to finish. After > > further investigation it was noticed that running systemctl > > unmask x takes around 5s more time on [version including > > patch vs. before that patch]" (timing out some tests). > > Reverting that patch fixed the drop in performance. > > Did some bug ever get filed for this to see if we can do a bit > better here? Not that I know of; neither for systemd nor gcc. > Some slowdown doesn't mean it's of the expected magnitude. Can you please rephrase that? > > Just a data point, but I believe also exactly your intended > > use. IMO including -ftrivial-auto-var-init is worth extra > > consideration. > > > > Alternatively, strike the while "cannot cause severe > > performance issues". > > > > brgds, H-P >
Re: RFC: Introduce -fhardened to enable security-related flags
Hans-Peter Nilsson via Gcc-patches writes: >> Date: Tue, 29 Aug 2023 15:42:27 -0400 >> From: Marek Polacek via Gcc-patches > >> Surely, there must be no ABI impact, the option cannot cause >> severe performance issues, > >> Currently, -fhardened enables: > ... >> -ftrivial-auto-var-init=zero > >> Thoughts? > > Regarding -ftrivial-auto-var-init=zero, I was consulted when > colleagues investigating a performance regression > pint-pointed it as *causing severe performance issues*; > cf. https://github.com/systemd/systemd.git commit 1a4e392760 > (TL;DR: adds "-ftrivial-auto-var-init=zero" to the systemd > build). > > The situation was described as "we noticed that some test > suites takes 35% percent longer time to finish. After > further investigation it was noticed that running systemctl > unmask x takes around 5s more time on [version including > patch vs. before that patch]" (timing out some tests). > Reverting that patch fixed the drop in performance. Did some bug ever get filed for this to see if we can do a bit better here? Some slowdown doesn't mean it's of the expected magnitude. > > Just a data point, but I believe also exactly your intended > use. IMO including -ftrivial-auto-var-init is worth extra > consideration. > > Alternatively, strike the while "cannot cause severe > performance issues". > > brgds, H-P
Re: RFC: Introduce -fhardened to enable security-related flags
> Date: Tue, 29 Aug 2023 15:42:27 -0400 > From: Marek Polacek via Gcc-patches > Surely, there must be no ABI impact, the option cannot cause > severe performance issues, > Currently, -fhardened enables: ... > -ftrivial-auto-var-init=zero > Thoughts? Regarding -ftrivial-auto-var-init=zero, I was consulted when colleagues investigating a performance regression pint-pointed it as *causing severe performance issues*; cf. https://github.com/systemd/systemd.git commit 1a4e392760 (TL;DR: adds "-ftrivial-auto-var-init=zero" to the systemd build). The situation was described as "we noticed that some test suites takes 35% percent longer time to finish. After further investigation it was noticed that running systemctl unmask x takes around 5s more time on [version including patch vs. before that patch]" (timing out some tests). Reverting that patch fixed the drop in performance. Just a data point, but I believe also exactly your intended use. IMO including -ftrivial-auto-var-init is worth extra consideration. Alternatively, strike the while "cannot cause severe performance issues". brgds, H-P
Re: RFC: Introduce -fhardened to enable security-related flags
Am Freitag, dem 15.09.2023 um 11:11 -0400 schrieb Marek Polacek: > On Wed, Aug 30, 2023 at 10:46:14AM +0200, Martin Uecker wrote: > > > Improving the security of software has been a major trend in the recent > > > years. Fortunately, GCC offers a wide variety of flags that enable extra > > > hardening. These flags aren't enabled by default, though. And since > > > there are a lot of hardening flags, with more to come, it's been difficult > > > to keep on top of them; more so for the users of GCC who ought not to be > > > expected to keep track of all the new options. > > > > > > To alleviate some of the problems I mentioned, we thought it would > > > be useful to provide a new umbrella option that enables a reasonable set > > > of hardening flags. What's "reasonable" in this context is not easy to > > > pin down. Surely, there must be no ABI impact, the option cannot cause > > > severe performance issues, and, I suspect, it should not cause build > > > errors by enabling stricter compile-time errors (such as, -Wimplicit-int, > > > -Wint-conversion). Including a controversial option in -fhardened > > > would likely cause that users would not use -fhardened at all. It's > > > roughly akin to -Wall or -O2 -- those also enable a reasonable set of > > > options, and evolve over time, and are not kept in sync with other > > > compilers. > > > > > > Currently, -fhardened enables: > > > > > > -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) > > > -D_GLIBCXX_ASSERTIONS > > > -ftrivial-auto-var-init=zero > > > -fPIE -pie -Wl,-z,relro,-z,now > > > -fstack-protector-strong > > > -fstack-clash-protection > > > -fcf-protection=full (x86 GNU/Linux only) > > > > > > -fsanitize=undefined is specifically not enabled. -fstrict-flex-arrays is > > > also liable to break a lot of code so I didn't include it. > > > > > > Appended is a proof-of-concept patch. It doesn't implement > > > --help=hardened > > > yet. A fairly crucial point is that -fhardened will not override options > > > that were specified on the command line (before or after -fhardened). For > > > example, > > > > > > -D_FORTIFY_SOURCE=1 -fhardened > > > > > > means that _FORTIFY_SOURCE=1 will be used. Similarly, > > > > > > -fhardened -fstack-protector > > > > > > will not enable -fstack-protector-strong. > > > > > > Thoughts? > > > > I think this is a great idea! Considering that it is difficult to > > decide what shoud be activated and what not and the baseline should > > not cause compile errors, I wonder whether there should be higher > > levels similar to -O1,2,3 ? > > Thanks. I would like to avoid any levels if at all possible; I think > they would be confusing. > > > Although it would be nice to have a one-letter or very short > > option similar to -O2 or -Wall, but maybe this is not possible > > because all short ones are already taken. Of course, > > "-fhardening" would already a huge improvement to the > > current situation. > > There are some free ones, like -Z, but I'm not confident I could take > it :). > It would send a message. Today I can get crazy optimizations with -O3 but for (somewhat) decent security, I need something like: -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) -D_GLIBCXX_ASSERTIONS -ftrivial-auto-var-init=pattern -fPIE -pie -Wl,-z,relro,-z,now -fstack-protector-strong -fstack-clash-protection -fcf-protection=full -fsanitize=undefined -fsanitize-undefined-trap-on-error -Wall -Wextra which also sends a message. Martin
Re: RFC: Introduce -fhardened to enable security-related flags
On Fri, Sep 01, 2023 at 10:09:28PM +, Qing Zhao via Gcc-patches wrote: > > > > On Aug 29, 2023, at 3:42 PM, Marek Polacek via Gcc-patches > > wrote: > > > > Improving the security of software has been a major trend in the recent > > years. Fortunately, GCC offers a wide variety of flags that enable extra > > hardening. These flags aren't enabled by default, though. And since > > there are a lot of hardening flags, with more to come, it's been difficult > > to keep on top of them; more so for the users of GCC who ought not to be > > expected to keep track of all the new options. > > > > To alleviate some of the problems I mentioned, we thought it would > > be useful to provide a new umbrella option that enables a reasonable set > > of hardening flags. What's "reasonable" in this context is not easy to > > pin down. Surely, there must be no ABI impact, the option cannot cause > > severe performance issues, and, I suspect, it should not cause build > > errors by enabling stricter compile-time errors (such as, -Wimplicit-int, > > -Wint-conversion). Including a controversial option in -fhardened > > would likely cause that users would not use -fhardened at all. It's > > roughly akin to -Wall or -O2 -- those also enable a reasonable set of > > options, and evolve over time, and are not kept in sync with other > > compilers. > > > > Currently, -fhardened enables: > > > > -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) > > -D_GLIBCXX_ASSERTIONS > > -ftrivial-auto-var-init=zero > > -fPIE -pie -Wl,-z,relro,-z,now > > -fstack-protector-strong > > -fstack-clash-protection > > -fcf-protection=full (x86 GNU/Linux only) > > > > -fsanitize=undefined is specifically not enabled. -fstrict-flex-arrays is > > also liable to break a lot of code so I didn't include it. > > > > Appended is a proof-of-concept patch. It doesn't implement --help=hardened > > yet. A fairly crucial point is that -fhardened will not override options > > that were specified on the command line (before or after -fhardened). For > > example, > > > > -D_FORTIFY_SOURCE=1 -fhardened > > > > means that _FORTIFY_SOURCE=1 will be used. Similarly, > > > > -fhardened -fstack-protector > > > > will not enable -fstack-protector-strong. > > > > Thoughts? > > In general, I think that it is a very good idea to provide umbrella options > for software security purpose. Thanks a lot for this work! And thank you for taking a look! > 1. I do agree with Martin, multiple-level control for this purpose might be > needed, > similar as multiple levels for warnings, and multiple levels for > optimizations. As I just mentioned in the other email, I'm reluctant to add more levels, like -fhardened=2, at this time. I think it's confusing, because if there are multiple choices, then as a user you have to figure out which one you want and this option is supposed to simplify things, not to puzzle people with one more decision. > Similar as optimization options, can we organize all the security options > together > In our manual, then the user will have a good central place to get more and > complete > Information of the security features our compiler provides? Maybe. But going through all the options and deciding whether it's a security options may be too big a task, especially if we are not exactly sure how we define security. > 2. What’s the major criteria to decide which security feature should go into > this list? > Later, when we have new security features, how to decide whether to add them > to > This list or not? >From my original post: What's "reasonable" in this context is not easy to pin down. Surely, there must be no ABI impact, the option cannot cause severe performance issues, and, I suspect, it should not cause build errors by enabling stricter compile-time errors (such as, -Wimplicit-int, -Wint-conversion). Including a controversial option in -fhardened would likely cause that users would not use -fhardened at all. > I am wondering why -fzero-call-used-regs is not included in the list and also > Why chose -ftrivial-auto-var-init=zero instead of > -ftrivial-auto-var-init=pattern? I can't readily evaluate the effect of -fzero-call-used-regs. But I did change =zero to =pattern. > 3. Looks like currently, -fhardened excludes all compilation-time Warning > options for security purpose, > (For example, -Warray-bounds, --Wstringop-overflow…) Correct. > And also excludes all sanitizer options for security purpose > (-fsanitizer=undifined) Correct. > So, shall we also provide an umbrella option for compilation-time warning > options for security purpose I don't think so... we already have -Wall and -Wextra. > And a umbrella option for sanitizer options (is the -fsanitizer=undefined > this one)? Yes, -fsanitizer=undefined is already an umbrella option. Thanks, Marek
Re: RFC: Introduce -fhardened to enable security-related flags
On Mon, Sep 04, 2023 at 11:40:34PM +0100, Richard Sandiford wrote: > Qing Zhao via Gcc-patches writes: > >> On Aug 29, 2023, at 3:42 PM, Marek Polacek via Gcc-patches > >> wrote: > >> > >> Improving the security of software has been a major trend in the recent > >> years. Fortunately, GCC offers a wide variety of flags that enable extra > >> hardening. These flags aren't enabled by default, though. And since > >> there are a lot of hardening flags, with more to come, it's been difficult > >> to keep on top of them; more so for the users of GCC who ought not to be > >> expected to keep track of all the new options. > >> > >> To alleviate some of the problems I mentioned, we thought it would > >> be useful to provide a new umbrella option that enables a reasonable set > >> of hardening flags. What's "reasonable" in this context is not easy to > >> pin down. Surely, there must be no ABI impact, the option cannot cause > >> severe performance issues, and, I suspect, it should not cause build > >> errors by enabling stricter compile-time errors (such as, -Wimplicit-int, > >> -Wint-conversion). Including a controversial option in -fhardened > >> would likely cause that users would not use -fhardened at all. It's > >> roughly akin to -Wall or -O2 -- those also enable a reasonable set of > >> options, and evolve over time, and are not kept in sync with other > >> compilers. > >> > >> Currently, -fhardened enables: > >> > >> -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) > >> -D_GLIBCXX_ASSERTIONS > >> -ftrivial-auto-var-init=zero > >> -fPIE -pie -Wl,-z,relro,-z,now > >> -fstack-protector-strong > >> -fstack-clash-protection > >> -fcf-protection=full (x86 GNU/Linux only) > >> > >> -fsanitize=undefined is specifically not enabled. -fstrict-flex-arrays is > >> also liable to break a lot of code so I didn't include it. > >> > >> Appended is a proof-of-concept patch. It doesn't implement --help=hardened > >> yet. A fairly crucial point is that -fhardened will not override options > >> that were specified on the command line (before or after -fhardened). For > >> example, > >> > >> -D_FORTIFY_SOURCE=1 -fhardened > >> > >> means that _FORTIFY_SOURCE=1 will be used. Similarly, > >> > >> -fhardened -fstack-protector > >> > >> will not enable -fstack-protector-strong. > >> > >> Thoughts? > > > > In general, I think that it is a very good idea to provide umbrella options > > for software security purpose. Thanks a lot for this work! > > > > 1. I do agree with Martin, multiple-level control for this purpose might be > > needed, > > similar as multiple levels for warnings, and multiple levels for > > optimizations. > > > > Similar as optimization options, can we organize all the security options > > together > > In our manual, then the user will have a good central place to get more and > > complete > > Information of the security features our compiler provides? > > > > 2. What’s the major criteria to decide which security feature should go > > into this list? > > Later, when we have new security features, how to decide whether to add > > them to > > This list or not? > > I am wondering why -fzero-call-used-regs is not included in the list and > > also > > FWIW, I wondered the same thing. Not a strong conviction that it should > be included -- maybe the code bloat is too much on some targets. But it > might be acceptable for the -fhardened equivalent of -O3, at least if > restricted to GPRs. I have no opinion here. I trust you so if you think it'd make sense, I will add it. :) The patch I posted today: https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630550.html doesn't include that option yet. > > Why chose -ftrivial-auto-var-init=zero instead of > > -ftrivial-auto-var-init=pattern? > > Yeah, IIRC -ftrivial-auto-var-init=zero was controversial with some > Clang maintainers because it effectively creates a language dialect. > -ftrivial-auto-var-init=pattern wasn't controversial in the same way. Thanks. I've adjusted the patch to enable -ftrivial-auto-var-init=pattern rather than -ftrivial-auto-var-init=zero. Marek
Re: RFC: Introduce -fhardened to enable security-related flags
On Wed, Aug 30, 2023 at 03:08:46PM +0200, Richard Biener wrote: > On Wed, Aug 30, 2023 at 12:51 PM Jakub Jelinek via Gcc-patches > wrote: > > > > On Tue, Aug 29, 2023 at 03:42:27PM -0400, Marek Polacek via Gcc-patches > > wrote: > > > + if (UNLIKELY (flag_hardened) > > > + && (opt->code == OPT_D || opt->code == OPT_U)) > > > + { > > > + if (!fortify_seen_p) > > > + fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15); > > > > Perhaps this should check that the char after it is either '\0' or '=', we > > shouldn't care if user defines or undefines _FORTIFY_SOURCE_WHATEVER macro. > > > > > + if (!cxx_assert_seen_p) > > > + cxx_assert_seen_p = !strcmp (opt->arg, > > > "_GLIBCXX_ASSERTIONS"); > > > > Like we don't care in this case about -D_GLIBCXX_ASSERTIONS42 > > > > > + } > > > + } > > > + > > > + if (flag_hardened) > > > + { > > > + if (!fortify_seen_p && optimize > 0) > > > + { > > > + if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35) > > > + cpp_define (parse_in, "_FORTIFY_SOURCE=3"); > > > + else > > > + cpp_define (parse_in, "_FORTIFY_SOURCE=2"); > > > > I wonder if it wouldn't be better to enable _FORTIFY_SOURCE=2 by default for > > -fhardened only for targets which actually have such a support in the C > > library. There is some poor man's _FORTIFY_SOURCE support in libssp, > > but e.g. one has to link with -lssp in that case and compile with > > -isystem `gcc -print-include-filename=include`/ssp . > > For glibc that is >= 2.3.4, > > https://maskray.me/blog/2022-11-06-fortify-source > > mentions NetBSD support since 2006, newlib since 2017, some Darwin libc, > > bionic (but seems they have only some clang support and dropped GCC > > support) and some third party reimplementation of libssp. > > Or do we just enable it and hope that either it works well or isn't > > supported at all quietly? E.g. it would certainly break the ssp case > > where -isystem finds ssp headers but -lssp isn't linked in. > > > > > @@ -4976,6 +4993,22 @@ process_command (unsigned int > > > decoded_options_count, > > > #endif > > > } > > > > > > + /* TODO: check if -static -pie works and maybe use it. */ > > > + if (flag_hardened && !any_link_options_p && !static_p) > > > +{ > > > + save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false); > > > + /* TODO: check if BIND_NOW/RELRO is supported. */ > > > + if (true) > > > + { > > > + /* These are passed straight down to collect2 so we have to break > > > + it up like this. */ > > > + add_infile ("-z", "*"); > > > + add_infile ("now", "*"); > > > + add_infile ("-z", "*"); > > > + add_infile ("relro", "*"); > > > > As the TODO comment says, to do that we need to check at configure time that > > linker supports -z now and -z relro options. > > > > > @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct > > > gcc_options *opts_set, > > > } > > > > > >/* We initialize opts->x_flag_stack_protect to -1 so that targets > > > - can set a default value. */ > > > + can set a default value. With --enable-default-ssp or -fhardened > > > + the default is -fstack-protector-strong. */ > > >if (opts->x_flag_stack_protect == -1) > > > -opts->x_flag_stack_protect = DEFAULT_FLAG_SSP; > > > +opts->x_flag_stack_protect = (opts->x_flag_hardened > > > + ? SPCT_FLAG_STRONG > > > + : DEFAULT_FLAG_SSP); > > > > This needs to be careful, -fstack-protector isn't supported on all targets > > (e.g. ia64) and we don't want toplev.cc warning: > > /* Targets must be able to place spill slots at lower addresses. If the > > target already uses a soft frame pointer, the transition is trivial. > > */ > > if (!FRAME_GROWS_DOWNWARD && flag_stack_protect) > > { > > warning_at (UNKNOWN_LOCATION, 0, > > "%<-fstack-protector%> not supported for this target"); > > flag_stack_protect = 0; > > } > > to be emitted whenever using -fhardened, it should not be enabled there > > silently (for ia64 Fedora/RHEL gcc actually had a short patch to make it > > work, turn the target into FRAME_GROWS_DOWNWARD one if -fstack-protect* was > > enabled and otherwise keep it !FRAME_GROWS_DOWNWARD). > > I'll note that with selectively enabling parts of -fhardening it can > also give a false > sensation of safety when under the hood we ignore half of the option due to > one or another reason ... I suppose you're right :/. > How does -fhardening reflect into -[gf]record-gcc-switches? Is it at > least possible > to verify the actually enabled bits? In DW_AT_producer it simply shows "-fhardened". It doesn't expand to what it actually enabled. I imagine gen_producer_string could be tweaked to print the options enabled by -fhardened.
Re: RFC: Introduce -fhardened to enable security-related flags
On Wed, Aug 30, 2023 at 12:50:40PM +0200, Jakub Jelinek wrote: > On Tue, Aug 29, 2023 at 03:42:27PM -0400, Marek Polacek via Gcc-patches wrote: > > + if (UNLIKELY (flag_hardened) > > + && (opt->code == OPT_D || opt->code == OPT_U)) > > + { > > + if (!fortify_seen_p) > > + fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15); > > Perhaps this should check that the char after it is either '\0' or '=', we > shouldn't care if user defines or undefines _FORTIFY_SOURCE_WHATEVER macro. Right, should be fixed in https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630550.html > > + if (!cxx_assert_seen_p) > > + cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS"); > > Like we don't care in this case about -D_GLIBCXX_ASSERTIONS42 This too. > > + } > > + } > > + > > + if (flag_hardened) > > + { > > + if (!fortify_seen_p && optimize > 0) > > + { > > + if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35) > > + cpp_define (parse_in, "_FORTIFY_SOURCE=3"); > > + else > > + cpp_define (parse_in, "_FORTIFY_SOURCE=2"); > > I wonder if it wouldn't be better to enable _FORTIFY_SOURCE=2 by default for > -fhardened only for targets which actually have such a support in the C > library. There is some poor man's _FORTIFY_SOURCE support in libssp, > but e.g. one has to link with -lssp in that case and compile with > -isystem `gcc -print-include-filename=include`/ssp . > For glibc that is >= 2.3.4, https://maskray.me/blog/2022-11-06-fortify-source > mentions NetBSD support since 2006, newlib since 2017, some Darwin libc, > bionic (but seems they have only some clang support and dropped GCC > support) and some third party reimplementation of libssp. > Or do we just enable it and hope that either it works well or isn't > supported at all quietly? E.g. it would certainly break the ssp case > where -isystem finds ssp headers but -lssp isn't linked in. I don't really have an idea how this should be handled. I left it as it was. > > @@ -4976,6 +4993,22 @@ process_command (unsigned int decoded_options_count, > > #endif > > } > > > > + /* TODO: check if -static -pie works and maybe use it. */ > > + if (flag_hardened && !any_link_options_p && !static_p) > > +{ > > + save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false); > > + /* TODO: check if BIND_NOW/RELRO is supported. */ > > + if (true) > > + { > > + /* These are passed straight down to collect2 so we have to break > > +it up like this. */ > > + add_infile ("-z", "*"); > > + add_infile ("now", "*"); > > + add_infile ("-z", "*"); > > + add_infile ("relro", "*"); > > As the TODO comment says, to do that we need to check at configure time that > linker supports -z now and -z relro options. I've added configure checks in https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630550.html > > @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct > > gcc_options *opts_set, > > } > > > >/* We initialize opts->x_flag_stack_protect to -1 so that targets > > - can set a default value. */ > > + can set a default value. With --enable-default-ssp or -fhardened > > + the default is -fstack-protector-strong. */ > >if (opts->x_flag_stack_protect == -1) > > -opts->x_flag_stack_protect = DEFAULT_FLAG_SSP; > > +opts->x_flag_stack_protect = (opts->x_flag_hardened > > + ? SPCT_FLAG_STRONG > > + : DEFAULT_FLAG_SSP); > > This needs to be careful, -fstack-protector isn't supported on all targets > (e.g. ia64) and we don't want toplev.cc warning: > /* Targets must be able to place spill slots at lower addresses. If the > target already uses a soft frame pointer, the transition is trivial. */ > if (!FRAME_GROWS_DOWNWARD && flag_stack_protect) > { > warning_at (UNKNOWN_LOCATION, 0, > "%<-fstack-protector%> not supported for this target"); > flag_stack_protect = 0; > } > to be emitted whenever using -fhardened, it should not be enabled there > silently (for ia64 Fedora/RHEL gcc actually had a short patch to make it > work, turn the target into FRAME_GROWS_DOWNWARD one if -fstack-protect* was > enabled and otherwise keep it !FRAME_GROWS_DOWNWARD). Unfortunately, I found out that I can't just check FRAME_GROWS_DOWNWARD. Some targets like rs6000 and mips use flag_stack_protect in the definition of FRAME_GROWS_DOWNWARD, and that is not usable in finish_options. There, you have to use opts->x_flag_*. Besides, some targets like BPF don't support -fstack-protector at all, but in finish_options we can't know that yet (AFAIK). I fixed it, but in a very ugly way, using a new global. :/ Thanks, Marek
Re: RFC: Introduce -fhardened to enable security-related flags
On Wed, Aug 30, 2023 at 05:06:57PM +0800, Xi Ruoyao via Gcc-patches wrote: > On Tue, 2023-08-29 at 15:42 -0400, Marek Polacek via Gcc-patches wrote: > > + if (UNLIKELY (flag_hardened) > > + && (opt->code == OPT_D || opt->code == OPT_U)) > > + { > > + if (!fortify_seen_p) > > + fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15); > > + if (!cxx_assert_seen_p) > > + cxx_assert_seen_p = !strcmp (opt->arg, > > "_GLIBCXX_ASSERTIONS"); > > It looks like there is some minor logic issue here: the first strncmp > will mistakenly match "-D_FORTIFY_SOURCE_FAKE", and the second strcmp > will not match "-D_GLIBCXX_ASSERTIONS=1". Thanks, you're right. Should be fixed in https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630550.html Marek
Re: RFC: Introduce -fhardened to enable security-related flags
On Wed, Aug 30, 2023 at 10:46:14AM +0200, Martin Uecker wrote: > > Improving the security of software has been a major trend in the recent > > years. Fortunately, GCC offers a wide variety of flags that enable extra > > hardening. These flags aren't enabled by default, though. And since > > there are a lot of hardening flags, with more to come, it's been difficult > > to keep on top of them; more so for the users of GCC who ought not to be > > expected to keep track of all the new options. > > > > To alleviate some of the problems I mentioned, we thought it would > > be useful to provide a new umbrella option that enables a reasonable set > > of hardening flags. What's "reasonable" in this context is not easy to > > pin down. Surely, there must be no ABI impact, the option cannot cause > > severe performance issues, and, I suspect, it should not cause build > > errors by enabling stricter compile-time errors (such as, -Wimplicit-int, > > -Wint-conversion). Including a controversial option in -fhardened > > would likely cause that users would not use -fhardened at all. It's > > roughly akin to -Wall or -O2 -- those also enable a reasonable set of > > options, and evolve over time, and are not kept in sync with other > > compilers. > > > > Currently, -fhardened enables: > > > > -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) > > -D_GLIBCXX_ASSERTIONS > > -ftrivial-auto-var-init=zero > > -fPIE -pie -Wl,-z,relro,-z,now > > -fstack-protector-strong > > -fstack-clash-protection > > -fcf-protection=full (x86 GNU/Linux only) > > > > -fsanitize=undefined is specifically not enabled. -fstrict-flex-arrays is > > also liable to break a lot of code so I didn't include it. > > > > Appended is a proof-of-concept patch. It doesn't implement --help=hardened > > yet. A fairly crucial point is that -fhardened will not override options > > that were specified on the command line (before or after -fhardened). For > > example, > > > > -D_FORTIFY_SOURCE=1 -fhardened > > > > means that _FORTIFY_SOURCE=1 will be used. Similarly, > > > > -fhardened -fstack-protector > > > > will not enable -fstack-protector-strong. > > > > Thoughts? > > I think this is a great idea! Considering that it is difficult to > decide what shoud be activated and what not and the baseline should > not cause compile errors, I wonder whether there should be higher > levels similar to -O1,2,3 ? Thanks. I would like to avoid any levels if at all possible; I think they would be confusing. > Although it would be nice to have a one-letter or very short > option similar to -O2 or -Wall, but maybe this is not possible > because all short ones are already taken. Of course, > "-fhardening" would already a huge improvement to the > current situation. There are some free ones, like -Z, but I'm not confident I could take it :). Marek
Re: RFC: Introduce -fhardened to enable security-related flags
On Wed, Aug 30, 2023 at 3:42 AM Marek Polacek via Gcc-patches wrote: > > Improving the security of software has been a major trend in the recent > years. Fortunately, GCC offers a wide variety of flags that enable extra > hardening. These flags aren't enabled by default, though. And since > there are a lot of hardening flags, with more to come, it's been difficult > to keep on top of them; more so for the users of GCC who ought not to be > expected to keep track of all the new options. > > To alleviate some of the problems I mentioned, we thought it would > be useful to provide a new umbrella option that enables a reasonable set > of hardening flags. What's "reasonable" in this context is not easy to > pin down. Surely, there must be no ABI impact, the option cannot cause > severe performance issues, and, I suspect, it should not cause build > errors by enabling stricter compile-time errors (such as, -Wimplicit-int, > -Wint-conversion). Including a controversial option in -fhardened > would likely cause that users would not use -fhardened at all. It's > roughly akin to -Wall or -O2 -- those also enable a reasonable set of > options, and evolve over time, and are not kept in sync with other > compilers. > > Currently, -fhardened enables: > > -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) > -D_GLIBCXX_ASSERTIONS > -ftrivial-auto-var-init=zero > -fPIE -pie -Wl,-z,relro,-z,now > -fstack-protector-strong > -fstack-clash-protection > -fcf-protection=full (x86 GNU/Linux only) > > -fsanitize=undefined is specifically not enabled. -fstrict-flex-arrays is > also liable to break a lot of code so I didn't include it. > > Appended is a proof-of-concept patch. It doesn't implement --help=hardened > yet. A fairly crucial point is that -fhardened will not override options > that were specified on the command line (before or after -fhardened). For > example, > > -D_FORTIFY_SOURCE=1 -fhardened > > means that _FORTIFY_SOURCE=1 will be used. Similarly, > > -fhardened -fstack-protector > > will not enable -fstack-protector-strong. > > Thoughts? > > --- > gcc/c-family/c-opts.cc | 25 > gcc/common.opt | 4 +++ > gcc/config/i386/i386-options.cc| 11 ++- > gcc/doc/invoke.texi| 29 +- > gcc/gcc.cc | 35 +- > gcc/opts.cc| 15 -- > gcc/testsuite/c-c++-common/fhardened-1.S | 6 > gcc/testsuite/c-c++-common/fhardened-1.c | 18 +++ > gcc/testsuite/c-c++-common/fhardened-10.c | 10 +++ > gcc/testsuite/c-c++-common/fhardened-2.c | 12 > gcc/testsuite/c-c++-common/fhardened-3.c | 12 > gcc/testsuite/c-c++-common/fhardened-5.c | 11 +++ > gcc/testsuite/c-c++-common/fhardened-6.c | 11 +++ > gcc/testsuite/c-c++-common/fhardened-7.c | 7 + > gcc/testsuite/c-c++-common/fhardened-8.c | 7 + > gcc/testsuite/c-c++-common/fhardened-9.c | 6 > gcc/testsuite/gcc.misc-tests/help.exp | 2 ++ > gcc/testsuite/gcc.target/i386/cf_check-6.c | 12 > gcc/toplev.cc | 6 > 19 files changed, 233 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/fhardened-1.S > create mode 100644 gcc/testsuite/c-c++-common/fhardened-1.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-10.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-2.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-3.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-5.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-6.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-7.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-8.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-9.c > create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-6.c > > diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc > index 4961af63de8..764714ba8a5 100644 > --- a/gcc/c-family/c-opts.cc > +++ b/gcc/c-family/c-opts.cc > @@ -1514,6 +1514,9 @@ c_finish_options (void) >cb_file_change (parse_in, cmd_map); >linemap_line_start (line_table, 0, 1); > > + bool fortify_seen_p = false; > + bool cxx_assert_seen_p = false; > + >/* All command line defines must have the same location. */ >cpp_force_token_locations (parse_in, line_table->highest_line); >for (size_t i = 0; i < deferred_count; i++) > @@ -1531,6 +1534,28 @@ c_finish_options (void) > else > cpp_assert (parse_in, opt->arg); > } > + > + if (UNLIKELY (flag_hardened) > + && (opt->code == OPT_D || opt->code == OPT_U)) > + { > + if (!fortify_seen_p) > + fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15); > + if
Re: RFC: Introduce -fhardened to enable security-related flags
Qing Zhao via Gcc-patches writes: >> On Aug 29, 2023, at 3:42 PM, Marek Polacek via Gcc-patches >> wrote: >> >> Improving the security of software has been a major trend in the recent >> years. Fortunately, GCC offers a wide variety of flags that enable extra >> hardening. These flags aren't enabled by default, though. And since >> there are a lot of hardening flags, with more to come, it's been difficult >> to keep on top of them; more so for the users of GCC who ought not to be >> expected to keep track of all the new options. >> >> To alleviate some of the problems I mentioned, we thought it would >> be useful to provide a new umbrella option that enables a reasonable set >> of hardening flags. What's "reasonable" in this context is not easy to >> pin down. Surely, there must be no ABI impact, the option cannot cause >> severe performance issues, and, I suspect, it should not cause build >> errors by enabling stricter compile-time errors (such as, -Wimplicit-int, >> -Wint-conversion). Including a controversial option in -fhardened >> would likely cause that users would not use -fhardened at all. It's >> roughly akin to -Wall or -O2 -- those also enable a reasonable set of >> options, and evolve over time, and are not kept in sync with other >> compilers. >> >> Currently, -fhardened enables: >> >> -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) >> -D_GLIBCXX_ASSERTIONS >> -ftrivial-auto-var-init=zero >> -fPIE -pie -Wl,-z,relro,-z,now >> -fstack-protector-strong >> -fstack-clash-protection >> -fcf-protection=full (x86 GNU/Linux only) >> >> -fsanitize=undefined is specifically not enabled. -fstrict-flex-arrays is >> also liable to break a lot of code so I didn't include it. >> >> Appended is a proof-of-concept patch. It doesn't implement --help=hardened >> yet. A fairly crucial point is that -fhardened will not override options >> that were specified on the command line (before or after -fhardened). For >> example, >> >> -D_FORTIFY_SOURCE=1 -fhardened >> >> means that _FORTIFY_SOURCE=1 will be used. Similarly, >> >> -fhardened -fstack-protector >> >> will not enable -fstack-protector-strong. >> >> Thoughts? > > In general, I think that it is a very good idea to provide umbrella options > for software security purpose. Thanks a lot for this work! > > 1. I do agree with Martin, multiple-level control for this purpose might be > needed, > similar as multiple levels for warnings, and multiple levels for > optimizations. > > Similar as optimization options, can we organize all the security options > together > In our manual, then the user will have a good central place to get more and > complete > Information of the security features our compiler provides? > > 2. What’s the major criteria to decide which security feature should go into > this list? > Later, when we have new security features, how to decide whether to add them > to > This list or not? > I am wondering why -fzero-call-used-regs is not included in the list and also FWIW, I wondered the same thing. Not a strong conviction that it should be included -- maybe the code bloat is too much on some targets. But it might be acceptable for the -fhardened equivalent of -O3, at least if restricted to GPRs. > Why chose -ftrivial-auto-var-init=zero instead of > -ftrivial-auto-var-init=pattern? Yeah, IIRC -ftrivial-auto-var-init=zero was controversial with some Clang maintainers because it effectively creates a language dialect. -ftrivial-auto-var-init=pattern wasn't controversial in the same way. Thanks, Richard
Re: RFC: Introduce -fhardened to enable security-related flags
> On Aug 29, 2023, at 3:42 PM, Marek Polacek via Gcc-patches > wrote: > > Improving the security of software has been a major trend in the recent > years. Fortunately, GCC offers a wide variety of flags that enable extra > hardening. These flags aren't enabled by default, though. And since > there are a lot of hardening flags, with more to come, it's been difficult > to keep on top of them; more so for the users of GCC who ought not to be > expected to keep track of all the new options. > > To alleviate some of the problems I mentioned, we thought it would > be useful to provide a new umbrella option that enables a reasonable set > of hardening flags. What's "reasonable" in this context is not easy to > pin down. Surely, there must be no ABI impact, the option cannot cause > severe performance issues, and, I suspect, it should not cause build > errors by enabling stricter compile-time errors (such as, -Wimplicit-int, > -Wint-conversion). Including a controversial option in -fhardened > would likely cause that users would not use -fhardened at all. It's > roughly akin to -Wall or -O2 -- those also enable a reasonable set of > options, and evolve over time, and are not kept in sync with other > compilers. > > Currently, -fhardened enables: > > -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) > -D_GLIBCXX_ASSERTIONS > -ftrivial-auto-var-init=zero > -fPIE -pie -Wl,-z,relro,-z,now > -fstack-protector-strong > -fstack-clash-protection > -fcf-protection=full (x86 GNU/Linux only) > > -fsanitize=undefined is specifically not enabled. -fstrict-flex-arrays is > also liable to break a lot of code so I didn't include it. > > Appended is a proof-of-concept patch. It doesn't implement --help=hardened > yet. A fairly crucial point is that -fhardened will not override options > that were specified on the command line (before or after -fhardened). For > example, > > -D_FORTIFY_SOURCE=1 -fhardened > > means that _FORTIFY_SOURCE=1 will be used. Similarly, > > -fhardened -fstack-protector > > will not enable -fstack-protector-strong. > > Thoughts? In general, I think that it is a very good idea to provide umbrella options for software security purpose. Thanks a lot for this work! 1. I do agree with Martin, multiple-level control for this purpose might be needed, similar as multiple levels for warnings, and multiple levels for optimizations. Similar as optimization options, can we organize all the security options together In our manual, then the user will have a good central place to get more and complete Information of the security features our compiler provides? 2. What’s the major criteria to decide which security feature should go into this list? Later, when we have new security features, how to decide whether to add them to This list or not? I am wondering why -fzero-call-used-regs is not included in the list and also Why chose -ftrivial-auto-var-init=zero instead of -ftrivial-auto-var-init=pattern? 3. Looks like currently, -fhardened excludes all compilation-time Warning options for security purpose, (For example, -Warray-bounds, --Wstringop-overflow…) And also excludes all sanitizer options for security purpose (-fsanitizer=undifined) So, shall we also provide an umbrella option for compilation-time warning options for security purpose And a umbrella option for sanitizer options (is the -fsanitizer=undefined this one)? Just some thoughts. -:). Qing
Re: RFC: Introduce -fhardened to enable security-related flags
On Wed, Aug 30, 2023 at 12:51 PM Jakub Jelinek via Gcc-patches wrote: > > On Tue, Aug 29, 2023 at 03:42:27PM -0400, Marek Polacek via Gcc-patches wrote: > > + if (UNLIKELY (flag_hardened) > > + && (opt->code == OPT_D || opt->code == OPT_U)) > > + { > > + if (!fortify_seen_p) > > + fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15); > > Perhaps this should check that the char after it is either '\0' or '=', we > shouldn't care if user defines or undefines _FORTIFY_SOURCE_WHATEVER macro. > > > + if (!cxx_assert_seen_p) > > + cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS"); > > Like we don't care in this case about -D_GLIBCXX_ASSERTIONS42 > > > + } > > + } > > + > > + if (flag_hardened) > > + { > > + if (!fortify_seen_p && optimize > 0) > > + { > > + if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35) > > + cpp_define (parse_in, "_FORTIFY_SOURCE=3"); > > + else > > + cpp_define (parse_in, "_FORTIFY_SOURCE=2"); > > I wonder if it wouldn't be better to enable _FORTIFY_SOURCE=2 by default for > -fhardened only for targets which actually have such a support in the C > library. There is some poor man's _FORTIFY_SOURCE support in libssp, > but e.g. one has to link with -lssp in that case and compile with > -isystem `gcc -print-include-filename=include`/ssp . > For glibc that is >= 2.3.4, https://maskray.me/blog/2022-11-06-fortify-source > mentions NetBSD support since 2006, newlib since 2017, some Darwin libc, > bionic (but seems they have only some clang support and dropped GCC > support) and some third party reimplementation of libssp. > Or do we just enable it and hope that either it works well or isn't > supported at all quietly? E.g. it would certainly break the ssp case > where -isystem finds ssp headers but -lssp isn't linked in. > > > @@ -4976,6 +4993,22 @@ process_command (unsigned int decoded_options_count, > > #endif > > } > > > > + /* TODO: check if -static -pie works and maybe use it. */ > > + if (flag_hardened && !any_link_options_p && !static_p) > > +{ > > + save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false); > > + /* TODO: check if BIND_NOW/RELRO is supported. */ > > + if (true) > > + { > > + /* These are passed straight down to collect2 so we have to break > > + it up like this. */ > > + add_infile ("-z", "*"); > > + add_infile ("now", "*"); > > + add_infile ("-z", "*"); > > + add_infile ("relro", "*"); > > As the TODO comment says, to do that we need to check at configure time that > linker supports -z now and -z relro options. > > > @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct > > gcc_options *opts_set, > > } > > > >/* We initialize opts->x_flag_stack_protect to -1 so that targets > > - can set a default value. */ > > + can set a default value. With --enable-default-ssp or -fhardened > > + the default is -fstack-protector-strong. */ > >if (opts->x_flag_stack_protect == -1) > > -opts->x_flag_stack_protect = DEFAULT_FLAG_SSP; > > +opts->x_flag_stack_protect = (opts->x_flag_hardened > > + ? SPCT_FLAG_STRONG > > + : DEFAULT_FLAG_SSP); > > This needs to be careful, -fstack-protector isn't supported on all targets > (e.g. ia64) and we don't want toplev.cc warning: > /* Targets must be able to place spill slots at lower addresses. If the > target already uses a soft frame pointer, the transition is trivial. */ > if (!FRAME_GROWS_DOWNWARD && flag_stack_protect) > { > warning_at (UNKNOWN_LOCATION, 0, > "%<-fstack-protector%> not supported for this target"); > flag_stack_protect = 0; > } > to be emitted whenever using -fhardened, it should not be enabled there > silently (for ia64 Fedora/RHEL gcc actually had a short patch to make it > work, turn the target into FRAME_GROWS_DOWNWARD one if -fstack-protect* was > enabled and otherwise keep it !FRAME_GROWS_DOWNWARD). I'll note that with selectively enabling parts of -fhardening it can also give a false sensation of safety when under the hood we ignore half of the option due to one or another reason ... How does -fhardening reflect into -[gf]record-gcc-switches? Is it at least possible to verify the actually enabled bits? Richard. > Jakub >
Re: RFC: Introduce -fhardened to enable security-related flags
On Tue, Aug 29, 2023 at 03:42:27PM -0400, Marek Polacek via Gcc-patches wrote: > + if (UNLIKELY (flag_hardened) > + && (opt->code == OPT_D || opt->code == OPT_U)) > + { > + if (!fortify_seen_p) > + fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15); Perhaps this should check that the char after it is either '\0' or '=', we shouldn't care if user defines or undefines _FORTIFY_SOURCE_WHATEVER macro. > + if (!cxx_assert_seen_p) > + cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS"); Like we don't care in this case about -D_GLIBCXX_ASSERTIONS42 > + } > + } > + > + if (flag_hardened) > + { > + if (!fortify_seen_p && optimize > 0) > + { > + if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35) > + cpp_define (parse_in, "_FORTIFY_SOURCE=3"); > + else > + cpp_define (parse_in, "_FORTIFY_SOURCE=2"); I wonder if it wouldn't be better to enable _FORTIFY_SOURCE=2 by default for -fhardened only for targets which actually have such a support in the C library. There is some poor man's _FORTIFY_SOURCE support in libssp, but e.g. one has to link with -lssp in that case and compile with -isystem `gcc -print-include-filename=include`/ssp . For glibc that is >= 2.3.4, https://maskray.me/blog/2022-11-06-fortify-source mentions NetBSD support since 2006, newlib since 2017, some Darwin libc, bionic (but seems they have only some clang support and dropped GCC support) and some third party reimplementation of libssp. Or do we just enable it and hope that either it works well or isn't supported at all quietly? E.g. it would certainly break the ssp case where -isystem finds ssp headers but -lssp isn't linked in. > @@ -4976,6 +4993,22 @@ process_command (unsigned int decoded_options_count, > #endif > } > > + /* TODO: check if -static -pie works and maybe use it. */ > + if (flag_hardened && !any_link_options_p && !static_p) > +{ > + save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false); > + /* TODO: check if BIND_NOW/RELRO is supported. */ > + if (true) > + { > + /* These are passed straight down to collect2 so we have to break > + it up like this. */ > + add_infile ("-z", "*"); > + add_infile ("now", "*"); > + add_infile ("-z", "*"); > + add_infile ("relro", "*"); As the TODO comment says, to do that we need to check at configure time that linker supports -z now and -z relro options. > @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct > gcc_options *opts_set, > } > >/* We initialize opts->x_flag_stack_protect to -1 so that targets > - can set a default value. */ > + can set a default value. With --enable-default-ssp or -fhardened > + the default is -fstack-protector-strong. */ >if (opts->x_flag_stack_protect == -1) > -opts->x_flag_stack_protect = DEFAULT_FLAG_SSP; > +opts->x_flag_stack_protect = (opts->x_flag_hardened > + ? SPCT_FLAG_STRONG > + : DEFAULT_FLAG_SSP); This needs to be careful, -fstack-protector isn't supported on all targets (e.g. ia64) and we don't want toplev.cc warning: /* Targets must be able to place spill slots at lower addresses. If the target already uses a soft frame pointer, the transition is trivial. */ if (!FRAME_GROWS_DOWNWARD && flag_stack_protect) { warning_at (UNKNOWN_LOCATION, 0, "%<-fstack-protector%> not supported for this target"); flag_stack_protect = 0; } to be emitted whenever using -fhardened, it should not be enabled there silently (for ia64 Fedora/RHEL gcc actually had a short patch to make it work, turn the target into FRAME_GROWS_DOWNWARD one if -fstack-protect* was enabled and otherwise keep it !FRAME_GROWS_DOWNWARD). Jakub
Re: RFC: Introduce -fhardened to enable security-related flags
On Tue, 2023-08-29 at 15:42 -0400, Marek Polacek via Gcc-patches wrote: > + if (UNLIKELY (flag_hardened) > + && (opt->code == OPT_D || opt->code == OPT_U)) > + { > + if (!fortify_seen_p) > + fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15); > + if (!cxx_assert_seen_p) > + cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS"); It looks like there is some minor logic issue here: the first strncmp will mistakenly match "-D_FORTIFY_SOURCE_FAKE", and the second strcmp will not match "-D_GLIBCXX_ASSERTIONS=1". > + } -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University
Re: RFC: Introduce -fhardened to enable security-related flags
> Improving the security of software has been a major trend in the recent > years. Fortunately, GCC offers a wide variety of flags that enable extra > hardening. These flags aren't enabled by default, though. And since > there are a lot of hardening flags, with more to come, it's been difficult > to keep on top of them; more so for the users of GCC who ought not to be > expected to keep track of all the new options. > > To alleviate some of the problems I mentioned, we thought it would > be useful to provide a new umbrella option that enables a reasonable set > of hardening flags. What's "reasonable" in this context is not easy to > pin down. Surely, there must be no ABI impact, the option cannot cause > severe performance issues, and, I suspect, it should not cause build > errors by enabling stricter compile-time errors (such as, -Wimplicit-int, > -Wint-conversion). Including a controversial option in -fhardened > would likely cause that users would not use -fhardened at all. It's > roughly akin to -Wall or -O2 -- those also enable a reasonable set of > options, and evolve over time, and are not kept in sync with other > compilers. > > Currently, -fhardened enables: > > -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) > -D_GLIBCXX_ASSERTIONS > -ftrivial-auto-var-init=zero > -fPIE -pie -Wl,-z,relro,-z,now > -fstack-protector-strong > -fstack-clash-protection > -fcf-protection=full (x86 GNU/Linux only) > > -fsanitize=undefined is specifically not enabled. -fstrict-flex-arrays is > also liable to break a lot of code so I didn't include it. > > Appended is a proof-of-concept patch. It doesn't implement --help=hardened > yet. A fairly crucial point is that -fhardened will not override options > that were specified on the command line (before or after -fhardened). For > example, > > -D_FORTIFY_SOURCE=1 -fhardened > > means that _FORTIFY_SOURCE=1 will be used. Similarly, > > -fhardened -fstack-protector > > will not enable -fstack-protector-strong. > > Thoughts? I think this is a great idea! Considering that it is difficult to decide what shoud be activated and what not and the baseline should not cause compile errors, I wonder whether there should be higher levels similar to -O1,2,3 ? Although it would be nice to have a one-letter or very short option similar to -O2 or -Wall, but maybe this is not possible because all short ones are already taken. Of course, "-fhardening" would already a huge improvement to the current situation. Martin
Re: RFC: Introduce -fhardened to enable security-related flags
On Tue, Aug 29, 2023 at 09:11:35PM +0100, Sam James via Gcc-patches wrote: > > Marek Polacek via Gcc-patches writes: > > > Improving the security of software has been a major trend in the recent > > years. Fortunately, GCC offers a wide variety of flags that enable extra > > hardening. These flags aren't enabled by default, though. And since > > there are a lot of hardening flags, with more to come, it's been difficult > > to keep on top of them; more so for the users of GCC who ought not to be > > expected to keep track of all the new options. > > > > To alleviate some of the problems I mentioned, we thought it would > > be useful to provide a new umbrella option that enables a reasonable set > > of hardening flags. What's "reasonable" in this context is not easy to > > pin down. Surely, there must be no ABI impact, the option cannot cause > > severe performance issues, and, I suspect, it should not cause build > > errors by enabling stricter compile-time errors (such as, -Wimplicit-int, > > -Wint-conversion). Including a controversial option in -fhardened > > would likely cause that users would not use -fhardened at all. It's > > roughly akin to -Wall or -O2 -- those also enable a reasonable set of > > options, and evolve over time, and are not kept in sync with other > > compilers. > > > > Currently, -fhardened enables: > > Right now, we patch the compiler in Gentoo to default to these > (some always, some only if the user requests hardening). > > It's a bit invasive (trivial, but just a bit messy) and it gets > pretty tedious to rebase it. Yeah, I bet. > I'd find it really helpful to be able > to instead default on -fhardened from a maintenance perspective. That's good feedback. > > > > -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) > > -D_GLIBCXX_ASSERTIONS > > -ftrivial-auto-var-init=zero > > -fPIE -pie -Wl,-z,relro,-z,now > > -fstack-protector-strong > > -fstack-clash-protection > > -fcf-protection=full (x86 GNU/Linux only) > > > > ... and I also think it's going to be useful for people when > debugging/developing. We can tell them to simply use -fhardened and > then they'll know the compiler will give them the equivalent of -Wall > in terms of sanity checks to help find problems. > > It should be useful for folks who just want to slap it in their CI as > well without keeping up with the various new developments and compiler > features. Yup, pretty much the intended usage. Thanks! Marek
Re: RFC: Introduce -fhardened to enable security-related flags
Marek Polacek via Gcc-patches writes: > Improving the security of software has been a major trend in the recent > years. Fortunately, GCC offers a wide variety of flags that enable extra > hardening. These flags aren't enabled by default, though. And since > there are a lot of hardening flags, with more to come, it's been difficult > to keep on top of them; more so for the users of GCC who ought not to be > expected to keep track of all the new options. > > To alleviate some of the problems I mentioned, we thought it would > be useful to provide a new umbrella option that enables a reasonable set > of hardening flags. What's "reasonable" in this context is not easy to > pin down. Surely, there must be no ABI impact, the option cannot cause > severe performance issues, and, I suspect, it should not cause build > errors by enabling stricter compile-time errors (such as, -Wimplicit-int, > -Wint-conversion). Including a controversial option in -fhardened > would likely cause that users would not use -fhardened at all. It's > roughly akin to -Wall or -O2 -- those also enable a reasonable set of > options, and evolve over time, and are not kept in sync with other > compilers. > > Currently, -fhardened enables: Right now, we patch the compiler in Gentoo to default to these (some always, some only if the user requests hardening). It's a bit invasive (trivial, but just a bit messy) and it gets pretty tedious to rebase it. I'd find it really helpful to be able to instead default on -fhardened from a maintenance perspective. > > -D_FORTIFY_SOURCE=3 (or =2 for older glibcs) > -D_GLIBCXX_ASSERTIONS > -ftrivial-auto-var-init=zero > -fPIE -pie -Wl,-z,relro,-z,now > -fstack-protector-strong > -fstack-clash-protection > -fcf-protection=full (x86 GNU/Linux only) > ... and I also think it's going to be useful for people when debugging/developing. We can tell them to simply use -fhardened and then they'll know the compiler will give them the equivalent of -Wall in terms of sanity checks to help find problems. It should be useful for folks who just want to slap it in their CI as well without keeping up with the various new developments and compiler features. > -fsanitize=undefined is specifically not enabled. -fstrict-flex-arrays is > also liable to break a lot of code so I didn't include it. > > Appended is a proof-of-concept patch. It doesn't implement --help=hardened > yet. A fairly crucial point is that -fhardened will not override options > that were specified on the command line (before or after -fhardened). For > example, > > -D_FORTIFY_SOURCE=1 -fhardened > > means that _FORTIFY_SOURCE=1 will be used. Similarly, > > -fhardened -fstack-protector > > will not enable -fstack-protector-strong. > > Thoughts? > > --- > gcc/c-family/c-opts.cc | 25 > gcc/common.opt | 4 +++ > gcc/config/i386/i386-options.cc| 11 ++- > gcc/doc/invoke.texi| 29 +- > gcc/gcc.cc | 35 +- > gcc/opts.cc| 15 -- > gcc/testsuite/c-c++-common/fhardened-1.S | 6 > gcc/testsuite/c-c++-common/fhardened-1.c | 18 +++ > gcc/testsuite/c-c++-common/fhardened-10.c | 10 +++ > gcc/testsuite/c-c++-common/fhardened-2.c | 12 > gcc/testsuite/c-c++-common/fhardened-3.c | 12 > gcc/testsuite/c-c++-common/fhardened-5.c | 11 +++ > gcc/testsuite/c-c++-common/fhardened-6.c | 11 +++ > gcc/testsuite/c-c++-common/fhardened-7.c | 7 + > gcc/testsuite/c-c++-common/fhardened-8.c | 7 + > gcc/testsuite/c-c++-common/fhardened-9.c | 6 > gcc/testsuite/gcc.misc-tests/help.exp | 2 ++ > gcc/testsuite/gcc.target/i386/cf_check-6.c | 12 > gcc/toplev.cc | 6 > 19 files changed, 233 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/fhardened-1.S > create mode 100644 gcc/testsuite/c-c++-common/fhardened-1.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-10.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-2.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-3.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-5.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-6.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-7.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-8.c > create mode 100644 gcc/testsuite/c-c++-common/fhardened-9.c > create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-6.c > > diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc > index 4961af63de8..764714ba8a5 100644 > --- a/gcc/c-family/c-opts.cc > +++ b/gcc/c-family/c-opts.cc > @@ -1514,6 +1514,9 @@ c_finish_options (void) >cb_file_change (parse_in, cmd_map); >linemap_line_start (line_table,