CVE-2022-23948 (Python, published 2022-09-21T19:15Z)
Description: value: A flaw was found in Keylime before 6.3.0. The logic in the Keylime agent for checking for a secure mount can be fooled by previously created unprivileged mounts allowing secrets to be leaked to other processes on the host.
Severity: nan (CVSS v3 base score: 7.5)
Weakness: NVD-CWE-noinfo \u2014 Insufficient Information
There is insufficient information about the issue to classify it; details are unkown or unspecified.
Repository: https://github.com/keylime/keylime
Files changed:
keylime/keylime_agent.py
keylime/secure_mount.py
test/test_secure_mount.py
Commit message: secure_mount: add umount function

The `mount` function is called several times in different places in the
code, but it takes care of not mounting the secure directory if is
already present.  The problem is that this is never umounted.

This patch add the `umount` function, and update the `mount` one to
track when a mount point needs to be umounted.

Also adjust the keylime_agent exit point to make sure that the `umount`
function is called at the end.

Signed-off-by: Alberto Planas <aplanas@suse.com>


Vulnerable code:
        logger.debug("...HTTP server stopped")
        logger.debug("... revocation notifier stopped")
    for line in open("/proc/self/mountinfo", "r"):
    secdir = config.WORK_DIR + "/secure"
        secdir = config.WORK_DIR + "/tmpfs-dev"
    def test_check_mounted_not_found(self, open_mock, logger_mock):
            )
    def test_check_mounted_found(self, open_mock, logger_mock):
            )
    def test_check_mounted_found_zero_optional_fields(self, open_mock, logger_mock):
    def test_check_mounted_found_extra_optional_fields(self, open_mock, logger_mock):
    def test_check_mounted_found_wrong_fs(self, open_mock, logger_mock):
            )
    def test_check_mounted_found_spaces(self, open_mock, logger_mock):
            )
    def test_check_mounted_wrong_format(self, open_mock, logger_mock):

Fixed code:
        logger.debug("HTTP server stopped...")
        logger.debug("Revocation notifier stopped...")
        secure_mount.umount()
        logger.debug("Umounting directories...")
# Store the mounted directories done by Keylime, so we can unmount
# them in reverse order
_MOUNTED = []

    for line in open("/proc/self/mountinfo", "r", encoding="utf-8"):
            # pylint: disable=raise-missing-from
    secdir = os.path.join(config.WORK_DIR, "secure")
        secdir = os.path.join(config.WORK_DIR, "tmpfs-dev")
        _MOUNTED.append(secdir)


def umount():
    """Umount all the devices mounted by Keylime."""
    while _MOUNTED:
        directory = _MOUNTED.pop()
        logger.info("Unmounting %s", directory)
        if check_mounted(directory):
            cmd = ("umount", directory)
            ret = cmd_exec.run(cmd, raiseOnError=False)
            if ret["code"] != 0:
                logger.error("%s cannot be umounted. "
                             "A running process can be keeping it bussy: %s",
                             directory, str(ret["reterr"]))
        else:
            logger.warning("%s already unmounted by another process", directory)
