ykohut/leapp-repository
1
0
Fork 0
forked from rpms/leapp-repository
Code Pull Requests Activity
254cdd4a81
leapp-repository/0032-Fix-certificate-symlink-handling.patch
Petr Stodulka 75c9028095 RHEL 8.10: CTC1 candidate 
- Enable new upgrade path for RHEL 8.10 -> RHEL 9.4 (including RHEL with SAP HANA)
- Introduce generic transition of systemd services states during the IPU
- Introduce possibility to upgrade with local repositories
- Improve possibilities of upgrade when a proxy is configured in DNF configutation file
- Fix handling of symlinks under /etc/pki when managing certificates
- Fix the upgrade with custom https repositories
- Default to the NO_RHSM mode when subscription-manager is not installed
- Detect customized configuration of dynamic linker
- Drop the invalid `tuv` target channel for the --channel option
- Fix the issue of going out of bounds in the isccfg parser
- Fix traceback when saving the rhsm facts results and the /etc/rhsm/facts directory doesn’t exist yet
- Load all rpm repository substitutions that dnf knows about, not just "releasever" only
- Simplify handling of upgrades on systems using RHUI, reducing the maintenance burden for cloud providers
- Detect possible unexpected RPM GPG keys has been installed during RPM transaction
- Resolves: RHEL-16729
2023-11-16 20:15:43 +01:00

456 lines
17 KiB
Diff
Raw Blame History

