feat: sanitize filenames in apt and yum modules to prevent path traversal

Signed-off-by: d9pouces <github@19pouces.net>

id_configurator/modules/apt.py

@@ -29,7 +29,7 @@ from pathlib import Path
 
 from id_configurator.config import AptSource
 from id_configurator.context import Context
-from id_configurator.modules.base import Module, register
+from id_configurator.modules.base import Module, register, sanitize_filename
 
 logger = logging.getLogger(__name__)
 
@@ -122,7 +122,7 @@ class AptModule(Module):
     def _resolve_signed_by(self, context: Context, source: AptSource) -> str | None:
         if source.signed_by:
             keyring = _dearmor_gpg_key(source.name, source.signed_by.encode("utf-8"))
-            relative = f"{_KEYRINGS_DIR}/{source.name}.gpg"
+            relative = f"{_KEYRINGS_DIR}/{sanitize_filename(source.name)}.gpg"
             context.write_file(relative, keyring)
             logger.info("installed apt keyring for source %s", source.name)
             return f"/{relative}"
@@ -134,7 +134,7 @@ class AptModule(Module):
         for source in context.config.configurations.apt.sources:
             signed_by_path = self._resolve_signed_by(context, source)
             context.write_file(
-                f"{_SOURCES_LIST_D}/{source.name}.sources",
+                f"{_SOURCES_LIST_D}/{sanitize_filename(source.name)}.sources",
                 source.to_deb822(signed_by_path).encode("utf-8"),
             )
             logger.info("added apt source %s", source.name)

id_configurator/modules/base.py

@@ -34,3 +34,18 @@ def register(module_cls: type[Module]) -> type[Module]:
 
 def all_modules() -> list[type[Module]]:
     return list(_REGISTRY)
+
+
+def sanitize_filename(name: str) -> str:
+    """Make a user-supplied name safe to use as a single filesystem path
+    component.
+
+    Modules that derive a file's name from configuration data (a
+    certificate's name, a repository's name, ...) must run it through this
+    first: without it, a name containing `/` (or `\\`, harmless on the
+    target systems this tool writes to but stripped for good measure)
+    would silently turn into extra, unintended path segments - creating
+    nested directories a careless or malicious name shouldn't be able to
+    create - instead of the single file the caller asked for.
+    """
+    return name.replace("/", "_").replace("\\", "_")

id_configurator/modules/certificates.py

@@ -49,7 +49,7 @@ import posixpath
 from pathlib import Path
 
 from id_configurator.context import Context
-from id_configurator.modules.base import Module, register
+from id_configurator.modules.base import Module, register, sanitize_filename
 
 logger = logging.getLogger(__name__)
 
@@ -142,7 +142,7 @@ class CertificatesModule(Module):
 
     def apply(self, context: Context) -> None:
         configured_names = {
-            certificate.name
+            sanitize_filename(certificate.name)
             for certificate in context.config.configurations.certificates
         }
         for existing in context.iter_src_dir(_LOCAL_CERTS_DIR):
@@ -152,7 +152,7 @@ class CertificatesModule(Module):
                 )
         for certificate in context.config.configurations.certificates:
             context.write_file(
-                f"{_LOCAL_CERTS_DIR}/{certificate.name}.crt",
+                f"{_LOCAL_CERTS_DIR}/{sanitize_filename(certificate.name)}.crt",
                 certificate.content.encode(),
             )
             logger.info("installed root certificate %s", certificate.name)
@@ -237,7 +237,7 @@ class CertificatesModule(Module):
                 )
         for certificate in context.config.configurations.certificates:
             context.write_file(
-                f"{_RHEL_ANCHORS_DIR}/{certificate.name}.crt",
+                f"{_RHEL_ANCHORS_DIR}/{sanitize_filename(certificate.name)}.crt",
                 certificate.content.encode(),
             )
 

id_configurator/modules/yum.py

@@ -30,7 +30,7 @@ from io import StringIO
 
 from id_configurator.config import NamedContent, YumRepository
 from id_configurator.context import Context
-from id_configurator.modules.base import Module, register
+from id_configurator.modules.base import Module, register, sanitize_filename
 
 logger = logging.getLogger(__name__)
 
@@ -71,7 +71,7 @@ def _write_repo_file(context: Context, repository: YumRepository) -> None:
     else:
         lines.append("gpgcheck=0")
     context.write_file(
-        f"{_REPOS_DIR}/{repository.name}.repo",
+        f"{_REPOS_DIR}/{sanitize_filename(repository.name)}.repo",
         ("\n".join(lines) + "\n").encode("utf-8"),
     )
     logger.info("added yum/dnf repository %s", repository.name)
