Fortify source should require -O2

[MarkusTeufelberger]

While trying to get clang's static analyzer to run with rippled, I encountered some weird looking errors when building gcc.debug:

#warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]

(Relevant lines in Sconstruct: around line 605)
Apparently, the fortify source switch only makes sense with optimizations turned ON. I already had a -Og switch in there a while ago, but it was patched out later in d26241d since it apparently caused some trouble.

[nbougalis]

I don't think that _FORTIFY_SOURCE requires -O2 as such. In fact, I have been happily compiling with _FORTIFY_SOURCE under gcc for a while and have never seen a complaint.

[MarkusTeufelberger]

glibc introduced this check: https://sourceware.org/git/?p=glibc.git;a=commit;f=include/features.h;h=05c2c9618f583ea4acd69b3fe5ae2a2922dd2ddc. Here's the gcc patch implementing the checks in general (http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html) and here the man page about it (http://man7.org/linux/man-pages/man7/feature_test_macros.7.html)

According to Debian: https://wiki.debian.org/Hardening#DEB_BUILD_HARDENING_FORTIFY_.28gcc.2Fg.2B-.2B-_-D_FORTIFY_SOURCE.3D2.29 and Ubuntu: https://wiki.ubuntu.com/ToolChain/CompilerFlags#A-D_FORTIFY_SOURCE.3D2

The real problem is what Debian patches in glibc (http://metadata.ftp-master.debian.org/changelogs/main/g/glibc/glibc_2.21-4_changelog):

Amongst other things: "[ Matthias Klose ]

• Add patches/any/local-revert-bz13979.diff: revert a commit that made
  attempts to build with FORTIFIED_SOURCE issue warnings if GCC didn't
  have optimisations turned on. This breaks some unclever AC macros."

This way you won't see this warning on debian based distros, it'll probably just fail silently. I assume you've been compiling happily on debian based distributions exclusively? I see this compliant with Arch Linux for example (which rarely modifies upstream code)...

[nbougalis]

Thanks @MarkusTeufelberger: I'll think about how we should tackle this.

[MarkusTeufelberger]

Apparently fixed by no longer fortifying debug builds (e60981f). Let's hope everyone out there runs release builds.

[nbougalis]

@MarkusTeufelberger: we don't fortify anything any more (commit b0704b4 is the version that was merged). What do you mean "Let's hope everyone out there runs release builds"? Do you think some additional changes are needed?

[MarkusTeufelberger]

As far as I understand it, FORTIFY_SOURCE helps with detecting buffer overflows both at compile time and run time. This can protect against some types of buffer overflow attacks, also in compromised running systems (crashing the binary instead of continuing).

The argument that address sanitizer works similarly is not a very good one (already pointed out in the pull request) and it is anyways not enabled by default in release builds (it also didn't build for me at least, see #1932 for the fix).

I personally would like to see as many hardening techniques enabled as possible for release builds. With sanitizers I'm not so sure, since they might fire because of problems in the code, not in the execution of the binary, but ASLR, FORTIFY_SOURCE etc. are defensive tools against attacks that are beyond what's written in your code, not just debugging helpers.

[nbougalis]

I agree that sanitizers aren't targeting the same case, and that using as much source-hardening as possible (or at least, offering the option) is a good idea. We can make it an option for those that build the release version of rippled themselves.