From d6ef3543ae16847d5a91fa9271acee9bd2164b32 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 2 Feb 2021 13:35:45 -0500 Subject: [PATCH 1/8] lint: Run mypy with --show-error-codes When using mypy ignore directives, the error code needs to be specified. Somehow mypy doesn't print it by default... --- test/lint/lint-python.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lint/lint-python.sh b/test/lint/lint-python.sh index 51815963f67..c448fa6f9a6 100755 --- a/test/lint/lint-python.sh +++ b/test/lint/lint-python.sh @@ -102,7 +102,7 @@ if ! PYTHONWARNINGS="ignore" flake8 --ignore=B,C,E,F,I,N,W --select=$(IFS=","; e EXIT_CODE=1 fi -if ! mypy --ignore-missing-imports $(git ls-files "test/functional/*.py" "contrib/devtools/*.py"); then +if ! mypy --ignore-missing-imports --show-error-codes $(git ls-files "test/functional/*.py" "contrib/devtools/*.py"); then EXIT_CODE=1 fi From bda62eab38c5dd74e222eddedbca19ace9df6daa Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 7 Jul 2021 14:35:38 +0800 Subject: [PATCH 2/8] ci: skip running the Linux test-security-check target for now The CI environment is a moving target, and these tests are somewhat fragile, so for now, disable them. --- ci/test/00_setup_env_native_multiprocess.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/ci/test/00_setup_env_native_multiprocess.sh b/ci/test/00_setup_env_native_multiprocess.sh index 1418dfbc513..8869b2a0839 100755 --- a/ci/test/00_setup_env_native_multiprocess.sh +++ b/ci/test/00_setup_env_native_multiprocess.sh @@ -13,5 +13,4 @@ export DEP_OPTS="DEBUG=1 MULTIPROCESS=1" export GOAL="install" export BITCOIN_CONFIG="--enable-debug CC=clang CXX=clang++" # Use clang to avoid OOM export TEST_RUNNER_ENV="BITCOIND=bitcoin-node" -export RUN_SECURITY_TESTS="true" export PIP_PACKAGES="lief" From 9fdc8afe117b7b1ea845f8acae9e831922b8f92b Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Thu, 21 Jan 2021 13:52:40 -0500 Subject: [PATCH 3/8] devtools: Improve *-check.py tool detection This is important to make sure that we're not testing tools different from the one we're building with. Introduce determine_wellknown_cmd, which encapsulates how we should handle well-known tools specification (IFS splitting, env override, etc.). --- Makefile.am | 13 +++++++------ contrib/devtools/symbol-check.py | 6 +++--- contrib/devtools/test-security-check.py | 11 ++++++----- contrib/devtools/test-symbol-check.py | 14 ++++++++------ contrib/devtools/utils.py | 22 ++++++++++++++++++++++ src/Makefile.am | 8 ++++---- 6 files changed, 50 insertions(+), 24 deletions(-) create mode 100755 contrib/devtools/utils.py diff --git a/Makefile.am b/Makefile.am index d84dab18427..3ac698b96f3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -58,6 +58,7 @@ DIST_SHARE = \ BIN_CHECKS=$(top_srcdir)/contrib/devtools/symbol-check.py \ $(top_srcdir)/contrib/devtools/security-check.py \ + $(top_srcdir)/contrib/devtools/utils.py \ $(top_srcdir)/contrib/devtools/pixie.py WINDOWS_PACKAGING = $(top_srcdir)/share/pixmaps/bitcoin.ico \ @@ -366,14 +367,14 @@ clean-local: clean-docs test-security-check: if TARGET_DARWIN - $(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_MACHO - $(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_MACHO + $(AM_V_at) CC='$(CC)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_MACHO + $(AM_V_at) CC='$(CC)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_MACHO endif if TARGET_WINDOWS - $(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_PE - $(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_PE + $(AM_V_at) CC='$(CC)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_PE + $(AM_V_at) CC='$(CC)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_PE endif if TARGET_LINUX - $(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_ELF - $(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_ELF + $(AM_V_at) CC='$(CC)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_ELF + $(AM_V_at) CC='$(CC)' CPPFILT='$(CPPFILT)' $(PYTHON) $(top_srcdir)/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_ELF endif diff --git a/contrib/devtools/symbol-check.py b/contrib/devtools/symbol-check.py index 407c0a2d722..61f727fa634 100755 --- a/contrib/devtools/symbol-check.py +++ b/contrib/devtools/symbol-check.py @@ -12,12 +12,13 @@ Example usage: ''' import subprocess import sys -import os from typing import List, Optional import lief import pixie +from utils import determine_wellknown_cmd + # Debian 8 (Jessie) EOL: 2020. https://wiki.debian.org/DebianReleases#Production_Releases # # - g++ version 4.9.2 (https://packages.debian.org/search?suite=jessie&arch=any&searchon=names&keywords=g%2B%2B) @@ -60,7 +61,6 @@ IGNORE_EXPORTS = { '_edata', '_end', '__end__', '_init', '__bss_start', '__bss_start__', '_bss_end__', '__bss_end__', '_fini', '_IO_stdin_used', 'stdin', 'stdout', 'stderr', 'environ', '_environ', '__environ', } -CPPFILT_CMD = os.getenv('CPPFILT', '/usr/bin/c++filt') # Allowed NEEDED libraries ELF_ALLOWED_LIBRARIES = { @@ -140,7 +140,7 @@ class CPPFilt(object): Use a pipe to the 'c++filt' command. ''' def __init__(self): - self.proc = subprocess.Popen(CPPFILT_CMD, stdin=subprocess.PIPE, stdout=subprocess.PIPE, universal_newlines=True) + self.proc = subprocess.Popen(determine_wellknown_cmd('CPPFILT', 'c++filt'), stdin=subprocess.PIPE, stdout=subprocess.PIPE, universal_newlines=True) def __call__(self, mangled): self.proc.stdin.write(mangled + '\n') diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index c079fe5b4d1..73c8732bb6d 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -9,6 +9,8 @@ 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(''' @@ -25,7 +27,7 @@ def clean_files(source, executable): os.remove(executable) def call_security_check(cc, source, executable, options): - subprocess.run([cc,source,'-o',executable] + options, check=True) + subprocess.run([*cc,source,'-o',executable] + options, check=True) p = subprocess.run(['./contrib/devtools/security-check.py',executable], stdout=subprocess.PIPE, universal_newlines=True) return (p.returncode, p.stdout.rstrip()) @@ -33,7 +35,7 @@ class TestSecurityChecks(unittest.TestCase): def test_ELF(self): source = 'test1.c' executable = 'test1' - cc = 'gcc' + cc = determine_wellknown_cmd('CC', 'gcc') write_testcode(source) self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), @@ -54,7 +56,7 @@ class TestSecurityChecks(unittest.TestCase): def test_PE(self): source = 'test1.c' executable = 'test1.exe' - cc = 'x86_64-w64-mingw32-gcc' + cc = determine_wellknown_cmd('CC', 'x86_64-w64-mingw32-gcc') write_testcode(source) self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--no-nxcompat','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va','-no-pie','-fno-PIE']), @@ -73,7 +75,7 @@ class TestSecurityChecks(unittest.TestCase): def test_MACHO(self): source = 'test1.c' executable = 'test1' - cc = 'clang' + cc = determine_wellknown_cmd('CC', 'clang') write_testcode(source) self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace','-Wl,-allow_stack_execute','-fno-stack-protector']), @@ -95,4 +97,3 @@ class TestSecurityChecks(unittest.TestCase): if __name__ == '__main__': unittest.main() - diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py index 6ce2fa35603..56ea02719dd 100755 --- a/contrib/devtools/test-symbol-check.py +++ b/contrib/devtools/test-symbol-check.py @@ -7,10 +7,13 @@ Test script for symbol-check.py ''' import os import subprocess +from typing import List import unittest -def call_symbol_check(cc, source, executable, options): - subprocess.run([cc,source,'-o',executable] + options, check=True) +from utils import determine_wellknown_cmd + +def call_symbol_check(cc: List[str], source, executable, options): + subprocess.run([*cc,source,'-o',executable] + options, check=True) p = subprocess.run(['./contrib/devtools/symbol-check.py',executable], stdout=subprocess.PIPE, universal_newlines=True) os.remove(source) os.remove(executable) @@ -20,7 +23,7 @@ class TestSymbolChecks(unittest.TestCase): def test_ELF(self): source = 'test1.c' executable = 'test1' - cc = 'gcc' + cc = determine_wellknown_cmd('CC', 'gcc') # renameat2 was introduced in GLIBC 2.28, so is newer than the upper limit # of glibc for all platforms @@ -82,7 +85,7 @@ class TestSymbolChecks(unittest.TestCase): def test_MACHO(self): source = 'test1.c' executable = 'test1' - cc = 'clang' + cc = determine_wellknown_cmd('CC', 'clang') with open(source, 'w', encoding="utf8") as f: f.write(''' @@ -132,7 +135,7 @@ class TestSymbolChecks(unittest.TestCase): def test_PE(self): source = 'test1.c' executable = 'test1.exe' - cc = 'x86_64-w64-mingw32-gcc' + cc = determine_wellknown_cmd('CC', 'x86_64-w64-mingw32-gcc') with open(source, 'w', encoding="utf8") as f: f.write(''' @@ -182,4 +185,3 @@ class TestSymbolChecks(unittest.TestCase): if __name__ == '__main__': unittest.main() - diff --git a/contrib/devtools/utils.py b/contrib/devtools/utils.py new file mode 100755 index 00000000000..68ad1c3aba1 --- /dev/null +++ b/contrib/devtools/utils.py @@ -0,0 +1,22 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +''' +Common utility functions +''' +import shutil +import sys +import os +from typing import List + + +def determine_wellknown_cmd(envvar, progname) -> List[str]: + maybe_env = os.getenv(envvar) + maybe_which = shutil.which(progname) + if maybe_env: + return maybe_env.split(' ') # Well-known vars are often meant to be word-split + elif maybe_which: + return [ maybe_which ] + else: + sys.exit(f"{progname} not found") diff --git a/src/Makefile.am b/src/Makefile.am index 9430b778f09..7de5fb36ed4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -814,23 +814,23 @@ clean-local: check-symbols: $(bin_PROGRAMS) if TARGET_DARWIN @echo "Checking macOS dynamic libraries..." - $(AM_V_at) OTOOL=$(OTOOL) $(PYTHON) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) + $(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) endif if TARGET_WINDOWS @echo "Checking Windows dynamic libraries..." - $(AM_V_at) OBJDUMP=$(OBJDUMP) $(PYTHON) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) + $(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) endif if TARGET_LINUX @echo "Checking glibc back compat..." - $(AM_V_at) CPPFILT=$(CPPFILT) $(PYTHON) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) + $(AM_V_at) CPPFILT='$(CPPFILT)' $(PYTHON) $(top_srcdir)/contrib/devtools/symbol-check.py $(bin_PROGRAMS) endif check-security: $(bin_PROGRAMS) if HARDEN @echo "Checking binary security..." - $(AM_V_at) OBJDUMP=$(OBJDUMP) OTOOL=$(OTOOL) $(PYTHON) $(top_srcdir)/contrib/devtools/security-check.py $(bin_PROGRAMS) + $(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/security-check.py $(bin_PROGRAMS) endif libbitcoin_ipc_mpgen_input = \ From 678348db515c770c4dddfac512cdd97be11d407d Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Thu, 10 Dec 2020 20:46:05 -0500 Subject: [PATCH 4/8] guix: Patch binutils to add security-related disable flags We use these flags in our test-security-check make target, but they are only available because debian patches them in. We can patch them in for our Guix builds so that we can check the sanity of our security/symbol checking suite before running them. --- contrib/guix/manifest.scm | 6 +- .../binutils-mingw-w64-disable-flags.patch | 171 ++++++++++++++++++ 2 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 contrib/guix/patches/binutils-mingw-w64-disable-flags.patch diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm index f4df4855fc4..e71cf525332 100644 --- a/contrib/guix/manifest.scm +++ b/contrib/guix/manifest.scm @@ -80,6 +80,10 @@ http://www.linuxfromscratch.org/hlfs/view/development/chapter05/gcc-pass1.html" (("-rpath=") "-rpath-link=")) #t)))))))) +(define (make-binutils-with-mingw-w64-disable-flags xbinutils) + (package-with-extra-patches xbinutils + (search-our-patches "binutils-mingw-w64-disable-flags.patch"))) + (define (make-cross-toolchain target base-gcc-for-libc base-kernel-headers @@ -168,7 +172,7 @@ desirable for building Bitcoin Core release binaries." (define (make-mingw-pthreads-cross-toolchain target) "Create a cross-compilation toolchain package for TARGET" - (let* ((xbinutils (cross-binutils target)) + (let* ((xbinutils (make-binutils-with-mingw-w64-disable-flags (cross-binutils target))) (pthreads-xlibc mingw-w64-x86_64-winpthreads) (pthreads-xgcc (make-gcc-with-pthreads (cross-gcc target diff --git a/contrib/guix/patches/binutils-mingw-w64-disable-flags.patch b/contrib/guix/patches/binutils-mingw-w64-disable-flags.patch new file mode 100644 index 00000000000..8f88eb9dfd5 --- /dev/null +++ b/contrib/guix/patches/binutils-mingw-w64-disable-flags.patch @@ -0,0 +1,171 @@ +Description: Add disable opposites to the security-related flags +Author: Stephen Kitt + +This patch adds "no-" variants to disable the various security flags: +"no-dynamicbase", "no-nxcompat", "no-high-entropy-va", "disable-reloc-section". + +--- a/ld/emultempl/pe.em ++++ b/ld/emultempl/pe.em +@@ -259,9 +261,11 @@ + (OPTION_ENABLE_LONG_SECTION_NAMES + 1) + /* DLLCharacteristics flags. */ + #define OPTION_DYNAMIC_BASE (OPTION_DISABLE_LONG_SECTION_NAMES + 1) +-#define OPTION_FORCE_INTEGRITY (OPTION_DYNAMIC_BASE + 1) ++#define OPTION_NO_DYNAMIC_BASE (OPTION_DYNAMIC_BASE + 1) ++#define OPTION_FORCE_INTEGRITY (OPTION_NO_DYNAMIC_BASE + 1) + #define OPTION_NX_COMPAT (OPTION_FORCE_INTEGRITY + 1) +-#define OPTION_NO_ISOLATION (OPTION_NX_COMPAT + 1) ++#define OPTION_NO_NX_COMPAT (OPTION_NX_COMPAT + 1) ++#define OPTION_NO_ISOLATION (OPTION_NO_NX_COMPAT + 1) + #define OPTION_NO_SEH (OPTION_NO_ISOLATION + 1) + #define OPTION_NO_BIND (OPTION_NO_SEH + 1) + #define OPTION_WDM_DRIVER (OPTION_NO_BIND + 1) +@@ -271,6 +275,7 @@ + #define OPTION_NO_INSERT_TIMESTAMP (OPTION_INSERT_TIMESTAMP + 1) + #define OPTION_BUILD_ID (OPTION_NO_INSERT_TIMESTAMP + 1) + #define OPTION_ENABLE_RELOC_SECTION (OPTION_BUILD_ID + 1) ++#define OPTION_DISABLE_RELOC_SECTION (OPTION_ENABLE_RELOC_SECTION + 1) + + static void + gld${EMULATION_NAME}_add_options +@@ -342,8 +347,10 @@ + {"enable-long-section-names", no_argument, NULL, OPTION_ENABLE_LONG_SECTION_NAMES}, + {"disable-long-section-names", no_argument, NULL, OPTION_DISABLE_LONG_SECTION_NAMES}, + {"dynamicbase",no_argument, NULL, OPTION_DYNAMIC_BASE}, ++ {"no-dynamicbase", no_argument, NULL, OPTION_NO_DYNAMIC_BASE}, + {"forceinteg", no_argument, NULL, OPTION_FORCE_INTEGRITY}, + {"nxcompat", no_argument, NULL, OPTION_NX_COMPAT}, ++ {"no-nxcompat", no_argument, NULL, OPTION_NO_NX_COMPAT}, + {"no-isolation", no_argument, NULL, OPTION_NO_ISOLATION}, + {"no-seh", no_argument, NULL, OPTION_NO_SEH}, + {"no-bind", no_argument, NULL, OPTION_NO_BIND}, +@@ -351,6 +358,7 @@ + {"tsaware", no_argument, NULL, OPTION_TERMINAL_SERVER_AWARE}, + {"build-id", optional_argument, NULL, OPTION_BUILD_ID}, + {"enable-reloc-section", no_argument, NULL, OPTION_ENABLE_RELOC_SECTION}, ++ {"disable-reloc-section", no_argument, NULL, OPTION_DISABLE_RELOC_SECTION}, + {NULL, no_argument, NULL, 0} + }; + +@@ -485,9 +494,12 @@ + in object files\n")); + fprintf (file, _(" --dynamicbase Image base address may be relocated using\n\ + address space layout randomization (ASLR)\n")); ++ fprintf (file, _(" --no-dynamicbase Image base address may not be relocated\n")); + fprintf (file, _(" --enable-reloc-section Create the base relocation table\n")); ++ fprintf (file, _(" --disable-reloc-section Disable the base relocation table\n")); + fprintf (file, _(" --forceinteg Code integrity checks are enforced\n")); + fprintf (file, _(" --nxcompat Image is compatible with data execution prevention\n")); ++ fprintf (file, _(" --no-nxcompat Image is not compatible with data execution prevention\n")); + fprintf (file, _(" --no-isolation Image understands isolation but do not isolate the image\n")); + fprintf (file, _(" --no-seh Image does not use SEH. No SE handler may\n\ + be called in this image\n")); +@@ -862,12 +874,21 @@ + case OPTION_ENABLE_RELOC_SECTION: + pe_dll_enable_reloc_section = 1; + break; ++ case OPTION_DISABLE_RELOC_SECTION: ++ pe_dll_enable_reloc_section = 0; ++ /* fall through */ ++ case OPTION_NO_DYNAMIC_BASE: ++ pe_dll_characteristics &= ~IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE; ++ break; + case OPTION_FORCE_INTEGRITY: + pe_dll_characteristics |= IMAGE_DLL_CHARACTERISTICS_FORCE_INTEGRITY; + break; + case OPTION_NX_COMPAT: + pe_dll_characteristics |= IMAGE_DLL_CHARACTERISTICS_NX_COMPAT; + break; ++ case OPTION_NO_NX_COMPAT: ++ pe_dll_characteristics &= ~IMAGE_DLL_CHARACTERISTICS_NX_COMPAT; ++ break; + case OPTION_NO_ISOLATION: + pe_dll_characteristics |= IMAGE_DLLCHARACTERISTICS_NO_ISOLATION; + break; +--- a/ld/emultempl/pep.em ++++ b/ld/emultempl/pep.em +@@ -237,9 +240,12 @@ + OPTION_ENABLE_LONG_SECTION_NAMES, + OPTION_DISABLE_LONG_SECTION_NAMES, + OPTION_HIGH_ENTROPY_VA, ++ OPTION_NO_HIGH_ENTROPY_VA, + OPTION_DYNAMIC_BASE, ++ OPTION_NO_DYNAMIC_BASE, + OPTION_FORCE_INTEGRITY, + OPTION_NX_COMPAT, ++ OPTION_NO_NX_COMPAT, + OPTION_NO_ISOLATION, + OPTION_NO_SEH, + OPTION_NO_BIND, +@@ -248,7 +254,8 @@ + OPTION_NO_INSERT_TIMESTAMP, + OPTION_TERMINAL_SERVER_AWARE, + OPTION_BUILD_ID, +- OPTION_ENABLE_RELOC_SECTION ++ OPTION_ENABLE_RELOC_SECTION, ++ OPTION_DISABLE_RELOC_SECTION + }; + + static void +@@ -315,9 +322,12 @@ + {"enable-long-section-names", no_argument, NULL, OPTION_ENABLE_LONG_SECTION_NAMES}, + {"disable-long-section-names", no_argument, NULL, OPTION_DISABLE_LONG_SECTION_NAMES}, + {"high-entropy-va", no_argument, NULL, OPTION_HIGH_ENTROPY_VA}, ++ {"no-high-entropy-va", no_argument, NULL, OPTION_NO_HIGH_ENTROPY_VA}, + {"dynamicbase",no_argument, NULL, OPTION_DYNAMIC_BASE}, ++ {"no-dynamicbase", no_argument, NULL, OPTION_NO_DYNAMIC_BASE}, + {"forceinteg", no_argument, NULL, OPTION_FORCE_INTEGRITY}, + {"nxcompat", no_argument, NULL, OPTION_NX_COMPAT}, ++ {"no-nxcompat", no_argument, NULL, OPTION_NO_NX_COMPAT}, + {"no-isolation", no_argument, NULL, OPTION_NO_ISOLATION}, + {"no-seh", no_argument, NULL, OPTION_NO_SEH}, + {"no-bind", no_argument, NULL, OPTION_NO_BIND}, +@@ -327,6 +337,7 @@ + {"no-insert-timestamp", no_argument, NULL, OPTION_NO_INSERT_TIMESTAMP}, + {"build-id", optional_argument, NULL, OPTION_BUILD_ID}, + {"enable-reloc-section", no_argument, NULL, OPTION_ENABLE_RELOC_SECTION}, ++ {"disable-reloc-section", no_argument, NULL, OPTION_DISABLE_RELOC_SECTION}, + {NULL, no_argument, NULL, 0} + }; + +@@ -448,11 +461,15 @@ + in object files\n")); + fprintf (file, _(" --high-entropy-va Image is compatible with 64-bit address space\n\ + layout randomization (ASLR)\n")); ++ fprintf (file, _(" --no-high-entropy-va Image is not compatible with 64-bit ASLR\n")); + fprintf (file, _(" --dynamicbase Image base address may be relocated using\n\ + address space layout randomization (ASLR)\n")); ++ fprintf (file, _(" --no-dynamicbase Image base address may not be relocated\n")); + fprintf (file, _(" --enable-reloc-section Create the base relocation table\n")); ++ fprintf (file, _(" --disable-reloc-section Disable the base relocation table\n")); + fprintf (file, _(" --forceinteg Code integrity checks are enforced\n")); + fprintf (file, _(" --nxcompat Image is compatible with data execution prevention\n")); ++ fprintf (file, _(" --no-nxcompat Image is not compatible with data execution prevention\n")); + fprintf (file, _(" --no-isolation Image understands isolation but do not isolate the image\n")); + fprintf (file, _(" --no-seh Image does not use SEH; no SE handler may\n\ + be called in this image\n")); +@@ -809,12 +826,24 @@ + case OPTION_ENABLE_RELOC_SECTION: + pep_dll_enable_reloc_section = 1; + break; ++ case OPTION_DISABLE_RELOC_SECTION: ++ pep_dll_enable_reloc_section = 0; ++ /* fall through */ ++ case OPTION_NO_DYNAMIC_BASE: ++ pe_dll_characteristics &= ~IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE; ++ /* fall through */ ++ case OPTION_NO_HIGH_ENTROPY_VA: ++ pe_dll_characteristics &= ~IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA; ++ break; + case OPTION_FORCE_INTEGRITY: + pe_dll_characteristics |= IMAGE_DLL_CHARACTERISTICS_FORCE_INTEGRITY; + break; + case OPTION_NX_COMPAT: + pe_dll_characteristics |= IMAGE_DLL_CHARACTERISTICS_NX_COMPAT; + break; ++ case OPTION_NO_NX_COMPAT: ++ pe_dll_characteristics &= ~IMAGE_DLL_CHARACTERISTICS_NX_COMPAT; ++ break; + case OPTION_NO_ISOLATION: + pe_dll_characteristics |= IMAGE_DLLCHARACTERISTICS_NO_ISOLATION; + break; From a8127b34bce3597b8091e14057c926197966a234 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 2 Mar 2021 17:19:03 -0500 Subject: [PATCH 5/8] build: Use and test PE binutils with --reloc-section Also fix test-security-check.py to account for new PE PIE failure indication. --- configure.ac | 1 + contrib/devtools/test-security-check.py | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 94f01a51a64..6f78e8dec42 100644 --- a/configure.ac +++ b/configure.ac @@ -900,6 +900,7 @@ if test x$use_hardening != xno; then ]) fi + AX_CHECK_LINK_FLAG([[-Wl,--enable-reloc-section]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,--enable-reloc-section"],, [[$LDFLAG_WERROR]]) AX_CHECK_LINK_FLAG([[-Wl,--dynamicbase]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,--dynamicbase"],, [[$LDFLAG_WERROR]]) AX_CHECK_LINK_FLAG([[-Wl,--nxcompat]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,--nxcompat"],, [[$LDFLAG_WERROR]]) AX_CHECK_LINK_FLAG([[-Wl,--high-entropy-va]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,--high-entropy-va"],, [[$LDFLAG_WERROR]]) diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index 73c8732bb6d..14058e2cc8d 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -59,15 +59,17 @@ class TestSecurityChecks(unittest.TestCase): cc = determine_wellknown_cmd('CC', 'x86_64-w64-mingw32-gcc') write_testcode(source) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--no-nxcompat','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va','-no-pie','-fno-PIE']), - (1, executable+': failed DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va','-no-pie','-fno-PIE']), - (1, executable+': failed DYNAMIC_BASE HIGH_ENTROPY_VA RELOC_SECTION')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--dynamicbase','-Wl,--no-high-entropy-va','-no-pie','-fno-PIE']), - (1, executable+': failed HIGH_ENTROPY_VA RELOC_SECTION')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--dynamicbase','-Wl,--high-entropy-va','-no-pie','-fno-PIE']), - (1, executable+': failed RELOC_SECTION')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--no-nxcompat','-Wl,--disable-reloc-section','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va','-no-pie','-fno-PIE']), + (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA NX RELOC_SECTION')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--disable-reloc-section','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va','-no-pie','-fno-PIE']), + (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA RELOC_SECTION')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va','-no-pie','-fno-PIE']), + (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va','-pie','-fPIE']), + (1, executable+': failed PIE DYNAMIC_BASE HIGH_ENTROPY_VA')) # -pie -fPIE does nothing unless --dynamicbase is also supplied + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--no-high-entropy-va','-pie','-fPIE']), + (1, executable+': failed HIGH_ENTROPY_VA')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--enable-reloc-section','-Wl,--dynamicbase','-Wl,--high-entropy-va','-pie','-fPIE']), (0, '')) clean_files(source, executable) From 1946b5f77cb5a6bb37500252079c3582cac4a6c9 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 1 Jul 2021 20:18:03 +0800 Subject: [PATCH 6/8] scripts: more robustly test macOS symbol checks --- contrib/devtools/test-symbol-check.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py index 56ea02719dd..68e479009b5 100755 --- a/contrib/devtools/test-symbol-check.py +++ b/contrib/devtools/test-symbol-check.py @@ -99,7 +99,7 @@ class TestSymbolChecks(unittest.TestCase): ''') - self.assertEqual(call_symbol_check(cc, source, executable, ['-lexpat']), + self.assertEqual(call_symbol_check(cc, 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')) @@ -116,7 +116,7 @@ class TestSymbolChecks(unittest.TestCase): } ''') - self.assertEqual(call_symbol_check(cc, source, executable, ['-framework', 'CoreGraphics']), + self.assertEqual(call_symbol_check(cc, source, executable, ['-framework', 'CoreGraphics', '-Wl,-platform_version','-Wl,macos', '-Wl,11.4', '-Wl,11.4']), (1, f'{executable}: failed MIN_OS SDK')) source = 'test3.c' @@ -129,7 +129,7 @@ class TestSymbolChecks(unittest.TestCase): } ''') - self.assertEqual(call_symbol_check(cc, source, executable, ['-mmacosx-version-min=10.14']), + self.assertEqual(call_symbol_check(cc, source, executable, ['-Wl,-platform_version','-Wl,macos', '-Wl,10.14', '-Wl,11.4']), (1, f'{executable}: failed SDK')) def test_PE(self): From 6cf3345297d371b4785d80d54e802b52ff09e8c2 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 7 Jul 2021 19:59:31 +0800 Subject: [PATCH 7/8] scripts: adjust test-symbol-check for guix release environment Now that our release binaries are build in a glibc 2.24 and 2.27 environment, we can't use a symbol from glibc 2.28 to test our checks. Replace renameat2() with nextup(), which was introduced in 2.24. Note that this also means re-disabling the test for RISC-V, however RISC-V is built in a glibc 2.27 environment, and our minimum required glibc for that binary is 2.27. --- contrib/devtools/test-symbol-check.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py index 68e479009b5..7d83c5f751f 100755 --- a/contrib/devtools/test-symbol-check.py +++ b/contrib/devtools/test-symbol-check.py @@ -19,32 +19,39 @@ def call_symbol_check(cc: List[str], source, executable, options): os.remove(executable) return (p.returncode, p.stdout.rstrip()) +def get_machine(cc: List[str]): + p = subprocess.run([*cc,'-dumpmachine'], stdout=subprocess.PIPE, universal_newlines=True) + return p.stdout.rstrip() + class TestSymbolChecks(unittest.TestCase): def test_ELF(self): source = 'test1.c' executable = 'test1' cc = determine_wellknown_cmd('CC', 'gcc') - # renameat2 was introduced in GLIBC 2.28, so is newer than the upper limit - # of glibc for all platforms + # there's no way to do this test for RISC-V at the moment; we build for + # RISC-V in a glibc 2.27 envinonment and we allow all symbols from 2.27. + if 'riscv' in get_machine(cc): + self.skipTest("test not available for RISC-V") + + # nextup was introduced in GLIBC 2.24, so is newer than our supported + # glibc (2.17), and available in our release build environment (2.24). with open(source, 'w', encoding="utf8") as f: f.write(''' #define _GNU_SOURCE - #include - #include + #include - int renameat2(int olddirfd, const char *oldpath, - int newdirfd, const char *newpath, unsigned int flags); + double nextup(double x); int main() { - renameat2(0, "test", 0, "test_", RENAME_EXCHANGE); + nextup(3.14); return 0; } ''') - self.assertEqual(call_symbol_check(cc, source, executable, []), - (1, executable + ': symbol renameat2 from unsupported version GLIBC_2.28\n' + + self.assertEqual(call_symbol_check(cc, source, executable, ['-lm']), + (1, executable + ': symbol nextup from unsupported version GLIBC_2.24\n' + executable + ': failed IMPORTED_SYMBOLS')) # -lutil is part of the libc6 package so a safe bet that it's installed From 5b4703c6a70db2fa72fcace56a15db07d4b0acf1 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Thu, 21 Jan 2021 14:04:13 -0500 Subject: [PATCH 8/8] guix: Test security-check sanity before performing them --- contrib/guix/libexec/build.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index 02ef8009639..0b96949a6b5 100755 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -299,10 +299,11 @@ mkdir -p "$DISTSRC" # Build Bitcoin Core make --jobs="$JOBS" ${V:+V=1} - # Perform basic ELF security checks on a series of executables. + # Check that symbol/security checks tools are sane. + make test-security-check ${V:+V=1} + # Perform basic security checks on a series of executables. make -C src --jobs=1 check-security ${V:+V=1} - # Check that executables only contain allowed gcc, glibc and libstdc++ - # version symbols for Linux distro back-compatibility. + # Check that executables only contain allowed version symbols. make -C src --jobs=1 check-symbols ${V:+V=1} mkdir -p "$OUTDIR"