@@ -100,7 +100,8 @@ def _validate_gpg_key(name: str, content: bytes) -> None:
 def _import_key(context: Context, key: NamedContent) -> None:
     _validate_gpg_key(key.name, key.content.encode("utf-8"))
     context.write_file(
-        f"{_RPM_GPG_DIR}/RPM-GPG-KEY-{key.name}", key.content.encode("utf-8")
+        f"{_RPM_GPG_DIR}/RPM-GPG-KEY-{sanitize_filename(key.name)}",
+        key.content.encode("utf-8"),
     )
     logger.info("imported yum/dnf GPG key %s", key.name)
 

tests/dockerfiles/repos/apt-signing-key.key.asc

@@ -0,0 +1,33 @@
+-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+lQOYBGqg6rMBCADaMUd27S41+kInPXuRZ0aL41aMZjqr+mw4W34w8i/JRYMJ6eGb
+sXf40AaFfS7zkbUpQZpmh3aYPF4jUAdWV03ZNNYdcGerIAKc2xsugSKjJIm1XPQq
+upz914b4/hp/UslDUH7SPZ+3Tq17TXW+xP6aUM9VcWMnlJT/DFgxh5sZB3nfDeYr
+yBV+PmEVfuQaUSIwKAh3Hv0ZaBHS8KzsawLzbJ+Pa9W9wjVME1izi9ctloGSxtLc
+p1suS2IS33QB0jGgj4VbuMcjo2iCpzW3Xd7jQ7WpXGjrl0bHnJTnWWB0ihDD3cJe
+tJlk9GScMQu3Z3th67DmqTXQru3btrDjxMifABEBAAEAB/sG3JffwJO8dNNRwpg5
+OWM5kPuuzKQ5tlVi7FWqmmMwVDRLwoQqzkj0lS4vUoGAfWuSEZMWeFodbYCUYbbp
+WzOCiivVnyxTcF1Eq4tOShdZxBNklbjEAzxeq0F+YRdaO7NCn6xtVizIqfvAre3v
+Aujy12m9ySQSIZUR9ERtwlL5VXj4yzcXciyM9aNbh2D4BFAoo+fBL1B5mhpKQ16B
+BafkpOf0myWZrwLdcn1t7BQ6UcLqsPJj6COCXXRDHoZvim38UBMMajNP4+8eC2Vk
+DDR6iFz53FRvIREQWkoEEW4P80v1lgcnP1apJGhkrJYCPM7ObZk0MoQkBiBdlPPQ
+VLiRBADhNsKjNd7v3Af5XGPJGWWZxCMxPlgwc6EMjCX3SQDql042eSIqwn1FFP6H
+Hi8lyq/XWz0JcUwcrpFxpIC3OLUsBleX3Z4E17Z6AotSV0FC6y23cqgtweMeLA0L
+tayMRmXJ7Aan+xfvi8MbW6lYGzM2uYHl27rVBuCt0kvxn+U4lwQA+ATO8KaBreCL
+E/vXGZaPWJs2GoiDBavKwqdYtlXDKlVP5XvBgaPfY6zKLtCsC6N+DKkVo42Stwu3
+oB6HSSLvXvFIhvUReh5PLpijmOroAJzvLQN9qmZBvF4IOUgaHmlYlL0DGzlxcwLr
+SdzqPaGRa+EgraFDDGGu5oQ5A2wNKTkEAMDS2w5SibOfsvnfK4aDc0mi8/fftzCO
+SwsvVzR0SmbXtaCPBnBjlA7YJhRZ6n/7nDpswrWPk8g125A0vK1cNRGM0ufKYW1f
+DMbB/aYrJjAKz8zWH6cgOD4fqM2gA9JounkIcJvWOToGume8pXhmem94u2FP8aik
+A6hL/Os+WRQcMNi0NWlkLWNvbmZpZ3VyYXRvci10ZXN0cyA8dGVzdHNAaWQtY29u
+ZmlndXJhdG9yLmludmFsaWQ+iQFuBBMBCABYFiEEaZF4ALsB1xUU3IP5rl3Himib
+1RgFAmqg6rMbFIAAAAAABAAObWFudTIsMi41KzEuMTIsMCwzAxsvBAULCQgHAgIi
+AgYVCgkICwIEFgIDAQIeBwIXgAAKCRCuXceKaJvVGFRrB/wIbG1HHQANDCHgyl4T
+RTPuKBX2c9lmJi2plp+T5L5sXyTMbAMLfTCkJWL/zHLM3N2v6GZuzbwZM8Dzj9zL
+LMpC6newmZe4YX+ZtyZGufBI963kh8PvlSWWUSyH+oYoIhlyJqj7tCQXoijEvQnS
+ReKGHUTvQGvQK2Ybp735NmAiDOM3/3KKh2Dqas/Tc2mxec4whr8RyUidsF+Yp6wT
+esfMxeax6DmWIcSLDWpUBndeahZ2Ff0c96BzbpF1v2hbvyI/IaP5snb4Nm1xpREm
++y8iDAQhOklfXCyXrWgN3sIrk3tDndGrn+nw0iusnaBVki0keqjlH0camoUP28tu
+3sxG
+=TrE4
+-----END PGP PRIVATE KEY BLOCK-----

