Page MenuHomePhabricator

misc updates security-check and symbol-check
ClosedPublic

Authored by PiRK on Jun 1 2023, 10:19.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC2b7e01f50b15: misc updates security-check and symbol-check
Summary

non backport changes:

  • fix identify_executable argument in security-check.py The argument name was wrong, and shown as unused by my IDE. It happenend to work by accident because in the loop where identify_executable is called there is a local variable filename which happened to be the expected thing.

test: use subprocess.run() in test-security-check.py

https://github.com/bitcoin/bitcoin/pull/18434/commits/9fe71a57a6780569e618cf9a8d4f1acf6321017f


scripts: prevent GCC optimising test symbols in test-symbol-check

I noticed in core#22381 that when the test-symbol-check target was being built with Clang and run in the CI it would fail due to using a too-new version of pow (used here). Our CIs use Focal (glibc 2.31) and the version of pow was the optimized version introduced in glibc 2.29:

  • Optimized generic exp, exp2, log, log2, pow, sinf, cosf, sincosf and tanf.

This made sense, except for that if it was failing when built using Clang, why hadn't it also been failing when being built with GCC?

Turns out GCC is optimizing away that call to pow at all optimization levels,

core#22645


build: Use and test PE binutils with --reloc-section

Also fix test-security-check.py to account for new PE PIE failure
indication.

https://github.com/bitcoin/bitcoin/pull/22381/commits/a8127b34bce3597b8091e14057c926197966a234


This fixes TestSecurityChecks.test_PE, I can now make it work on my machine.
contrib/devtools/test-security-check.py TestSecurityChecks.test_PE

This is a backport of core#22645, and a partial backport of core#18434 (I didn't do the CI part, as these tests are still not ready) and core#22381 (other commits are already backported)

Depends on D13966

Test Plan

gitian builds
guix build

contrib\devtools\test-security-check.py
contrib\devtools\test-symbol-check.py

test-security-check.py now works for both test_ELF and test_PE. test_macho would require running the test on a Mac.

test-symbol-check.py still only works for test_ELF. test_PE needs more backports.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 23875
Build 47360: Build Diff
Build 47359: arc lint + arc unit

Event Timeline

PiRK requested review of this revision.Jun 1 2023, 10:19

@bot gitian-osx

Tail of the build log:

 * [new tag]             phabricator/diff/40375 -> phabricator/diff/40375
 * [new tag]             phabricator/diff/40379 -> phabricator/diff/40379
 * [new tag]             phabricator/diff/40380 -> phabricator/diff/40380
 * [new tag]             phabricator/diff/40381 -> phabricator/diff/40381
 * [new tag]             phabricator/diff/40400 -> phabricator/diff/40400
 * [new tag]             phabricator/diff/40401 -> phabricator/diff/40401
 * [new tag]             phabricator/diff/40402 -> phabricator/diff/40402
 * [new tag]             phabricator/diff/40404 -> phabricator/diff/40404
 * [new tag]             phabricator/diff/40405 -> phabricator/diff/40405
 * [new tag]             phabricator/diff/40406 -> phabricator/diff/40406
 * [new tag]             phabricator/diff/40408 -> phabricator/diff/40408
 * [new tag]             phabricator/diff/40420 -> phabricator/diff/40420
 * [new tag]             phabricator/diff/40429 -> phabricator/diff/40429
 * [new tag]             phabricator/diff/40439 -> phabricator/diff/40439
 * [new tag]             phabricator/diff/40441 -> phabricator/diff/40441
 * [new tag]             phabricator/diff/40461 -> phabricator/diff/40461
 * [new tag]             phabricator/diff/40463 -> phabricator/diff/40463
 * [new tag]             phabricator/diff/40469 -> phabricator/diff/40469
 * [new tag]             phabricator/diff/40476 -> phabricator/diff/40476
 * [new tag]             phabricator/diff/40477 -> phabricator/diff/40477
 * [new tag]             phabricator/diff/40478 -> phabricator/diff/40478
 * [new tag]             phabricator/diff/40479 -> phabricator/diff/40479
 * [new tag]             phabricator/diff/40480 -> phabricator/diff/40480
 * [new tag]             phabricator/diff/40487 -> phabricator/diff/40487
 * [new tag]             phabricator/diff/40489 -> phabricator/diff/40489
 * [new tag]             phabricator/diff/40490 -> phabricator/diff/40490
 * [new tag]             phabricator/diff/40495 -> phabricator/diff/40495
 * [new tag]             phabricator/diff/40507 -> phabricator/diff/40507
 * [new tag]             phabricator/diff/40508 -> phabricator/diff/40508
 * [new tag]             phabricator/diff/40509 -> phabricator/diff/40509
 * [new tag]             phabricator/diff/40510 -> phabricator/diff/40510
 * [new tag]             phabricator/diff/40511 -> phabricator/diff/40511
 * [new tag]             phabricator/diff/40512 -> phabricator/diff/40512
 * [new tag]             phabricator/diff/40513 -> phabricator/diff/40513
 * [new tag]             phabricator/diff/40518 -> phabricator/diff/40518
 * [new tag]             phabricator/diff/8992  -> phabricator/diff/8992
 * [new tag]             phabricator/diff/8993  -> phabricator/diff/8993
 * [new branch]          master                 -> master
