Page MenuHomePhabricator

[CMAKE] Harden the executables
ClosedPublic

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

Details

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

deadalnix requested changes to this revision.Mar 14 2019, 02:53

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
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.

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.
deadalnix added inline comments.
src/CMakeLists.txt
76 ↗(On Diff #7701)

Using link libraries is the right thing to do.

This revision is now accepted and ready to land.Mar 18 2019, 17:04

Use link_libraries as suggested in feedback

This revision was automatically updated to reflect the committed changes.