Internal review

Task 1 — Wire format spec review (v0 + v1)

Spec-versus-implementation alignment audit of the v0 and v1 wire formats and their parsers.

Task 1 — Wire format spec review (v0 + v1)

Date: 2026-05-11 Reviewer: Cho García Scope: crypto/spec/format-v0.md, crypto/spec/format-v1.md, and the corresponding implementations in @shieldfive/crypto (vendored at 1.0.0-alpha.3, repo HEAD a5e99ac) and the web app integration (web/utils/sfCryptoWorkerImpl.ts, web/utils/sfCryptoDecrypt.ts, web/utils/sfCryptoWorker.ts, web HEAD 37272e7f). Methodology: spec-vs-implementation alignment audit. Both specs were read end-to-end without consulting code; the implementation was then read against the spec notes; the dispatcher was checked at the boundary. Streaming back-pressure invariants were treated as out of scope (Task 5).

Summary

The v0 spec and implementation are tightly aligned — v0 is a small surface and the legacy reader/writer match the spec byte-for-byte. The v1 spec and implementation are also functionally aligned, but the v1 spec has three spec-internal contradictions and one spec-vs-implementation drift that an independent implementer would trip over. Highest-impact findings:

  • 1.1 — the v1 prose says file_id is "mixed into every chunk's AAD" but the explicit AAD construction omits it; implementation matches the explicit construction. Structural binding via key/nonce derivations is preserved.
  • 1.4c — the header section's MUST verify header_mac before parsing suite_payload is unimplementable for the PQ-hybrid suite by construction; the same spec's PQ-hybrid section prescribes the opposite order, and the implementation follows the latter. The spec contradicts itself.
  • 1.2 — salt="" in the nonce-prefix derivation is ambiguous between RFC 5869's "absent salt" (HashLen zeros) and a literal zero-length string; the implementation uses HashLen zeros without saying so in the spec.

None reach Critical/High; confidentiality and authenticity properties hold under both prose and explicit definitions when read by the existing implementation. The findings are documentation correctness and forward- looking hygiene issues for independent implementers and Phase 3.

Findings

Finding 1.1 — Spec prose contradicts the explicit AAD definition (fileid is _not in AAD)

Severity: Medium Affects: v1 Component: spec

Description. crypto/spec/format-v1.md contains two contradictory descriptions of how file_id is bound to chunk ciphertext:

  • Prose at the header section (around the file_id field description):

    "It is mixed into every chunk's AAD to bind ciphertext to its file."

  • Explicit construction in the chunks section:

    aad = "shieldfive/v1/chunk" || uint64_be(i) || uint64_be(total_chunks) || uint8(is_final) "aad is a fixed-length 36 bytes (19 bytes domain string + 8 + 8 + 1)."

The two statements are mutually exclusive. The implementation matches the explicit definition: crypto/src/format/header.ts:219-236 (buildChunkAad) writes only the domain bytes, chunk index, total chunks and the is_final flag — file_id is absent. crypto/src/internal/types.ts:90 fixes CHUNK_AAD_LENGTH = 19 + 8 + 8 + 1 = 36, leaving no room for a 16-byte file_id.

file_id IS structurally bound to every chunk through:

So the cross-file splice resistance the prose claims IS present, but it arises from key/nonce derivation rather than AAD content.

Reproduction or evidence. Inspect any v1-encrypted blob and pass any chunk's ciphertext into a verifier that reconstructs AAD as "shieldfive/v1/chunk" || idx || total || is_final — verification succeeds. Adding file_id to that AAD makes verification fail. The implementation's AAD construction is identical across all three suites (grep for buildChunkAad confirms a single shared implementation).

Recommendation. Fix the spec prose, not the code, since changing the AAD would break the on-disk format. Replace "mixed into every chunk's AAD to bind ciphertext to its file" with something like:

"It is mixed into every chunk's key and nonce derivation (see the per-suite definitions below) to bind ciphertext to its file. file_id is not present in the AAD itself; the cross-file splice resistance is structural, via the suite-specific HKDF salt."

