python
gh-112301: Add fortify source level 3 to default compiler options
gh-112301: Added -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 to default compiler options for all builds
This option adds runtime protections for glibc to abort execution when unsafe behavior is encountered. Here are the GNU docs on the option
This is a very brief writeup that I found useful from Red Hat explaining some benefits
Also the OpenSSF compiler hardening guidance gives a very good description of the option
This is an option that theoretically affects the runtime so pyperformance benchmarks were run. The benchmark for this branch shows little overall impact but does impact some benchmark types:
NOTE: I would recommend looking into the details of the benchmarks in the links
| Tag | Geometric Mean |
|---|---|
| apps | 1.00x slower |
| asyncio | Not Significant |
| math | 1.01x faster |
| regex | 1.03x slower |
| serialize | 1.01x faster |
| startup | 1.00x faster |
| template | 1.00x slower |
| overall | 1.00x faster |
A benchmark was run a few weeks ago with this option that was slightly different:
| Tag | Geometric Mean |
|---|---|
| apps | 1.03x slower |
| asyncio | 1.01x slower |
| math | 1.00x slower |
| regex | 1.02x faster |
| serialize | 1.01x slower |
| startup | 1.01x slower |
| template | 1.01x slower |
| overall | 1.01x slower |
The benchmarks show that overall there is little memory impact and overall very small performance impact but within benchmark types there is some movement . Many compilers use -D_FORTIFY_SOURCE=2 by default. Level 3 adds additional bounds checking. My recommendation and the recommendation of the OpenSSF guidance is to raise to level 3 for this protection but would like to discuss further.
Attn: @mdboom
:robot: New build scheduled with the buildbot fleet by @corona10 for commit 453e34fa3ef4b3f223dd7429617aae89677ffec0 :robot:
If you want to schedule another build, you need to add the :hammer: test-with-buildbots label again.
@nohlson I will take a look at this PR by this weekend, sorry, I am too busy these days.
@nohlson I have 2 questions.
- Is this flag the subset of the overall policy that we want to measure? Consider applying flags for warnings about potential security issues #112301 (comment)
- Do you have a plan to provide some kind of
./configure --disable-openssf-guide?
- This is a subset of the overall policy based on the OpenSSF guidance.
- I do not intend to add an argument to configure to turn off these options, however I would be interested in maybe including the ability to disable the new options that do impact performance benchmarks. This is theoretically one of those options however we didn't see a measurable impact. I could see there being a configure argument that disables the performance impacting options if we were to introduce one that has a measurable impact.
I would prefer the options we introduce for this effort to be always on, and put a lot of consideration into benchmark impacting options we do include, and possibly not enabling them if we deem the impact too much rather than enabling everything and making a new configure argument.
@nohlson
I do not intend to add an argument to configure to turn off these options, however I would be interested in maybe including the ability to disable the new options that do impact performance benchmarks.
I am pretty sure that there will be users who want to disable options that we added for the OpenSSF guide with a single configuration command :) Adding the optional argument will be helpful to them(even if enabling OpenSSF options is more recommended), I will submit the PR in this week.
This change broke RHEL8 buildbot, test_cext and test_cppext fail with:
/usr/include/features.h:393:5: error: #warning _FORTIFY_SOURCE > 2 is treated like 2 on this platform [-Werror=cpp]
# warning _FORTIFY_SOURCE > 2 is treated like 2 on this platform
^~~~~~~
cc1plus: all warnings being treated as errors
I suggest checking for the FORTIFY flag using
-Werrorin configure.
We already do.
We already do.
For other flags yes, but apparently, not for _FORTIFY_SOURCE=3:
# Enable flags that warn and protect for potential security vulnerabilities.
# These flags should be enabled by default for all builds.
AX_CHECK_COMPILE_FLAG([-fstack-protector-strong], [BASECFLAGS="$BASECFLAGS -fstack-protector-strong"], [AC_MSG_WARN([-fstack-protector-strong not supported])], [-Werror])
AX_CHECK_COMPILE_FLAG([-Wtrampolines], [BASECFLAGS="$BASECFLAGS -Wtrampolines"], [AC_MSG_WARN([-Wtrampolines not supported])], [-Werror])
AX_CHECK_COMPILE_FLAG([-D_FORTIFY_SOURCE=3], [BASECFLAGS="$BASECFLAGS -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3"], [AC_MSG_WARN([-D_FORTIFY_SOURCE=3 not supported])])