build: Drop option to disable hardening.

davidgumberg commented at 6:57 pm on March 14, 2025: contributor

Follow up to #32038 which dropped NO_HARDEN from depends builds, this PR drops the ENABLE_HARDENING build option since disabling hardening of binaries should not be a supported or maintained use case. With this change, hardening flags are always enabled.

Individual hardening flags and options can still be disabled by appending flags, e.g.:

0cmake -B build \
1  -DAPPEND_CPPFLAGS='-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 -fno-stack-protector -fcf-protection=none -fno-stack-clash-protection' \
2  -DAPPEND_LDFLAGS='-Wl,-z,lazy -Wl,-z,norelro -Wl,-z,noseparate-code'

There is an issue with NetBSD 10.0’s dynamic linker that makes one of the hardening linker flags, -z separate-code, problematic, so this PR also introduces a check to prevent the use of this flag in NetBSD versions < 11.0, (where this issue is fixed). The fix for this might be backported to NetBSD 10.0.

I suggest reviewing the diff with whitespace changes hidden (git diff -w or using github’s hide whitespace option)

DrahtBot commented at 6:57 pm on March 14, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32071.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto, laanwj
Stale ACK	maflcko, vasild

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

DrahtBot added the label Build system on Mar 14, 2025

maflcko commented at 8:44 am on March 15, 2025: member

review ACK ecf2046d4b5c43ddf64f62f09cd3ed70dd5caafb 📮

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK ecf2046d4b5c43ddf64f62f09cd3ed70dd5caafb 📮
3kuLcmyw1Kv99uptN9UVbNG0JwSyCRffWoUy3VFb9NZGP/v7+SpCYNa5PrWtPOGQHROhCkU6Rgf2BpXdp95KKAg==

laanwj approved

laanwj commented at 7:50 am on March 17, 2025: member

Concept and code review ACK ecf2046d4b5c43ddf64f62f09cd3ed70dd5caafb rationale: #32038 (comment) #32038 (comment)

in CMakeLists.txt:484 in ecf2046d4b outdated

487-    try_append_linker_flag("/DYNAMICBASE" TARGET hardening_interface)
488-    try_append_linker_flag("/HIGHENTROPYVA" TARGET hardening_interface)
489-    try_append_linker_flag("/NXCOMPAT" TARGET hardening_interface)
490-  else()
491+add_library(hardening_interface INTERFACE)
492+target_link_libraries(core_interface INTERFACE hardening_interface)

vasild commented at 11:59 am on March 17, 2025:

hardening_interface is not needed anymore? The surrounding code appends stuff directly to core_interface. Maybe remove these 2 lines and s/hardening_interface/core_interface/ all over the place?

davidgumberg commented at 2:59 am on March 18, 2025:

Thanks for catching this, will address in a refactor follow-up, or if I touch here again.

davidgumberg commented at 10:11 pm on March 21, 2025:

Fixed in latest push

vasild approved

vasild commented at 11:59 am on March 17, 2025: contributor

ACK ecf2046d4b5c43ddf64f62f09cd3ed70dd5caafb

hebasto commented at 1:12 pm on March 17, 2025: member

Approach ACK ecf2046d4b5c43ddf64f62f09cd3ed70dd5caafb.

The only case I’m aware of where some hardening flags cause an issue is on NetBSD 10, and this is expected to be fixed in NetBSD 11.

For more details, see:

laanwj commented at 3:01 pm on March 17, 2025: member

The only case I’m aware of where some hardening flags cause an issue is on NetBSD 10, and this is expected to be fixed in NetBSD 11.

Right. Not sure if it’s worth it in this specific case, as it’s going to be fixed upstream soon, but imo in general if some flags (hardening or otherwise) are a problem on some platform this should be taken into account by the build system automatically instead of having the user disable hardening entirely, which is a hassle and creates a bigger risk than necessary.

davidgumberg commented at 3:56 am on March 18, 2025: contributor

The only case I’m aware of where some hardening flags cause an issue is on NetBSD 10, and this is expected to be fixed in NetBSD 11.

in general if some flags (hardening or otherwise) are a problem on some platform this should be taken into account by the build system automatically instead of having the user disable hardening entirely.

Okay, I think this branch will break building on NetBSD, and shouldn’t be merged as-is, I’ll follow up.

fanquake commented at 3:59 am on March 18, 2025: member

I’ll follow up.

