Merge #17863: scripts: Add MACHO dylib checks to symbol-check.py

c491368d8cfddf3a5b6d574f10ed67492fcecbed scripts: add MACHO dylib checking to symbol-check.py (fanquake)
76bf97213f4b153dd3ccf1314088a73c4804601d scripts: fix check-symbols & check-security argument passing (fanquake)

Pull request description:

  Based on #17857.

  This adds dynamic library checks for MACHO executables to symbol-check.py. The script has been modified to function more like `security-check.py`. The error output is now also slightly different. i.e:
  ```bash
  # Linux x86
  bitcoin-cli: symbol operator new[](unsigned long) from unsupported version GLIBCXX_3.4
  bitcoin-cli: export of symbol vtable for std::basic_ios<char, std::char_traits<char> > not allowed
  bitcoin-cli: NEEDED library libstdc++.so.6 is not allowed
  bitcoin-cli: failed IMPORTED_SYMBOLS EXPORTED_SYMBOLS LIBRARY_DEPENDENCIES

  # RISCV (skips exported symbols checks)
  bitcoin-tx: symbol operator new[](unsigned long) from unsupported version GLIBCXX_3.4
  bitcoin-tx: NEEDED library libstdc++.so.6 is not allowed
  bitcoin-tx: failed IMPORTED_SYMBOLS LIBRARY_DEPENDENCIES

  # macOS
  Checking macOS dynamic libraries...
  libboost_filesystem.dylib is not in ALLOWED_LIBRARIES!
  bitcoind: failed DYNAMIC_LIBRARIES
  ```

  Compared to `v0.19.0.1` the macOS allowed dylibs has been slimmed down somewhat:
  ```diff
   src/qt/bitcoin-qt:
   /usr/lib/libSystem.B.dylib
  -/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration
   /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit
   /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation
   /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
   /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit
   /System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices
   /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  -/System/Library/Frameworks/Security.framework/Versions/A/Security
  -/System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration
   /System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics
  -/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL
  -/System/Library/Frameworks/AGL.framework/Versions/A/AGL
   /System/Library/Frameworks/Carbon.framework/Versions/A/Carbon
   /usr/lib/libc++.1.dylib
  -/System/Library/Frameworks/CFNetwork.framework/Versions/A/CFNetwork
   /System/Library/Frameworks/CoreText.framework/Versions/A/CoreText
   /System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO
   /usr/lib/libobjc.A.dylib
  ```

ACKs for top commit:
  laanwj:
    ACK c491368d8cfddf3a5b6d574f10ed67492fcecbed

Tree-SHA512: f8624e4964e80b3e0d34e8d3cc33f3107938f3ef7a01c07828f09b902b5ea31a53c50f9be03576e1896ed832cf2c399e03a7943a4f537a1e1c705f3804aed979

2 files changed, 122 insertions, 28 deletions

diff --git a/contrib/devtools/README.md b/contrib/devtools/README.md
index c35affac59..515a0d8fc6 100644
--- a/contrib/devtools/README.md
+++ b/contrib/devtools/README.md
@@ -103,17 +103,21 @@ Perform basic security checks on a series of executables.
 symbol-check.py
 ===============
 
-A script to check that the (Linux) executables produced by gitian only contain
-allowed gcc, glibc and libstdc++ version symbols. This makes sure they are
-still compatible with the minimum supported Linux distribution versions.
+A script to check that the executables produced by gitian only contain
+certain symbols and are only linked against allowed libraries.
+
+For Linux this means checking for allowed gcc, glibc and libstdc++ version symbols.
+This makes sure they are still compatible with the minimum supported distribution versions.
+
+For macOS we check that the executables are only linked against libraries we allow.
 
 Example usage after a gitian build:
 
     find ../gitian-builder/build -type f -executable | xargs python3 contrib/devtools/symbol-check.py
 
-If only supported symbols are used the return value will be 0 and the output will be empty.
+If no errors occur the return value will be 0 and the output will be empty.
 
-If there are 'unsupported' symbols, the return value will be 1 a list like this will be printed:
+If there are any errors the return value will be 1 and output like this will be printed:
 
     .../64/test_bitcoin: symbol memcpy from unsupported version GLIBC_2.14
     .../64/test_bitcoin: symbol __fdelt_chk from unsupported version GLIBC_2.15
