diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ccdf72b..fa88b81 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,10 +17,31 @@ on: # seven others down with it, which is why it gets the full OS matrix rather # than a token Linux run. # -# Cargo.lock is gitignored (library convention), so `--locked` is deliberately -# NOT used: with no committed lockfile it fails rather than resolves. +# Cargo.lock is now committed (issue #15), so the resolved dependency graph is +# recorded and the `deny` job audits it. `--locked` is still deliberately NOT +# used, matching the binary crates' ci.yml: CI's sibling path-patch layout +# resolves the freemkv crates as path packages, which `--locked` can't satisfy. jobs: + # ── Single source of truth for the MSRV ───────────────────────────────────── + # + # `rust-version` in Cargo.toml is the ONE place the MSRV is declared. This job + # reads it; every job that must build/test AT the MSRV consumes + # `needs.resolve-msrv.outputs.v` instead of a hardcoded pin, so the floor + # cannot drift out of sync across workflow files. The `test` job building + # green at this value IS the verification that the declared MSRV is real. + # clippy deliberately does NOT use it — it runs on `stable` (lint debt is + # caught against current Rust). + resolve-msrv: + runs-on: ubuntu-latest + outputs: + v: ${{ steps.msrv.outputs.v }} + steps: + - uses: actions/checkout@v7 + - id: msrv + shell: bash + run: echo "v=$(grep -m1 '^rust-version' Cargo.toml | sed -E 's/.*\"([^\"]+)\".*/\1/')" >> "$GITHUB_OUTPUT" + # CI owns the changelog: main may never carry a released version without a # CHANGELOG.md entry for it. The release orchestrator dates an existing # `## [X.Y.Z]` heading — it does NOT invent one — so a version bump that @@ -48,24 +69,36 @@ jobs: echo "::warning::${msg} (allowed on ${GITHUB_REF_NAME:-unknown}; required before it can reach main)." lint: + needs: resolve-msrv runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - uses: dtolnay/rust-toolchain@1.88.0 + # fmt runs on the pinned MSRV toolchain; clippy runs on stable. Linting on + # stable makes new-lint debt a BLOCKING gate the pinned version would + # otherwise hide. The MSRV pin is installed second so it stays the default + # toolchain (fmt uses it implicitly); clippy is invoked with `+stable`. + - uses: dtolnay/rust-toolchain@stable with: - components: clippy, rustfmt + components: clippy + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ needs.resolve-msrv.outputs.v }} + components: rustfmt - uses: Swatinem/rust-cache@v2 with: shared-key: freemkv-unlock-ci - run: cargo fmt --check # --all-targets so the test code is linted too, not just the library. - - run: cargo clippy --all-targets -- -D warnings + - run: cargo +stable clippy --all-targets -- -D warnings test: + needs: resolve-msrv runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - uses: dtolnay/rust-toolchain@1.88.0 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ needs.resolve-msrv.outputs.v }} - uses: Swatinem/rust-cache@v2 with: shared-key: freemkv-unlock-ci @@ -79,13 +112,15 @@ jobs: # release gate). It mirrors the `test` job's environment — same pinned # toolchain, ubuntu-latest, no `if:` so it runs on every branch and PR just # as `test` does. `--locked` is intentionally omitted to match this crate's - # policy (Cargo.lock is gitignored; --locked would fail without it). + # policy (Cargo.lock is committed but --locked can't satisfy the path-patch layout). coverage: + needs: resolve-msrv runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - uses: dtolnay/rust-toolchain@1.88.0 + - uses: dtolnay/rust-toolchain@master with: + toolchain: ${{ needs.resolve-msrv.outputs.v }} components: llvm-tools-preview - uses: Swatinem/rust-cache@v2 with: @@ -99,6 +134,21 @@ jobs: slug: freemkv/freemkv-unlock fail_ci_if_error: false + # cargo-deny gates the RESOLVED dependency graph: RUSTSEC advisories, a + # permissive licence allow-list, and source/ban hygiene. It mirrors the + # `test` job's checkout (single root checkout, no siblings) and runs on + # `stable` — advisory/licence debt is caught against current Rust, not the + # MSRV pin — so no `needs: resolve-msrv`. + deny: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: dtolnay/rust-toolchain@stable + - uses: EmbarkStudios/cargo-deny-action@v2 + with: + command: check advisories bans licenses sources + manifest-path: Cargo.toml + # The crate ships on macOS and Windows through its consumers, so it has to # compile there. Build the tests (not just `cargo check`) — that forces # codegen and catches errors in test-only code, which `check` skips. @@ -115,11 +165,14 @@ jobs: # than a slow one: release.sh's CI gate refuses while any run for the # commit is still in progress, so a never-scheduled job blocks releases # silently -- see the note on real-media in qa.yml. + needs: resolve-msrv if: github.ref_name == 'qa' || github.ref_name == 'main' runs-on: macos-latest steps: - uses: actions/checkout@v7 - - uses: dtolnay/rust-toolchain@1.88.0 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ needs.resolve-msrv.outputs.v }} - uses: Swatinem/rust-cache@v2 with: shared-key: freemkv-unlock-ci @@ -138,11 +191,14 @@ jobs: # than a slow one: release.sh's CI gate refuses while any run for the # commit is still in progress, so a never-scheduled job blocks releases # silently -- see the note on real-media in qa.yml. + needs: resolve-msrv if: github.ref_name == 'qa' || github.ref_name == 'main' runs-on: windows-latest steps: - uses: actions/checkout@v7 - - uses: dtolnay/rust-toolchain@1.88.0 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ needs.resolve-msrv.outputs.v }} - uses: Swatinem/rust-cache@v2 with: shared-key: freemkv-unlock-ci diff --git a/.github/workflows/comment-guard.yml b/.github/workflows/comment-guard.yml new file mode 100644 index 0000000..0f8c2c9 --- /dev/null +++ b/.github/workflows/comment-guard.yml @@ -0,0 +1,17 @@ +name: comment-guard + +# Caps explanatory prose by COMMENT INTENT (not visibility): +# //! / /*! module docs — unbounded (module-scoped, can't hide per-item text) +# /// / /** */ item docs — 8 prose lines (fenced ``` examples excluded) +# // / /* */ asides — 3 lines (single-blank-separated runs count as one) +# Overflow belongs in the module //! or docs/.md with a one-line // pointer. + +on: [push, pull_request] + +jobs: + comment-guard: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - name: Run comment-guard + run: python3 ci/comment-guard.py diff --git a/.github/workflows/qa.yml b/.github/workflows/qa.yml index b36f2ed..84e2b64 100644 --- a/.github/workflows/qa.yml +++ b/.github/workflows/qa.yml @@ -24,6 +24,19 @@ on: workflow_dispatch: jobs: + # Single source of truth for the MSRV — see ci.yml. Jobs that build/test AT + # the MSRV read `needs.resolve-msrv.outputs.v` from Cargo.toml's rust-version; + # clippy (cross-lint) runs on stable instead. + resolve-msrv: + runs-on: ubuntu-latest + outputs: + v: ${{ steps.msrv.outputs.v }} + steps: + - uses: actions/checkout@v7 + - id: msrv + shell: bash + run: echo "v=$(grep -m1 '^rust-version' Cargo.toml | sed -E 's/.*\"([^\"]+)\".*/\1/')" >> "$GITHUB_OUTPUT" + # ── Name the candidate ──────────────────────────────────────────────── # # Every push to `qa` is a release candidate, so every push gets a tag: @@ -65,6 +78,7 @@ jobs: # changes what the optimiser can prove. A test that only passes in debug is # a test that never guarded the binary anyone actually ships. release-tests: + needs: resolve-msrv strategy: fail-fast: false matrix: @@ -74,7 +88,9 @@ jobs: - uses: actions/checkout@v7 with: path: freemkv-unlock - - uses: dtolnay/rust-toolchain@1.88.0 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ needs.resolve-msrv.outputs.v }} - uses: Swatinem/rust-cache@v2 with: workspaces: freemkv-unlock @@ -90,7 +106,9 @@ jobs: - uses: actions/checkout@v7 with: path: freemkv-unlock - - uses: dtolnay/rust-toolchain@1.88.0 + # clippy runs on stable (matches ci.yml's lint job); build/test stay on + # the MSRV pin. See ci.yml for why the lint toolchain need not be the MSRV. + - uses: dtolnay/rust-toolchain@stable with: components: clippy - uses: Swatinem/rust-cache@v2 @@ -105,12 +123,15 @@ jobs: # of the SCSI transport and platform layers compile on no other runner, so # without this they are first built at release time. windows-build: + needs: resolve-msrv runs-on: windows-latest steps: - uses: actions/checkout@v7 with: path: freemkv-unlock - - uses: dtolnay/rust-toolchain@1.88.0 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ needs.resolve-msrv.outputs.v }} - uses: Swatinem/rust-cache@v2 with: workspaces: freemkv-unlock diff --git a/.gitignore b/.gitignore index 022a2bb..910d7e0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,3 @@ /target -Cargo.lock .cargo/config.toml +NEXT_SESSION.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 748ecbe..b098858 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## [1.7.0] — 2026-09-02 + +### Fixed + +- `ld/profile`: `find_by_drive_id` no longer mis-binds a drive whose reported `product_id` is real but not catalogued to a sibling model's profile. The product-id-blind loose match is now gated on an empty product id, so a wrong profile — and its wrong LibreDrive microcode — can't be selected for a drive that does report a product id. + +### Changed + +- Version aligned to 1.7.0 for the unified release. + +### Docs + +- Added a third-party NOTICE clarifying that the LibreDrive microcode bundled in `src/ld/profiles.json` is proprietary MakeMKV material, not covered by this repository's MIT license. + ## [1.6.14] — 2026-08-31 ### Changed diff --git a/Cargo.lock b/Cargo.lock new file mode 100644 index 0000000..5d20d09 --- /dev/null +++ b/Cargo.lock @@ -0,0 +1,454 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "aes" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8eb277bec05f56a0e0591f155a484cbd0f4f07ff2905051a48c72f004f7ed58" +dependencies = [ + "cipher", + "cpubits", + "cpufeatures 0.3.1", +] + +[[package]] +name = "autocfg" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" + +[[package]] +name = "base64" +version = "0.23.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5" + +[[package]] +name = "block-buffer" +version = "0.10.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71" +dependencies = [ + "generic-array", +] + +[[package]] +name = "block-buffer" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa" +dependencies = [ + "hybrid-array", +] + +[[package]] +name = "cfg-if" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" + +[[package]] +name = "chacha20" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "65c35e4b699c7e15ccbe7ee35c005e4fc0a278d22238a2857e6ce2dadeda1b06" +dependencies = [ + "cfg-if", + "cpufeatures 0.3.1", + "rand_core", +] + +[[package]] +name = "cipher" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8cf2a2c93cd704877c0858356ed03480ff301ee950b43f1cbe4573b088bfa6c" +dependencies = [ + "crypto-common 0.2.2", + "inout", +] + +[[package]] +name = "const-oid" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6ef517f0926dd24a1582492c791b6a4818a4d94e789a334894aa15b0d12f55c" + +[[package]] +name = "cpubits" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "15b85f9c39137c3a891689859392b1bd49812121d0d61c9caf00d46ed5ce06ae" + +[[package]] +name = "cpufeatures" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59ed5838eebb26a2bb2e58f6d5b5316989ae9d08bab10e0e6d103e656d1b0280" +dependencies = [ + "libc", +] + +[[package]] +name = "cpufeatures" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ca28b0ae3115b884660db4118d803791fd6756b6e88f39c0f3f7859060d7566" +dependencies = [ + "libc", +] + +[[package]] +name = "crypto-common" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a" +dependencies = [ + "generic-array", + "typenum", +] + +[[package]] +name = "crypto-common" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce6e4c961d6cd6c9a86db418387425e8bdeaf05b3c8bc1411e6dca4c252f1453" +dependencies = [ + "hybrid-array", +] + +[[package]] +name = "digest" +version = "0.10.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" +dependencies = [ + "block-buffer 0.10.4", + "crypto-common 0.1.7", +] + +[[package]] +name = "digest" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1dd6dbb5841937940781866fa1281a1ff7bd3bf827091440879f9994983d5c2" +dependencies = [ + "block-buffer 0.12.1", + "const-oid", + "crypto-common 0.2.2", +] + +[[package]] +name = "freemkv-unlock" +version = "1.6.14" +dependencies = [ + "aes", + "base64", + "num-bigint", + "num-traits", + "rand", + "serde", + "serde_json", + "sha1", + "sha2", + "tracing", +] + +[[package]] +name = "generic-array" +version = "0.14.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" +dependencies = [ + "typenum", + "version_check", +] + +[[package]] +name = "getrandom" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "300e883d756b2e4ec94e02791f39b04b522276138852cfc41d9fb7e904106099" +dependencies = [ + "cfg-if", + "libc", + "r-efi", + "rand_core", +] + +[[package]] +name = "hybrid-array" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "707114b52a152fa7bdb290cd7cd5912d9467273b6d74e21b8d81aca1f8533f6b" +dependencies = [ + "typenum", +] + +[[package]] +name = "inout" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4250ce6452e92010fdf7268ccc5d14faa80bb12fc741938534c58f16804e03c7" +dependencies = [ + "hybrid-array", +] + +[[package]] +name = "itoa" +version = "1.0.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" + +[[package]] +name = "libc" +version = "0.2.189" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" + +[[package]] +name = "memchr" +version = "2.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" + +[[package]] +name = "num-bigint" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93e7820bc0a80a0238e650327316f929ba18d5be054b647490a3a6a339f3e7c0" +dependencies = [ + "num-integer", + "num-traits", +] + +[[package]] +name = "num-integer" +version = "0.1.47" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7ce2d95d4b3734dc35aa2f45e1aa22cd416814592a4f9d9205e11affd5b8e10b" +dependencies = [ + "num-traits", +] + +[[package]] +name = "num-traits" +version = "0.2.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" +dependencies = [ + "autocfg", +] + +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + +[[package]] +name = "pin-project-lite" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" + +[[package]] +name = "proc-macro2" +version = "1.0.107" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "985e7ec9bb745e6ce6535b544d84d6cd6f7ad8bd711c398938ae983b91a766d9" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.47" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fbf4db142a473a8d80c26bbf18454ed458bf8d26c8219c331daecfdbd079001" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "r-efi" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" + +[[package]] +name = "rand" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7f5fa3a058cd35567ef9bfa5e75732bee0f9e4c55fa90477bef2dfcdbc4be80" +dependencies = [ + "chacha20", + "getrandom", + "rand_core", +] + +[[package]] +name = "rand_core" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63b8176103e19a2643978565ca18b50549f6101881c443590420e4dc998a3c69" + +[[package]] +name = "serde" +version = "1.0.229" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4148590afebada386688f18773da617792bf2ef03ffc1e4cbd2b1d45b023e0ba" +dependencies = [ + "serde_core", + "serde_derive", +] + +[[package]] +name = "serde_core" +version = "1.0.229" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67dca2c9c51e58a4791a4b1ed58308b39c64224d349a935ab5039aa360942a48" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.229" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.4", +] + +[[package]] +name = "serde_json" +version = "1.0.151" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" +dependencies = [ + "itoa", + "memchr", + "serde", + "serde_core", + "zmij", +] + +[[package]] +name = "sha1" +version = "0.10.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a978451301f4db1d02937a4ab3ccce137717b81826e79b7d49ffe3244a13c3b8" +dependencies = [ + "cfg-if", + "cpufeatures 0.2.17", + "digest 0.10.7", +] + +[[package]] +name = "sha2" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "446ba717509524cb3f22f17ecc096f10f4822d76ab5c0b9822c5f9c284e825f4" +dependencies = [ + "cfg-if", + "cpufeatures 0.3.1", + "digest 0.11.3", +] + +[[package]] +name = "syn" +version = "2.0.119" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "872831b642d1a07999a962a351ed35b955ea2cfc8f3862091e2a240a84f17297" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "syn" +version = "3.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6275cddf4610d1775e6d1fe9469b2e77d0f39fd98fb7450901b821e0c53649f" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "tracing" +version = "0.1.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63e71662fa4b2a2c3a26f570f037eb95bb1f85397f3cd8076caed2f026a6d100" +dependencies = [ + "pin-project-lite", + "tracing-attributes", + "tracing-core", +] + +[[package]] +name = "tracing-attributes" +version = "0.1.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.119", +] + +[[package]] +name = "tracing-core" +version = "0.1.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "db97caf9d906fbde555dd62fa95ddba9eecfd14cb388e4f491a66d74cd5fb79a" +dependencies = [ + "once_cell", +] + +[[package]] +name = "typenum" +version = "1.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20" + +[[package]] +name = "unicode-ident" +version = "1.0.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" + +[[package]] +name = "version_check" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" + +[[package]] +name = "zmij" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29666d0abbfad1e3dc4dcf6144730dd3a3ab225bbbdac83319345b1b44ccfc1b" + +[[patch.unused]] +name = "freemkv-engine" +version = "1.6.14" + +[[patch.unused]] +name = "freemkv-i18n" +version = "1.6.14" + +[[patch.unused]] +name = "freemkv-keysources" +version = "1.6.14" + +[[patch.unused]] +name = "libfreemkv" +version = "1.6.14" diff --git a/Cargo.toml b/Cargo.toml index f45e767..19558d0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "freemkv-unlock" -version = "1.6.14" +version = "1.7.0" edition = "2024" rust-version = "1.88" license = "MIT" diff --git a/NOTICE b/NOTICE new file mode 100644 index 0000000..44b47d1 --- /dev/null +++ b/NOTICE @@ -0,0 +1,24 @@ +NOTICE — Third-Party Drive Microcode +==================================== + +freemkv-unlock's own source code is licensed under the MIT License (see LICENSE). +The MIT License does NOT cover the drive-microcode payloads bundled in +`src/ld/profiles.json`. + +Those payloads are volatile, RAM-only optical-drive microcode ("LibreDrive") +authored and owned by the MakeMKV team (GuinpinSoft inc.). Each payload embeds +its own copyright notice, e.g.: + + "This is volatile RAM-only microcode for LibreDrive by MakeMKV - + http://www.makemkv.com/libredrive/ + Copyright (c) MakeMKV team, all rights reserved." + +They are included solely to recognize supported drives and perform the +non-persistent, RAM-only unlock in interoperation with MakeMKV's LibreDrive, on +drives their owners already possess. They are third-party proprietary material — +**Copyright (c) MakeMKV team, all rights reserved** — and are **NOT** licensed +under MIT. No ownership is claimed or transferred. + +freemkv is an independent project, not affiliated with, endorsed by, or +sponsored by MakeMKV / GuinpinSoft inc. With thanks to the MakeMKV team for +LibreDrive. See https://www.makemkv.com/ and http://www.makemkv.com/libredrive/. diff --git a/README.md b/README.md index 09af705..2c66bb4 100644 --- a/README.md +++ b/README.md @@ -36,4 +36,13 @@ for u in freemkv_unlock::all_unlockers() { To remove an unlocker, delete its module directory and its one line in `all_unlockers()` — nothing else changes. -License: MIT. +## License + +freemkv-unlock's own source is MIT — see [LICENSE](LICENSE). + +`src/ld/profiles.json` bundles volatile, RAM-only **LibreDrive** microcode +authored and owned by the **MakeMKV team** (GuinpinSoft inc.) — *Copyright (c) +MakeMKV team, all rights reserved.* That microcode is third-party proprietary +material, **not** covered by the MIT license; no ownership is claimed. See +[`NOTICE`](NOTICE). With thanks to the MakeMKV team for +[LibreDrive](http://www.makemkv.com/libredrive/). diff --git a/ci/comment-guard.py b/ci/comment-guard.py new file mode 100644 index 0000000..d52dbe0 --- /dev/null +++ b/ci/comment-guard.py @@ -0,0 +1,252 @@ +#!/usr/bin/env python3 +"""comment-guard: cap explanatory prose by COMMENT INTENT, not visibility. + +Three tiers, keyed on the syntax the author reached for: + + * `//!` / `/*!` — a module (or crate) design doc. UNBOUNDED. There is one per + file, at the top, documenting the module as a whole, so it can't be used to + smuggle per-item rationale the way item docs can. Long correctness / design + essays (e.g. a memory-ordering proof) legitimately live here. + * `///` / `/** */` — item documentation. Up to DOC_CAP *prose* lines. Fenced + ``` code blocks do NOT count, so real examples aren't punished. This holds + whether or not the item is `pub`: a `pub(crate)` fn deserves a real doc + paragraph just like a `pub` one. Capping `///` here (rather than exempting + it, as the old guard did — issue #17) is what stops rationale being hidden + behind a doc sigil, while still leaving room for a genuine paragraph. + * `//` / `/* */` — a quick internal aside. Up to INLINE_CAP lines. This is the + shape that sprawls into walls, so it's kept tight: stanzas separated by a + single blank line count CUMULATIVELY, so one essay can't be split into runs. + +Overflow belongs in the module `//!`, or in docs/.md with a one-line `//` +pointer — both show up in review. + +Scans src/ and tests/ under the given root (default cwd). `--selftest` runs +built-in cases. Exit non-zero if any block exceeds its cap. +""" +import re +import sys +from pathlib import Path + +INLINE_CAP = 3 +DOC_CAP = 8 + +_RAW_OPEN = re.compile(r'r(#*)"') # raw string opener: r"…", r#"…"#, br##"…"## … + + +def _strip_line_comment(s): + """Return (kind, body) for a whole-line comment, else (None, None). + + kind is 'doc' for `///`/`//!`, 'plain' for other `//`. A trailing comment on + a code line is not a whole-line comment and is ignored (single-line, cheap). + """ + if s.startswith("//!"): + return "doc", s[3:] + if s.startswith("///") and not s.startswith("////"): + return "doc", s[3:] + if s.startswith("//"): + return "plain", s[2:] + return None, None + + +def _fence_toggle(body): + """True if this doc body line opens/closes a ``` fenced code block.""" + return body.strip().startswith("```") + + +def _classify(lines): + """Yield per-line dicts: comment kind ('doc'/'plain'/None), doc-ness of + block comments, and code info. Block comments are tracked across lines. + """ + info = [] + in_block = False + block_is_doc = False + raw_hashes = None # None = not in a raw string; int N = open, needs '"' + N*'#' + for raw in lines: + s = raw.lstrip() + rec = {"raw": raw, "s": s, "comment": None, "is_doc": False, "code": False} + # Inside a raw string literal (e.g. an embedded HTML/CSS/JS asset): the + # bytes are string data, not Rust comments, so the guard is blind to them + # until the string closes. Prevents `/* */` or `//` lines in an embedded + # page from being counted as comment blocks. + if raw_hashes is not None: + if ('"' + "#" * raw_hashes) in raw: + raw_hashes = None + info.append(rec) + continue + if in_block: + rec["comment"] = "doc" if block_is_doc else "plain" + rec["is_doc"] = block_is_doc + if "*/" in raw: + in_block = False + info.append(rec) + continue + # A block comment only starts a comment-line if it's the first token. + if s.startswith("/*"): + rec["is_doc"] = s.startswith("/**") or s.startswith("/*!") + rec["comment"] = "doc" if rec["is_doc"] else "plain" + if "*/" not in s[2:]: + in_block = True + block_is_doc = rec["is_doc"] + info.append(rec) + continue + kind, _ = _strip_line_comment(s) + if kind is not None: + rec["comment"] = kind + rec["is_doc"] = kind == "doc" + info.append(rec) + continue + # Code (or blank). Blank = empty after strip. + rec["code"] = s != "" + # A code line may OPEN a raw string that spans following lines. Detected + # only on code (comments were handled above), so `// see r"x"` is safe. + m = _RAW_OPEN.search(raw) + if m and ('"' + "#" * len(m.group(1))) not in raw[m.end():]: + raw_hashes = len(m.group(1)) + info.append(rec) + return info + + +def violations(path): + lines = path.read_text(encoding="utf-8", errors="replace").splitlines() + info = _classify(lines) + n = len(info) + out = [] + + i = 0 + while i < n: + rec = info[i] + if rec["comment"] is None: + i += 1 + continue + + start = i + if rec["is_doc"]: + # Doc run: contiguous doc lines. `//!` / `/*!` module docs are + # UNBOUNDED; any other doc run caps PROSE (fenced ``` excluded) at + # DOC_CAP, regardless of the documented item's visibility. + j = i + prose = 0 + in_fence = False + module_doc = False + while j < n and info[j]["comment"] == "doc": + s = info[j]["s"] + if s.startswith("//!") or s.startswith("/*!"): + module_doc = True + body = ( + s[3:] if (s.startswith("///") or s.startswith("//!")) else s + ) + if _fence_toggle(body): + in_fence = not in_fence + elif not in_fence and body.strip() != "": + prose += 1 + j += 1 + if not module_doc and prose > DOC_CAP: + out.append((start + 1, prose, DOC_CAP, "doc prose")) + i = j + continue + + # Internal (plain) run, with single-blank bridging (cumulative). + j = i + count = 0 + while j < n: + r = info[j] + if r["comment"] == "plain": + count += 1 + j += 1 + continue + if r["comment"] == "doc": + break + if r["comment"] is None and not r["code"]: + # blank: bridge only a SINGLE blank, else stop + if j + 1 < n and info[j + 1]["comment"] == "plain": + j += 1 + continue + break + break # code + if count > INLINE_CAP: + out.append((start + 1, count, INLINE_CAP, "inline comment")) + i = j + return out + + +def main(argv): + if "--selftest" in argv: + return _selftest() + root = Path(argv[1]) if len(argv) > 1 else Path.cwd() + files = sorted(p for d in ("src", "tests") for p in (root / d).rglob("*.rs")) + total = 0 + for f in files: + for start, length, cap, kind in violations(f): + rel = f.relative_to(root) + print(f"{rel}:{start}: {kind} block is {length} lines (max {cap})") + total += 1 + if total: + print(f"\ncomment-guard: {total} comment block(s) exceed their cap.") + print( + f"Inline // asides cap at {INLINE_CAP}; /// item-doc prose caps at " + f"{DOC_CAP} (fenced examples excluded); //! module docs are unbounded. " + "Trim, move the rationale into the module //!, or docs/.md " + "with a // pointer." + ) + return 1 + return 0 + + +def _selftest(): + import tempfile + + cases = [ + # (rust source, expected number of violations) + # short /// doc -> ok + ("/// short doc\npub fn a() {}\n", 0), + # /// on a PRIVATE item now gets the doc cap (8), so 4 lines is fine + ("/// l1\n/// l2\n/// l3\n/// l4\nfn priv_a() {}\n", 0), + # ...but /// is NOT unlimited: 9 prose lines on a private item fails + ("".join(f"/// l{k}\n" for k in range(9)) + "fn priv_b() {}\n", 1), + # 8 prose /// on a pub item -> ok (boundary) + ("".join(f"/// l{k}\n" for k in range(8)) + "pub fn c() {}\n", 0), + # 9 prose /// on a pub item -> violation + ("".join(f"/// l{k}\n" for k in range(9)) + "pub fn d() {}\n", 1), + # /// with a long fenced example -> example excluded, ok + ( + "/// summary\n/// # Examples\n/// ```\n" + + "".join("/// code\n" for _ in range(12)) + + "/// ```\npub fn e() {}\n", + 0, + ), + # pub(crate) gets the SAME doc cap as pub (no pub/private split anymore) + ("".join(f"/// l{k}\n" for k in range(6)) + "pub(crate) fn f() {}\n", 0), + # bare inline 4 lines -> violation + ("// a\n// b\n// c\n// d\nlet x = 1;\n", 1), + # blank-split dodge: 3 + blank + 3 inline -> cumulative -> violation + ("// a\n// b\n// c\n\n// d\n// e\n// f\nfn z() {}\n", 1), + # plain /* */ block, 5 lines -> inline cap -> violation + ("/* one\n two\n three\n four\n five */\nfn q() {}\n", 1), + # comment-like lines INSIDE a raw string are string data, not comments + ( + 'const H: &str = r##""##;\nfn h() {}\n', + 0, + ), + # //! module doc may run long -> EXEMPT (no cap) + ("".join(f"//! l{k}\n" for k in range(40)) + "\npub fn m() {}\n", 0), + # /*! block module doc may run long -> EXEMPT + ("/*! l0\n" + "".join(f" l{k}\n" for k in range(30)) + "*/\nfn n() {}\n", 0), + ] + ok = True + with tempfile.TemporaryDirectory() as d: + for idx, (src, want) in enumerate(cases): + p = Path(d) / f"case{idx}.rs" + p.write_text(src) + got = len(violations(p)) + flag = "ok" if got == want else "FAIL" + if got != want: + ok = False + print(f" selftest {idx}: want {want} got {got} [{flag}]") + print("selftest PASSED" if ok else "selftest FAILED") + return 0 if ok else 1 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/deny.toml b/deny.toml new file mode 100644 index 0000000..b13a90c --- /dev/null +++ b/deny.toml @@ -0,0 +1,42 @@ +# cargo-deny — gates the RESOLVED dependency graph (issue #15). +# Runs in CI against the committed Cargo.lock: RUSTSEC advisories, a permissive +# licence allow-list, and source/ban hygiene. Update the lock (cargo update) +# to clear an advisory rather than adding an `ignore` entry without cause. + +[advisories] +yanked = "deny" +# ignore = ["RUSTSEC-0000-0000"] # only with a written justification + +[licenses] +# The whole tree is permissive. r-efi's LGPL arm is satisfied by Apache-2.0 +# (its expression is "Apache-2.0 OR LGPL-2.1-or-later"), so no copyleft is +# allowed here. Add a licence only when a real dependency introduces it. +allow = [ + "MIT", + "Apache-2.0", + "Apache-2.0 WITH LLVM-exception", + "0BSD", + "Unicode-3.0", + "Unlicense", + "Zlib", + "BSD-2-Clause", + "BSD-3-Clause", + "ISC", + "CDLA-Permissive-2.0", +] +confidence-threshold = 0.9 + +[bans] +multiple-versions = "warn" # transitive dupes (digest 0.10/0.11 etc.) are noise, not a gate +wildcards = "deny" +# The freemkv siblings resolve as path deps under CI's path-patch layout, which +# reads as a wildcard version req. That is internal, not a supply-chain wildcard. +allow-wildcard-paths = true + +[sources] +unknown-registry = "deny" +unknown-git = "deny" +# The freemkv siblings are first-party git-tag deps; allow git sources from +# our own org only (CI resolves them as git, not the local path override). +[sources.allow-org] +github = ["freemkv"] diff --git a/docs/aacs-handshake.md b/docs/aacs-handshake.md new file mode 100644 index 0000000..382ed61 --- /dev/null +++ b/docs/aacs-handshake.md @@ -0,0 +1,184 @@ +# AACS bus authentication handshake + +## Flow + +1. Invalidate AGIDs → allocate fresh AGID +2. Send host certificate + nonce +3. Receive drive certificate + nonce +4. Receive drive key point + signature, verify +5. Sign host key point, send +6. ECDH: host_priv × drive_key_point → bus key (low 128 bits of x) +7. Read VID or Read Data Keys (encrypted with bus key) + +## Why `scsi_read`/`scsi_write` check status at the seam + +The contract this crate is built on (`ScsiTransport::execute`) returns `Ok` +on a SCSI sense and `Err` ONLY on a transport-layer fault. These two helpers +used to propagate the `?` and nothing else — so a CHECK CONDITION (`Ok`, +non-zero `status`) handed the caller back its own ZERO-FILLED buffer and +every REPORT/SEND KEY step consumed zeros as valid drive data: a drive +certificate of zeros, a key point of zeros, a Volume ID of zeros, all +reported as success. It has not fired in production only because the sole +transport implementor violates the contract in the opposite direction (it +returns `Err` for any non-zero status). Plugging in a CONFORMING transport +would have turned the whole handshake into a zero-buffer success, so the +status and the transferred length are checked here, at the seam. + +## Why `scsi_write` shares this reasoning + +Same contract reasoning as `scsi_read`: a drive that REFUSES the host +certificate answers with `Ok` + CHECK CONDITION, and treating that as a +successful send carried the handshake on against a drive that never +accepted it. + +## `ec_mul` design notes + +**Constant-time tradeoff:** this branches on `scalar.bit(0)` and clones +BigUints per iteration, so its timing is data-dependent on the secret scalar +(the long-term host private key in `ecdsa_sign`, the ephemeral key in ECDH). +This is deliberate: the handshake runs once per disc against a local optical +drive, so throughput and the narrow local-timing surface do not justify +pulling in a vetted constant-time backend. Revisit if this ever signs in a +remote/shared context. + +**Cofactor:** both AACS curves used here have cofactor 1, so a point that +lies on the curve is automatically in the prime-order subgroup — no +small-subgroup defense / `n·P == O` check is required for the inputs this is +called with. + +## `verify_cert_p256` — provisional AACS 2.0 cert layout + +Verifies an AACS 2.0 drive certificate (type 0x11) against an AACS 2.0 LA +key. PROVISIONAL LAYOUT — the byte offsets below are NOT yet confirmed +against a genuine AACS 2.0 cert, and no such cert exists on this machine to +pin them (an exhaustive hunt found only 2.0 *content* certs, a different +format; a real host/drive AKE cert needs a live USB/SCSI capture). What IS +confirmed: first, the P-256/SHA-256 ECDSA primitive and curve constants are +correct (`ecdsa_verify_p256_verifies_a_genuine_aacs2_content_cert` verifies a +real 2.0 content cert under the published CC key); second, the 132-byte +4-byte-header framing MATCHES the step-6 drive-key message (x[4..36] / +y[36..68] / r[68..100] / s[100..132]), the natural 20→32-byte scaling of the +established AACS 1.0 84-byte key-message framing. + +What is NOT confirmed: that a *certificate* shares that framing. No known +implementation verifies 2.0 certs against a confirmed layout (the published +spec draft leaves that format ambiguous), and the AACS 2.0 draft architecture +lists the 2.0 host cert (type 0x12) with a 6-byte Host ID + a NEW 4-byte +paired-Device-Key-Set field before the 64-byte public key — implying a +~140–144-byte cert with the pubkey near offset 12–16 and a signed range of +~76–80 bytes, NOT 68. The drive cert (type 0x11) has no paired-DKS field +(~140 bytes, pubkey ~[12..76], signed ~first 76). So the offsets HERE +(pubkey [4..68], signed [..68]) are a best-effort self-consistent +placeholder, chosen to match the confirmed key message; they will almost +certainly move when a real cert lands (see the `#[ignore]` real-cert slot). +This matters for USEFULNESS, not SAFETY: a wrong signed range makes +verification FAIL, and since the caller treats a verify failure as FATAL, +the path fails CLOSED (rejects) rather than trusting anything — never the +reverse. + +The `>= 132` guard keeps the `cert[100..132]` slice safe while letting a +full-length cert reach verification (the old `>= 138` guard rejected every +132-byte input unconditionally). + +The LA anchor is a PARAMETER, not the compiled-in constant, so a unit test +can drive the whole chain under a test LA keypair (sign a synthetic drive +cert, prove it is accepted / a bad-sig cert is rejected) without the real +anchor's private half, which no one has. Production always threads +`AACS2_LA_PUB_X/Y` from `aacs2_authenticate_p256`. + +## Why an unknown drive-cert type must be rejected + +This used to be an `if` / `else if` with NO `else`, so an unknown type +(e.g. 0x02) fell straight through: `is_aacs20` stayed false, so the step-6 +key-signature check still ran — against `cert_pub_key(drive_cert)`, the +drive's OWN key lifted out of a cert that was never checked against the LA. +A rogue USB bridge could set byte 0 to 0x02, present its own keypair, sign +`host_nonce || drive_key_point`, and the handshake would return Ok with an +attacker-chosen bus key. The chain of trust must not appear to have run +when it did not. + +## AACS 2.0 chain-of-trust gate (`aacs2_authenticate_p256_with_agid`) + +The SAME shape the AACS 1.0 sibling (`aacs_authenticate_with_agid`) +enforces, now MANDATORY because this path is live. Two distinct rejections, +both BEFORE any drive key is trusted: + +**(a) Cert TYPE gate.** An AACS 2.0 drive cert is type 0x11. Anything else +is unexpected on the native P-256 AKE and is REJECTED here, never allowed to +fall through. The earlier `if type==0x11 && !verify` form left NO branch for +a non-0x11 type: a rogue bridge could send a type-0x10 (or any) cert, skip +verification entirely, and still reach the step-6 key check against +`cert_pub_key_p256(drive_cert)` — its OWN key, lifted from a cert the LA +never signed — winning an attacker-chosen bus key. The chain of trust must +not merely LOOK like it ran. + +**(b) Cert VERIFY is FATAL.** A type-0x11 cert whose P-256/SHA-256 signature +does not verify under the real 2.0 LA anchor (`AACS2_LA_PUB`, on-curve — see +`la_anchor_keys_are_on_curve`) is REJECTED, not logged-and-continued. +`ecdsa_verify_p256` range/curve-checks every point (`point_on_curve`), so a +forged or off-curve cert key cannot slip past. The previous "non-fatal for +backward compat" behaviour was the exact hole the SECURITY note demanded be +closed before wiring. + +## `run_cert_handshake` — cert-agnostic AKE selection + +The AACS 1.0 host cert is always present and is tried FIRST: an AACS 2.0 +drive accepts a 1.0 host cert for backward compatibility, which is enough to +learn the Volume ID. If the drive REFUSES the 1.0 cert (a plain cert-level +rejection — NOT a dead or wedged bus) and THIS host cert also carries AACS +2.0 (P-256) credentials, fall back to the native P-256 AKE. + +This does NOT branch on "do we possess 2.0 certs?" as a feature switch: it +consumes whatever the supplied `HostCert` carries, exactly as the 1.0 path +consumes `hc.certificate`. A host cert without v2 credentials simply has +nothing more to present and keeps its 1.0 error; a host cert WITH them +drives the P-256 AKE the moment the drive asks for it. When a real 2.0 host +cert is supplied at runtime, the P-256 handshake runs — no code change, no +possession gate. + +A transport fault on the 1.0 attempt short-circuits the fallback: +re-running the P-256 AKE against a bus that is gone re-issues AGID +invalidation + SEND/REPORT KEY for nothing and buries the real (replug) +cause. The `Err` arm below still classifies it as `Transport`. + +## AACS 2.0 (P-256) host-certificate handshake — live-path test plumbing + +No genuine 2.0 host cert exists for us to sign with (a side agent is hunting +one; see the `#[ignore]` slot at the end). So the tests below prove the +PLUMBING under a self-generated test LA keypair: a 132-byte drive cert +signed by the test LA is threaded through the SAME code the production +anchor uses, only the anchor differs (a parameter, not a recompile). The +layout mirrored is the code's self-consistent 4-byte-header framing — +header(4)+pub_x(32)+pub_y(32)+sig_r(32)+sig_s(32)=132, LA signs SHA-256 over +cert[..68] — identical to the step-6 drive-key message framing and the +156-byte SEND KEY host-cert payload (4+20 nonce+132 cert). + +## `ecdsa_verify_p256_verifies_a_genuine_aacs2_content_cert` — why it matters + +SANITY: `ecdsa_verify_p256` + the P-256 curve constants + SHA-256 are correct +against GENUINE AACS 2.0 material. No real 2.0 host/drive AKE cert exists +anywhere on this machine (an exhaustive hunt found none — it needs a live +USB/SCSI AKE capture), but a real AACS 2.0 CONTENT certificate does +(`research/aacs/iso-aacs/CivilWar/Content000.cer`, type 0x10, 232 bytes). It +is signed by the AACS 2.0 *Content Cert* key (P-256/SHA-256, 64-byte +signature over the first `signed_len` bytes) — a DIFFERENT key for a +DIFFERENT purpose than the AKE LA anchor, but it exercises the EXACT same +`ecdsa_verify_p256` primitive `verify_cert_p256` relies on; only the pubkey +and the offsets differ. Proving it verifies here pins that the P-256 +ECDSA/SHA-256 math and the `point_on_curve` gate are right, so when a +genuine 2.0 host/drive cert (and its correct signed range) lands, only the +offsets — not the crypto — remain to be fixed. + +The CC pubkey is the published AACS 2.0 Content Certificate public key +(P-256). Corrupting any `P256_*` constant, or removing SHA-256, makes this +real-vector verification go red. + +## Cert support + +- AACS 1.0: custom 160-bit curve, SHA-1, 20-byte keys +- AACS 2.0: an AACS 2.0 drive accepts an AACS 1.0 host cert for backward + compatibility (that yields the Volume ID), and the native P-256/SHA-256 + AACS 2.0 AKE is the fallback when the drive refuses the 1.0 cert AND the + supplied host cert carries AACS 2.0 (P-256) credentials. Both paths are + LIVE and share the same chain-of-trust hardening; see + `run_cert_handshake` for the cert-agnostic dispatch. diff --git a/docs/drive-profile-visibility.md b/docs/drive-profile-visibility.md new file mode 100644 index 0000000..8fdcdf3 --- /dev/null +++ b/docs/drive-profile-visibility.md @@ -0,0 +1,16 @@ +# `DriveProfile` field visibility and `allow(dead_code)` + +Only `identity` and `signature` are public — they are what the catalog is +public FOR ("is this drive supported?", and the emulator's impersonation). +Everything below them is unlock MECHANISM: the MediaTek firmware image and +the per-drive vendor CDB templates. `Cargo.toml` says this crate is never +published because it carries drive firmware; re-exporting `DriveProfile` +with a `pub firmware: Vec` handed that firmware, and every vendor CDB, +to any downstream crate that called `ld::profile()`. They are `pub(crate)` +so the mechanism stays inside the unlocker. + +`allow(dead_code)`: this is the on-disk catalog schema, and several +captured CDB templates have no in-crate consumer yet. They were `pub` +before, so the compiler saw the (external) use; making them `pub(crate)` +is what exposes them as unread. Deleting them would silently drop fields +from the format the profile-extraction pipeline emits. diff --git a/docs/module-visibility.md b/docs/module-visibility.md new file mode 100644 index 0000000..87a6bc3 --- /dev/null +++ b/docs/module-visibility.md @@ -0,0 +1,29 @@ +# Module visibility rationale (`src/lib.rs`) + +Why each unlocker module is `pub mod` or `mod`, and what's actually public +about it. The consumer never reaches an unlocker impl directly — only through +[`all_unlockers`] — so "public" here means "exposes something other than the +unlocker impl itself." + +## `ld` + +Public ONLY for its drive-profile catalog (`ld::profiles` / the `Profiles` +object) and, under the `emulation` feature, the unlock-handshake wire format +the bdemu test-emulator needs. The unlocker impl itself (`Mt1959Unlocker`) is +`pub(crate)` — clients still reach unlockers only through `all_unlockers`. +`aacs` and `css` carry no such public catalog, so they stay fully private. + +## `renesas` + +Public for its `is_renesas` drive-probe, which reports a dead bus as +`Err(UnlockError::Transport)` rather than "not a Renesas drive" — a +distinction the consumer needs before it can trust a negative probe result. +The unlocker impl (`Renesas`) is `pub(crate)` — reached only through +`all_unlockers`. + +## `freemkv` + +Carries no public catalog (the firmware self-identifies rather than matching +a bundled profile) and no emulation wire format yet, so it stays fully +private — the unlocker impl (`FreemkvUnlocker`) is reached only through +`all_unlockers`. diff --git a/docs/scsi-mock-fixture.md b/docs/scsi-mock-fixture.md new file mode 100644 index 0000000..508d049 --- /dev/null +++ b/docs/scsi-mock-fixture.md @@ -0,0 +1,9 @@ +# `scsi::mock` — crate-wide mock transport + +The crate-wide mock transport. Before this existed, EVERY mock in the crate +returned `Ok` with status 0 and a full `bytes_transferred` — so no test could +ever exercise the three outcomes the contract actually distinguishes, and a +whole class of transport-fault misclassification bugs sat uncaught (a dead bus +read as "this unlocker doesn't apply", a zero-filled buffer read as valid +drive data). This fixture can express all of them, and is the red-before-green +vehicle for the transport-contract fixes across ld / aacs / css / renesas. diff --git a/docs/unlocked-debug-redaction.md b/docs/unlocked-debug-redaction.md new file mode 100644 index 0000000..ecda3ef --- /dev/null +++ b/docs/unlocked-debug-redaction.md @@ -0,0 +1,10 @@ +# Why `Unlocked`'s `Debug` impl is hand-written + +`Unlocked` is the public carrier that leaves this crate, and both `bus_key` +(the AACS bus key) and `vid` (the Volume ID that feeds VUK derivation) are key +material that must never reach a log or a test-failure message in plaintext. +The `AacsAuth` / `CertHandshake` siblings already redact; a derived `Debug` +here would print the very bytes they hide. + +Presence is still observable (`Some`/`None`) so a log can say WHETHER a key +was obtained without revealing WHAT it is. diff --git a/src/aacs/handshake.rs b/src/aacs/handshake.rs index 19bc783..69fce60 100644 --- a/src/aacs/handshake.rs +++ b/src/aacs/handshake.rs @@ -1,43 +1,22 @@ //! AACS bus authentication handshake — ECDH key agreement + bus key derivation. //! -//! Implements the AACS SCSI authentication protocol to obtain: -//! - Volume ID (VID) — needed for VUK derivation -//! - Read Data Key — needed for AACS 2.0 (UHD) bus decryption +//! Implements the AACS SCSI authentication protocol to obtain the Volume ID +//! (needed for VUK derivation) and, for AACS 2.0 (UHD), the Read Data Key: +//! allocate an AGID, exchange host/drive certs and key points, verify +//! signatures, derive the bus key via ECDH, then read VID / Read Data Keys. //! -//! Flow: -//! 1. Invalidate AGIDs → allocate fresh AGID -//! 2. Send host certificate + nonce -//! 3. Receive drive certificate + nonce -//! 4. Receive drive key point + signature, verify -//! 5. Sign host key point, send -//! 6. ECDH: host_priv × drive_key_point → bus key (low 128 bits of x) -//! 7. Read VID or Read Data Keys (encrypted with bus key) -//! -//! Supports: -//! - AACS 1.0: custom 160-bit curve, SHA-1, 20-byte keys -//! - AACS 2.0: an AACS 2.0 drive accepts an AACS 1.0 host cert for backward -//! compatibility (that yields the Volume ID), and the native P-256/SHA-256 -//! AACS 2.0 AKE is the fallback when the drive refuses the 1.0 cert AND the -//! supplied host cert carries AACS 2.0 (P-256) credentials. Both paths are -//! LIVE and share the same chain-of-trust hardening; see -//! [`run_cert_handshake`] for the cert-agnostic dispatch. +//! Supports AACS 1.0 and AACS 2.0 cert chains; see docs/aacs-handshake.md +//! for the full step list and cert-fallback rules, and +//! [`run_cert_handshake`] for the dispatch. use crate::aacs::error::{Error, Result}; use crate::scsi::{DataDirection, ScsiTransport}; use num_bigint::BigUint; use num_traits::{One, Zero}; use sha1::{Digest, Sha1}; -/// Map a SCSI-layer error from a handshake step onto a cert/key-specific -/// code — but only when the failure is *not* a transport-layer wedge. -/// -/// A SEND KEY / REPORT KEY step can fail because the drive genuinely -/// rejected the host certificate or key (a real `Aacs*` condition), or -/// because the transport died mid-handshake (bridge wedge / USB -/// disconnect). Collapsing the latter into a cert/key code tells the -/// operator the drive rejected their credentials, sending them down a -/// keydb/host-cert rabbit hole for what is actually a replug/power-cycle -/// situation. Preserve the transport error so the true root cause is -/// surfaced; otherwise substitute the handshake-specific code. +// Map a SCSI-layer error onto a cert/key-specific code, unless it's a +// transport-layer wedge (replug/power-cycle) — else the operator gets sent +// down a keydb/host-cert rabbit hole for what is really a dead transport. fn handshake_err(err: Error, fallback: Error) -> Error { if err.is_scsi_transport_failure() { err @@ -46,19 +25,8 @@ fn handshake_err(err: Error, fallback: Error) -> Error { } } -/// Execute a SCSI command that reads data from the device. -/// -/// THE contract this crate is built on ([`ScsiTransport::execute`]) returns -/// `Ok` on a SCSI sense and `Err` ONLY on a transport-layer fault. These two -/// helpers used to propagate the `?` and nothing else — so a CHECK CONDITION -/// (`Ok`, non-zero `status`) handed the caller back its own ZERO-FILLED buffer -/// and every REPORT/SEND KEY step consumed zeros as valid drive data: a drive -/// certificate of zeros, a key point of zeros, a Volume ID of zeros, all -/// reported as success. It has not fired in production only because the sole -/// transport implementor violates the contract in the opposite direction (it -/// returns `Err` for any non-zero status). Plugging in a CONFORMING transport -/// would have turned the whole handshake into a zero-buffer success, so the -/// status and the transferred length are checked here, at the seam. +// See docs/aacs-handshake.md — why status/length are checked here, at the +// seam, instead of trusting `ScsiTransport::execute`'s `Ok` result. fn scsi_read(session: &mut dyn ScsiTransport, cdb: &[u8], len: usize) -> Result> { let mut buf = vec![0u8; len]; let r = session.execute(cdb, DataDirection::FromDevice, &mut buf, 5_000)?; @@ -73,10 +41,8 @@ fn scsi_read(session: &mut dyn ScsiTransport, cdb: &[u8], len: usize) -> Result< Ok(buf) } -/// Execute a SCSI command that writes data to the device. Same contract -/// reasoning as [`scsi_read`]: a drive that REFUSES the host certificate -/// answers with `Ok` + CHECK CONDITION, and treating that as a successful send -/// carried the handshake on against a drive that never accepted it. +// See docs/aacs-handshake.md — shares scsi_read's status-check reasoning: +// a drive REFUSING the host cert answers `Ok` + CHECK CONDITION. fn scsi_write(session: &mut dyn ScsiTransport, cdb: &[u8], data: &[u8]) -> Result<()> { let mut buf = data.to_vec(); let r = session.execute(cdb, DataDirection::ToDevice, &mut buf, 5_000)?; @@ -97,14 +63,9 @@ fn check_status(cdb: &[u8], r: &crate::scsi::ScsiResult) -> Result<()> { }) } -/// Release an AGID by issuing REPORT KEY format 0x3F for it. -/// -/// A drive has only four AGIDs. Every error path out of an authentication -/// attempt used to abandon the one it had allocated; the leak self-heals -/// because each attempt starts by invalidating all four, but leaving a drive -/// with three of four AGIDs held between attempts is a state we should not be -/// creating. Best-effort by construction — a failure to release is not a -/// failure of the operation that is already failing. +// Release an AGID (REPORT KEY format 0x3F). A drive has only four, so avoid +// leaving one held between attempts. Best-effort: a release failure isn't a +// failure of the operation that's already failing. fn release_agid(session: &mut dyn ScsiTransport, agid: u8) { let cdb = cdb_report_key(agid, 0x3F, 2); let _ = scsi_read(session, &cdb, 2); @@ -164,11 +125,9 @@ const P256_GY: [u8; 32] = [ 0x2B, 0xCE, 0x33, 0x57, 0x6B, 0x31, 0x5E, 0xCE, 0xCB, 0xB6, 0x40, 0x68, 0x37, 0xBF, 0x51, 0xF5, ]; -/// AACS 2.0 LA public key for cert verification (P-256), big-endian. -/// The published AACS2 Licensing Administrator key — used to verify type 0x11 -/// drive certificates. Confirmed on the P-256 curve (y² ≡ x³+ax+b mod p) by -/// `la_anchor_keys_are_on_curve`; the earlier compiled-in value was OFF-CURVE and -/// so rejected every certificate `verify_cert_p256` was handed. +// AACS 2.0 LA public key for cert verification (P-256), big-endian; used to +// verify type 0x11 drive certificates. Confirmed on-curve by +// `la_anchor_keys_are_on_curve` (the earlier compiled-in value was OFF-CURVE). const AACS2_LA_PUB_X: [u8; 32] = [ 0xDC, 0x88, 0x52, 0xA0, 0xA7, 0xF0, 0xD0, 0x24, 0xD4, 0xC4, 0xCA, 0xC3, 0x1F, 0x32, 0x5F, 0x90, 0x3D, 0x0D, 0x23, 0xFC, 0x65, 0xEE, 0xBB, 0x1C, 0x75, 0x90, 0xB9, 0x62, 0xDB, 0x57, 0x43, 0x2E, @@ -178,14 +137,9 @@ const AACS2_LA_PUB_Y: [u8; 32] = [ 0x0F, 0x68, 0x4A, 0x05, 0x96, 0xE9, 0xCE, 0x00, 0xC4, 0xD3, 0xFE, 0x6E, 0x24, 0x45, 0x4D, 0xD0, ]; -// ── AACS 1.0 LA (Licensing Administrator) public key for cert verification ── -// -// The published AACS 1.0 LA key on the 160-bit curve, big-endian. Validated two -// ways before landing: it is on-curve (`la_anchor_keys_are_on_curve`) AND it -// verifies a genuine LA-signed host certificate -// (`verify_cert_accepts_a_genuine_la_signed_host_cert`). The prior compiled-in -// value was OFF-CURVE, so `ecdsa_verify` (which now range/curve-checks Q) — and -// therefore `verify_cert` — rejected every real certificate. +// AACS 1.0 LA (Licensing Administrator) public key on the 160-bit curve, +// big-endian; validated on-curve and against a genuine LA-signed host cert +// (the prior compiled-in value was OFF-CURVE, rejecting every real cert). const AACS_LA_PUB_X: [u8; 20] = [ 0x63, 0xC2, 0x1D, 0xFF, 0xB2, 0xB2, 0x79, 0x8A, 0x13, 0xB5, 0x8D, 0x61, 0x16, 0x6C, 0x4E, 0x4A, 0xAC, 0x8A, 0x07, 0x72, @@ -370,21 +324,9 @@ fn ec_double(pt: &EcPoint, a: &BigUint, p: &BigUint) -> EcPoint { EcPoint::new(x3, y3) } -/// Scalar multiplication using double-and-add. -/// -/// NOTE (constant-time tradeoff): this branches on `scalar.bit(0)` and -/// clones BigUints per iteration, so its timing is data-dependent on the -/// secret scalar (the long-term host private key in `ecdsa_sign`, the -/// ephemeral key in ECDH). This is a deliberate tradeoff: the handshake -/// runs once per disc against a local optical drive, so throughput and -/// the narrow local-timing surface do not justify pulling in a vetted -/// constant-time backend. Revisit if this ever signs in a remote/shared -/// context. -/// -/// NOTE (cofactor): both AACS curves used here have cofactor 1, so a -/// point that lies on the curve is automatically in the prime-order -/// subgroup — no small-subgroup defense / `n·P == O` check is required -/// for the inputs this is called with. +// Scalar multiplication using double-and-add. Not constant-time (timing +// depends on the secret scalar) — accepted tradeoff for a local, once-per- +// disc handshake. See docs/aacs-handshake.md for the full rationale. fn ec_mul(k: &BigUint, pt: &EcPoint, a: &BigUint, p: &BigUint) -> EcPoint { if k.is_zero() { return EcPoint::infinity(); @@ -405,11 +347,9 @@ fn ec_mul(k: &BigUint, pt: &EcPoint, a: &BigUint, p: &BigUint) -> EcPoint { result } -/// True if the point (x, y) satisfies y² ≡ x³ + ax + b (mod p) and lies -/// in the field (x, y < p). Guards the ECDH multiply against the classic -/// invalid-curve attack: a drive that supplies an off-curve key point can -/// otherwise steer the scalar multiply onto a weak curve and leak the host -/// scalar. Caller must reject the point when this returns false. +// True if (x, y) satisfies y² ≡ x³ + ax + b (mod p) and lies in the field. +// Guards ECDH against the invalid-curve attack: an off-curve drive key point +// can steer the scalar multiply onto a weak curve. Caller must reject on false. fn point_on_curve(x: &BigUint, y: &BigUint, a: &BigUint, b: &BigUint, p: &BigUint) -> bool { if x >= p || y >= p { return false; @@ -446,10 +386,9 @@ fn ecdsa_sign(priv_key: &[u8; 20], data: &[u8]) -> ([u8; 20], [u8; 20]) { let z = BigUint::from_bytes_be(&hash); loop { - // Generate random k via rejection sampling. Reducing raw RNG bytes - // modulo n would bias k toward small values (n is not a power of - // two); a biased ECDSA nonce is a known key-recovery weakness, so - // we reject and redraw any candidate >= n instead. + // Rejection sampling for k: reducing raw RNG bytes mod n would bias + // k toward small values (n isn't a power of two), a known ECDSA + // key-recovery weakness — redraw any candidate >= n instead. let mut k_bytes = [0u8; 20]; use rand::Rng; rand::rng().fill_bytes(&mut k_bytes); @@ -502,13 +441,9 @@ fn ecdsa_verify( let g = EcPoint::from_bytes(&EC_GX, &EC_GY); let q = EcPoint::from_bytes(pub_x, pub_y); - // Validate the public key BEFORE using it. Without this, a Q of (0,0) — the - // additive identity in affine coordinates — makes `u2·Q` the point at - // infinity, so verification collapses to `r == x(u1·G) mod n`, which a - // forger satisfies with NO knowledge of any private key. `point_on_curve` - // also range-checks (x,y < p) and, since b != 0, rejects (0,0) (its rhs is - // `b`, never 0). This is the mechanism that turns the drive-cert type-byte - // bypass into a one-liner exploit, so the check belongs in the primitive. + // Validate the public key BEFORE using it: without this, Q=(0,0) makes + // `u2·Q` the point at infinity, so verification collapses to + // `r == x(u1·G) mod n` — forgeable with no private-key knowledge. if !point_on_curve(&q.x, &q.y, &a, &b, &p) { return false; } @@ -643,43 +578,9 @@ fn ecdsa_verify_p256(pub_x: &[u8], pub_y: &[u8], sig_r: &[u8], sig_s: &[u8], dat &r_point.x % &n == r } -/// Verify an AACS 2.0 drive certificate (type 0x11) against an AACS 2.0 LA key. -/// -/// PROVISIONAL LAYOUT — the byte offsets below are NOT yet confirmed against a -/// genuine AACS 2.0 cert, and no such cert exists on this machine to pin them -/// (an exhaustive hunt found only 2.0 *content* certs, a different format; a -/// real host/drive AKE cert needs a live USB/SCSI capture). What IS confirmed: -/// first, the P-256/SHA-256 ECDSA primitive and curve constants are correct -/// (`ecdsa_verify_p256_verifies_a_genuine_aacs2_content_cert` verifies a real -/// 2.0 content cert under the published CC key); second, the 132-byte -/// 4-byte-header framing MATCHES the step-6 drive-key message (x[4..36] / -/// y[36..68] / r[68..100] / s[100..132]), the natural 20→32-byte scaling of -/// the established AACS 1.0 84-byte key-message framing. -/// What is NOT confirmed: that a *certificate* shares that framing. No known -/// implementation verifies 2.0 certs against a confirmed layout (the published -/// spec draft leaves that format ambiguous), and the AACS 2.0 draft architecture -/// lists the 2.0 host cert -/// (type 0x12) with a 6-byte Host ID + a NEW 4-byte paired-Device-Key-Set field -/// before the 64-byte public key — implying a ~140–144-byte cert with the -/// pubkey near offset 12–16 and a signed range of ~76–80 bytes, NOT 68. The -/// drive cert (type 0x11) has no paired-DKS field (~140 bytes, pubkey ~[12..76], -/// signed ~first 76). So the offsets HERE (pubkey [4..68], signed [..68]) are a -/// best-effort self-consistent placeholder, chosen to match the confirmed key -/// message; they will almost certainly move when a real cert lands (see the -/// `#[ignore]` real-cert slot). This matters for USEFULNESS, not SAFETY: a -/// wrong signed range makes verification FAIL, and since the caller treats a -/// verify failure as FATAL, the path fails CLOSED (rejects) rather than -/// trusting anything — never the reverse. -/// -/// The `>= 132` guard keeps the `cert[100..132]` slice safe while letting a -/// full-length cert reach verification (the old `>= 138` guard rejected every -/// 132-byte input unconditionally). -/// -/// The LA anchor is a PARAMETER, not the compiled-in constant, so a unit test -/// can drive the whole chain under a test LA keypair (sign a synthetic drive -/// cert, prove it is accepted / a bad-sig cert is rejected) without the real -/// anchor's private half, which no one has. Production always threads -/// `AACS2_LA_PUB_X/Y` from [`aacs2_authenticate_p256`]. +// Verify an AACS 2.0 drive cert (type 0x11) against an AACS 2.0 LA key. See +// docs/aacs-handshake.md — offsets are PROVISIONAL pending a real captured +// cert; LA anchor is a parameter so tests can drive it standalone. fn verify_cert_p256(cert: &[u8], la_x: &[u8; 32], la_y: &[u8; 32]) -> bool { if cert.len() < 132 { return false; @@ -689,13 +590,9 @@ fn verify_cert_p256(cert: &[u8], la_x: &[u8; 32], la_y: &[u8; 32]) -> bool { ecdsa_verify_p256(la_x, la_y, sig_r, sig_s, &cert[..68]) } -/// Extract public key from an AACS 2.0 certificate (32-byte x,y). -/// -/// Returns a zeroed key pair if `cert` is too short to hold the fixed -/// offsets (matches the `>= 132` guard in `verify_cert_p256`), so a -/// short/hostile cert cannot panic on the slice index. Offsets follow the -/// 4-byte-header layout: pub_x at [4..36], pub_y at [36..68] — the same -/// framing the step-6 drive-key message uses. +// Extract public key from an AACS 2.0 cert (32-byte x,y). Returns zeroed +// key pair if `cert` is too short for the fixed offsets (matches the +// `>= 132` guard in `verify_cert_p256`) so a hostile cert can't panic. fn cert_pub_key_p256(cert: &[u8]) -> ([u8; 32], [u8; 32]) { let mut x = [0u8; 32]; let mut y = [0u8; 32]; @@ -743,11 +640,8 @@ fn verify_cert(cert: &[u8]) -> bool { if cert.len() < 92 { return false; } - // Certificate format (92 bytes, 12-byte header): header(12) + pub_x(20) at - // [12..32] + pub_y(20) at [32..52] + sig_r(20) at [52..72] + sig_s(20) at - // [72..92]. Signature is over the first 52 bytes (header + public key). - // NOTE: `cert_pub_key` reads pub_x at offset 12, so the header is 12 bytes, - // not the 10 an earlier comment claimed. + // Format (92 bytes, 12-byte header): pub_x(20)@[12..32], pub_y(20)@[32..52], + // sig_r(20)@[52..72], sig_s(20)@[72..92]; signed over first 52 bytes. let mut sig_r = [0u8; 20]; let mut sig_s = [0u8; 20]; sig_r.copy_from_slice(&cert[52..72]); @@ -756,11 +650,9 @@ fn verify_cert(cert: &[u8]) -> bool { ecdsa_verify(&AACS_LA_PUB_X, &AACS_LA_PUB_Y, &sig_r, &sig_s, &cert[..52]) } -/// Extract public key from certificate. -/// -/// Returns a zeroed key pair if `cert` is too short to hold the fixed -/// offsets (matches the `>= 92` guard in `verify_cert`), so a -/// short/hostile cert cannot panic on the slice index. +// Extract public key from certificate. Returns a zeroed key pair if `cert` +// is too short for the fixed offsets (matches the `>= 92` guard in +// `verify_cert`), so a short/hostile cert cannot panic on the slice index. fn cert_pub_key(cert: &[u8]) -> ([u8; 20], [u8; 20]) { let mut x = [0u8; 20]; let mut y = [0u8; 20]; @@ -868,16 +760,9 @@ fn generate_host_key_pair() -> ([u8; 20], [u8; 20], [u8; 20]) { (key, pub_x, pub_y) } -// ── AES-CMAC (for MAC verification) ──────────────────────────────────────── - -/// AES-128-CMAC, single-complete-block case ONLY. -/// -/// Implements just the exactly-16-byte message path: it derives subkey -/// K1 and XORs the one full block. It does NOT derive K2 or apply the -/// `0x80` 10*-padding, so it is correct only for a 16-byte input — the -/// `&[u8; 16]` signature enforces that at compile time. Do NOT generalize -/// this to multi-block or short-final-block messages without adding K2 + -/// padding. +// ── AES-CMAC (for MAC verification) ── single-complete-block case ONLY: +// derives subkey K1 and XORs one full block, no K2 / `0x80` 10*-padding. +// Correct only for 16-byte input (enforced by `&[u8; 16]`) — don't generalize. fn aes_cmac_16(data: &[u8; 16], key: &[u8; 16]) -> [u8; 16] { use aes::Aes128; use aes::cipher::{Array, BlockCipherEncrypt, KeyInit}; @@ -1062,25 +947,13 @@ fn aacs_authenticate_with_agid( return Err(Error::AacsCertVerify); } } else if is_aacs20 { - // AACS 2.0 certificate — verification intentionally skipped here. - // Reason: backward compatibility. AACS 2.0 drives accept AACS 1.0 host - // certs, so we proceed with the AACS 1.0 flow regardless. The P-256 - // LA public key needed to verify 2.0 certs is not always available, and - // failing here would break handshakes with drives that work fine otherwise. - // The 2.0 cert lays out its public key and signature at different byte - // offsets than the 1.0 cert, so the step-6 verify below (which reads - // 1.0 offsets) cannot validate a 2.0 cert and is skipped for it. + // AACS 2.0 cert — verify intentionally skipped: 2.0 drives accept + // AACS 1.0 host certs for backward compat, the P-256 LA key isn't + // always available, and 2.0 offsets differ from the 1.0 step-6 verify. } else { - // Unknown certificate type — REJECT. Neither the AACS 1.0 LA verify - // above nor the documented 2.0 backward-compat skip applies. This used - // to be an if / else-if with NO else, so an unknown type (e.g. 0x02) - // fell straight through: `is_aacs20` stayed false, so the step-6 - // key-signature check below still ran — against `cert_pub_key(drive_cert)`, - // the drive's OWN key lifted out of a cert that was never checked against - // the LA. A rogue USB bridge could set byte 0 to 0x02, present its own - // keypair, sign `host_nonce || drive_key_point`, and the handshake would - // return Ok with an attacker-chosen bus key. The chain of trust must not - // appear to have run when it did not. + // Unknown cert type — REJECT (see docs/aacs-handshake.md: a missing + // else here used to let an unverified cert's own key satisfy the + // step-6 signature check, an attacker-chosen-bus-key hole). tracing::warn!( target: "freemkv::disc", phase = "aacs_cert_unknown_type", @@ -1100,11 +973,9 @@ fn aacs_authenticate_with_agid( drive_key_point.copy_from_slice(&response[4..44]); drive_key_sig.copy_from_slice(&response[44..84]); - // Verify drive key signature: sign(drive_nonce=host_nonce || drive_key_point). - // Skipped for an AACS 2.0 (type 0x11) cert: `cert_pub_key` reads the public - // key at AACS-1.0 byte offsets, which don't apply to a 2.0 cert, so the - // verify would be meaningless (it would reject every 2.0 drive). Mirrors the - // cert-verify skip above; the ECDH key exchange still proceeds. + // Verify sign(host_nonce || drive_key_point). Skipped for AACS 2.0 (0x11): + // `cert_pub_key` reads AACS-1.0 offsets, meaningless for a 2.0 cert. + // Mirrors the cert-verify skip above; ECDH still proceeds. if !is_aacs20 { let (drive_pub_x, drive_pub_y) = cert_pub_key(&drive_cert); let mut verify_data = [0u8; 60]; @@ -1157,11 +1028,9 @@ fn aacs_authenticate_with_agid( }) } -/// Native AACS 2.0 handshake using P-256/SHA-256 against the real 2.0 LA -/// anchor. Same SCSI protocol as the 1.0 AKE, larger payloads (32-byte keys, -/// 132-byte certs). This is the production entry the cert-agnostic dispatch in -/// [`run_cert_handshake`] reaches when a supplied host cert carries AACS 2.0 -/// credentials and the drive refused the 1.0 cert. +// Native AACS 2.0 handshake using P-256/SHA-256 against the real 2.0 LA +// anchor; same SCSI protocol as the 1.0 AKE, larger payloads. Production +// entry `run_cert_handshake` reaches when the host cert carries 2.0 creds. fn aacs2_authenticate_p256( session: &mut dyn ScsiTransport, host_priv_key: &[u8; 32], @@ -1176,10 +1045,9 @@ fn aacs2_authenticate_p256( ) } -/// [`aacs2_authenticate_p256`] with the LA trust anchor as a parameter. Only a -/// test injects a non-production anchor (to exercise the full AKE under a test -/// LA keypair — the real anchor's private half does not exist for us to sign -/// synthetic certs with). Production threads `AACS2_LA_PUB_X/Y`. +// `aacs2_authenticate_p256` with the LA trust anchor as a parameter, so +// tests can exercise the full AKE under a test LA keypair (the real +// anchor's private half doesn't exist). Production threads `AACS2_LA_PUB_X/Y`. fn aacs2_authenticate_p256_with_anchor( session: &mut dyn ScsiTransport, host_priv_key: &[u8; 32], @@ -1203,10 +1071,8 @@ fn aacs2_authenticate_p256_with_anchor( scsi_read(session, &cdb, 8).map_err(|e| handshake_err(e, Error::AacsAgidAlloc))?; let agid = (response[7] >> 6) & 0x03; - // From here on we HOLD the AGID. All seven fallible steps below used to - // abandon it on the way out — the exact leak `975315d` fixed in the AACS 1.0 - // twin (`aacs_authenticate` / `aacs_authenticate_with_agid`) but left - // untouched on this v2 path. Release it on ANY failure, mirroring v1. + // From here we HOLD the AGID; release on ANY failure below, mirroring + // the leak fix (`975315d`) applied to the AACS 1.0 twin. let r = aacs2_authenticate_p256_with_agid(session, agid, host_priv_key, host_cert, la_x, la_y); if r.is_err() { release_agid(session, agid); @@ -1251,28 +1117,9 @@ fn aacs2_authenticate_p256_with_agid( drive_nonce.copy_from_slice(&response[4..24]); let drive_cert = &response[24..156]; - // Chain-of-trust gate — the SAME shape the AACS 1.0 sibling - // (`aacs_authenticate_with_agid`) enforces, now MANDATORY because this path - // is live. Two distinct rejections, both BEFORE any drive key is trusted: - // - // (a) Cert TYPE gate. An AACS 2.0 drive cert is type 0x11. Anything else - // is unexpected on the native P-256 AKE and is REJECTED here, never - // allowed to fall through. The earlier `if type==0x11 && !verify` - // form left NO branch for a non-0x11 type: a rogue bridge could send - // a type-0x10 (or any) cert, skip verification entirely, and still - // reach the step-6 key check against `cert_pub_key_p256(drive_cert)` - // — its OWN key, lifted from a cert the LA never signed — winning an - // attacker-chosen bus key. The chain of trust must not merely LOOK - // like it ran. - // - // (b) Cert VERIFY is FATAL. A type-0x11 cert whose P-256/SHA-256 - // signature does not verify under the real 2.0 LA anchor - // (`AACS2_LA_PUB`, on-curve — see `la_anchor_keys_are_on_curve`) is - // REJECTED, not logged-and-continued. `ecdsa_verify_p256` - // range/curve-checks every point (`point_on_curve`), so a forged or - // off-curve cert key cannot slip past. The previous "non-fatal for - // backward compat" behaviour was the exact hole the SECURITY note - // demanded be closed before wiring. + // Chain-of-trust gate, mandatory on this live path (see + // docs/aacs-handshake.md): (a) reject any non-0x11 cert type outright, + // (b) treat cert verify failure as FATAL, not logged-and-continued. if drive_cert[0] != 0x11 { tracing::warn!( target: "freemkv::disc", @@ -1376,10 +1223,9 @@ pub fn read_volume_id(session: &mut dyn ScsiTransport, auth: &mut AacsAuth) -> R vid.copy_from_slice(&response[4..20]); mac.copy_from_slice(&response[20..36]); - // Verify MAC: AES-CMAC(VID, bus_key) should equal mac. Compared in constant - // time: `!=` on a byte array short-circuits at the first differing byte, and - // the adversary here is a malicious USB bridge that can time the host's - // reply and learn the MAC prefix byte by byte. Cheap to close. + // Verify MAC = AES-CMAC(VID, bus_key), constant-time: a malicious USB + // bridge could otherwise time a `!=` short-circuit to learn the MAC + // prefix byte by byte. let calc_mac = aes_cmac_16(&vid, &auth.bus_key); if !ct_eq_16(&calc_mac, &mac) { return Err(Error::AacsVidMac); @@ -1404,14 +1250,9 @@ pub fn read_data_keys( enc_rdk.copy_from_slice(&response[4..20]); enc_wdk.copy_from_slice(&response[20..36]); - // Unlike the Volume ID block, format 0x84 carries no MAC — both 16-byte - // slots are key material, so there is nothing to authenticate the response - // against. What we CAN refuse is a response the drive plainly did not fill: - // an all-zero key block AES-decrypts to two perfectly plausible-looking - // 16-byte values, and returning those as `Ok((key, key))` is precisely the - // failure-that-looks-like-success shape — a bus-encrypted disc would be - // "decrypted" with a garbage key and rc=0. (`scsi_read` already guarantees - // the full 36 bytes arrived with a GOOD status.) + // Format 0x84 carries no MAC to verify, but an all-zero key block is a + // response the drive plainly didn't fill — refuse it, since decrypting + // with a garbage key would otherwise look like a successful `Ok`. if enc_rdk == [0u8; 16] && enc_wdk == [0u8; 16] { return Err(Error::AacsDataKey); } @@ -1471,12 +1312,9 @@ pub fn run_cert_handshake( "handshake starting" ); - // Cert-attempt wedge guard. An earlier version fired up to 16 AACS - // authenticate attempts back-to-back with no pause — 80-160 SCSI - // REPORT_KEY/SEND_KEY commands in a few hundred ms, which can drive - // consumer optical drives into a fast-fail firmware wedge (every CDB - // returns ILLEGAL_REQUEST until power-cycled). Defense-in-depth: cap - // attempts, sleep between, bail early on the drive's wedge sense. + // Cert-attempt wedge guard: an earlier version fired attempts back-to-back + // with no pause, which can drive consumer optical drives into a fast-fail + // firmware wedge. Defense-in-depth: cap attempts, sleep between, bail early. const MAX_CERT_ATTEMPTS: usize = 3; const PER_CERT_BACKOFF_MS: u64 = 1000; let mut last_err_code: Option = None; @@ -1484,25 +1322,9 @@ pub fn run_cert_handshake( if idx > 0 { std::thread::sleep(std::time::Duration::from_millis(PER_CERT_BACKOFF_MS)); } - // Cert-agnostic AKE selection. The AACS 1.0 host cert is always present - // and is tried FIRST: an AACS 2.0 drive accepts a 1.0 host cert for - // backward compatibility, which is enough to learn the Volume ID. If the - // drive REFUSES the 1.0 cert (a plain cert-level rejection — NOT a dead - // or wedged bus) and THIS host cert also carries AACS 2.0 (P-256) - // credentials, fall back to the native P-256 AKE. - // - // This does NOT branch on "do we possess 2.0 certs?" as a feature - // switch: it consumes whatever the supplied `HostCert` carries, exactly - // as the 1.0 path consumes `hc.certificate`. A host cert without v2 - // credentials simply has nothing more to present and keeps its 1.0 - // error; a host cert WITH them drives the P-256 AKE the moment the drive - // asks for it. When a real 2.0 host cert is supplied at runtime, the - // P-256 handshake runs — no code change, no possession gate. - // - // A transport fault on the 1.0 attempt short-circuits the fallback: - // re-running the P-256 AKE against a bus that is gone re-issues AGID - // invalidation + SEND/REPORT KEY for nothing and buries the real - // (replug) cause. The Err arm below still classifies it as Transport. + // Cert-agnostic AKE selection: try AACS 1.0 first (also works for + // 2.0 drives via backward compat); fall back to P-256 only on a + // cert-level 1.0 rejection with v2 creds available (docs/aacs-handshake.md). let attempt = match aacs_authenticate(scsi, &hc.private_key, &hc.certificate) { Ok(auth) => Ok(auth), Err(e) if e.is_scsi_transport_failure() => Err(e), @@ -1552,14 +1374,9 @@ pub fn run_cert_handshake( "auth + VID read OK, but the drive served no read_data_key (bus key); \ a bus-encrypted disc stays undecryptable until it does" ); - // A bus that died during the format-0x84 read is NOT - // "the drive served no data key" — its sibling - // `read_volume_id` arm above already learned this. Left - // unclassified, a dead bus returned - // `Ok(CertHandshake{ volume_id: Some, read_data_key: None })`, - // i.e. a dead bus rendered as a successful-looking unlock - // (the flagship failure-that-looks-like-success). Release - // the AGID and abort like the VID path. + // A dead bus here is NOT "no data key served" — left + // unclassified it returned a successful-looking unlock + // with read_data_key: None. Release AGID, abort like VID. if transport { release_agid(scsi, auth.agid); return Err(UnlockError::Transport); @@ -1583,14 +1400,9 @@ pub fn run_cert_handshake( } Err(e) => { last_err_code = Some(e.code()); - // A transport fault must ABORT, not roll on to the next cert. - // `handshake_err` deliberately preserves transport errors so - // the true root cause is surfaced — and then this arm threw - // that away: a transport fault has no sense, so the - // `unwrap_or(false)` below fell through to `continue`, and the - // handshake ran all three certs with a 1 s backoff against a - // dead bus before reporting HandshakeRejected (a cert problem) - // for what is a replug. + // A transport fault must ABORT, not roll on to the next cert: + // it has no sense, so `unwrap_or(false)` below would otherwise + // fall through to `continue` and misreport it as a cert problem. if e.is_scsi_transport_failure() { tracing::warn!( target: "freemkv::disc", @@ -1601,10 +1413,9 @@ pub fn run_cert_handshake( ); return Err(UnlockError::Transport); } - // Read the wedge sense off the structured ScsiSense, NOT - // `e.code()` (a flat constant for every ScsiError). On - // ILLEGAL_REQUEST the drive is signalling it won't talk to us - // — trying more certs worsens the wedge, so bail immediately. + // Wedge sense comes from structured ScsiSense, not `e.code()` + // (flat for every ScsiError). On ILLEGAL_REQUEST, bail now + // rather than worsen the wedge by trying more certs. let sense = e.scsi_sense(); if sense.map(|s| s.is_illegal_request()).unwrap_or(false) { tracing::warn!( @@ -1640,13 +1451,9 @@ pub(crate) mod tests { use super::*; use crate::scsi::mock::{MockTransport, Reply}; - // ── Transport-contract tests ──────────────────────────────────────────── - // - // Before these, every one of this module's 41 tests was pure math: nothing - // drove `aacs_authenticate` / `run_cert_handshake` / `read_volume_id` - // through a transport at all, and no mock in the crate could return `Err` - // or a CHECK CONDITION. That gap is what let the handshake read its own - // zero-filled buffers as drive data and treat a dead bus as a cert problem. + // ── Transport-contract tests ── before these, every test here was pure + // math: nothing drove the handshake fns through a transport that could + // return `Err`/CHECK CONDITION, letting zero-filled buffers pass as data. /// A host cert good enough to reach the SCSI steps (the crypto is exercised /// by the math tests; these tests are about the transport contract). @@ -1694,10 +1501,8 @@ pub(crate) mod tests { )); } - /// Catches deleting the `status` check in `scsi_write`: a drive REFUSING the - /// host certificate answers `Ok` + CHECK CONDITION, and treating that as a - /// successful send carried the handshake on against a drive that never - /// accepted it. + // Catches deleting the `status` check in `scsi_write`: a drive REFUSING + // the cert answers `Ok` + CHECK CONDITION, which must not read as sent. #[test] fn scsi_write_rejects_a_check_condition() { let mut t = MockTransport::always(Reply::illegal_request()); @@ -1716,12 +1521,9 @@ pub(crate) mod tests { assert!(e.is_scsi_transport_failure()); } - /// THE defect-3 test. A dead bus must ABORT the handshake. It used to fall - /// into `continue` — a transport fault carries no sense, so the wedge check - /// `sense.map(..).unwrap_or(false)` was false — and the handshake then ran - /// every remaining cert with a 1 s backoff against a bus that was gone, - /// finally reporting HandshakeRejected (a credentials problem) for what is a - /// replug. Catches restoring that `continue`. + // Defect-3: a dead bus must ABORT, not `continue` into every remaining + // cert with backoff (a transport fault has no sense, so the wedge check + // was false and it misreported HandshakeRejected for a replug). #[test] fn transport_fault_aborts_the_cert_loop_immediately() { let mut t = MockTransport::always(Reply::TransportFault); @@ -1738,10 +1540,9 @@ pub(crate) mod tests { ); } - /// A drive that rejects every cert with ILLEGAL REQUEST is a credentials / - /// wedge situation, NOT a dead bus: it must still report HandshakeRejected - /// so the consumer falls through. Guards against over-correcting defect 3 - /// into "every failure aborts the rip". + // A drive rejecting every cert with ILLEGAL REQUEST is a credentials / + // wedge situation, NOT a dead bus: must still report HandshakeRejected + // (guards against over-correcting defect 3 into "every failure aborts"). #[test] fn drive_rejection_is_still_handshake_rejected_not_transport() { let mut t = MockTransport::always(Reply::illegal_request()); @@ -1750,13 +1551,9 @@ pub(crate) mod tests { assert_eq!(err, crate::UnlockError::HandshakeRejected); } - /// A drive that rejects the FIRST cert (a plain cert-level rejection, not a - /// wedge or a dead bus) must roll on to the second cert — and the roll-on - /// is gated by the per-cert backoff sleep (`idx > 0`), which fires only on - /// the second and later attempts. Pins both: the loop actually retries - /// with a second host cert (not just re-reporting the first failure), and - /// the backoff genuinely elapses real time (catches deleting the sleep or - /// gating it on the wrong condition). + // A cert-level rejection of the FIRST cert must roll on to the second, + // gated by the per-cert backoff (`idx > 0`). Pins both the retry and + // that the backoff genuinely elapses real time. #[test] fn a_rejected_first_cert_backs_off_and_tries_the_second() { let mut t = MockTransport::always(Reply::illegal_request()); @@ -1773,13 +1570,9 @@ pub(crate) mod tests { assert_eq!(t.calls(), 10, "must have actually issued a second attempt"); } - /// Script a full, successful AACS 1.0 mutual auth against the mock. - /// - /// The drive presents a type-0x11 (AACS 2.0) certificate, which the - /// handshake accepts without verifying either the cert or the key signature - /// (documented backward-compat behaviour), and a drive key point of the - /// curve generator so the ECDH bus-key derivation succeeds. `tail` is - /// appended for whatever the test wants to happen after authentication. + // Script a full, successful AACS 1.0 mutual auth against the mock. Drive + // presents a type-0x11 cert (accepted unverified per backward-compat) and + // a generator-point drive key so ECDH succeeds. `tail` runs after auth. fn authenticated_script(tail: Vec) -> Vec { let mut cert_resp = vec![0u8; 116]; cert_resp[24] = 0x11; // drive cert type 0x11 → verification skipped @@ -1802,11 +1595,9 @@ pub(crate) mod tests { s } - /// THE defect-9 test. Authentication SUCCEEDED, then the bus died during the - /// Volume ID read. That is not "this disc has no Volume ID" — it used to - /// return VidUnavailable unconditionally, telling the consumer to carry on - /// with a transport that is gone. Catches dropping the - /// `is_scsi_transport_failure` branch on the VID path. + // Defect-9: auth SUCCEEDED, then the bus died during the VID read. Must + // not return VidUnavailable unconditionally (telling the consumer to + // carry on with a dead transport) — catches dropping that branch. #[test] fn transport_fault_reading_the_volume_id_is_transport_not_vid_unavailable() { let mut t = MockTransport::scripted( @@ -1817,9 +1608,8 @@ pub(crate) mod tests { assert_eq!(err, crate::UnlockError::Transport); } - /// The counterpart: a drive that answers the Volume ID read with a MAC that - /// doesn't verify really is VidUnavailable — the defect-9 fix must not turn - /// every VID failure into a rip-aborting transport error. + // Counterpart: a bad-MAC VID read really is VidUnavailable — the defect-9 + // fix must not turn every VID failure into a transport error. #[test] fn bad_volume_id_mac_is_still_vid_unavailable() { let mut t = MockTransport::scripted( @@ -1836,11 +1626,9 @@ pub(crate) mod tests { assert_eq!(last[10] & 0x3F, 0x3F, "AGID released on the failure path"); } - /// THE defect-6 test. Format 0x84 carries no MAC, so an all-zero response - /// AES-decrypts to two plausible 16-byte values that used to be returned as - /// `Ok((key, key))` — a garbage bus key reported as success, which is how a - /// bus-encrypted disc gets "decrypted" at rc=0. Catches removing the - /// all-zero guard. + // Defect-6: format 0x84 has no MAC, so an all-zero response used to + // AES-decrypt to a plausible garbage bus key reported as `Ok`. Catches + // removing the all-zero guard. #[test] fn read_data_keys_refuses_an_all_zero_response() { let mut t = MockTransport::always(Reply::good(vec![0u8; 36])); @@ -1856,8 +1644,8 @@ pub(crate) mod tests { assert!(auth.read_data_key.is_none(), "no key may be recorded"); } - /// A non-zero key block still decrypts and is returned — the defect-6 guard - /// must reject only the response the drive plainly never filled. + // A non-zero key block still decrypts and is returned — the defect-6 + // guard must reject only a response the drive plainly never filled. #[test] fn read_data_keys_accepts_a_non_zero_response() { let mut resp = vec![0u8; 36]; @@ -1874,15 +1662,9 @@ pub(crate) mod tests { assert_eq!(auth.read_data_key, Some(rdk)); } - /// THE defect-D1 test (chain-of-trust gap). A drive certificate whose type - /// byte is neither 0x01 (AACS 1.0) nor 0x11 (AACS 2.0) must be REJECTED at - /// the cert-type gate. It used to be an if / else-if with no else, so an - /// unknown type fell through: `is_aacs20` stayed false, the step-6 - /// key-signature check still ran against the drive's OWN key lifted from a - /// cert that was never LA-verified, and a rogue bridge presenting type 0x02 - /// with its own keypair could win an attacker-chosen bus key. The distinct - /// `AacsCertVerify` (not the step-6 `AacsKeyVerify`) pins that rejection - /// happens BEFORE any key is trusted. + // Defect-D1: a cert type byte that's neither 0x01 nor 0x11 must be + // REJECTED at the cert-type gate, not fall through to trust the drive's + // own unverified key. `AacsCertVerify` pins it happens before any key is trusted. #[test] fn unknown_drive_cert_type_is_rejected_before_trusting_any_key() { let mut cert_resp = vec![0u8; 116]; @@ -1912,13 +1694,9 @@ pub(crate) mod tests { ); } - /// A transport that plays the DRIVE side of an AACS 1.0 handshake well - /// enough for `aacs_authenticate` + `read_volume_id` to SUCCEED, then dies - /// on the format-0x84 read-data-keys command. It picks its own ECDH keypair - /// and derives the SAME bus key the host does (ECDH is symmetric), so it can - /// answer the Volume ID read with a MAC that actually verifies — the only - /// way to drive `run_cert_handshake` past the VID gate and into the - /// read-data-keys arm with the crate's static mocks unable to. + // Plays the DRIVE side of an AACS 1.0 handshake well enough for auth + + // VID read to SUCCEED, then dies on read-data-keys. Derives the same bus + // key as the host (ECDH is symmetric) so its VID MAC actually verifies. pub(crate) struct DriveEmu { drive_priv: [u8; 20], drive_x: [u8; 20], @@ -2024,13 +1802,9 @@ pub(crate) mod tests { } } - /// THE defect-D5 test (failure-that-looks-like-success). Auth + Volume ID - /// read SUCCEED, then the bus dies during the format-0x84 read-data-keys - /// command. The read_data_keys arm of `run_cert_handshake` did not check - /// `is_scsi_transport_failure()` (its VID-read sibling does), so a dead bus - /// returned `Ok(CertHandshake{ volume_id: Some, read_data_key: None })` — a - /// dead bus rendered as a successful-looking unlock. Catches dropping the - /// transport classification on the data-key arm. + // Defect-D5: auth + VID SUCCEED, then the bus dies on read-data-keys. + // That arm must classify `is_scsi_transport_failure()` like its VID-read + // sibling, or a dead bus renders as a successful-looking unlock. #[test] fn transport_fault_reading_data_keys_is_transport_not_success() { let mut t = DriveEmu::new(); @@ -2265,10 +2039,7 @@ pub(crate) mod tests { #[test] fn test_aes_cmac_nist_kat_full_block() { - // NIST SP 800-38B Appendix D.1, Example 2 (Mlen = 128): - // K = 2b7e1516 28aed2a6 abf71588 09cf4f3c - // M = 6bc1bee2 2e409f96 e93d7e11 7393172a - // T = 070a16b4 6b4d4144 f79bdd9d d04a287c + // NIST SP 800-38B Appendix D.1, Example 2 (Mlen = 128). let key = [ 0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6, 0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c, @@ -2287,10 +2058,9 @@ pub(crate) mod tests { #[test] fn test_vid_mac_verify_roundtrip() { - // Simulate the drive-side: pick a (bus_key, vid), compute the MAC, and - // verify the host-side check accepts it. Then mutate VID and MAC each - // in turn and verify both mutations cause a mismatch (the path that - // would yield Error::AacsVidMac in read_volume_id). + // Simulate the drive-side MAC, verify the host-side check accepts it, + // then mutate VID and MAC in turn and confirm each causes a mismatch + // (the path that yields Error::AacsVidMac in read_volume_id). let bus_key = [ 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10, @@ -2334,10 +2104,8 @@ pub(crate) mod tests { #[test] fn test_vid_mac_all_zero_mac_rejected() { - // Defensive: a buggy or hostile drive that returns all-zero MAC must - // be rejected (the real MAC over any non-trivial VID is nearly never - // 0...0). This guards against a class of "drive returned garbage" - // failures masquerading as success. + // Defensive: a buggy/hostile drive returning an all-zero MAC must be + // rejected — the real MAC over any non-trivial VID is nearly never 0. let bus_key = [ 0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6, 0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c, @@ -2352,10 +2120,8 @@ pub(crate) mod tests { #[test] fn test_verify_cert_p256_short_cert_no_panic() { - // The drive cert the handshake passes in is exactly 132 bytes - // (&response[24..156]); verify_cert_p256 slices cert[68..100]/[100..132] - // and reads cert[..68]. A cert shorter than 132 must return false (cannot - // verify), never panic. Sweep the boundary around the 132 guard. + // verify_cert_p256 slices cert[68..100]/[100..132]/[..68]; a cert + // shorter than 132 must return false, never panic. Sweep the boundary. for len in [0usize, 67, 68, 99, 100, 131] { let cert = vec![0x11u8; len]; assert!( @@ -2409,18 +2175,9 @@ pub(crate) mod tests { ); } - // NOTE: the former `test_verify_host_cert_from_keydb` was removed. It was - // env-gated (HOST_CERT_PATH), inert in CI, and asserted NOTHING — it only - // `eprintln!`d PASS/FAIL. The one property it was reaching for (are the LA - // anchors sound enough to verify a real cert?) is now pinned mechanically by - // `la_anchor_keys_are_on_curve` below, which needs no external file and will - // enforce the moment the real published keys land. - - // ════════════════════════════════════════════════════════════════════ - // Hardening additions - // ════════════════════════════════════════════════════════════════════ - - // ── EC curve invariants: a, b chosen so 4a³+27b² != 0 (nonsingular) ──── + // `test_verify_host_cert_from_keydb` was removed (env-gated, inert, asserted + // nothing); its property is now pinned by `la_anchor_keys_are_on_curve`. + // ── Hardening additions: EC curve invariants (4a³+27b² != 0) ── #[test] fn aacs1_curve_is_nonsingular() { @@ -2586,10 +2343,8 @@ pub(crate) mod tests { #[test] fn cert_pub_key_p256_extracts_offsets_4_36_68() { - // AACS 2.0 (132-byte cert, 4-byte header): pub_x at [4..36], pub_y at - // [36..68] — the same framing the step-6 drive-key message uses. Catches - // regressing to the old 10-byte-header [10..42]/[42..74] offsets, which - // belonged to a 138-byte cert the handshake never actually reads. + // AACS 2.0 (132-byte cert): pub_x@[4..36], pub_y@[36..68]. Catches + // regressing to the old 10-byte-header [10..42]/[42..74] offsets. let mut cert = vec![0u8; 132]; for b in &mut cert[4..36] { *b = 0xC3; @@ -2615,10 +2370,8 @@ pub(crate) mod tests { #[test] fn ecdsa_sign_outputs_are_fixed_width_and_verify() { - // Sign/verify already covered; here assert the (r,s) are full-width - // (the to_bytes_be_padded path must not emit short arrays — a fixed - // [u8;20] return enforces width, but verify the values are non-trivial - // and round-trip). + // Sign/verify already covered; assert (r,s) are full-width and + // non-trivial, and round-trip through verify. let (priv_key, px, py) = generate_host_key_pair(); let (r, s) = ecdsa_sign(&priv_key, b"payload"); assert_ne!(r, [0u8; 20]); @@ -2702,27 +2455,15 @@ pub(crate) mod tests { #[test] fn verify_cert_p256_accepts_132_byte_length_without_panic() { - // 132 bytes is the real cert length (and the minimum that satisfies the - // guard); the slices cert[68..100]/[100..132] are all in-bounds. The - // signature won't verify (all-zero bytes) but it must NOT panic and must - // return false. Catches regressing the guard back to 138 (which rejected - // every real 132-byte cert unconditionally). + // 132 bytes is the real cert length; slices are in-bounds, sig won't + // verify but must NOT panic. Catches regressing the guard to 138. let cert = vec![0x00u8; 132]; assert!(!verify_cert_p256(&cert, &AACS2_LA_PUB_X, &AACS2_LA_PUB_Y)); } - /// THE on-curve guard for the AACS Licensing Administrator anchors. - /// - /// The chain of trust for AACS cert verification roots in the LA public - /// keys (`AACS_LA_PUB_X/Y` for 1.0, `AACS2_LA_PUB_X/Y` for 2.0). Both must - /// lie on their curve or `ecdsa_verify`'s new `point_on_curve(Q)` guard - /// rejects every certificate signed by them. This asserts each anchor - /// satisfies y² ≡ x³ + ax + b (mod p) AND lies in the prime-order subgroup - /// (n·Q == O; both curves have cofactor 1, so on-curve ⇒ in-subgroup, but the - /// order check is cheap insurance). - /// - /// MUTATION: reverting either anchor to the previous OFF-CURVE value makes - /// `point_on_curve` fail here (red-before-green for the anchor landing). + // On-curve guard for the LA anchors: both must satisfy y² ≡ x³+ax+b (mod + // p) and lie in the prime-order subgroup, or `point_on_curve(Q)` rejects + // every cert they sign. Reverting to the OFF-CURVE value fails this. #[test] fn la_anchor_keys_are_on_curve() { // AACS 1.0 LA key on the 160-bit curve. @@ -2763,20 +2504,9 @@ pub(crate) mod tests { } } - /// End-to-end proof that the landed AACS 1.0 LA anchor actually verifies a - /// GENUINE LA-signed certificate — the property the removed, env-gated, - /// assertion-free `test_verify_host_cert_from_keydb` only ever gestured at. - /// - /// The fixture is a real 92-byte AACS 1.0 host certificate (a *public* - /// verification key + a revoked host cert — safe to embed; it carries NO - /// private key). Layout: [0]type [1]version [2..4]len=0x005c [4..10]hostID - /// [10..12]resv [12..52]pubkey(x||y) [52..92]sig(r||s); the LA signs - /// SHA-1(cert[0..52]). A revoked cert still carries a valid LA signature, so - /// this exercises only the signature path. - /// - /// MUTATION: reverting AACS_LA_PUB_X/Y to the previous OFF-CURVE constant - /// makes `verify_cert` return false here (red-before-green for the anchor). - /// It also fails if `verify_cert`'s offsets/SHA-1 or `ecdsa_verify` regress. + // End-to-end proof the landed AACS 1.0 LA anchor verifies a GENUINE + // LA-signed cert. Fixture is a real, revoked 92-byte host cert (public + // key only — safe to embed). Reverting the anchor to OFF-CURVE fails this. #[test] fn verify_cert_accepts_a_genuine_la_signed_host_cert() { // Raw hex of the 92-byte genuine host certificate. @@ -2797,19 +2527,9 @@ pub(crate) mod tests { ); } - // ════════════════════════════════════════════════════════════════════ - // AACS 2.0 (P-256) host-certificate handshake — live-path proofs. - // - // No genuine 2.0 host cert exists for us to sign with (a side agent is - // hunting one; see the #[ignore] slot at the end). So these prove the - // PLUMBING under a self-generated test LA keypair: a 132-byte drive cert - // signed by the test LA is threaded through the SAME code the production - // anchor uses, only the anchor differs (a parameter, not a recompile). The - // layout mirrored here is the code's self-consistent 4-byte-header framing — - // header(4)+pub_x(32)+pub_y(32)+sig_r(32)+sig_s(32)=132, LA signs SHA-256 - // over cert[..68] — identical to the step-6 drive-key message framing and - // the 156-byte SEND KEY host-cert payload (4+20 nonce+132 cert). - // ════════════════════════════════════════════════════════════════════ + // AACS 2.0 (P-256) host-cert handshake — live-path proofs under a + // self-generated test LA keypair (no genuine 2.0 cert exists to sign + // with). See docs/aacs-handshake.md for the framing detail. /// Build a synthetic 132-byte AACS 2.0 drive certificate carrying /// `(pub_x,pub_y)`, signed by the test LA private key over cert[..68] @@ -2831,13 +2551,9 @@ pub(crate) mod tests { cert } - /// A validly-signed synthetic 2.0 cert VERIFIES under its (test) LA anchor, - /// its embedded public key extracts, and a signature by the matching private - /// key round-trips through `ecdsa_verify_p256`. A one-bit flip in the LA - /// signature makes verification FALSE. - /// - /// MUTATION: reverting `verify_cert_p256` to the old `>= 138` length guard, - /// or breaking the SHA-256 signed range, makes the accept assertion go red. + // A validly-signed synthetic 2.0 cert VERIFIES under its test LA anchor, + // its pubkey extracts, and a matching-key signature round-trips. A + // one-bit flip in the LA signature makes verification FALSE. #[test] fn p256_synthetic_la_signed_cert_verifies_and_extracts() { let (la_priv, la_x, la_y) = generate_host_key_pair_p256(); @@ -2873,12 +2589,9 @@ pub(crate) mod tests { ); } - /// A P-256 drive emulator that plays the DRIVE side of the native AACS 2.0 - /// AKE well enough for `aacs2_authenticate_p256_with_anchor` to run to - /// completion and derive a bus key. It holds a long-term keypair whose - /// public half is embedded in `cert` (the test LA signed it), an ephemeral - /// keypair for the ECDH, and — mirroring the 1.0 `DriveEmu` — derives the - /// same bus key the host does so the exchange is symmetric. + // Plays the DRIVE side of the native AACS 2.0 AKE well enough for + // `aacs2_authenticate_p256_with_anchor` to complete and derive a bus key, + // mirroring the 1.0 `DriveEmu` (same bus key on both sides, ECDH symmetric). struct DriveEmuP256 { lt_priv: [u8; 32], eph_priv: [u8; 32], @@ -3023,13 +2736,9 @@ pub(crate) mod tests { ); } - /// THE fatal-verify proof (b): a type-0x11 cert whose LA signature is - /// corrupt must ABORT the handshake — not log-and-continue. - /// - /// MUTATION: reverting the `if !verify_cert_p256(..) { return Err }` back to - /// the old non-fatal `debug!(aacs2_cert_verify_skipped)` lets the AKE reach - /// step 6 (the cert key still matches the long-term key, so the key sig - /// verifies) and return Ok — this assertion then goes red. + // Fatal-verify proof (b): a type-0x11 cert with a corrupt LA signature + // must ABORT, not log-and-continue. Reverting the `if !verify_cert_p256 + // { return Err }` to the old non-fatal debug! lets this return Ok. #[test] fn p256_ake_bad_cert_signature_is_fatal() { let (la_priv, la_x, la_y) = generate_host_key_pair_p256(); @@ -3048,12 +2757,9 @@ pub(crate) mod tests { ); } - /// THE type-gate proof (c): a cert whose type byte is not 0x11 must be - /// REJECTED even when its LA signature is otherwise valid. - /// - /// MUTATION: removing the `if drive_cert[0] != 0x11 { return Err }` gate lets - /// a type-0x10 cert (validly LA-signed here) reach step 6 against its own - /// embedded key and derive an attacker-choosable bus key → Ok → red. + // Type-gate proof (c): a cert type byte != 0x11 must be REJECTED even + // with an otherwise-valid LA signature. Removing the `!= 0x11` gate lets + // it reach step 6 and derive an attacker-choosable bus key. #[test] fn p256_ake_unknown_cert_type_is_rejected() { let (la_priv, la_x, la_y) = generate_host_key_pair_p256(); @@ -3072,13 +2778,9 @@ pub(crate) mod tests { ); } - /// THE off-curve proof (d), AKE level: a validly-signed cert followed by an - /// OFF-CURVE drive key point at step 6 must abort at the bus-key derivation - /// (`compute_bus_key_p256`'s invalid-curve guard) rather than multiply onto - /// a weak curve. - /// - /// MUTATION: removing the `point_on_curve` guard in `compute_bus_key_p256` - /// lets the off-curve multiply proceed and return a bus key → Ok → red. + // Off-curve proof (d), AKE level: a valid cert followed by an OFF-CURVE + // drive key point at step 6 must abort at bus-key derivation + // (`compute_bus_key_p256`'s invalid-curve guard), not multiply onto a weak curve. #[test] fn p256_ake_off_curve_drive_point_is_rejected() { let (la_priv, la_x, la_y) = generate_host_key_pair_p256(); @@ -3097,14 +2799,9 @@ pub(crate) mod tests { ); } - /// THE ECDSA-P256 public-key validation proof (d), primitive level: the - /// identity point Q=(0,0) collapses `u2·Q` and forges a signature with NO - /// private key. `point_on_curve(Q)` (rhs is `b` != 0, so (0,0) is rejected) - /// stops it. The forged (r, s=1) below is constructed to VERIFY if the guard - /// is absent, so this is a genuine red-before-green for that guard. - /// - /// MUTATION: deleting the `point_on_curve(&q.x,&q.y,..)` check in - /// `ecdsa_verify_p256` lets the forgery verify → this assertion goes red. + // ECDSA-P256 pubkey validation proof, primitive level: Q=(0,0) collapses + // `u2·Q` and forges a signature with NO private key. `point_on_curve(Q)` + // stops it; the forged (r, s=1) below VERIFIES if that guard is absent. #[test] fn ecdsa_verify_p256_rejects_identity_key_forgery() { let p = BigUint::from_bytes_be(&P256_P); @@ -3145,25 +2842,9 @@ pub(crate) mod tests { ); } - /// SANITY: `ecdsa_verify_p256` + the P-256 curve constants + SHA-256 are - /// correct against GENUINE AACS 2.0 material. No real 2.0 host/drive AKE cert - /// exists anywhere on this machine (an exhaustive hunt found none — it needs - /// a live USB/SCSI AKE capture), but a real AACS 2.0 CONTENT certificate does - /// (`research/aacs/iso-aacs/CivilWar/Content000.cer`, type 0x10, 232 bytes). - /// It is signed by the AACS 2.0 *Content Cert* key (P-256/SHA-256, 64-byte - /// signature over the first `signed_len` bytes) — a DIFFERENT key for a - /// DIFFERENT purpose than the AKE LA anchor, but it exercises the EXACT same - /// `ecdsa_verify_p256` primitive `verify_cert_p256` relies on; only the - /// pubkey and the offsets differ. Proving it verifies here pins that the - /// P-256 ECDSA/SHA-256 math and the `point_on_curve` gate are right, so when - /// a genuine 2.0 host/drive cert (and its correct signed range) lands, only - /// the offsets — not the crypto — remain to be fixed. - /// - /// The CC pubkey is the published AACS 2.0 Content Certificate public key - /// (P-256). - /// - /// MUTATION: corrupting any `P256_*` constant, or removing SHA-256, makes - /// this real-vector verification go red. + // SANITY: `ecdsa_verify_p256` + P-256 constants + SHA-256 verify against + // GENUINE AACS 2.0 material (a real Content Cert, since no AKE cert + // exists locally). See docs/aacs-handshake.md for the full rationale. #[test] fn ecdsa_verify_p256_verifies_a_genuine_aacs2_content_cert() { fn hx(s: &str) -> Vec { @@ -3200,13 +2881,9 @@ pub(crate) mod tests { ); } - // ── Round-2 coverage: EC math edge cases ──────────────────────────────── - - /// `mod_inv` returns `None` when `gcd(a, m) != 1` — the caller-facing - /// contract every call site (`ec_add`, `ec_double`, `ecdsa_verify*`) - /// pattern-matches on. Called directly with a non-coprime pair (both - /// even), since every production caller passes a genuinely prime curve - /// modulus for which this never fires. + // ── Round-2 coverage: EC math edge cases ── `mod_inv` returns `None` + // when `gcd(a, m) != 1`; called directly with a non-coprime pair since + // production always passes a prime curve modulus. #[test] fn mod_inv_returns_none_for_non_coprime_inputs() { let a = BigUint::from(4u32); @@ -3214,12 +2891,9 @@ pub(crate) mod tests { assert!(mod_inv(&a, &m).is_none()); } - /// `ec_add`'s `dx_inv` guard: when the x-difference is not invertible mod - /// the modulus, addition must return the point at infinity rather than - /// panic or produce a bogus point. Only reachable with a non-prime - /// modulus (every real curve here uses a prime `p`, under which two - /// distinct field elements always have an invertible difference) — a - /// direct, pure-function exercise of the defensive branch. + // `ec_add`'s `dx_inv` guard: a non-invertible x-difference must yield + // infinity, not panic or a bogus point. Only reachable with a non-prime + // modulus (real curves here use prime `p`) — a direct exercise of it. #[test] fn ec_add_returns_infinity_when_dx_not_invertible() { let a = BigUint::from(1u32); @@ -3258,10 +2932,9 @@ pub(crate) mod tests { assert!(r.infinity); } - /// `ecdsa_verify` (AACS 1.0) rejects an off-curve public key up front — - /// the same invalid-curve defence `ecdsa_verify_p256` has its own direct - /// test for (`ecdsa_verify_p256_rejects_identity_key_forgery`), but the - /// 1.0 primitive had none. + // `ecdsa_verify` (AACS 1.0) rejects an off-curve public key up front, + // the same invalid-curve defence the P-256 primitive has its own test + // for, but the 1.0 primitive had none. #[test] fn ecdsa_verify_rejects_off_curve_public_key() { let (priv_key, px, py) = generate_host_key_pair(); @@ -3339,12 +3012,9 @@ pub(crate) mod tests { assert_eq!(t.calls(), 0, "must reject before touching the transport"); } - /// A type-0x01 (AACS 1.0) drive certificate whose LA signature does not - /// verify must be rejected at the cert-verify gate (`Error::AacsCertVerify`, - /// via `verify_cert`) — the type-0x01 sibling of the type-0x11 - /// (`unknown_drive_cert_type_is_rejected_before_trusting_any_key`) and - /// type-0x02 gates, neither of which exercises this specific `if - /// drive_cert[0] == 0x01 { if !verify_cert(..) }` branch. + // A type-0x01 drive cert with a bad LA signature must be rejected at the + // cert-verify gate (`AacsCertVerify` via `verify_cert`) — the type-0x01 + // sibling of the type-0x11/0x02 gate tests, exercising a different branch. #[test] fn aacs_authenticate_v1_type01_bad_signature_is_rejected() { let mut cert_resp = vec![0u8; 116]; @@ -3365,15 +3035,9 @@ pub(crate) mod tests { assert!(matches!(err, Error::AacsCertVerify)); } - // ── Round-2 coverage: AACS 2.0 (P-256) wiring ─────────────────────────── - - /// `aacs2_authenticate_p256` — the thin wrapper threading the REAL - /// production LA anchor (`AACS2_LA_PUB_X/Y`), as opposed to the - /// test-injectable `_with_anchor` every other P-256 AKE test calls — had - /// no direct test. A dead bus on the very first step proves the wrapper - /// forwards correctly and its `if r.is_err() { release_agid }` cleanup - /// runs (the AGID-allocation call itself fails, so no AGID to release — - /// asserting the call count instead pins that no extra CDB is issued). + // ── Round-2 coverage: AACS 2.0 (P-256) wiring ── `aacs2_authenticate_p256` + // (the thin wrapper threading the REAL production LA anchor) had no + // direct test; a dead bus on the first step proves it forwards correctly. #[test] fn aacs2_authenticate_p256_wrapper_propagates_transport_fault() { let mut t = MockTransport::always(Reply::TransportFault); @@ -3397,13 +3061,9 @@ pub(crate) mod tests { assert_eq!(t.calls(), 0, "must reject before touching the transport"); } - /// THE step-6 signature proof, P-256 AKE: a validly LA-signed type-0x11 - /// cert passes the cert-verify gate, but the drive's key-point signature - /// at step 6 is corrupted. Must abort at `ecdsa_verify_p256` (returning - /// `Error::AacsKeyVerify`) — distinct from the cert-verify gate - /// (`p256_ake_bad_cert_signature_is_fatal`) and the bus-key invalid-curve - /// guard (`p256_ake_off_curve_drive_point_is_rejected`), neither of which - /// exercises this specific check. + // Step-6 signature proof, P-256 AKE: a valid cert but a corrupted + // drive key-point signature at step 6 must abort at `ecdsa_verify_p256` + // (`AacsKeyVerify`) — distinct from the cert-verify and off-curve tests. #[test] fn p256_ake_bad_step6_signature_is_rejected() { let (la_priv, la_x, la_y) = generate_host_key_pair_p256(); @@ -3422,13 +3082,9 @@ pub(crate) mod tests { ); } - // ── Round-2 coverage: run_cert_handshake full success ─────────────────── - - /// THE full happy path: auth, Volume ID read, AND the read-data-keys - /// (bus key) read all succeed against a self-consistent drive emulator. - /// Every other `run_cert_handshake` test above deliberately stops short - /// (dead bus / bad MAC / all-zero keys) — this is the only one that - /// reaches the function's final `Ok(CertHandshake { .. })` return. + // ── Round-2 coverage: run_cert_handshake full success ── auth, VID read, + // AND read-data-keys all succeed here; every other test above stops + // short — this is the only one reaching the final `Ok(CertHandshake)`. #[test] fn run_cert_handshake_succeeds_end_to_end() { let mut t = DriveEmu::new(); @@ -3443,12 +3099,9 @@ pub(crate) mod tests { assert!(ch.read_data_key_err.is_none()); } - /// SLOT for a REAL AACS 2.0 host certificate (a side agent is sourcing one). - /// When it lands, drop the genuine 132-byte drive cert + the real - /// `AACS2_LA_PUB_X/Y` anchor here and remove `#[ignore]`: it pins the exact - /// wire layout and signed range end-to-end against real material, the way - /// `verify_cert_accepts_a_genuine_la_signed_host_cert` does for AACS 1.0. - /// Until then this documents the missing fixture without failing CI. + // SLOT for a REAL AACS 2.0 host certificate (a side agent is sourcing + // one): when it lands, drop the fixture + real LA anchor here and remove + // `#[ignore]`. Until then this documents the missing fixture without failing CI. #[test] #[ignore = "pending a genuine AACS 2.0 host/drive certificate fixture"] fn real_aacs2_cert_verifies_under_the_published_la_anchor() { diff --git a/src/aacs/mod.rs b/src/aacs/mod.rs index 6eed789..35aeae6 100644 --- a/src/aacs/mod.rs +++ b/src/aacs/mod.rs @@ -149,11 +149,9 @@ mod tests { let _ = AacsCert::new(); } - /// THE full success path through `Unlocker::unlock_bus` itself: a - /// self-consistent AACS 1.0 drive emulator carries auth + VID + data-key - /// reads all the way to `Ok(Unlocked { .. })`, so the crate's public entry - /// point — not just `run_cert_handshake` underneath it — is proven to wire - /// `volume_id` and `read_data_key` into `Unlocked` correctly. + // Full success path through `Unlocker::unlock_bus`: a self-consistent + // AACS 1.0 emulator proves the public entry point (not just + // `run_cert_handshake`) wires `volume_id`/`read_data_key` into `Unlocked`. #[test] fn unlock_bus_succeeds_end_to_end() { let mut t = handshake::tests::DriveEmu::new(); diff --git a/src/css/error.rs b/src/css/error.rs index 101c452..cdc91e8 100644 --- a/src/css/error.rs +++ b/src/css/error.rs @@ -104,10 +104,9 @@ mod tests { ); } - /// `is_transport_failure` is false for `CssAuthFailed`, false for a - /// `Scsi` error carrying a sense (a drive rejection, not a dead bus) even - /// at the transport-failure status, and true only for the genuine - /// transport fault (status + no sense). + // False for `CssAuthFailed` and for a `Scsi` error with a sense (a + // drive rejection, not a dead bus) even at transport-failure status; + // true only for the genuine transport fault (status + no sense). #[test] fn is_transport_failure_false_paths() { assert!(!Error::CssAuthFailed.is_transport_failure()); diff --git a/src/css/mod.rs b/src/css/mod.rs index 55784af..3954f9f 100644 --- a/src/css/mod.rs +++ b/src/css/mod.rs @@ -12,18 +12,9 @@ mod error; use crate::css::error::{Error, Result, step_err}; use crate::scsi::{DataDirection, ScsiTransport}; -/// Issue ONE CSS bus-auth CDB, honouring the transport contract. -/// -/// The single seam every bus-auth step now goes through. It closes two holes -/// that each step open-coded before: -/// * `Err` was collapsed to `CssAuthFailed`, so a dead bus looked like "this -/// unlocker doesn't apply" and the consumer kept probing it ([`step_err`]); -/// * `Ok` was never inspected, but per the contract a CHECK CONDITION arrives -/// as `Ok` with a non-zero `status` — so a refused command handed the step -/// back its own zero-filled buffer to parse as drive data. -/// -/// `min_bytes` is the number of bytes the step must actually receive (0 for a -/// write / no-data command, whose transferred count no transport promises). +// Issue ONE CSS bus-auth CDB, honouring the transport contract: treats a +// non-zero status as CssAuthFailed and checks bytes_transferred >= min_bytes +// rather than trusting an unfilled buffer. See docs/css-mod.md#css_scsi. fn css_scsi( scsi: &mut dyn ScsiTransport, cdb: &[u8], @@ -209,13 +200,9 @@ impl Default for DvdUnlocker { impl crate::Unlocker for DvdUnlocker { fn name(&self) -> &'static str { - // User-facing unlocker label. This unlocker's job is the DVD - // read-enablement bus-auth (it clears the drive's scrambled-read - // barrier and learns no key) — a property of the DVD medium, NOT of - // whether the content happens to be CSS-scrambled. Reporting it as - // "DVD" is honest: on any DVD the bus-auth ran; the CSS descramble - // itself is keyless and handled downstream, so it is not a separate - // "did an unlocker run" signal. + // "DVD" names the medium this bus-auth unlocks, not the CSS scheme: + // the bus-auth runs on any DVD regardless of whether the content is + // actually scrambled. See docs/css-mod.md#dvdunlocker-name. "DVD" } @@ -227,10 +214,9 @@ impl crate::Unlocker for DvdUnlocker { scsi: &mut dyn ScsiTransport, _ctx: &crate::UnlockCtx, ) -> std::result::Result { - // Self-guard against the hardware — do NOT trust the caller-declared - // DiscKind alone. If the drive does not report a DVD profile, refuse - // (NotApplicable) WITHOUT issuing any CSS CDB, so a mis-routed - // Blu-ray/UHD is never sent CSS bus-auth. + // Self-guard against the hardware, not just the caller-declared + // DiscKind: refuse (NotApplicable) without issuing any CSS CDB when + // the drive doesn't report a DVD profile. if !mounted_disc_is_dvd(scsi)? { tracing::debug!( target: "freemkv::css", @@ -247,14 +233,9 @@ impl crate::Unlocker for DvdUnlocker { } } -/// Transport-level "is the mounted disc a DVD?" probe (GET CONFIGURATION -/// current-profile, DVD family `0x0010..=0x001F`). Lets the DvdUnlocker -/// self-verify against the drive instead of trusting the caller's DiscKind. -/// -/// `Err(Transport)` on a dead bus. This is the FIRST command the DVD unlocker -/// issues, and its match had no `Err` arm at all: a transport fault fell into -/// `_ => false`, which the caller turned into `NotApplicable` — a dead bus read -/// as "not a DVD", with nothing logged anywhere on the path. +// Probes GET CONFIGURATION current-profile (DVD family 0x0010..=0x001F) so +// DvdUnlocker can self-verify against the drive rather than trust the +// caller's DiscKind. See docs/css-mod.md#mounted_disc_is_dvd. fn mounted_disc_is_dvd( scsi: &mut dyn ScsiTransport, ) -> std::result::Result { @@ -305,19 +286,16 @@ fn mounted_disc_is_dvd( fn unlock_css_reads_inner(scsi: &mut dyn ScsiTransport, _lba: u32) -> Result<()> { tracing::debug!(target: "freemkv::css", "css unlock: begin"); - // The bus-auth challenge-response sets the drive's Authentication Success - // Flag (ASF=1), which is what opens scrambled-sector reads. This is the - // ONLY step required to unlock reads; a failure here is fatal — we - // genuinely cannot read scrambled sectors. + // The bus-auth challenge-response sets ASF=1, which is what opens + // scrambled-sector reads. It's the only required step, so a failure + // here is fatal. let agid = establish_authenticated_session(scsi).inspect_err(|e| { tracing::warn!(target: "freemkv::css", error_code = e.code(), "css unlock: bus authentication failed"); })?; tracing::debug!(target: "freemkv::css", agid, "css unlock: bus authentication ok"); - // Disc-key REPORT KEY: issued BEST-EFFORT for any firmware that ties part - // of its read-unlock to it. The bytes are unused (the descramble key is - // recovered keylessly) and a failure is NON-FATAL — the gate is already - // open from bus-auth. This replaces the title-key REPORT KEY, whose hard - // failure used to abort the whole unlock (the 7014 bug on USB bridges). + // Disc-key REPORT KEY: best-effort only, for firmware that ties part of + // its read-unlock to it; bytes are unused and failure is non-fatal. + // See docs/css-mod.md#disc-key-report-key (7014 bug history). if let Err(e) = read_disc_key(scsi, agid) { tracing::debug!(target: "freemkv::css", error_code = e.code(), "css unlock: disc-key REPORT KEY skipped (non-fatal)"); } @@ -327,14 +305,8 @@ fn unlock_css_reads_inner(scsi: &mut dyn ScsiTransport, _lba: u32) -> Result<()> // ── Step 1: Bus Authentication ──────────────────────────────────────────── -/// Run the CSS bus-authentication challenge-response (invalidate AGIDs → -/// allocate AGID → host challenge → brute-force the variant → drive challenge → -/// send host key). Completing the handshake sets the drive's Authentication -/// Success Flag (ASF=1) — which is the ENTIRE purpose: it unlocks -/// scrambled-sector reads. Returns the negotiated AGID (the caller needs it for -/// the best-effort disc-key REPORT KEY). The CSS bus key is intentionally NOT -/// derived: descrambling is keyless — the key is recovered directly from the -/// data — so the bus key has no consumer. +// Runs the CSS bus-auth handshake to set ASF=1 (invalidate AGIDs, allocate, +// challenge/response). Returns the AGID; no bus key is derived. See docs/css-mod.md. fn establish_authenticated_session(scsi: &mut dyn ScsiTransport) -> Result { // Invalidate all AGIDs via REPORT KEY format 0x3F. This is also what makes // an abandoned AGID self-heal — which is not a reason to abandon one. @@ -453,22 +425,15 @@ fn authenticate_with_agid(scsi: &mut dyn ScsiTransport, agid: u8) -> Result<()> )?; // The authenticated session (ASF=1) is now established — scrambled-sector - // reads are unlocked, which is the only thing we needed. The CSS bus key - // would be CryptKey(2, variant, key1 || key2), but it has no consumer - // (descrambling is keyless — the key is recovered directly from the - // data), so it is not derived. + // reads are unlocked. The CSS bus key would be CryptKey(2, variant, + // key1 || key2), but has no consumer (descrambling is keyless). Ok(()) } // ── Step 2: Disc Key ────────────────────────────────────────────────────── -/// Issue READ DVD STRUCTURE format 0x02 (the Disc Key block — opcode 0xAD, NOT -/// the REPORT KEY 0xA4 disc-key block; format 0x01 is Copyright Information) -/// purely for the bus-auth unlock side -/// effect. The returned block contents are not used — the descramble title key -/// is recovered keylessly elsewhere, so the genuine disc-key REPORT KEY is -/// intentionally skipped. (If a drive is ever found where bus-auth alone does -/// not open scrambled reads, a real REPORT KEY format 0x02 belongs here.) +// Issues READ DVD STRUCTURE format 0x02 (opcode 0xAD) purely for its +// bus-auth side effect; returned bytes are unused. See docs/css-mod.md#read_disc_key. fn read_disc_key(scsi: &mut dyn ScsiTransport, agid: u8) -> Result<()> { // READ DVD STRUCTURE, format 0x02 (disc key), 2048+4 bytes let alloc_len: u16 = 2048 + 4; @@ -493,10 +458,8 @@ fn read_disc_key(scsi: &mut dyn ScsiTransport, agid: u8) -> Result<()> { fn crypt_key(key_type: usize, variant: u8, challenge: &[u8; 10]) -> [u8; 5] { // key_type indexes PERM_CHALLENGE ([_;3]); variant indexes - // VARIANTS/PERM_VARIANT ([_;32]). All internal callers pass key_type in - // 0..3 and variant in 0..32; the asserts document the contract for the - // pub(crate) test entry point test_crypt_key and turn a would-be - // out-of-bounds panic into an explicit precondition violation. + // VARIANTS/PERM_VARIANT ([_;32]). Asserts turn an out-of-bounds index + // into an explicit precondition violation. See docs/css-mod.md. debug_assert!(key_type < 3, "crypt_key: key_type out of range"); debug_assert!((variant as usize) < 32, "crypt_key: variant out of range"); let perm = &PERM_CHALLENGE[key_type]; @@ -650,14 +613,9 @@ fn send_key_cdb(agid: u8, format: u8, param_len: u16) -> [u8; 12] { mod tests { use super::*; - /// SECURITY REGRESSION GUARD: no instrumentation in libfreemkv may emit - /// raw key material. Scan every source file for a `tracing` field that - /// binds a forbidden key name to a value-producing expression (`= expr` - /// or `%expr` / `?expr`). The only allowed forms are a string literal - /// (e.g. `disc_key = ""`) or a `_fp` fingerprint field. - /// - /// This is a source-scan test (not a runtime capture) so it stays cheap - /// and catches re-introductions at compile/CI time. + // SECURITY REGRESSION GUARD: scans source files for a `tracing` field + // binding a forbidden key name to a value expression (only a string + // literal or `_fp` field is allowed). See docs/css-mod.md#key-guard. #[test] fn no_key_bytes_in_instrumentation() { use std::path::Path; @@ -709,10 +667,9 @@ mod tests { continue; } for &name in forbidden { - // A fingerprint field (`_fp = ...`) is allowed. - // Match `` followed by optional fingerprint - // suffix then `=` and a value that is NOT a string - // literal redaction marker. + // A fingerprint field (`_fp = ...`) is allowed; + // match `` then `=` with a value that is not a + // string-literal redaction marker. if let Some(idx) = line.find(name) { let after = &line[idx + name.len()..]; let after = after.trim_start(); @@ -743,10 +700,9 @@ mod tests { } } - // Scan this crate's `src` plus the sibling workspace crates so the - // key-material logging guard covers every crate that can reach the - // CSS/AACS internals, not just libfreemkv. Missing sibling dirs (e.g. - // when building the crate standalone) are simply skipped. + // Scan this crate's `src` plus sibling workspace crates so the + // guard covers everything that can reach CSS/AACS internals. + // Missing sibling dirs (standalone builds) are simply skipped. let manifest = Path::new(env!("CARGO_MANIFEST_DIR")); let workspace = manifest.parent().unwrap_or(manifest); let mut violations = Vec::new(); @@ -796,15 +752,8 @@ mod tests { // ── CSS constant-table integrity ─────────────────────────────────────── - /// Each PERM_CHALLENGE row is a permutation of indices 0..10 (it reorders - /// the 10 challenge bytes). A non-permutation would drop/duplicate - /// challenge bytes, weakening or corrupting the bus key derivation. - /// - /// Grounding: crypt_key does `scratch[i] = challenge[perm[i]]` for i in - /// 0..10 — perm must be a bijection on 0..10 to use every challenge byte - /// exactly once. - /// Mutation: change PERM_CHALLENGE[0] entry `9` to `8` (duplicate) -> the - /// "covers 0..10" assert fires. + // Each PERM_CHALLENGE row must be a permutation of indices 0..10; a + // non-permutation would drop/duplicate bytes. See docs/css-mod.md#perm-challenge-rows. #[test] fn perm_challenge_rows_are_permutations() { for (row, perm) in PERM_CHALLENGE.iter().enumerate() { @@ -821,15 +770,9 @@ mod tests { } } - /// Each PERM_VARIANT row maps the 32 variants to 32 distinct 5-bit values - /// (it is a permutation of 0..32). key_type 1 uses PERM_VARIANT[0], - /// key_type 2 uses PERM_VARIANT[1] to pick the css_variant; a collision - /// would make two variants indistinguishable. - /// - /// Grounding: `css_variant = PERM_VARIANT[k][variant]` then indexes - /// VARIANTS[css_variant] (0..32). - /// Mutation: set PERM_VARIANT[0][1] = PERM_VARIANT[0][0] -> duplicate - /// assert fires; also any value >= 32 would later index VARIANTS OOB. + // Each PERM_VARIANT row must map the 32 variants to 32 distinct 5-bit + // values; a collision would make two variants indistinguishable. + // See docs/css-mod.md#perm-variant-rows. #[test] fn perm_variant_rows_are_permutations_of_0_31() { for (row, perm) in PERM_VARIANT.iter().enumerate() { @@ -849,16 +792,8 @@ mod tests { // ── crypt_key behaviour ──────────────────────────────────────────────── - /// crypt_key result depends on every challenge byte. The challenge is - /// permuted into `scratch` and folded through the LFSR seeding and the 6 - /// XOR rounds. Flipping any single challenge byte must change the output. - /// - /// Grounding: scratch[i]=challenge[perm[i]] for all 10 i, and scratch - /// seeds both LFSRs (bytes 5..10 via tmp1) and the round terms (bytes - /// 0..5). - /// Mutation: in `scratch[i] = challenge[perm[i]]` replace with - /// `challenge[i]` for a perm that drops a byte — or hardcode one scratch - /// entry — and some challenge byte stops mattering; this fails. + // crypt_key's result must depend on every challenge byte: flipping any + // single byte must change the output. See docs/css-mod.md#crypt-key-byte-dependence. #[test] fn crypt_key_depends_on_every_challenge_byte() { let base: [u8; 10] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; @@ -874,17 +809,9 @@ mod tests { } } - /// crypt_key(0, v, ..) must produce a DISTINCT result for each of the 32 - /// variants on a fixed challenge. bus_auth brute-forces the variant by - /// matching crypt_key(0, v, host_challenge) == key1; if two variants - /// collided, the wrong variant could be selected and the whole auth - /// derail. - /// - /// Grounding: variant selects css_variant -> VARIANTS[css_variant] -> cse, - /// which feeds every round; distinct variants give distinct cse-driven - /// keys in practice. - /// Mutation: make `cse` ignore the variant (e.g. `let cse = 0`) -> all 32 - /// outputs collapse to one value; the distinctness assert fires. + // crypt_key(0, v, ..) must be distinct for each of the 32 variants: + // bus-auth brute-forces the variant by matching against key1, so a + // collision could select the wrong one. See docs/css-mod.md. #[test] fn crypt_key_type0_distinct_per_variant() { let challenge: [u8; 10] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; @@ -899,16 +826,9 @@ mod tests { } } - /// crypt_key enforces its documented precondition `key_type < 3` via - /// debug_assert (active in test builds). A key_type of 3 would index - /// PERM_CHALLENGE (len 3) out of bounds; the assert turns that into an - /// explicit precondition panic. - /// - /// Grounding: `debug_assert!(key_type < 3, ...)`; PERM_CHALLENGE has 3 - /// rows (indices 0,1,2). - /// Mutation: delete the debug_assert AND the match-arm guard — but the - /// match `_ =>` arm would then index PERM_CHALLENGE[3] OOB and panic - /// differently; with the assert in place this test pins the contract. + // crypt_key's `key_type < 3` debug_assert must fire for key_type 3 + // (which would otherwise index PERM_CHALLENGE out of bounds). + // See docs/css-mod.md#crypt-key-preconditions. #[test] #[should_panic] fn crypt_key_rejects_out_of_range_key_type() { @@ -916,12 +836,9 @@ mod tests { let _ = crypt_key(3, 0, &challenge); } - /// crypt_key enforces `variant < 32` via debug_assert. A variant of 32 - /// would index VARIANTS / PERM_VARIANT (len 32) out of bounds. - /// - /// Grounding: `debug_assert!((variant as usize) < 32, ...)`. - /// Mutation: removing the assert makes this index VARIANTS[32] (still a - /// panic, but unguarded); the assert documents/enforces the contract. + // crypt_key's `variant < 32` debug_assert must fire for variant 32 + // (which would otherwise index VARIANTS/PERM_VARIANT out of bounds). + // See docs/css-mod.md#crypt-key-preconditions. #[test] #[should_panic] fn crypt_key_rejects_out_of_range_variant() { @@ -931,17 +848,8 @@ mod tests { // ── SCSI CDB builders (MMC REPORT KEY / SEND KEY layout) ─────────────── - /// report_key_cdb encodes a 12-byte MMC REPORT KEY (opcode 0xA4) CDB: - /// byte 0 = operation code 0xA4 - /// bytes 8-9 = allocation length, big-endian - /// byte 10 = (AGID << 6) | (key_format & 0x3F) - /// All other bytes are zero. - /// - /// Grounding: MMC REPORT KEY CDB; the AGID is the top 2 bits of byte 10, - /// key format the low 6 bits. - /// Mutation: change `(alloc_len >> 8)` to `alloc_len` for byte 8 (lose the - /// big-endian split) -> byte 8/9 assert fails. Change `agid << 6` to - /// `agid << 5` -> the AGID-position assert fails. + // report_key_cdb encodes a 12-byte MMC REPORT KEY (opcode 0xA4) CDB: + // byte 0=0xA4, bytes 8-9=big-endian len, byte 10=(AGID<<6)|(format&0x3F). #[test] fn report_key_cdb_matches_mmc_layout() { let cdb = report_key_cdb(0b10, 0x04, 0x010C); // AGID=2, format=0x04, len=268 @@ -962,26 +870,18 @@ mod tests { assert_eq!(cdb.len(), 12, "REPORT KEY is a 12-byte CDB"); } - /// The key format field is masked to 6 bits: a format with high bits set - /// must not corrupt the AGID. report_key_cdb(0, 0xFF, _) -> byte 10 low 6 - /// bits = 0x3F, AGID = 0. - /// - /// Grounding: `(agid << 6) | (format & 0x3F)`. - /// Mutation: drop the `& 0x3F` mask -> 0xFF would overwrite the AGID bits; - /// byte 10 would be 0xFF not 0x3F, this fails. + // The key-format field is masked to 6 bits so a format with high bits + // set (e.g. 0xFF) cannot corrupt the AGID bits of byte 10. + // See docs/css-mod.md#cdb-builders. #[test] fn report_key_cdb_masks_format_to_6_bits() { let cdb = report_key_cdb(0, 0xFF, 8); assert_eq!(cdb[10], 0x3F, "format masked to 6 bits, AGID stays 0"); } - /// send_key_cdb encodes a 12-byte MMC SEND KEY (opcode 0xA3) CDB with the - /// parameter-list length at bytes 8-9 (big-endian) and AGID/format at byte - /// 10. - /// - /// Grounding: MMC SEND KEY CDB layout. - /// Mutation: change opcode to SCSI_REPORT_KEY -> opcode assert fails; - /// swap bytes 8/9 -> length assert fails. + // send_key_cdb encodes a 12-byte MMC SEND KEY (opcode 0xA3) CDB with the + // parameter-list length at bytes 8-9 (big-endian) and AGID/format at + // byte 10. See docs/css-mod.md#cdb-builders. #[test] fn send_key_cdb_matches_mmc_layout() { let cdb = send_key_cdb(0b11, 0x03, 0x000C); // AGID=3, format=3, param_len=12 @@ -996,16 +896,9 @@ mod tests { assert_eq!(cdb.len(), 12); } - /// Allocation length larger than 255 must split across bytes 8 (high) and - /// 9 (low) — a 16-bit big-endian field. report_key_cdb with alloc_len - /// 0x0804 (2052, the disc-key block size used in read_disc_key) -> byte 8 - /// = 0x08, byte 9 = 0x04. - /// - /// Grounding: read_disc_key uses `alloc_len = 2048 + 4 = 2052 = 0x0804` - /// and writes `cdb[8] = (alloc_len >> 8); cdb[9] = alloc_len`. - /// Mutation: write only byte 9 (`cdb[9] = alloc_len as u8`) without byte 8 - /// -> the drive sees a 4-byte transfer, truncating the disc-key block; - /// this asserts the high byte is present. + // Allocation length > 255 must split across bytes 8 (high) and 9 (low) + // as a 16-bit big-endian field, e.g. 0x0804 (the disc-key block size). + // See docs/css-mod.md#cdb-builders. #[test] fn report_key_cdb_alloc_len_is_16bit_big_endian() { let cdb = report_key_cdb(0, 0x00, 0x0804); @@ -1013,10 +906,9 @@ mod tests { assert_eq!(cdb[9], 0x04, "low byte of 2052-byte transfer"); } - /// The unlocker's user-facing name is "DVD" (the medium it read-unlocks), - /// not "CSS" (the scheme). Apps render the unlocker report from this name, - /// so it is a stable contract — the bus-auth ran on any DVD, encrypted or - /// not, and the report must say so rather than conflate it with a CSS crack. + // The unlocker's user-facing name is "DVD" (the medium), not "CSS" (the + // scheme); apps render the unlocker report from this name, so it is a + // stable contract. #[test] fn dvd_unlocker_is_named_dvd() { use crate::Unlocker; @@ -1056,10 +948,9 @@ mod tests { assert_eq!(r.unwrap_err(), UnlockError::NotApplicable); } - /// Defense in depth: even when the caller declares `DiscKind::Css`, the - /// DvdUnlocker self-verifies against the drive's GET CONFIGURATION profile. - /// A drive reporting a Blu-ray profile → `NotApplicable`, and NOT a single - /// CSS CDB is issued (no bus-auth fired at a BD). + // Defense in depth: even when the caller declares `DiscKind::Css`, + // DvdUnlocker self-verifies against GET CONFIGURATION; a BD profile + // must yield NotApplicable with no CSS CDB issued. #[test] fn dvd_unlocker_self_guards_against_non_dvd() { use crate::scsi::{DataDirection, ScsiResult}; @@ -1120,11 +1011,9 @@ mod tests { use crate::scsi::mock::{MockTransport, Reply}; use crate::{DiscKind, DriveId, UnlockCtx, UnlockError, Unlocker}; - /// THE defect-7 test. `mounted_disc_is_dvd` is the FIRST command the DVD - /// unlocker issues and its match had no `Err` arm: a transport fault fell - /// into `_ => false`, so a dead bus was reported as "not a DVD" - /// (`NotApplicable`) and the consumer kept probing it — with nothing logged - /// anywhere on the path. Catches restoring that catch-all arm. + // Defect-7 regression: a transport fault on the first probe command + // must abort as Transport, not fall through to NotApplicable (which + // let the consumer keep probing a dead bus). See docs/css-mod.md. #[test] fn transport_fault_probing_for_a_dvd_aborts() { let id = DriveId::default(); @@ -1149,11 +1038,9 @@ mod tests { assert_eq!(r.unwrap_err(), UnlockError::NotApplicable); } - /// A DVD is mounted, then the bus dies during bus-auth. THE defect-2 test: - /// every CSS step used to do `.map_err(|_| CssAuthFailed)`, discarding the - /// `ScsiError` and making the `Transport` arm of the error conversion - /// unreachable from any real call site — so a dead bus arrived as - /// `NotApplicable` and the consumer went on probing it. + // Defect-2 regression: a DVD is mounted, then the bus dies mid + // bus-auth — must abort as Transport, not collapse to CssAuthFailed / + // NotApplicable. See docs/css-mod.md. #[test] fn transport_fault_during_bus_auth_aborts() { let id = DriveId::default(); @@ -1178,10 +1065,9 @@ mod tests { assert_eq!(r.unwrap_err(), UnlockError::NotApplicable); } - /// A conforming transport answering the AGID allocation with a CHECK - /// CONDITION must NOT let the handshake carry on off the caller's own - /// zero-filled buffer (agid 0, a challenge answered by zeros). Catches - /// dropping the `status` check in `css_scsi`. + // A CHECK CONDITION on AGID allocation must not let the handshake carry + // on off the caller's own zero-filled buffer; catches a dropped + // `status` check in `css_scsi`. #[test] fn agid_allocation_check_condition_fails_the_auth() { // 4 AGID invalidations are best-effort; the 5th command is the alloc. @@ -1198,10 +1084,9 @@ mod tests { assert!(matches!(e, Error::CssAuthFailed)); } - /// Defect 18: an AGID allocated and then lost to a failed challenge must be - /// RELEASED (REPORT KEY format 0x3F), not abandoned. A drive has four; the - /// leak self-heals because the next session invalidates all of them, but - /// leaving a drive short between attempts is a state we should not create. + // Defect 18: an AGID lost to a failed challenge must be RELEASED + // (REPORT KEY format 0x3F), not abandoned, even though a drive's four + // AGIDs self-heal on the next session's invalidation pass. #[test] fn a_failed_handshake_releases_the_agid_it_allocated() { let mut t = MockTransport::scripted( @@ -1281,10 +1166,9 @@ mod tests { ); } - /// Host challenge + Key1 both succeed, but the drive challenge REPORT KEY - /// (step 3) is refused. Distinct from `key1_report_failure_fails_the_auth` - /// (step 2) and `key2_send_failure_fails_the_auth` (step 4) below — each - /// pins a different `css_scsi` call site's `?`. + // Host challenge + Key1 succeed but the drive-challenge REPORT KEY + // (step 3) is refused; distinct from the step-2 and step-4 failure + // tests, each pinning a different `css_scsi` call site's `?`. #[test] fn drive_challenge_report_failure_fails_the_auth() { struct FailAtDriveChallenge(FakeDvdDrive); @@ -1368,13 +1252,8 @@ mod tests { // ── Full happy-path bus-auth ──────────────────────────────────────────── - /// A fake drive that plays its half of the CSS bus-auth handshake for - /// real: it captures the host's genuinely-random per-session challenge - /// and answers Key1 with the actual `crypt_key(0, variant, ..)` for a - /// fixed `variant`, so the host's brute-force loop matches it honestly - /// (not via a canned constant the loop happens to hit). Also answers - /// GET CONFIGURATION as a DVD and READ DVD STRUCTURE (disc key) as a - /// success, so the full `unlock_bus` path can be driven end to end. + // A fake drive that plays its half of the CSS handshake for real: + // answers Key1 honestly, and DVD/disc-key probes as success. struct FakeDvdDrive { variant: u8, host_challenge: [u8; 10], @@ -1445,12 +1324,9 @@ mod tests { } } - /// End-to-end happy path for the bus-auth handshake: AGID allocation, - /// host challenge, a Key1 that genuinely matches the brute-forced - /// variant, the drive-challenge/Key2 exchange, and the disc-key REPORT - /// KEY all succeed. Exercises `establish_authenticated_session`'s `Ok` - /// return and `read_disc_key`'s success path, neither of which the - /// failure-path tests above reach. + // End-to-end happy path for the bus-auth handshake (AGID, challenge, + // Key1/Key2, disc-key REPORT KEY), exercising the `Ok` returns that + // the failure-path tests above never reach. #[test] fn full_bus_auth_and_disc_key_succeed() { let mut t = FakeDvdDrive { @@ -1463,10 +1339,9 @@ mod tests { read_disc_key(&mut t, agid).expect("disc-key REPORT KEY succeeds"); } - /// The same happy path driven through the public `unlock_bus` entry - /// point (DVD self-guard, full bus-auth, best-effort disc key), proving - /// `DvdUnlocker::unlock_bus` returns `Ok(Unlocked::default())` on success - /// — the path every failure-injection test above deliberately avoids. + // The same happy path through the public `unlock_bus` entry point, + // proving it returns `Ok(Unlocked::default())` on success — the path + // every failure-injection test above deliberately avoids. #[test] fn dvd_unlocker_succeeds_end_to_end() { let id = DriveId::default(); @@ -1497,10 +1372,9 @@ mod tests { unlock_css_reads(&mut t, 0).expect("css bus-auth + best-effort disc key succeed"); } - /// A drive whose bus-auth succeeds but refuses the best-effort disc-key - /// REPORT KEY (CHECK CONDITION). `unlock_css_reads_inner` must swallow - /// that failure — the read barrier is already open from bus-auth, so the - /// whole unlock must still report `Ok`, not propagate the disc-key error. + // A drive whose bus-auth succeeds but refuses the best-effort disc-key + // REPORT KEY; `unlock_css_reads_inner` must swallow that failure since + // the read barrier is already open from bus-auth. struct DiscKeyRefusingDrive(FakeDvdDrive); impl ScsiTransport for DiscKeyRefusingDrive { @@ -1532,10 +1406,9 @@ mod tests { .expect("bus-auth succeeded; a refused best-effort disc-key read must not fail it"); } - /// `crypt_key`'s `key_type == 2` arm (`PERM_VARIANT[1]`) has no - /// production caller (only key_type 0, the variant brute-force, and 1, - /// the drive-challenge Key2, are ever invoked from `authenticate_with_agid`) - /// but is a pure, directly-testable function of its documented contract. + // `crypt_key`'s key_type==2 arm (`PERM_VARIANT[1]`) has no production + // caller (only 0 and 1 are invoked from `authenticate_with_agid`) but + // is a pure, directly-testable function. #[test] fn crypt_key_type2_is_deterministic_and_distinct_from_type1() { let challenge: [u8; 10] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; diff --git a/src/freemkv/mod.rs b/src/freemkv/mod.rs index d164bf8..a550411 100644 --- a/src/freemkv/mod.rs +++ b/src/freemkv/mod.rs @@ -1,62 +1,12 @@ //! freemkv — the self-identifying custom-firmware unlocker. //! -//! Unlike [`crate::ld`] (MediaTek MT1959, matched against a bundled drive -//! profile database), a freemkv-firmware drive needs no catalog: it answers a -//! single vendor Identity probe with an ASCII payload starting `"freemkv"`. -//! That self-identification IS the detection mechanism — there is no profile -//! lookup here and none is needed. +//! Detects a freemkv-firmware drive by issuing the vendor Identity command +//! (sub-function `0x01` of the `0x3C` READ BUFFER "knock" ABI) and checking +//! that the response starts `b"freemkv"` — no bundled profile database is +//! needed, unlike [`crate::ld`], because the firmware self-identifies. //! -//! # The freemkv vendor ABI (READ BUFFER hijack) -//! -//! Every freemkv command hijacks the standard SCSI `READ BUFFER` (`0x3C`) in -//! "knock mode". The 10-byte CDB is -//! `3C 0E C0 DE 00` -//! — opcode `0x3C`, mode byte `0x0E`, the `C0 DE` knock, a one-byte -//! sub-function, a one-byte state, and a 24-bit big-endian allocation length. -//! (This mirrors the canonical `freemkv-firmware` `abi.rs` frame exactly; the -//! host must never diverge from it.) -//! -//! Sub-functions (`cdb[4]`): -//! - `0x01` **Identity** — read; returns `b"freemkv"` + version, ignores state. -//! - `0x02` **Speed** — the state byte IS the speed cap value: `0x00` = OEM, -//! `0x01`..=`0xFF` a read-speed ceiling, `0xFF` = uncapped / max. This is NOT -//! a plain on/off toggle — see [`SPEED_CAP_OEM`] / [`SPEED_CAP_MAX`]. -//! - `0x03` **Volume ID (VID)** — plaintext Volume ID read; `state 01` = on, -//! `state 00` = OEM. The firmware handler produces the AACS Volume ID and -//! returns the RAW 16-byte VID in the data-in response — NO `b"freemkv"` -//! magic prefix (see the LEAN/RAW rule below). One command returns the VID — -//! there is no separate standard `READ DISC STRUCTURE`. Needs a disc: with no -//! medium loaded the firmware answers CHECK CONDITION (no-medium), which the -//! host reads as "no VID", not a fault. -//! A firmware DEBUG build may also treat the STATE byte as a retrieval -//! VARIANT selector (`0x01`..=`0x05`); see [`read_vid_variant`] / -//! [`probe_vid_variants`], gated behind [`VID_VARIANT_MIN`]..=[`VID_VARIANT_MAX`]. -//! - `0x04` **Bus Encryption** — toggle; `state 01` removes the in-transit -//! (bus) encryption layer so sectors come back in the clear (still AACS -//! content-encrypted; the host applies title keys). -//! - `0x05` **Region-free** — toggle; `state 01` makes the drive RPC -//! region-free. -//! - `0x06`..=`0x08` — reserved; not implemented here. -//! - `0x09` **DumpAll** — diagnostic RAM read; 32-bit address big-endian in -//! `cdb[5..9]`, returns a fixed 64-byte window. -//! -//! # LEAN / RAW replies (FINAL) -//! -//! Only subfn `0x01` **Identity** returns a magic-led reply (`b"freemkv"` + -//! version). EVERY other sub-function is LEAN and RAW: the data-in response is -//! the raw payload with NO `b"freemkv"` prefix. Only the host can issue the -//! `3C 0E C0 DE` knock, so any GOOD-status reply is ours by definition — the -//! magic would be redundant. The Identity magic is therefore the ONE -//! freemkv-detection probe (see [`IDENTITY_MARKER`]); the VID read parses 16 -//! raw bytes directly. -//! -//! # Toggle polarity (UNIFORM) -//! -//! Across EVERY toggle the state byte is `0x00` = OEM behaviour, `0x01` = -//! patched / enabled. There is no per-feature inversion — see [`STATE_OFF`] / -//! [`STATE_ON`]. The two exceptions are not toggles at all: Speed (`0x02`) -//! carries a cap value in the state byte, and DumpAll (`0x09`) carries a 32-bit -//! address across `cdb[5..9]`. +//! The full knock CDB layout, sub-function table, fixed-length reply rules, +//! and toggle-polarity details are documented in `docs/freemkv-abi.md`. use crate::scsi::{DataDirection, ScsiTransport}; use crate::{UnlockCtx, UnlockError, Unlocked, Unlocker}; @@ -70,89 +20,75 @@ const KNOCK_MODE: u8 = 0x0E; const KNOCK_MAGIC: [u8; 2] = [0xC0, 0xDE]; /// State byte that selects OEM behaviour (clears the feature's RAM flag). The -/// OFF pole of EVERY toggle — polarity is uniform across all sub-functions. +/// OFF pole of the Region toggle. const STATE_OFF: u8 = 0x00; /// State byte that enables the patched behaviour (sets the feature's RAM flag). -/// The ON pole of EVERY toggle. +/// The ON pole of the Region toggle. const STATE_ON: u8 = 0x01; /// Speed sub-function (`0x02`) cap value selecting OEM read-speed behaviour. -/// The state byte of subfn 0x02 IS the cap, so this coincides with -/// [`STATE_OFF`] numerically but is a distinct concept (a cap value, not a -/// toggle pole). The default rip flow uses [`SPEED_CAP_MAX`]; this OEM pole is -/// kept as the documented other end of the cap range. #[allow(dead_code)] const SPEED_CAP_OEM: u8 = 0x00; /// Speed sub-function (`0x02`) cap value selecting the uncapped / maximum read /// speed (full riplock lift). `0x01`..=`0xFE` select an intermediate ceiling. const SPEED_CAP_MAX: u8 = 0xFF; +/// Raw Read (`0x04`) mode: OEM cert enforcement. +#[allow(dead_code)] +const RAW_READ_OFF: u8 = 0x00; +/// Raw Read (`0x04`) mode: "cert is valid" — the drive reports host auth as +/// already succeeded, so a bare `0xAD` fmt `0x80` yields the VID with NO cert +/// and NO AKE. This is the mode this unlocker sets. +const RAW_READ_CERT_VALID: u8 = 0x01; +/// Raw Read (`0x04`) mode: "accept any host cert, revoked or not" — the host +/// still runs the real AKE. Driven by the AACS cert unlocker, not here. +#[allow(dead_code)] +const RAW_READ_ACCEPT_ANY: u8 = 0x02; + /// Vendor sub-functions (CDB byte 4). These numeric values ARE the wire -/// protocol and match `freemkv-firmware`'s `abi.rs`. +/// protocol and match `freemkv-firmware`'s `abi.rs::SubFn`. mod subfn { /// Identity — detection (read; ignores state). pub(super) const IDENTITY: u8 = 0x01; /// Speed / riplock (state byte IS the cap value: `00` = OEM, `01`..=`FF` /// ceiling, `FF` = max). pub(super) const SPEED: u8 = 0x02; - /// Volume ID (self-contained read; `01` = produce+return the AACS VID). - pub(super) const VID: u8 = 0x03; - /// Bus Encryption (toggle; `01` = remove the in-transit bus-encryption layer). - pub(super) const BUS_ENCRYPTION: u8 = 0x04; /// Region-free (toggle; `01` = RPC region-free). - pub(super) const REGION: u8 = 0x05; - /// Reserved sub-functions — not implemented here, kept for wire-format - /// completeness so no other feature accidentally reuses these slots. - #[allow(dead_code)] - pub(super) const RESERVED_06: u8 = 0x06; - #[allow(dead_code)] - pub(super) const RESERVED_07: u8 = 0x07; - #[allow(dead_code)] - pub(super) const RESERVED_08: u8 = 0x08; + pub(super) const REGION: u8 = 0x03; + /// Raw Read (transport unlock; state selects the cert-gate mode). + pub(super) const RAW_READ: u8 = 0x04; /// DumpAll diagnostic RAM read (address big-endian in `cdb[5..9]`). pub(super) const DUMP_ALL: u8 = 0x09; } -/// The ASCII magic that leads the Identity (subfn 01) reply ONLY — the -/// `RESP_MAGIC` of the canonical `abi.rs`. Under the LEAN/RAW rule no other -/// sub-function carries it (a GOOD-status reply to the host's knock is ours by -/// definition), so this doubles as the ENTIRE freemkv-detection mechanism (no -/// bundled profile database — the firmware self-identifies). +/// The ASCII magic that leads the Identity (subfn 01) reply — the `RESP_MAGIC` +/// of the canonical `abi.rs`. This is the ENTIRE freemkv-detection mechanism +/// (no bundled profile database — the firmware self-identifies). const IDENTITY_MARKER: &[u8] = b"freemkv"; /// Response buffer size for the Identity probe — comfortably larger than the -/// current `"freemkv 0.6.0"` payload. +/// current `"freemkv 0.6.4"` payload. const IDENTITY_RESPONSE_LEN: usize = 32; -/// Allocation length for a toggle command. A toggle only flips a RAM flag and -/// returns GOOD status — there is no response payload to read back — so the -/// data-in transfer is empty. -const TOGGLE_ALLOC_LEN: u32 = 0; +// Fixed response length for EVERY freemkv knock command; the drive commits this +// many bytes on data-in regardless of subfn. A short/zero allocation desyncs the +// transfer (ABORTED COMMAND, then a wedged FIFO) — see docs/freemkv-abi.md. +const KNOCK_RESP_LEN: usize = 64; /// The 16-byte AACS Volume ID. const VID_LEN: usize = 16; -/// Allocation length for the subfn-0x03 VID read. Under the LEAN/RAW rule the -/// reply is exactly the 16 raw VID bytes (no magic prefix), so this is -/// [`VID_LEN`] — the CDB requests exactly 16 bytes (`00 00 10` big-endian). -const VID_ALLOC_LEN: u32 = VID_LEN as u32; -/// First STATE value the subfn-0x03 VID-variant harness probes. A firmware -/// DEBUG build treats the STATE byte as a retrieval-variant selector; this is -/// the low end of that range (the production [`FreemkvUnlocker::read_vid`] uses -/// [`STATE_ON`], which coincides numerically with this first variant). -const VID_VARIANT_MIN: u8 = 1; -/// Last STATE value the subfn-0x03 VID-variant harness probes (inclusive). -const VID_VARIANT_MAX: u8 = 5; +/// AACS `READ DISC STRUCTURE` (`0xAD`) format for the Volume ID. +const DISC_STRUCT_FMT_VID: u8 = 0x80; +/// Response length requested for the format-0x80 VID structure: a 4-byte header, +/// the 16-byte VID, and a 16-byte MAC. On the bare-read path (no AKE → no bus +/// key) the MAC cannot be verified and is ignored; only `buf[4..20]` matters. +const VID_STRUCT_LEN: u16 = 36; /// Bytes returned by one DumpAll (subfn 0x09) diagnostic read — the firmware /// always commits a fixed 64-byte window. const MEMREAD_LEN: usize = 64; -/// One row of the [`FreemkvUnlocker::probe_vid_variants`] sweep: the STATE -/// `variant` that was probed and its per-variant outcome (a plausible VID, or a -/// non-fatal reason there wasn't one). -type VidVariantOutcome = (u8, std::result::Result<[u8; VID_LEN], UnlockError>); - /// Build the 10-byte knock CDB for a freemkv vendor sub-function: /// `3C 0E C0 DE 00`. fn build_cdb(subfn: u8, state: u8, alloc_len: u32) -> [u8; 10] { @@ -171,11 +107,9 @@ fn build_cdb(subfn: u8, state: u8, alloc_len: u32) -> [u8; 10] { ] } -/// Build the DumpAll (subfn 0x09) CDB: read [`MEMREAD_LEN`] bytes at the 32-bit -/// `addr`, packed big-endian into `cdb[5..9]`. The fixed 64-byte window means -/// the native allocation-length field is reused to carry the address (matching -/// `freemkv-firmware`'s `abi.rs::build_memread_cdb`), so this does NOT go -/// through [`build_cdb`]. +// Build the DumpAll (subfn 0x09) CDB: addr packed big-endian into cdb[5..9] (the +// allocation-length field is reused to carry the address for the fixed 64-byte +// window, matching freemkv-firmware's abi.rs::build_memread_cdb) — bypasses build_cdb. fn build_memread_cdb(addr: u32) -> [u8; 10] { let a = addr.to_be_bytes(); [ @@ -192,6 +126,20 @@ fn build_memread_cdb(addr: u32) -> [u8; 10] { ] } +// Build the STANDARD AACS READ DISC STRUCTURE (0xAD) CDB for the Volume ID +// (format 0x80, Blu-ray, AGID 0) — NOT a vendor knock; valid only once Raw +// Read is on. Mirrors libfreemkv's read_volume_id. +fn build_read_disc_structure_vid_cdb() -> [u8; 12] { + let mut cdb = [0u8; 12]; + cdb[0] = crate::scsi::SCSI_READ_DISC_STRUCTURE; + cdb[1] = 0x01; // Blu-ray + cdb[7] = DISC_STRUCT_FMT_VID; + cdb[8] = (VID_STRUCT_LEN >> 8) as u8; + cdb[9] = (VID_STRUCT_LEN & 0xFF) as u8; + // cdb[10] = agid << 6; AGID 0 on the bare path (no AKE) → 0. + cdb +} + /// Whether a transport error is a genuine dead bus (a senseless /// transport-failure status) rather than a drive rejection surfaced through a /// non-conforming transport (`Err` carrying a sense). @@ -199,23 +147,9 @@ fn is_dead_bus(e: &crate::scsi::ScsiError) -> bool { e.status == crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE && e.sense.is_none() } -/// Whether a 16-byte reply is a plausible Volume ID: exactly [`VID_LEN`] bytes -/// (guaranteed by the array type) and not all-zero. An all-zero buffer is what a -/// CHECK CONDITION or an unmapped debug variant leaves behind, so the -/// [`FreemkvUnlocker::probe_vid_variants`] harness treats it as "no VID" rather -/// than a bogus hit. -fn is_plausible_vid(vid: &[u8; VID_LEN]) -> bool { - vid.iter().any(|&b| b != 0) -} - -/// The freemkv custom-firmware unlocker. `pub(crate)` — clients reach it only -/// through [`crate::all_unlockers`], never by name (the locked-design -/// contract). -/// -/// Detects a drive by issuing the subfn-01 Identity command and checking the -/// response begins with `"freemkv"` — there is no bundled profile catalog to -/// match against (unlike [`crate::ld::Mt1959Unlocker`]), because the firmware -/// answers for itself. +// The freemkv custom-firmware unlocker; pub(crate) so clients reach it only via +// crate::all_unlockers. Detection: subfn-01 Identity, checking the response starts +// "freemkv" — no bundled profile catalog needed, unlike crate::ld::Mt1959Unlocker. pub(crate) struct FreemkvUnlocker; impl FreemkvUnlocker { @@ -223,13 +157,9 @@ impl FreemkvUnlocker { FreemkvUnlocker } - /// Issue the subfn-01 Identity command. `Ok(true)` if the drive - /// self-identifies as freemkv firmware (response starts `"freemkv"`); - /// `Ok(false)` if the drive rejects the command or answers with anything - /// else (not a freemkv drive — fall through). A dead bus is - /// `Err(UnlockError::Transport)` — this is the FIRST command this - /// unlocker issues, so a transport fault here must abort the consumer, - /// not be read as "not a freemkv drive". + // Issue the subfn-01 Identity command: Ok(true) if response starts "freemkv", + // Ok(false) if rejected/mismatched (not this firmware). A dead bus is + // Err(Transport) — this is the FIRST command, so a transport fault must abort. fn identify(&self, scsi: &mut dyn ScsiTransport) -> std::result::Result { let cdb = build_cdb(subfn::IDENTITY, STATE_OFF, IDENTITY_RESPONSE_LEN as u32); let mut buf = vec![0u8; IDENTITY_RESPONSE_LEN]; @@ -248,9 +178,6 @@ impl FreemkvUnlocker { Ok(matched) } Err(e) => { - // Only a senseless transport-failure status is a dead bus; - // anything else (a conforming or non-conforming transport - // reporting a drive rejection) means "not a freemkv drive". if is_dead_bus(&e) { tracing::warn!( target: "freemkv::disc", @@ -270,29 +197,30 @@ impl FreemkvUnlocker { } } - /// Issue a payload-less sub-function carrying an explicit `state` byte (a - /// toggle pole or the Speed cap value). `Ok(())` on GOOD status; - /// `Err(NotApplicable)` if the drive rejects it (firmware lacks the feature - /// — fall through / best-effort); `Err(Transport)` only on a dead bus. + // Issue a payload-less sub-function carrying an explicit state byte (toggle + // pole, Speed cap, or Raw Read mode). Ok(()) on GOOD status; NotApplicable + // if rejected; Transport only on a dead bus. fn send_state( &self, scsi: &mut dyn ScsiTransport, subfn: u8, state: u8, ) -> std::result::Result<(), UnlockError> { - let cdb = build_cdb(subfn, state, TOGGLE_ALLOC_LEN); - // A toggle carries no response payload — zero-length data-in. - let mut buf: [u8; 0] = []; - match scsi.execute(&cdb, DataDirection::None, &mut buf, 5_000) { + // Every knock command returns a fixed KNOCK_RESP_LEN data-in payload; a + // toggle reads and DISCARDS it (only the GOOD status matters), but it MUST + // set up the data-in phase or the drive aborts + wedges (see KNOCK_RESP_LEN). + let cdb = build_cdb(subfn, state, KNOCK_RESP_LEN as u32); + let mut buf = [0u8; KNOCK_RESP_LEN]; + match scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000) { Ok(r) if r.status == 0 => Ok(()), Ok(r) => { tracing::debug!( target: "freemkv::disc", - phase = "freemkv_toggle_rejected", + phase = "freemkv_subfn_rejected", subfn, state, status = r.status, - "freemkv toggle rejected by the drive" + "freemkv sub-function rejected by the drive" ); Err(UnlockError::NotApplicable) } @@ -300,42 +228,27 @@ impl FreemkvUnlocker { if is_dead_bus(&e) { tracing::warn!( target: "freemkv::disc", - phase = "freemkv_toggle_transport_fault", + phase = "freemkv_subfn_transport_fault", subfn, - "transport fault on a freemkv toggle; aborting" + "transport fault on a freemkv sub-function; aborting" ); return Err(UnlockError::Transport); } tracing::debug!( target: "freemkv::disc", - phase = "freemkv_toggle_rejected_as_err", + phase = "freemkv_subfn_rejected_as_err", subfn, status = e.status, - "freemkv toggle rejected (via Err)" + "freemkv sub-function rejected (via Err)" ); Err(UnlockError::NotApplicable) } } } - /// Issue a uniform-polarity toggle sub-function (`on` → [`STATE_ON`], - /// otherwise [`STATE_OFF`]). Thin wrapper over [`Self::send_state`] for the - /// features whose state byte is a plain on/off pole (VID gate, Bus - /// Encryption, Region) — NOT Speed, whose state byte is a cap value. - fn set_toggle( - &self, - scsi: &mut dyn ScsiTransport, - subfn: u8, - on: bool, - ) -> std::result::Result<(), UnlockError> { - let state = if on { STATE_ON } else { STATE_OFF }; - self.send_state(scsi, subfn, state) - } - - /// Speed / riplock (subfn 0x02). The state byte IS the cap: `cap` = `0x00` - /// ([`SPEED_CAP_OEM`]) restores OEM behaviour, `0xFF` ([`SPEED_CAP_MAX`]) - /// lifts riplock to full speed, and `0x01`..=`0xFE` select an intermediate - /// read-speed ceiling. + /// Speed / riplock (subfn 0x02). The state byte IS the cap: `0x00` restores + /// OEM behaviour, `0xFF` lifts riplock to full speed, `0x01`..=`0xFE` select + /// an intermediate ceiling. fn set_speed( &self, scsi: &mut dyn ScsiTransport, @@ -344,71 +257,32 @@ impl FreemkvUnlocker { self.send_state(scsi, subfn::SPEED, cap) } - /// Bus Encryption toggle (subfn 0x04). `on` turns off the AACS bus-encryption - /// layer so sectors come back in the clear (still AACS content-encrypted; - /// the host applies title keys). - fn set_bus_encryption( - &self, - scsi: &mut dyn ScsiTransport, - on: bool, - ) -> std::result::Result<(), UnlockError> { - self.set_toggle(scsi, subfn::BUS_ENCRYPTION, on) - } - - /// Region toggle (subfn 0x05). `on` = DVD region-free. Not part of the - /// default rip flow (region behaviour is a deliberate opt-in), but exposed - /// for completeness. - #[allow(dead_code)] + /// Region toggle (subfn 0x03). `on` = DVD RPC region-free. fn set_region_free( &self, scsi: &mut dyn ScsiTransport, on: bool, ) -> std::result::Result<(), UnlockError> { - self.set_toggle(scsi, subfn::REGION, on) + let state = if on { STATE_ON } else { STATE_OFF }; + self.send_state(scsi, subfn::REGION, state) } - /// Read the AACS Volume ID via the self-contained subfn-0x03 command. - /// - /// Issues ONE command — `3C 0E C0 DE 03 01 00 00 10 00` (`state 01`, alloc - /// 16) — and reads the VID directly from that command's data-in reply. Under - /// the LEAN/RAW rule the reply is the RAW 16-byte VID with NO `b"freemkv"` - /// prefix (only Identity is magic-led): a GOOD-status reply to the host's - /// own knock is ours by definition. There is NO separate READ DISC - /// STRUCTURE. - /// - /// `Ok(Some(vid))` on a full 16-byte GOOD reply; `Ok(None)` when the read is - /// short / drive-rejected (no VID, but not fatal — e.g. no disc / no medium - /// CHECK CONDITION); `Err(Transport)` only on a dead bus. + // Raw Read (subfn 0x04) in RAW_READ_CERT_VALID mode: tells the drive host auth + // already succeeded so a bare 0xAD fmt 0x80 returns the VID with no cert/AKE. + // Load-bearing: if rejected, this firmware can't do the one-command unlock. + fn set_raw_read(&self, scsi: &mut dyn ScsiTransport) -> std::result::Result<(), UnlockError> { + self.send_state(scsi, subfn::RAW_READ, RAW_READ_CERT_VALID) + } + + // Read the AACS VID with a BARE 0xAD fmt 0x80 (valid only after set_raw_read): + // no AKE/cert/bus key needed; VID comes back clear at response[4..20], MAC at + // [20..36] is unverifiable and ignored. Ok/Err semantics: see docs/freemkv-abi.md. fn read_vid( &self, scsi: &mut dyn ScsiTransport, - ) -> std::result::Result, UnlockError> { - match self.read_vid_variant(scsi, STATE_ON) { - Ok(vid) => Ok(Some(vid)), - Err(UnlockError::Transport) => Err(UnlockError::Transport), - // No medium / short / rejected — a clean "no VID", not a fault. - Err(_) => Ok(None), - } - } - - /// Read the RAW 16-byte VID for one subfn-0x03 STATE `variant`. - /// - /// Issues `3C 0E C0 DE 03 00 00 10 00` (alloc [`VID_LEN`]) and - /// returns the 16 raw bytes on GOOD status. This is the primitive both the - /// production [`Self::read_vid`] (`variant` = [`STATE_ON`]) and the debug - /// [`Self::probe_vid_variants`] harness build on — a firmware DEBUG build - /// selects a retrieval variant via this STATE byte. - /// - /// `Ok(vid)` on a full 16-byte GOOD reply; `Err(VidUnavailable)` on a - /// CHECK CONDITION / short / rejected read (no VID / no medium — a clean - /// non-fatal outcome); `Err(Transport)` only on a dead bus. - fn read_vid_variant( - &self, - scsi: &mut dyn ScsiTransport, - variant: u8, ) -> std::result::Result<[u8; VID_LEN], UnlockError> { - let cdb = build_cdb(subfn::VID, variant, VID_ALLOC_LEN); - let mut buf = [0u8; VID_LEN]; + let cdb = build_read_disc_structure_vid_cdb(); + let mut buf = [0u8; VID_STRUCT_LEN as usize]; let result = match scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000) { Ok(r) => r, Err(e) => { @@ -418,9 +292,8 @@ impl FreemkvUnlocker { tracing::debug!( target: "freemkv::disc", phase = "freemkv_vid_rejected_as_err", - variant, status = e.status, - "VID read rejected (via Err); no Volume ID" + "bare VID read rejected (via Err); no Volume ID" ); return Err(UnlockError::VidUnavailable); } @@ -431,64 +304,43 @@ impl FreemkvUnlocker { tracing::debug!( target: "freemkv::disc", phase = "freemkv_vid_check_condition", - variant, status = result.status, - "VID read returned a drive sense (no medium / no VID)" + "bare VID read returned a drive sense (no medium / Raw Read not honoured)" ); return Err(UnlockError::VidUnavailable); } - if result.bytes_transferred < VID_LEN { + // Need the 4-byte header + the 16-byte VID. + if result.bytes_transferred < 4 + VID_LEN { tracing::debug!( target: "freemkv::disc", phase = "freemkv_vid_short_response", - variant, bytes_transferred = result.bytes_transferred, - "VID response too short" + "bare VID response too short" + ); + return Err(UnlockError::VidUnavailable); + } + let mut vid = [0u8; VID_LEN]; + vid.copy_from_slice(&buf[4..4 + VID_LEN]); + // An all-zero VID is what a permissive stub or an unfilled response + // leaves behind — reject it rather than pass a bogus key downstream. + if vid.iter().all(|&b| b == 0) { + tracing::debug!( + target: "freemkv::disc", + phase = "freemkv_vid_all_zero", + "bare VID read returned an all-zero Volume ID" ); return Err(UnlockError::VidUnavailable); } tracing::debug!( target: "freemkv::disc", phase = "freemkv_vid_ok", - variant, - "Volume ID retrieved" + "Volume ID retrieved via bare 0xAD read" ); - Ok(buf) - } - - /// DEBUG harness: probe every subfn-0x03 STATE variant in - /// [`VID_VARIANT_MIN`]..=[`VID_VARIANT_MAX`] IN ORDER, returning one - /// `(variant, result)` per attempt so an operator can see which variant a - /// firmware DEBUG build maps to a plausible VID. - /// - /// A dead bus aborts the whole sweep (`Err(Transport)`); every other - /// per-variant outcome is captured in the returned Vec. A "plausible" VID is - /// 16 bytes and not all-zero — [`is_plausible_vid`] applies that rule, so a - /// variant that returns an all-zero buffer surfaces as `Err(VidUnavailable)` - /// rather than a bogus success. - #[allow(dead_code)] - fn probe_vid_variants( - &self, - scsi: &mut dyn ScsiTransport, - ) -> std::result::Result, UnlockError> { - let mut out = Vec::with_capacity((VID_VARIANT_MAX - VID_VARIANT_MIN + 1) as usize); - for variant in VID_VARIANT_MIN..=VID_VARIANT_MAX { - let res = match self.read_vid_variant(scsi, variant) { - // A dead bus is not a per-variant condition — abort the sweep. - Err(UnlockError::Transport) => return Err(UnlockError::Transport), - Ok(vid) if is_plausible_vid(&vid) => Ok(vid), - Ok(_) => Err(UnlockError::VidUnavailable), - Err(e) => Err(e), - }; - out.push((variant, res)); - } - Ok(out) + Ok(vid) } /// DumpAll diagnostic RAM read (subfn 0x09): return the 64-byte window at /// `addr`. A host-side diagnostic path only — not used by the unlock flow. - /// `Err(Transport)` on a dead bus; `Err(NotApplicable)` on a drive - /// rejection. #[allow(dead_code)] fn dump_ram( &self, @@ -510,17 +362,9 @@ impl FreemkvUnlocker { } } - /// The full freemkv unlock sequence, in fixed order: - /// `01 Identity → 05 Region-free → 04 Bus-off → 03 VID → 02 Speed`. - /// - /// Identity is the only hard gate (a non-freemkv drive is `NotApplicable`). - /// The toggles are best-effort: a firmware that lacks one is logged and - /// skipped (`NotApplicable` from a toggle does not abort), but a dead bus - /// (`Transport`) always aborts — every command here is a freemkv vendor CDB, - /// so a transport fault means the drive stopped answering. `drive_unlocked` - /// reflects whether the bus layer was actually removed (subfn 0x04 accepted). - /// After this returns, disc data is read normally (`READ(10)`) OUTSIDE the - /// unlocker — this only flips the switches and returns the VID. + // Full freemkv unlock sequence: 01 Identity (hard gate) → 03 Region → 02 Speed + // (best-effort) → 04 01 Raw Read → bare 0xAD VID (both load-bearing, no + // fallback). See docs/freemkv-abi.md for full failure-mode semantics. fn full_unlock( &self, scsi: &mut dyn ScsiTransport, @@ -529,43 +373,53 @@ impl FreemkvUnlocker { if !self.identify(scsi)? { return Err(UnlockError::NotApplicable); } - // Run a best-effort toggle, propagating only a dead bus. + // Best-effort feature toggle: only a dead bus aborts. let best_effort = |r: std::result::Result<(), UnlockError>, what: &'static str| - -> std::result::Result { + -> std::result::Result<(), UnlockError> { match r { - Ok(()) => Ok(true), + Ok(()) => Ok(()), Err(UnlockError::Transport) => Err(UnlockError::Transport), Err(_) => { tracing::debug!( target: "freemkv::disc", - phase = "freemkv_toggle_unavailable", - toggle = what, - "toggle unavailable; continuing" + phase = "freemkv_feature_unavailable", + feature = what, + "feature unavailable; continuing" ); - Ok(false) + Ok(()) } } }; - // 05 — Region-free. + // 03 — Region-free (feature). best_effort(self.set_region_free(scsi, true), "region")?; - // 04 — Bus encryption OFF (clear sectors in transit). Tracked so the - // result honestly reports whether the bus layer came off. - let bus_off = best_effort(self.set_bus_encryption(scsi, true), "bus")?; - // 03 — Volume ID (the one value only the drive can give us). - let vid = self.read_vid(scsi)?; - // 02 — Speed / riplock lift to full. + // 02 — Speed / riplock lift to full (feature). best_effort(self.set_speed(scsi, SPEED_CAP_MAX), "speed")?; + // 04 01 — Raw Read "cert valid" (LOAD-BEARING). No fallback: a firmware + // that rejects it can't do the one-command unlock. + match self.set_raw_read(scsi) { + Ok(()) => {} + Err(UnlockError::Transport) => return Err(UnlockError::Transport), + Err(_) => { + tracing::debug!( + target: "freemkv::disc", + phase = "freemkv_raw_read_rejected", + "Raw Read (04 01) rejected — cannot unlock this drive" + ); + return Err(UnlockError::VidUnavailable); + } + } + // Bare 0xAD VID read (LOAD-BEARING). No VID ⇒ fail (propagates). + let vid = self.read_vid(scsi)?; tracing::debug!( target: "freemkv::disc", phase = "freemkv_unlocked", - bus_off, - "freemkv drive unlocked" + "freemkv drive unlocked (Raw Read on, VID read)" ); Ok(Unlocked { - vid, + vid: Some(vid), bus_key: None, - drive_unlocked: bus_off, + drive_unlocked: true, }) } } @@ -575,12 +429,9 @@ impl Unlocker for FreemkvUnlocker { "freemkv" } - /// Unlock DRIVE FEATURES on a freemkv drive: recognise it, lift riplock - /// (subfn 0x02) as a best-effort feature, and read the AACS Volume ID via - /// the self-contained subfn-0x03 command (one command returns the VID; no - /// separate READ DISC STRUCTURE). Bus removal is a separate capability - /// ([`Unlocker::unlock_bus`] → bus encryption), so this does NOT report - /// `drive_unlocked`. + /// Unlock a freemkv drive at drive-prep: recognise it, lift riplock and + /// region (best-effort), turn on Raw Read, and read the Volume ID with a + /// bare `0xAD`. One pass turns every capability on and returns the VID. fn unlock_features( &self, scsi: &mut dyn ScsiTransport, @@ -590,10 +441,8 @@ impl Unlocker for FreemkvUnlocker { } /// Same full sequence as [`Unlocker::unlock_features`]: a freemkv drive is - /// unlocked in one pass (`01→05→04→03→02`), turning every capability on and - /// returning the Volume ID. Both trait entry points delegate to - /// [`FreemkvUnlocker::full_unlock`]; `drive_unlocked` reflects whether the - /// bus layer (subfn 0x04) actually came off. + /// unlocked in one pass, removing bus encryption (Raw Read on) and returning + /// the Volume ID. fn unlock_bus( &self, scsi: &mut dyn ScsiTransport, @@ -638,15 +487,17 @@ mod tests { fn freemkv_identity_payload() -> Vec { let mut p = vec![0u8; IDENTITY_RESPONSE_LEN]; - let s = b"freemkv 0.6.0"; + let s = b"freemkv 0.6.4"; p[..s.len()].copy_from_slice(s); p } - /// A well-formed subfn-0x03 VID response under the LEAN/RAW rule: the raw - /// 16-byte VID with NO `b"freemkv"` prefix. - fn vid_response(vid: [u8; 16]) -> Vec { - vid.to_vec() + /// A well-formed format-0x80 VID structure: 4-byte header, 16-byte VID at + /// offset 4, 16-byte MAC (zeroed on the bare path — it isn't checked). + fn vid_ds_response(vid: [u8; 16]) -> Vec { + let mut p = vec![0u8; VID_STRUCT_LEN as usize]; + p[4..20].copy_from_slice(&vid); + p } // ── CDB shape ──────────────────────────────────────────────────────── @@ -669,72 +520,69 @@ mod tests { /// The 24-bit allocation length is big-endian across bytes 6..8. #[test] fn build_cdb_encodes_alloc_len_24bit_big_endian() { - let cdb = build_cdb(subfn::VID, STATE_ON, 0x01_2345); + let cdb = build_cdb(subfn::RAW_READ, RAW_READ_CERT_VALID, 0x01_2345); assert_eq!([cdb[6], cdb[7], cdb[8]], [0x01, 0x23, 0x45]); - assert_eq!(cdb[4], subfn::VID); + assert_eq!(cdb[4], subfn::RAW_READ); assert_eq!(cdb[5], 0x01); } - /// Toggle polarity is UNIFORM: 0x00 = OEM, 0x01 = patched, for every plain - /// on/off subfn (VID gate, Bus Encryption, Region). Speed (a cap value) and - /// DumpAll (an address) are excluded — they are not toggles. + /// The sub-function numbers ARE the wire protocol and match the firmware + /// `abi.rs::SubFn`: 01 Identity, 02 Speed, 03 Region, 04 Raw Read, 09 DumpAll. #[test] - fn toggle_polarity_is_uniform_off_00_on_01() { - assert_eq!(STATE_OFF, 0x00); - assert_eq!(STATE_ON, 0x01); - for &sf in &[subfn::VID, subfn::BUS_ENCRYPTION, subfn::REGION] { - assert_eq!(build_cdb(sf, STATE_OFF, TOGGLE_ALLOC_LEN)[5], 0x00); - assert_eq!(build_cdb(sf, STATE_ON, TOGGLE_ALLOC_LEN)[5], 0x01); - } - } - - /// The sub-function numbers ARE the wire protocol — pin every one, including - /// the reserved 0x06..=0x08 slots and DumpAll's move to 0x09. - #[test] - fn subfn_values_are_pinned() { + fn subfn_values_match_firmware_abi() { assert_eq!(subfn::IDENTITY, 0x01); assert_eq!(subfn::SPEED, 0x02); - assert_eq!(subfn::VID, 0x03); - assert_eq!(subfn::BUS_ENCRYPTION, 0x04); - assert_eq!(subfn::REGION, 0x05); - assert_eq!(subfn::RESERVED_06, 0x06); - assert_eq!(subfn::RESERVED_07, 0x07); - assert_eq!(subfn::RESERVED_08, 0x08); + assert_eq!(subfn::REGION, 0x03); + assert_eq!(subfn::RAW_READ, 0x04); assert_eq!(subfn::DUMP_ALL, 0x09); } - /// Exact CDB bytes for each sub-function that build_cdb serves — the pinned - /// wire format, one assertion per subfn. + /// The Raw Read mode selectors match the firmware: 00 OEM, 01 cert-valid + /// (this unlocker), 02 accept-any-cert (the AKE path). + #[test] + fn raw_read_modes_are_pinned() { + assert_eq!(RAW_READ_OFF, 0x00); + assert_eq!(RAW_READ_CERT_VALID, 0x01); + assert_eq!(RAW_READ_ACCEPT_ANY, 0x02); + } + + /// Exact CDB bytes for each vendor sub-function that build_cdb serves. #[test] fn build_cdb_exact_bytes_per_subfn() { assert_eq!( build_cdb(subfn::IDENTITY, STATE_OFF, IDENTITY_RESPONSE_LEN as u32), [0x3C, 0x0E, 0xC0, 0xDE, 0x01, 0x00, 0x00, 0x00, 0x20, 0x00] ); + // Toggles carry the fixed KNOCK_RESP_LEN (64 = 0x40) data-in allocation in + // bytes 6..8 — the host must read the response or the drive wedges. assert_eq!( - build_cdb(subfn::SPEED, SPEED_CAP_MAX, TOGGLE_ALLOC_LEN), - [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0xFF, 0x00, 0x00, 0x00, 0x00] + build_cdb(subfn::SPEED, SPEED_CAP_MAX, KNOCK_RESP_LEN as u32), + [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0xFF, 0x00, 0x00, 0x40, 0x00] ); assert_eq!( - build_cdb(subfn::VID, STATE_ON, VID_ALLOC_LEN), - [0x3C, 0x0E, 0xC0, 0xDE, 0x03, 0x01, 0x00, 0x00, 0x10, 0x00] + build_cdb(subfn::REGION, STATE_ON, KNOCK_RESP_LEN as u32), + [0x3C, 0x0E, 0xC0, 0xDE, 0x03, 0x01, 0x00, 0x00, 0x40, 0x00] ); assert_eq!( - build_cdb(subfn::BUS_ENCRYPTION, STATE_ON, TOGGLE_ALLOC_LEN), - [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x01, 0x00, 0x00, 0x00, 0x00] + build_cdb(subfn::RAW_READ, RAW_READ_CERT_VALID, KNOCK_RESP_LEN as u32), + [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x01, 0x00, 0x00, 0x40, 0x00] ); - assert_eq!( - build_cdb(subfn::REGION, STATE_ON, TOGGLE_ALLOC_LEN), - [0x3C, 0x0E, 0xC0, 0xDE, 0x05, 0x01, 0x00, 0x00, 0x00, 0x00] - ); - // DumpAll does NOT go through build_cdb (it packs an address), covered - // by dump_ram_builds_memread_cdb_and_returns_the_window. + } + + /// The bare VID read is the STANDARD 0xAD READ DISC STRUCTURE (format 0x80, + /// Blu-ray, AGID 0, len 36) — NOT a vendor knock. + #[test] + fn build_read_disc_structure_vid_cdb_is_standard_0xad_fmt_80() { + let cdb = build_read_disc_structure_vid_cdb(); + assert_eq!(cdb[0], crate::scsi::SCSI_READ_DISC_STRUCTURE); + assert_eq!(cdb[1], 0x01); // Blu-ray + assert_eq!(cdb[7], 0x80); // AACS Volume ID + assert_eq!([cdb[8], cdb[9]], [0x00, 0x24]); // 36, BE16 + assert_eq!(cdb[10], 0x00); // AGID 0 (no AKE) } // ── Detection ──────────────────────────────────────────────────────── - /// A response beginning `"freemkv"` is a hit, and the Identity CDB is - /// exactly `3C 0E C0 DE 01 00 00 00 20 00`. #[test] fn identify_true_on_freemkv_marker_and_issues_identity_cdb() { let mut t = MockTransport::always(Reply::good(freemkv_identity_payload())); @@ -745,7 +593,6 @@ mod tests { ); } - /// Any other payload (e.g. an MT1959 or unrelated drive) is a miss. #[test] fn identify_false_on_non_matching_payload() { let mut t = FakeTransport { @@ -754,29 +601,24 @@ mod tests { assert!(!FreemkvUnlocker::new().identify(&mut t).expect("no fault")); } - /// A short response that can't even carry the marker is a miss, not a panic. #[test] fn identify_false_on_short_response() { let mut t = MockTransport::always(Reply::short(freemkv_identity_payload(), 3)); assert!(!FreemkvUnlocker::new().identify(&mut t).expect("no fault")); } - /// A drive rejection (CHECK CONDITION) is "not a freemkv drive", not a fault. #[test] fn identify_false_when_command_rejected() { let mut t = MockTransport::always(Reply::illegal_request()); assert!(!FreemkvUnlocker::new().identify(&mut t).expect("no fault")); } - /// The same rejection via a non-conforming transport (`Err` with a sense) - /// reaches the same answer, not a dead bus. #[test] fn identify_false_when_command_rejected_as_err() { let mut t = MockTransport::always(Reply::illegal_request_as_err()); assert!(!FreemkvUnlocker::new().identify(&mut t).expect("no fault")); } - /// A genuine transport fault on the FIRST command must abort the consumer. #[test] fn identify_transport_fault_aborts() { let mut t = MockTransport::always(Reply::TransportFault); @@ -786,68 +628,27 @@ mod tests { ); } - // ── Toggles: exact CDB bytes for each sub-function / state ──────────── + // ── Feature toggles ────────────────────────────────────────────────── - /// Speed carries the cap value in the state byte: `SPEED_CAP_MAX` (0xFF) - /// lifts riplock to full speed. #[test] fn set_speed_max_issues_the_cap_ff_cdb() { let mut t = MockTransport::always(Reply::good(vec![])); FreemkvUnlocker::new() .set_speed(&mut t, SPEED_CAP_MAX) .expect("ok"); - assert_eq!(t.cdbs.len(), 1); assert_eq!( t.cdbs[0], - [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0xFF, 0x00, 0x00, 0x00, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0xFF, 0x00, 0x00, 0x40, 0x00] ); } - /// `SPEED_CAP_OEM` (0x00) restores OEM read-speed behaviour. - #[test] - fn set_speed_oem_issues_the_cap_00_cdb() { - let mut t = MockTransport::always(Reply::good(vec![])); - FreemkvUnlocker::new() - .set_speed(&mut t, SPEED_CAP_OEM) - .expect("ok"); - assert_eq!( - t.cdbs[0], - [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00] - ); - } - - /// An intermediate cap value is placed verbatim in the state byte. #[test] fn set_speed_intermediate_cap_is_placed_in_state_byte() { let mut t = MockTransport::always(Reply::good(vec![])); FreemkvUnlocker::new().set_speed(&mut t, 0x42).expect("ok"); assert_eq!( t.cdbs[0], - [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x42, 0x00, 0x00, 0x00, 0x00] - ); - } - - #[test] - fn set_bus_encryption_on_issues_the_state_01_cdb() { - let mut t = MockTransport::always(Reply::good(vec![])); - FreemkvUnlocker::new() - .set_bus_encryption(&mut t, true) - .expect("ok"); - assert_eq!( - t.cdbs[0], - [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x01, 0x00, 0x00, 0x00, 0x00] - ); - } - - #[test] - fn set_bus_encryption_off_issues_the_state_00_cdb() { - let mut t = MockTransport::always(Reply::good(vec![])); - FreemkvUnlocker::new() - .set_bus_encryption(&mut t, false) - .expect("ok"); - assert_eq!( - t.cdbs[0], - [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x42, 0x00, 0x00, 0x40, 0x00] ); } @@ -859,21 +660,27 @@ mod tests { .expect("ok"); assert_eq!( t.cdbs[0], - [0x3C, 0x0E, 0xC0, 0xDE, 0x05, 0x01, 0x00, 0x00, 0x00, 0x00] + [0x3C, 0x0E, 0xC0, 0xDE, 0x03, 0x01, 0x00, 0x00, 0x40, 0x00] + ); + } + + #[test] + fn set_raw_read_issues_the_04_01_cdb() { + let mut t = MockTransport::always(Reply::good(vec![])); + FreemkvUnlocker::new().set_raw_read(&mut t).expect("ok"); + assert_eq!( + t.cdbs[0], + [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x01, 0x00, 0x00, 0x40, 0x00] ); } - /// A toggle rejected by the drive is `NotApplicable`, not a fault. #[test] fn toggle_drive_rejection_is_not_applicable() { let mut t = MockTransport::always(Reply::illegal_request()); - let err = FreemkvUnlocker::new() - .set_bus_encryption(&mut t, true) - .unwrap_err(); + let err = FreemkvUnlocker::new().set_raw_read(&mut t).unwrap_err(); assert_eq!(err, UnlockError::NotApplicable); } - /// A dead bus on a toggle propagates as `Transport`. #[test] fn toggle_transport_fault_propagates() { let mut t = MockTransport::always(Reply::TransportFault); @@ -883,58 +690,65 @@ mod tests { assert_eq!(err, UnlockError::Transport); } - // ── read_vid (self-contained subfn 0x03) ───────────────────────────── + // ── Bare 0xAD VID read ─────────────────────────────────────────────── - /// read_vid issues ONE command — RAW, alloc 16: `3C 0E C0 DE 03 01 00 00 10 - /// 00` — and reads the 16 raw VID bytes from that reply (NO `b"freemkv"` - /// prefix). No separate READ DISC STRUCTURE. + /// read_vid issues the STANDARD 0xAD READ DISC STRUCTURE and lifts the VID + /// from response[4..20]. #[test] - fn read_vid_issues_one_raw_alloc16_subfn_03_command_and_parses_the_vid() { + fn read_vid_issues_standard_0xad_and_parses_offset_4() { let vid = [0x5Au8; 16]; - let mut t = MockTransport::always(Reply::good(vid_response(vid))); + let mut t = MockTransport::always(Reply::good(vid_ds_response(vid))); let got = FreemkvUnlocker::new().read_vid(&mut t).expect("parse ok"); - assert_eq!(got, Some(vid)); + assert_eq!(got, vid); assert_eq!(t.cdbs.len(), 1, "exactly one command"); - assert_eq!( - t.cdbs[0], - [0x3C, 0x0E, 0xC0, 0xDE, 0x03, 0x01, 0x00, 0x00, 0x10, 0x00] - ); + assert_eq!(t.cdbs[0][0], crate::scsi::SCSI_READ_DISC_STRUCTURE); + assert_eq!(t.cdbs[0][7], 0x80); } - /// The reply is the RAW 16 bytes with NO magic prefix — the whole buffer IS - /// the VID, byte-for-byte. + /// The MAC region (bytes 20..36) is ignored on the bare path — a zero MAC + /// still yields the VID. #[test] - fn read_vid_parses_raw_16_bytes_no_magic() { + fn read_vid_ignores_the_unverifiable_mac() { let vid: [u8; 16] = [ 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, ]; - let resp = vid_response(vid); - assert_eq!(resp.len(), VID_LEN, "raw 16 bytes, no prefix"); - assert_eq!(&resp[..], &vid); + let resp = vid_ds_response(vid); // MAC left zero let mut t = MockTransport::always(Reply::good(resp)); + assert_eq!(FreemkvUnlocker::new().read_vid(&mut t).expect("ok"), vid); + } + + /// A response too short to carry header + VID → VidUnavailable. + #[test] + fn read_vid_short_response_is_vid_unavailable() { + let mut t = MockTransport::always(Reply::short(vid_ds_response([0x11; 16]), 19)); assert_eq!( - FreemkvUnlocker::new().read_vid(&mut t).expect("ok"), - Some(vid) + FreemkvUnlocker::new().read_vid(&mut t).unwrap_err(), + UnlockError::VidUnavailable ); } - /// A response too short to carry the 16-byte VID → `Ok(None)` (no VID, not fatal). + /// A CHECK CONDITION (no medium / Raw Read not honoured) → VidUnavailable, + /// never a VID parsed from the zero fill. #[test] - fn read_vid_short_response_is_none() { - let mut t = MockTransport::always(Reply::short(vid_response([0x11; 16]), 15)); - assert_eq!(FreemkvUnlocker::new().read_vid(&mut t).expect("ok"), None); - } - - /// A CHECK CONDITION (no medium / no disc) → clean `Ok(None)`, never a VID - /// parsed from the zero fill. - #[test] - fn read_vid_check_condition_is_none() { + fn read_vid_check_condition_is_vid_unavailable() { let mut t = MockTransport::always(Reply::illegal_request()); - assert_eq!(FreemkvUnlocker::new().read_vid(&mut t).expect("ok"), None); + assert_eq!( + FreemkvUnlocker::new().read_vid(&mut t).unwrap_err(), + UnlockError::VidUnavailable + ); + } + + /// An all-zero VID (permissive stub / unfilled response) is rejected. + #[test] + fn read_vid_all_zero_is_vid_unavailable() { + let mut t = MockTransport::always(Reply::good(vid_ds_response([0u8; 16]))); + assert_eq!( + FreemkvUnlocker::new().read_vid(&mut t).unwrap_err(), + UnlockError::VidUnavailable + ); } - /// A dead bus on the VID read propagates as `Transport`. #[test] fn read_vid_transport_fault_propagates() { let mut t = MockTransport::always(Reply::TransportFault); @@ -944,149 +758,20 @@ mod tests { ); } - // ── VID-variant debug harness (subfn 0x03 STATE selects a variant) ──── + // ── full_unlock / unlock_features / unlock_bus ─────────────────────── - /// The variant range is pinned: 1..=5, the STATE values a firmware DEBUG - /// build maps to retrieval variants. + /// The full unlock runs `01→03→02→04→AD` in order: identity, region, speed, + /// raw-read, bare VID read. This is the load-bearing sequence test. #[test] - fn vid_variant_range_is_pinned_1_to_5() { - assert_eq!(VID_VARIANT_MIN, 1); - assert_eq!(VID_VARIANT_MAX, 5); - } - - /// Each variant places its number verbatim in the STATE byte (cdb[5]) and - /// keeps the RAW alloc-16 shape (`00 00 10` at bytes 6..8). - #[test] - fn read_vid_variant_places_variant_in_state_byte() { - for variant in VID_VARIANT_MIN..=VID_VARIANT_MAX { - let vid = [variant; 16]; - let mut t = MockTransport::always(Reply::good(vid_response(vid))); - let got = FreemkvUnlocker::new() - .read_vid_variant(&mut t, variant) - .expect("variant ok"); - assert_eq!(got, vid); - assert_eq!( - t.cdbs[0], - [ - 0x3C, 0x0E, 0xC0, 0xDE, 0x03, variant, 0x00, 0x00, 0x10, 0x00 - ], - "variant {variant} CDB", - ); - } - } - - /// A no-medium CHECK CONDITION on a variant is the clean "no VID" - /// `VidUnavailable`, not a fault. - #[test] - fn read_vid_variant_check_condition_is_vid_unavailable() { - let mut t = MockTransport::always(Reply::illegal_request()); - assert_eq!( - FreemkvUnlocker::new() - .read_vid_variant(&mut t, VID_VARIANT_MIN) - .unwrap_err(), - UnlockError::VidUnavailable - ); - } - - /// A dead bus on a variant propagates as `Transport`. - #[test] - fn read_vid_variant_transport_fault_propagates() { - let mut t = MockTransport::always(Reply::TransportFault); - assert_eq!( - FreemkvUnlocker::new() - .read_vid_variant(&mut t, VID_VARIANT_MIN) - .unwrap_err(), - UnlockError::Transport - ); - } - - /// probe_vid_variants issues variants 1..=5 IN ORDER — one command per - /// variant, each carrying its number in the STATE byte. - #[test] - fn probe_vid_variants_issues_variants_1_to_5_in_order() { - let mut t = MockTransport::always(Reply::good(vid_response([0xA5; 16]))); - let results = FreemkvUnlocker::new() - .probe_vid_variants(&mut t) - .expect("no dead bus"); - // One result per variant, in order. - let variants: Vec = results.iter().map(|(v, _)| *v).collect(); - assert_eq!(variants, vec![1, 2, 3, 4, 5]); - // One CDB per variant, each with its number in cdb[5], in order. - assert_eq!(t.cdbs.len(), 5); - for (i, variant) in (VID_VARIANT_MIN..=VID_VARIANT_MAX).enumerate() { - assert_eq!( - t.cdbs[i], - [ - 0x3C, 0x0E, 0xC0, 0xDE, 0x03, variant, 0x00, 0x00, 0x10, 0x00 - ], - ); - } - } - - /// A variant returning a plausible (non-zero) 16-byte VID is `Ok(vid)`; an - /// all-zero reply is rejected as `VidUnavailable`, not a bogus hit. - #[test] - fn probe_vid_variants_flags_plausible_vs_all_zero() { - // Variant 1 rejected (no VID), variants 2..5 return a plausible VID. - let plausible = [0x3Cu8; 16]; - let mut t = MockTransport::scripted( - vec![ - Reply::illegal_request(), - Reply::good(vid_response(plausible)), - Reply::good(vid_response([0u8; 16])), // all-zero → not plausible - Reply::good(vid_response(plausible)), - Reply::good(vid_response(plausible)), - ], - Reply::TransportFault, - ); - let results = FreemkvUnlocker::new() - .probe_vid_variants(&mut t) - .expect("no dead bus"); - assert_eq!(results[0], (1, Err(UnlockError::VidUnavailable))); - assert_eq!(results[1], (2, Ok(plausible))); - assert_eq!(results[2], (3, Err(UnlockError::VidUnavailable))); - assert_eq!(results[3], (4, Ok(plausible))); - assert_eq!(results[4], (5, Ok(plausible))); - } - - /// A dead bus aborts the whole sweep with `Transport`, not a per-variant result. - #[test] - fn probe_vid_variants_transport_fault_aborts_sweep() { - let mut t = MockTransport::always(Reply::TransportFault); - assert_eq!( - FreemkvUnlocker::new() - .probe_vid_variants(&mut t) - .unwrap_err(), - UnlockError::Transport - ); - } - - /// `is_plausible_vid`: 16 non-zero bytes pass, an all-zero buffer fails. - #[test] - fn is_plausible_vid_rejects_all_zero() { - assert!(is_plausible_vid(&[0x01; 16])); - assert!(is_plausible_vid(&{ - let mut v = [0u8; 16]; - v[15] = 1; - v - })); - assert!(!is_plausible_vid(&[0u8; 16])); - } - - // ── unlock_features / unlock_bus ───────────────────────────────────── - - /// The full unlock runs `01→05→04→03→02` in order: identity, region-free, - /// bus-off, VID, speed. This is the load-bearing sequence test. - #[test] - fn full_unlock_issues_01_05_04_03_02_in_order() { + fn full_unlock_issues_identity_region_speed_rawread_then_bare_vid() { let vid = [0x7Cu8; 16]; let mut t = MockTransport::scripted( vec![ Reply::good(freemkv_identity_payload()), // 01 Identity - Reply::good(vec![]), // 05 Region-free - Reply::good(vec![]), // 04 Bus-off - Reply::good(vid_response(vid)), // 03 VID + Reply::good(vec![]), // 03 Region Reply::good(vec![]), // 02 Speed + Reply::good(vec![]), // 04 01 Raw Read + Reply::good(vid_ds_response(vid)), // 0xAD bare VID ], Reply::TransportFault, ); @@ -1094,51 +779,27 @@ mod tests { let u = FreemkvUnlocker::new() .unlock_features(&mut t, &ctx(&id)) .expect("recognized"); - assert!(u.drive_unlocked, "bus toggle accepted → drive unlocked"); + assert!(u.drive_unlocked, "Raw Read on ⇒ drive unlocked"); assert_eq!(u.vid, Some(vid)); - // subfn bytes in issue order: 01 Identity, 05 Region, 04 Bus, 03 VID, 02 Speed. - let subfns: Vec = t.cdbs.iter().map(|c| c[4]).collect(); - assert_eq!(subfns, vec![0x01, 0x05, 0x04, 0x03, 0x02]); - // Speed carries the max cap in its state byte. - assert_eq!(t.cdbs[4][5], SPEED_CAP_MAX); + // Four vendor knocks then the standard 0xAD read. + assert_eq!(t.cdbs.len(), 5); + let vendor_subfns: Vec = t.cdbs[..4].iter().map(|c| c[4]).collect(); + assert_eq!(vendor_subfns, vec![0x01, 0x03, 0x02, 0x04]); + assert_eq!(t.cdbs[3][5], RAW_READ_CERT_VALID); // Raw Read state 01 + assert_eq!(t.cdbs[4][0], crate::scsi::SCSI_READ_DISC_STRUCTURE); } - /// `unlock_features` recognises the drive, runs the full sequence, and - /// reports the VID plus `drive_unlocked` (the bus toggle came off). + /// A missing region/speed feature does not fail the unlock (best-effort). #[test] - fn unlock_features_recognizes_and_reads_vid() { - let vid = [0x7Cu8; 16]; - // identity, region, bus, vid, speed. - let mut t = MockTransport::scripted( - vec![ - Reply::good(freemkv_identity_payload()), - Reply::good(vec![]), - Reply::good(vec![]), - Reply::good(vid_response(vid)), - Reply::good(vec![]), - ], - Reply::TransportFault, - ); - let id = crate::DriveId::default(); - let u = FreemkvUnlocker::new() - .unlock_features(&mut t, &ctx(&id)) - .expect("recognized"); - assert!(u.drive_unlocked, "bus toggle accepted"); - assert_eq!(u.vid, Some(vid)); - } - - /// A missing speed feature does not fail the unlock — VID is read before - /// speed (best-effort riplock), so a rejected speed toggle is tolerated. - #[test] - fn unlock_features_tolerates_missing_speed() { + fn full_unlock_tolerates_missing_region_and_speed() { let vid = [0x3Cu8; 16]; let mut t = MockTransport::scripted( vec![ Reply::good(freemkv_identity_payload()), // identity - Reply::good(vec![]), // region - Reply::good(vec![]), // bus - Reply::good(vid_response(vid)), // vid - Reply::illegal_request(), // speed toggle unsupported + Reply::illegal_request(), // region unsupported + Reply::illegal_request(), // speed unsupported + Reply::good(vec![]), // raw read + Reply::good(vid_ds_response(vid)), // bare VID ], Reply::TransportFault, ); @@ -1147,6 +808,48 @@ mod tests { .unlock_features(&mut t, &ctx(&id)) .expect("recognized"); assert_eq!(u.vid, Some(vid)); + assert!(u.drive_unlocked); + } + + /// Raw Read is LOAD-BEARING: if the drive rejects it, the unlock FAILS with + /// VidUnavailable — no fallback. + #[test] + fn full_unlock_fails_when_raw_read_rejected() { + let mut t = MockTransport::scripted( + vec![ + Reply::good(freemkv_identity_payload()), // identity + Reply::good(vec![]), // region + Reply::good(vec![]), // speed + Reply::illegal_request(), // raw read REJECTED + ], + Reply::TransportFault, + ); + let id = crate::DriveId::default(); + let err = FreemkvUnlocker::new() + .unlock_features(&mut t, &ctx(&id)) + .unwrap_err(); + assert_eq!(err, UnlockError::VidUnavailable); + } + + /// No VID (Raw Read accepted but the bare read yields nothing) is a FAIL, + /// not an Ok with vid: None. + #[test] + fn full_unlock_fails_when_no_vid() { + let mut t = MockTransport::scripted( + vec![ + Reply::good(freemkv_identity_payload()), // identity + Reply::good(vec![]), // region + Reply::good(vec![]), // speed + Reply::good(vec![]), // raw read + Reply::illegal_request(), // bare VID: no medium + ], + Reply::TransportFault, + ); + let id = crate::DriveId::default(); + let err = FreemkvUnlocker::new() + .unlock_features(&mut t, &ctx(&id)) + .unwrap_err(); + assert_eq!(err, UnlockError::VidUnavailable); } #[test] @@ -1169,19 +872,17 @@ mod tests { assert_eq!(err, UnlockError::Transport); } - /// `unlock_bus` runs the same full sequence; the bus toggle (subfn 0x04) is - /// the third CDB (after identity + region), and `drive_unlocked` is set. + /// unlock_bus runs the same full sequence and reports the VID + unlocked. #[test] - fn unlock_bus_enables_bus_encryption_and_reports_unlocked() { + fn unlock_bus_runs_full_sequence_and_reports_unlocked() { let vid = [0x9Au8; 16]; - // identity, region, bus, vid, speed. let mut t = MockTransport::scripted( vec![ Reply::good(freemkv_identity_payload()), Reply::good(vec![]), Reply::good(vec![]), - Reply::good(vid_response(vid)), Reply::good(vec![]), + Reply::good(vid_ds_response(vid)), ], Reply::TransportFault, ); @@ -1191,35 +892,6 @@ mod tests { .expect("bus removed"); assert!(u.drive_unlocked); assert_eq!(u.vid, Some(vid)); - // The bus encryption toggle is the third CDB (identity, region, THEN bus). - assert_eq!( - t.cdbs[2], - [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x01, 0x00, 0x00, 0x00, 0x00] - ); - } - - /// A drive that rejects the bus toggle does NOT abort the unlock (best-effort): - /// the sequence continues, the VID is still read, and `drive_unlocked` is - /// reported false because the bus layer did not come off. - #[test] - fn unlock_bus_bus_encryption_rejected_is_best_effort() { - let vid = [0x55u8; 16]; - let mut t = MockTransport::scripted( - vec![ - Reply::good(freemkv_identity_payload()), // identity - Reply::good(vec![]), // region - Reply::illegal_request(), // bus toggle unsupported - Reply::good(vid_response(vid)), // vid still read - Reply::good(vec![]), // speed - ], - Reply::TransportFault, - ); - let id = crate::DriveId::default(); - let u = FreemkvUnlocker::new() - .unlock_bus(&mut t, &ctx(&id)) - .expect("continues despite bus rejection"); - assert!(!u.drive_unlocked, "bus toggle rejected → not unlocked"); - assert_eq!(u.vid, Some(vid)); } #[test] @@ -1234,8 +906,6 @@ mod tests { // ── DumpAll (subfn 0x09) ───────────────────────────────────────────── - /// DumpAll packs the 32-bit address big-endian into `cdb[5..9]` (subfn now - /// at 0x09) and returns the 64-byte window. #[test] fn dump_ram_builds_memread_cdb_and_returns_the_window() { let mut payload = vec![0xABu8; MEMREAD_LEN]; @@ -1251,8 +921,6 @@ mod tests { ); } - /// The DumpAll CDB carries the subfn at cdb[4] and the full 32-bit address - /// big-endian across cdb[5..9], leaving the trailing control byte zero. #[test] fn build_memread_cdb_packs_address_big_endian_at_5_to_9() { let cdb = build_memread_cdb(0xDEAD_BEEF); diff --git a/src/ld/NOTICE b/src/ld/NOTICE new file mode 100644 index 0000000..d63c5e0 --- /dev/null +++ b/src/ld/NOTICE @@ -0,0 +1,15 @@ +THIRD-PARTY NOTICE for profiles.json +==================================== + +`profiles.json` in this directory contains volatile, RAM-only optical-drive +microcode ("LibreDrive") authored and owned by the MakeMKV team (GuinpinSoft +inc.). Each payload embeds its own notice: + + "This is volatile RAM-only microcode for LibreDrive by MakeMKV - + http://www.makemkv.com/libredrive/ + Copyright (c) MakeMKV team, all rights reserved." + +This microcode is third-party proprietary material — Copyright (c) MakeMKV team, +all rights reserved — and is NOT covered by this repository's MIT LICENSE. No +ownership is claimed. See the repository-root NOTICE. Thanks to the MakeMKV team +for LibreDrive: https://www.makemkv.com/ , http://www.makemkv.com/libredrive/ . diff --git a/src/ld/README.md b/src/ld/README.md index afbcc52..b6ddfb8 100644 --- a/src/ld/README.md +++ b/src/ld/README.md @@ -19,3 +19,13 @@ This module performs only the **non-persistent** unlock — the access state it sets up lives in drive RAM and is gone on power cycle. The one-time, permanent drive preparation is the drive owner's own manual step and is **never** automated here. + +## Third-party microcode (`profiles.json`) + +`profiles.json` bundles volatile, RAM-only optical-drive microcode +("LibreDrive") authored and owned by the **MakeMKV team** (GuinpinSoft inc.). +Each payload carries its own notice — *"Copyright (c) MakeMKV team, all rights +reserved."* This microcode is **third-party proprietary material and is NOT +covered by this repository's MIT license**; no ownership is claimed. See the +[`NOTICE`](./NOTICE) in this directory and the repository-root `NOTICE`. With +thanks to the MakeMKV team for [LibreDrive](http://www.makemkv.com/libredrive/). diff --git a/src/ld/mod.rs b/src/ld/mod.rs index 40553f5..eafa519 100644 --- a/src/ld/mod.rs +++ b/src/ld/mod.rs @@ -6,13 +6,9 @@ //! encryption AT THE DRIVE (the unlocked drive serves clear content) and //! reporting the OEM Volume ID. -// `cdb` carries ONLY the unlock-handshake wire format that the bdemu emulator -// needs (the real unlocker drives its CDBs from per-drive profile templates, not -// these constants). Compile it only when the `emulation` feature exposes it, so -// it never dead-codes in a normal build. -// Compiled under `cfg(test)` as well as the feature: its tests pin the unlock -// wire format, and gating the whole module on a non-default feature meant CI -// (which builds with default features) never ran them. +// `cdb` is only the bdemu emulator's wire format (real unlocking uses profile +// templates), gated behind `emulation`; also compiled under `cfg(test)` so its +// wire-format tests run in default-feature CI. See docs/ld-mod.md — cdb gating. #[cfg(any(feature = "emulation", test))] mod cdb; mod error; @@ -24,12 +20,8 @@ use crate::scsi::{DataDirection, ScsiTransport}; use crate::{DriveId, UnlockCtx, UnlockError, Unlocked, Unlocker}; // ── Public profile catalog ────────────────────────────────────────────────── -// -// The catalog of drives the MT1959 unlocker recognizes is the one piece of -// ld worth exposing publicly: it answers "is this drive supported?" without -// unlocking, and the bdemu test-emulator reads it to impersonate a supported -// drive. The unlock *mechanism* (firmware blobs, upload sequence, CDB wire -// format) stays private — only the catalog and its match result are public. +// Only the catalog is public (supported-drive lookup, used by bdemu); the +// unlock mechanism stays private. See docs/ld-mod.md — public catalog design. pub use profile::{DriveProfile as Profile, Identity, Platform, ProfileMatch, Profiles}; @@ -54,12 +46,9 @@ pub fn profile(drive_id: &DriveId) -> Option { #[cfg(feature = "emulation")] pub use cdb::{UNLOCK_MARKER, is_unlock_read_buffer}; -/// The MT1959 unlocker. `pub(crate)` — clients reach it only through -/// [`crate::all_unlockers`], never by name (the locked-design contract). -/// -/// Matches a drive against the bundled profile database and, on a hit, -/// runs the MediaTek MT1959 firmware-unlock (and disc-speed calibration) -/// handshake over the raw SCSI transport. +// The MT1959 unlocker, reached only through `crate::all_unlockers` (never by +// name). Matches a drive against the bundled profile database and runs the +// firmware-unlock + disc-speed-calibration handshake over raw SCSI. pub(crate) struct Mt1959Unlocker; impl Mt1959Unlocker { @@ -76,17 +65,9 @@ pub(crate) fn firmware_name(id: &DriveId) -> Option<&'static str> { } impl Mt1959Unlocker { - /// Read the OEM Volume ID via the matched profile's vendor CDB. - /// - /// Takes the profile the caller ALREADY matched rather than re-running - /// `find_bundled`: the catalog is 206 entries and drive-prep used to scan it - /// twice per drive (once here, once in `firmware_unlock`) for the same - /// answer. - /// - /// `Ok(Some(vid))` on a well-formed 36-byte response (signature `00 22 00`, - /// VID at `[4..20]`); `Ok(None)` when there is no OEM-VID CDB or the response - /// is short / bad-signature / drive-rejected (the drive is still unlocked, - /// just no VID); `Err` only on a transport fault. + // Read the OEM Volume ID via the matched profile's vendor CDB (profile passed + // in to avoid a redundant 206-entry catalog scan). Ok(Some) on a well-formed + // response; Ok(None) if unreadable; Err only on a transport fault. fn read_oem_vid( &self, scsi: &mut dyn ScsiTransport, @@ -101,10 +82,9 @@ impl Mt1959Unlocker { let mut buf = vec![0u8; RESPONSE_LEN]; let result = scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000)?; - // Per the transport contract a drive sense arrives as `Ok` with a - // non-zero `status`, NOT as `Err`. Without this check a CHECK CONDITION - // would be read as a successful 36-byte response and the caller's own - // zero fill parsed as a Volume ID. + // Per the transport contract a drive sense arrives as `Ok` with non-zero + // `status`, not `Err` — without this check a CHECK CONDITION reads as a + // successful response and the zero-filled buffer parses as a bogus VID. if result.status != 0 { tracing::warn!( target: "freemkv::disc", @@ -139,15 +119,9 @@ impl Mt1959Unlocker { } impl Mt1959Unlocker { - /// The MediaTek firmware unlock. Because the MT1959 unlocker removes AACS bus - /// encryption AT THE DRIVE (the unlocked drive serves CLEAR content), this ONE - /// operation satisfies BOTH the drive-features and the bus-removal capability - /// — so `unlock_features` and `unlock_bus` both delegate here. The result - /// carries `drive_unlocked: true` (no bus key needed) and the OEM Volume ID. - /// - /// A no-firmware-route drive (Renesas / no profile) returns `NotApplicable`; a - /// transport fault propagates as `Transport`; any other firmware failure also - /// falls through as `NotApplicable`. + // The MediaTek firmware unlock. Since it removes AACS at the drive (clear + // content), this one op satisfies both features and bus-removal, so both + // trait methods delegate here. See docs/ld-mod.md — firmware_unlock contract. fn firmware_unlock( &self, scsi: &mut dyn ScsiTransport, @@ -167,14 +141,9 @@ impl Mt1959Unlocker { // A transport fault → UnlockError::Transport; any other firmware failure // → NotApplicable (via From). mt.init(scsi)?; - // `init` only proves the handshake COMPLETED — `do_unlock` returns Ok on - // a response that carried the per-drive signature but not both firmware - // markers. Only `is_unlocked()` means the drive actually reached the - // extended-access state and serves clear content. Reporting - // `drive_unlocked: true` off `init` alone told the consumer the bus was - // clear on a half-unlocked drive, which suppressed the cert-auth - // fallback and shipped ciphertext at rc=0. A partial unlock must fall - // through to the next unlocker. + // `init` only proves the handshake completed, not that the drive reached + // extended-access state. Reporting unlocked off `init` alone shipped + // ciphertext at rc=0. See docs/ld-mod.md — half-unlock fallback. if !mt.is_unlocked() { tracing::warn!( target: "freemkv::disc", @@ -183,19 +152,13 @@ impl Mt1959Unlocker { ); return Err(UnlockError::NotApplicable); } - // Prime the per-region speed table. Best-effort — it must not fail the - // unlock — but NOT silent: speed calibration can fail completely, and an - // unlogged `let _ =` made a fully-failed calibration indistinguishable - // from a successful one in the rip log. + // Prime the per-region speed table. Best-effort (must not fail the unlock) + // but not silent — an unlogged `let _ =` made a failed calibration + // indistinguishable from success in the rip log. if let Err(e) = mt.probe_disc(scsi) { - // A transport fault here is a DEAD BUS, not a speed-calibration miss. - // The rest of this path (`read_oem_vid`) is a no-op for the 140/206 - // profiles that carry no `read_vid_cdb`, so it never touches the bus - // again — meaning this swallowed fault was the ONLY dead-bus signal, - // and warn-and-continue turned it into `Ok(Unlocked{drive_unlocked: - // true})`: a dead bus rendered as a fully-unlocked drive (the flagship - // failure-that-looks-like-success). A genuine calibration miss (drive - // sense / short reply) still continues on the default speed table. + // A transport fault here is a dead bus, not a calibration miss — most + // profiles never touch the bus again, so this was the only dead-bus + // signal. See docs/ld-mod.md — probe_disc dead-bus classification. if e.is_transport_failure() { tracing::warn!( target: "freemkv::disc", @@ -423,12 +386,9 @@ mod tests { assert_eq!(err, UnlockError::NotApplicable); } - /// THE defect-1 test. The drive answers every unlock READ_BUFFER with a - /// response carrying the per-drive signature and the primary firmware marker - /// but NOT the secondary one — `do_unlock` returns `Ok` and `init()` - /// succeeds, yet the drive is NOT in the extended-access state. Reporting - /// `drive_unlocked: true` here suppresses the cert-auth fallback. Catches - /// removing the `is_unlocked()` gate in `firmware_unlock`. + // THE defect-1 test: response carries the signature + primary marker but not + // the secondary one, so init() succeeds but the drive isn't in extended-access + // state. Catches removing the `is_unlocked()` gate in `firmware_unlock`. #[test] fn half_unlocked_drive_falls_through_instead_of_claiming_unlocked() { use crate::scsi::mock::{MockTransport, Reply}; @@ -475,16 +435,9 @@ mod tests { assert!(u.drive_unlocked); } - /// THE probe-disc dead-bus test. The drive unlocks fully (both firmware - /// markers), then the bus DIES during disc-speed calibration. `probe_disc` - /// used to be warn-and-continued regardless of the fault, and — because the - /// 140/206 profiles without a `read_vid_cdb` never touch the bus again — a - /// dead bus was reported as `Ok(Unlocked{drive_unlocked:true})`: a dead bus - /// rendered as a successful unlock (the flagship failure-that-looks-like- - /// success). `firmware_unlock` must now abort with `Transport`. - /// MUTATION: reverting the `if e.is_transport_failure()` return in - /// `firmware_unlock` (warn-and-continue), OR flattening the probe loops' - /// transport classification, makes this go red. + // THE probe-disc dead-bus test: bus dies during speed calibration after a + // full unlock; must abort with Transport, not report a successful unlock. + // See docs/ld-mod.md — probe_disc dead-bus test / mutation notes. #[test] fn transport_fault_during_probe_disc_is_transport_not_a_successful_unlock() { let id = known_vid_drive_id(); @@ -534,11 +487,9 @@ mod tests { assert_eq!(err, UnlockError::Transport); } - /// A drive-sense (not a dead bus) rejecting the speed probe is a genuine - /// calibration miss, not a transport fault — `firmware_unlock` must - /// warn-and-continue on the drive's default speed table, still reporting a - /// full unlock. The non-transport sibling of - /// `transport_fault_during_probe_disc_is_transport_not_a_successful_unlock`. + // A drive-sense (not a dead bus) rejecting the speed probe is a genuine + // calibration miss: warn-and-continue on the default speed table, still + // reporting a full unlock. Non-transport sibling of the probe-fault test. #[test] fn drive_sense_during_probe_disc_still_reports_a_successful_unlock() { let id = known_vid_drive_id(); diff --git a/src/ld/platform/mod.rs b/src/ld/platform/mod.rs index adff12c..1084d3a 100644 --- a/src/ld/platform/mod.rs +++ b/src/ld/platform/mod.rs @@ -31,10 +31,8 @@ mod tests { use super::*; use crate::scsi::mock::{MockTransport, Reply}; - /// A minimal driver that takes every `PlatformDriver` default (`is_unlocked`) - /// so the shim's dispatch and the default's `false` are both exercised — - /// every real driver (`Mt1959`) overrides `is_unlocked`, so without this the - /// default body is dead in coverage even though it is reachable code. + // Minimal driver taking the `PlatformDriver` default `is_unlocked`, so + // the default's `false` path is exercised (real drivers override it). struct StubDriver; impl PlatformDriver for StubDriver { diff --git a/src/ld/platform/mt1959/mod.rs b/src/ld/platform/mt1959/mod.rs index 791b4c1..a867b88 100644 --- a/src/ld/platform/mt1959/mod.rs +++ b/src/ld/platform/mt1959/mod.rs @@ -117,21 +117,17 @@ impl Mt1959 { expected: usize, ) -> Result { // The READ_BUFFER CDB transfer-length is a single byte; an - // `expected` above 255 cannot be expressed and would silently - // truncate. All in-crate callers pass small fixed sizes (4); guard - // the invariant rather than emit a malformed CDB. + // `expected` above 255 would silently truncate. Guard the + // invariant rather than emit a malformed CDB. debug_assert!( expected <= u8::MAX as usize, "read_buffer_probe expected exceeds 1-byte CDB length field" ); let cdb = self.read_buffer_sub(sub_cmd, address, expected as u8); let result = scsi.execute(&cdb, DataDirection::FromDevice, buf, 5_000)?; - // A drive sense arrives as `Ok` with a non-zero status (the transport - // contract), and a merely SHORT response is the drive answering badly — - // neither is a dead bus. Fabricating a transport-failure status here - // turned a short probe reply into a hard abort of a rip that would - // otherwise have succeeded; a real transport fault still propagates - // through the `?` above with its own status. + // A drive sense arrives as `Ok` with a non-zero status; a merely + // SHORT response is the drive answering badly — neither is a dead + // bus. See docs/mt1959.md — short-probe-not-transport-failure. if result.status != 0 || result.bytes_transferred != expected { return Err(Error::Scsi { opcode: SCSI_READ_BUFFER, @@ -183,13 +179,9 @@ impl Mt1959 { }); } - // `response` is a fixed 64-byte buffer, so `response.len()` is - // always >= every offset below — the meaningful bound is how many - // bytes the drive actually delivered. Every marker check MUST be - // reachable: when the checks were each guarded by their own `n >= ..` - // the drive delivering ZERO bytes skipped all three and `do_unlock` - // returned `Ok` on a buffer the drive never wrote. Require enough bytes - // to actually validate, up front. + // `response` is a fixed 64-byte buffer; the meaningful bound is how + // many bytes the drive actually delivered. Require enough bytes to + // validate before checking markers, up front. See docs/mt1959.md — min-unlock-response-length. let n = result.bytes_transferred.min(response.len()); if n < MIN_UNLOCK_RESPONSE { tracing::debug!( @@ -212,15 +204,9 @@ impl Mt1959 { return Err(Error::UnlockFailed); } - // Extended-access state is active when BOTH the per-drive - // signature matched AND the response carries the secondary - // marker at offset 16 (repeated through bytes 16..64) AND the - // primary mode marker at [12..16] is present. The active-mode - // marker at [12..16] is the primary gate; the [16..20] marker - // is the redundant confirmation the firmware writes through - // the rest of the response. Requiring both before we tell the - // upper layer "OEM path is live" keeps any partial / corrupted - // response from steering us off the cert-auth fallback. + // Extended-access state requires the signature match AND both the + // primary marker at [12..16] AND the secondary marker at [16..20]. + // See docs/mt1959.md — unlock-marker-gating. self.unlocked = response[FIRMWARE_MODE_OFFSET..FIRMWARE_MODE_OFFSET + 4] == FIRMWARE_MODE_SIG; @@ -295,11 +281,8 @@ impl Mt1959 { ); return Err(Error::UnlockFailed); } - // A transport fault means the bus is gone. Re-uploading firmware - // into a dead bus three times accomplishes nothing and the - // generic UnlockFailed it used to end on is classified as - // "unlocker not applicable", so the consumer kept probing a dead - // drive instead of aborting. Surface the fault unchanged. + // A dead bus must abort here, not be retried three times as + // firmware reloads. See docs/mt1959.md — run-init-transport-fault. Err(e) if e.is_transport_failure() => { tracing::warn!( target: "freemkv::disc", @@ -365,10 +348,8 @@ impl Mt1959 { self.do_unlock(scsi)?; } - // Detect disc type from capacity to select probe mode. - // BD: 3C 01 44 12 01 00 00 00 04 00 (init_addr = 0x0100) - // UHD: 3C 01 44 12 02 00 00 00 04 00 (init_addr = 0x0200) - // Empirically verified via SCSI capture: BD and UHD use different init addresses. + // Detect disc type from capacity to select probe mode (BD vs UHD + // init address). See docs/mt1959.md — probe-init-address-detection. let cap_cdb = [ SCSI_READ_CAPACITY, 0x00, @@ -382,16 +363,12 @@ impl Mt1959 { 0x00, ]; let mut cap_buf = [0u8; READ_CAPACITY_RESPONSE_SIZE]; - // A transport fault here is a dead bus: propagate it (the caller - // classifies it as `Transport` and aborts) instead of silently - // defaulting the disc type. A drive sense (`Ok`, non-zero status) or a - // short reply just means "capacity unknown" -> assume BD. + // A transport fault here is a dead bus: propagate it. A drive + // sense or short reply just means "capacity unknown" -> assume BD. let cap = scsi.execute(&cap_cdb, DataDirection::FromDevice, &mut cap_buf, 5_000)?; let disc_sectors = if cap.status == 0 && cap.bytes_transferred >= 4 { - // last_lba + 1 = sector count. A 0xFFFFFFFF last-LBA is the - // READ CAPACITY(10) "capacity exceeds 32 bits" sentinel; saturate - // rather than wrap to 0 (which would misclassify a huge disc as - // BD). A saturated count stays above the UHD threshold -> UHD. + // last_lba + 1 = sector count; saturate on the 32-bit-overflow + // sentinel rather than wrap to 0. See docs/mt1959.md — capacity-saturation. u32::from_be_bytes([cap_buf[0], cap_buf[1], cap_buf[2], cap_buf[3]]).saturating_add(1) } else { 0 @@ -434,14 +411,9 @@ impl Mt1959 { ); }) { - // A dead bus MUST keep its transport classification: this loop is - // reached from `probe_disc`, whose caller aborts the rip only when - // it sees `Transport`. Flattening every failure to `UnlockFailed` - // (which classifies as "not applicable") let a bus that died mid - // speed-probe fall through to `Ok(Unlocked{drive_unlocked:true})`. - // A short / rejected probe reply is still the drive's OWN failure — - // keep it as `UnlockFailed` so it doesn't hard-abort a rip that - // could continue on the default speed table. + // A dead bus MUST keep its transport classification; a + // short/rejected reply is the drive's own failure and stays + // `UnlockFailed`. See docs/mt1959.md — probe-failure-classification. if e.is_transport_failure() { return Err(e); } @@ -450,10 +422,8 @@ impl Mt1959 { addr = addr.wrapping_add(PROBE_STEP); } - // Pass 2: continue past the coarse range. It used to restart at 0 with - // the SAME PROBE_STEP, re-issuing all 88 pass-1 addresses verbatim — 88 - // redundant SCSI round-trips per disc, which dominates drive prep at - // real bus latency and teaches the drive's speed table nothing new. + // Pass 2: continue past the coarse range (do not restart at 0). + // See docs/mt1959.md — probe-pass-overlap-fix. let mut addr: u32 = PROBE_COARSE_END as u32; while addr < PROBE_FINE_END { let mut resp = [0u8; PROBE_RESPONSE_SIZE as usize]; @@ -464,12 +434,9 @@ impl Mt1959 { &mut resp, PROBE_RESPONSE_SIZE as usize, ) { - // Fine probing is best-effort — a short / rejected reply just ends - // the sweep early. But a bus that DIED here must not be swallowed: - // a bare `break` returned `Ok(())` from `run_probe`, so a dead bus - // mid-pass-2 vanished into `Ok(Unlocked{drive_unlocked:true})` with - // no log at all (the flagship failure-that-looks-like-success). A - // transport fault keeps its classification and aborts. + // Fine probing is best-effort — a short/rejected reply just ends + // the sweep early, but a transport fault must keep its + // classification and abort. See docs/mt1959.md — probe-pass2-transport-fault. if e.is_transport_failure() { tracing::warn!( target: "freemkv::disc", @@ -619,10 +586,9 @@ mod tests { assert_eq!(len, 2208, "MODE SELECT length = firmware.len(), not 2496"); } - /// variant_a had no direct test at all (variant_b did). Pins its upload - /// sequence: WRITE_BUFFER carrying the firmware at its real per-drive - /// length in the CDB's 24-bit length field, then the 0x45 verify READ_BUFFER - /// whose result used to be discarded outright. + // Pins variant_a's upload sequence: WRITE_BUFFER at the real per-drive + // length in the CDB's 24-bit length field, then the 0x45 verify + // READ_BUFFER whose result used to be discarded outright. #[test] fn variant_a_uploads_at_the_real_length_and_issues_the_verify_read() { let mut profile = fixture_profile([0x11, 0x22, 0x33, 0x44]); @@ -660,12 +626,9 @@ mod tests { assert!(t.cdbs.is_empty(), "no CDB may reach the drive"); } - /// THE defect-15 test. A drive that returns ZERO bytes used to skip the - /// signature check, the firmware-active check AND the mode check — every - /// gate was individually guarded by its own `n >= ..` — so `do_unlock` - /// returned `Ok(vec![0u8; 64])` and (with the old `drive_unlocked: true`) - /// reported a fully unlocked drive. Catches restoring those per-check - /// guards. + // THE defect-15 test: a zero-byte response used to skip every marker + // check (each individually `n >= ..` guarded) and report a fully + // unlocked drive. See docs/mt1959.md — defect-15. #[test] fn do_unlock_rejects_a_zero_length_response() { let sig = [0x99, 0x9E, 0xC3, 0x75]; @@ -706,10 +669,9 @@ mod tests { assert!(!e.is_transport_failure(), "a sense is not a dead bus"); } - /// THE defect-16 test (the inverse misclassification). A merely SHORT probe - /// response used to be fabricated into a transport-failure status, which - /// hard-aborts a rip that would otherwise have succeeded. Catches - /// reintroducing the fabricated 0xFF. + // THE defect-16 test: a merely short probe response used to be + // fabricated into a transport-failure status, hard-aborting a rip + // that would otherwise have succeeded. #[test] fn short_probe_response_is_not_a_transport_failure() { let mut t = crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::short( @@ -744,11 +706,9 @@ mod tests { assert!(e.is_transport_failure()); } - /// THE defect-4 test. A dead bus must abort `run_init` at once. It used to - /// match `Err(_)` and re-upload firmware into the dead bus on all three - /// attempts, then return the generic `UnlockFailed`, which classifies as - /// `NotApplicable` — so the consumer kept probing. Catches removing the - /// `is_transport_failure` arm. + // THE defect-4 test: a dead bus must abort `run_init` at once, not + // re-upload firmware on all three attempts and return the generic + // `UnlockFailed`. See docs/mt1959.md — defect-4. #[test] fn run_init_aborts_on_a_transport_fault_without_reloading_firmware() { let mut t = @@ -823,11 +783,9 @@ mod tests { } } - /// A drive SENSE (not a dead bus) during the coarse speed probe is the - /// drive's OWN failure, not a transport fault — `run_probe` must report - /// `UnlockFailed`, not `Transport`. The sibling of - /// `transport_fault_during_pass1_probe_stays_a_transport_failure`, which - /// only exercises the `is_transport_failure()` arm. + // A drive SENSE (not a dead bus) during the coarse probe is the + // drive's own failure — `run_probe` must report `UnlockFailed`, not + // `Transport`. Sibling of the pass-1 transport-fault test below. #[test] fn drive_sense_during_pass1_probe_is_unlock_failed_not_transport() { struct SenseAtFirstProbe; @@ -866,12 +824,9 @@ mod tests { assert!(matches!(e, Error::UnlockFailed)); } - /// THE probe pass-1 dead-bus test. A bus that dies during the coarse speed - /// probe must keep its transport classification out of `run_probe` so the - /// caller (`firmware_unlock`) aborts with `Transport` rather than reporting a - /// fully-unlocked drive. - /// MUTATION: flattening the pass-1 error to `Error::UnlockFailed` makes the - /// `is_transport_failure` assert go red (UnlockFailed maps to NotApplicable). + // THE probe pass-1 dead-bus test: a bus that dies during the coarse + // probe must keep its transport classification so the caller aborts + // with `Transport`. See docs/mt1959.md — pass1-dead-bus-mutation. #[test] fn transport_fault_during_pass1_probe_stays_a_transport_failure() { let mut t = ProbeFaultsTransport { fault_at: 0 }; @@ -884,12 +839,9 @@ mod tests { assert_eq!(crate::UnlockError::from(e), crate::UnlockError::Transport); } - /// THE probe pass-2 dead-bus test. Pass 1 completes; the bus dies in the fine - /// probe. The loop used to `break` on ANY error and return `Ok(())`, so a - /// dead bus mid-pass-2 vanished into `Ok`. It must now propagate a transport - /// fault (a short/rejected fine probe still ends the sweep benignly). - /// MUTATION: restoring the bare `break` makes `expect_err` panic (run_probe - /// returns Ok). + // THE probe pass-2 dead-bus test: the loop used to `break` on ANY + // error and return `Ok(())`, swallowing a dead bus mid-pass-2. It + // must now propagate. See docs/mt1959.md — pass2-dead-bus-mutation. #[test] fn transport_fault_during_pass2_probe_stays_a_transport_failure() { let mut t = ProbeFaultsTransport { @@ -904,10 +856,8 @@ mod tests { assert_eq!(crate::UnlockError::from(e), crate::UnlockError::Transport); } - /// `validate`'s transport-abort branch had no test. A dead bus must propagate - /// as a transport fault on the FIRST attempt, not be retried five times. - /// MUTATION: turning the `Err(e) => return Err(Error::from(e))` arm into a - /// `continue` makes this exhaust to `UnlockFailed` and the assert go red. + // `validate`'s transport-abort branch: a dead bus must propagate on + // the first attempt, not be retried five times. #[test] fn validate_propagates_a_transport_fault_without_retrying() { let mut t = @@ -932,13 +882,9 @@ mod tests { assert_eq!(t.calls(), 5, "retries the full five attempts"); } - /// variant_a's firmware-verify-read transport-abort branch (~variant_a.rs:97) - /// had no test. WRITE_BUFFER succeeds, the 0x45 verify read faults: a dead bus - /// must abort THERE, not be swallowed into the unlock retries. - /// MUTATION: dropping the `if err.is_transport_failure() { return Err }` lets - /// execution fall through to `do_unlock` (scripted to answer, so it fails on - /// the signature as `UnlockFailed`), and the `is_transport_failure` assert - /// goes red — a genuine red-before-green, not green-both-ways. + // variant_a's firmware-verify-read transport-abort branch: WRITE_BUFFER + // succeeds, the 0x45 verify read faults — a dead bus must abort there, + // not be swallowed into the unlock retries. See docs/mt1959.md — variant-a-verify-read-transport-abort. #[test] fn variant_a_verify_read_transport_fault_aborts() { let mut profile = fixture_profile([0; 4]); @@ -974,14 +920,9 @@ mod tests { assert!(t.cdbs.is_empty(), "no CDB may reach the drive"); } - /// THE full success path: WRITE_BUFFER upload, a fully-good verify read - /// (exercising the `Ok(r) if status==0 && bytes_transferred==len` arm no - /// other variant_a test reaches), then BOTH `do_unlock` calls succeed — - /// the first fatally-gated, the second best-effort. Every other - /// variant_a test above deliberately stops short (empty firmware, a - /// dead bus, or `do_unlock` failing against `RecordingTransport`'s empty - /// response) — this is the only one that reaches `load_firmware`'s final - /// `Ok(())`. + // THE full success path: WRITE_BUFFER upload, a fully-good verify + // read, then both `do_unlock` calls succeed. The only variant_a test + // that reaches `load_firmware`'s final `Ok(())`. #[test] fn variant_a_load_firmware_succeeds_end_to_end() { let sig = [0x11, 0x22, 0x33, 0x44]; @@ -1002,12 +943,9 @@ mod tests { assert!(mt.init_complete, "do_unlock #1 must have set init_complete"); } - /// variant_b had no transport-abort test. A firmware-upload STEP that hits a - /// dead bus (here the metadata read) must abort via `trace_step`, not be - /// swallowed and carried into the unlock retries. - /// MUTATION: dropping `trace_step`'s `if err.is_transport_failure()` return - /// lets execution reach `do_unlock` (scripted good → signature fails as - /// `UnlockFailed`), so the `is_transport_failure` assert goes red. + // variant_b's firmware-upload step (here the metadata read) hitting a + // dead bus must abort via `trace_step`, not be swallowed into the + // unlock retries. See docs/mt1959.md — variant-b-upload-step-transport-abort. #[test] fn variant_b_upload_step_transport_fault_aborts() { let mut profile = fixture_profile([0; 4]); @@ -1056,12 +994,9 @@ mod tests { } } - /// Build a synthetic 64-byte unlock response. - /// - /// `mode_marker`: bytes [12..16]. Pass `FIRMWARE_ACTIVE_SIG` for the - /// active-mode primary marker. - /// `id_marker`: bytes [16..20] (and repeated through [20..64] in - /// real responses; only [16..20] is checked). + // Build a synthetic 64-byte unlock response. `mode_marker` fills + // bytes [12..16]; `id_marker` fills bytes [16..20] (repeated through + // [20..64] in real responses; only [16..20] is checked). fn build_response(signature: [u8; 4], mode_marker: [u8; 4], id_marker: [u8; 4]) -> Vec { let mut r = vec![0u8; 64]; r[0..4].copy_from_slice(&signature); @@ -1144,11 +1079,9 @@ mod tests { assert_eq!(t.calls(), 1, "must not retry a signature mismatch"); } - /// `run_init`'s retry loop: `do_unlock` fails generically (too short to - /// validate — not a signature mismatch, not a transport fault), the - /// firmware reload it falls back to also fails generically (an empty - /// firmware blob) rather than fatally, so the loop must `continue` and - /// try again — three times — before exhausting to the last error. + // `run_init`'s retry loop: a generic `do_unlock` failure plus a + // generic firmware-reload failure must `continue` and retry three + // times before exhausting to the last error. #[test] fn run_init_exhausts_after_three_generic_failures() { let mut t = crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::short( @@ -1168,11 +1101,9 @@ mod tests { ); } - /// Same shape as `run_init_exhausts_after_three_generic_failures`, but for - /// a MODE_B (`is_variant_b = true`) drive — pins that `run_init`'s mode - /// dispatch (`self.mode == MODE_A`) actually reaches `variant_b::load_firmware` - /// and not just its `variant_a` sibling, which every other `run_init` test - /// above exercises exclusively. + // Same shape as the mode-A generic-failure test, but for a MODE_B + // drive — pins that `run_init`'s dispatch actually reaches + // `variant_b::load_firmware`, not just its variant_a sibling. #[test] fn run_init_exhausts_after_three_generic_failures_mode_b() { let mut t = crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::short( @@ -1190,10 +1121,9 @@ mod tests { ); } - /// Same generic `do_unlock` failure, but this time the firmware-reload - /// fallback itself hits a dead bus. That must abort `run_init` - /// immediately (transport classification preserved), not be folded into - /// `last_err` and retried. + // Same generic `do_unlock` failure, but the firmware-reload fallback + // itself hits a dead bus — must abort `run_init` immediately, not be + // folded into `last_err` and retried. #[test] fn run_init_firmware_reload_transport_fault_aborts() { let mut profile = fixture_profile([0; 4]); @@ -1232,10 +1162,9 @@ mod tests { assert!(mt.init_complete, "run_probe must do_unlock itself first"); } - /// A READ CAPACITY drive sense (status != 0) means "capacity unknown" -> - /// `disc_sectors` falls back to 0, which is still below the UHD - /// threshold, so probing continues with the BD init address rather than - /// aborting. + // A READ CAPACITY drive sense means "capacity unknown" -> falls back + // to 0 sectors, which is below the UHD threshold, so probing + // continues with the BD init address rather than aborting. #[test] fn run_probe_capacity_check_condition_falls_back_to_zero_sectors() { struct CapacitySenseTransport; @@ -1327,10 +1256,9 @@ mod tests { ); } - /// The fine (pass-2) probe sweep ends early on a merely-rejected probe - /// reply (not a transport fault) — it must `break` out of the sweep and - /// let `run_probe` still return `Ok`, rather than propagating the drive's - /// refusal as a hard failure. + // The fine (pass-2) probe sweep ends early on a merely-rejected + // probe reply (not a transport fault) — it must `break` and let + // `run_probe` still return `Ok`. #[test] fn run_probe_pass2_ends_early_on_a_rejected_probe_without_failing() { struct Pass2RejectsTransport { diff --git a/src/ld/platform/mt1959/variant_a.rs b/src/ld/platform/mt1959/variant_a.rs index 44eeaf7..9abefcd 100644 --- a/src/ld/platform/mt1959/variant_a.rs +++ b/src/ld/platform/mt1959/variant_a.rs @@ -21,9 +21,8 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re } // Upload firmware via WRITE_BUFFER. The CDB's length is a 24-bit field; - // if the blob exceeds that, the encoded length would silently disagree - // with the bytes actually sent (`data`). Reject rather than upload a - // length-mismatched command. + // reject blobs that would exceed it rather than send a length-mismatched + // command. let len = firmware.len(); if len > WRITE_BUFFER_MAX_LEN { return Err(crate::ld::error::Error::UnlockFailed); @@ -43,12 +42,9 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re let mut data = firmware.clone(); scsi.execute(&cdb, DataDirection::ToDevice, &mut data, 30_000)?; - // Verify the firmware loaded (non-fatal — different buffer_id 0x45). There - // is no documented expected payload to compare against, so the outcome is - // TRACED rather than asserted: discarding it with `let _ =` meant a corrupt - // upload proceeded straight into the do_unlock retries with no diagnostic - // whatsoever, which is exactly the silent-failure shape this crate exists to - // avoid. + // Verify the firmware loaded (non-fatal — different buffer_id 0x45). No + // documented expected payload exists, so the outcome is traced rather + // than asserted, instead of silently discarded before the unlock retries. let verify_cdb = [ super::SCSI_READ_BUFFER, super::MODE_A, @@ -100,10 +96,9 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re } } - // Double unlock after firmware upload. The first establishes the - // unlock and is fatal on failure; the second is a confirmation pass and - // is best-effort (matching variant B), so a benign hiccup on the - // redundant call doesn't fail an already-successful unlock. + // Double unlock after firmware upload: first is fatal on failure, second + // is a best-effort confirmation pass (matching variant B) so a benign + // hiccup there doesn't fail an already-successful unlock. mt.do_unlock(scsi)?; let _ = mt.do_unlock(scsi); Ok(()) diff --git a/src/ld/platform/mt1959/variant_b.rs b/src/ld/platform/mt1959/variant_b.rs index 7c21daf..bb36c1d 100644 --- a/src/ld/platform/mt1959/variant_b.rs +++ b/src/ld/platform/mt1959/variant_b.rs @@ -13,11 +13,9 @@ const FIRMWARE_EXTRA: [u8; 16] = [0; 16]; /// that drive — real profiles must supply their own (see `DriveProfile`). const VENDOR_VERIFY: [u8; 10] = [0xF1, 0x01, 0x02, 0x00, 0x0D, 0x30, 0x01, 0xF3, 0xAD, 0x23]; -/// Trace the outcome of a best-effort firmware-upload step and swallow -/// everything EXCEPT a transport fault. These steps are advisory (the unlock -/// retries below are the real gate) but their results were previously thrown -/// away entirely, so a firmware upload could fail end to end and leave nothing -/// in the log to say so — and a dead bus kept the sequence running. +// See docs/variant-b-firmware-upload.md — trace_step rationale. +// Swallows everything except a transport fault; these steps are advisory, +// the unlock retries below are the real gate. fn trace_step(phase: &'static str, r: crate::scsi::Result) -> Result<()> { match r { Ok(res) if res.status == 0 => { @@ -60,12 +58,9 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re return Err(crate::ld::error::Error::UnlockFailed); } - // Step 1: Upload the firmware via MODE SELECT. The profile's `firmware` is - // the exact per-drive image — extracted at the drive's own load-CDB length - // (2192..2528 bytes; the old fixed 0x9C0 truncated some drives and over-read - // others into blob strings). Upload all of it. MODE SELECT(10)'s - // parameter-list length is 16-bit, so reject only a blob that can't be - // expressed in the CDB. + // Step 1: Upload the firmware via MODE SELECT (see docs/variant-b-firmware-upload.md). + // MODE SELECT(10)'s parameter-list length is 16-bit, so reject only a + // blob that can't be expressed in the CDB. let write_len = firmware.len(); if write_len > u16::MAX as usize { return Err(crate::ld::error::Error::UnlockFailed); @@ -128,13 +123,9 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re scsi.execute(&write_extra_cdb, DataDirection::ToDevice, &mut data2, 5_000), )?; - // Step 4: Vendor verify (0xF1 — B-only, not standard SCSI). PER-DRIVE: take - // it from the profile (39 distinct values across the 140 B drives). The - // const is only a legacy fallback — it carries one drive's token. - // The result used to be discarded with `let _ =` despite the comment calling - // this a verify step: a corrupt upload proceeded straight to the do_unlock - // retries with no diagnostic. There is no documented expected payload, so - // the outcome is TRACED; only a dead bus aborts. + // Step 4: Vendor verify (0xF1 — B-only, not standard SCSI), per-drive. + // See docs/variant-b-firmware-upload.md for why the outcome is traced + // rather than discarded. let verify_cdb = mt.profile.fw_verify_cdb.unwrap_or(VENDOR_VERIFY); let mut dummy = [0u8; 0]; trace_step( @@ -142,11 +133,9 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re scsi.execute(&verify_cdb, DataDirection::None, &mut dummy, 5_000), )?; - // Step 5: Unlock retries (up to 5, then a final fatal attempt). On a - // successful unlock we issue one confirmation pass; its result is - // intentionally best-effort — the first call already established the - // unlock state, so a hiccup on the redundant confirmation must not fail - // an otherwise-good unlock. + // Step 5: Unlock retries (up to 5, then a final fatal attempt). The + // confirmation pass after success is intentionally best-effort — see + // docs/variant-b-firmware-upload.md. for _attempt in 0..5 { match mt.do_unlock(scsi) { Ok(_) => { diff --git a/src/ld/profile.rs b/src/ld/profile.rs index b645dff..1dbb2d5 100644 --- a/src/ld/profile.rs +++ b/src/ld/profile.rs @@ -42,20 +42,10 @@ pub struct Identity { /// Per-drive profile. /// -/// VISIBILITY: only `identity` and `signature` are public — they are what the -/// catalog is public FOR ("is this drive supported?", and the emulator's -/// impersonation). Everything below them is unlock MECHANISM: the MediaTek -/// firmware image and the per-drive vendor CDB templates. `Cargo.toml` says this -/// crate is never published because it carries drive firmware; re-exporting -/// `DriveProfile` with a `pub firmware: Vec` handed that firmware, and every -/// vendor CDB, to any downstream crate that called `ld::profile()`. They are -/// `pub(crate)` so the mechanism stays inside the unlocker. -/// -/// `allow(dead_code)`: this is the on-disk catalog schema, and several captured -/// CDB templates have no in-crate consumer yet. They were `pub` before, so the -/// compiler saw the (external) use; making them `pub(crate)` is what exposes -/// them as unread. Deleting them would silently drop fields from the format the -/// profile-extraction pipeline emits. +/// Only `identity` and `signature` are public; everything else (firmware +/// image, per-drive vendor CDB templates) is `pub(crate)` unlock mechanism +/// that must stay inside this unpublished crate. +/// See docs/drive-profile-visibility.md — field-visibility and allow(dead_code) rationale. #[allow(dead_code)] #[derive(Clone, Deserialize)] pub struct DriveProfile { @@ -72,11 +62,8 @@ pub struct DriveProfile { pub(crate) firmware: Vec, // ── OEM-extended-access CDB templates ────────────────────────────── - // - // All optional — older profile blobs that pre-date the CDB capture - // pipeline simply omit these fields and decode as `None`. Encoded - // in the JSON as lowercase hex strings without separators - // (e.g. `"3c014410e29100002400"` for a 10-byte CDB). + // All optional (`None` if pre-capture-pipeline). Hex strings, no + // separators, e.g. `"3c014410e29100002400"`. #[serde(default)] pub(crate) unlock_init_value: u8, #[serde(default)] @@ -113,14 +100,9 @@ pub struct DriveProfile { pub(crate) speed_calc_table: Option>, } -// Hand-written, REDACTING Debug. `DriveProfile` is `pub` and reachable through -// the public `Profiles` catalog (`ld::bundled` / `ld::profiles`), whose own -// derived `Debug` would recurse into this one. `Cargo.toml` says this crate is -// NEVER published because it carries drive firmware; a derived `Debug` would -// render that firmware image AND every captured per-drive vendor CDB (the unlock -// MECHANISM) straight into any log. Show only the PUBLIC identity + signature -// (what the catalog is public FOR); collapse the firmware to a byte length and -// omit the vendor CDB templates entirely so no mechanism bytes reach a log. +// Hand-written, REDACTING Debug: a derived one would recurse through the +// public `Profiles` catalog and print raw firmware + vendor CDB bytes into +// logs. Show only identity + signature; collapse firmware to a length. impl std::fmt::Debug for DriveProfile { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("DriveProfile") @@ -167,23 +149,16 @@ pub struct ProfileMatch { pub platform: Platform, } -// ── Parsing ──────────────────────────────────────────────────────────── - -/// Decode an even-length ASCII hex string into bytes. -/// -/// Operates on raw bytes rather than `&str` char-boundary slices: a -/// non-ASCII input (e.g. a hand-edited profile with a multi-byte char) -/// could otherwise have `&s[i..i+2]` land inside a UTF-8 char boundary and -/// panic. Hex is ASCII, so any non-ASCII or non-hex byte simply fails to -/// decode. The error is a stable, language-neutral token (`"hex"`), not a -/// translatable English message. +// ── Parsing: decode hex on raw bytes (not `&str` slices) so non-ASCII +// can't land `&s[i..i+2]` inside a UTF-8 char boundary and panic — it +// just fails to decode with `"hex"`. fn decode_hex(s: &str) -> std::result::Result, &'static str> { let bytes = s.as_bytes(); if !bytes.len().is_multiple_of(2) { return Err("hex"); } let mut out = Vec::with_capacity(bytes.len() / 2); - for pair in bytes.chunks_exact(2) { + for pair in bytes.as_chunks::<2>().0.iter() { let hi = (pair[0] as char).to_digit(16).ok_or("hex")?; let lo = (pair[1] as char).to_digit(16).ok_or("hex")?; out.push((hi * 16 + lo) as u8); @@ -223,9 +198,7 @@ where } // ── Fixed-length hex deserializers for CDB templates ──────────────────── -// -// Profile JSON encodes CDBs as lowercase hex strings without separators. -// An empty string / null / missing field decodes as `None`. +// Lowercase hex strings, no separators; empty/null/missing decodes as `None`. fn parse_hex_bytes(s: &str) -> std::result::Result, &'static str> { decode_hex(s) @@ -306,12 +279,8 @@ pub fn bundled() -> Option<&'static Profiles> { CACHE .get_or_init(|| match load_from_str(BUNDLED_PROFILES) { Ok(p) => Some(p), - // The `None` is cached PERMANENTLY (correctly — the blob is fixed at - // compile time), which means a bad shipped profiles.json makes - // Mt1959Unlocker report "drive not supported" for every drive for the - // life of the process. Unlogged, that is indistinguishable from a - // genuinely uncataloged drive; log it once, loudly, at the one place - // the parse happens. + // `None` caches permanently; unlogged that's indistinguishable + // from a genuinely uncataloged drive, so log once, loudly, here. Err(e) => { tracing::error!( target: "freemkv::disc", @@ -340,13 +309,13 @@ fn load_from_str(data: &str) -> Result { /// Find a profile matching a drive's INQUIRY fields. /// -/// Per platform section (MT1959-A, then MT1959-B, then Renesas), tried in -/// order of decreasing specificity: full match including `product_id` and -/// `firmware_date` (skipped when the drive reports no product), then the same -/// without `product_id`. Both require `firmware_date`; every profile carries a -/// unique identity under these fields, so a match is exact — no looser -/// vendor/revision fallback that could pick the wrong same-model variant. All -/// comparisons are whitespace-trimmed. Returns the first section that matches. +/// Per platform section (MT1959-A, then MT1959-B, then Renesas). A drive that +/// reports a `product_id` matches ONLY on the full identity including it — if +/// no cataloged profile matches exactly, the drive is treated as uncataloged +/// (no match), never bound to a same-vendor/revision sibling. The looser +/// four-field pass (vendor/revision/vendor_specific/firmware_date) runs only +/// when the drive reports no `product_id` at all. All comparisons are +/// whitespace-trimmed. Returns the first section that matches. pub fn find_by_drive_id(profiles: &Profiles, drive_id: &crate::DriveId) -> Option { let v = drive_id.vendor_id.trim(); let prod = drive_id.product_id.trim(); @@ -374,12 +343,17 @@ pub fn find_by_drive_id(profiles: &Profiles, drive_id: &crate::DriveId) -> Optio }); } - if let Some(p) = list.iter().find(|p| { - p.identity.vendor_id.trim() == v - && p.identity.product_revision.trim() == r - && p.identity.vendor_specific.trim() == vs - && p.identity.firmware_date.trim() == date - }) { + // The product_id-blind pass runs ONLY when the drive reports none: a + // drive that reports a product_id but misses the exact match is an + // uncataloged variant, else it'd bind to a sibling's wrong firmware. + if prod.is_empty() + && let Some(p) = list.iter().find(|p| { + p.identity.vendor_id.trim() == v + && p.identity.product_revision.trim() == r + && p.identity.vendor_specific.trim() == vs + && p.identity.firmware_date.trim() == date + }) + { return Some(ProfileMatch { profile: p.clone(), platform, @@ -441,6 +415,16 @@ mod tests { let id0 = make_drive_id("TIE", "1.00", "XX00000", "200001010000"); let m0 = find_by_drive_id(&profiles, &id0).unwrap(); assert_eq!(m0.profile.signature, [0xaa, 0xaa, 0xaa, 0xaa]); + + // A REPORTED product_id that matches no cataloged entry must NOT fall + // through to the four-field pass and bind to a sibling (MODEL-A/B); + // an uncataloged variant is no match, not a wrong match. + let mut idc = make_drive_id("TIE", "1.00", "XX00000", "200001010000"); + idc.product_id = "MODEL-C".to_string(); + assert!( + find_by_drive_id(&profiles, &idc).is_none(), + "an uncataloged product_id must not mis-bind to a same-vendor sibling", + ); } #[test] @@ -543,10 +527,8 @@ mod tests { assert_eq!(parse_hex4("aabbccdd").unwrap(), [0xaa, 0xbb, 0xcc, 0xdd]); } - /// Platform::name() returns stable, non-empty, language-neutral identifiers. - /// These strings are logged and keyed on in caller code; changing them is a - /// breaking change. - /// Mutation: swapping Mt1959A and Mt1959B names silently misroutes firmware upload. + // Platform::name() strings are logged/keyed by callers; changing them is + // a breaking change. Mutation: swapping A/B names misroutes firmware upload. #[test] fn platform_name_is_stable() { // The exact strings are part of the public stable API (logged/keyed). @@ -555,11 +537,9 @@ mod tests { assert_eq!(Platform::Renesas.name(), "Renesas"); } - /// find_by_drive_id: exact match (including firmware_date) wins over loose match. - /// Spec: two-pass — first an exact match including firmware_date, then looser. - /// Build two synthetic Profiles entries that differ only by firmware_date, - /// and verify the correct one is selected. - /// Mutation: doing only the loose pass would return the first entry regardless of date. + // find_by_drive_id: two-pass — exact match (incl. firmware_date) wins + // over loose. Mutation: skipping the exact pass returns the first entry + // regardless of date. #[test] fn find_by_drive_id_exact_date_wins_over_loose() { use serde_json::json; @@ -651,10 +631,8 @@ mod tests { assert!(matches!(result, Err(Error::ProfileParse))); } - /// Bundled profiles is non-empty (mt1959_a has at least one entry). - /// This pins the embedded JSON: if profiles.json is accidentally emptied - /// or truncated, this test goes red. - /// Mutation: clearing profiles.json would make this fail. + // Pins the embedded JSON: if profiles.json is emptied/truncated, this + // goes red. #[test] fn bundled_profiles_has_entries() { let profiles = load_bundled().unwrap(); @@ -664,10 +642,8 @@ mod tests { ); } - /// deserialization of a profile with missing optional CDB fields - /// produces None for those fields (not a parse error). - /// Spec: all CDB template fields are `#[serde(default)]` — they are optional. - /// Mutation: making a CDB field required breaks backward-compat with old blobs. + // All CDB fields are `#[serde(default)]`. Mutation: making one required + // breaks backward-compat with old blobs. #[test] fn profile_optional_cdb_fields_default_to_none() { use serde_json::json; @@ -715,12 +691,8 @@ mod tests { ); } - /// `DriveProfile`'s `Debug` must NOT render the firmware image or the - /// per-drive vendor CDB templates — this crate is unpublished precisely - /// because it carries drive firmware, and the profile is reachable through - /// the public `Profiles` catalog. - /// MUTATION: restoring `#[derive(Debug)]` prints the raw firmware bytes - /// (e.g. the 0xEE marker below) and the CDB bytes, so this goes red. + // `Debug` must not render firmware/CDB bytes. Mutation: restoring + // `#[derive(Debug)]` prints them (e.g. the 0xEE marker below), goes red. #[test] fn drive_profile_debug_redacts_firmware_and_cdbs() { use serde_json::json; @@ -749,10 +721,8 @@ mod tests { assert!(s.contains("VIS"), "identity must remain visible: {s}"); } - /// deserialize_hex4 of an empty string must produce [0;4] without error. - /// This matches `deserialize_hex4`'s explicit early-return for empty strings. - /// Mutation: treating empty string as an error prevents profiles where signature - /// was not captured from loading. + // deserialize_hex4("") must produce [0;4]. Mutation: treating empty as + // an error blocks profiles with no captured signature from loading. #[test] fn profile_empty_signature_deserialises_as_zeroes() { use serde_json::json; diff --git a/src/lib.rs b/src/lib.rs index 107451c..12e2b9e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,34 +1,28 @@ //! freemkv-unlock — the unlock layer for the freemkv toolchain. //! //! An **unlocker removes a drive-level bus-encryption barrier** so the drive -//! serves readable (de-bus'd / de-scrambled) sectors. Content-key decryption is -//! a separate layer — the consumer's (libfreemkv's) job. +//! serves readable (de-bus'd / de-scrambled) sectors. Content-key decryption +//! is a separate layer — the consumer's (libfreemkv's) job. //! -//! This crate defines the [`Unlocker`] contract + the SCSI transport contract, -//! and holds the self-contained unlocker modules (firmware / AACS cert / CSS). -//! libfreemkv depends on this crate and dispatches via [`all_unlockers`]; it -//! never names an individual unlocker. To remove an unlocker, delete its module -//! dir and its one line in [`all_unlockers`] — nothing else changes. +//! This crate defines the [`Unlocker`] contract + the SCSI transport +//! contract, and holds the self-contained unlocker modules (firmware / AACS +//! cert / CSS). libfreemkv depends on this crate and dispatches via +//! [`all_unlockers`]; it never names an individual unlocker directly. pub mod scsi; mod aacs; mod css; -// `ld` is public ONLY for its drive-profile catalog (`ld::profiles` / the -// `Profiles` object) and, under the `emulation` feature, the unlock-handshake -// wire format the bdemu test-emulator needs. The unlocker impl itself -// (`Mt1959Unlocker`) is `pub(crate)` — clients still reach unlockers only through -// [`all_unlockers`]. `aacs` and `css` carry no such public catalog, so they -// stay fully private. +// `ld` is public only for its drive-profile catalog + (under `emulation`) the +// handshake wire format bdemu needs; the unlocker impl stays `pub(crate)`. +// See docs/module-visibility.md — module visibility rationale. pub mod ld; -// `renesas` is public for its `is_renesas` drive-probe (which reports a dead -// bus as `Err(UnlockError::Transport)` rather than "not a Renesas drive"); the unlocker impl -// (`Renesas`) is `pub(crate)` — reached only through [`all_unlockers`]. +// `renesas` is public for its `is_renesas` drive-probe (dead bus vs. "not a +// Renesas drive"); the unlocker impl stays `pub(crate)`. +// See docs/module-visibility.md — module visibility rationale. pub mod renesas; -// `freemkv` carries no public catalog (the firmware self-identifies rather -// than matching a bundled profile) and no emulation wire format yet, so it -// stays fully private — the unlocker impl (`FreemkvUnlocker`) is reached only -// through [`all_unlockers`]. +// `freemkv` self-identifies rather than matching a bundled profile, so it +// stays fully private. See docs/module-visibility.md — module visibility rationale. mod freemkv; use scsi::ScsiTransport; @@ -95,13 +89,9 @@ pub struct Unlocked { pub drive_unlocked: bool, } -// Hand-written, REDACTING Debug — `Unlocked` is the public carrier that leaves -// this crate, and both `bus_key` (the AACS bus key) and `vid` (the Volume ID -// that feeds VUK derivation) are key material that must never reach a log or a -// test-failure message in plaintext. The `AacsAuth` / `CertHandshake` siblings -// already redact; a derived `Debug` here would print the very bytes they hide. -// Presence is still observable (Some/None) so a log can say WHETHER a key was -// obtained without revealing WHAT it is. +// Hand-written, REDACTING Debug: `bus_key`/`vid` are key material that must +// never reach a log in plaintext; presence (Some/None) stays observable. +// See docs/unlocked-debug-redaction.md — full rationale. impl std::fmt::Debug for Unlocked { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Unlocked") @@ -206,11 +196,9 @@ pub fn all_unlockers() -> Vec> { mod tests { use super::*; - /// The dispatch order is a documented contract (`firmware → cert → css`) and - /// had NO test: `all_unlockers` is the one place an unlocker is named, and - /// the consumer stops at the first that doesn't decline, so reordering this - /// list silently changes which unlocker claims a drive. Catches a reorder, - /// a rename, and an accidental addition/removal. + // Dispatch order is a documented contract (`firmware → cert → css`); the + // consumer stops at the first unlocker that doesn't decline, so reordering + // this list silently changes which unlocker claims a drive. #[test] fn all_unlockers_dispatch_order_is_firmware_then_cert_then_css() { let names: Vec<&'static str> = all_unlockers().iter().map(|u| u.name()).collect(); @@ -265,10 +253,9 @@ mod tests { assert_eq!(unlocker_name(&DriveId::default()), None); } - /// `Unlocked` is the public carrier that leaves the crate, and its `bus_key` - /// and `vid` are key material. Its `Debug` must NOT print those bytes. - /// MUTATION: restoring `#[derive(Debug)]` prints the raw byte arrays, so the - /// distinctive marker bytes appear in the output and this goes red. + // `bus_key`/`vid` are key material; `Debug` must NOT print those bytes. + // MUTATION: restoring `#[derive(Debug)]` prints the raw byte arrays, so + // the marker bytes appear in the output and this test goes red. #[test] fn unlocked_debug_redacts_key_material() { let u = Unlocked { diff --git a/src/renesas/mod.rs b/src/renesas/mod.rs index a7b9e1b..631612c 100644 --- a/src/renesas/mod.rs +++ b/src/renesas/mod.rs @@ -19,17 +19,13 @@ const RENESAS_MARKER_OFFSET: usize = 16; /// `Ok(true)` if `scsi` is a Renesas-platform drive (Pioneer or HL-DT-ST /// Renesas). /// -/// Issues the vendor READ_BUFFER 0x02/0xF1 probe: a Renesas controller serves a -/// 48-byte identity block whose bytes `[16..19]` are the ASCII `SAT` interface -/// tag. A MediaTek drive rejects it — as a CHECK CONDITION (`Ok` with a -/// non-zero status) or, through a non-conforming transport, as an `Err` -/// carrying a sense — which is `Ok(false)`: not a Renesas drive. This is the -/// definitive Renesas-vs-MediaTek split. +/// Issues the vendor READ_BUFFER 0x02/0xF1 probe: a Renesas controller serves +/// a 48-byte identity block whose bytes `[16..19]` are the ASCII `SAT` +/// interface tag. A rejection (CHECK CONDITION or `Err` with a sense) is +/// `Ok(false)`: not a Renesas drive. /// -/// A TRANSPORT fault is `Err(Transport)`. It used to be folded into the same -/// `false` as a drive rejection, and because this probe is the first SCSI -/// command the unlocker issues, a dead bus read as "not a Renesas drive" — the -/// consumer then walked the remaining unlockers against a bus that was gone. +/// `Err(Transport)` on a dead bus. See docs/renesas-mod.md for why a +/// transport fault must not be folded into `Ok(false)`. pub fn is_renesas(scsi: &mut dyn ScsiTransport) -> std::result::Result { let mut buf = [0u8; RB_F1_LEN]; match scsi.execute(&RB_F1_CDB, DataDirection::FromDevice, &mut buf, 5_000) { @@ -70,28 +66,9 @@ impl Renesas { Renesas } - /// The Pioneer/Renesas vendor "open" — MakeMKV's live sequence, replicated in - /// full: a primary read, and on its failure a payload-less "knock" + a second - /// read at a different window. - /// - /// A. `READ_BUFFER 0x02/0xB0 @0x000004` (164 B) — the primary open. - /// B. (only if A refuses) `WRITE_BUFFER 0x02/0x41 @0xA5AAAA` (0-byte knock) - /// then `READ_BUFFER 0x02/0xB0 @0x500000` (164 B). - /// - /// Success is by SCSI *status* alone (the 164-byte payload CONTENT is not - /// validated — verified empirically: a real drive's table and an all-zero - /// buffer are accepted identically as long as status is GOOD). We run the - /// whole A→knock→B path rather than bailing after A, because the knock is a - /// per-firmware fallback: some firmware revisions only answer the `0x500000` - /// window *after* the knock (MakeMKV issues the knock exactly when A returns - /// CHECK CONDITION). If NEITHER A nor B returns GOOD, the drive did not open → - /// defer to the next unlocker. - /// - /// (There may be more than one knock offset across firmware families; only - /// the observed `0xA5AAAA` is issued here — add variants as they are proven.) - /// - /// `Ok(true)` = opened (A or B GOOD). `Ok(false)` = neither opened (CHECK - /// CONDITION). `Err(Transport)` = dead bus. + // MakeMKV's vendor "open" sequence: primary read (A), and on refusal a + // knock + second read (B). See docs/renesas-mod.md for the full sequence + // and why we run A→knock→B rather than bailing after A. fn vendor_open(scsi: &mut dyn ScsiTransport) -> std::result::Result { const RB_B0_04_CDB: [u8; 10] = [0x3C, 0x02, 0xB0, 0x00, 0x00, 0x04, 0x00, 0x00, 0xA4, 0x00]; const KNOCK_A5AAAA_CDB: [u8; 10] = @@ -142,17 +119,9 @@ impl Unlocker for Renesas { "Renesas" } - /// Recognize and open a Renesas/Pioneer drive the way MakeMKV does: - /// 1. `READ_BUFFER 0xF1` identity gate (the `SAT` marker) — else this is - /// not our drive → `NotApplicable`. - /// 2. the single vendor open read `READ_BUFFER 0xB0 @0x04` — GOOD status - /// confirms the drive is rip-ready. A CHECK CONDITION here is exactly - /// where MakeMKV gives up on the drive, so we defer to the next unlocker - /// (`NotApplicable`) rather than claim a drive we could not open. - /// - /// On success returns `drive_unlocked: false`: the vendor open makes the - /// drive readable, but AACS bus decryption is still the host cert's job (the - /// cert unlocker runs next), so we do not assert the bus is handled here. + // Gate on the `0xF1` SAT identity, then the vendor open read `0xB0@0x04`; + // `drive_unlocked: false` since AACS bus decryption is the cert's job. + // See docs/renesas-mod.md for the full MakeMKV-parity rationale. fn unlock_features( &self, scsi: &mut dyn ScsiTransport, @@ -210,11 +179,8 @@ mod tests { } } - /// Rejects the command the way a MediaTek drive does: ILLEGAL REQUEST. The - /// sense is what distinguishes this from a dead bus — the old fixture sent - /// `sense: None`, which is the wire shape of a TRANSPORT fault, and the test - /// built on it asserted `NotApplicable`, pinning exactly the - /// misclassification this file had. + // Rejects like a MediaTek drive: ILLEGAL REQUEST with a sense, which is + // what distinguishes this from a dead bus. See docs/renesas-mod.md. struct RejectingTransport; impl ScsiTransport for RejectingTransport { fn execute( @@ -303,10 +269,8 @@ mod tests { assert_eq!(err, UnlockError::NotApplicable); } - /// The same rejection delivered the way a CONFORMING transport delivers it — - /// `Ok` with a CHECK CONDITION status — must reach the same answer. Catches - /// dropping the `r.status == 0` check, which would read the probe buffer's - /// zero fill as an identity block. + // Same rejection via a CONFORMING transport (`Ok` + CHECK CONDITION); + // must reach the same answer. See docs/renesas-mod.md. #[test] fn check_condition_is_not_a_renesas_drive() { use crate::scsi::mock::{MockTransport, Reply}; @@ -332,12 +296,8 @@ mod tests { ); } - /// A genuine Renesas drive (passes the `0xF1` SAT gate) that REFUSES the - /// vendor open read `RB 0xB0@0x04` with CHECK CONDITION — the exact point - /// where MakeMKV abandons the drive. `unlock_features` must defer to the - /// next unlocker (`NotApplicable`), not claim a drive it could not open. - /// MUTATION: dropping the `vendor_open` gate makes this return `Ok` and - /// wrongly claim the drive. + // A recognized Renesas drive that REFUSES the vendor open read must + // defer to the next unlocker, not claim the drive. See docs/renesas-mod.md. #[test] fn open_rejection_defers_to_next_unlocker() { struct GateOkOpenRefused; diff --git a/src/scsi.rs b/src/scsi.rs index 7b180be..c09024c 100644 --- a/src/scsi.rs +++ b/src/scsi.rs @@ -100,14 +100,8 @@ pub(crate) fn build_set_cd_speed(read_speed: u16) -> [u8; 12] { } // ── Test fixture ──────────────────────────────────────────────────────────── -// -// The crate-wide mock transport. Before this existed, EVERY mock in the crate -// returned `Ok` with status 0 and a full `bytes_transferred` — so no test could -// ever exercise the three outcomes the contract actually distinguishes, and a -// whole class of transport-fault misclassification bugs sat uncaught (a dead bus -// read as "this unlocker doesn't apply", a zero-filled buffer read as valid -// drive data). This fixture can express all of them, and is the red-before-green -// vehicle for the transport-contract fixes across ld / aacs / css / renesas. +// Crate-wide mock transport, able to express all three transport outcomes. +// See docs/scsi-mock-fixture.md — why this fixture exists #[cfg(test)] #[allow(dead_code)] // a fixture: each helper is used by a subset of the modules pub(crate) mod mock { @@ -322,10 +316,9 @@ mod tests { assert_eq!(build_set_cd_speed(0xFFFF)[2..4], [0xFF, 0xFF]); } - /// The fixture itself must honour the contract it exists to test: a drive - /// sense is `Ok` with a non-zero status, a bus fault is `Err`. If this ever - /// drifts, every transport-contract test built on it silently stops testing - /// anything. + // The fixture must honour the contract it tests: a drive sense is `Ok` + // with non-zero status, a bus fault is `Err`. If this drifts, every + // transport-contract test built on it silently stops testing anything. #[test] fn mock_transport_expresses_the_three_contract_outcomes() { let mut t = MockTransport::scripted(