From c0d49d739d39573b59c827c89f56386d162d9381 Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Wed, 13 Mar 2019 18:44:24 +0000
Subject: [PATCH] Add fixes for handling swap file and other nit fixes (#1485)
RH-Author: Vitaly Kuznetsov <vkuznets@redhat.com>
Message-id: <20190313184424.29299-1-vkuznets@redhat.com>
Patchwork-id: 84860
O-Subject: [RHEL8 WALinuxAgent PATCH] Add fixes for handling swap file and other nit fixes (#1485)
Bugzilla: 1688276
RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>
RH-Acked-by: Mohammed Gamal <mgamal@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1684181
Brew: https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=20581233
Tested: by me
This is to fix CVE-2019-0804: swapfile is created with weak permission.
commit 8b2fa7d6051d0ee9952be4b42185c24d2a2eacff
Author: Varad Meru <vrdmr@users.noreply.github.com>
Date: Tue Mar 12 12:54:08 2019 -0700
Add fixes for handling swap file and other nit fixes (#1485)
* Add fixes for handling swap file and other nit fixes
* Fixing bytearray and other nits
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
Conflicts:
azurelinuxagent/daemon/resourcedisk/freebsd.py
(requires additional commits, irrelevant to RHEL)
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
---
azurelinuxagent/daemon/resourcedisk/default.py | 74 +++++++++++++++++++-------
azurelinuxagent/daemon/resourcedisk/freebsd.py | 53 ++++++++++++------
tests/distro/test_resourceDisk.py | 47 ++++++++++++++--
3 files changed, 133 insertions(+), 41 deletions(-)
diff --git a/azurelinuxagent/daemon/resourcedisk/default.py b/azurelinuxagent/daemon/resourcedisk/default.py
index 0f0925d..cfb76d2 100644
--- a/azurelinuxagent/daemon/resourcedisk/default.py
+++ b/azurelinuxagent/daemon/resourcedisk/default.py
@@ -17,6 +17,7 @@
import os
import re
+import stat
import sys
import threading
from time import sleep
@@ -124,12 +125,13 @@ class ResourceDiskHandler(object):
force_option = 'F'
if self.fs == 'xfs':
force_option = 'f'
- mkfs_string = "mkfs.{0} -{2} {1}".format(self.fs, partition, force_option)
+ mkfs_string = "mkfs.{0} -{2} {1}".format(
+ self.fs, partition, force_option)
if "gpt" in ret[1]:
logger.info("GPT detected, finding partitions")
parts = [x for x in ret[1].split("\n") if
- re.match("^\s*[0-9]+", x)]
+ re.match(r"^\s*[0-9]+", x)]
logger.info("Found {0} GPT partition(s).", len(parts))
if len(parts) > 1:
logger.info("Removing old GPT partitions")
@@ -138,18 +140,23 @@ class ResourceDiskHandler(object):
shellutil.run("parted {0} rm {1}".format(device, i))
logger.info("Creating new GPT partition")
- shellutil.run("parted {0} mkpart primary 0% 100%".format(device))
+ shellutil.run(
+ "parted {0} mkpart primary 0% 100%".format(device))
logger.info("Format partition [{0}]", mkfs_string)
shellutil.run(mkfs_string)
else:
logger.info("GPT not detected, determining filesystem")
- ret = self.change_partition_type(suppress_message=True, option_str="{0} 1 -n".format(device))
+ ret = self.change_partition_type(
+ suppress_message=True,
+ option_str="{0} 1 -n".format(device))
ptype = ret[1].strip()
if ptype == "7" and self.fs != "ntfs":
logger.info("The partition is formatted with ntfs, updating "
"partition type to 83")
- self.change_partition_type(suppress_message=False, option_str="{0} 1 83".format(device))
+ self.change_partition_type(
+ suppress_message=False,
+ option_str="{0} 1 83".format(device))
self.reread_partition_table(device)
logger.info("Format partition [{0}]", mkfs_string)
shellutil.run(mkfs_string)
@@ -169,7 +176,8 @@ class ResourceDiskHandler(object):
attempts -= 1
if not os.path.exists(partition):
- raise ResourceDiskError("Partition was not created [{0}]".format(partition))
+ raise ResourceDiskError(
+ "Partition was not created [{0}]".format(partition))
logger.info("Mount resource disk [{0}]", mount_string)
ret, output = shellutil.run_get_output(mount_string, chk_err=False)
@@ -215,14 +223,19 @@ class ResourceDiskHandler(object):
"""
command_to_use = '--part-type'
- input = "sfdisk {0} {1} {2}".format(command_to_use, '-f' if suppress_message else '', option_str)
- err_code, output = shellutil.run_get_output(input, chk_err=False, log_cmd=True)
+ input = "sfdisk {0} {1} {2}".format(
+ command_to_use, '-f' if suppress_message else '', option_str)
+ err_code, output = shellutil.run_get_output(
+ input, chk_err=False, log_cmd=True)
# fall back to -c
if err_code != 0:
- logger.info("sfdisk with --part-type failed [{0}], retrying with -c", err_code)
+ logger.info(
+ "sfdisk with --part-type failed [{0}], retrying with -c",
+ err_code)
command_to_use = '-c'
- input = "sfdisk {0} {1} {2}".format(command_to_use, '-f' if suppress_message else '', option_str)
+ input = "sfdisk {0} {1} {2}".format(
+ command_to_use, '-f' if suppress_message else '', option_str)
err_code, output = shellutil.run_get_output(input, log_cmd=True)
if err_code == 0:
@@ -245,16 +258,30 @@ class ResourceDiskHandler(object):
else:
return 'mount {0} {1}'.format(partition, mount_point)
+ @staticmethod
+ def check_existing_swap_file(swapfile, swaplist, size):
+ if swapfile in swaplist and os.path.isfile(
+ swapfile) and os.path.getsize(swapfile) == size:
+ logger.info("Swap already enabled")
+ # restrict access to owner (remove all access from group, others)
+ swapfile_mode = os.stat(swapfile).st_mode
+ if swapfile_mode & (stat.S_IRWXG | stat.S_IRWXO):
+ swapfile_mode = swapfile_mode & ~(stat.S_IRWXG | stat.S_IRWXO)
+ logger.info(
+ "Changing mode of {0} to {1:o}".format(
+ swapfile, swapfile_mode))
+ os.chmod(swapfile, swapfile_mode)
+ return True
+
+ return False
+
def create_swap_space(self, mount_point, size_mb):
size_kb = size_mb * 1024
size = size_kb * 1024
swapfile = os.path.join(mount_point, 'swapfile')
swaplist = shellutil.run_get_output("swapon -s")[1]
- if swapfile in swaplist \
- and os.path.isfile(swapfile) \
- and os.path.getsize(swapfile) == size:
- logger.info("Swap already enabled")
+ if self.check_existing_swap_file(swapfile, swaplist, size):
return
if os.path.isfile(swapfile) and os.path.getsize(swapfile) != size:
@@ -296,7 +323,8 @@ class ResourceDiskHandler(object):
os.remove(filename)
# If file system is xfs, use dd right away as we have been reported that
- # swap enabling fails in xfs fs when disk space is allocated with fallocate
+ # swap enabling fails in xfs fs when disk space is allocated with
+ # fallocate
ret = 0
fn_sh = shellutil.quote((filename,))
if self.fs != 'xfs':
@@ -305,13 +333,21 @@ class ResourceDiskHandler(object):
# Probable errors:
# - OSError: Seen on Cygwin, libc notimpl?
# - AttributeError: What if someone runs this under...
+ fd = None
+
try:
- with open(filename, 'w') as f:
- os.posix_fallocate(f.fileno(), 0, nbytes)
- return 0
- except:
+ fd = os.open(
+ filename,
+ os.O_CREAT | os.O_WRONLY | os.O_EXCL,
+ stat.S_IRUSR | stat.S_IWUSR)
+ os.posix_fallocate(fd, 0, nbytes)
+ return 0
+ except BaseException:
# Not confident with this thing, just keep trying...
pass
+ finally:
+ if fd is not None:
+ os.close(fd)
# fallocate command
ret = shellutil.run(
diff --git a/azurelinuxagent/daemon/resourcedisk/freebsd.py b/azurelinuxagent/daemon/resourcedisk/freebsd.py
index a65d7f8..a29df3a 100644
--- a/azurelinuxagent/daemon/resourcedisk/freebsd.py
+++ b/azurelinuxagent/daemon/resourcedisk/freebsd.py
@@ -22,6 +22,7 @@ import azurelinuxagent.common.utils.shellutil as shellutil
from azurelinuxagent.common.exception import ResourceDiskError
from azurelinuxagent.daemon.resourcedisk.default import ResourceDiskHandler
+
class FreeBSDResourceDiskHandler(ResourceDiskHandler):
"""
This class handles resource disk mounting for FreeBSD.
@@ -34,6 +35,7 @@ class FreeBSDResourceDiskHandler(ResourceDiskHandler):
1. MBR: The resource disk partition is /dev/da1s1
2. GPT: The resource disk partition is /dev/da1p2, /dev/da1p1 is for reserved usage.
"""
+
def __init__(self):
super(FreeBSDResourceDiskHandler, self).__init__()
@@ -50,25 +52,30 @@ class FreeBSDResourceDiskHandler(ResourceDiskHandler):
def mount_resource_disk(self, mount_point):
fs = self.fs
if fs != 'ufs':
- raise ResourceDiskError("Unsupported filesystem type:{0}, only ufs is supported.".format(fs))
+ raise ResourceDiskError(
+ "Unsupported filesystem type:{0}, only ufs is supported.".format(fs))
# 1. Detect device
err, output = shellutil.run_get_output('gpart list')
if err:
- raise ResourceDiskError("Unable to detect resource disk device:{0}".format(output))
+ raise ResourceDiskError(
+ "Unable to detect resource disk device:{0}".format(output))
disks = self.parse_gpart_list(output)
device = self.osutil.device_for_ide_port(1)
- if device is None or not device in disks:
- # fallback logic to find device
- err, output = shellutil.run_get_output('camcontrol periphlist 2:1:0')
+ if device is None or device not in disks:
+ # fallback logic to find device
+ err, output = shellutil.run_get_output(
+ 'camcontrol periphlist 2:1:0')
if err:
# try again on "3:1:0"
- err, output = shellutil.run_get_output('camcontrol periphlist 3:1:0')
+ err, output = shellutil.run_get_output(
+ 'camcontrol periphlist 3:1:0')
if err:
- raise ResourceDiskError("Unable to detect resource disk device:{0}".format(output))
+ raise ResourceDiskError(
+ "Unable to detect resource disk device:{0}".format(output))
- # 'da1: generation: 4 index: 1 status: MORE\npass2: generation: 4 index: 2 status: LAST\n'
+ # 'da1: generation: 4 index: 1 status: MORE\npass2: generation: 4 index: 2 status: LAST\n'
for line in output.split('\n'):
index = line.find(':')
if index > 0:
@@ -89,9 +96,11 @@ class FreeBSDResourceDiskHandler(ResourceDiskHandler):
elif partition_table_type == 'GPT':
provider_name = device + 'p2'
else:
- raise ResourceDiskError("Unsupported partition table type:{0}".format(output))
+ raise ResourceDiskError(
+ "Unsupported partition table type:{0}".format(output))
- err, output = shellutil.run_get_output('gpart show -p {0}'.format(device))
+ err, output = shellutil.run_get_output(
+ 'gpart show -p {0}'.format(device))
if err or output.find(provider_name) == -1:
raise ResourceDiskError("Resource disk partition not found.")
@@ -110,14 +119,24 @@ class FreeBSDResourceDiskHandler(ResourceDiskHandler):
mount_cmd = 'mount -t {0} {1} {2}'.format(fs, partition, mount_point)
err = shellutil.run(mount_cmd, chk_err=False)
if err:
- logger.info('Creating {0} filesystem on partition {1}'.format(fs, partition))
- err, output = shellutil.run_get_output('newfs -U {0}'.format(partition))
+ logger.info(
+ 'Creating {0} filesystem on partition {1}'.format(
+ fs, partition))
+ err, output = shellutil.run_get_output(
+ 'newfs -U {0}'.format(partition))
if err:
- raise ResourceDiskError("Failed to create new filesystem on partition {0}, error:{1}"
- .format(partition, output))
+ raise ResourceDiskError(
+ "Failed to create new filesystem on partition {0}, error:{1}" .format(
+ partition, output))
err, output = shellutil.run_get_output(mount_cmd, chk_err=False)
if err:
- raise ResourceDiskError("Failed to mount partition {0}, error {1}".format(partition, output))
-
- logger.info("Resource disk partition {0} is mounted at {1} with fstype {2}", partition, mount_point, fs)
+ raise ResourceDiskError(
+ "Failed to mount partition {0}, error {1}".format(
+ partition, output))
+
+ logger.info(
+ "Resource disk partition {0} is mounted at {1} with fstype {2}",
+ partition,
+ mount_point,
+ fs)
return mount_point
diff --git a/tests/distro/test_resourceDisk.py b/tests/distro/test_resourceDisk.py
index d2ce6e1..5f9db0a 100644
--- a/tests/distro/test_resourceDisk.py
+++ b/tests/distro/test_resourceDisk.py
@@ -18,6 +18,8 @@
# http://msdn.microsoft.com/en-us/library/cc227282%28PROT.10%29.aspx
# http://msdn.microsoft.com/en-us/library/cc227259%28PROT.13%29.aspx
+import os
+import stat
import sys
from azurelinuxagent.common.utils import shellutil
from azurelinuxagent.daemon.resourcedisk import get_resourcedisk_handler
@@ -38,6 +40,11 @@ class TestResourceDisk(AgentTestCase):
# assert
assert os.path.exists(test_file)
+ # only the owner should have access
+ mode = os.stat(test_file).st_mode & (
+ stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO)
+ assert mode == stat.S_IRUSR | stat.S_IWUSR
+
# cleanup
os.remove(test_file)
@@ -49,7 +56,7 @@ class TestResourceDisk(AgentTestCase):
file_size = 1024 * 128
# execute
- if sys.version_info >= (3,3):
+ if sys.version_info >= (3, 3):
with patch("os.posix_fallocate",
side_effect=Exception('failure')):
get_resourcedisk_handler().mkfile(test_file, file_size)
@@ -76,20 +83,20 @@ class TestResourceDisk(AgentTestCase):
resource_disk_handler.mkfile(test_file, file_size)
# assert
- if sys.version_info >= (3,3):
+ if sys.version_info >= (3, 3):
with patch("os.posix_fallocate") as posix_fallocate:
self.assertEqual(0, posix_fallocate.call_count)
assert run_patch.call_count == 1
assert "dd if" in run_patch.call_args_list[0][0][0]
-
def test_change_partition_type(self):
resource_handler = get_resourcedisk_handler()
# test when sfdisk --part-type does not exist
with patch.object(shellutil, "run_get_output",
side_effect=[[1, ''], [0, '']]) as run_patch:
- resource_handler.change_partition_type(suppress_message=True, option_str='')
+ resource_handler.change_partition_type(
+ suppress_message=True, option_str='')
# assert
assert run_patch.call_count == 2
@@ -99,12 +106,42 @@ class TestResourceDisk(AgentTestCase):
# test when sfdisk --part-type exists
with patch.object(shellutil, "run_get_output",
side_effect=[[0, '']]) as run_patch:
- resource_handler.change_partition_type(suppress_message=True, option_str='')
+ resource_handler.change_partition_type(
+ suppress_message=True, option_str='')
# assert
assert run_patch.call_count == 1
assert "sfdisk --part-type" in run_patch.call_args_list[0][0][0]
+ def test_check_existing_swap_file(self):
+ test_file = os.path.join(self.tmp_dir, 'test_swap_file')
+ file_size = 1024 * 128
+ if os.path.exists(test_file):
+ os.remove(test_file)
+
+ with open(test_file, "wb") as file:
+ file.write(bytearray(file_size))
+
+ os.chmod(test_file, stat.S_ISUID | stat.S_ISGID | stat.S_IRUSR |
+ stat.S_IWUSR | stat.S_IRWXG | stat.S_IRWXO) # 0o6677
+
+ def swap_on(_): # mimic the output of "swapon -s"
+ return [
+ "Filename Type Size Used Priority",
+ "{0} partition 16498684 0 -2".format(test_file)
+ ]
+
+ with patch.object(shellutil, "run_get_output", side_effect=swap_on):
+ get_resourcedisk_handler().check_existing_swap_file(
+ test_file, test_file, file_size)
+
+ # it should remove access from group, others
+ mode = os.stat(test_file).st_mode & (stat.S_ISUID | stat.S_ISGID |
+ stat.S_IRWXU | stat.S_IWUSR | stat.S_IRWXG | stat.S_IRWXO) # 0o6777
+ assert mode == stat.S_ISUID | stat.S_ISGID | stat.S_IRUSR | stat.S_IWUSR # 0o6600
+
+ os.remove(test_file)
+
if __name__ == '__main__':
unittest.main()
--
1.8.3.1