Enabling hardening options for Wesnoth's executables
Moderator: Forum Moderators
- Pentarctagon
- Project Manager
- Posts: 5533
- Joined: March 22nd, 2009, 10:50 pm
- Location: Earth (occasionally)
Enabling hardening options for Wesnoth's executables
It was mentioned on Discord that it would be beneficial to enable by default the hardening/security features added in PR #3015. To expand a bit on what it does:
- Makes Wesnoth a Position Independent Executable(PIE, info). This allows it to make better use of ASLR, and is done vs the
-fPIE -pie
options. - Protects against stack smashing/buffer overflow attacks. This is done by the
-D_FORTIFY_SOURCE=2
define(info) and the-fstack-protector-strong
option(info, more info). - Uses full RELRO(info) via the options
-Wl,-z,now,-z,relro
.
-fstack-protector-strong
as well as PIE on certain architectures.99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
take one down, patch it around
-2,147,483,648 little bugs in the code
Re: Enabling hardening options for Wesnoth's executables
Absolutely nothing in any halfway sane codebase should be relying on things which these security mitigations block.Pentarctagon wrote: ↑June 11th, 2018, 6:02 am The main potential issues being that I don't believe Wesnoth's unit tests have ever been run with this options and so it is possible they fail and require some additional changes, as well as there being some minimal performance impact from-fstack-protector-strong
as well as PIE on certain architectures.
I'm in favor of enabling the hardening options by default.
- Pentarctagon
- Project Manager
- Posts: 5533
- Joined: March 22nd, 2009, 10:50 pm
- Location: Earth (occasionally)
Re: Enabling hardening options for Wesnoth's executables
Based on the Debian build log, Wesnoth is already built there with
-D_FORTIFY_SOURCE=2
, -fstack-protector-strong
, and -Wl,-z,relro
. The new options from above then would only be -Wl,-z,now
and enabling PIE.99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
take one down, patch it around
-2,147,483,648 little bugs in the code
Re: Enabling hardening options for Wesnoth's executables
Of course, some players build Wesnoth themselves, and there are also other GNU/Linux distributions than Debian.Pentarctagon wrote: ↑June 11th, 2018, 8:46 am Based on the Debian build log, Wesnoth is already built there with-D_FORTIFY_SOURCE=2
,-fstack-protector-strong
, and-Wl,-z,relro
. The new options from above then would only be-Wl,-z,now
and enabling PIE.
Re: Enabling hardening options for Wesnoth's executables
Arch builds with
Edit: these are added by default to every arch package, our own flags will take higher priority.
-D_FORTIFY_SOURCE=2
, -fstack-protector-strong -fno-plt
and with the linker flags -Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now
.Edit: these are added by default to every arch package, our own flags will take higher priority.
Try out the dark board theme.
- Pentarctagon
- Project Manager
- Posts: 5533
- Joined: March 22nd, 2009, 10:50 pm
- Location: Earth (occasionally)
Re: Enabling hardening options for Wesnoth's executables
Right, my point wasn't that it shouldn't be done, but rather that other places already use some of these flags, so the potential for any unexpected issues is smaller. Arch goes a bit further than Debian it looks like, using the fulljyrkive wrote: ↑June 11th, 2018, 9:28 amOf course, some players build Wesnoth themselves, and there are also other GNU/Linux distributions than Debian.Pentarctagon wrote: ↑June 11th, 2018, 8:46 am Based on the Debian build log, Wesnoth is already built there with-D_FORTIFY_SOURCE=2
,-fstack-protector-strong
, and-Wl,-z,relro
. The new options from above then would only be-Wl,-z,now
and enabling PIE.
-Wl,-z,relro,-z,now
, so it looks like the only truly new option would be enabling PIE. That's a bit unexpected, though I suppose it might be because PIE is expected to have a much larger impact on things that are 32-bit still, so it wasn't enabled everywhere by default? Ubuntu as of 17.10 should have it enabled everywhere, though.99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
take one down, patch it around
-2,147,483,648 little bugs in the code
- Pentarctagon
- Project Manager
- Posts: 5533
- Joined: March 22nd, 2009, 10:50 pm
- Location: Earth (occasionally)
Re: Enabling hardening options for Wesnoth's executables
Another question, I suppose, would be whether to enable the Wformat-security option. Wesnoth currently doesn't output any warnings when it's enabled though, and I don't know how useful it would actually be.
99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
take one down, patch it around
-2,147,483,648 little bugs in the code
Re: Enabling hardening options for Wesnoth's executables
We already ensure with Travis CI that build passes even with fullPentarctagon wrote: ↑June 11th, 2018, 10:02 pm Another question, I suppose, would be whether to enable the Wformat-security option. Wesnoth currently doesn't output any warnings when it's enabled though, and I don't know how useful it would actually be.
-Wall -Werror
(with the exception of some warnings we have suppressed, but -Wformat
isn't among them).- Pentarctagon
- Project Manager
- Posts: 5533
- Joined: March 22nd, 2009, 10:50 pm
- Location: Earth (occasionally)
Re: Enabling hardening options for Wesnoth's executables
Alright, in that case, I'll enable hardening by default tomorrow, unless there are any objections raised before then.
99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
take one down, patch it around
-2,147,483,648 little bugs in the code
Re: Enabling hardening options for Wesnoth's executables
How about adding
-Wl,-O1,--sort-common,--as-needed
to the Optimized builds?Try out the dark board theme.
- Pentarctagon
- Project Manager
- Posts: 5533
- Joined: March 22nd, 2009, 10:50 pm
- Location: Earth (occasionally)
Re: Enabling hardening options for Wesnoth's executables
Could you provide more information on what they do?
-Wl,-O1
: It looks like this wouldn't apply to regular executables?--sort-common
: I'm not quite sure what to make of it.--as-needed
: Sounds like an equivalent functionality could be achieved by just not providing unused libraries on the command line?
99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
take one down, patch it around
-2,147,483,648 little bugs in the code
Re: Enabling hardening options for Wesnoth's executables
I'm not having deeper information besides the manpage either.
-Wl,-O1
: I think that would apply for the hardened builds though.--sort-common
: It sounds like it would slightly reduce the size.--as-needed
: Sounds so...
Try out the dark board theme.
- Pentarctagon
- Project Manager
- Posts: 5533
- Joined: March 22nd, 2009, 10:50 pm
- Location: Earth (occasionally)
Re: Enabling hardening options for Wesnoth's executables
The commands:
all produce an executable with the exact same size, so without more information I'm not sure they provide any benefit, at least in Wesnoth's case.
---edit---
To add to that a bit - I'm not opposed to adding additional optimization flags, especially seeing as I've added a fair few myself, but there should definitely be some definable benefit from adding them(ie: adding
Code: Select all
scons wesnoth --option-cache="" --debug=time ctool=gcc cxxtool=g++ ccache=yes build=release nls=no jobs=12 extra_flags_release=""
3bb4a7e5f6154a9589b0102f6e85b872ea8b39ab51625676f44865a215b4e1b5
25962480 bytes
scons wesnoth --option-cache="" --debug=time ctool=gcc cxxtool=g++ ccache=yes build=release nls=no jobs=12 extra_flags_release="-Wl,--sort-common"
3bb4a7e5f6154a9589b0102f6e85b872ea8b39ab51625676f44865a215b4e1b5
25962480 bytes
scons wesnoth --option-cache="" --debug=time ctool=gcc cxxtool=g++ ccache=yes build=release nls=no jobs=12 extra_flags_release="-Wl,--as-needed"
3bb4a7e5f6154a9589b0102f6e85b872ea8b39ab51625676f44865a215b4e1b5
25962480 bytes
scons wesnoth --option-cache="" --debug=time ctool=gcc cxxtool=g++ ccache=yes build=release nls=no jobs=12 extra_flags_release="-Wl,-O1"
2a06a81d9f29c409861384ddffcd6325de507d458b64110d4d47208f7c7df1f3
25962480 bytes
---edit---
To add to that a bit - I'm not opposed to adding additional optimization flags, especially seeing as I've added a fair few myself, but there should definitely be some definable benefit from adding them(ie: adding
-fno-plt
to the hardening flags once Wesnoth's minimum GCC/clang versions support it). So in the cases of --sort-common
and --as-needed
, it seems to me that, at least for Wesnoth, they don't actually do anything. As far as -Wl,-O1
goes, I'm not opposed per se, but I am somewhat suspicious of it both because it produced an executable of the *exact* same size(which seems strange/unlikely to me, even if the hash differed) and because it also seems strange to me that it hasn't become the default behavior of the linker in the ~12 years since it was introduced if it provided noticeable benefits.99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
take one down, patch it around
-2,147,483,648 little bugs in the code
- Pentarctagon
- Project Manager
- Posts: 5533
- Joined: March 22nd, 2009, 10:50 pm
- Location: Earth (occasionally)
Re: Enabling hardening options for Wesnoth's executables
To add some more/summarize:
fno-plt
: GCC 6+, is a minor optimization flag that can be enabled along with full RELRO.-mmitigate-rop
: GCC 6+, is an attempt to try to mitigate Return Oriented Programming. However, even GCC's documentation currently says that this is "limited in what it can do and should not be relied on to provide serious protection". Some additional information is here.-D_GLIBCXX_ASSERTIONS
: Enables bounds checking for strings and containers as well as null pointer checks when dereferencing smart pointers, as per Redhat's recommended flags and the GCC documentation. This does however seem to generate a false positive warning with GCC 7, possible related to this bug.-fstack-clash-protection
: GCC 8+, improved stack protections, also as per the above Redhat recommended flags as well as the GCC documentation. There is the more widely availablefstack-check
option, however it apparently has some issues and although it was brought up for being enabled by default in Arch, this does not seem to have happened.
99 little bugs in the code, 99 little bugs
take one down, patch it around
-2,147,483,648 little bugs in the code
take one down, patch it around
-2,147,483,648 little bugs in the code