diff --git a/scripts/auto_fetch.py b/scripts/auto_fetch.py index c39587b7..609160a6 100644 --- a/scripts/auto_fetch.py +++ b/scripts/auto_fetch.py @@ -25,7 +25,6 @@ import sys import urllib.error import urllib.parse import urllib.request -import zlib from pathlib import Path sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) @@ -147,35 +146,38 @@ def _size(entry: dict) -> int | None: return None -def verifiable(entry: dict) -> bool: - """Whether a download can be proved to be this file. +CRYPTO_HASHES = ( + ("sha256", hashlib.sha256), + ("sha1", hashlib.sha1), + ("md5", hashlib.md5), +) - A sha1, sha256 or md5 proves it, a crc32 only with the size beside it, - the way the resolver accepts one. Without any of them a search can find - a file of that name and never accept it. + +def verifiable(entry: dict) -> bool: + """Whether a network download can be proved to be this file. + + Only a cryptographic hash proves it. A crc32 and a size can be met by + crafted bytes, so they identify a file the database already holds + (step 1) and never accept one a remote source serves. """ - if any(_accepted(entry, field) for field in ("sha1", "sha256", "md5")): - return True - return bool(_crc32s(entry)) and _size(entry) is not None + return any(_accepted(entry, field) for field, _digest in CRYPTO_HASHES) def verify_content(data: bytes, expected: dict) -> bool: - """Verify downloaded content matches one of the expected hashes. + """Whether downloaded bytes match every cryptographic hash declared. - The strongest hash the entry declares decides. + Each declared field must hold the bytes' digest among its accepted + values. An entry declaring none is refused. """ - for field, digest in ( - ("sha1", hashlib.sha1), - ("sha256", hashlib.sha256), - ("md5", hashlib.md5), - ): - accepted = _accepted(expected, field) - if accepted: - return digest(data).hexdigest() in accepted - crcs, size = _crc32s(expected), _size(expected) - if crcs and size is not None: - return len(data) == size and f"{zlib.crc32(data) & 0xFFFFFFFF:08x}" in crcs - return False + declared = [ + (accepted, digest) + for field, digest in CRYPTO_HASHES + for accepted in [_accepted(expected, field)] + if accepted + ] + return bool(declared) and all( + digest(data).hexdigest() in accepted for accepted, digest in declared + ) def _read_limited( @@ -413,8 +415,8 @@ def fetch_missing( if not verifiable(entry): # Searched, a file of that name could only ever be refused. - print(" [-] No sha1, sha256, md5 or crc32 with size to check a " - "download against: not searched") + print(" [-] Cannot be verified from the network: declares no " + "cryptographic hash. Not searched, nothing written") still_missing.append(entry) stats["unverifiable"] += 1 continue @@ -565,7 +567,7 @@ def main() -> None: print( f"\n Results: {stats['found']} found, {stats['not_found']} not found, " f"{stats['errors']} not fully searched, " - f"{stats['unverifiable']} without a hash to search by" + f"{stats['unverifiable']} without a cryptographic hash" ) if args.create_issues and all_still_missing: diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index a263fb3e..3fe7785d 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -5605,19 +5605,50 @@ class AutoFetchProvesEveryHashItIsGiven(unittest.TestCase): DATA = b"firmware bytes" - def test_sha256_and_crc32_with_size_prove_a_download(self): + def test_a_sha256_proves_a_download(self): import hashlib - import zlib from scripts import auto_fetch sha256 = {"sha256": hashlib.sha256(self.DATA).hexdigest()} self.assertTrue(auto_fetch.verify_content(self.DATA, sha256)) self.assertFalse(auto_fetch.verify_content(b"other", sha256)) - crc = {"crc32": f"{zlib.crc32(self.DATA):08X}", "size": len(self.DATA)} - self.assertTrue(auto_fetch.verify_content(self.DATA, crc)) - self.assertFalse(auto_fetch.verify_content(self.DATA, dict(crc, size=1))) - self.assertFalse(auto_fetch.verifiable({"crc32": crc["crc32"]})) + + def test_every_declared_cryptographic_hash_must_match(self): + import hashlib + + from scripts import auto_fetch + + both = {"sha256": hashlib.sha256(self.DATA).hexdigest(), "md5": "0" * 32} + self.assertFalse(auto_fetch.verify_content(self.DATA, both)) + both["md5"] = hashlib.md5(self.DATA).hexdigest() + self.assertTrue(auto_fetch.verify_content(self.DATA, both)) + + def test_crc32_and_size_never_accept_a_download(self): + """A crc32 can be forged: bytes a source crafts to it would land in + bios/.""" + import urllib.request + import zlib + from unittest import mock + + from scripts import auto_fetch + + crc = {"name": "forged.bin", "system": "test-system", + "crc32": f"{zlib.crc32(self.DATA):08X}", "size": len(self.DATA)} + self.assertFalse(auto_fetch.verify_content(self.DATA, crc)) + self.assertFalse(auto_fetch.verifiable(crc)) + + served = io.BytesIO(self.DATA) + with tempfile.TemporaryDirectory() as bios, \ + mock.patch.object(urllib.request, "urlopen", lambda *a, **k: served), \ + mock.patch.object(auto_fetch, "step2_scan_branches", + lambda e: self.DATA), \ + contextlib.redirect_stdout(io.StringIO()) as out: + result = auto_fetch.fetch_missing([crc], {"files": {}}, bios) + self.assertEqual(list(Path(bios).rglob("*")), []) + self.assertEqual(result["stats"]["found"], 0) + self.assertEqual(result["stats"]["unverifiable"], 1) + self.assertIn("declares no cryptographic hash", out.getvalue()) def test_a_held_sha256_is_found_in_the_database(self): from scripts import auto_fetch @@ -5641,7 +5672,7 @@ class AutoFetchProvesEveryHashItIsGiven(unittest.TestCase): branches.assert_not_called() self.assertEqual(result["stats"]["unverifiable"], 1) self.assertNotIn("Would search", out.getvalue()) - self.assertIn("not searched", out.getvalue()) + self.assertIn("Not searched", out.getvalue()) class AForcedRehashIsCheckedAgainstTheLastOne(unittest.TestCase): diff --git a/wiki/tools.md b/wiki/tools.md index 51be8bab..cbaaf6ce 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -653,7 +653,7 @@ python scripts/refresh_stale.py --only platforms,targets --jobs 6 | `region_audit.py` | Confront every profile `region:` with the regions No-Intro names for the same dump (`--json`, `--db`, `--emulators-dir`). An opinion, never an authority: the code decides, and a disagreement is a line to read in the emulator's source | | `validate_pr.py` | Validate BIOS files in pull requests, post markdown report | | `validate_schemas.py` | Validate the data contracts: schemas and semantic invariants. `--source-only` checks `emulators/` and `platforms/` alone, which is what PR validation runs | -| `auto_fetch.py` | Fetch missing BIOS files from known sources (4-step pipeline). A download is accepted by the sha1, sha256 or md5 the entry declares, or its crc32 with the size. An entry with none of them is reported and not searched | +| `auto_fetch.py` | Fetch missing BIOS files from known sources (4-step pipeline). A download is accepted only when every cryptographic hash the entry declares (sha256, sha1, md5) matches. crc32 and size find a file the database already holds, never accept a download. An entry with no cryptographic hash is reported and not searched | | `list_platforms.py` | List active platforms (`--include-archived` adds the archived ones, used by CI) | | `download.py` | Download a pack from GitHub releases, every part checked (Python, stdlib only) | | `download.sh` | Same, as a shell one-liner (`curl` + `unzip`) |