From 46c11c155ce19c496bec2e1c0060c593cb038432 Mon Sep 17 00:00:00 2001 From: Wenqi Li Date: Mon, 31 Aug 2026 17:32:07 +0100 Subject: [PATCH 1/2] fix(setup): verify Kitware archive key fingerprint Validate the downloaded archive key against Kitware's published primary fingerprint before dearmoring or installing it. Reject missing, mismatched, or bundled extra primary keys. Co-authored-by: Codex Signed-off-by: Wenqi Li --- .lint/codespell_ignore_words.txt | 1 + src/holoscan_cli/utils/host_setup.py | 51 ++++++++++++- tests/unit/test_host_setup.py | 109 +++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 4 deletions(-) diff --git a/.lint/codespell_ignore_words.txt b/.lint/codespell_ignore_words.txt index d2fdbb76..92350d4d 100644 --- a/.lint/codespell_ignore_words.txt +++ b/.lint/codespell_ignore_words.txt @@ -4,6 +4,7 @@ assertIn EHR ehr +fpr INFOR Infor infor diff --git a/src/holoscan_cli/utils/host_setup.py b/src/holoscan_cli/utils/host_setup.py index 0ea1a837..faaa6099 100644 --- a/src/holoscan_cli/utils/host_setup.py +++ b/src/holoscan_cli/utils/host_setup.py @@ -47,6 +47,9 @@ "arm64": "fbb763bb818ca8ff3302a7764a95c63a42d80b7f864e87639833c55e59b6aadf", "linux": "a31a87a87593f5ca575d924f5e12cd0fcda1c81528f4cc3aebe0669e1643678f", } +_KITWARE_ARCHIVE_KEY_URL = "https://apt.kitware.com/keys/kitware-archive-latest.asc" +# Published at https://apt.kitware.com/. Update deliberately when Kitware rotates its key. +_KITWARE_ARCHIVE_KEY_FINGERPRINT = "4DBEBE3EEC96E7B8C6EC5BE99E92FDC6C5B9BA75" class PackageInstallationError(Exception): @@ -209,6 +212,26 @@ def get_ubuntu_codename() -> str: # ---- high-level setup_* orchestrators --------------------------------------- +def _primary_key_fingerprints(key_listing: bytes) -> List[str]: + """Extract primary-key fingerprints from GPG's machine-readable output.""" + fingerprints: List[str] = [] + primary_index = None + for line in key_listing.decode(errors="replace").splitlines(): + fields = line.split(":") + record_type = fields[0] + if record_type == "pub": + fingerprints.append("") + primary_index = len(fingerprints) - 1 + elif record_type == "sub": + primary_index = None + elif record_type == "fpr" and primary_index is not None: + fingerprint = fields[9] if len(fields) > 9 else "" + if re.fullmatch(r"[0-9A-Fa-f]+", fingerprint): + fingerprints[primary_index] = fingerprint.upper() + primary_index = None + return fingerprints + + def setup_cmake(min_version: str = "3.26.4", dry_run: bool = False) -> None: """Setup CMake from Kitware if needed""" global _apt_updated @@ -236,10 +259,30 @@ def setup_cmake(min_version: str = "3.26.4", dry_run: bool = False) -> None: gpg = shutil.which("gpg") or "/usr/bin/gpg" try: key = subprocess.run( - [wget, "-qO-", "https://apt.kitware.com/keys/kitware-archive-latest.asc"], + [wget, "-qO-", _KITWARE_ARCHIVE_KEY_URL], + check=True, + capture_output=True, + ).stdout + key_listing = subprocess.run( + [ + gpg, + "--batch", + "--no-options", + "--with-colons", + "--fingerprint", + "--show-keys", + ], + input=key, check=True, capture_output=True, ).stdout + fingerprints = _primary_key_fingerprints(key_listing) + if fingerprints != [_KITWARE_ARCHIVE_KEY_FINGERPRINT]: + found = ", ".join(fingerprint or "" for fingerprint in fingerprints) + fatal( + "Kitware apt archive key fingerprint verification failed: " + f"expected {_KITWARE_ARCHIVE_KEY_FINGERPRINT}, found {found or ''}." + ) dearmored = subprocess.run( [gpg, "--dearmor"], input=key, @@ -247,11 +290,11 @@ def setup_cmake(min_version: str = "3.26.4", dry_run: bool = False) -> None: capture_output=True, ).stdout except FileNotFoundError as e: - fatal(f"Failed to download the Kitware apt archive key: {e}") + fatal(f"Failed to download or process the Kitware apt archive key: {e}") except subprocess.CalledProcessError as e: fatal( - "Failed to download the Kitware apt archive key " - f"(is the network available?): {e.stderr.decode(errors='replace').strip()}" + "Failed to download or process the Kitware apt archive key: " + f"{e.stderr.decode(errors='replace').strip()}" ) write_system_file(keyring_path, dearmored) write_system_file("/etc/apt/sources.list.d/kitware.list", source_line) diff --git a/tests/unit/test_host_setup.py b/tests/unit/test_host_setup.py index e1635f5b..8a1993d0 100644 --- a/tests/unit/test_host_setup.py +++ b/tests/unit/test_host_setup.py @@ -3,11 +3,120 @@ from __future__ import annotations +import subprocess + import pytest from holoscan_cli.utils import host_setup +def _prepare_cmake_setup(monkeypatch): + package_calls = [] + writes = [] + commands = [] + monkeypatch.setattr(host_setup, "_apt_updated", False) + monkeypatch.setattr(host_setup, "get_installed_package_version", lambda _: None) + monkeypatch.setattr(host_setup, "get_ubuntu_codename", lambda: "jammy") + monkeypatch.setattr(host_setup.shutil, "which", lambda name: f"/usr/bin/{name}") + monkeypatch.setattr( + host_setup, + "install_packages_if_missing", + lambda packages, **kwargs: package_calls.append((packages, kwargs)), + ) + monkeypatch.setattr( + host_setup, + "write_system_file", + lambda path, content, **kwargs: writes.append((path, content, kwargs)), + ) + monkeypatch.setattr( + host_setup, + "run_command", + lambda command, **kwargs: commands.append((command, kwargs)), + ) + return package_calls, writes, commands + + +def _gpg_listing(*primary_fingerprints: str) -> bytes: + records = [] + for fingerprint in primary_fingerprints: + records.extend(("pub::::::::::", f"fpr:::::::::{fingerprint}:")) + return "\n".join(records).encode() + + +def test_setup_cmake_verifies_kitware_key_before_install(monkeypatch): + package_calls, writes, commands = _prepare_cmake_setup(monkeypatch) + key = b"armored Kitware key" + key_listing = ( + _gpg_listing(host_setup._KITWARE_ARCHIVE_KEY_FINGERPRINT) + + b"\nsub::::::::::\nfpr:::::::::8DB54F8C710EB2D4EF4F1BFA65ADECD7A7039392:" + ) + dearmored = b"dearmored Kitware key" + outputs = iter((key, key_listing, dearmored)) + subprocess_calls = [] + + def fake_subprocess_run(command, **kwargs): + subprocess_calls.append((command, kwargs)) + return subprocess.CompletedProcess(command, 0, stdout=next(outputs), stderr=b"") + + monkeypatch.setattr(host_setup.subprocess, "run", fake_subprocess_run) + + host_setup.setup_cmake() + + assert [call[0] for call in subprocess_calls] == [ + ["/usr/bin/wget", "-qO-", host_setup._KITWARE_ARCHIVE_KEY_URL], + [ + "/usr/bin/gpg", + "--batch", + "--no-options", + "--with-colons", + "--fingerprint", + "--show-keys", + ], + ["/usr/bin/gpg", "--dearmor"], + ] + assert subprocess_calls[1][1]["input"] == key + assert writes[0] == ( + "/usr/share/keyrings/kitware-archive-keyring.gpg", + dearmored, + {}, + ) + assert package_calls == [ + (["gpg", "wget"], {"dry_run": False}), + (["cmake", "cmake-curses-gui"], {"dry_run": False}), + ] + assert commands == [(["apt-get", "update"], {"dry_run": False, "as_root": True})] + + +@pytest.mark.parametrize( + "fingerprints", + [ + (), + ("0" * 40,), + (host_setup._KITWARE_ARCHIVE_KEY_FINGERPRINT, "0" * 40), + ], +) +def test_setup_cmake_rejects_unexpected_kitware_primary_keys(monkeypatch, capsys, fingerprints): + _package_calls, writes, commands = _prepare_cmake_setup(monkeypatch) + key = b"untrusted armored key" + outputs = iter((key, _gpg_listing(*fingerprints))) + subprocess_calls = [] + + def fake_subprocess_run(command, **kwargs): + subprocess_calls.append((command, kwargs)) + return subprocess.CompletedProcess(command, 0, stdout=next(outputs), stderr=b"") + + monkeypatch.setattr(host_setup.subprocess, "run", fake_subprocess_run) + + with pytest.raises(SystemExit) as exc_info: + host_setup.setup_cmake() + + assert exc_info.value.code == 1 + assert host_setup._KITWARE_ARCHIVE_KEY_FINGERPRINT in capsys.readouterr().err + assert len(subprocess_calls) == 2 + assert writes == [] + assert commands == [] + + def test_install_packages_if_missing_installs_only_missing_and_pinned(monkeypatch): installed = {"already": "1.0.0"} updates = [] From b9e498e0fc3687363b7af766da1162c777a62382 Mon Sep 17 00:00:00 2001 From: Wenqi Li Date: Mon, 31 Aug 2026 17:45:44 +0100 Subject: [PATCH 2/2] refactor(setup): simplify Kitware key verification Use a compact Kitware-specific fingerprint predicate and replace the setup-wide mocks with one focused unit test. Co-authored-by: Codex Signed-off-by: Wenqi Li --- src/holoscan_cli/utils/host_setup.py | 40 ++++------ tests/unit/test_host_setup.py | 112 ++------------------------- 2 files changed, 19 insertions(+), 133 deletions(-) diff --git a/src/holoscan_cli/utils/host_setup.py b/src/holoscan_cli/utils/host_setup.py index faaa6099..b6bf9634 100644 --- a/src/holoscan_cli/utils/host_setup.py +++ b/src/holoscan_cli/utils/host_setup.py @@ -47,7 +47,6 @@ "arm64": "fbb763bb818ca8ff3302a7764a95c63a42d80b7f864e87639833c55e59b6aadf", "linux": "a31a87a87593f5ca575d924f5e12cd0fcda1c81528f4cc3aebe0669e1643678f", } -_KITWARE_ARCHIVE_KEY_URL = "https://apt.kitware.com/keys/kitware-archive-latest.asc" # Published at https://apt.kitware.com/. Update deliberately when Kitware rotates its key. _KITWARE_ARCHIVE_KEY_FINGERPRINT = "4DBEBE3EEC96E7B8C6EC5BE99E92FDC6C5B9BA75" @@ -212,24 +211,13 @@ def get_ubuntu_codename() -> str: # ---- high-level setup_* orchestrators --------------------------------------- -def _primary_key_fingerprints(key_listing: bytes) -> List[str]: - """Extract primary-key fingerprints from GPG's machine-readable output.""" - fingerprints: List[str] = [] - primary_index = None - for line in key_listing.decode(errors="replace").splitlines(): - fields = line.split(":") - record_type = fields[0] - if record_type == "pub": - fingerprints.append("") - primary_index = len(fingerprints) - 1 - elif record_type == "sub": - primary_index = None - elif record_type == "fpr" and primary_index is not None: - fingerprint = fields[9] if len(fields) > 9 else "" - if re.fullmatch(r"[0-9A-Fa-f]+", fingerprint): - fingerprints[primary_index] = fingerprint.upper() - primary_index = None - return fingerprints +def _is_expected_kitware_key(key_listing: bytes) -> bool: + """Return whether GPG reported only Kitware's expected primary key.""" + records = key_listing.decode(errors="replace").splitlines() + fingerprints = [record.split(":")[9].upper() for record in records if record.startswith("fpr:")] + return sum(record.startswith("pub:") for record in records) == 1 and fingerprints[:1] == [ + _KITWARE_ARCHIVE_KEY_FINGERPRINT + ] def setup_cmake(min_version: str = "3.26.4", dry_run: bool = False) -> None: @@ -259,7 +247,7 @@ def setup_cmake(min_version: str = "3.26.4", dry_run: bool = False) -> None: gpg = shutil.which("gpg") or "/usr/bin/gpg" try: key = subprocess.run( - [wget, "-qO-", _KITWARE_ARCHIVE_KEY_URL], + [wget, "-qO-", "https://apt.kitware.com/keys/kitware-archive-latest.asc"], check=True, capture_output=True, ).stdout @@ -276,12 +264,10 @@ def setup_cmake(min_version: str = "3.26.4", dry_run: bool = False) -> None: check=True, capture_output=True, ).stdout - fingerprints = _primary_key_fingerprints(key_listing) - if fingerprints != [_KITWARE_ARCHIVE_KEY_FINGERPRINT]: - found = ", ".join(fingerprint or "" for fingerprint in fingerprints) + if not _is_expected_kitware_key(key_listing): fatal( "Kitware apt archive key fingerprint verification failed: " - f"expected {_KITWARE_ARCHIVE_KEY_FINGERPRINT}, found {found or ''}." + f"expected {_KITWARE_ARCHIVE_KEY_FINGERPRINT}." ) dearmored = subprocess.run( [gpg, "--dearmor"], @@ -290,11 +276,11 @@ def setup_cmake(min_version: str = "3.26.4", dry_run: bool = False) -> None: capture_output=True, ).stdout except FileNotFoundError as e: - fatal(f"Failed to download or process the Kitware apt archive key: {e}") + fatal(f"Failed to download the Kitware apt archive key: {e}") except subprocess.CalledProcessError as e: fatal( - "Failed to download or process the Kitware apt archive key: " - f"{e.stderr.decode(errors='replace').strip()}" + "Failed to download the Kitware apt archive key " + f"(is the network available?): {e.stderr.decode(errors='replace').strip()}" ) write_system_file(keyring_path, dearmored) write_system_file("/etc/apt/sources.list.d/kitware.list", source_line) diff --git a/tests/unit/test_host_setup.py b/tests/unit/test_host_setup.py index 8a1993d0..b107550f 100644 --- a/tests/unit/test_host_setup.py +++ b/tests/unit/test_host_setup.py @@ -3,118 +3,18 @@ from __future__ import annotations -import subprocess - import pytest from holoscan_cli.utils import host_setup -def _prepare_cmake_setup(monkeypatch): - package_calls = [] - writes = [] - commands = [] - monkeypatch.setattr(host_setup, "_apt_updated", False) - monkeypatch.setattr(host_setup, "get_installed_package_version", lambda _: None) - monkeypatch.setattr(host_setup, "get_ubuntu_codename", lambda: "jammy") - monkeypatch.setattr(host_setup.shutil, "which", lambda name: f"/usr/bin/{name}") - monkeypatch.setattr( - host_setup, - "install_packages_if_missing", - lambda packages, **kwargs: package_calls.append((packages, kwargs)), - ) - monkeypatch.setattr( - host_setup, - "write_system_file", - lambda path, content, **kwargs: writes.append((path, content, kwargs)), - ) - monkeypatch.setattr( - host_setup, - "run_command", - lambda command, **kwargs: commands.append((command, kwargs)), - ) - return package_calls, writes, commands - - -def _gpg_listing(*primary_fingerprints: str) -> bytes: - records = [] - for fingerprint in primary_fingerprints: - records.extend(("pub::::::::::", f"fpr:::::::::{fingerprint}:")) - return "\n".join(records).encode() - +def test_kitware_key_requires_one_matching_primary_key(): + expected = host_setup._KITWARE_ARCHIVE_KEY_FINGERPRINT.encode() + listing = b"pub::::::::::\nfpr:::::::::" + expected + b":\nsub::::::::::\nfpr:::::::::1234:" -def test_setup_cmake_verifies_kitware_key_before_install(monkeypatch): - package_calls, writes, commands = _prepare_cmake_setup(monkeypatch) - key = b"armored Kitware key" - key_listing = ( - _gpg_listing(host_setup._KITWARE_ARCHIVE_KEY_FINGERPRINT) - + b"\nsub::::::::::\nfpr:::::::::8DB54F8C710EB2D4EF4F1BFA65ADECD7A7039392:" - ) - dearmored = b"dearmored Kitware key" - outputs = iter((key, key_listing, dearmored)) - subprocess_calls = [] - - def fake_subprocess_run(command, **kwargs): - subprocess_calls.append((command, kwargs)) - return subprocess.CompletedProcess(command, 0, stdout=next(outputs), stderr=b"") - - monkeypatch.setattr(host_setup.subprocess, "run", fake_subprocess_run) - - host_setup.setup_cmake() - - assert [call[0] for call in subprocess_calls] == [ - ["/usr/bin/wget", "-qO-", host_setup._KITWARE_ARCHIVE_KEY_URL], - [ - "/usr/bin/gpg", - "--batch", - "--no-options", - "--with-colons", - "--fingerprint", - "--show-keys", - ], - ["/usr/bin/gpg", "--dearmor"], - ] - assert subprocess_calls[1][1]["input"] == key - assert writes[0] == ( - "/usr/share/keyrings/kitware-archive-keyring.gpg", - dearmored, - {}, - ) - assert package_calls == [ - (["gpg", "wget"], {"dry_run": False}), - (["cmake", "cmake-curses-gui"], {"dry_run": False}), - ] - assert commands == [(["apt-get", "update"], {"dry_run": False, "as_root": True})] - - -@pytest.mark.parametrize( - "fingerprints", - [ - (), - ("0" * 40,), - (host_setup._KITWARE_ARCHIVE_KEY_FINGERPRINT, "0" * 40), - ], -) -def test_setup_cmake_rejects_unexpected_kitware_primary_keys(monkeypatch, capsys, fingerprints): - _package_calls, writes, commands = _prepare_cmake_setup(monkeypatch) - key = b"untrusted armored key" - outputs = iter((key, _gpg_listing(*fingerprints))) - subprocess_calls = [] - - def fake_subprocess_run(command, **kwargs): - subprocess_calls.append((command, kwargs)) - return subprocess.CompletedProcess(command, 0, stdout=next(outputs), stderr=b"") - - monkeypatch.setattr(host_setup.subprocess, "run", fake_subprocess_run) - - with pytest.raises(SystemExit) as exc_info: - host_setup.setup_cmake() - - assert exc_info.value.code == 1 - assert host_setup._KITWARE_ARCHIVE_KEY_FINGERPRINT in capsys.readouterr().err - assert len(subprocess_calls) == 2 - assert writes == [] - assert commands == [] + assert host_setup._is_expected_kitware_key(listing) + assert not host_setup._is_expected_kitware_key(listing.replace(expected, b"0" * 40)) + assert not host_setup._is_expected_kitware_key(listing + b"\npub::::::::::") def test_install_packages_if_missing_installs_only_missing_and_pinned(monkeypatch):