Require signed agent releases (Refs #64)
This commit is contained in:
+153
-6
@@ -5,7 +5,9 @@ import io
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
import shutil
|
||||
import stat
|
||||
import subprocess
|
||||
import tempfile
|
||||
import time
|
||||
import unittest
|
||||
@@ -41,6 +43,7 @@ class UpdateTests(unittest.TestCase):
|
||||
directory.mkdir(parents=True)
|
||||
paths = {
|
||||
"AGENT_CONFIG": self.config_dir / "agent.json",
|
||||
"RELEASE_KEYRING": self.config_dir / "release-signing.gpg",
|
||||
"AGENT_EXECUTABLE": self.bin_dir / "electric-cherry-agent.py",
|
||||
"UNINSTALL_EXECUTABLE": self.bin_dir / "electric-cherry-uninstall.py",
|
||||
"UNINSTALL_REQUEST": self.config_dir / "decommission.json",
|
||||
@@ -62,9 +65,12 @@ class UpdateTests(unittest.TestCase):
|
||||
if directory and not path.is_dir() or not directory and not path.is_file():
|
||||
raise module.UpdateError("Wrong file type")
|
||||
return path
|
||||
self.patches.extend([patch.object(module, "trusted", side_effect=trusted_for_test), patch.object(module, "sync_parent"), patch.object(module, "systemctl", side_effect=self.systemctl)])
|
||||
self.signature_patch = patch.object(module, "verify_release_signature")
|
||||
self.patches.extend([patch.object(module, "trusted", side_effect=trusted_for_test), patch.object(module, "sync_parent"), patch.object(module, "systemctl", side_effect=self.systemctl), self.signature_patch])
|
||||
for item in self.patches:
|
||||
item.start()
|
||||
mocked = item.start()
|
||||
if item is self.signature_patch:
|
||||
self.signature_verifier = mocked
|
||||
self.config = {"hostId": "host-one", "root": str(self.root), "controller": "https://controller.example", "credential": "secret-enrollment-credential"}
|
||||
self.config_bytes = json.dumps(self.config).encode()
|
||||
module.AGENT_CONFIG.write_bytes(self.config_bytes)
|
||||
@@ -78,7 +84,8 @@ class UpdateTests(unittest.TestCase):
|
||||
self.write_health = True
|
||||
self.downloads = []
|
||||
self.source = CANDIDATE
|
||||
self.release = {"version": "1.1.0", "sha256": module.sha256(CANDIDATE), "protocolVersion": 1, "updaterProtocolVersion": 1, "platform": "debian-amd64"}
|
||||
self.signature = b"detached signature"
|
||||
self.release = {"version": "1.1.0", "sha256": module.sha256(CANDIDATE), "protocolVersion": 1, "updaterProtocolVersion": 2, "platform": "debian-amd64"}
|
||||
|
||||
def tearDown(self):
|
||||
for item in reversed(self.patches):
|
||||
@@ -108,6 +115,10 @@ class UpdateTests(unittest.TestCase):
|
||||
self.downloads.append(path)
|
||||
if path.endswith("manifest.json"):
|
||||
return json.dumps(self.release).encode()
|
||||
if path.endswith("manifest.json.sig"):
|
||||
if isinstance(self.signature, Exception):
|
||||
raise self.signature
|
||||
return self.signature
|
||||
if path.endswith("ec-agent.py"):
|
||||
return self.source
|
||||
self.fail("Unexpected download " + path)
|
||||
@@ -128,6 +139,7 @@ class UpdateTests(unittest.TestCase):
|
||||
|
||||
def test_verified_upgrade_waits_for_authenticated_candidate_health(self):
|
||||
self.assertEqual(self.updater().run(), "updated")
|
||||
self.signature_verifier.assert_called_once_with(json.dumps(self.release).encode(), self.signature)
|
||||
self.assertEqual(module.AGENT_EXECUTABLE.read_bytes(), CANDIDATE)
|
||||
self.assertEqual(module.UNINSTALL_EXECUTABLE.read_bytes(), CANDIDATE)
|
||||
self.assertEqual(module.AGENT_CONFIG.read_bytes(), self.config_bytes)
|
||||
@@ -137,6 +149,36 @@ class UpdateTests(unittest.TestCase):
|
||||
self.assertFalse((module.UPDATE_DIR / "agent.backup.py").exists())
|
||||
self.assertEqual(next(command for command in self.commands if command[0] != "show"), ("stop", module.AGENT_SERVICE))
|
||||
|
||||
def test_invalid_signature_rejects_attacker_source_and_matching_manifest_checksum(self):
|
||||
self.source = b'VERSION = "1.1.0"\nraise RuntimeError("attacker code")\n'
|
||||
self.release["sha256"] = module.sha256(self.source)
|
||||
self.signature_verifier.side_effect = module.IntegrityError("Invalid release signature")
|
||||
self.assertEqual(self.updater().run(), "failed")
|
||||
self.assert_unchanged()
|
||||
self.assertEqual(self.commands, [])
|
||||
self.assertEqual(self.downloads, ["/api/agent/release/manifest.json", "/api/agent/release/manifest.json.sig"])
|
||||
self.assertFalse(module.UPDATE_JOURNAL.exists())
|
||||
self.assertFalse((module.UPDATE_DIR / "failed-release.json").exists())
|
||||
self.assertNotIn("targetVersion", self.status())
|
||||
|
||||
def test_signature_is_verified_before_manifest_json_is_parsed(self):
|
||||
updater = self.updater()
|
||||
updater.fetch = lambda path, limit: b"untrusted invalid JSON" if path.endswith("manifest.json") else self.signature
|
||||
self.signature_verifier.side_effect = module.IntegrityError("Invalid release signature")
|
||||
with patch.object(module.json, "loads", side_effect=AssertionError("Parsed unsigned metadata")):
|
||||
with self.assertRaisesRegex(module.IntegrityError, "Invalid release signature"):
|
||||
updater.manifest()
|
||||
self.signature_verifier.assert_called_once_with(b"untrusted invalid JSON", self.signature)
|
||||
|
||||
def test_missing_signature_never_downloads_code_or_stops_agent(self):
|
||||
self.signature = module.ContactError("No signature is published")
|
||||
self.assertEqual(self.updater().run(), "failed")
|
||||
self.assertIn("signature", self.status()["error"])
|
||||
self.assert_unchanged()
|
||||
self.assertEqual(self.commands, [])
|
||||
self.assertFalse(any(path.endswith("ec-agent.py") for path in self.downloads))
|
||||
self.signature_verifier.assert_not_called()
|
||||
|
||||
def test_equal_or_older_versions_do_not_download_or_restart(self):
|
||||
for target in ("1.0.0", "0.9.9"):
|
||||
with self.subTest(target=target):
|
||||
@@ -207,7 +249,7 @@ class UpdateTests(unittest.TestCase):
|
||||
self.downloads.clear()
|
||||
self.source = CANDIDATE
|
||||
self.assertEqual(self.updater().run(), "failed")
|
||||
self.assertEqual(self.downloads, ["/api/agent/release/manifest.json"])
|
||||
self.assertEqual(self.downloads, ["/api/agent/release/manifest.json", "/api/agent/release/manifest.json.sig"])
|
||||
|
||||
def test_invalid_or_mismatched_python_version_is_never_installed(self):
|
||||
for source in (b'VERSION = "1.1.0"\ninvalid syntax ?\n', b'VERSION = "5.0.0"\n'):
|
||||
@@ -220,7 +262,7 @@ class UpdateTests(unittest.TestCase):
|
||||
|
||||
def test_incompatible_protocol_platform_or_version_never_downloads_code(self):
|
||||
original = dict(self.release)
|
||||
for key, value in (("protocolVersion", 2), ("protocolVersion", True), ("platform", "linux-arm64"), ("version", "1.1.0-rc1"), ("version", "01.1.0"), ("updaterProtocolVersion", 2)):
|
||||
for key, value in (("protocolVersion", 2), ("protocolVersion", True), ("platform", "linux-arm64"), ("version", "1.1.0-rc1"), ("version", "01.1.0"), ("updaterProtocolVersion", 1), ("updaterProtocolVersion", 3)):
|
||||
with self.subTest(key=key, value=value):
|
||||
self.release = {**original, key: value}
|
||||
self.assertIn(self.updater().run(), ("failed", "deferred"))
|
||||
@@ -239,7 +281,7 @@ class UpdateTests(unittest.TestCase):
|
||||
self.assertEqual(sum(command == ("start", module.AGENT_SERVICE) for command in self.commands), 2)
|
||||
self.downloads.clear()
|
||||
self.assertEqual(self.updater().run(), "failed")
|
||||
self.assertEqual(self.downloads, ["/api/agent/release/manifest.json"])
|
||||
self.assertEqual(self.downloads, ["/api/agent/release/manifest.json", "/api/agent/release/manifest.json.sig"])
|
||||
|
||||
def test_changed_artifact_of_failed_target_can_be_retried(self):
|
||||
updater = self.updater()
|
||||
@@ -266,9 +308,12 @@ class UpdateTests(unittest.TestCase):
|
||||
def test_crash_between_executable_replacements_recovers_from_backups(self):
|
||||
self.crash_during_second_replacement()
|
||||
self.downloads.clear()
|
||||
self.signature_verifier.reset_mock()
|
||||
self.signature_verifier.side_effect = module.IntegrityError("Signing keyring unavailable during recovery")
|
||||
self.assertEqual(self.updater().run(), "failed")
|
||||
self.assert_unchanged()
|
||||
self.assertEqual(self.downloads, [])
|
||||
self.signature_verifier.assert_not_called()
|
||||
self.assertTrue(self.active)
|
||||
self.assertFalse(module.UPDATE_JOURNAL.exists())
|
||||
|
||||
@@ -413,6 +458,108 @@ class UpdateSecurityTests(unittest.TestCase):
|
||||
with self.subTest(value=value), self.assertRaises(module.IntegrityError):
|
||||
module.version(value)
|
||||
|
||||
def test_release_keyring_rejects_untrusted_ownership_permissions_and_file_types(self):
|
||||
for mode, uid in ((stat.S_IFREG | 0o644, 1000), (stat.S_IFREG | 0o664, 0), (stat.S_IFREG | 0o646, 0), (stat.S_IFLNK | 0o777, 0), (stat.S_IFDIR | 0o755, 0)):
|
||||
def file_info(path):
|
||||
selected_mode, selected_uid = (mode, uid) if path == module.RELEASE_KEYRING else (stat.S_IFDIR | 0o755, 0)
|
||||
return os.stat_result((selected_mode, 1, 1, 1, selected_uid, 0, 0, 0, 0, 0))
|
||||
with self.subTest(mode=mode, uid=uid), patch.object(Path, "lstat", autospec=True, side_effect=file_info), patch.object(module.subprocess, "run") as run:
|
||||
with self.assertRaisesRegex(module.IntegrityError, "keyring is missing or unsafe"):
|
||||
module.verify_release_signature(b"manifest", b"signature")
|
||||
run.assert_not_called()
|
||||
|
||||
|
||||
class SignatureVerificationFixture:
|
||||
def setUp(self):
|
||||
self.temporary = tempfile.TemporaryDirectory()
|
||||
self.addCleanup(self.temporary.cleanup)
|
||||
self.base = Path(self.temporary.name)
|
||||
self.keyring = self.base / "release-signing.gpg"
|
||||
self.keyring.write_bytes(b"trusted host public keys")
|
||||
self.update_dir = self.base / "update"
|
||||
self.update_dir.mkdir(mode=0o700)
|
||||
for name, value in (("RELEASE_KEYRING", self.keyring), ("UPDATE_DIR", self.update_dir)):
|
||||
entry = patch.object(module, name, value)
|
||||
entry.start()
|
||||
self.addCleanup(entry.stop)
|
||||
# Test fixtures are owned by the test runner rather than system root.
|
||||
# Root ownership and unsafe modes are checked independently above.
|
||||
entry = patch.object(module, "trusted", side_effect=lambda path, **_: Path(path))
|
||||
self.trusted = entry.start()
|
||||
self.addCleanup(entry.stop)
|
||||
|
||||
|
||||
class SignatureVerificationTests(SignatureVerificationFixture, unittest.TestCase):
|
||||
def test_gpgv_uses_only_host_keyring_and_exact_bytes_with_an_isolated_home(self):
|
||||
manifest = b'{"version": "1.2.3"}\n'
|
||||
signature = b"binary detached signature\x00\xff"
|
||||
def verify(command, **kwargs):
|
||||
home = Path(command[2])
|
||||
self.assertEqual(command[:2], ["gpgv", "--homedir"])
|
||||
self.assertEqual(command[3:5], ["--keyring", str(self.keyring)])
|
||||
self.assertEqual(home.parent, self.update_dir)
|
||||
self.assertEqual(stat.S_IMODE(home.stat().st_mode), 0o700)
|
||||
self.assertEqual(Path(command[5]).read_bytes(), signature)
|
||||
self.assertEqual(Path(command[6]).read_bytes(), manifest)
|
||||
self.assertEqual(kwargs["timeout"], 30)
|
||||
self.assertEqual(kwargs["stdin"], subprocess.DEVNULL)
|
||||
return subprocess.CompletedProcess(command, 0)
|
||||
with patch.object(module.subprocess, "run", side_effect=verify) as run:
|
||||
module.verify_release_signature(manifest, signature)
|
||||
run.assert_called_once()
|
||||
self.assertEqual(self.trusted.call_args_list[0].args, (self.keyring,))
|
||||
self.assertEqual(list(self.update_dir.iterdir()), [])
|
||||
|
||||
def test_missing_keyring_or_signature_never_runs_a_verifier(self):
|
||||
with patch.object(module.subprocess, "run") as run:
|
||||
self.trusted.side_effect = module.UpdateError("Missing keyring")
|
||||
with self.assertRaisesRegex(module.IntegrityError, "keyring"):
|
||||
module.verify_release_signature(b"manifest", b"signature")
|
||||
self.trusted.side_effect = lambda path, **_: Path(path)
|
||||
with self.assertRaisesRegex(module.IntegrityError, "detached manifest signature"):
|
||||
module.verify_release_signature(b"manifest", b"")
|
||||
run.assert_not_called()
|
||||
|
||||
def test_bad_unknown_or_unavailable_verifier_fails_closed_and_cleans_temporary_files(self):
|
||||
for result in (subprocess.CompletedProcess([], 1), subprocess.CompletedProcess([], 2), FileNotFoundError("gpgv missing"), subprocess.TimeoutExpired("gpgv", 30)):
|
||||
with self.subTest(result=result):
|
||||
kwargs = {"side_effect": result} if isinstance(result, Exception) else {"return_value": result}
|
||||
with patch.object(module.subprocess, "run", **kwargs):
|
||||
with self.assertRaises(module.IntegrityError):
|
||||
module.verify_release_signature(b"manifest", b"signature")
|
||||
self.assertEqual(list(self.update_dir.iterdir()), [])
|
||||
|
||||
|
||||
@unittest.skipUnless(shutil.which("gpg") and shutil.which("gpgv"), "GnuPG signing and verification tools are required")
|
||||
class RealSignatureVerificationTests(SignatureVerificationFixture, unittest.TestCase):
|
||||
@classmethod
|
||||
def setUpClass(cls):
|
||||
cls.keys = tempfile.TemporaryDirectory()
|
||||
cls.addClassCleanup(cls.keys.cleanup)
|
||||
cls.homes = []
|
||||
for index in range(2):
|
||||
home = Path(cls.keys.name) / str(index)
|
||||
home.mkdir(mode=0o700)
|
||||
cls.homes.append(home)
|
||||
subprocess.run(["gpg", "--homedir", str(home), "--batch", "--pinentry-mode", "loopback", "--passphrase", "", "--quick-generate-key", f"Release Test {index} <release-{index}@example.test>", "ed25519", "sign", "0"], check=True, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE)
|
||||
if shutil.which("gpgconf"):
|
||||
cls.addClassCleanup(subprocess.run, ["gpgconf", "--homedir", str(home), "--kill", "gpg-agent"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False)
|
||||
cls.public_key = subprocess.run(["gpg", "--homedir", str(cls.homes[0]), "--batch", "--export"], check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE).stdout
|
||||
|
||||
def sign(self, manifest, index=0):
|
||||
return subprocess.run(["gpg", "--homedir", str(self.homes[index]), "--batch", "--pinentry-mode", "loopback", "--passphrase", "", "--digest-algo", "SHA512", "--detach-sign"], input=manifest, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True).stdout
|
||||
|
||||
def test_real_signatures_authenticate_exact_manifest_and_reject_unknown_keys(self):
|
||||
self.keyring.write_bytes(self.public_key)
|
||||
manifest = b'{"version":"1.1.0","sha256":"trusted-code-checksum"}\n'
|
||||
signature = self.sign(manifest)
|
||||
module.verify_release_signature(manifest, signature)
|
||||
for changed_manifest, changed_signature in ((manifest + b" ", signature), (manifest.replace(b"trusted-code-checksum", b"attacker-code-checksum"), signature), (manifest, signature[:-1]), (manifest, self.sign(manifest, 1))):
|
||||
with self.subTest(manifest=changed_manifest, signature=changed_signature):
|
||||
with self.assertRaises(module.IntegrityError):
|
||||
module.verify_release_signature(changed_manifest, changed_signature)
|
||||
self.assertEqual(list(self.update_dir.iterdir()), [])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user