Refresh agent signing keys from repository (Refs #64)
This commit is contained in:
+117
-29
@@ -43,7 +43,6 @@ 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",
|
||||
@@ -65,6 +64,10 @@ 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.public_key = b"repository public key"
|
||||
key_patch = patch.object(module, "fetch_release_key", return_value=self.public_key)
|
||||
self.fetch_key = key_patch.start()
|
||||
self.addCleanup(key_patch.stop)
|
||||
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:
|
||||
@@ -139,7 +142,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.signature_verifier.assert_called_once_with(json.dumps(self.release).encode(), self.signature, self.public_key)
|
||||
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)
|
||||
@@ -168,7 +171,7 @@ class UpdateTests(unittest.TestCase):
|
||||
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)
|
||||
self.signature_verifier.assert_called_once_with(b"untrusted invalid JSON", self.signature, self.public_key)
|
||||
|
||||
def test_missing_signature_never_downloads_code_or_stops_agent(self):
|
||||
self.signature = module.ContactError("No signature is published")
|
||||
@@ -188,6 +191,32 @@ class UpdateTests(unittest.TestCase):
|
||||
self.assertFalse(any(path.endswith("ec-agent.py") for path in self.downloads))
|
||||
self.assertEqual(self.commands, [])
|
||||
|
||||
def test_every_check_fetches_key_before_controller_even_without_a_new_version(self):
|
||||
self.release["version"] = "1.0.0"
|
||||
keys = [b"first public key", b"replacement public key"]
|
||||
self.fetch_key.side_effect = keys
|
||||
updater = self.updater()
|
||||
def controller_fetch(path, limit):
|
||||
self.assertGreater(self.fetch_key.call_count, self.signature_verifier.call_count)
|
||||
return self.fetch(path, limit)
|
||||
updater.fetch = controller_fetch
|
||||
for _ in keys:
|
||||
self.assertEqual(updater.run(), "current")
|
||||
self.assertEqual(self.fetch_key.call_count, 2)
|
||||
self.assertEqual([call.args[2] for call in self.signature_verifier.call_args_list], keys)
|
||||
self.assert_unchanged()
|
||||
self.assertEqual(self.commands, [])
|
||||
|
||||
def test_repository_failure_never_uses_old_host_key_or_contacts_controller(self):
|
||||
(self.config_dir / "release-signing.gpg").write_bytes(b"old trusted key")
|
||||
self.fetch_key.side_effect = module.ContactError("Repository unavailable")
|
||||
self.assertEqual(self.updater().run(), "deferred")
|
||||
self.assert_unchanged()
|
||||
self.assertEqual(self.downloads, [])
|
||||
self.assertEqual(self.commands, [])
|
||||
self.assertTrue(self.active)
|
||||
self.signature_verifier.assert_not_called()
|
||||
|
||||
def test_busy_operation_defers_update_without_stopping_agent(self):
|
||||
with module.exclusive_lock(self.root / "agent" / "activity.lock"):
|
||||
self.assertEqual(self.updater().run(), "deferred")
|
||||
@@ -309,11 +338,14 @@ class UpdateTests(unittest.TestCase):
|
||||
self.crash_during_second_replacement()
|
||||
self.downloads.clear()
|
||||
self.signature_verifier.reset_mock()
|
||||
self.fetch_key.reset_mock()
|
||||
self.fetch_key.side_effect = module.ContactError("Repository offline during recovery")
|
||||
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.fetch_key.assert_not_called()
|
||||
self.assertTrue(self.active)
|
||||
self.assertFalse(module.UPDATE_JOURNAL.exists())
|
||||
|
||||
@@ -458,15 +490,48 @@ 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 RepositoryKeyDownloadTests(unittest.TestCase):
|
||||
def test_fixed_https_url_no_credentials_no_cache_and_no_redirects(self):
|
||||
with patch.object(module.urllib.request, "build_opener") as opener:
|
||||
opener.return_value.open.return_value = Response(b"public keys")
|
||||
self.assertEqual(module.fetch_release_key(), b"public keys")
|
||||
request = opener.return_value.open.call_args.args[0]
|
||||
self.assertEqual(request.full_url, "https://gitea.thewrightserver.net/josh/electric-cherry/raw/branch/main/agent/release-signing.gpg")
|
||||
self.assertIsNone(request.get_header("Authorization"))
|
||||
self.assertIsNone(request.get_header("Cookie"))
|
||||
self.assertEqual(request.get_header("Cache-control"), "no-cache")
|
||||
self.assertEqual(opener.return_value.open.call_args.kwargs["timeout"], 25)
|
||||
redirect = opener.call_args.args[0]
|
||||
for destination in ("https://attacker.example/key.gpg", "http://gitea.thewrightserver.net/key.gpg"):
|
||||
with self.subTest(destination=destination), self.assertRaises(module.ContactError):
|
||||
redirect.redirect_request(None, None, 302, "redirect", {}, destination)
|
||||
|
||||
def test_missing_empty_oversized_and_truncated_keys_fail_closed(self):
|
||||
cases = [Response(b""), Response(b"x", {"Content-Length": str(module.MAX_KEYRING + 1)}),
|
||||
Response(b"x" * (module.MAX_KEYRING + 1)),
|
||||
Response(b"partial", {"Content-Length": "100"}),
|
||||
Response(b"key", {"Content-Length": "invalid"})]
|
||||
for status in (301, 304, 404, 500):
|
||||
response = Response(b"public key")
|
||||
response.status = status
|
||||
cases.append(response)
|
||||
for response in cases:
|
||||
with self.subTest(status=response.status, headers=response.headers), patch.object(module.urllib.request, "build_opener") as opener:
|
||||
opener.return_value.open.return_value = response
|
||||
with self.assertRaises(module.UpdateError):
|
||||
module.fetch_release_key()
|
||||
with patch.object(module.urllib.request, "build_opener") as opener:
|
||||
opener.return_value.open.side_effect = OSError("TLS or network failure")
|
||||
with self.assertRaisesRegex(module.ContactError, "release-signing repository is unavailable"):
|
||||
module.fetch_release_key()
|
||||
|
||||
def test_trickling_repository_download_has_a_deadline(self):
|
||||
with patch.object(module.urllib.request, "build_opener") as opener:
|
||||
opener.return_value.open.return_value = Response(b"public key")
|
||||
with patch.object(module.time, "monotonic", side_effect=[0, 0, 121]):
|
||||
with self.assertRaisesRegex(module.ContactError, "timed out"):
|
||||
module.fetch_release_key()
|
||||
|
||||
|
||||
class SignatureVerificationFixture:
|
||||
@@ -478,10 +543,9 @@ class SignatureVerificationFixture:
|
||||
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)
|
||||
entry = patch.object(module, "UPDATE_DIR", self.update_dir)
|
||||
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))
|
||||
@@ -490,13 +554,14 @@ class SignatureVerificationFixture:
|
||||
|
||||
|
||||
class SignatureVerificationTests(SignatureVerificationFixture, unittest.TestCase):
|
||||
def test_gpgv_uses_only_host_keyring_and_exact_bytes_with_an_isolated_home(self):
|
||||
def test_gpgv_uses_only_downloaded_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(command[3:5], ["--keyring", str(home / "release-signing.gpg")])
|
||||
self.assertEqual(Path(command[4]).read_bytes(), self.keyring.read_bytes())
|
||||
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)
|
||||
@@ -505,19 +570,18 @@ class SignatureVerificationTests(SignatureVerificationFixture, unittest.TestCase
|
||||
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)
|
||||
module.verify_release_signature(manifest, signature, self.keyring.read_bytes())
|
||||
run.assert_called_once()
|
||||
self.assertEqual(self.trusted.call_args_list[0].args, (self.keyring,))
|
||||
self.assertEqual(self.trusted.call_args_list[0].args, (self.update_dir,))
|
||||
self.assertEqual(list(self.update_dir.iterdir()), [])
|
||||
|
||||
def test_missing_keyring_or_signature_never_runs_a_verifier(self):
|
||||
def test_empty_or_oversized_keyring_or_missing_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)
|
||||
for key in (b"", b"x" * (module.MAX_KEYRING + 1)):
|
||||
with self.subTest(size=len(key)), self.assertRaisesRegex(module.IntegrityError, "keyring"):
|
||||
module.verify_release_signature(b"manifest", b"signature", key)
|
||||
with self.assertRaisesRegex(module.IntegrityError, "detached manifest signature"):
|
||||
module.verify_release_signature(b"manifest", b"")
|
||||
module.verify_release_signature(b"manifest", b"", self.keyring.read_bytes())
|
||||
run.assert_not_called()
|
||||
|
||||
def test_bad_unknown_or_unavailable_verifier_fails_closed_and_cleans_temporary_files(self):
|
||||
@@ -526,7 +590,7 @@ class SignatureVerificationTests(SignatureVerificationFixture, unittest.TestCase
|
||||
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")
|
||||
module.verify_release_signature(b"manifest", b"signature", self.keyring.read_bytes())
|
||||
self.assertEqual(list(self.update_dir.iterdir()), [])
|
||||
|
||||
|
||||
@@ -549,15 +613,39 @@ class RealSignatureVerificationTests(SignatureVerificationFixture, unittest.Test
|
||||
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_repository_key_rotation_removes_old_signer_on_the_next_check(self):
|
||||
replacement = subprocess.run(["gpg", "--homedir", str(self.homes[1]), "--batch", "--export"],
|
||||
check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE).stdout
|
||||
manifest = json.dumps({"version": "1.1.0", "sha256": module.sha256(CANDIDATE),
|
||||
"protocolVersion": 1, "updaterProtocolVersion": 2,
|
||||
"platform": "debian-amd64",
|
||||
"keyUrl": "https://controller.example/attacker-key"}).encode()
|
||||
old_signature, new_signature = self.sign(manifest), self.sign(manifest, 1)
|
||||
updater = module.Updater()
|
||||
updater.installation = {"controller": "https://controller.example"}
|
||||
with patch.object(module.urllib.request, "build_opener") as opener:
|
||||
opener.return_value.open.side_effect = [Response(value) for value in (
|
||||
self.public_key, manifest, old_signature,
|
||||
replacement, manifest, old_signature,
|
||||
replacement, manifest, new_signature)]
|
||||
self.assertEqual(updater.manifest()["version"], "1.1.0")
|
||||
with self.assertRaisesRegex(module.IntegrityError, "not signed by a trusted release key"):
|
||||
updater.manifest()
|
||||
self.assertEqual(updater.manifest()["version"], "1.1.0")
|
||||
urls = [call.args[0].full_url for call in opener.return_value.open.call_args_list]
|
||||
self.assertEqual(urls[::3], [module.RELEASE_KEY_URL] * 3)
|
||||
self.assertNotIn("https://controller.example/attacker-key", urls)
|
||||
self.assertEqual(list(self.update_dir.iterdir()), [])
|
||||
|
||||
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)
|
||||
module.verify_release_signature(manifest, signature, self.keyring.read_bytes())
|
||||
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)
|
||||
module.verify_release_signature(changed_manifest, changed_signature, self.keyring.read_bytes())
|
||||
self.assertEqual(list(self.update_dir.iterdir()), [])
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user