Also call out at the AAD definition that this is a deliberate choice and that future implementers MUST NOT add file_id to the AAD bytes.

Land alongside a deterministic test vector that pins the AAD bytes for chunk 0 and a chunk N≠0, exercising the no-file_id-in-AAD invariant. Without a test vector, a future contributor who "helpfully" adds file_id to AAD construction would not be caught by the test suite — the regression would only surface as wire-incompatibility against old files.

Finding 1.2 — Spec ambiguity: salt="" in nonce-prefix derivation; implementation uses HashLen zeros

Severity: Medium Affects: v1 (AES-GCM and XChaCha suites) Component: spec + implementation

Description. Both v1 suites define the chunk nonce prefix as:

chunk_nonce(file_id, i) = truncate12(HKDF-SHA-256(ikm=file_id, salt="", info="...nonce-prefix", L=12))[0..3] || uint64_be(i)

The string salt="" is ambiguous. RFC 5869 §2.2 distinguishes:

  • "salt not provided": the extract step uses HashLen zeros (32 bytes for SHA-256).
  • salt = '' (zero-length string): used as-is, producing a different HKDF-Extract output.

The implementation calls hkdfSha256({ ikm: fileId, info: ..., length: 4 }) with no salt field — see crypto/src/suites/aes-gcm-v1/index.ts:79-86. The HKDF helper substitutes 32 zero bytes when salt is absent — see crypto/src/internal/hkdf.ts:20:

const salt = options.salt ?? new Uint8Array(32) // SHA-256 zero-salt fallback

Encrypt and decrypt go through the same helper, so v1 files are internally consistent. However, an independent implementer who reads the spec and implements salt=Uint8Array(0) will produce a different nonce prefix and fail to decrypt files emitted by this library — and conversely.

Reproduction or evidence. Two HKDF-SHA-256 calls with the same ikm, info, and L, one with salt=Uint8Array(0) and one with salt=Uint8Array(32) /*zeros*/, produce different outputs.

Recommendation. Update the spec to remove the ambiguity. Recommended phrasing:

chunk_nonce(file_id, i) = HKDF-SHA-256(ikm=file_id, salt=zeros(32), info="...nonce-prefix", L=4) || uint64_be(i)

Or, equivalently, "salt absent (per RFC 5869 default)" — but the explicit form is clearer for implementers. Apply the same change to the XChaCha suite (currently L=24, [0..15] truncate). Also publish a test vector that covers nonce-prefix derivation so any future spec/implementation drift is caught at CI.

Land alongside a deterministic test vector for nonce-prefix derivation that pins the absent-salt = 32-zero-bytes convention. A third-party implementer reading the current spec ambiguously and shipping a salt=Uint8Array(0) implementation would produce different bytes and fail to interop with shieldfive.com files. The test vector pins which convention is canonical.

Finding 1.3 — Spec defines nonce prefix via redundant truncateN(...)[0..k] indirection

Severity: Low Affects: v1 (AES-GCM and XChaCha suites) Component: spec

Description. The spec writes the nonce prefix as truncate12(HKDF-SHA-256(..., L=12))[0..3] for AES-GCM. HKDF-Expand is deterministic in its first L bytes, so HKDF(L=12)[0..3] and HKDF(L=4) return identical bytes. The implementation uses HKDF(L=4) directly. This is not drift — outputs are identical — but the spec is harder to implement than necessary.

Recommendation. Replace with HKDF(..., L=4) (and L=16 for XChaCha) directly. Pairs naturally with the spec change in 1.2.

Finding 1.4a — AES-GCM-v1 decryptBlob parses suite_payload before MAC verification

Severity: Low Affects: v1 (AES-GCM-v1 whole-blob entry point only; streaming decryptor and web worker are fine) Component: implementation

Description. Spec § "Header" states:

"Readers MUST verify header_mac before parsing suite_payload or any chunk."

The library's whole-blob decryptor calls parseAesGcmV1SuitePayload before verifyHeaderMac — see crypto/src/suites/aes-gcm-v1/api.ts:208-212:

parseAesGcmV1SuitePayload(parsed.suitePayload) // parses suite payload
await verifyHeaderMac(parsed, contentKey) // then verifies MAC

