Merge #18434: tests: add a test-security target and run it in CI

9fe71a57a6780569e618cf9a8d4f1acf6321017f test: use subprocess.run() in test-security-check.py (fanquake)
968aaae940b064f21eddee6bb461aa08f777544c tests: run test-security-check.py in CI (fanquake)

Pull request description:

  [Wladimir asked](https://github.com/bitcoin/bitcoin/pull/18415#issuecomment-603843094) about running the `test-security-check.py` script in our CI. This PR adds a target for that: `make test-security` and adds it to a few CI jobs.

ACKs for top commit:
  laanwj:
    ACK 9fe71a57a6780569e618cf9a8d4f1acf6321017f

Tree-SHA512: d00ebbefbd57ab22436f284837c320f73238ec9967495adc4f2f9a4d574b3b1595c19ce41d53ff4060d5cd7174dbc311235d5877c90e8af2f5587735e7236056

8 files changed, 27 insertions, 4 deletions

diff --git a/Makefile.am b/Makefile.am
index 45dab3930d..75a164f49e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -342,3 +342,14 @@ clean-local: clean-docs
 	rm -rf coverage_percent.txt test_bitcoin.coverage/ total.coverage/ fuzz.coverage/ test/tmp/ cache/ $(OSX_APP)
 	rm -rf test/functional/__pycache__ test/functional/test_framework/__pycache__ test/cache share/rpcauth/__pycache__
 	rm -rf osx_volname dist/ dpi36.background.tiff dpi72.background.tiff
+
+test-security-check:
+if TARGET_DARWIN
+	$(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_MACHO
+endif
+if TARGET_WINDOWS
+	$(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_PE
+endif
+if TARGET_LINUX
+	$(AM_V_at) $(PYTHON) $(top_srcdir)/contrib/devtools/test-security-check.py TestSecurityChecks.test_ELF
+endif
diff --git a/ci/test/00_setup_env.sh b/ci/test/00_setup_env.sh
index 56b8fc976e..711f436630 100755
--- a/ci/test/00_setup_env.sh
+++ b/ci/test/00_setup_env.sh
@@ -33,6 +33,7 @@ export HOST=${HOST:-$("$BASE_ROOT_DIR/depends/config.guess")}
 export USE_BUSY_BOX=${USE_BUSY_BOX:-false}
 export RUN_UNIT_TESTS=${RUN_UNIT_TESTS:-true}
 export RUN_FUNCTIONAL_TESTS=${RUN_FUNCTIONAL_TESTS:-true}
+export RUN_SECURITY_TESTS=${RUN_SECURITY_TESTS:-false}
 export TEST_RUNNER_ENV=${TEST_RUNNER_ENV:-}
 export RUN_FUZZ_TESTS=${RUN_FUZZ_TESTS:-false}
 export CONTAINER_NAME=${CONTAINER_NAME:-ci_unnamed}
diff --git a/ci/test/00_setup_env_mac_host.sh b/ci/test/00_setup_env_mac_host.sh
index 982e38daee..b24dc7278d 100644
--- a/ci/test/00_setup_env_mac_host.sh
+++ b/ci/test/00_setup_env_mac_host.sh
@@ -11,6 +11,7 @@ export PIP_PACKAGES="zmq"
 export GOAL="install"
 export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --enable-werror"
 export TEST_RUNNER_EXTRA="wallet_disable"  # Only run wallet_disable as a smoke test, see https://github.com/bitcoin/bitcoin/pull/17240#issuecomment-546022121 why the other tests are disabled
+export RUN_SECURITY_TESTS="true"
 # Run without depends
 export NO_DEPENDS=1
 export OSX_SDK=""
diff --git a/ci/test/00_setup_env_native_qt5.sh b/ci/test/00_setup_env_native_qt5.sh
index fa07990756..496251a125 100644
--- a/ci/test/00_setup_env_native_qt5.sh
+++ b/ci/test/00_setup_env_native_qt5.sh
@@ -10,6 +10,7 @@ export CONTAINER_NAME=ci_native_qt5
 export PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libdbus-1-dev libharfbuzz-dev"
 export DEP_OPTS="NO_QT=1 NO_UPNP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1"
 export TEST_RUNNER_EXTRA="--previous-releases --coverage --extended --exclude feature_dbcrash"  # Run extended tests so that coverage does not fail, but exclude the very slow dbcrash
+export RUN_SECURITY_TESTS="true"
 export RUN_UNIT_TESTS_SEQUENTIAL="true"
 export RUN_UNIT_TESTS="false"
 export GOAL="install"
diff --git a/ci/test/00_setup_env_win64.sh b/ci/test/00_setup_env_win64.sh
index 8f0c62a1a6..90c3ebd28e 100644
--- a/ci/test/00_setup_env_win64.sh
+++ b/ci/test/00_setup_env_win64.sh
@@ -10,5 +10,6 @@ export CONTAINER_NAME=ci_win64
 export HOST=x86_64-w64-mingw32
 export PACKAGES="python3 nsis g++-mingw-w64-x86-64 wine-binfmt wine64"
 export RUN_FUNCTIONAL_TESTS=false
+export RUN_SECURITY_TESTS="true"
 export GOAL="deploy"
 export BITCOIN_CONFIG="--enable-reduce-exports --disable-gui-tests"
diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh
index 0d23d9eed2..96d44328b8 100755
--- a/ci/test/06_script_b.sh
+++ b/ci/test/06_script_b.sh
@@ -39,6 +39,12 @@ if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then
   END_FOLD
 fi
 
+if [ "$RUN_SECURITY_TESTS" = "true" ]; then
+  BEGIN_FOLD security-tests
+  DOCKER_EXEC make test-security-check
+  END_FOLD
+fi
+
 if [ "$RUN_FUZZ_TESTS" = "true" ]; then
   BEGIN_FOLD fuzz-tests
   DOCKER_EXEC LD_LIBRARY_PATH=$DEPENDS_DIR/$HOST/lib test/fuzz/test_runner.py ${FUZZ_TESTS_CONFIG} $MAKEJOBS -l DEBUG ${DIR_FUZZ_IN}
diff --git a/configure.ac b/configure.ac
index 0218bf3022..474d8a7919 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1600,6 +1600,7 @@ fi
 
 AM_CONDITIONAL([TARGET_DARWIN], [test x$TARGET_OS = xdarwin])
 AM_CONDITIONAL([BUILD_DARWIN], [test x$BUILD_OS = xdarwin])
+AM_CONDITIONAL([TARGET_LINUX], [test x$TARGET_OS = xlinux])
 AM_CONDITIONAL([TARGET_WINDOWS], [test x$TARGET_OS = xwindows])
 AM_CONDITIONAL([ENABLE_WALLET],[test x$enable_wallet = xyes])
 AM_CONDITIONAL([ENABLE_TESTS],[test x$BUILD_TEST = xyes])
@@ -1688,6 +1689,8 @@ AC_SUBST(HAVE_WEAK_GETAUXVAL)
 AC_CONFIG_FILES([Makefile src/Makefile doc/man/Makefile share/setup.nsi share/qt/Info.plist test/config.ini])
 AC_CONFIG_FILES([contrib/devtools/split-debug.sh],[chmod +x contrib/devtools/split-debug.sh])
 AM_COND_IF([HAVE_DOXYGEN], [AC_CONFIG_FILES([doc/Doxyfile])])
+AC_CONFIG_LINKS([contrib/devtools/security-check.py:contrib/devtools/security-check.py])
+AC_CONFIG_LINKS([contrib/devtools/test-security-check.py:contrib/devtools/test-security-check.py])
 AC_CONFIG_LINKS([contrib/filter-lcov.py:contrib/filter-lcov.py])
 AC_CONFIG_LINKS([test/functional/test_runner.py:test/functional/test_runner.py])
 AC_CONFIG_LINKS([test/fuzz/test_runner.py:test/fuzz/test_runner.py])
diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py
index ea70b27941..629eba4f28 100755
--- a/contrib/devtools/test-security-check.py
+++ b/contrib/devtools/test-security-check.py
@@ -20,10 +20,9 @@ def write_testcode(filename):
     ''')
 
 def call_security_check(cc, source, executable, options):
-    subprocess.check_call([cc,source,'-o',executable] + options)
-    p = subprocess.Popen(['./security-check.py',executable], stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE, universal_newlines=True)
-    (stdout, stderr) = p.communicate()
-    return (p.returncode, stdout.rstrip())
+    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())
 
 class TestSecurityChecks(unittest.TestCase):
     def test_ELF(self):