1
0
forked from pool/aws-efs-utils

Accepting request 692718 from Cloud:Tools

- Update to version 1.7
  + subprocess usage: explicitly pass `close_fds = True`
  + state_file_dir: choose safe default mode, make mode configurable
  + choose_tls_port(): reuse socket and explicitly close it in all cases
  + watchdog: be robust against unrelated localhost based nfs mounts
- Drop hardening patches merged upstream
  + 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
- from version 1.6
  + fix for additional unexpected arguments
  + add test for additional unexpected arguments

OBS-URL: https://build.opensuse.org/request/show/692718
OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/aws-efs-utils?expand=0&rev=3
This commit is contained in:
Dominique Leuenberger 2019-04-11 06:47:58 +00:00 committed by Git OBS Bridge
commit 1efbd179f6
10 changed files with 42 additions and 319 deletions

View File

@ -1,96 +0,0 @@
From fbd8d90c88ee26e6020bae0983db7214464a4c46 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@suse.de>
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

View File

@ -1,64 +0,0 @@
From 8ea6a3ada710cf833dac549b973d09512bce1b78 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@suse.de>
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

View File

@ -1,34 +0,0 @@
From ba525f6b6a41ff9df720e073df9e5bed4c59c360 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@suse.de>
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

View File

@ -1,49 +0,0 @@
From 9a8526d26596b3d2e839841fa44b601069828fa9 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@suse.de>
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

View File

@ -1,45 +0,0 @@
From abe6750a7ee39852b011faa0791963d854788984 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@suse.de>
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

View File

@ -1,3 +1,21 @@
-------------------------------------------------------------------
Tue Apr 9 22:29:17 UTC 2019 - John Paul Adrian Glaubitz <adrian.glaubitz@suse.com>
- Update to version 1.7
+ subprocess usage: explicitly pass `close_fds = True`
+ state_file_dir: choose safe default mode, make mode configurable
+ choose_tls_port(): reuse socket and explicitly close it in all cases
+ watchdog: be robust against unrelated localhost based nfs mounts
- Drop hardening patches merged upstream
+ 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
- from version 1.6
+ fix for additional unexpected arguments
+ add test for additional unexpected arguments
------------------------------------------------------------------- -------------------------------------------------------------------
Wed Apr 3 08:38:34 UTC 2019 - John Paul Adrian Glaubitz <adrian.glaubitz@suse.com> Wed Apr 3 08:38:34 UTC 2019 - John Paul Adrian Glaubitz <adrian.glaubitz@suse.com>

View File

@ -17,7 +17,7 @@
Name: aws-efs-utils Name: aws-efs-utils
Version: 1.5 Version: 1.7
Release: 0 Release: 0
Summary: Utilities for using the EFS file systems Summary: Utilities for using the EFS file systems
License: MIT License: MIT
@ -25,12 +25,6 @@ Group: System/Management
Url: https://github.com/aws/efs-utils Url: https://github.com/aws/efs-utils
Source0: https://github.com/aws/efs-utils/archive/v%{version}.tar.gz Source0: https://github.com/aws/efs-utils/archive/v%{version}.tar.gz
Patch: efs-switchparser.patch 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
BuildRequires: systemd-rpm-macros BuildRequires: systemd-rpm-macros
Requires: nfs-utils Requires: nfs-utils
@ -44,12 +38,7 @@ This package provides utilities for using the EFS file systems.
%prep %prep
%setup -n efs-utils-%{version} %setup -n efs-utils-%{version}
find . -name "*.py" -exec sed -i 's/env python/python3/' {} + find . -name "*.py" -exec sed -i 's/env python/python3/' {} +
%patch %patch -p1
%patch1 -p1
%patch2 -p1
%patch3 -p1
%patch4 -p1
%patch5 -p1
%build %build
# No build required # No build required

View File

