Mailinglist Archive: opensuse-commit (1903 mails)

< Previous Next >
commit aws-efs-utils for openSUSE:Factory
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
<adrian.glaubitz@xxxxxxxx>
+
+- 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
+
+-------------------------------------------------------------------

New:
----
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

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Other differences:
------------------
++++++ aws-efs-utils.spec ++++++
--- /var/tmp/diff_new_pack.TQM2CR/_old 2019-04-04 12:08:08.481398642 +0200
+++ /var/tmp/diff_new_pack.TQM2CR/_new 2019-04-04 12:08:08.485398638 +0200
@@ -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 @@
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 @@
%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

++++++ 0001-subprocess-usage-explicitly-pass-close_fds-True.patch ++++++
From fbd8d90c88ee26e6020bae0983db7214464a4c46 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@xxxxxxx>
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 <matthias.gerstner@xxxxxxx>
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

++++++ 0003-pytest-adjust-tests-to-new-state_file_dir_mode-confi.patch ++++++
From ba525f6b6a41ff9df720e073df9e5bed4c59c360 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@xxxxxxx>
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

++++++ 0004-choose_tls_port-reuse-socket-and-explicitly-close-it.patch ++++++
From 9a8526d26596b3d2e839841fa44b601069828fa9 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner@xxxxxxx>
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 <matthias.gerstner@xxxxxxx>
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


< Previous Next >
This Thread