parseAesGcmV1SuitePayload today only checks the field length and slices two byte ranges — no semantic interpretation, no decapsulation, no key import. So the current AES-GCM-v1 entry point violates the MUST in form but not in spirit. The web worker (web/utils/sfCryptoWorkerImpl.ts:396-402) correctly parses then verifies, and never touches suite_payload. The streaming decryptor (crypto/src/streams/aes-gcm-v1.ts:301-312) does the right thing too: parseHeader → verifyHeaderMac → only then processes chunks (and never parses suite_payload at all in the AES-GCM-v1 case).

Reproduction or evidence. Read decryptBlob lines 209 and 212 in the order they are written.

Recommendation. Swap the two lines so verifyHeaderMac runs first. Apply the same fix to XChaCha (Finding 1.4b). For PQ-hybrid, the call order is structurally fixed by the spec — see Finding 1.4c.

Add a comment to parseHeader's docstring:

"parseHeader does not verify the header MAC. For non-KEM suites, callers MUST call verifyHeaderMac BEFORE doing anything with parsed.suitePayload, including length checks, field extraction, or downstream parsing. KEM suites (PQ-hybrid) cannot follow this rule by construction; see spec/format-v1.md § '0x03'."

Finding 1.4b — XChaCha-v1 decryptBlob parses suite_payload before MAC verification

Severity: Low Affects: v1 (XChaCha-v1 whole-blob entry point) Component: implementation

Description. Same shape as Finding 1.4a in a different file. See crypto/src/suites/xchacha-v1/api.ts:177-178:

parseXChaChaV1SuitePayload(parsed.suitePayload) // parses suite payload
await verifyHeaderMac(parsed, contentKey) // then verifies MAC

parseXChaChaV1SuitePayload is, like its AES counterpart, only a length-check + slice — no semantic interpretation. Same MUST violation in form, same benign-in-practice behavior.

Reproduction or evidence. Read decryptBlob lines 177 and 178 in the order they are written.

Recommendation. Swap the two lines so verifyHeaderMac runs first. Bundle with the 1.4a fix in a single PR against @shieldfive/crypto.

Finding 1.4c — Spec contradiction: header § MUST is unimplementable for the PQ-hybrid suite

Severity: Medium Affects: v1 (PQ-hybrid suite) Component: spec (Phase 3 prerequisite)

Description. The spec contains two rules that cannot both be obeyed for the PQ-hybrid suite:

  • Spec § "Header":

    "Readers MUST verify header_mac before parsing suite_payload or any chunk."

  • Spec § "0x03 — pq-hybrid… Decryption":
    1. Reader decapsulates mlkem_ciphertext with sk_pq to recover S_pq.
    2. Reader unwraps classical_wrapped with K_c to recover S_c.
    3. Reader recomputes K = HKDF(...). 4. Reader verifies header_mac (which uses K).

For PQ-hybrid, the header MAC key IS the combined key K, and K can only be computed from suite_payload via ML-KEM decapsulation + classical unwrap. Therefore steps 1-3 (which "parse" suite_payload in the strongest sense — actually decapsulating attacker-controlled bytes through ML-KEM) MUST happen before step 4. The header § MUST is unimplementable for this suite by construction.

The implementation (crypto/src/suites/pq-hybrid-v1/api.ts:168-186) follows the PQ-hybrid § order exactly: parseHeader → decapsulateFromHeader (ML-KEM-1024 decap on parsed.suitePayload[0..1568) + secretbox unwrap of the classical share) → verifyHeaderMac(parsed, combinedKey). There is no implementation bug to fix here; the order is forced by the construction.

Why this is Medium and not Low. Two related concerns:

  1. Spec correctness. A specification that contradicts itself is itself a security defect — independent implementers will read the header MUST and either reject the PQ-hybrid suite as unspecified-correct or, worse, attempt some made-up workaround that diverges from this implementation.
  2. Phase 3 readiness. PQ-hybrid is the default suite for new files per the spec (§ "Suite payloads / 0x03 (default)"). Once Phase 3 ships, this becomes the production hot path. The security argument that ML-KEM decap on attacker-controlled bytes is safe rests on ML-KEM-1024's IND-CCA2 property and the secretbox AEAD on the classical share — both primitives must remain unbroken for the pre-MAC decapsulation to be safe. This is a real argument and it holds, but it should be written down in the spec, not implicit.