From 5202c9b126c06057e9145b4b7e02afe50c1f879d Mon Sep 17 00:00:00 2001
From: David Kubek <dkubek@redhat.com>
Date: Tue, 24 Oct 2023 11:49:16 +0200
Subject: [PATCH 32/38] Fix certificate symlink handling
In response to the identified flaws in the originally delivered fix, for
feature enabling http repositories, this commit addresses the following
issues:
1. Previously, files installed via RPMs that were originally symlinks
were being switched to standard files. This issue has been resolved
by preserving symlinks within the /etc/pki directory. Any symlink
pointing to a file within the /etc/pki directory (whether present in
the source system or installed by a package in the container) will be
present in the container, ensuring changes to certificates are
properly propagated.
2. Lists of trusted CAs were not being updated, as the update-ca-trust
call was missing inside the container. This commit now includes the
necessary update-ca-trust call.
The solution specification has been modified as follows:
- Certificate _files_ in /etc/pki (excluding symlinks) are copied to
the container as in the original solution.
- Files installed by packages within the container are preserved and
given higher priority.
- Handling of symlinks is enhanced, ensuring that symlinks within
the /etc/pki directory are preserved, while any symlink pointing
outside the /etc/pki directory will be copied as a file.
- Certificates are updated using `update-ca-trust`.
---
.../libraries/userspacegen.py | 124 ++++++++--
.../tests/unit_test_targetuserspacecreator.py | 224 ++++++++++++++++++
2 files changed, 332 insertions(+), 16 deletions(-)
diff --git a/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py
index 039b99a5..050ad7fe 100644
--- a/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py
+++ b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py
@@ -331,12 +331,80 @@ def _get_files_owned_by_rpms(context, dirpath, pkgs=None, recursive=False):
return files_owned_by_rpms
+def _copy_decouple(srcdir, dstdir):
+ """
+ Copy `srcdir` to `dstdir` while decoupling symlinks.
+
+ What we mean by decoupling the `srcdir` is that any symlinks pointing
+ outside the directory will be copied as regular files. This means that the
+ directory will become independent from its surroundings with respect to
+ symlinks. Any symlink (or symlink chains) within the directory will be
+ preserved.
+
+ """
+
+ for root, dummy_dirs, files in os.walk(srcdir):
+ for filename in files:
+ relpath = os.path.relpath(root, srcdir)
+ source_filepath = os.path.join(root, filename)
+ target_filepath = os.path.join(dstdir, relpath, filename)
+
+ # Skip and report broken symlinks
+ if not os.path.exists(source_filepath):
+ api.current_logger().warning(
+ 'File {} is a broken symlink! Will not copy the file.'.format(source_filepath))
+ continue
+
+ # Copy symlinks to the target userspace
+ source_is_symlink = os.path.islink(source_filepath)
+ pointee = None
+ if source_is_symlink:
+ pointee = os.readlink(source_filepath)
+
+ # If source file is a symlink within `srcdir` then preserve it,
+ # otherwise resolve and copy it as a file it points to
+ if pointee is not None and not pointee.startswith(srcdir):
+ # Follow the path until we hit a file or get back to /etc/pki
+ while not pointee.startswith(srcdir) and os.path.islink(pointee):
+ pointee = os.readlink(pointee)
+
+ # Pointee points to a _regular file_ outside /etc/pki so we
+ # copy it instead
+ if not pointee.startswith(srcdir) and not os.path.islink(pointee):
+ source_is_symlink = False
+ source_filepath = pointee
+ else:
+ # pointee points back to /etc/pki
+ pass
+
+ # Ensure parent directory exists
+ parent_dir = os.path.dirname(target_filepath)
+ # Note: This is secure because we know that parent_dir is located
+ # inside of `$target_userspace/etc/pki` which is a directory that
+ # is not writable by unprivileged users. If this function is used
+ # elsewhere we may need to be more careful before running `mkdir -p`.
+ run(['mkdir', '-p', parent_dir])
+
+ if source_is_symlink:
+ # Preserve the owner and permissions of the original symlink
+ run(['ln', '-s', pointee, target_filepath])
+ run(['chmod', '--reference={}'.format(source_filepath), target_filepath])
+ continue
+
+ run(['cp', '-a', source_filepath, target_filepath])
+
+
def _copy_certificates(context, target_userspace):
"""
- Copy the needed certificates into the container, but preserve original ones
+ Copy certificates from source system into the container, but preserve
+ original ones
Some certificates are already installed in the container and those are
default certificates for the target OS, so we preserve these.
+
+ We respect the symlink hierarchy of the source system within the /etc/pki
+ folder. Dangling symlinks will be ignored.
+
"""
target_pki = os.path.join(target_userspace, 'etc', 'pki')
@@ -346,36 +414,56 @@ def _copy_certificates(context, target_userspace):
files_owned_by_rpms = _get_files_owned_by_rpms(target_context, '/etc/pki', recursive=True)
api.current_logger().debug('Files owned by rpms: {}'.format(' '.join(files_owned_by_rpms)))
+ # Backup container /etc/pki
run(['mv', target_pki, backup_pki])
- context.copytree_from('/etc/pki', target_pki)
+ # Copy source /etc/pki to the container
+ _copy_decouple('/etc/pki', target_pki)
+
+ # Assertion: after running _copy_decouple(), no broken symlinks exist in /etc/pki in the container
+ # So any broken symlinks created will be by the installed packages.
+
+ # Recover installed packages as they always get precedence
for filepath in files_owned_by_rpms:
src_path = os.path.join(backup_pki, filepath)
dst_path = os.path.join(target_pki, filepath)
# Resolve and skip any broken symlinks
is_broken_symlink = False
- while os.path.islink(src_path):
- # The symlink points to a path relative to the target userspace so
- # we need to readjust it
- next_path = os.path.join(target_userspace, os.readlink(src_path)[1:])
- if not os.path.exists(next_path):
- is_broken_symlink = True
-
- # The path original path of the broken symlink in the container
- report_path = os.path.join(target_pki, os.path.relpath(src_path, backup_pki))
- api.current_logger().warning('File {} is a broken symlink!'.format(report_path))
- break
-
- src_path = next_path
+ pointee = None
+ if os.path.islink(src_path):
+ pointee = os.path.join(target_userspace, os.readlink(src_path)[1:])
+
+ seen = set()
+ while os.path.islink(pointee):
+ # The symlink points to a path relative to the target userspace so
+ # we need to readjust it
+ pointee = os.path.join(target_userspace, os.readlink(src_path)[1:])
+ if not os.path.exists(pointee) or pointee in seen:
+ is_broken_symlink = True
+
+ # The path original path of the broken symlink in the container
+ report_path = os.path.join(target_pki, os.path.relpath(src_path, backup_pki))
+ api.current_logger().warning(
+ 'File {} is a broken symlink! Will not copy!'.format(report_path))
+ break
+
+ seen.add(pointee)
if is_broken_symlink:
continue
+ # Cleanup conflicting files
run(['rm', '-rf', dst_path])
+
+ # Ensure destination exists
parent_dir = os.path.dirname(dst_path)
run(['mkdir', '-p', parent_dir])
- run(['cp', '-a', src_path, dst_path])
+
+ # Copy the new file
+ run(['cp', '-R', '--preserve=all', src_path, dst_path])
+
+ run(['rm', '-rf', backup_pki])
def _prep_repository_access(context, target_userspace):
@@ -387,6 +475,10 @@ def _prep_repository_access(context, target_userspace):
backup_yum_repos_d = os.path.join(target_etc, 'yum.repos.d.backup')
_copy_certificates(context, target_userspace)
+ # NOTE(dkubek): context.call(['update-ca-trust']) seems to not be working.
+ # I am not really sure why. The changes to files are not
+ # being written to disk.
+ run(["chroot", target_userspace, "/bin/bash", "-c", "su - -c update-ca-trust"])
if not rhsm.skip_rhsm():
run(['rm', '-rf', os.path.join(target_etc, 'rhsm')])
diff --git a/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py b/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py
index cc684c7d..1a1ee56e 100644
--- a/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py
+++ b/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py
@@ -1,4 +1,8 @@
+from __future__ import division
+
import os
+import subprocess
+import sys
from collections import namedtuple
import pytest
@@ -11,6 +15,12 @@ from leapp.libraries.common.config import architecture
from leapp.libraries.common.testutils import CurrentActorMocked, logger_mocked, produce_mocked
from leapp.utils.deprecation import suppress_deprecation
+if sys.version_info < (2, 8):
+ from pathlib2 import Path
+else:
+ from pathlib import Path
+
+
CUR_DIR = os.path.dirname(os.path.abspath(__file__))
_CERTS_PATH = os.path.join(CUR_DIR, '../../../files', userspacegen.PROD_CERTS_FOLDER)
_DEFAULT_CERT_PATH = os.path.join(_CERTS_PATH, '8.1', '479.pem')
@@ -48,6 +58,220 @@ class MockedMountingBase(object):
pass
+def traverse_structure(structure, root=Path('/')):
+ for filename, links_to in structure.items():
+ filepath = root / filename
+
+ if isinstance(links_to, dict):
+ for pair in traverse_structure(links_to, filepath):
+ yield pair
+ else:
+ yield (filepath, links_to)
+
+
+def assert_directory_structure_matches(root, initial, expected):
+ # Assert every file that is supposed to be present is present
+ for filepath, links_to in traverse_structure(expected, root=root / 'expected'):
+ assert filepath.exists()
+
+ if links_to is None:
+ assert filepath.is_file()
+ continue
+
+ assert filepath.is_symlink()
+ assert os.readlink(str(filepath)) == str(root / 'initial' / links_to.lstrip('/'))
+
+ # Assert there are no extra files
+ result_dir = str(root / 'expected')
+ for fileroot, dummy_dirs, files in os.walk(result_dir):
+ for filename in files:
+ dir_path = os.path.relpath(fileroot, result_dir).split('/')
+
+ cwd = expected
+ for directory in dir_path:
+ cwd = cwd[directory]
+
+ assert filename in cwd
+
+ filepath = os.path.join(fileroot, filename)
+ if os.path.islink(filepath):
+ links_to = '/' + os.path.relpath(os.readlink(filepath), str(root / 'initial'))
+ assert cwd[filename] == links_to
+
+
+@pytest.fixture
+def temp_directory_layout(tmp_path, initial_structure):
+ for filepath, links_to in traverse_structure(initial_structure, root=tmp_path / 'initial'):
+ file_path = tmp_path / filepath
+ file_path.parent.mkdir(parents=True, exist_ok=True)
+
+ if links_to is None:
+ file_path.touch()
+ continue
+
+ file_path.symlink_to(tmp_path / 'initial' / links_to.lstrip('/'))
+
+ (tmp_path / 'expected').mkdir()
+ assert (tmp_path / 'expected').exists()
+
+ return tmp_path
+
+
+# The semantics of initial_structure and expected_structure are as follows:
+#
+# 1. The outermost dictionary encodes the root of a directory structure
+#
+# 2. Depending on the value for a key in a dict, each key in the dictionary
+# denotes the name of either a:
+# a) directory -- if value is dict
+# b) regular file -- if value is None
+# c) symlink -- if a value is str
+#
+# 3. The value of a symlink entry is a absolute path to a file in the context of
+# the structure.
+#
+@pytest.mark.parametrize('initial_structure,expected_structure', [
+ ({ # Copy a regular file
+ 'dir': {
+ 'fileA': None
+ }
+ }, {
+ 'dir': {
+ 'fileA': None
+ }
+ }),
+ ({ # Do not copy a broken symlink
+ 'dir': {
+ 'fileA': 'nonexistent'
+ }
+ }, {
+ 'dir': {}
+ }),
+ ({ # Copy a regular symlink
+ 'dir': {
+ 'fileA': '/dir/fileB',
+ 'fileB': None
+ }
+ }, {
+ 'dir': {
+ 'fileA': '/dir/fileB',
+ 'fileB': None
+ }
+ }),
+ ({ # Do not copy a chain of broken symlinks
+ 'dir': {
+ 'fileA': '/dir/fileB',
+ 'fileB': 'nonexistent'
+ }
+ }, {
+ 'dir': {}
+ }),
+ ({ # Copy a chain of symlinks
+ 'dir': {
+ 'fileA': '/dir/fileB',
+ 'fileB': '/dir/fileC',
+ 'fileC': None
+ }
+ }, {
+ 'dir': {
+ 'fileA': '/dir/fileB',
+ 'fileB': '/dir/fileC',
+ 'fileC': None
+ }
+ }),
+ ({ # Circular symlinks
+ 'dir': {
+ 'fileA': '/dir/fileB',
+ 'fileB': '/dir/fileC',
+ 'fileC': '/dir/fileC',
+ }
+ }, {
+ 'dir': {}
+ }),
+ ({ # Copy a link to a file outside the considered directory as file
+ 'dir': {
+ 'fileA': '/dir/fileB',
+ 'fileB': '/dir/fileC',
+ 'fileC': '/outside/fileOut',
+ 'fileE': None
+ },
+ 'outside': {
+ 'fileOut': '/outside/fileD',
+ 'fileD': '/dir/fileE'
+ }
+ }, {
+ 'dir': {
+ 'fileA': '/dir/fileB',
+ 'fileB': '/dir/fileC',
+ 'fileC': '/dir/fileE',
+ 'fileE': None,
+ }
+ }),
+ ({ # Same test with a nested structure within the source dir
+ 'dir': {
+ 'nested': {
+ 'fileA': '/dir/nested/fileB',
+ 'fileB': '/dir/nested/fileC',
+ 'fileC': '/outside/fileOut',
+ 'fileE': None
+ }
+ },
+ 'outside': {
+ 'fileOut': '/outside/fileD',
+ 'fileD': '/dir/nested/fileE'
+ }
+ }, {
+ 'dir': {
+ 'nested': {
+ 'fileA': '/dir/nested/fileB',
+ 'fileB': '/dir/nested/fileC',
+ 'fileC': '/dir/nested/fileE',
+ 'fileE': None
+ }
+ }
+ }),
+ ({ # Same test with a nested structure in the outside dir
+ 'dir': {
+ 'fileA': '/dir/fileB',
+ 'fileB': '/dir/fileC',
+ 'fileC': '/outside/nested/fileOut',
+ 'fileE': None
+ },
+ 'outside': {
+ 'nested': {
+ 'fileOut': '/outside/nested/fileD',
+ 'fileD': '/dir/fileE'
+ }
+ }
+ }, {
+ 'dir': {
+ 'fileA': '/dir/fileB',
+ 'fileB': '/dir/fileC',
+ 'fileC': '/dir/fileE',
+ 'fileE': None,
+ }
+ }),
+]
+)
+def test_copy_decouple(monkeypatch, temp_directory_layout, initial_structure, expected_structure):
+
+ def run_mocked(command):
+ subprocess.Popen(
+ ' '.join(command),
+ shell=True,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT,
+ ).wait()
+
+ monkeypatch.setattr(userspacegen, 'run', run_mocked)
+ userspacegen._copy_decouple(
+ str(temp_directory_layout / 'initial' / 'dir'),
+ str(temp_directory_layout / 'expected' / 'dir'),
+ )
+
+ assert_directory_structure_matches(temp_directory_layout, initial_structure, expected_structure)
+
+
@pytest.mark.parametrize('result,dst_ver,arch,prod_type', [
(os.path.join(_CERTS_PATH, '8.1', '479.pem'), '8.1', architecture.ARCH_X86_64, 'ga'),
(os.path.join(_CERTS_PATH, '8.1', '419.pem'), '8.1', architecture.ARCH_ARM64, 'ga'),
--
2.41.0
View Git Blame Copy Permalink