--- Building for bullseye amd64 ---
Stopping target if it is up
Error response from daemon: No such container: gitian-target
Error: No such container: gitian-target
Making a new image copy
Starting target
Checking if target is up.
Preparing build environment
Updating apt-get repository (log in var/install.log)
Installing additional packages (log in var/install.log)
Upgrading system, may take a while (log in var/install.log)
Creating package manifest
Creating build script (var/build-script)
Running build script (log in var/build.log)
./bin/gbuild:23:in `system!': failed to run on-target setarch x86_64 bash -x < var/build-script > var/build.log 2>&1 (RuntimeError)
	from ./bin/gbuild:185:in `build_one_configuration'
	from ./bin/gbuild:339:in `block (2 levels) in <main>'
	from ./bin/gbuild:334:in `each'
	from ./bin/gbuild:334:in `block in <main>'
	from ./bin/gbuild:332:in `each'
	from ./bin/gbuild:332:in `<main>'
Build gitian-osx failed with exit code 1
PiRK planned changes to this revision.Jun 1 2023, 11:33
PiRK edited the summary of this revision. (Show Details)

rebase onto D13963

Tail of the build log:

 * [new tag]             phabricator/diff/40419 -> phabricator/diff/40419
 * [new tag]             phabricator/diff/40422 -> phabricator/diff/40422
 * [new tag]             phabricator/diff/40423 -> phabricator/diff/40423
 * [new tag]             phabricator/diff/40428 -> phabricator/diff/40428
 * [new tag]             phabricator/diff/40433 -> phabricator/diff/40433
 * [new tag]             phabricator/diff/40447 -> phabricator/diff/40447
 * [new tag]             phabricator/diff/40450 -> phabricator/diff/40450
 * [new tag]             phabricator/diff/40451 -> phabricator/diff/40451
 * [new tag]             phabricator/diff/40461 -> phabricator/diff/40461
 * [new tag]             phabricator/diff/40463 -> phabricator/diff/40463
 * [new tag]             phabricator/diff/40467 -> phabricator/diff/40467
 * [new tag]             phabricator/diff/40469 -> phabricator/diff/40469
 * [new tag]             phabricator/diff/40474 -> phabricator/diff/40474
 * [new tag]             phabricator/diff/40476 -> phabricator/diff/40476
 * [new tag]             phabricator/diff/40477 -> phabricator/diff/40477
 * [new tag]             phabricator/diff/40478 -> phabricator/diff/40478
 * [new tag]             phabricator/diff/40479 -> phabricator/diff/40479
 * [new tag]             phabricator/diff/40480 -> phabricator/diff/40480
 * [new tag]             phabricator/diff/40482 -> phabricator/diff/40482
 * [new tag]             phabricator/diff/40486 -> phabricator/diff/40486
 * [new tag]             phabricator/diff/40489 -> phabricator/diff/40489
 * [new tag]             phabricator/diff/40491 -> phabricator/diff/40491
 * [new tag]             phabricator/diff/40495 -> phabricator/diff/40495
 * [new tag]             phabricator/diff/40507 -> phabricator/diff/40507
 * [new tag]             phabricator/diff/40508 -> phabricator/diff/40508
 * [new tag]             phabricator/diff/40509 -> phabricator/diff/40509
 * [new tag]             phabricator/diff/40510 -> phabricator/diff/40510
 * [new tag]             phabricator/diff/40511 -> phabricator/diff/40511
 * [new tag]             phabricator/diff/40512 -> phabricator/diff/40512
 * [new tag]             phabricator/diff/40513 -> phabricator/diff/40513
 * [new tag]             phabricator/diff/40522 -> phabricator/diff/40522
 * [new tag]             phabricator/diff/40523 -> phabricator/diff/40523
 * [new tag]             phabricator/diff/40524 -> phabricator/diff/40524
 * [new tag]             phabricator/diff/40525 -> phabricator/diff/40525
 * [new tag]             phabricator/diff/40526 -> phabricator/diff/40526
 * [new tag]             phabricator/diff/40527 -> phabricator/diff/40527
 * [new tag]             phabricator/diff/40528 -> phabricator/diff/40528
 * [new tag]             phabricator/diff/8992  -> phabricator/diff/8992
 * [new tag]             phabricator/diff/8993  -> phabricator/diff/8993
 * [new branch]          master                 -> master
--- Building for bullseye amd64 ---
Stopping target if it is up
Making a new image copy
Starting target
Checking if target is up.
Preparing build environment
Updating apt-get repository (log in var/install.log)
Installing additional packages (log in var/install.log)
Upgrading system, may take a while (log in var/install.log)
Creating package manifest
Creating build script (var/build-script)
Running build script (log in var/build.log)
./bin/gbuild:23:in `system!': failed to run on-target setarch x86_64 bash -x < var/build-script > var/build.log 2>&1 (RuntimeError)
	from ./bin/gbuild:185:in `build_one_configuration'
	from ./bin/gbuild:339:in `block (2 levels) in <main>'
	from ./bin/gbuild:334:in `each'
	from ./bin/gbuild:334:in `block in <main>'
	from ./bin/gbuild:332:in `each'
	from ./bin/gbuild:332:in `<main>'