Reproduction or evidence. Read the two spec sections side-by-side. Read crypto/src/suites/pq-hybrid-v1/api.ts:168-186 to confirm the implementation follows the PQ-hybrid § flow.

Recommendation. Spec change, not code change:

  1. In § "Header", scope the MUST: change "Readers MUST verify header_mac before parsing suite_payload or any chunk" to "For suites whose header MAC key is not derived from suite_payload (currently 0x01, 0x02), readers MUST verify header_mac before parsing suite_payload or any chunk. KEM suites (currently 0x03) verify header_mac after decapsulation per their suite-specific decryption flow; see § '0x03' below."
  2. In § "0x03 — pq-hybrid", add a security argument paragraph after step 5: "Pre-MAC decapsulation is safe because ML-KEM-1024 is IND-CCA2 secure (decapsulation on adversarial ciphertext yields no useful information about sk_pq) and the classical share is wrapped with XSalsa20-Poly1305 secretbox (AEAD on attacker-influenced bytes). A wrong combined key K causes the subsequent header_mac verification to fail, surfacing the failure to the application." Land this as a Phase 3 prerequisite.

Track this as Status: Open, Phase 3 prerequisite in the triage log.

Finding 1.5 — validateHeaderInputs does not enforce the (totalChunks × chunkSize, plaintextSize) invariant on encrypt

Severity: Low Affects: v1 Component: implementation

Description. parseHeader (crypto/src/format/header.ts:155-165) enforces, for totalChunks > 0:

(totalChunks - 1) * chunkSize + 1 <= plaintextSize <= totalChunks * chunkSize

This matches spec § "Chunks": non-final chunks are exactly chunk_size bytes; final chunk is between 1 and chunk_size.

validateHeaderInputs (crypto/src/format/header.ts:238-274) runs on the encrypt side and only validates each field individually — no cross-field check. A buggy encoder could emit a header that the same library's parser would later reject. Today this is unreachable because all encoders compute Math.ceil(plaintextSize / chunkSize) themselves, but nothing in validateHeaderInputs would catch a regression that decoupled those fields.

Recommendation. Replicate the cross-field check from parseHeader inside validateHeaderInputs. Consider extracting both into a shared assertHeaderConsistency(fields) helper so encode and decode share one truth.

Finding 1.6 — Web app dispatcher silently defaults cipherVersion to v0 when undefined

Severity: Low Affects: cross-version Component: implementation

Description. web/utils/sfCryptoDecrypt.ts:31:

const version = input.cipherVersion ?? 1

The accompanying comment (line 14-19) cites a transitional shim ("share page until verify-password is updated in Stage 2c"). A v1 file passed in without cipherVersion is dispatched to the v0 path. The v0 reader interprets the SF5 magic + suite + flags + file_id + ... bytes as ciphertext and AES-GCM authentication fails on "chunk 0" — confidentiality is preserved, but the error message is misleading.