tests/dockerfiles/repos/apt-signing-key.pub.asc

@@ -0,0 +1,19 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mQENBGqg6rMBCADaMUd27S41+kInPXuRZ0aL41aMZjqr+mw4W34w8i/JRYMJ6eGb
+sXf40AaFfS7zkbUpQZpmh3aYPF4jUAdWV03ZNNYdcGerIAKc2xsugSKjJIm1XPQq
+upz914b4/hp/UslDUH7SPZ+3Tq17TXW+xP6aUM9VcWMnlJT/DFgxh5sZB3nfDeYr
+yBV+PmEVfuQaUSIwKAh3Hv0ZaBHS8KzsawLzbJ+Pa9W9wjVME1izi9ctloGSxtLc
+p1suS2IS33QB0jGgj4VbuMcjo2iCpzW3Xd7jQ7WpXGjrl0bHnJTnWWB0ihDD3cJe
+tJlk9GScMQu3Z3th67DmqTXQru3btrDjxMifABEBAAG0NWlkLWNvbmZpZ3VyYXRv
+ci10ZXN0cyA8dGVzdHNAaWQtY29uZmlndXJhdG9yLmludmFsaWQ+iQFuBBMBCABY
+FiEEaZF4ALsB1xUU3IP5rl3Himib1RgFAmqg6rMbFIAAAAAABAAObWFudTIsMi41
+KzEuMTIsMCwzAxsvBAULCQgHAgIiAgYVCgkICwIEFgIDAQIeBwIXgAAKCRCuXceK
+aJvVGFRrB/wIbG1HHQANDCHgyl4TRTPuKBX2c9lmJi2plp+T5L5sXyTMbAMLfTCk
+JWL/zHLM3N2v6GZuzbwZM8Dzj9zLLMpC6newmZe4YX+ZtyZGufBI963kh8PvlSWW
+USyH+oYoIhlyJqj7tCQXoijEvQnSReKGHUTvQGvQK2Ybp735NmAiDOM3/3KKh2Dq
+as/Tc2mxec4whr8RyUidsF+Yp6wTesfMxeax6DmWIcSLDWpUBndeahZ2Ff0c96Bz
+bpF1v2hbvyI/IaP5snb4Nm1xpREm+y8iDAQhOklfXCyXrWgN3sIrk3tDndGrn+nw
+0iusnaBVki0keqjlH0camoUP28tu3sxG
+=qZrp
+-----END PGP PUBLIC KEY BLOCK-----

tests/test_apt.py

@@ -32,7 +32,7 @@ def make_config() -> Configuration:
     return Configuration.model_validate(SOURCE_CONTENT)
 
 