diff --git a/contrib/devtools/symbol-check.py b/contrib/devtools/symbol-check.py
index 0c59ab6239..f92d997621 100755
--- a/contrib/devtools/symbol-check.py
+++ b/contrib/devtools/symbol-check.py
@@ -15,6 +15,7 @@ import subprocess
 import re
 import sys
 import os
+from typing import List, Optional, Tuple
 
 # Debian 8 (Jessie) EOL: 2020. https://wiki.debian.org/DebianReleases#Production_Releases
 #
@@ -52,8 +53,10 @@ IGNORE_EXPORTS = {
 }
 READELF_CMD = os.getenv('READELF', '/usr/bin/readelf')
 CPPFILT_CMD = os.getenv('CPPFILT', '/usr/bin/c++filt')
+OTOOL_CMD = os.getenv('OTOOL', '/usr/bin/otool')
+
 # Allowed NEEDED libraries
-ALLOWED_LIBRARIES = {
+ELF_ALLOWED_LIBRARIES = {
 # bitcoind and bitcoin-qt
 'libgcc_s.so.1', # GCC base support
 'libc.so.6', # C library
@@ -79,6 +82,25 @@ ARCH_MIN_GLIBC_VER = {
 'AArch64':(2,17),
 'RISC-V': (2,27)
 }
+
+MACHO_ALLOWED_LIBRARIES = {
+# bitcoind and bitcoin-qt
+'libc++.1.dylib', # C++ Standard Library
+'libSystem.B.dylib', # libc, libm, libpthread, libinfo
+# bitcoin-qt only
+'AppKit', # user interface
+'ApplicationServices', # common application tasks.
+'Carbon', # deprecated c back-compat API
+'CoreFoundation', # low level func, data types
+'CoreGraphics', # 2D rendering
+'CoreServices', # operating system services
+'CoreText', # interface for laying out text and handling fonts.
+'Foundation', # base layer functionality for apps/frameworks
+'ImageIO', # read and write image file formats.
+'IOKit', # user-space access to hardware devices and drivers.
+'libobjc.A.dylib', # Objective-C runtime library
+}
+
 class CPPFilt(object):
     '''
     Demangle C++ symbol names.
@@ -98,15 +120,15 @@ class CPPFilt(object):
         self.proc.stdout.close()
         self.proc.wait()
 
-def read_symbols(executable, imports=True):
+def read_symbols(executable, imports=True) -> List[Tuple[str, str, str]]:
     '''
-    Parse an ELF executable and return a list of (symbol,version) tuples
+    Parse an ELF executable and return a list of (symbol,version, arch) tuples
     for dynamic, imported symbols.
     '''
     p = subprocess.Popen([READELF_CMD, '--dyn-syms', '-W', '-h', executable], stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE, universal_newlines=True)
     (stdout, stderr) = p.communicate()
     if p.returncode:
-        raise IOError('Could not read symbols for %s: %s' % (executable, stderr.strip()))
+        raise IOError('Could not read symbols for {}: {}'.format(executable, stderr.strip()))
     syms = []
     for line in stdout.splitlines():
         line = line.split()
@@ -121,7 +143,7 @@ def read_symbols(executable, imports=True):
                 syms.append((sym, version, arch))
     return syms
 
-def check_version(max_versions, version, arch):
+def check_version(max_versions, version, arch) -> bool:
     if '_' in version:
         (lib, _, ver) = version.rpartition('_')
     else:
@@ -132,7 +154,7 @@ def check_version(max_versions, version, arch):
         return False
     return ver <= max_versions[lib] or lib == 'GLIBC' and ver <= ARCH_MIN_GLIBC_VER[arch]
 
-def read_libraries(filename):
+def elf_read_libraries(filename) -> List[str]:
     p = subprocess.Popen([READELF_CMD, '-d', '-W', filename], stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE, universal_newlines=True)
     (stdout, stderr) = p.communicate()
     if p.returncode:
@@ -148,26 +170,94 @@ def read_libraries(filename):
                 raise ValueError('Unparseable (NEEDED) specification')
     return libraries
 
-if __name__ == '__main__':
+def check_imported_symbols(filename) -> bool:
     cppfilt = CPPFilt()
+    ok = True
+    for sym, version, arch in read_symbols(filename, True):
+        if version and not check_version(MAX_VERSIONS, version, arch):
+            print('{}: symbol {} from unsupported version {}'.format(filename, cppfilt(sym), version))
+            ok = False
+    return ok
+
+def check_exported_symbols(filename) -> bool:
+    cppfilt = CPPFilt()
+    ok = True
+    for sym,version,arch in read_symbols(filename, False):
+        if arch == 'RISC-V' or sym in IGNORE_EXPORTS:
+            continue
+        print('{}: export of symbol {} not allowed'.format(filename, cppfilt(sym)))
+        ok = False
+    return ok
+
+def check_ELF_libraries(filename) -> bool:
+    ok = True
+    for library_name in elf_read_libraries(filename):
+        if library_name not in ELF_ALLOWED_LIBRARIES:
+            print('{}: NEEDED library {} is not allowed'.format(filename, library_name))
+            ok = False
+    return ok
+
+def macho_read_libraries(filename) -> List[str]:
+    p = subprocess.Popen([OTOOL_CMD, '-L', filename], stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE, universal_newlines=True)
+    (stdout, stderr) = p.communicate()
+    if p.returncode:
+        raise IOError('Error opening file')
+    libraries = []
+    for line in stdout.splitlines():
+        tokens = line.split()
+        if len(tokens) == 1: # skip executable name
+            continue
+        libraries.append(tokens[0].split('/')[-1])
+    return libraries
+
+def check_MACHO_libraries(filename) -> bool:
+    ok = True
+    for dylib in macho_read_libraries(filename):
+        if dylib not in MACHO_ALLOWED_LIBRARIES:
+            print('{} is not in ALLOWED_LIBRARIES!'.format(dylib))
+            ok = False
+    return ok
+
+CHECKS = {
+'ELF': [
+    ('IMPORTED_SYMBOLS', check_imported_symbols),
+    ('EXPORTED_SYMBOLS', check_exported_symbols),
+    ('LIBRARY_DEPENDENCIES', check_ELF_libraries)
+],
+'MACHO': [
+    ('DYNAMIC_LIBRARIES', check_MACHO_libraries)
+]
+}
+
+def identify_executable(executable) -> Optional[str]:
+    with open(filename, 'rb') as f:
+        magic = f.read(4)
+    if magic.startswith(b'MZ'):
+        return 'PE'
+    elif magic.startswith(b'\x7fELF'):
+        return 'ELF'
+    elif magic.startswith(b'\xcf\xfa'):
+        return 'MACHO'
+    return None
+
+if __name__ == '__main__':
     retval = 0
     for filename in sys.argv[1:]:
-        # Check imported symbols
-        for sym,version,arch in read_symbols(filename, True):
-            if version and not check_version(MAX_VERSIONS, version, arch):
-                print('%s: symbol %s from unsupported version %s' % (filename, cppfilt(sym), version))
-                retval = 1
-        # Check exported symbols
-        if arch != 'RISC-V':
-            for sym,version,arch in read_symbols(filename, False):
-                if sym in IGNORE_EXPORTS:
-                    continue
-                print('%s: export of symbol %s not allowed' % (filename, cppfilt(sym)))
-                retval = 1
-        # Check dependency libraries
-        for library_name in read_libraries(filename):
-            if library_name not in ALLOWED_LIBRARIES:
-                print('%s: NEEDED library %s is not allowed' % (filename, library_name))
+        try:
+            etype = identify_executable(filename)
+            if etype is None:
+                print('{}: unknown format'.format(filename))
                 retval = 1
+                continue
 
+            failed = []
+            for (name, func) in CHECKS[etype]:
+                if not func(filename):
+                    failed.append(name)
+            if failed:
+                print('{}: failed {}'.format(filename, ' '.join(failed)))
+                retval = 1
+        except IOError:
+            print('{}: cannot open'.format(filename))
+            retval = 1
     sys.exit(retval)