Do you want to split out 7d34c19853e7a5528d69c5f30580e7e9712e61f0? That can be merged now.

davidgumberg commented at 4:34 am on March 18, 2025: contributor

Do you want to split out 7d34c19? That can be merged now.

Opened https://github.com/bitcoin/bitcoin/pull/32087

fanquake referenced this in commit ece0b41da6 on Mar 18, 2025

hebasto commented at 9:28 am on March 18, 2025: member

The only case I’m aware of where some hardening flags cause an issue is on NetBSD 10, and this is expected to be fixed in NetBSD 11.

in general if some flags (hardening or otherwise) are a problem on some platform this should be taken into account by the build system automatically instead of having the user disable hardening entirely.

Okay, I think this branch will break building on NetBSD, and shouldn’t be merged as-is, I’ll follow up.

Perhaps I should clarify that only the libbitcoinkernel.so shared library is broken. All executables run fine, except that invoking ldd (or any other operation involving the dynamic linker) fails.

Therefore, I’m fine with dropping NO_HARDEN.

fanquake commented at 7:24 am on March 21, 2025: member

@davidgumberg want to rebase here?

davidgumberg force-pushed on Mar 21, 2025

davidgumberg commented at 10:13 pm on March 21, 2025: contributor

Force-pushed to rebase and address @vasild feedback to drop hardening_interface, and added a check to CMakeLists.txt to avoid -z separate-code on NetBSD < 11.0 (thanks @hebasto). I’ve also updated the PR description.

davidgumberg force-pushed on Mar 21, 2025

in CMakeLists.txt:541 in 0e98ec819b outdated

585+  # have 4 `PT_LOAD` segments.
586+  # Relevant discussions:
587+  # - https://github.com/bitcoin/bitcoin/pull/28724#issuecomment-2589347934
588+  # - https://mail-index.netbsd.org/tech-userlevel/2023/01/05/msg013666.html
589+  if(CMAKE_SYSTEM_NAME STREQUAL "NetBSD" AND CMAKE_SYSTEM_VERSION VERSION_LESS 11.0)
590+      try_append_linker_flag("-Wl,-z,noseparate-code" TARGET core_interface)

hebasto commented at 1:28 pm on March 24, 2025:

style nit: two-spaces indent.

davidgumberg commented at 8:40 pm on March 24, 2025:

Thanks, fixed

hebasto approved

hebasto commented at 1:42 pm on March 24, 2025: member

ACK 0e98ec819b65cfc1728a6aecf5e43a7c73756a66, I have reviewed the code and it looks OK.

Here are NetBSD CI jobs for this branch: https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14036068740.

DrahtBot requested review from vasild on Mar 24, 2025

DrahtBot requested review from maflcko on Mar 24, 2025

DrahtBot requested review from laanwj on Mar 24, 2025

build: Use `-z noseparate-code` on NetBSD < 11.0

This can be dropped once Bitcoin Core no longer supports NetBSD 10.0 or
if upstream fix is backported.

NetBSD's dynamic linker ld.elf_so < 11.0 supports exactly 2 `PT_LOAD`
segments and binaries linked with `-z separate-code` have 4 `PT_LOAD`
segments.

https://github.com/bitcoin/bitcoin/pull/28724#issuecomment-2589347934
https://mail-index.netbsd.org/tech-userlevel/2023/01/05/msg013666.html

f57db75e91

build: Drop option for disabling hardening

Building unhardened executables is not a supported use case that should
be maintained and those that want unhardened executables can still
override them by appending disable flags.

For example:

cmake -B build -DAPPEND_CPPFLAGS='-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 -fno-stack-protector -fcf-protection=none -fno-stack-clash-protection' -DAPPEND_LDFLAGS='-Wl,-z,lazy -Wl,-z,norelro -Wl,-z,noseparate-code'

00ba3ba303

build: refactor: hardening flags -> core_interface 77e553ab6a

davidgumberg force-pushed on Mar 24, 2025

hebasto approved

hebasto commented at 8:46 am on March 25, 2025: member

re-ACK 77e553ab6a0a98d065884a83490f28eec6df0e23.

laanwj approved

laanwj commented at 8:51 am on March 25, 2025: member

re-ACK 77e553ab6a0a98d065884a83490f28eec6df0e23

fanquake commented at 8:08 am on March 27, 2025: member

cc @theuni

build: Drop option to disable hardening. #32071

Code Coverage & Benchmarks

Reviews

Conflicts