-def make_config_with(source_extra: dict) -> Configuration:
+def make_config_with(source_extra: dict, name: str = "interdiode") -> Configuration:
     data = {
         "uuid": "12345678-1234-5678-1234-567812345678",
         "organisation": "InterDiode",
@@ -40,7 +40,7 @@ def make_config_with(source_extra: dict) -> Configuration:
             "apt": {
                 "sources": [
                     {
-                        "name": "interdiode",
+                        "name": name,
                         "uris": ["http://example.invalid/debian"],
                         "suites": ["stable"],
                         **source_extra,
@@ -227,6 +227,26 @@ def test_apply_without_signed_by_writes_no_keyring_or_field(
     )
 
 
+def test_apply_sanitizes_source_name_with_slash(tmp_path, monkeypatch) -> None:
+    mock_run = _mock_gpg_dearmor_success(monkeypatch)
+    dst = tmp_path / "dst"
+    context = Context(
+        config=make_config_with({"signed-by": KEY_CONTENT}, name="../../evil"),
+        dst=dst,
+        src=None,
+    )
+
+    AptModule().apply(context)
+
+    keyring = dst / "etc/apt/keyrings/.._.._evil.gpg"
+    assert keyring.read_bytes() == KEYRING_BYTES
+    source_file = dst / "etc/apt/sources.list.d/.._.._evil.sources"
+    assert "Signed-By: /etc/apt/keyrings/.._.._evil.gpg" in source_file.read_text(
+        encoding="utf-8"
+    )
+    mock_run.assert_called_once()
+
+
 def test_resolve_signed_by_raises_when_gpg_not_on_path(tmp_path, monkeypatch) -> None:
     monkeypatch.setattr("id_configurator.modules.apt.shutil.which", lambda _: None)
     dst = tmp_path / "dst"

tests/test_certificates.py

@@ -54,6 +54,27 @@ def test_apply_writes_certificate_and_rebuilds_bundle(tmp_path) -> None:
     assert openssl_default.readlink().as_posix() == "/etc/ssl/certs/ca-certificates.crt"
 
 
+def test_apply_sanitizes_certificate_name_with_slash(tmp_path) -> None:
+    data = {
+        "uuid": "12345678-1234-5678-1234-567812345678",
+        "organisation": "InterDiode",
+        "configurations": {
+            "certificates": [{"name": "../../evil", "content": CERT_CONTENT}]
+        },
+    }
+    dst = tmp_path / "dst"
+    context = Context(config=Configuration.model_validate(data), dst=dst, src=None)
+
+    CertificatesModule().apply(context)
+
+    local_cert = dst / "usr/local/share/ca-certificates/.._.._evil.crt"
+    assert local_cert.read_bytes() == b"fake-cert-bytes"
+    # The "/" must never be interpreted as a path separator: without
+    # sanitizing, "../../evil" would have reshuffled the write up to
+    # usr/local/evil.crt instead of staying under ca-certificates/.
+    assert not (dst / "usr/local/evil.crt").exists()
+
+
 def test_apply_merges_conf_listed_and_local_certificates(tmp_path) -> None:
     src = tmp_path / "src"
     local_certs_dir = src / "usr/local/share/ca-certificates"

tests/test_modules_base.py

@@ -1,5 +1,10 @@
 from id_configurator.modules import base as base_module
-from id_configurator.modules.base import Module, all_modules, register
+from id_configurator.modules.base import (
+    Module,
+    all_modules,
+    register,
+    sanitize_filename,
+)
 
 
 def test_register_and_all_modules_are_isolated(monkeypatch) -> None:
@@ -17,3 +22,15 @@ def test_register_and_all_modules_are_isolated(monkeypatch) -> None:
     register(DummyModule)
 
     assert all_modules() == [DummyModule]
+
+
+def test_sanitize_filename_leaves_plain_names_untouched() -> None:
+    assert sanitize_filename("interdiode") == "interdiode"
+
+
+def test_sanitize_filename_replaces_forward_slash() -> None:
+    assert sanitize_filename("../../etc/passwd") == ".._.._etc_passwd"
+
+
+def test_sanitize_filename_replaces_backslash() -> None:
+    assert sanitize_filename(r"a\b") == "a_b"

tests/test_yum.py

@@ -165,6 +165,36 @@ def test_apply_disables_existing_repos_before_adding_configured_ones(tmp_path) -
     )
 
 
+def test_apply_sanitizes_repository_and_key_names_with_slash(
+    tmp_path, monkeypatch
+) -> None:
+    mock_run = _mock_gpg_success(monkeypatch)
+    data = {
+        "uuid": "12345678-1234-5678-1234-567812345678",
+        "organisation": "InterDiode",
+        "configurations": {
+            "yum": {
+                "repositories": [
+                    {
+                        "name": "../../evil-repo",
+                        "index-url": "https://example.invalid/yum",
+                    }
+                ],
+                "keys": [{"name": "../../evil-key", "content": KEY_CONTENT}],
+            }
+        },
+    }
+    dst = tmp_path / "dst"
+    context = Context(config=make_config(data), dst=dst, src=None)
+
+    YumModule().apply(context)
+
+    assert (dst / "etc/yum.repos.d/.._.._evil-repo.repo").exists()
+    key_content = (dst / "etc/pki/rpm-gpg/RPM-GPG-KEY-.._.._evil-key").read_bytes()
+    assert key_content == b"fake-rpm-gpg-key-bytes"
+    mock_run.assert_called_once()
+
+
 def test_import_key_raises_when_gpg_binary_missing(tmp_path, monkeypatch) -> None:
     monkeypatch.setattr(
         "id_configurator.modules.yum.subprocess.run",