Re: Playing around with security hardening compiler flags

[Alexander Leidinger]

Am 2024-11-17 19:28, schrieb Dimitry Andric:

On 17 Nov 2024, at 16:30, Alexander Leidinger <alexan...@leidinger.net> wrote:

Hi,

after reading

https://security.googleblog.com/2024/11/retrofitting-spatial-safety-to-hundreds.html

   https://libcxx.llvm.org/Hardening.html

https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html

I played around a bit with some of the flags there (in CFLAGS).

What doesn't work:


What works and results in a world+kernel which is able to boot:
- -D_GLIBCXX_ASSERTIONS
- -fstrict-flex-arrays=1
- -fstack-clash-protection
- -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE

- -fstrict-flex-arrays=3 (variable array issue in IIRC a tool for ath) - -fstrict-flex-arrays=2 (issue in another area, haven't checked further)

FWIW the default hardening mode for libc++ is already extensive. There is also a debug mode, but that is not suitable for general use. I have not yet considered any WITH/WITHOUT options to fiddle with this, since it is an option with 4 possible values: none, fast, extensive, and debug.

Great, personally I don't need more.

_GLIBCXX_ASSERTIONS is a similar directive for libstdc++, so it won't make much difference for the base system, but it could be good for some ports. (Not sure about the overhead though.)

I am unsure about the usefulness of -fstrict-flex-arrays, I have not really played with this option. I would expect more warnings to come out?

From the 3rd link above:
---snip---

[...]



---snip---

Compiler Flag           Supported since         Description
-fstrict-flex-arrays=3  GCC 13.0.0

-fstrict-flex-arrays=2  GCC 13.0.0

-fstrict-flex-arrays=1  GCC 13.0.0

-fstrict-flex-arrays=0  GCC 13.0.0

By default, GCC and Clang treat all trailing arrays (arrays that are placed as the last member or a structure) as flexible-sized arrays, regardless of declared size for the purposes of __builtin_object_size() calculations used by _FORTIFY_SOURCE60. This disables various bounds checks that do not always need to be disabled. In this guide we recommend using the standard C99 flexible array notation [] instead of non-standard [0] or misleading [1], and then using -fstrict-flex-arrays=3 to improve bounds checking in such cases. In this case, code that uses [0] for a flexible array will need to be modified to use [] instead. Code that uses [1] for a flexible arrays needs to be modified to use [] and also extensively modified to eliminate off-by-one errors. Using [1] is not just misleading64, it’s error-prone; beware that existing code using [1] to indicate a flexible array may currently have off-by-one errors65. Once in place, bounds-checking can occur in arrays with fixed declared sizes at the end of a struct. In addition, the source code unambiguously indicates, in a standard way, the cases where a flexible array is in use. There is normally no significant performance trade-off for this option (once any necessary changes have been made). Clang 16.0.0 Consider trailing array (at the end of struct) as flexible array only if declared as [] Clang 15.0.0 Consider trailing array as a flexible array only if declared as [], or [0] Clang 15.0.0 Consider trailing array as a flexible array only if declared as [], [0], or [1] Clang 15.0.0 Consider any trailing array (at the end of a struct) a flexible array (the default) We fail to build with =3 (with IIRC failure to access array[0]) and =2 (with IIRC failure to access array[3]), but the build works with =1. So I expect a few more checks to be enabled than with the default of =0. Ideally we may want to get up to =3.

Last but not least, -fstack-clash-protection might be useful, but I think it might need some additional runtime support? E.g. in libc?

---snip---
diff --git share/mk/bsd.sys.mk share/mk/bsd.sys.mk
index 63774e85716..cc13b5ccc46 100644
--- share/mk/bsd.sys.mk
+++ share/mk/bsd.sys.mk
@@ -304,12 +304,12 @@ CXXFLAGS.clang+=   -Wno-c++11-extensions
 FORTIFY_SOURCE?=       0
 .if ${MK_SSP} != "no"
 # Don't use -Wstack-protector as it breaks world with -Werror.
-SSP_CFLAGS?=   -fstack-protector-strong
+SSP_CFLAGS?=   -fstack-protector-strong -fstack-clash-protection
 CFLAGS+=       ${SSP_CFLAGS}
 .endif # SSP
 .if ${FORTIFY_SOURCE} > 0
-CFLAGS+=       -D_FORTIFY_SOURCE=${FORTIFY_SOURCE}
-CXXFLAGS+=     -D_FORTIFY_SOURCE=${FORTIFY_SOURCE}

 .endif

 # Additional flags passed in CFLAGS and CXXFLAGS when MK_DEBUG_FILES is
---snip---



Any opinions? More discussion here, or rather opening a review for it?

Bye,
Alexander.

--
http://www.Leidinger.net alexan...@leidinger.net: PGP 0x8F31830F9F2772BF
http://www.FreeBSD.org    netch...@freebsd.org  : PGP 0x8F31830F9F2772BF

Just from reading what is written in the 3rd link above about it, it may be more a question if the correct runtime value for our stack gap is compiled in (or the right sysctl is used to query it at runtime during a compiler run), than libc support. I quickly gobbled-up this (tabs are probably mis-converted to spaces during copy&paste of the diff here): +CFLAGS+= -D_FORTIFY_SOURCE=${FORTIFY_SOURCE} -fstrict-flex-arrays=1 +CXXFLAGS+= -D_FORTIFY_SOURCE=${FORTIFY_SOURCE} -D_GLIBCXX_ASSERTIONS -fstrict-flex-arrays=1 As we don't have the gcc libstdc++ in the tree, it may be debatable if it needed to enable those assertions, but given the interest in IIRC hackers@ about libstd++ and libc++ it may not be that faaaaar off.

signature.asc
Description: OpenPGP digital signature