Recommendation. Stage 2c is shipped as of 2026-05-11 (PRs #115, #116, #117). All known callers now thread cipherVersion. Remove the ?? 1 default and require an explicit version in a follow-up web PR; the transitional rationale in the comment at lines 14-19 can be deleted at the same time.

Finding 1.7 — Worker fileId byte-equality check is non-constant-time (informational, non-issue)

Severity: Informational Affects: v1 Component: implementation

Description. web/utils/sfCryptoWorkerImpl.ts:407-416:

for (let i = 0; i < expectedFileId.length; i++) {
  if (expectedFileId[i] !== parsed.fileId[i]) {
    throw new Error('crypto worker v1: header fileId does not match expected')
  }
}

Early-return byte comparison. In a different context this would be a timing leak. Here it is not exploitable: parsed.fileId is in cleartext on disk (the v1 header is unencrypted), and a successful header MAC verification three lines earlier (line 402) already proves the caller has the matching content key for this file_id. The timing channel reveals a value already public.

Recommendation. Optional: switch to constantTimeEqual from crypto/src/internal/runtime for code-style consistency. No security action required.

Finding 1.8 — Spec underspecifies the suite_payload zero-fill convention for vault-stored wrapped keys

Severity: Low Affects: v1 (AES-GCM-v1 explicitly; unclear for the other two) Component: spec

Description. Spec § "Suite payloads / 0x01 aes-256-gcm-v1":

"files stored in the ShieldFive vault store the wrapped key in the database and set this field to 60 bytes of zero."

The "field" in question is the 72-byte suite payload (60-byte wrapped_key

  • 12-byte wrap_iv). The spec is silent on whether wrap_iv is also zeroed, or set to a real-but-unused IV, when the wrapped key lives in the database. The implementation (web/utils/sfCryptoWorkerImpl.ts:252) zero-fills all 72 bytes:
const suitePayloadOverride = new Uint8Array(V1_AES_GCM_SUITE_PAYLOAD_LENGTH)

This is benign, but the spec should match.

Recommendation. Spec should say: "when the wrapped key is stored out-of-band, all 72 bytes (wrapped_key + wrap_iv) are set to zero." Apply the same wording to the XChaCha suite (96-byte zero fill) and clarify the PQ-hybrid case.

Finding 1.9 — V0 trailing-bytes check rejects exactly-tag-size remainder

Severity: Informational Affects: v0 Component: implementation

Description. crypto/src/suites/aes-gcm-v0/api.ts:79-83:

} else if (trailingBytes <= V0_TAG_BYTES) {
  throw new Error(
    `decryptV0: trailing bytes (${trailingBytes}) too small for an AEAD tag — file truncated`,
  )
}

The v0 spec is silent on the minimum chunk size. The implementation rejects trailers ≤ 16 bytes (a 16-byte trailer would be a tag-only chunk with empty plaintext, which is meaningless). This is sensible defense in depth and not a drift; flagged here so it's not surprising during future review.

Recommendation. None. If desired, add a one-liner to the v0 spec saying empty final chunks are not permitted by the reference reader.

Finding 1.10 — Versioning policy: implementation refuses unknown minor versions unconditionally; spec permits an opt-in override

Severity: Informational Affects: v1 Component: implementation (intentional gap)

Description. Spec § "Versioning policy":

"Old readers encountering a minor version they don't recognize MUST refuse to decrypt unless explicitly configured to ignore unknown minor versions."

The implementation refuses unconditionally — the magic check (crypto/src/format/header.ts:117-121) compares the full 5-byte FORMAT_MAGIC literal 0x53 0x46 0x35 0x01 0x00, so a non-zero minor version fails as bad_magic. There is no override flag.

This satisfies the MUST. The override path is simply not exposed, which is the safer default.

Recommendation. None for the current release. If/when a minor version bump ships, decide whether to expose the override at that point.

Finding 1.11 — V0/V1 isolation is structural (positive finding)

Severity: Informational Affects: cross-version Component: both

Description. The two formats cannot be silently confused:

  • v1 file fed to decryptV0: the v0 reader has no header concept and begins AES-GCM decryption at byte 0 with iv = noncePrefix(4) || 0. The first 16 bytes of a v1 file are magic(5) || suite(1) || flags(1) || file_id(...), none of which form a valid AES-GCM tag for that key/IV/AAD combination. Decrypt throws "chunk 0 failed authentication". No silent decryption.
  • v0 file fed to parseHeader: v0 has no header, so the first 5 bytes are arbitrary AES-GCM ciphertext bytes. The chance of those being exactly 53 46 35 01 00 is 2⁻⁴⁰ per file. parseHeader throws HeaderError('bad_magic'). No silent dispatch.
  • Web app dispatcher (web/utils/sfCryptoDecrypt.ts:30-39) routes by an explicit cipher_version field from the database (1=v0, 2=v1, anything else throws Unsupported cipher_version). It does not auto-detect.
  • The crypto library does not auto-detect across formats. It exposes looksLikeV0 (crypto/src/suites/aes-gcm-v0/api.ts:144-155) but only as a heuristic, clearly documented as such, and not wired into the decrypt pipeline. The auto-routing v1 dispatcher (crypto/src/index.ts:115-191) refuses to handle v0 by design ("v0 has no header, so the dispatcher cannot route to it").

