From c0d49d739d39573b59c827c89f56386d162d9381 Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov 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 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 RH-Acked-by: Mohammed Gamal 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 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 Conflicts: azurelinuxagent/daemon/resourcedisk/freebsd.py (requires additional commits, irrelevant to RHEL) Signed-off-by: Vitaly Kuznetsov Signed-off-by: Danilo C. L. de Paula --- 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