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