import tempfile
    def setUp(self):
        """Remove global state from secure_mount module."""
        # pylint: disable=protected-access
        secure_mount._MOUNTED = []
    def test_check_mounted_not_found(self, open_mock, _logger_mock):
        )
    def test_check_mounted_found(self, open_mock, _logger_mock):
        )
    def test_check_mounted_found_zero_optional_fields(self, open_mock, _logger_mock):
    def test_check_mounted_found_extra_optional_fields(self, open_mock, _logger_mock):
    def test_check_mounted_found_wrong_fs(self, open_mock, _logger_mock):
        )
    def test_check_mounted_found_spaces(self, open_mock, _logger_mock):
        )
    def test_check_mounted_wrong_format(self, open_mock, _logger_mock):
    @patch("keylime.secure_mount.logger")
    @patch("keylime.secure_mount.config")
    def test_check_mount_no_secure(self, config_mock, _logger_mock):
        """Test when mounted outside tmpfs."""
        config_mock.MOUNT_SECURE = False

        with tempfile.TemporaryDirectory() as tmpdirname:
            config_mock.WORK_DIR = tmpdirname
            self.assertEqual(secure_mount.mount(), f"{tmpdirname}/tmpfs-dev")
            # pylint: disable=protected-access
            self.assertEqual(secure_mount._MOUNTED, [])

    @patch("keylime.secure_mount.check_mounted")
    @patch("keylime.secure_mount.config")
    def test_check_mount_secure_already_mounted(self, config_mock, check_mounted_mock):
        """Test when mounting in tmpfs but is already present."""
        config_mock.MOUNT_SECURE = True
        check_mounted_mock.return_value = True

        with tempfile.TemporaryDirectory() as tmpdirname:
            config_mock.WORK_DIR = tmpdirname
            self.assertEqual(secure_mount.mount(), f"{tmpdirname}/secure")
            # pylint: disable=protected-access
            self.assertEqual(secure_mount._MOUNTED, [])

    @patch("keylime.secure_mount.logger")
    @patch("keylime.secure_mount.check_mounted")
    @patch("keylime.secure_mount.config")
    @patch("keylime.secure_mount.os.path.exists")
    @patch("keylime.secure_mount.cmd_exec")
    def test_check_mount_secure_already_created(
        self, _cmd_exec_mock, exists_mock, config_mock, check_mounted_mock, _logger_mock
    ):
        """Test when mounting in tmpfs but the mount point is present."""
        exists_mock.return_value = True
        config_mock.MOUNT_SECURE = True
        check_mounted_mock.return_value = False

        with tempfile.TemporaryDirectory() as tmpdirname:
            config_mock.WORK_DIR = tmpdirname
            self.assertEqual(secure_mount.mount(), f"{tmpdirname}/secure")
            # pylint: disable=protected-access
            self.assertEqual(secure_mount._MOUNTED, [f"{tmpdirname}/secure"])

    @patch("keylime.secure_mount.logger")
    @patch("keylime.secure_mount.check_mounted")
    @patch("keylime.secure_mount.config")
    @patch("keylime.secure_mount.os.path.exists")
    @patch("keylime.secure_mount.os.makedirs")
    @patch("keylime.secure_mount.cmd_exec")
    def test_check_mount(
        self,
        _cmd_exec_mock,
        _makedirs_mock,
        exists_mock,
        config_mock,
        check_mounted_mock,
        _logger_mock,
    ):
        """Test when mounting in tmpfs but the mount point is not present."""
        exists_mock.return_value = False
        config_mock.MOUNT_SECURE = True
        check_mounted_mock.return_value = False

        with tempfile.TemporaryDirectory() as tmpdirname:
            config_mock.WORK_DIR = tmpdirname
            self.assertEqual(secure_mount.mount(), f"{tmpdirname}/secure")
            # pylint: disable=protected-access
            self.assertEqual(secure_mount._MOUNTED, [f"{tmpdirname}/secure"])

    def test_check_umount_empty(self):
        """Test umount when there are nothing to clean."""
        secure_mount.umount()
        # pylint: disable=protected-access
        self.assertEqual(secure_mount._MOUNTED, [])

    @patch("keylime.secure_mount.logger")
    @patch("keylime.secure_mount.check_mounted")
    @patch("keylime.secure_mount.cmd_exec")
    def test_check_umount_mount(self, cmd_exec_mock, check_mounted_mock, logger_mock):
        """Test umount for a single mount."""
        # pylint: disable=protected-access
        secure_mount._MOUNTED = ["/secdir"]
        check_mounted_mock.return_value = True
        cmd_exec_mock.run.return_value = {"code": 0}
        secure_mount.umount()
        self.assertEqual(secure_mount._MOUNTED, [])
        cmd_exec_mock.run.assert_called_once()
        logger_mock.error.assert_not_called()
        logger_mock.warning.assert_not_called()

    @patch("keylime.secure_mount.logger")
    @patch("keylime.secure_mount.check_mounted")
    @patch("keylime.secure_mount.cmd_exec")
    def test_check_umount_mount_busy(
        self, cmd_exec_mock, check_mounted_mock, logger_mock
    ):
        """Test umount for a single mount that fail."""
        # pylint: disable=protected-access
        secure_mount._MOUNTED = ["/secdir"]
        check_mounted_mock.return_value = True
        cmd_exec_mock.run.return_value = {"code": 1, "reterr": "Device busy"}
        secure_mount.umount()
        self.assertEqual(secure_mount._MOUNTED, [])
        cmd_exec_mock.run.assert_called_once()
        logger_mock.error.assert_called_once()
        logger_mock.warning.assert_not_called()

    @patch("keylime.secure_mount.logger")
    @patch("keylime.secure_mount.check_mounted")
    @patch("keylime.secure_mount.cmd_exec")
    def test_check_umount_umount(self, cmd_exec_mock, check_mounted_mock, logger_mock):
        """Test umount for a single umount and no creation."""
        # pylint: disable=protected-access
        secure_mount._MOUNTED = ["/secdir"]
        check_mounted_mock.return_value = False
        secure_mount.umount()
        self.assertEqual(secure_mount._MOUNTED, [])
        cmd_exec_mock.run.assert_not_called()
        logger_mock.error.assert_not_called()
        logger_mock.warning.assert_called_once()