[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

Branch

cmake_hardening_all

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 5242
Build 8547: Bitcoin ABC Buildbot (legacy)
Build 8546: arc lint + arc unit

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.