Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 17 Nov 2024 20:11:15 +0100
From:      Alexander Leidinger <Alexander@Leidinger.net>
To:        Dimitry Andric <dim@freebsd.org>
Cc:        Current FreeBSD <freebsd-current@freebsd.org>
Subject:   Re: Playing around with security hardening compiler flags
Message-ID:  <caaa4972dc71dbf6141e1c64d874416d@Leidinger.net>
In-Reply-To: <812A3C4D-35FA-4F98-B279-F550D3296C12@FreeBSD.org>
References:  <01a4b49d43860c30e480ec7cf5bd08f9@Leidinger.net> <812A3C4D-35FA-4F98-B279-F550D3296C12@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)

--=_f63264dc3e6605e398929c29d0ddc7a9
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=UTF-8;
 format=flowed

Am 2024-11-17 19:28, schrieb Dimitry Andric:
> On 17 Nov 2024, at 16:30, Alexander Leidinger <Alexander@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:
>> - -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)
>> 
>> 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
> 
> 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---
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).
---snip---

Compiler Flag 	        Supported since 	Description
-fstrict-flex-arrays=3 	GCC 13.0.0
                         Clang 16.0.0 	Consider trailing array (at the 
end of struct) as flexible array only if declared as []
-fstrict-flex-arrays=2 	GCC 13.0.0
                         Clang 15.0.0 	Consider trailing array as a 
flexible array only if declared as [], or [0]
-fstrict-flex-arrays=1 	GCC 13.0.0
                         Clang 15.0.0 	Consider trailing array as a 
flexible array only if declared as [], [0], or [1]
-fstrict-flex-arrays=0 	GCC 13.0.0
                         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?

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):
---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}
+CFLAGS+=       -D_FORTIFY_SOURCE=${FORTIFY_SOURCE} 
-fstrict-flex-arrays=1
+CXXFLAGS+=     -D_FORTIFY_SOURCE=${FORTIFY_SOURCE} 
-D_GLIBCXX_ASSERTIONS -fstrict-flex-arrays=1
  .endif

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

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.

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

Bye,
Alexander.

-- 
http://www.Leidinger.net Alexander@Leidinger.net: PGP 0x8F31830F9F2772BF
http://www.FreeBSD.org    netchild@FreeBSD.org  : PGP 0x8F31830F9F2772BF

--=_f63264dc3e6605e398929c29d0ddc7a9
Content-Type: application/pgp-signature;
 name=signature.asc
Content-Disposition: attachment;
 filename=signature.asc;
 size=833
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEER9UlYXp1PSd08nWXEg2wmwP42IYFAmc6P+MACgkQEg2wmwP4
2IbuOQ/+NGshnIusUFp0RMcC/GrL/W8oPLMpIuFdy8iFPnJh3ajzia48NrSXFHNa
znejorsWoAKM4i1i/TM+YeMNEML/YLctU0FwWhaPtflfSRlF4yS0+uf3e7mN4r1b
iDSFhejzxhLfppN/AhABANxS1LqCJhJ6m4eXDW7DW2ojVyxDw0BQUMPZs6XEKgdv
12fjaZwGL4gsjD0bUZzUoECQ8qT6JZZv8HxqQzpTCfTrIQB0PYN4MKmjaV6RO+xS
dMGA2haDSV8Wkd3X1lXTjzL8ZIeDUA2hKOTf8N5hOOVunzxOkooxWkE17Eocf7gf
7bOju98f+4iAlLh9bIWOyNRAZHlegn/WTsS4aKPF4TbAMuibYkLAlm1ntcbuo0Ko
gzQpXLcGvJrGy36uAE1kLwcB4gIAQuEQfrHV84KZnsmXj4pVuXM9ctvdcIDSnrV4
1lJL5iGiqZ42eJxyOzQFNMUu5zdhFJ1H+Cd91iPjdNE5yEHvzY8rhzzNnHXIM3iT
BHag/Cq2n5KqNSOPPUBI+N4SVYZpOP2E5h8mZxDlEzQBR6Xgs6IW+d4hQE54P6WC
XgDDTkcWVIpwYTvt8kuXj4ZELizAQoGa1PT7+Tv7o8nXgYCUJGsMJsbiXfuibji+
Gf++cPn9kk3+n0sFl6ToWJzNxE133WnaXlISCdaF6DLZVkLof7Q=
=tXaT
-----END PGP SIGNATURE-----

--=_f63264dc3e6605e398929c29d0ddc7a9--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?caaa4972dc71dbf6141e1c64d874416d>