The dispatcher branch for unknown cipher_version throws a clear Unsupported cipher_version: <n> message — checked by inspection.

Recommendation. None. Worth documenting as a deliberate property of the system in spec/format-v1.md § "Compatibility with v0" so future contributors don't try to add cross-format auto-detection.

What I checked but did not find issues with

The following areas were reviewed against both spec and implementation; no drift was found.

v0 (legacy)

  • AES-GCM call with aad=∅ matches spec.
  • IV construction prefix(4) || uint64_be(i) matches spec exactly (crypto/src/suites/aes-gcm-v0/api.ts:38-43).
  • 32-byte content key, 4-byte nonce prefix, 16-byte tag size all match spec.
  • decryptV0 imports key with ['decrypt'] only (no encrypt usage imported).
  • The migration bridge's encryptV0 is a bit-for-bit reproduction of the legacy production worker (per the bridge's docstring and code).
  • Worker handler (crypto/src/migration/v0-bridge.ts:217-347) validates key length, nonce-prefix length, chunk index bounds, and rejects unknown message types.

v1 — header

  • Magic bytes literal 53 46 35 01 00 matches spec (crypto/src/internal/types.ts:39-41).
  • Suite byte values 0x01/0x02/0x03 match spec; 0x00 is implicitly invalid (not in SUITE) and rejected by VALID_SUITES.has (crypto/src/format/header.ts:127-129).
  • Reserved flags byte must be 0x00, enforced as MUST (crypto/src/format/header.ts:131-135).
  • Header field order, sizes, and encodings (uint16/32/64 BE) match spec exactly: magic(5) || suite(1) || flags(1) || fileId(16) || chunkSize(4) || totalChunks(8) || plaintextSize(8) || suitePayloadLen(2) || suitePayload(N) || mac(32).
  • header_mac_key derivation HKDF(content_key, salt=file_id, info="shieldfive/v1/header-mac", L=32) matches spec (crypto/src/format/header.ts:72-82).
  • Header MAC scope = all bytes before the MAC field. Verified by parsed.unauthenticatedBytes = input.slice(0, macStart) and hmacSha256Verify(macKey, parsed.unauthenticatedBytes, parsed.mac) (crypto/src/format/header.ts:181, 201-214).
  • Header MAC verification uses constant-time comparison (crypto/src/internal/hmac.ts:26-33 → constantTimeEqual).
  • parseHeader cross-field consistency checks (chunk size in range, total chunks in range, plaintext_size consistent with totalChunks × chunkSize) go beyond what the spec strictly requires.

v1 — chunks (AES-GCM-v1)

  • AAD construction "shieldfive/v1/chunk" || uint64_be(i) || uint64_be(total) || uint8(is_final) is identical on encrypt and decrypt; both call the shared buildChunkAad (crypto/src/suites/aes-gcm-v1/index.ts:139-179).
  • is_final flag is set only for chunkIndex === totalChunks - 1. Confirmed in encrypt and decrypt paths.
  • Chunk-key derivation HKDF(content_key, salt=file_id, info="shieldfive/v1/aes-gcm/chunk-key", L=32) matches spec.
  • IV construction noncePrefix(4) || uint64_be(i) matches spec; nonce prefix is HKDF-derived from file_id (subject to the spec ambiguity in Finding 1.2).
  • Length-prefix layout: 4-byte big-endian uint32 immediately before each ciphertext, matching spec.
  • Non-final chunk size enforcement: cipherLen === chunkSize + 16 is enforced (crypto/src/suites/aes-gcm-v1/api.ts:236-244 and crypto/src/streams/aes-gcm-v1.ts:340-348).
  • Final chunk size enforcement: 1 + 16 ≤ cipherLen ≤ chunkSize + 16.
  • Trailing-bytes-after-last-chunk rejection in both decryptBlob (api.ts:267-269) and the decrypt stream's flush (streams/aes-gcm-v1.ts:399-401).
  • bytesEmitted === plaintextSize final consistency check.
  • parseHeader rejects totalChunks === 0 && plaintextSize !== 0 (header.ts:156-158).

