From 76c090145e9bb64fe4ef6a663723dd0e9028ed10 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 30 Jan 2025 11:20:20 +0000 Subject: [PATCH] guix: remove test-security/symbol-check scripts These scripts are becoming more of nuisance, than a value-add; particularly since we've been building releases using Guix. Adding new (release bin) tests can be harder, because it requires constructing a failing test, which is becoming less easy e.g trying to disable a feature or protection that has been built into the compiler/toolchain by default. In the pre-Guix days, these were valuable to sanity-check the environment, because we were pulling that pre-built from Ubuntu, with little control. At this point, it's less clear what these scripts are (sanity) checking. Note that these also weren't completely ported to CMake (#31698), see also #31715 which contains other fixes that would be needed for these test-tests, to accomodate future changes. --- cmake/module/Maintenance.cmake | 19 --- contrib/devtools/README.md | 4 +- contrib/devtools/test-security-check.py | 130 ------------------ contrib/devtools/test-symbol-check.py | 174 ------------------------ contrib/guix/libexec/build.sh | 2 - 5 files changed, 2 insertions(+), 327 deletions(-) delete mode 100755 contrib/devtools/test-security-check.py delete mode 100755 contrib/devtools/test-symbol-check.py diff --git a/cmake/module/Maintenance.cmake b/cmake/module/Maintenance.cmake index 61251d24397..d53e7d331e2 100644 --- a/cmake/module/Maintenance.cmake +++ b/cmake/module/Maintenance.cmake @@ -23,25 +23,6 @@ function(add_maintenance_targets) return() endif() - if(CMAKE_SYSTEM_NAME STREQUAL "Darwin") - set(exe_format MACHO) - elseif(WIN32) - set(exe_format PE) - else() - set(exe_format ELF) - endif() - - # In CMake, the components of the compiler invocation are separated into distinct variables: - # - CMAKE_CXX_COMPILER: the full path to the compiler binary itself (e.g., /usr/bin/clang++). - # - CMAKE_CXX_COMPILER_ARG1: a string containing initial compiler options (e.g., --target=x86_64-apple-darwin -nostdlibinc). - # By concatenating these variables, we form the complete command line to be passed to a Python script via the CXX environment variable. - string(STRIP "${CMAKE_CXX_COMPILER} ${CMAKE_CXX_COMPILER_ARG1}" cxx_compiler_command) - add_custom_target(test-security-check - COMMAND ${CMAKE_COMMAND} -E env CXX=${cxx_compiler_command} CXXFLAGS=${CMAKE_CXX_FLAGS} LDFLAGS=${CMAKE_EXE_LINKER_FLAGS} ${PYTHON_COMMAND} ${PROJECT_SOURCE_DIR}/contrib/devtools/test-security-check.py TestSecurityChecks.test_${exe_format} - COMMAND ${CMAKE_COMMAND} -E env CXX=${cxx_compiler_command} CXXFLAGS=${CMAKE_CXX_FLAGS} LDFLAGS=${CMAKE_EXE_LINKER_FLAGS} ${PYTHON_COMMAND} ${PROJECT_SOURCE_DIR}/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_${exe_format} - VERBATIM - ) - foreach(target IN ITEMS bitcoind bitcoin-qt bitcoin-cli bitcoin-tx bitcoin-util bitcoin-wallet test_bitcoin bench_bitcoin) if(TARGET ${target}) list(APPEND executables $) diff --git a/contrib/devtools/README.md b/contrib/devtools/README.md index 56eaeef8150..bf0070f2f53 100644 --- a/contrib/devtools/README.md +++ b/contrib/devtools/README.md @@ -115,8 +115,8 @@ example: BUILDDIR=$PWD/build contrib/devtools/gen-bitcoin-conf.sh ``` -security-check.py and test-security-check.py -============================================ +security-check.py +================= Perform basic security checks on a series of executables. diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py deleted file mode 100755 index 99f171608dd..00000000000 --- a/contrib/devtools/test-security-check.py +++ /dev/null @@ -1,130 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2015-2022 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. -''' -Test script for security-check.py -''' -import lief -import os -import subprocess -import unittest - -from utils import determine_wellknown_cmd - -def write_testcode(filename): - with open(filename, 'w', encoding="utf8") as f: - f.write(''' - #include - int main() - { - std::printf("the quick brown fox jumps over the lazy god\\n"); - return 0; - } - ''') - -def clean_files(source, executable): - os.remove(source) - os.remove(executable) - -def env_flags() -> list[str]: - # This should behave the same as AC_TRY_LINK, so arrange well-known flags - # in the same order as autoconf would. - # - # See the definitions for ac_link in autoconf's lib/autoconf/c.m4 file for - # reference. - flags: list[str] = [] - for var in ['CXXFLAGS', 'CPPFLAGS', 'LDFLAGS']: - flags += filter(None, os.environ.get(var, '').split(' ')) - return flags - -def call_security_check(cxx: str, source: str, executable: str, options) -> tuple: - subprocess.run([*cxx,source,'-o',executable] + env_flags() + options, check=True) - p = subprocess.run([os.path.join(os.path.dirname(__file__), 'security-check.py'), executable], stdout=subprocess.PIPE, text=True) - return (p.returncode, p.stdout.rstrip()) - -def get_arch(cxx, source, executable): - subprocess.run([*cxx, source, '-o', executable] + env_flags(), check=True) - binary = lief.parse(executable) - arch = binary.abstract.header.architecture - os.remove(executable) - return arch - -class TestSecurityChecks(unittest.TestCase): - def test_ELF(self): - source = 'test1.cpp' - executable = 'test1' - cxx = determine_wellknown_cmd('CXX', 'g++') - write_testcode(source) - arch = get_arch(cxx, source, executable) - - if arch == lief.ARCHITECTURES.X86: - pass_flags = ['-D_FORTIFY_SOURCE=3', '-Wl,-znoexecstack', '-Wl,-zrelro', '-Wl,-z,now', '-pie', '-fPIE', '-Wl,-z,separate-code', '-fcf-protection=full'] - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-zexecstack']), (1, executable + ': failed NX')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-no-pie','-fno-PIE']), (1, executable + ': failed PIE')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-znorelro']), (1, executable + ': failed RELRO')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-z,noseparate-code']), (1, executable + ': failed SEPARATE_CODE')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-fcf-protection=none']), (1, executable + ': failed CONTROL_FLOW')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-U_FORTIFY_SOURCE']), (1, executable + ': failed FORTIFY')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags), (0, '')) - else: - pass_flags = ['-D_FORTIFY_SOURCE=3', '-Wl,-znoexecstack', '-Wl,-zrelro', '-Wl,-z,now', '-pie', '-fPIE', '-Wl,-z,separate-code'] - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-zexecstack']), (1, executable + ': failed NX')) - # LIEF fails to parse RISC-V with no PIE correctly, and doesn't detect the fortified function, - # so skip this test for RISC-V (for now). See https://github.com/lief-project/LIEF/issues/1082. - if arch != lief.ARCHITECTURES.RISCV: - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-no-pie','-fno-PIE']), (1, executable + ': failed PIE')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-U_FORTIFY_SOURCE']), (1, executable + ': failed FORTIFY')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-znorelro']), (1, executable + ': failed RELRO')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-z,noseparate-code']), (1, executable + ': failed SEPARATE_CODE')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags), (0, '')) - - clean_files(source, executable) - - def test_PE(self): - source = 'test1.cpp' - executable = 'test1.exe' - cxx = determine_wellknown_cmd('CXX', 'x86_64-w64-mingw32-g++') - write_testcode(source) - - pass_flags = ['-Wl,--nxcompat', '-Wl,--enable-reloc-section', '-Wl,--dynamicbase', '-Wl,--high-entropy-va', '-pie', '-fPIE', '-fcf-protection=full', '-fstack-protector-all', '-lssp'] - - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-fno-stack-protector']), (1, executable + ': failed CANARY')) - # https://github.com/lief-project/LIEF/issues/1076 - in future, we could test this individually. - # self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,--disable-reloc-section']), (1, executable + ': failed RELOC_SECTION')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,--disable-nxcompat']), (1, executable + ': failed NX')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,--disable-dynamicbase']), (1, executable + ': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA')) # -pie -fPIE does nothing without --dynamicbase - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,--disable-high-entropy-va']), (1, executable + ': failed HIGH_ENTROPY_VA')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-fcf-protection=none']), (1, executable + ': failed CONTROL_FLOW')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags), (0, '')) - - clean_files(source, executable) - - def test_MACHO(self): - source = 'test1.cpp' - executable = 'test1' - cxx = determine_wellknown_cmd('CXX', 'clang++') - write_testcode(source) - arch = get_arch(cxx, source, executable) - - if arch == lief.ARCHITECTURES.X86: - pass_flags = ['-Wl,-pie', '-fstack-protector-all', '-fcf-protection=full', '-Wl,-fixup_chains'] - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-no_pie', '-Wl,-no_fixup_chains']), (1, executable+': failed FIXUP_CHAINS PIE')) # -fixup_chains is incompatible with -no_pie - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-no_fixup_chains']), (1, executable + ': failed FIXUP_CHAINS')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-fno-stack-protector']), (1, executable + ': failed CANARY')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-flat_namespace']), (1, executable + ': failed NOUNDEFS')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-fcf-protection=none']), (1, executable + ': failed CONTROL_FLOW')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags), (0, '')) - else: - # arm64 darwin doesn't support non-PIE binaries or executable stacks - pass_flags = ['-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti'] - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-mbranch-protection=none']), (1, executable + ': failed BRANCH_PROTECTION')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-no_fixup_chains']), (1, executable + ': failed FIXUP_CHAINS')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-fno-stack-protector']), (1, executable + ': failed CANARY')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-Wl,-flat_namespace']), (1, executable + ': failed NOUNDEFS')) - self.assertEqual(call_security_check(cxx, source, executable, pass_flags), (0, '')) - - clean_files(source, executable) - -if __name__ == '__main__': - unittest.main() diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py deleted file mode 100755 index b4b524dac0c..00000000000 --- a/contrib/devtools/test-symbol-check.py +++ /dev/null @@ -1,174 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2020-2022 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. -''' -Test script for symbol-check.py -''' -import os -import subprocess -import unittest - -from utils import determine_wellknown_cmd - -def call_symbol_check(cxx: list[str], source, executable, options): - # This should behave the same as AC_TRY_LINK, so arrange well-known flags - # in the same order as autoconf would. - # - # See the definitions for ac_link in autoconf's lib/autoconf/c.m4 file for - # reference. - env_flags: list[str] = [] - for var in ['CXXFLAGS', 'CPPFLAGS', 'LDFLAGS']: - env_flags += filter(None, os.environ.get(var, '').split(' ')) - - subprocess.run([*cxx,source,'-o',executable] + env_flags + options, check=True) - p = subprocess.run([os.path.join(os.path.dirname(__file__), 'symbol-check.py'), executable], stdout=subprocess.PIPE, text=True) - os.remove(source) - os.remove(executable) - return (p.returncode, p.stdout.rstrip()) - -class TestSymbolChecks(unittest.TestCase): - def test_ELF(self): - source = 'test1.cpp' - executable = 'test1' - cxx = determine_wellknown_cmd('CXX', 'g++') - - # -lutil is part of the libc6 package so a safe bet that it's installed - # it's also out of context enough that it's unlikely to ever become a real dependency - source = 'test2.cpp' - executable = 'test2' - with open(source, 'w', encoding="utf8") as f: - f.write(''' - #include - - int main() - { - login(0); - return 0; - } - ''') - - self.assertEqual(call_symbol_check(cxx, source, executable, ['-lutil']), - (1, executable + ': libutil.so.1 is not in ALLOWED_LIBRARIES!\n' + - executable + ': failed LIBRARY_DEPENDENCIES')) - - # finally, check a simple conforming binary - source = 'test3.cpp' - executable = 'test3' - with open(source, 'w', encoding="utf8") as f: - f.write(''' - #include - - int main() - { - std::printf("42"); - return 0; - } - ''') - - self.assertEqual(call_symbol_check(cxx, source, executable, []), - (0, '')) - - def test_MACHO(self): - source = 'test1.cpp' - executable = 'test1' - cxx = determine_wellknown_cmd('CXX', 'clang++') - - with open(source, 'w', encoding="utf8") as f: - f.write(''' - #include - - int main() - { - XML_ExpatVersion(); - return 0; - } - - ''') - - self.assertEqual(call_symbol_check(cxx, source, executable, ['-lexpat', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']), - (1, 'libexpat.1.dylib is not in ALLOWED_LIBRARIES!\n' + - f'{executable}: failed DYNAMIC_LIBRARIES MIN_OS SDK')) - - source = 'test2.cpp' - executable = 'test2' - with open(source, 'w', encoding="utf8") as f: - f.write(''' - #include - - int main() - { - CGMainDisplayID(); - return 0; - } - ''') - - self.assertEqual(call_symbol_check(cxx, source, executable, ['-framework', 'CoreGraphics', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']), - (1, f'{executable}: failed MIN_OS SDK')) - - source = 'test3.cpp' - executable = 'test3' - with open(source, 'w', encoding="utf8") as f: - f.write(''' - int main() - { - return 0; - } - ''') - - self.assertEqual(call_symbol_check(cxx, source, executable, ['-Wl,-platform_version','-Wl,macos', '-Wl,13.0', '-Wl,11.4']), - (1, f'{executable}: failed SDK')) - - def test_PE(self): - source = 'test1.cpp' - executable = 'test1.exe' - cxx = determine_wellknown_cmd('CXX', 'x86_64-w64-mingw32-g++') - - with open(source, 'w', encoding="utf8") as f: - f.write(''' - #include - - int main() - { - PdhConnectMachineA(NULL); - return 0; - } - ''') - - self.assertEqual(call_symbol_check(cxx, source, executable, ['-lpdh', '-Wl,--major-subsystem-version', '-Wl,6', '-Wl,--minor-subsystem-version', '-Wl,2']), - (1, 'pdh.dll is not in ALLOWED_LIBRARIES!\n' + - executable + ': failed DYNAMIC_LIBRARIES')) - - source = 'test2.cpp' - executable = 'test2.exe' - - with open(source, 'w', encoding="utf8") as f: - f.write(''' - int main() - { - return 0; - } - ''') - - self.assertEqual(call_symbol_check(cxx, source, executable, ['-Wl,--major-subsystem-version', '-Wl,9', '-Wl,--minor-subsystem-version', '-Wl,9']), - (1, executable + ': failed SUBSYSTEM_VERSION')) - - source = 'test3.cpp' - executable = 'test3.exe' - with open(source, 'w', encoding="utf8") as f: - f.write(''' - #include - - int main() - { - CoFreeUnusedLibrariesEx(0,0); - return 0; - } - ''') - - self.assertEqual(call_symbol_check(cxx, source, executable, ['-lole32', '-Wl,--major-subsystem-version', '-Wl,6', '-Wl,--minor-subsystem-version', '-Wl,2']), - (0, '')) - - -if __name__ == '__main__': - unittest.main() diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index 6c252e78702..ddb8297d9e6 100755 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -247,8 +247,6 @@ mkdir -p "$DISTSRC" # Build Bitcoin Core cmake --build build -j "$JOBS" ${V:+--verbose} - # Check that symbol/security checks tools are sane. - cmake --build build --target test-security-check ${V:+--verbose} # Perform basic security checks on a series of executables. cmake --build build -j 1 --target check-security ${V:+--verbose} # Check that executables only contain allowed version symbols.