diff --git a/id_configurator/config.py b/id_configurator/config.py index 766e575..c964669 100644 --- a/id_configurator/config.py +++ b/id_configurator/config.py @@ -26,13 +26,23 @@ def _yes_no(value: bool) -> str: return "yes" if value else "no" +def _fold_deb822_multiline(field: str, value: str) -> str: + """Render `field` as a DEB822 folded multi-line field (sources.list(5)): + each of `value`'s own lines indented by one space, with a truly blank + line represented as a lone `.` - exactly how an ASCII-armored GPG key + block is embedded directly in a `Signed-By` field, verified against a + real `apt-get update` run against a genuinely signed repository.""" + folded = "\n".join(f" {line}" if line else " ." for line in value.splitlines()) + return f"{field}:\n{folded}" + + class AptSource(BaseModel): """A single APT source, in the DEB822 format (see sources.list(5)). Two ways to tell APT which key to trust this source with: - - `signed_by`: the ASCII-armored (.asc) *content* of a GPG public key. - The `apt` module dearmors it into a dedicated keyring and writes that - keyring's path as the rendered `Signed-By` field. + - `signed_by`: the ASCII-armored (.asc) *content* of a GPG public key, + folded inline as a multi-line `Signed-By` field - no separate keyring + file, no `gpg` binary needed to apply this configuration. - `signed_by_file`: the *path* to a keyring already expected to exist on the target system, written to the rendered `Signed-By` field verbatim. If both are set, `signed_by` takes precedence. @@ -66,61 +76,66 @@ class AptSource(BaseModel): date_max_future: str | None = Field(default=None, alias="date-max-future") description: str | None = None - def to_deb822(self, signed_by_path: str | None = None) -> str: + def to_deb822( + self, + signed_by_armored_key: str | None = None, + signed_by_path: str | None = None, + ) -> str: """Render this source as a DEB822 stanza (as used in /etc/apt/sources.list.d/*.sources). - `signed_by_path` is the value actually written as the `Signed-By` - field, if given - resolved by the caller (the `apt` module) from - `self.signed_by`/`self.signed_by_file`, neither of which is ever - rendered here directly. + Neither `self.signed_by` nor `self.signed_by_file` is ever rendered + directly - the caller (the `apt` module) resolves which one applies + and passes the result in: `signed_by_armored_key` is folded inline + as a multi-line `Signed-By` field (see `_fold_deb822_multiline`); + `signed_by_path` is written as a plain single-line `Signed-By` + field. If both are given, `signed_by_armored_key` takes precedence. """ - fields = [ - ("Types", " ".join(self.types)), - ("URIs", " ".join(self.uris)), - ("Suites", " ".join(self.suites)), + lines = [ + f"Types: {' '.join(self.types)}", + f"URIs: {' '.join(self.uris)}", + f"Suites: {' '.join(self.suites)}", ] if self.components: - fields.append(("Components", " ".join(self.components))) + lines.append(f"Components: {' '.join(self.components)}") if self.architectures: - fields.append(("Architectures", " ".join(self.architectures))) + lines.append(f"Architectures: {' '.join(self.architectures)}") if self.languages: - fields.append(("Languages", " ".join(self.languages))) + lines.append(f"Languages: {' '.join(self.languages)}") if self.targets: - fields.append(("Targets", " ".join(self.targets))) - if signed_by_path: - fields.append(("Signed-By", signed_by_path)) + lines.append(f"Targets: {' '.join(self.targets)}") + if signed_by_armored_key: + lines.append(_fold_deb822_multiline("Signed-By", signed_by_armored_key)) + elif signed_by_path: + lines.append(f"Signed-By: {signed_by_path}") if self.trusted is not None: - fields.append(("Trusted", _yes_no(self.trusted))) + lines.append(f"Trusted: {_yes_no(self.trusted)}") if self.pdiffs is not None: - fields.append(("PDiffs", _yes_no(self.pdiffs))) + lines.append(f"PDiffs: {_yes_no(self.pdiffs)}") if self.by_hash is not None: - fields.append(("By-Hash", _yes_no(self.by_hash))) + lines.append(f"By-Hash: {_yes_no(self.by_hash)}") if self.allow_insecure is not None: - fields.append(("Allow-Insecure", _yes_no(self.allow_insecure))) + lines.append(f"Allow-Insecure: {_yes_no(self.allow_insecure)}") if self.allow_weak is not None: - fields.append(("Allow-Weak", _yes_no(self.allow_weak))) + lines.append(f"Allow-Weak: {_yes_no(self.allow_weak)}") if self.allow_downgrade_to_insecure is not None: - fields.append( - ( - "Allow-Downgrade-To-Insecure", - _yes_no(self.allow_downgrade_to_insecure), - ) + lines.append( + f"Allow-Downgrade-To-Insecure: {_yes_no(self.allow_downgrade_to_insecure)}" ) if self.check_valid_until is not None: - fields.append(("Check-Valid-Until", _yes_no(self.check_valid_until))) + lines.append(f"Check-Valid-Until: {_yes_no(self.check_valid_until)}") if self.valid_until_min: - fields.append(("Valid-Until-Min", self.valid_until_min)) + lines.append(f"Valid-Until-Min: {self.valid_until_min}") if self.valid_until_max: - fields.append(("Valid-Until-Max", self.valid_until_max)) + lines.append(f"Valid-Until-Max: {self.valid_until_max}") if self.check_date is not None: - fields.append(("Check-Date", _yes_no(self.check_date))) + lines.append(f"Check-Date: {_yes_no(self.check_date)}") if self.date_max_future: - fields.append(("Date-Max-Future", self.date_max_future)) + lines.append(f"Date-Max-Future: {self.date_max_future}") if self.description: - fields.append(("Description", self.description)) - fields.append(("Enabled", _yes_no(self.enabled))) - return "\n".join(f"{key}: {value}" for key, value in fields) + "\n" + lines.append(f"Description: {self.description}") + lines.append(f"Enabled: {_yes_no(self.enabled)}") + return "\n".join(lines) + "\n" class AptConfig(BaseModel): diff --git a/id_configurator/modules/apt.py b/id_configurator/modules/apt.py index 8f6167a..765b0d9 100644 --- a/id_configurator/modules/apt.py +++ b/id_configurator/modules/apt.py @@ -7,27 +7,19 @@ longer takes effect, and the sources declared in the configuration are added as new, enabled `.sources` files. Each source can be trusted two ways: `signed_by` holds the ASCII-armored -content of a GPG public key, which is converted with `gpg --dearmor` into a -binary keyring dedicated to this one source, written to -`/etc/apt/keyrings/.gpg` - it is that path, not the key's own content, -that ends up in the rendered `.sources` file's `Signed-By` field. -`signed_by_file` instead names a keyring already expected to exist on the -target system, used verbatim as `Signed-By` (no file is written for it); if -both are given, `signed_by` takes precedence. `gpg` is only used here for -the armored-to-binary format conversion; the actual signature verification -of the repository itself is left to APT when it fetches the repository's -Release/InRelease file. +content of a GPG public key, folded inline as a multi-line `Signed-By` +field (per sources.list(5) - verified empirically against a real, +genuinely signed repository, so no `gpg` binary or separate keyring file +is needed at all to apply this configuration). `signed_by_file` instead +names a keyring already expected to exist on the target system, used +verbatim as `Signed-By`; if both are given, `signed_by` takes precedence. """ from __future__ import annotations import logging -import shutil -import subprocess -import tempfile from pathlib import Path -from id_configurator.config import AptSource from id_configurator.context import Context from id_configurator.modules.base import Module, register, sanitize_filename @@ -36,7 +28,6 @@ logger = logging.getLogger(__name__) _APT_DIR = "etc/apt" _SOURCES_LIST = "etc/apt/sources.list" _SOURCES_LIST_D = "etc/apt/sources.list.d" -_KEYRINGS_DIR = "etc/apt/keyrings" def _disable_list_content(content: str) -> str: @@ -59,28 +50,6 @@ def _disable_deb822_content(content: str) -> str: return "\n\n".join(disabled_stanzas) + "\n" -def _dearmor_gpg_key(name: str, content: bytes) -> bytes: - """Convert an ASCII-armored GPG public key into apt's binary keyring format.""" - with tempfile.TemporaryDirectory() as gnupg_home: - path = shutil.which("gpg") - if path is not None: - result = subprocess.run( - [path, "--homedir", gnupg_home, "--batch", "--dearmor"], - input=content, - capture_output=True, - check=False, - ) - else: - raise RuntimeError( - "gpg is not available; the GNUPG package must be installed in the environment running id-configurator" - ) - if result.returncode != 0: - raise RuntimeError( - f"invalid GPG key for apt source {name}: {result.stderr.decode('utf-8', 'replace')}" - ) - return result.stdout - - @register class AptModule(Module): name = "apt" @@ -119,22 +88,14 @@ class AptModule(Module): ) logger.info("disabled existing apt source %s", entry.name) - 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}/{sanitize_filename(source.name)}.gpg" - context.write_file(relative, keyring) - logger.info("installed apt keyring for source %s", source.name) - return f"/{relative}" - if source.signed_by_file: - return source.signed_by_file - return None - - def _write_configured_sources(self, context: Context) -> None: + @staticmethod + def _write_configured_sources(context: Context) -> None: for source in context.config.configurations.apt.sources: - signed_by_path = self._resolve_signed_by(context, source) context.write_file( f"{_SOURCES_LIST_D}/{sanitize_filename(source.name)}.sources", - source.to_deb822(signed_by_path).encode("utf-8"), + source.to_deb822( + signed_by_armored_key=source.signed_by, + signed_by_path=source.signed_by_file, + ).encode("utf-8"), ) logger.info("added apt source %s", source.name) diff --git a/library/id_configurator.py b/library/id_configurator.py index 95ad49e..bc93ff2 100644 --- a/library/id_configurator.py +++ b/library/id_configurator.py @@ -125,10 +125,9 @@ options: type: str description: - ASCII-armored content of a GPG public key to trust this - source with. The `apt` module dearmors it into a dedicated - keyring and writes that keyring's path as the rendered - Signed-By field. Takes precedence over signed_by_file if - both are set. + source with. Folded inline as a multi-line Signed-By field + (no separate keyring file or gpg binary needed). Takes + precedence over signed_by_file if both are set. signed_by_file: type: str description: diff --git a/tests/dockerfiles/Apt.Dockerfile b/tests/dockerfiles/Apt.Dockerfile index 860d578..06bb4ac 100644 --- a/tests/dockerfiles/Apt.Dockerfile +++ b/tests/dockerfiles/Apt.Dockerfile @@ -1,7 +1,5 @@ FROM debian:trixie-slim AS target FROM interdiode/id-configurator:latest AS configuration -RUN apt-get update && apt-get install -y --no-install-recommends gnupg \ - && rm -rf /var/lib/apt/lists/* COPY --from=target / /src/ COPY --from=generated-config Apt.yaml /config.yaml RUN id-configurator apply --config /config.yaml --src /src --dst /dst diff --git a/tests/test_apt.py b/tests/test_apt.py index c9aad78..5accb33 100644 --- a/tests/test_apt.py +++ b/tests/test_apt.py @@ -1,14 +1,13 @@ -import subprocess -from unittest.mock import MagicMock - -import pytest - from id_configurator.config import Configuration from id_configurator.context import Context from id_configurator.modules.apt import AptModule -KEY_CONTENT = "fake-armored-gpg-key" -KEYRING_BYTES = b"fake-dearmored-keyring-bytes" +KEY_CONTENT = ( + "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + "\n" + "mQENBGqh08IBCAC0\n" + "-----END PGP PUBLIC KEY BLOCK-----\n" +) SOURCE_CONTENT = { "uuid": "12345678-1234-5678-1234-567812345678", @@ -52,16 +51,6 @@ def make_config_with(source_extra: dict, name: str = "interdiode") -> Configurat return Configuration.model_validate(data) -def _mock_gpg_dearmor_success(monkeypatch, output: bytes = KEYRING_BYTES) -> MagicMock: - mock_run = MagicMock( - return_value=subprocess.CompletedProcess( - args=[], returncode=0, stdout=output, stderr=b"" - ) - ) - monkeypatch.setattr("id_configurator.modules.apt.subprocess.run", mock_run) - return mock_run - - def test_is_relevant_false_without_src(tmp_path) -> None: context = Context(config=make_config(), dst=tmp_path / "dst", src=None) assert AptModule().is_relevant(context) is False @@ -81,10 +70,7 @@ def test_is_relevant_true_with_apt_dir(tmp_path) -> None: assert AptModule().is_relevant(context) is True -def test_apply_disables_existing_sources_and_adds_configured_ones( - tmp_path, monkeypatch -) -> None: - mock_run = _mock_gpg_dearmor_success(monkeypatch) +def test_apply_disables_existing_sources_and_adds_configured_ones(tmp_path) -> None: src = tmp_path / "src" apt_dir = src / "etc/apt" sources_list_d = apt_dir / "sources.list.d" @@ -133,22 +119,21 @@ def test_apply_disables_existing_sources_and_adds_configured_ones( assert not (dst / "etc/apt/sources.list.d/ignored.txt").exists() - keyring = (dst / "etc/apt/keyrings/interdiode.gpg").read_bytes() - assert keyring == KEYRING_BYTES - mock_run.assert_called_once() - assert mock_run.call_args.kwargs["input"] == KEY_CONTENT.encode("utf-8") - configured_source = (dst / "etc/apt/sources.list.d/interdiode.sources").read_text( encoding="utf-8" ) assert configured_source == ( "Types: deb\nURIs: http://example.invalid/debian\nSuites: stable\n" - "Signed-By: /etc/apt/keyrings/interdiode.gpg\nEnabled: yes\n" + "Signed-By:\n" + " -----BEGIN PGP PUBLIC KEY BLOCK-----\n" + " .\n" + " mQENBGqh08IBCAC0\n" + " -----END PGP PUBLIC KEY BLOCK-----\n" + "Enabled: yes\n" ) -def test_apply_without_existing_top_level_sources_list(tmp_path, monkeypatch) -> None: - _mock_gpg_dearmor_success(monkeypatch) +def test_apply_without_existing_top_level_sources_list(tmp_path) -> None: src = tmp_path / "src" (src / "etc/apt").mkdir(parents=True) dst = tmp_path / "dst" @@ -160,8 +145,7 @@ def test_apply_without_existing_top_level_sources_list(tmp_path, monkeypatch) -> assert (dst / "etc/apt/sources.list.d/interdiode.sources").exists() -def test_apply_uses_signed_by_file_when_no_key_content(tmp_path, monkeypatch) -> None: - mock_run = _mock_gpg_dearmor_success(monkeypatch) +def test_apply_uses_signed_by_file_when_no_key_content(tmp_path) -> None: dst = tmp_path / "dst" context = Context( config=make_config_with({"signed-by-file": "/etc/apt/keyrings/existing.gpg"}), @@ -171,8 +155,6 @@ def test_apply_uses_signed_by_file_when_no_key_content(tmp_path, monkeypatch) -> AptModule().apply(context) - mock_run.assert_not_called() - assert not (dst / "etc/apt/keyrings").exists() configured_source = (dst / "etc/apt/sources.list.d/interdiode.sources").read_text( encoding="utf-8" ) @@ -182,10 +164,7 @@ def test_apply_uses_signed_by_file_when_no_key_content(tmp_path, monkeypatch) -> ) -def test_apply_signed_by_takes_precedence_over_signed_by_file( - tmp_path, monkeypatch -) -> None: - mock_run = _mock_gpg_dearmor_success(monkeypatch) +def test_apply_signed_by_takes_precedence_over_signed_by_file(tmp_path) -> None: dst = tmp_path / "dst" context = Context( config=make_config_with( @@ -200,25 +179,19 @@ def test_apply_signed_by_takes_precedence_over_signed_by_file( AptModule().apply(context) - mock_run.assert_called_once() - assert (dst / "etc/apt/keyrings/interdiode.gpg").read_bytes() == KEYRING_BYTES configured_source = (dst / "etc/apt/sources.list.d/interdiode.sources").read_text( encoding="utf-8" ) - assert "Signed-By: /etc/apt/keyrings/interdiode.gpg\n" in configured_source + assert "Signed-By:\n -----BEGIN PGP PUBLIC KEY BLOCK-----" in configured_source + assert "existing.gpg" not in configured_source -def test_apply_without_signed_by_writes_no_keyring_or_field( - tmp_path, monkeypatch -) -> None: - mock_run = _mock_gpg_dearmor_success(monkeypatch) +def test_apply_without_signed_by_writes_no_field(tmp_path) -> None: dst = tmp_path / "dst" context = Context(config=make_config_with({}), dst=dst, src=None) AptModule().apply(context) - mock_run.assert_not_called() - assert not (dst / "etc/apt/keyrings").exists() configured_source = (dst / "etc/apt/sources.list.d/interdiode.sources").read_text( encoding="utf-8" ) @@ -227,8 +200,7 @@ 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) +def test_apply_sanitizes_source_name_with_slash(tmp_path) -> None: dst = tmp_path / "dst" context = Context( config=make_config_with({"signed-by": KEY_CONTENT}, name="../../evil"), @@ -238,33 +210,6 @@ def test_apply_sanitizes_source_name_with_slash(tmp_path, monkeypatch) -> 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" - context = Context(config=make_config(), dst=dst, src=None) - - with pytest.raises(RuntimeError, match="gpg is not available"): - AptModule().apply(context) - - -def test_resolve_signed_by_raises_on_invalid_key(tmp_path, monkeypatch) -> None: - mock_run = MagicMock( - return_value=subprocess.CompletedProcess( - args=[], returncode=2, stdout=b"", stderr=b"gpg: no valid data found" - ) - ) - monkeypatch.setattr("id_configurator.modules.apt.subprocess.run", mock_run) - dst = tmp_path / "dst" - context = Context(config=make_config(), dst=dst, src=None) - - with pytest.raises(RuntimeError, match="invalid GPG key for apt source interdiode"): - AptModule().apply(context) + assert source_file.exists() + assert not (dst / "etc/apt/sources.list.d/evil.sources").exists() diff --git a/tests/test_config.py b/tests/test_config.py index 11f0ba8..e7a1027 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -226,3 +226,39 @@ def test_apt_source_to_deb822_full() -> None: "Description: InterDiode APT repository", "Enabled: no", ] + + +def test_apt_source_to_deb822_folds_inline_signed_by_key() -> None: + source = AptSource( + name="interdiode", uris=["http://example.invalid/debian"], suites=["stable"] + ) + armored_key = ( + "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + "\n" + "mQENBGqh08IBCAC0\n" + "-----END PGP PUBLIC KEY BLOCK-----\n" + ) + rendered = source.to_deb822(signed_by_armored_key=armored_key) + assert rendered == ( + "Types: deb\n" + "URIs: http://example.invalid/debian\n" + "Suites: stable\n" + "Signed-By:\n" + " -----BEGIN PGP PUBLIC KEY BLOCK-----\n" + " .\n" + " mQENBGqh08IBCAC0\n" + " -----END PGP PUBLIC KEY BLOCK-----\n" + "Enabled: yes\n" + ) + + +def test_apt_source_to_deb822_signed_by_armored_key_takes_precedence() -> None: + source = AptSource( + name="interdiode", uris=["http://example.invalid/debian"], suites=["stable"] + ) + rendered = source.to_deb822( + signed_by_armored_key="-----BEGIN PGP PUBLIC KEY BLOCK-----\nabc\n-----END PGP PUBLIC KEY BLOCK-----\n", + signed_by_path="/etc/apt/keyrings/unused.gpg", + ) + assert "Signed-By:\n" in rendered + assert "unused.gpg" not in rendered