Hello community,
here is the log from the commit of package aws-efs-utils for openSUSE:Factory checked in at 2019-04-04 12:07:59
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Comparing /work/SRC/openSUSE:Factory/aws-efs-utils (Old)
and /work/SRC/openSUSE:Factory/.aws-efs-utils.new.3908 (New)
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "aws-efs-utils"
Thu Apr 4 12:07:59 2019 rev:2 rq:691287 version:1.5
Changes:
--------
--- /work/SRC/openSUSE:Factory/aws-efs-utils/aws-efs-utils.changes 2019-02-28 21:39:30.429619356 +0100
+++ /work/SRC/openSUSE:Factory/.aws-efs-utils.new.3908/aws-efs-utils.changes 2019-04-04 12:08:06.897400144 +0200
@@ -1,0 +2,10 @@
+Wed Apr 3 08:38:34 UTC 2019 - John Paul Adrian Glaubitz
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 ++++++ 0002-state_file_dir-choose-safe-default-mode-make-mode-co.patch ++++++
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
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
`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 ++++++ 0003-pytest-adjust-tests-to-new-state_file_dir_mode-confi.patch ++++++ 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 ++++++ 0004-choose_tls_port-reuse-socket-and-explicitly-close-it.patch ++++++
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 ++++++ 0005-watchdog-be-robust-against-unrelated-localhost-based.patch ++++++
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