@ -1,6 +1,7 @@
--- src/mount_efs/__init__.py.orig diff -Nru efs-utils-1.7.orig/src/mount_efs/__init__.py efs-utils-1.7/src/mount_efs/__init__.py
+++ src/mount_efs/__init__.py --- efs-utils-1.7.orig/src/mount_efs/__init__.py 2019-04-09 20:27:34.000000000 +0200
@@ -44,9 +44,9 @@ from contextlib import contextmanager +++ efs-utils-1.7/src/mount_efs/__init__.py 2019-04-09 23:59:43.477327640 +0200
@@ -44,9 +44,9 @@
from logging.handlers import RotatingFileHandler from logging.handlers import RotatingFileHandler
try: try:
@ -12,7 +13,7 @@
try: try:
from urllib2 import urlopen, URLError from urllib2 import urlopen, URLError
@@ -517,7 +517,7 @@ def assert_root(): @@ -537,7 +537,7 @@
def read_config(config_file=CONFIG_FILE): def read_config(config_file=CONFIG_FILE):
@ -21,9 +22,10 @@
p.read(config_file) p.read(config_file)
return p return p
--- src/watchdog/__init__.py.orig diff -Nru efs-utils-1.7.orig/src/watchdog/__init__.py efs-utils-1.7/src/watchdog/__init__.py
+++ src/watchdog/__init__.py --- efs-utils-1.7.orig/src/watchdog/__init__.py 2019-04-09 20:27:34.000000000 +0200
@@ -21,9 +21,9 @@ from logging.handlers import RotatingFil +++ efs-utils-1.7/src/watchdog/__init__.py 2019-04-09 23:59:43.477327640 +0200
@@ -21,9 +21,9 @@
from signal import SIGTERM from signal import SIGTERM
try: try:
@ -33,9 +35,9 @@
- from configparser import ConfigParser - from configparser import ConfigParser
+ import configparser as cp + import configparser as cp
VERSION = '1.5' VERSION = '1.7'
@@ -275,7 +275,7 @@ def assert_root(): @@ -280,7 +280,7 @@
def read_config(config_file=CONFIG_FILE): def read_config(config_file=CONFIG_FILE):
@ -44,8 +46,9 @@
p.read(config_file) p.read(config_file)
return p return p
--- test/mount_efs_test/test_choose_tls_port.py.orig diff -Nru efs-utils-1.7.orig/test/mount_efs_test/test_choose_tls_port.py efs-utils-1.7/test/mount_efs_test/test_choose_tls_port.py
+++ test/mount_efs_test/test_choose_tls_port.py --- efs-utils-1.7.orig/test/mount_efs_test/test_choose_tls_port.py 2019-04-09 20:27:34.000000000 +0200
+++ efs-utils-1.7/test/mount_efs_test/test_choose_tls_port.py 2019-04-09 23:59:43.477327640 +0200
@@ -7,9 +7,13 @@ @@ -7,9 +7,13 @@
# #
@ -61,7 +64,7 @@
import pytest import pytest
from mock import MagicMock from mock import MagicMock
@@ -19,7 +23,7 @@ DEFAULT_TLS_PORT_RANGE_HIGH = 20449 @@ -19,7 +23,7 @@
def _get_config(): def _get_config():
@ -70,8 +73,9 @@
config.add_section(mount_efs.CONFIG_SECTION) config.add_section(mount_efs.CONFIG_SECTION)
config.set(mount_efs.CONFIG_SECTION, 'port_range_lower_bound', str(DEFAULT_TLS_PORT_RANGE_LOW)) config.set(mount_efs.CONFIG_SECTION, 'port_range_lower_bound', str(DEFAULT_TLS_PORT_RANGE_LOW))
config.set(mount_efs.CONFIG_SECTION, 'port_range_upper_bound', str(DEFAULT_TLS_PORT_RANGE_HIGH)) config.set(mount_efs.CONFIG_SECTION, 'port_range_upper_bound', str(DEFAULT_TLS_PORT_RANGE_HIGH))
--- test/mount_efs_test/test_write_stunnel_config_file.py.orig diff -Nru efs-utils-1.7.orig/test/mount_efs_test/test_write_stunnel_config_file.py efs-utils-1.7/test/mount_efs_test/test_write_stunnel_config_file.py
+++ test/mount_efs_test/test_write_stunnel_config_file.py --- efs-utils-1.7.orig/test/mount_efs_test/test_write_stunnel_config_file.py 2019-04-09 20:27:34.000000000 +0200
+++ efs-utils-1.7/test/mount_efs_test/test_write_stunnel_config_file.py 2019-04-09 23:59:43.477327640 +0200
@@ -7,9 +7,13 @@ @@ -7,9 +7,13 @@
# #
@ -87,7 +91,7 @@
import pytest import pytest
FS_ID = 'fs-deadbeef' FS_ID = 'fs-deadbeef'
@@ -32,7 +36,7 @@ def _get_config(mocker, stunnel_debug_en @@ -32,7 +36,7 @@
if stunnel_check_cert_validity is None: if stunnel_check_cert_validity is None:
stunnel_check_cert_validity = stunnel_check_cert_validity_supported stunnel_check_cert_validity = stunnel_check_cert_validity_supported

View File

@ -1,3 +0,0 @@
version https://git-lfs.github.com/spec/v1
oid sha256:f8796fed18f6941f1eb00d946099daeb10ced2202c53abdee2d6bb14427d9406
size 29114

3
v1.7.tar.gz Normal file
View File

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:b9374777da47e1c6be8042546a6139e1d9a21dfca87f37aefd9f78d4a486cf7a
size 29384