test-security-check
script is hard to understand, and change (i.e https://github.com/bitcoin/bitcoin/pull/29987/files#diff-52aa0cda44721f089e53b128cb1232a876006ef257b211655456b17dfb2ec712); tests are also not done in isolation (when-possible). Fix that, and add missing checks. Simplifies future toolchain/security/hardening changes.
contrib: simplify test-security-check
#30423
pull
fanquake
wants to merge
4
commits into
bitcoin:master
from
fanquake:test_test_cleanups
changing
3
files
+73 −119
-
fanquake commented at 4:10 pm on July 10, 2024: memberThe current
-
DrahtBot commented at 4:10 pm on July 10, 2024: contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage
For detailed information about the code coverage, see the test coverage report.
Reviews
See the guideline for information on the review process.
Type Reviewers ACK hebasto, TheCharlatan Concept ACK theuni Approach ACK tdb3 If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #27038 (security-check: test for
_FORTIFY_SOURCE
usage in release binaries by fanquake) - #26950 (cleanse: switch to SecureZeroMemory for Windows cross-compile, check for usage by fanquake)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
- #27038 (security-check: test for
-
fanquake added the label DrahtBot Guix build requested on Jul 10, 2024
-
hebasto commented at 4:40 pm on July 10, 2024: member
tests are also not done in isolation (when-possible).
Mind elaborating this statement please? I mean,
contrib/devtools/test-security-check.py
is supposed to run in the Guix environment. So what kind of “isolation” do you mean? -
DrahtBot commented at 10:59 pm on July 10, 2024: contributor
Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]
-
DrahtBot removed the label DrahtBot Guix build requested on Jul 10, 2024
-
DrahtBot added the label Scripts and tools on Jul 10, 2024
-
fanquake commented at 12:45 pm on July 11, 2024: member
So what kind of “isolation” do you mean?
Testing one thing per test.
-
in contrib/devtools/test-security-check.py:85 in ea096063c3 outdated
94- (1, executable+': failed HIGH_ENTROPY_VA CONTROL_FLOW')) 95- self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE','-fstack-protector-all', '-lssp']), 96- (1, executable+': failed CONTROL_FLOW')) 97- self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full','-fstack-protector-all', '-lssp']), 98- (0, '')) 99+ pass_flags = ['-pie','-fPIE', '-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va', '-fcf-protection=full','-fstack-protector-all', '-lssp']
fanquake commented at 1:47 pm on July 11, 2024:Those look like the wrong docs:
0x86_64-w64-mingw32-ld --help | grep pie 1 -pie, --pic-executable Create a position independent executable 2 -no-pie Create a position dependent executable (default)
hebasto commented at 3:02 pm on July 11, 2024:There seems to be some inconsistency in docs. For instance, in https://manpages.debian.org/bookworm/binutils-mingw-w64-x86-64/x86_64-w64-mingw32-ld.1.en.html:
-pie –pic-executable Create a position independent executable. This is currently only supported on ELF platforms. Position independent executables are similar to shared libraries in that they are relocated by the dynamic linker to the virtual address the OS chooses for them (which can vary between invocations). Like normal dynamically linked executables they can be executed and symbols defined in the executable cannot be overridden by shared libraries. -no-pie Create a position dependent executable. This is the default.
in contrib/devtools/test-security-check.py:88 in ea096063c3 outdated
97- self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE', '-fcf-protection=full','-fstack-protector-all', '-lssp']), 98- (0, '')) 99+ pass_flags = ['-pie','-fPIE', '-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va', '-fcf-protection=full','-fstack-protector-all', '-lssp'] 100+ 101+ self.assertEqual(call_security_check(cc, source, executable, pass_flags + ['-fno-stack-protector']), (1, executable+': failed Canary')) 102+ self.assertEqual(call_security_check(cc, source, executable, pass_flags + ['-Wl,--disable-reloc-section']), (1, executable+': failed RELOC_SECTION'))
hebasto commented at 1:44 pm on July 11, 2024:This check fails:
0FAIL: test_PE (__main__.TestSecurityChecks) 1---------------------------------------------------------------------- 2Traceback (most recent call last): 3 File "/distsrc-base/distsrc-ea096063c384-x86_64-w64-mingw32/./contrib/devtools/test-security-check.py", line 88, in test_PE 4 self.assertEqual(call_security_check(cc, source, executable, pass_flags + ['-Wl,--disable-reloc-section']), (1, executable+': failed RELOC_SECTION')) 5AssertionError: Tuples differ: (1, 'test1.exe: failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA') != (1, 'test1.exe: failed RELOC_SECTION') 6 7First differing element 1: 8'test1.exe: failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA' 9'test1.exe: failed RELOC_SECTION' 10 11- (1, 'test1.exe: failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA') 12+ (1, 'test1.exe: failed RELOC_SECTION') 13 14---------------------------------------------------------------------- 15Ran 1 test in 0.181s 16 17FAILED (failures=1) 18make: *** [Makefile:1348: test-security-check] Error 1
fanquake commented at 2:09 pm on July 11, 2024:This has turned up what looks like a bug in LIEF: https://github.com/lief-project/LIEF/issues/1076. Will re-enable the test later.hebasto commented at 1:44 pm on July 11, 2024: memberConcept ACK.
nit: Spaces after commas and around
+
can used more consistently for better readability.fanquake force-pushed on Jul 11, 2024fanquake force-pushed on Jul 11, 2024fanquake marked this as a draft on Jul 11, 2024tdb3 approvedtdb3 commented at 8:21 pm on July 14, 2024: contributorApproach ACK Nice. Increases readability.
nit: Thought about opportunities for deduplication (e.g. creating a dictionary/list containing test cases with associated parameters/arguments, then looping through it). It’s probably overkill for a file as small as
test-security-check.py
and wouldn’t necessarily enhance readability.fanquake force-pushed on Jul 16, 2024contrib: simplify MACHO test-security-check 6c9746ff92contrib: simplify PE test-security-check 1810e20677contrib: simplify ELF test-security-check 51d8f435c9fanquake force-pushed on Jul 18, 2024contrib: assume binary existence in sec/sym checks
If the binaries don't exist, the Guix build has failed for some other reason. There's no need to check for unknown architectures, or executable formats, as the only ones that could be built are those that we've configured toolchains for in Guix. We've also been doing this inconsistently across the two scripts.
fanquake marked this as ready for review on Jul 19, 2024hebasto approvedDrahtBot requested review from tdb3 on Jul 22, 2024DrahtBot requested review from theuni on Jul 22, 2024fanquake added the label DrahtBot Guix build requested on Jul 22, 2024DrahtBot commented at 11:29 pm on July 22, 2024: contributorGuix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]
DrahtBot removed the label DrahtBot Guix build requested on Jul 22, 2024fanquake commented at 10:34 am on July 23, 2024: memberGuix Build (x86_64):
0222a30ae2094e1e2536e5a1e8794d04f300fbf6ed0f67e0f6dcb4160e3d075a7 guix-build-1bc9f64bee91/output/aarch64-linux-gnu/SHA256SUMS.part 1f7a7b2a6f7e85aa633d6cdb9907bf11cf05c3bdabdf32a4fefaf2bb63264583d guix-build-1bc9f64bee91/output/aarch64-linux-gnu/bitcoin-1bc9f64bee91-aarch64-linux-gnu-debug.tar.gz 28884a25e5a214b224d2c7d73e19786174ddb4eed37c2daa0dc57c23d1f49fc3c guix-build-1bc9f64bee91/output/aarch64-linux-gnu/bitcoin-1bc9f64bee91-aarch64-linux-gnu.tar.gz 336855764cdf7488af17c16524adda0b3c019d08f47e87a8aea5596aade9eba87 guix-build-1bc9f64bee91/output/arm-linux-gnueabihf/SHA256SUMS.part 48bbf0f76f2abe4ee6bdfb1eba9b67410f187afc60433e5be5f7fb11c184660a9 guix-build-1bc9f64bee91/output/arm-linux-gnueabihf/bitcoin-1bc9f64bee91-arm-linux-gnueabihf-debug.tar.gz 5db5ff885a8b728415988456b3f02dd5178124a9ff4ea5f0946f3327038ea33f4 guix-build-1bc9f64bee91/output/arm-linux-gnueabihf/bitcoin-1bc9f64bee91-arm-linux-gnueabihf.tar.gz 6314ce4cd1ccd1ca089c29a68d9f4d031c9dc5e0cf5f6d9b0234db1d7cdc7d641 guix-build-1bc9f64bee91/output/arm64-apple-darwin/SHA256SUMS.part 761d6eb60cdd3b37abf57b560ceb7fc617f56d8d0ab28605673faa93470e9da7a guix-build-1bc9f64bee91/output/arm64-apple-darwin/bitcoin-1bc9f64bee91-arm64-apple-darwin-unsigned.tar.gz 8bbff8c672e248e2e08ceea8641ca3c58576031afaa801b920b2d6e3483684a6c guix-build-1bc9f64bee91/output/arm64-apple-darwin/bitcoin-1bc9f64bee91-arm64-apple-darwin-unsigned.zip 9863ff8c24b3dffb27d548cd5e80ff9205f6b06bc21a4a8249db26acf1fb1d7e0 guix-build-1bc9f64bee91/output/arm64-apple-darwin/bitcoin-1bc9f64bee91-arm64-apple-darwin.tar.gz 10c441e38346c2276f5d3e35e22b00c2bd31c8410d19b0726cce155ffe1c6649a3 guix-build-1bc9f64bee91/output/dist-archive/bitcoin-1bc9f64bee91.tar.gz 11ac7784cf706551e704103e99ee1a7df79ae13870fa5e3dcd2ea8cbc74a36e775 guix-build-1bc9f64bee91/output/powerpc64-linux-gnu/SHA256SUMS.part 12fb4f88f4f6e6baeb6b14e7dd107b3678bc3174192d42d9844cce7f24ea4ecc30 guix-build-1bc9f64bee91/output/powerpc64-linux-gnu/bitcoin-1bc9f64bee91-powerpc64-linux-gnu-debug.tar.gz 13b86ca80ab5110228bb1c32641a75a5fbfb77454583a5cff6695f3335fc089b1e guix-build-1bc9f64bee91/output/powerpc64-linux-gnu/bitcoin-1bc9f64bee91-powerpc64-linux-gnu.tar.gz 140a345fe896f8bf91ed5092799a6512e3baa1f934b663cbe43ddf2e70333c0b76 guix-build-1bc9f64bee91/output/riscv64-linux-gnu/SHA256SUMS.part 1514d06d3b49bfc8d94aa548799b7f886efba1253a5e13f3179cecfbc2a4531e0d guix-build-1bc9f64bee91/output/riscv64-linux-gnu/bitcoin-1bc9f64bee91-riscv64-linux-gnu-debug.tar.gz 16bca5ca1bbc303fa078567e284e3e6983fc2e7ab9c6ca24cc200185acb608b845 guix-build-1bc9f64bee91/output/riscv64-linux-gnu/bitcoin-1bc9f64bee91-riscv64-linux-gnu.tar.gz 17b7b916e97a5a9a24f83938c3dc06e8ef6541e3112f6becb372d4a0430221c4da guix-build-1bc9f64bee91/output/x86_64-apple-darwin/SHA256SUMS.part 189f1c2868a1704d60889e8675785d2a02a92b5d32d28e61cc9b9c06e12f44bd40 guix-build-1bc9f64bee91/output/x86_64-apple-darwin/bitcoin-1bc9f64bee91-x86_64-apple-darwin-unsigned.tar.gz 19ca290ebae788bcad2c21882cb7962d40d41878df8ccb6e5ab9cb434aa02ab6e0 guix-build-1bc9f64bee91/output/x86_64-apple-darwin/bitcoin-1bc9f64bee91-x86_64-apple-darwin-unsigned.zip 20323ea529b7646a8d0326e465d64ef36ef9b304a0f8b379d6e61f4c6aaa9479b1 guix-build-1bc9f64bee91/output/x86_64-apple-darwin/bitcoin-1bc9f64bee91-x86_64-apple-darwin.tar.gz 21ab99ee9a4e76f8f81958e0ae5381d422961e1e4ca1e38e88b73a858024d4933b guix-build-1bc9f64bee91/output/x86_64-linux-gnu/SHA256SUMS.part 228e5044e712fd0d674dba95bf3b02f3c1f872c3f0933f15f6fd142aee51072d65 guix-build-1bc9f64bee91/output/x86_64-linux-gnu/bitcoin-1bc9f64bee91-x86_64-linux-gnu-debug.tar.gz 23583e16f677421420cfddc4b591c7fbad7879a154572567fe8ca7d94ea16d4ab6 guix-build-1bc9f64bee91/output/x86_64-linux-gnu/bitcoin-1bc9f64bee91-x86_64-linux-gnu.tar.gz 24e8cea74fcfd0202186659d76c5d3bf2d1b35560bb74d71395b1540b0223ebcf2 guix-build-1bc9f64bee91/output/x86_64-w64-mingw32/SHA256SUMS.part 25f67bbf2cc9c48ae29b0543f62fec5337e711cac8b4dede393aa7d805a4d4c926 guix-build-1bc9f64bee91/output/x86_64-w64-mingw32/bitcoin-1bc9f64bee91-win64-debug.zip 268aa117609c8857365ed69a5e646aa49a1034800bfd8e40436cede8ae3a69b5cd guix-build-1bc9f64bee91/output/x86_64-w64-mingw32/bitcoin-1bc9f64bee91-win64-setup-unsigned.exe 27e26f68b8949c5c5fdea3f513b88f34ddae8f2ff5624383b58ff4041924ac6d66 guix-build-1bc9f64bee91/output/x86_64-w64-mingw32/bitcoin-1bc9f64bee91-win64-unsigned.tar.gz 28a4816c6b72892cfbb7518592b031df3d27937c2097a3a7928dda5872424a78ee guix-build-1bc9f64bee91/output/x86_64-w64-mingw32/bitcoin-1bc9f64bee91-win64.zip
hebasto commented at 4:21 pm on July 23, 2024: memberMy Guix build:
0x86_64 1222a30ae2094e1e2536e5a1e8794d04f300fbf6ed0f67e0f6dcb4160e3d075a7 guix-build-1bc9f64bee91/output/aarch64-linux-gnu/SHA256SUMS.part 2f7a7b2a6f7e85aa633d6cdb9907bf11cf05c3bdabdf32a4fefaf2bb63264583d guix-build-1bc9f64bee91/output/aarch64-linux-gnu/bitcoin-1bc9f64bee91-aarch64-linux-gnu-debug.tar.gz 38884a25e5a214b224d2c7d73e19786174ddb4eed37c2daa0dc57c23d1f49fc3c guix-build-1bc9f64bee91/output/aarch64-linux-gnu/bitcoin-1bc9f64bee91-aarch64-linux-gnu.tar.gz 436855764cdf7488af17c16524adda0b3c019d08f47e87a8aea5596aade9eba87 guix-build-1bc9f64bee91/output/arm-linux-gnueabihf/SHA256SUMS.part 58bbf0f76f2abe4ee6bdfb1eba9b67410f187afc60433e5be5f7fb11c184660a9 guix-build-1bc9f64bee91/output/arm-linux-gnueabihf/bitcoin-1bc9f64bee91-arm-linux-gnueabihf-debug.tar.gz 6db5ff885a8b728415988456b3f02dd5178124a9ff4ea5f0946f3327038ea33f4 guix-build-1bc9f64bee91/output/arm-linux-gnueabihf/bitcoin-1bc9f64bee91-arm-linux-gnueabihf.tar.gz 7314ce4cd1ccd1ca089c29a68d9f4d031c9dc5e0cf5f6d9b0234db1d7cdc7d641 guix-build-1bc9f64bee91/output/arm64-apple-darwin/SHA256SUMS.part 861d6eb60cdd3b37abf57b560ceb7fc617f56d8d0ab28605673faa93470e9da7a guix-build-1bc9f64bee91/output/arm64-apple-darwin/bitcoin-1bc9f64bee91-arm64-apple-darwin-unsigned.tar.gz 9bbff8c672e248e2e08ceea8641ca3c58576031afaa801b920b2d6e3483684a6c guix-build-1bc9f64bee91/output/arm64-apple-darwin/bitcoin-1bc9f64bee91-arm64-apple-darwin-unsigned.zip 10863ff8c24b3dffb27d548cd5e80ff9205f6b06bc21a4a8249db26acf1fb1d7e0 guix-build-1bc9f64bee91/output/arm64-apple-darwin/bitcoin-1bc9f64bee91-arm64-apple-darwin.tar.gz 11c441e38346c2276f5d3e35e22b00c2bd31c8410d19b0726cce155ffe1c6649a3 guix-build-1bc9f64bee91/output/dist-archive/bitcoin-1bc9f64bee91.tar.gz 12ac7784cf706551e704103e99ee1a7df79ae13870fa5e3dcd2ea8cbc74a36e775 guix-build-1bc9f64bee91/output/powerpc64-linux-gnu/SHA256SUMS.part 13fb4f88f4f6e6baeb6b14e7dd107b3678bc3174192d42d9844cce7f24ea4ecc30 guix-build-1bc9f64bee91/output/powerpc64-linux-gnu/bitcoin-1bc9f64bee91-powerpc64-linux-gnu-debug.tar.gz 14b86ca80ab5110228bb1c32641a75a5fbfb77454583a5cff6695f3335fc089b1e guix-build-1bc9f64bee91/output/powerpc64-linux-gnu/bitcoin-1bc9f64bee91-powerpc64-linux-gnu.tar.gz 150a345fe896f8bf91ed5092799a6512e3baa1f934b663cbe43ddf2e70333c0b76 guix-build-1bc9f64bee91/output/riscv64-linux-gnu/SHA256SUMS.part 1614d06d3b49bfc8d94aa548799b7f886efba1253a5e13f3179cecfbc2a4531e0d guix-build-1bc9f64bee91/output/riscv64-linux-gnu/bitcoin-1bc9f64bee91-riscv64-linux-gnu-debug.tar.gz 17bca5ca1bbc303fa078567e284e3e6983fc2e7ab9c6ca24cc200185acb608b845 guix-build-1bc9f64bee91/output/riscv64-linux-gnu/bitcoin-1bc9f64bee91-riscv64-linux-gnu.tar.gz 18b7b916e97a5a9a24f83938c3dc06e8ef6541e3112f6becb372d4a0430221c4da guix-build-1bc9f64bee91/output/x86_64-apple-darwin/SHA256SUMS.part 199f1c2868a1704d60889e8675785d2a02a92b5d32d28e61cc9b9c06e12f44bd40 guix-build-1bc9f64bee91/output/x86_64-apple-darwin/bitcoin-1bc9f64bee91-x86_64-apple-darwin-unsigned.tar.gz 20ca290ebae788bcad2c21882cb7962d40d41878df8ccb6e5ab9cb434aa02ab6e0 guix-build-1bc9f64bee91/output/x86_64-apple-darwin/bitcoin-1bc9f64bee91-x86_64-apple-darwin-unsigned.zip 21323ea529b7646a8d0326e465d64ef36ef9b304a0f8b379d6e61f4c6aaa9479b1 guix-build-1bc9f64bee91/output/x86_64-apple-darwin/bitcoin-1bc9f64bee91-x86_64-apple-darwin.tar.gz 22ab99ee9a4e76f8f81958e0ae5381d422961e1e4ca1e38e88b73a858024d4933b guix-build-1bc9f64bee91/output/x86_64-linux-gnu/SHA256SUMS.part 238e5044e712fd0d674dba95bf3b02f3c1f872c3f0933f15f6fd142aee51072d65 guix-build-1bc9f64bee91/output/x86_64-linux-gnu/bitcoin-1bc9f64bee91-x86_64-linux-gnu-debug.tar.gz 24583e16f677421420cfddc4b591c7fbad7879a154572567fe8ca7d94ea16d4ab6 guix-build-1bc9f64bee91/output/x86_64-linux-gnu/bitcoin-1bc9f64bee91-x86_64-linux-gnu.tar.gz 25e8cea74fcfd0202186659d76c5d3bf2d1b35560bb74d71395b1540b0223ebcf2 guix-build-1bc9f64bee91/output/x86_64-w64-mingw32/SHA256SUMS.part 26f67bbf2cc9c48ae29b0543f62fec5337e711cac8b4dede393aa7d805a4d4c926 guix-build-1bc9f64bee91/output/x86_64-w64-mingw32/bitcoin-1bc9f64bee91-win64-debug.zip 278aa117609c8857365ed69a5e646aa49a1034800bfd8e40436cede8ae3a69b5cd guix-build-1bc9f64bee91/output/x86_64-w64-mingw32/bitcoin-1bc9f64bee91-win64-setup-unsigned.exe 28e26f68b8949c5c5fdea3f513b88f34ddae8f2ff5624383b58ff4041924ac6d66 guix-build-1bc9f64bee91/output/x86_64-w64-mingw32/bitcoin-1bc9f64bee91-win64-unsigned.tar.gz 29a4816c6b72892cfbb7518592b031df3d27937c2097a3a7928dda5872424a78ee guix-build-1bc9f64bee91/output/x86_64-w64-mingw32/bitcoin-1bc9f64bee91-win64.zip
TheCharlatan approvedTheCharlatan commented at 7:09 pm on July 23, 2024: contributorACK 1bc9f64bee919bc46eb061ef8c66f936eb6a8918fanquake merged this on Jul 24, 2024fanquake closed this on Jul 24, 2024
fanquake deleted the branch on Jul 24, 2024
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-21 15:12 UTC
More mirrored repositories can be found on mirror.b10c.me