v1 — chunks (XChaCha and PQ hybrid)

  • Both suites use the same buildChunkAad (verified by grep — single implementation in format/header.ts, three callers in the suite modules).
  • PQ-hybrid header MAC is keyed with the combined key K = HKDF(S_c || S_pq, salt=file_id, info="shieldfive/v1/pq-hybrid/combine") (crypto/src/suites/pq-hybrid-v1/index.ts:138-152 and api.ts:91-103, 186), matching spec § "0x03 — pq-hybrid… Decryption" step 4.
  • PQ-hybrid chunk context derives chunk_key = HKDF(K, salt=file_id, info="shieldfive/v1/xchacha/chunk-key", L=32) — i.e., uses the XChaCha suite's chunk-key derivation with K in the role of content_key. Matches spec § "0x03" step 5 ("Subsequent chunks use the XChaCha20-Poly1305 chunk format with K").
  • XChaCha-v1 decryptBlob call ordering checked explicitly at crypto/src/suites/xchacha-v1/api.ts:177-178 — same spec-MUST violation as AES-GCM-v1, filed as Finding 1.4b.
  • PQ-hybrid-v1 decryptBlob call ordering checked explicitly at crypto/src/suites/pq-hybrid-v1/api.ts:168-186: parseHeader → decapsulateFromHeader → verifyHeaderMac. The decapsulation-before-MAC ordering is structurally required by the PQ-hybrid construction (the MAC key IS the combined key, recovered via decapsulation) and matches the spec's PQ-hybrid § decryption flow. The attendant spec-internal contradiction with the header § MUST is filed as Finding 1.4c (the implementation itself is not buggy here; the spec is).

Web app — v1 worker

  • initDecryptV1 order is correct: probe → parseHeader → verifyHeaderMac → caller-fileId cross-check → push into stream (web/utils/sfCryptoWorkerImpl.ts:392-440).
  • Caller-supplied fileId is checked for length match and byte equality against parsed.fileId (Finding 1.7 covers the timing characteristic).
  • initEncryptV1 correctly zeros the 72-byte suite payload override (Finding 1.8 covers the spec-side ambiguity).
  • The worker only supports the AES-GCM-v1 suite — it does not import XChaCha or PQ-hybrid streaming code, and a 256-byte probe would not cover the PQ-hybrid 1741-byte header. This is consistent with the documented scope (Stage 2 is AES-GCM only).
  • Worker validates content key length (32) and file_id length (16) on init.
  • The deferred-header-bytes pattern — write at chunk-0 to avoid HWM=0 back-pressure — is documented and intentional. Per task scope, the back-pressure invariants themselves are out of scope (Task 5); the wire output it produces is correct (header || length-prefix || ciphertext for chunk 0; length-prefix || ciphertext for subsequent chunks).

Cross-version dispatch

  • Cipher_version=1 → v0 path; cipher_version=2 → v1 path; any other value throws (Finding 1.10/1.11 cover behavior).
  • Library does not auto-detect across formats; v0 has no magic to detect.
  • A v1 blob fed to decryptV0 fails AEAD authentication; a v0 blob fed to parseHeader fails magic check.

Out of scope (deferred to other tasks)

  • Key derivation review (envelope-to-content-key, master-secret-to-keypair paths, KDF parameters, key hierarchy) — Task 2.
  • AEAD nonce reuse audit (cross-file, cross-suite, cross-key-rotation) — Task 4.
  • Streaming invariants (back-pressure, deferred-header-bytes deadlock prevention, producer-consumer ordering) — Task 5.
  • Threat model coverage and mapping to the v1 design properties — Task 3.
  • The looksLikeV0 heuristic's failure modes when used outside the dispatcher — not exercised in production code paths reviewed here.

References