Build gitian-osx failed with exit code 1
PiRK planned changes to this revision.Jun 1 2023, 15:58
PiRK edited the summary of this revision. (Show Details)

Enable control-flow enforcement for hardened builds

@bot gitian-osx gitian-linux gitian-win

PiRK planned changes to this revision.Jun 2 2023, 13:20

The control flow thing is not working on MacOS: https://build.bitcoinabc.org/repository/download/BitcoinABC_BitcoinAbcStaging/568733:id/gitian-osx.tar.gz!/build.log

[1/6] Running security-check.py on bitcoin-cli...
FAILED: src/CMakeFiles/security-check-bitcoin-cli
cd /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src && /usr/bin/python3.9 /home/debian/build/bitcoin/contrib/devtools/security-check.py /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/bitcoin-cli
/home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/bitcoin-cli: failed CONTROL_FLOW
[2/6] Running security-check.py on bitcoin-tx...
FAILED: src/CMakeFiles/security-check-bitcoin-tx
cd /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src && /usr/bin/python3.9 /home/debian/build/bitcoin/contrib/devtools/security-check.py /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/bitcoin-tx
/home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/bitcoin-tx: failed CONTROL_FLOW
[3/6] Running security-check.py on bitcoin-seeder...
FAILED: src/seeder/CMakeFiles/security-check-bitcoin-seeder
cd /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/seeder && /usr/bin/python3.9 /home/debian/build/bitcoin/contrib/devtools/security-check.py /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/seeder/bitcoin-seeder
/home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/seeder/bitcoin-seeder: failed CONTROL_FLOW
[4/6] Running security-check.py on bitcoin-wallet...
FAILED: src/CMakeFiles/security-check-bitcoin-wallet
cd /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src && /usr/bin/python3.9 /home/debian/build/bitcoin/contrib/devtools/security-check.py /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/bitcoin-wallet
/home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/bitcoin-wallet: failed CONTROL_FLOW
[5/6] Running security-check.py on bitcoind...
FAILED: src/CMakeFiles/security-check-bitcoind
cd /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src && /usr/bin/python3.9 /home/debian/build/bitcoin/contrib/devtools/security-check.py /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/bitcoind
/home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/bitcoind: failed CONTROL_FLOW
[6/6] Running security-check.py on bitcoin-qt...
FAILED: src/qt/CMakeFiles/security-check-bitcoin-qt
cd /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/qt && /usr/bin/python3.9 /home/debian/build/bitcoin/contrib/devtools/security-check.py /home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/qt/BitcoinABC-Qt.app/Contents/MacOS/BitcoinABC-Qt
/home/debian/build/bitcoin/distsrc-x86_64-apple-darwin/src/qt/BitcoinABC-Qt.app/Contents/MacOS/BitcoinABC-Qt: failed CONTROL_FLOW
ninja: build stopped: subcommand failed.

Fabien requested changes to this revision.Jun 2 2023, 13:26
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/CMakeLists.txt
191 ↗(On Diff #40542)

This is duplicating the -Wstack-protector flag.
Also my understanding is that the -fcf-protection is only valid for x86_64 darwin, but this will turn it on for all the builds which would not work ?

src/CMakeLists.txt
191 ↗(On Diff #40542)

There are other PRs later that add -fcf-protection for the other architectures. But right now I can't make it work for any of them, so 'ill drop this part of the diff so I can unblock the dependency that fixes symbol check.

PiRK edited the summary of this revision. (Show Details)

drop the control flow backports, it does not work and should be done in a separate diff

@bot gitian-osx gitian-linux gitian-win

This revision is now accepted and ready to land.Jun 2 2023, 16:06
This revision was automatically updated to reflect the committed changes.