diff --git a/0001-subprocess-usage-explicitly-pass-close_fds-True.patch b/0001-subprocess-usage-explicitly-pass-close_fds-True.patch new file mode 100644 index 0000000..82a5d91 --- /dev/null +++ b/0001-subprocess-usage-explicitly-pass-close_fds-True.patch @@ -0,0 +1,96 @@ +From fbd8d90c88ee26e6020bae0983db7214464a4c46 Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 20 Feb 2019 09:43:15 +0100 +Subject: [PATCH 1/6] subprocess usage: explicitly pass `close_fds = True` + +In python2 the default for `close_fds` is still False, therefore it is +possible that open file descriptors like the logfile are inherited to +child processes. This is prevented by explicitly passing this parameter +to all subprocess invocations. +--- + src/mount_efs/__init__.py | 18 ++++++++++-------- + src/watchdog/__init__.py | 2 +- + 2 files changed, 11 insertions(+), 9 deletions(-) + +diff --git a/src/mount_efs/__init__.py b/src/mount_efs/__init__.py +index 833158f..8b15409 100755 +--- a/src/mount_efs/__init__.py ++++ b/src/mount_efs/__init__.py +@@ -235,7 +235,7 @@ def is_stunnel_option_supported(stunnel_output, stunnel_option_name): + + + def get_version_specific_stunnel_options(config): +- proc = subprocess.Popen(['stunnel', '-help'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) ++ proc = subprocess.Popen(['stunnel', '-help'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True) + proc.wait() + _, err = proc.communicate() + +@@ -355,7 +355,7 @@ def check_network_status(fs_id, init_system): + return + + with open(os.devnull, 'w') as devnull: +- rc = subprocess.call(['systemctl', 'status', 'network.target'], stdout=devnull, stderr=devnull) ++ rc = subprocess.call(['systemctl', 'status', 'network.target'], stdout=devnull, stderr=devnull, close_fds=True) + + if rc != 0: + fatal_error('Failed to mount %s because the network was not yet available, add "_netdev" to your mount options' % fs_id, +@@ -364,19 +364,20 @@ def check_network_status(fs_id, init_system): + + def start_watchdog(init_system): + if init_system == 'init': +- proc = subprocess.Popen(['/sbin/status', WATCHDOG_SERVICE], stdout=subprocess.PIPE, stderr=subprocess.PIPE) ++ proc = subprocess.Popen( ++ ['/sbin/status', WATCHDOG_SERVICE], stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True) + status, _ = proc.communicate() + if 'stop' in status: + with open(os.devnull, 'w') as devnull: +- subprocess.Popen(['/sbin/start', WATCHDOG_SERVICE], stdout=devnull, stderr=devnull) ++ subprocess.Popen(['/sbin/start', WATCHDOG_SERVICE], stdout=devnull, stderr=devnull, close_fds=True) + elif 'start' in status: + logging.debug('%s is already running', WATCHDOG_SERVICE) + + elif init_system == 'systemd': +- rc = subprocess.call(['systemctl', 'is-active', '--quiet', WATCHDOG_SERVICE]) ++ rc = subprocess.call(['systemctl', 'is-active', '--quiet', WATCHDOG_SERVICE], close_fds=True) + if rc != 0: + with open(os.devnull, 'w') as devnull: +- subprocess.Popen(['systemctl', 'start', WATCHDOG_SERVICE], stdout=devnull, stderr=devnull) ++ subprocess.Popen(['systemctl', 'start', WATCHDOG_SERVICE], stdout=devnull, stderr=devnull, close_fds=True) + else: + logging.debug('%s is already running', WATCHDOG_SERVICE) + +@@ -404,7 +405,8 @@ def bootstrap_tls(config, init_system, dns_name, fs_id, mountpoint, options, sta + + # launch the tunnel in a process group so if it has any child processes, they can be killed easily by the mount watchdog + logging.info('Starting TLS tunnel: "%s"', ' '.join(tunnel_args)) +- tunnel_proc = subprocess.Popen(tunnel_args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, preexec_fn=os.setsid) ++ tunnel_proc = subprocess.Popen( ++ tunnel_args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, preexec_fn=os.setsid, close_fds=True) + logging.info('Started TLS tunnel, pid: %d', tunnel_proc.pid) + + temp_tls_state_file = write_tls_tunnel_state_file(fs_id, mountpoint, tls_port, tunnel_proc.pid, tunnel_args, +@@ -458,7 +460,7 @@ def mount_nfs(dns_name, path, mountpoint, options): + + logging.info('Executing: "%s"', ' '.join(command)) + +- proc = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) ++ proc = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True) + out, err = proc.communicate() + + if proc.returncode == 0: +diff --git a/src/watchdog/__init__.py b/src/watchdog/__init__.py +index ea465a7..caca0d9 100755 +--- a/src/watchdog/__init__.py ++++ b/src/watchdog/__init__.py +@@ -150,7 +150,7 @@ def is_pid_running(pid): + def start_tls_tunnel(child_procs, state_file, command): + # launch the tunnel in a process group so if it has any child processes, they can be killed easily + logging.info('Starting TLS tunnel: "%s"', ' '.join(command)) +- tunnel = subprocess.Popen(command, preexec_fn=os.setsid) ++ tunnel = subprocess.Popen(command, preexec_fn=os.setsid, close_fds=True) + + if not is_pid_running(tunnel.pid): + fatal_error('Failed to initialize TLS tunnel for %s' % state_file, 'Failed to start TLS tunnel.') +-- +2.21.0 + diff --git a/0002-state_file_dir-choose-safe-default-mode-make-mode-co.patch b/0002-state_file_dir-choose-safe-default-mode-make-mode-co.patch new file mode 100644 index 0000000..a066256 --- /dev/null +++ b/0002-state_file_dir-choose-safe-default-mode-make-mode-co.patch @@ -0,0 +1,64 @@ +From 8ea6a3ada710cf833dac549b973d09512bce1b78 Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 20 Feb 2019 10:32:08 +0100 +Subject: [PATCH 2/6] state_file_dir: choose safe default mode, make mode + configurable + +`os.makedirs()` uses default mode 0777 in Python2. Therefore the +protection level of the state_file_dir depends on the inherited umask. A +default mode of 0750 is a good conservative default for this. To allow +admins and system integrators to tune this setting it is configurable +via the new config file setting 'state_file_dir_mode'. +--- + dist/efs-utils.conf | 2 ++ + src/mount_efs/__init__.py | 16 +++++++++++++++- + 2 files changed, 17 insertions(+), 1 deletion(-) + +diff --git a/dist/efs-utils.conf b/dist/efs-utils.conf +index 36e8de0..75d2111 100644 +--- a/dist/efs-utils.conf ++++ b/dist/efs-utils.conf +@@ -10,6 +10,8 @@ + logging_level = INFO + logging_max_bytes = 1048576 + logging_file_count = 10 ++# mode for /var/run/efs in octal ++state_file_dir_mode = 750 + + [mount] + dns_name_format = {fs_id}.efs.{region}.amazonaws.com +diff --git a/src/mount_efs/__init__.py b/src/mount_efs/__init__.py +index 8b15409..a095ba7 100755 +--- a/src/mount_efs/__init__.py ++++ b/src/mount_efs/__init__.py +@@ -387,12 +387,26 @@ def start_watchdog(init_system): + logging.warning(error_message) + + ++def create_state_file_dir(config, state_file_dir): ++ mode = 0o750 ++ try: ++ mode_str = config.get(CONFIG_SECTION, 'state_file_dir_mode') ++ try: ++ mode = int(mode_str, 8) ++ except ValueError: ++ logging.warn('Bad state_file_dir_mode "%s" in config file "%s"', mode_str, CONFIG_FILE) ++ except ConfigParser.NoOptionError: ++ pass ++ ++ os.makedirs(state_file_dir, mode) ++ ++ + @contextmanager + def bootstrap_tls(config, init_system, dns_name, fs_id, mountpoint, options, state_file_dir=STATE_FILE_DIR): + start_watchdog(init_system) + + if not os.path.exists(state_file_dir): +- os.makedirs(state_file_dir) ++ create_state_file_dir(config, state_file_dir) + + tls_port = choose_tls_port(config) + options['tlsport'] = tls_port +-- +2.21.0 + diff --git a/0003-pytest-adjust-tests-to-new-state_file_dir_mode-confi.patch b/0003-pytest-adjust-tests-to-new-state_file_dir_mode-confi.patch new file mode 100644 index 0000000..c9a5325 --- /dev/null +++ b/0003-pytest-adjust-tests-to-new-state_file_dir_mode-confi.patch @@ -0,0 +1,34 @@ +From ba525f6b6a41ff9df720e073df9e5bed4c59c360 Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Fri, 22 Feb 2019 12:46:49 +0100 +Subject: [PATCH 3/6] pytest: adjust tests to new state_file_dir_mode config + option + +This adds a side_effect to test_bootstrap_tls to cover the new +stat_file_dir_mode config option. +--- + test/mount_efs_test/test_bootstrap_tls.py | 8 ++++++++ + 1 file changed, 8 insertions(+) + +diff --git a/test/mount_efs_test/test_bootstrap_tls.py b/test/mount_efs_test/test_bootstrap_tls.py +index af1df21..39d5d70 100644 +--- a/test/mount_efs_test/test_bootstrap_tls.py ++++ b/test/mount_efs_test/test_bootstrap_tls.py +@@ -66,6 +66,14 @@ def test_bootstrap_tls_state_file_nonexistent_dir(mocker, tmpdir): + mocker.patch('os.kill') + state_file_dir = str(tmpdir.join(tempfile.mktemp())) + ++ def config_get_side_effect(section, field): ++ if section == mount_efs.CONFIG_SECTION and field == 'state_file_dir_mode': ++ return '0755' ++ else: ++ raise ValueError('Unexpected arguments') ++ ++ MOCK_CONFIG.get.side_effect = config_get_side_effect ++ + assert not os.path.exists(state_file_dir) + + with mount_efs.bootstrap_tls(MOCK_CONFIG, INIT_SYSTEM, DNS_NAME, FS_ID, MOUNT_POINT, {}, state_file_dir): +-- +2.21.0 + diff --git a/0004-choose_tls_port-reuse-socket-and-explicitly-close-it.patch b/0004-choose_tls_port-reuse-socket-and-explicitly-close-it.patch new file mode 100644 index 0000000..6b060cc --- /dev/null +++ b/0004-choose_tls_port-reuse-socket-and-explicitly-close-it.patch @@ -0,0 +1,49 @@ +From 9a8526d26596b3d2e839841fa44b601069828fa9 Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 20 Feb 2019 10:58:11 +0100 +Subject: [PATCH 4/6] choose_tls_port(): reuse socket and explicitly close it + in all cases + +This function only closes the socket on success, i.e. for each +unsuccessful bind attempt a socket "leaks". It does not actually leak, +because the Python interface implements reference counting. Still it is +unclean, because after successful bind the socket is explicitly closed. +So either the application is responsible for closing the socket, or not. +Since it is better not to rely on the implementation of the Python +interpreter and the socket module it should be prefered to always +explicitly close the socket. + +Also this function opens a new socket for each port to try. This is +inefficient, since the same socket can be reused for testing. Therefore +only open and close a single socket. +--- + src/mount_efs/__init__.py | 5 ++++- + 1 file changed, 4 insertions(+), 1 deletion(-) + +diff --git a/src/mount_efs/__init__.py b/src/mount_efs/__init__.py +index a095ba7..9f1ee46 100755 +--- a/src/mount_efs/__init__.py ++++ b/src/mount_efs/__init__.py +@@ -180,8 +180,9 @@ def choose_tls_port(config): + ports_to_try = tls_ports[mid:] + tls_ports[:mid] + assert len(tls_ports) == len(ports_to_try) + ++ sock = socket.socket() ++ + for tls_port in ports_to_try: +- sock = socket.socket() + try: + sock.bind(('localhost', tls_port)) + sock.close() +@@ -189,6 +190,8 @@ def choose_tls_port(config): + except socket.error: + continue + ++ sock.close() ++ + fatal_error('Failed to locate an available port in the range [%d, %d], ' + 'try specifying a different port range in %s' + % (lower_bound, upper_bound, CONFIG_FILE)) +-- +2.21.0 + diff --git a/0005-watchdog-be-robust-against-unrelated-localhost-based.patch b/0005-watchdog-be-robust-against-unrelated-localhost-based.patch new file mode 100644 index 0000000..12f6ecd --- /dev/null +++ b/0005-watchdog-be-robust-against-unrelated-localhost-based.patch @@ -0,0 +1,45 @@ +From abe6750a7ee39852b011faa0791963d854788984 Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 20 Feb 2019 11:19:28 +0100 +Subject: [PATCH 5/6] watchdog: be robust against unrelated localhost based nfs + mounts + +While a bit exotic there can exist mounts of locally exported nfs +shares that aren't related to EFS. In this case the watchdog fails, +because it tries to access the port option that is not present in these +unrelated mount entries. + +To fix this discard entries from /proc/mounts that don't carry a port +option. +--- + src/watchdog/__init__.py | 7 ++++++- + 1 file changed, 6 insertions(+), 1 deletion(-) + +diff --git a/src/watchdog/__init__.py b/src/watchdog/__init__.py +index caca0d9..d002cf9 100755 +--- a/src/watchdog/__init__.py ++++ b/src/watchdog/__init__.py +@@ -95,6 +95,9 @@ def get_file_safe_mountpoint(mount): + mountpoint = mountpoint[1:] + + opts = parse_options(mount.options) ++ if 'port' not in opts: ++ # some other localhost nfs mount not running over stunnel ++ return None + return mountpoint + '.' + opts['port'] + + +@@ -113,7 +116,9 @@ def get_current_local_nfs_mounts(mount_file='/proc/mounts'): + + mount_dict = {} + for m in mounts: +- mount_dict[get_file_safe_mountpoint(m)] = m ++ safe_mnt = get_file_safe_mountpoint(m) ++ if safe_mnt: ++ mount_dict[safe_mnt] = m + + return mount_dict + +-- +2.21.0 + diff --git a/aws-efs-utils.changes b/aws-efs-utils.changes index 58a7d68..4153728 100644 --- a/aws-efs-utils.changes +++ b/aws-efs-utils.changes @@ -1,3 +1,13 @@ +------------------------------------------------------------------- +Wed Apr 3 08:38:34 UTC 2019 - John Paul Adrian Glaubitz + +- Include hardening and robustness fixes from security audit (bsc#1125133) + + 0001-subprocess-usage-explicitly-pass-close_fds-True.patch + + 0002-state_file_dir-choose-safe-default-mode-make-mode-co.patch + + 0003-pytest-adjust-tests-to-new-state_file_dir_mode-confi.patch + + 0004-choose_tls_port-reuse-socket-and-explicitly-close-it.patch + + 0005-watchdog-be-robust-against-unrelated-localhost-based.patch + ------------------------------------------------------------------- Thu Feb 14 14:54:30 UTC 2019 - Robert Schweikert diff --git a/aws-efs-utils.spec b/aws-efs-utils.spec index 4420cae..19b248f 100644 --- a/aws-efs-utils.spec +++ b/aws-efs-utils.spec @@ -1,5 +1,5 @@ # -# spec file for package amazon-efs-utils +# spec file for package aws-efs-utils # # Copyright (c) 2019 SUSE LINUX GmbH, Nuernberg, Germany. # @@ -12,9 +12,10 @@ # license that conforms to the Open Source Definition (Version 1.9) # published by the Open Source Initiative. -# Please submit bugfixes or comments via http://bugs.opensuse.org/ +# Please submit bugfixes or comments via https://bugs.opensuse.org/ # + Name: aws-efs-utils Version: 1.5 Release: 0 @@ -24,6 +25,12 @@ Group: System/Management Url: https://github.com/aws/efs-utils Source0: https://github.com/aws/efs-utils/archive/v%{version}.tar.gz Patch: efs-switchparser.patch +# Hardening patches (see: https://github.com/aws/efs-utils/pull/26 and bsc#1125133) +Patch1: 0001-subprocess-usage-explicitly-pass-close_fds-True.patch +Patch2: 0002-state_file_dir-choose-safe-default-mode-make-mode-co.patch +Patch3: 0003-pytest-adjust-tests-to-new-state_file_dir_mode-confi.patch +Patch4: 0004-choose_tls_port-reuse-socket-and-explicitly-close-it.patch +Patch5: 0005-watchdog-be-robust-against-unrelated-localhost-based.patch BuildRequires: systemd BuildRequires: systemd-rpm-macros Requires: nfs-utils @@ -38,6 +45,11 @@ This package provides utilities for using the EFS file systems. %setup -n efs-utils-%{version} find . -name "*.py" -exec sed -i 's/env python/python3/' {} + %patch +%patch1 -p1 +%patch2 -p1 +%patch3 -p1 +%patch4 -p1 +%patch5 -p1 %build # No build required