Skip to content

Commit 50e9e24

Browse files
committed
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`.
1 parent 17c88d9 commit 50e9e24

File tree

2 files changed

+156
-15
lines changed

2 files changed

+156
-15
lines changed

repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py

Lines changed: 78 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,53 @@ def _get_files_owned_by_rpms(context, dirpath, pkgs=None, recursive=False):
345345
return files_owned_by_rpms
346346

347347

348+
def _copy_decouple(srcdir, dstdir):
349+
for root, _, files in os.walk(srcdir):
350+
for filename in files:
351+
relpath = os.path.relpath(root, srcdir)
352+
source_filepath = os.path.join(root, filename)
353+
target_filepath = os.path.join(dstdir, relpath, filename)
354+
355+
# Skip and report broken symlinks (this follows any possible chain of symlinks)
356+
if not os.path.exists(source_filepath):
357+
api.current_logger().warning('File {} is a broken symlink!'.format(source_filepath))
358+
continue
359+
360+
# Copy symlinks to the target userspace
361+
pointee = None
362+
if os.path.islink(source_filepath):
363+
pointee = os.readlink(source_filepath)
364+
365+
# If source file is a symlink within `src` then preserve it,
366+
# otherwise resolve and copy it as a file it points to
367+
if pointee is not None and not pointee.startswith(srcdir):
368+
# Follow the path until we hit a file or get back to /etc/pki
369+
while not pointee.startswith(srcdir) and os.path.islink(pointee):
370+
pointee = os.readlink(pointee)
371+
372+
# Pointee points to a file outside /etc/pki so we copy it instead
373+
if not pointee.startswith(srcdir) and not os.path.islink(pointee):
374+
source_filepath = pointee
375+
else:
376+
# pointee points back to /etc/pki
377+
pass
378+
379+
# Ensure parent directory exists
380+
parent_dir = os.path.dirname(target_filepath)
381+
if not os.path.exists(parent_dir):
382+
os.makedirs(parent_dir, exist_ok=True)
383+
384+
if os.path.islink(source_filepath):
385+
# Translate pointee to target /etc/pki
386+
target_pointee = os.path.join(dstdir, os.path.relpath(pointee, root))
387+
# TODO(dkubek): Preserve the owner and permissions of the original symlink
388+
run(['ln', '-s', target_pointee, target_filepath])
389+
run(['chmod', '--reference={}'.format(source_filepath), target_filepath])
390+
continue
391+
392+
run(['cp', '-a', source_filepath, target_filepath])
393+
394+
348395
def _copy_certificates(context, target_userspace):
349396
"""
350397
Copy the needed certificates into the container, but preserve original ones
@@ -360,36 +407,51 @@ def _copy_certificates(context, target_userspace):
360407
files_owned_by_rpms = _get_files_owned_by_rpms(target_context, '/etc/pki', recursive=True)
361408
api.current_logger().debug('Files owned by rpms: {}'.format(' '.join(files_owned_by_rpms)))
362409

410+
# Backup container /etc/pki
363411
run(['mv', target_pki, backup_pki])
364-
context.copytree_from('/etc/pki', target_pki)
365412

413+
# Copy source /etc/pki to the container
414+
_copy_decouple('/etc/pki', target_pki)
415+
416+
# Assertion: If skip_broken_symlinks is True
417+
# => no broken symlinks exist in /etc/pki
418+
# So any new broken symlinks created will be by the installed packages.
419+
420+
# Recover installed packages as they always get precedence
366421
for filepath in files_owned_by_rpms:
367422
src_path = os.path.join(backup_pki, filepath)
368423
dst_path = os.path.join(target_pki, filepath)
369424

370425
# Resolve and skip any broken symlinks
371426
is_broken_symlink = False
372-
while os.path.islink(src_path):
373-
# The symlink points to a path relative to the target userspace so
374-
# we need to readjust it
375-
next_path = os.path.join(target_userspace, os.readlink(src_path)[1:])
376-
if not os.path.exists(next_path):
377-
is_broken_symlink = True
378-
379-
# The path original path of the broken symlink in the container
380-
report_path = os.path.join(target_pki, os.path.relpath(src_path, backup_pki))
381-
api.current_logger().warning('File {} is a broken symlink!'.format(report_path))
382-
break
383-
384-
src_path = next_path
427+
pointee = None
428+
if os.path.islink(src_path):
429+
pointee = os.path.join(target_userspace, os.readlink(src_path)[1:])
430+
431+
while os.path.islink(pointee):
432+
# The symlink points to a path relative to the target userspace so
433+
# we need to readjust it
434+
pointee = os.path.join(target_userspace, os.readlink(src_path)[1:])
435+
if not os.path.exists(pointee):
436+
is_broken_symlink = True
437+
438+
# The path original path of the broken symlink in the container
439+
report_path = os.path.join(target_pki, os.path.relpath(src_path, backup_pki))
440+
api.current_logger().warning('File {} is a broken symlink!'.format(report_path))
441+
break
385442

386443
if is_broken_symlink:
387444
continue
388445

446+
# Cleanup conflicting files
389447
run(['rm', '-rf', dst_path])
448+
449+
# Ensure destination exists
390450
parent_dir = os.path.dirname(dst_path)
391451
run(['mkdir', '-p', parent_dir])
392-
run(['cp', '-a', src_path, dst_path])
452+
453+
# Copy the new file
454+
run(['cp', '-R', '--preserve=all', src_path, dst_path])
393455

394456

395457
def _prep_repository_access(context, target_userspace):
@@ -401,6 +463,7 @@ def _prep_repository_access(context, target_userspace):
401463
backup_yum_repos_d = os.path.join(target_etc, 'yum.repos.d.backup')
402464

403465
_copy_certificates(context, target_userspace)
466+
context.call(['update-ca-trust', 'extract'])
404467

405468
if not rhsm.skip_rhsm():
406469
run(['rm', '-rf', os.path.join(target_etc, 'rhsm')])

repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import os
2+
import subprocess
23
from collections import namedtuple
4+
from pathlib import Path
35

46
import pytest
57

@@ -48,6 +50,82 @@ def __exit__(self, exception_type, exception_value, traceback):
4850
pass
4951

5052

53+
def traverse_structure(structure, root=Path('/')):
54+
for filename, links_to in structure.items():
55+
filepath = root / filename
56+
57+
if isinstance(links_to, dict):
58+
yield from traverse_structure(links_to, filepath)
59+
else:
60+
yield (filepath, links_to)
61+
62+
63+
def assert_directory_structure_matches(root, initial, expected):
64+
for filepath, links_to in traverse_structure(expected, root=root / 'expected'):
65+
assert filepath.exists()
66+
67+
if links_to is None:
68+
assert filepath.is_file()
69+
continue
70+
71+
assert filepath.is_symlink()
72+
assert os.readlink(filepath) == str(root / 'expected' / links_to.lstrip('/'))
73+
74+
75+
@pytest.fixture
76+
def temp_directory_layout(tmp_path, initial_structure):
77+
for filepath, links_to in traverse_structure(initial_structure, root=tmp_path / 'initial'):
78+
file_path = tmp_path / filepath
79+
file_path.parent.mkdir(parents=True, exist_ok=True)
80+
81+
if links_to is None:
82+
file_path.touch()
83+
continue
84+
85+
file_path.symlink_to(tmp_path / 'initial' / links_to.lstrip('/'))
86+
87+
(tmp_path / 'expected').mkdir()
88+
assert (tmp_path / 'expected').exists()
89+
90+
return tmp_path
91+
92+
93+
# The semantics of initial_structure and expected_structure are as follows:
94+
#
95+
# 1. The outermost dictionary encodes the root of a directory structure
96+
#
97+
# 2. Depending on the value for a key in a dict, each key in the dictionary
98+
# denotes the name of either a:
99+
# a) directory -- if value is dict
100+
# b) regular file -- if value is None
101+
# c) symlink -- if a value is str
102+
#
103+
# 3. The value of a symlink entry is a absolute path to a file in the context of
104+
# the structure.
105+
#
106+
@pytest.mark.parametrize('initial_structure,expected_structure', [
107+
({'dir': {'fileA': None}}, {'dir': {'fileA': None}}),
108+
({'dir': {'fileA': 'nonexistent'}}, {'dir': {}}),
109+
({'dir': {'fileA': '/dir/fileB', 'fileB': None}}, {'dir': {'fileA': '/dir/fileB', 'fileB': None}}),
110+
({'dir': {'fileA': '/dir/fileB', 'fileB': 'nonexistent'}},
111+
{'dir': {}}),
112+
({'dir': {'fileA': '/dir/fileB', 'fileB': '/dir/fileC', 'fileC': None}},
113+
{'dir': {'fileA': '/dir/fileB', 'fileB': '/dir/fileC', 'fileC': None}}),
114+
({'dir': {'fileA': '/dir/fileB', 'fileB': '/dir/fileC', 'fileC': '/outside/fileOut', 'fileE': None},
115+
'outside': {'fileOut': '/outside/fileD', 'fileD': '/dir/fileE'}},
116+
{'dir': {'fileA': '/dir/fileB', 'fileB': '/dir/fileC', 'fileC': '/dir/fileE'}}),
117+
]
118+
)
119+
def test_copy_decouple(monkeypatch, temp_directory_layout, initial_structure, expected_structure):
120+
monkeypatch.setattr(userspacegen, 'run', subprocess.run)
121+
userspacegen._copy_decouple(
122+
str(temp_directory_layout / 'initial' / 'dir'),
123+
str(temp_directory_layout / 'expected' / 'dir'),
124+
)
125+
126+
assert_directory_structure_matches(temp_directory_layout, initial_structure, expected_structure)
127+
128+
51129
@pytest.mark.parametrize('result,dst_ver,arch,prod_type', [
52130
(os.path.join(_CERTS_PATH, '8.1', '479.pem'), '8.1', architecture.ARCH_X86_64, 'ga'),
53131
(os.path.join(_CERTS_PATH, '8.1', '419.pem'), '8.1', architecture.ARCH_ARM64, 'ga'),

0 commit comments

Comments
 (0)