[CMAKE] Harden the executables
ClosedPublic
Actions

Authored by Fabien on Mar 13 2019, 13:51.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rSTAGINGa423ecd14cd9: [CMAKE] Harden the executables
rABCa423ecd14cd9: [CMAKE] Harden the executables

Summary

This adds the hardening options to cmake.
This diffs is a squashed version of D2658, D2659, D2660 plus the
hardening options for windows executables.

Depends on D2666

Test Plan

# Linux
mkdir buildcmake && cd buildcmake
cmake -GNinja ..
ninja -v
../contrib/devtools/security-check.py src/bitcoind

The security-check script should output no error.

# Win32
rm -rf *
cmake -GNinja .. -DBUILD_BITCOIN_SEEDER=OFF \
  -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/Win32.cmake
ninja -v
../contrib/devtools/security-check.py src/bitcoind.exe

The security-check script should output no error.

# Win64
rm -rf *
cmake -GNinja .. -DBUILD_BITCOIN_SEEDER=OFF \
  -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/Win64.cmake
ninja -v
../contrib/devtools/security-check.py src/bitcoind.exe

The security-check script should output no error.

OSX build is not working yet with CMake.

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Mar 13 2019, 13:51

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 13 2019, 13:51

Herald added a reviewer: deadalnix. · View Herald Transcript

Herald added a subscriber: schancel. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Fabien edited the test plan for this revision. (Show Details)Mar 13 2019, 13:53

Fabien edited the test plan for this revision. (Show Details)Mar 13 2019, 13:55

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5216: Diff 7698.Mar 13 2019, 14:07

Fabien mentioned this in D2658: [CMAKE] Add stack protection flag.Mar 13 2019, 14:52

Fabien mentioned this in D2660: [CMAKE] Define FORTIFY_SOURCE=2.

Fabien mentioned this in D2659: [CMAKE] Add the linker flags to make the relocated sections read only.

Things have been reordered for no apparent good reason. That just makes the review difficult.

src/CMakeLists.txt
66 ↗	(On Diff #7698)	Why only C++ ?
77 ↗	(On Diff #7698)	You are testing for this twice.
79 ↗	(On Diff #7698)	No. You add a dependency to the library the proper way.

This revision now requires changes to proceed.Mar 14 2019, 02:53

Fabien added inline comments.Mar 14 2019, 12:09

src/CMakeLists.txt
66 ↗	(On Diff #7698)	This is what autotools do, but I agree this is weird. I can include C if you are comfortable with it.

Address feedback

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Fabien added inline comments.Mar 14 2019, 12:14

src/CMakeLists.txt
76 ↗	(On Diff #7701)	I see 3 ways of linking against libssp for all the executables: Just adding `-lssp`, which works but is not very elegant Using `link_libraries` but this will eventually add multiple times the `-lssp` due to dependencies Using `target_link_libraries` on a dependency shared by all executables, which is what is done here.