Skip to content

toyos-crypto stage 4, the test side: vectors, integer oracle, differential, fuzz, controls, Miri and the constant-time harness - #993

Merged
Japabu merged 18 commits into
mainfrom
wt/toyos-crypto-sec
Oct 22, 2026
Merged

Japabu merged 18 commits into
mainfrom
wt/toyos-crypto-sec

Conversation

@Japabu

@Japabu Japabu commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

What this is

Stage 4 ("Secure") of issues/toyos-crypto-is-in-the-tree-on-par-with-ring-and-secure.md, the test-side items only. The arithmetic modules are untouched: typed domains and the arithmetic rewrite belong to the speed effort. These tests check whatever arithmetic sits underneath, so they guard that rewrite.

What changes

  • Independent vectors. 43 Wycheproof files (testvectors_v1 at 7a325932, the stage 1 pin). That is every file for an interface the crate offers:

    The NIST vectors are CAVP SHAVS (SHA-384 added to tests/cavp, which this crate now also reads), GCMVS AES-128/256 encryption, ECDSA SigVer and PKV, and ACVP HMAC beside the existing ML-KEM files. Each case is either judged against the file's verdict or counted outside the interface under a named reason. Wycheproof's totals are held to numberOfTests; each NIST file's to a count taken from its text apart from the parse (Count = , Len = , Msg = , Qx = , "tcId"). Every file is byte for byte, pinned by sha256 in NOTICE with a COMMITTED_FILES row; the old tests/cavp files now have rows too.

  • No fiat-crypto (owner, 2026-10-21). The dev-dependency is gone, and Poly1305's oracle is now num-bigint. test_arithmetic is a harness over one Field trait. field25519 and the four NIST Montgomery moduli are checked against integers on:

    • every pair of limb and modulus boundary values;
    • seeded values;
    • chains that feed each result back unreduced;
    • inversion, distributivity, and (x − y) + y.

    scalar25519 is checked the same way: reduction, S < L, add and mul. A new representation is checked by implementing the trait. ML-KEM's mod-q reduction is checked exhaustively, and Compress/Decompress over every input.

  • Differential. A primitive with no instruction path (curves, X25519, Ed25519, RSA, ML-KEM, WPA2's HMAC-SHA1) is the same code in every form, so its differential, fuzz and vector tests run in the portable form and are ignored by name in the other two. 10,000 seeded cases per primitive against ring and RustCrypto (no rsa), spread over all CPUs. Each case is seeded from its own number, so a failure names a case that reproduces. The PMK alone runs 200 cases, because each one is 8,192 HMAC computations.

  • Fuzz. 4,000 structured cases per decoder: SEC 1, DER, RSA components, Ed25519 encodings, ML-KEM keys and ciphertexts, and the rustls provider's key shares, RSAPublicKey DER and records. Each verdict comes from an independent decoder.

  • Negative controls. 15 checked patches (tag compares ×5 including GCM's one-batch path, on-curve, coordinate range, scalar range, ML-KEM coefficient range, S < L, canonical Ed25519 y, and four length checks). At the measured head, each makes its named test fail. Two first survived the whole suite: SEC 1 coordinate range and RSA signature length. 8c785b4 adds the tests that catch them.

  • Miri. Every portable unit test but the provider's runs under Miri, with new unit tests for erase, the wipes, a dropped Secret and ct. The heavy tests take a spread of their inputs under cfg(miri). A control is included: erase one byte past its array is reported as UB.

  • Constant-time harness. Guest programs crypto_constant_time and crypto_constant_time_curves (tests/guest/rust-tests/src/constant_time.rs):

    • fixed-versus-random, with the class drawn per measurement;
    • inputs made a block of 1,000 ahead of their measurements, and the same preparation for either class;
    • 1,000,000 measurements per routine, timed on the TSC behind CPUID at both ends (serializing on Intel and AMD; LFENCE orders on AMD only under DE_CFG bit 1, and QEMU's default CPU has no RDTSCP);
    • Welch's t over the raw set and dudect's 100 percentile crops; a routine passes when |t| < 4.5;
    • a null control (one distribution in both classes, must stay below 4.5) and a leaky control (a 16-byte early-exit tag comparison after a ChaCha20-Poly1305 seal, must exceed 10), at the start and end of each program. A run whose controls fail exits nonzero.

    Metal rows crypto_secret_timing and crypto_secret_timing_curves, the T14 and the AMD bench, each in its own boot. They are named-only (Arm::named_only, read by metal::select): a pass takes them by their whole name or their boot, never unfiltered and never by part of a name, as stage 4 keeps the result out of the suite. Their judge only requires that the job ran with its controls held. Each bound is the slower bench's measured job time plus half: the fast job took 970 s (T14) and 822 s (AMD), the curves job 7,036 s (T14) and 6,234 s (AMD).

Checks

The head is 9217793, on main d7f269d, which includes #997, #999 and #1000.

Evidence:

Costs

Measured in one session on the development Mac. The crate's three forms ran in 20 s wall and 15 s of CPU on main (5e01f8b); this branch takes 74 s wall and 396 s of CPU, almost all of it the portable form's curve arithmetic, ours and RustCrypto's. That will shrink with the speed work. GitHub's runner is unmeasured, because CI skips drafts. The repository gains about 17 MB of vector files.

Risks and open items

  • The T14 P-256 finding is open. It was measured on the old code and is re-measured on crypto: P-256 special reduction, Booth windows, joint ECDSA verify (parity item 8a) #1000's code by the staged boots.

    • The hypothesis, not established, is value-dependent instruction latency with DOITM off: the fixed scalar keeps the running total at the identity point.
    • If DOITM is the cause, whether to turn it on, always or for crypto work only, is the owner's decision, made with the cost row.
  • ML-KEM key generation is not timed. Its matrix expansion depends on rho, which is public but derived from the secret seed, so the two classes cannot share it.

  • Not done here:

    • typed domains (the speed work's);
    • module contracts naming secret inputs;
    • code inspection for secret-dependent branches and addresses;
    • the audit decision.
  • fiat-crypto is gone from toyos-crypto. Cargo.lock still names it:

    The issue's record holds in either landing order.

  • The GitHub runner's wall time for the host job is unmeasured, because CI skips drafts.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q

@Japabu

Japabu commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

Review r1 of #993 at ec5a0cd

Architecture

This PR is test-side only. No production line of toyos-crypto changes: the edits to ct, secret, mlkem and poly1305 are all #[cfg(test)]. The crate keeps no dependency except the optional rustls, and fiat-crypto has left its manifest. The oracles are dev-only.

The one harness change is Arm::adds_ms. It generalises members_ms and is clean.

The vector, integer-oracle, differential and fuzz layers are well built:

  • Wycheproof counts are held to numberOfTests.
  • Each case ID reproduces.
  • The independent decoders are RustCrypto, ring and dalek.
  • The 15 controls are red by exact test at 86e8642 (controls/head/summary-head.txt). I checked the patches' intent against the named tests.

NOTICE checks out. I recomputed the sha256 of all 56 vector and CAVP files, and every one matches NOTICE. The byte sizes match too. I spot-checked three files against upstream at the pinned commits: aes_gmac_test.json, mlkem_768_semi_expanded_decaps_test.json (Wycheproof 7a325932) and HMAC-SHA2-256-2.0 (ACVP-Server 975de31e). All three are byte-identical. COMMITTED_FILES gains exactly 52 rows: 33 Wycheproof, 4 ACVP HMAC and 15 CAVP.

The problems are in the constant-time harness and its registration, in a few accounting and cost claims, and in the fiat record.

Net: about 2,900 lines of test Rust, 312 lines of licence rows, about 24 MB of vectors. No production growth.

Findings

BLOCKER 1. The metal rows contradict the recorded design and enter every full metal pass. tests/toyos.rs:1364-1383, tests/common/metal.rs:170.

  • Stage 4's Record rule (landed in issues: toyos-crypto is in the tree, on par with ring, and secure (track) #913) says the constant-time result "is not a suite row, and it is rerun when the routine or the compiler changes". This PR registers two METAL rows and does not mention the change.
  • metal::select takes every row when no filter is given (all). Every unfiltered --metal pass now adds a boot of about 52 min and one of about 5.2 h to each bench, for rows whose judge decides nothing.
  • The filter word crypto also pulls them in beside crypto_instrument.

Two fixes are acceptable:

  • Return the change to the owner as a design change.
  • Make the rows opt-in by exact name, never selected by an unfiltered run, and state that in the issue.

BLOCKER 2. The harness confounds class with input preparation next to the clock. tests/guest/rust-tests/src/constant_time.rs:88-96; the header claim at :10.

  • once draws the class, runs make, then calls ticks() straight away. The Random class's make runs 32 to 64 SplitMix rounds and their stores. The Fixed class returns a constant.
  • LFENCE orders execution but does not drain the store buffer. Predictor, store-buffer and cache state therefore differ by class at the start of the timed region.
  • With 10^6 samples, an offset of a few cycles crosses |t| 4.5 on short routines. The header's "so only the routine is timed" is not established.
  • Inference, not proof: both exploratory runs flag only the shortest routines and pass every long one. On TCG that is ChaCha/OpenSSH, HMAC refusal and HKDF (qemu-tcg-tsc-20000.log). On the Mac it is ChaCha/OpenSSH and HMAC (mac-exploratory-fast-100k.log). A per-measurement constant offset produces exactly this pattern.

Fix:

  • Do it as dudect does: draw the classes and build the inputs for a block, for example 10,000, before timing any of them, and give both classes the same preparation work.
  • Add a null control to the same run: an A/A row where both classes come from one distribution, or a routine that is constant time by construction. It must stay below 4.5.

Without the null control, a LEAKS verdict on a bench cannot be told apart from a harness artefact. The leaky control only shows that the clock works.

BLOCKER 3. The metal bounds are not derived for the benches. tests/toyos.rs:1386-1401.

  • crypto_ct_ms scales a smoke run that was a debug build: ./target/debug/crypto_constant_time, 24.09 s and 145.30 s (mac-exploratory-*-20k.log). That run was on a loaded Mac. The guest runs [profile.toyos].
  • "four times that, for a bench that is slower" has no basis.
  • 1_011_000 / 31_000 truncates to 32, not 32.6.
  • The bench runs now in progress supply the recorded values. Set adds_ms from them with a stated margin before landing, and replace the comment with that derivation.

BLOCKER 4. The NIST "declared count" check is a tautology, and the PR claims more than it does. lib/crypto/tests/nist.rs:175, 221, 243.

  • report(file, Some(records.len())) compares the loop with the records it parsed. Every record goes to exactly one branch, so the comparison cannot fail.
  • A parser that merges two records loses cases silently. For example, a missing blank line overwrites one record's fields with the next in the BTreeMap.
  • SHAVS's message count and ACVP HMAC/ML-KEM's counts are not held at all (None).
  • The PR body says "The total is checked against the file's declared count" of every vector file. That is true only for Wycheproof.
  • Fix: hold each CAVP parse to an independent count, such as the Count = (GCM) or Msg = (SigVer/PKV) lines in the raw text, or the files' published totals (7,875, 1,125, 180). Otherwise narrow the claim to Wycheproof.

BLOCKER 5. Four committed vector files test nothing, or nothing a cheaper test lacks.

  • mlkem_768_semi_expanded_decaps_test.json: 9 cases, every one counted outside with no call made (wycheproof.rs:503).
  • rsa_signature_8192_{sha256,sha384,sha512}_test.json: 1.8 MB and 776 cases. Every case is "modulus outside 2048 to 4096 bits" (:261).
    • fuzz.rs::rsa_components already holds that bound: random 512- and 513-byte moduli straddle 4,096 bits.
    • Including these files also contradicts the selection rule the module header and NOTICE state. ML-KEM-512/1024, SHA-1 PSS and the like are excluded as other interfaces.
  • Delete the four files with their NOTICE and COMMITTED_FILES rows.

BLOCKER 6. The arithmetic-only differential and fuzz run three times over identical code. build/ci.rs:463 (CRYPTO_FORMS).

  • portable gates only src/arch/x86_64.rs. The curves, scalar25519, RSA and ML-KEM are the same code in every form. On an aarch64 host, "dispatched" and "portable" are identical throughout.
  • forms-1.log shows the differential at 12.7 to 14.1 s wall in each of the three forms, about 200 s of CPU each. Nearly all of it is curve arithmetic.
  • Run the seeded P-256, P-384, Ed25519, X25519, RSA and ML-KEM cases, and their decoders' fuzz, in one form. Keep AES-GCM, GHASH, ChaCha and SHA-2 in every form.
  • The cost record also contradicts itself, and part of it is unmeasured:
    • The PR says "up from about 2 s".
    • The issue (:386) says "against about 20 s before".
    • No baseline is in the evidence.
    • The GitHub runner's added wall time is unknown, because CI skipped the draft.
  • Measure main's baseline in the same session and record one number.

BLOCKER 7. The fiat-crypto record is inaccurate. Issue :315-321, PR body "Neither involves toyos-crypto".

  • curve25519-dalek 4.1.3 names fiat-crypto 0.2.9 behind its unset cfg. It is reached through toyos-crypto's own dev-dependencies x25519-dalek 2.0.1 and ed25519-dalek 2.2.0, not only through lib/update, so it does not go at stage 6.
  • curve25519-dalek 5.0.0-pre.6, under russh, also names fiat-crypto 0.3.0. The record does not mention it.
  • The owner's ruling is "no fiat code as a dependency of any kind". Correct the record so the owner can decide whether a lock entry that is never built is acceptable. If it is not, dropping the dalek oracles is one option: ring covers Ed25519, and an integer ladder or the RFC 7748 vectors cover X25519.

BLOCKER 8. The Miri status will be stale when it lands. Issue :390-393.

  • The record lists Miri under Done and says the full run "was still going at the time of writing".
  • The recorded exit is the portable configuration's unit tests. The run (miri-1.log, PID 49269) has spent 1 h 51 min in gcm::tests::counters_continue_across_the_stitched_and_tail_paths as of this review.
  • Before landing, either record its result, or reduce that test under cfg(miri) as poly1305 and test_arithmetic already do and record the result, or move the item to Open with its exit.

SUGGESTION A. Size the leaky control to the leaks the rows exist to catch. constant_time.rs:142.

  • The control compares 64 iterations against 1: 363 against 51 ticks on TCG, so |t| is about 59 at 20k samples. It proves the clock, not sensitivity.
  • A 16-byte early-exit compare after the same AEAD open work would match the refusal rows. So would one evidence run with ct::equal patched to exit early.
  • Optionally, run the control again at the end of the 5 h boot as well.

SUGGESTION B. Cite AMD as well. arch/counter.rs:5 cites only the Intel SDM. On AMD, LFENCE is dispatch-serialising only under DE_CFG (MSR C001_1029) bit 1, or where CPUID Fn8000_0021 EAX[2] says it always is (AMD APM vol. 3, RDTSC; AMD's speculation-management paper). The kernel does not set that bit. Confirm it for the AMD bench and cite the source.

SUGGESTION C. CAVP GCMVS decrypt costs much for little.

  • gcmDecrypt{128,256}.rsp are 5.75 MB and contribute 750 judged cases. Of each file's 7,875 cases, 5,250 are skipped for their IV, and 2,250 repeat the same Length refusal (vector-tallies.log).
  • The encrypt files alone meet "NIST ACVP or CAVP for AES-GCM". Wycheproof and the 10k differential already refuse tampered records.
  • SigVer.rsp is 730 KB for 90 judged cases. It is kept for P-384 SigVer, which Wycheproof also covers.

SUGGESTION D. Small readability fixes.

  • ct.rs:44: "over their whole range" is false, since the test checks a handful of values.
  • differential.rs:606: sig.pop().map(drop).unwrap_or(()) should be sig.pop();.
  • differential.rs:489: assert_eq!((ours, ours), (strict, ring)) reads better as two assertions.

Answers to the questions asked

  • Controlled inputs and meaningful failure:
    • Yes for the vectors (Wycheproof), the integer oracle, the differential, the fuzz, the controls and the Miri control.
    • No for the timing harness until B2 is fixed: its verdicts are not interpretable in either direction.
    • Partly for the NIST counts (B4).
  • Vector accounting: honest for Wycheproof. Tautological for CAVP and absent for ACVP (B4). Four files carry no check (B5).
  • Harness statistics: Welch's t with Welford updates, dudect's 100 crops, a per-measurement class draw and |t| 4.5 are sound and match the spec. The TSC between LFENCEs is right on Intel; AMD is open (S-B). The defect is the input-preparation confound (B2). The judge asserts only that the job passed, which implies the control was seen, and that it printed lines. That fits evidence-only rows, but not a suite registration (B1).
  • Size and cost: trim B5 (about 1.9 MB) now and consider S-C (about 6.5 MB). Cut the triple curve run (B6), which removes most of the added CPU without losing coverage.
  • Metal bounds: not justified (B3).
  • NOTICE / COMMITTED_FILES: correct as committed. They change with B5 and S-C.

SEND BACK

Japabu added a commit that referenced this pull request Oct 22, 2026
… apart

Review r1 of #993:
- The 8192-bit RSA files' every case was the one size refusal
  `fuzz::rsa_components` already holds, and the semi-expanded ML-KEM file
  made no call at all: both are other interfaces, as the module says of
  the files never taken. GCMVS's decrypt files gave 750 judged cases in
  15,750, refusals Wycheproof and the differential already hold; the
  encrypt files stay.
- Each CAVP file's records are held to a count taken from its text apart
  from the parse (`Count = `, `Len = `, `Msg = `, `Qx = ` and the Monte
  Carlo `MD = ` lines), and each ACVP file's cases to its `"tcId"`s, so a
  parser that merged two records reds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Japabu added a commit that referenced this pull request Oct 22, 2026
Review r1 of #993: `portable` changes only the instruction paths, so the
curves, scalar25519, RSA, ML-KEM, X25519, Ed25519 and WPA2's HMAC-SHA1
are the same code in every form, and their differential, fuzz and vector
tests ran three times for one result. They now run in the `portable`
form and are ignored, by name and reason, in the other two. AES-GCM,
GHASH, ChaCha20, Poly1305 and SHA-2 stay in every form.

Under Miri the heavy unit tests take a spread of their inputs, as
`poly1305` and `test_arithmetic` already did: GCM's lengths and GHASH's
bits, one ML-KEM key generation, and fewer arithmetic edge pairs. The
readability points of the review are taken too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Japabu added a commit that referenced this pull request Oct 22, 2026
…ized counter

Review r1 of #993, and the first bench boots: every routine read LEAKS on
both benches, the leaky control at |t| ~7,000, which fits a confound of
the harness rather than of the crate.

- Inputs are made a block of 1,000 at a time before any of them is
  timed, and each maker draws the same random bytes for either class, so
  what lies between two counter reads is the routine alone. The class
  draw is still per measurement.
- A null control, both classes one distribution, runs at the start and
  the end of each program and must stay below |t| 4.5; the leaky control,
  now a 16-byte early-exit tag comparison after a ChaCha20-Poly1305 seal,
  must exceed 10 at both. A run whose controls fail exits nonzero, its
  verdicts all printed.
- The counter is read behind `CPUID` at both ends, which serializes on
  Intel and AMD alike; `LFENCE` orders execution on AMD only under DE_CFG
  bit 1, which this kernel does not set, and QEMU's default CPU has no
  `RDTSCP`.
- The two metal rows are named-only: a pass takes them by their whole
  name or their boot, never unfiltered and never by a part of the name,
  as stage 4 keeps the result out of the suite (`metal::select`).
- The bounds come from the benches: the fast job took 968 s on the T14
  and 823 s on the AMD bench; the curves job is derived from the T14's
  X25519 time.

Under TCG at 20,000 measurements both programs end cleanly, the leaky
control at |t| 10 to 22 and the null control below 2.1, and no routine
reads LEAKS; TCG's timings judge nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Japabu added a commit that referenced this pull request Oct 22, 2026
The record now states what holds and what it cost: 43 Wycheproof files and
NIST's held to independent counts, the arithmetic-only tests in one form
with main's cost beside this one's, the complete Miri run, the timing
rows named-only, and the first bench boots read as the harness confound
they were. The fiat-crypto record names every lock entry and how it is
reached: the dalek 4.1.3 and 5.0.0-pre.6 entries sit behind a `cfg`
nothing sets, but russh's `p384` 0.14.0-rc.8 compiles fiat code into
sshserver today.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

Response to review r1. The head is 7a36687; the code is at 9022dcc. Evidence: https://github.com/ToyOSOrg/evidence/tree/187abe7793116cbbbfb4dec63098032b1b8d61de/pr-993/r2

  1. Rows enter every pass. Fixed, as the orchestrator ruled: the rows are named-only. metal::select gains named_only, and METAL_NAMED_ONLY lists both rows. A pass takes one only by its whole name or its boot, never unfiltered and never by part of a name (crypto, secret_timing). metal_words_take_rows_members_and_whole_boots holds this, and --metal --list was checked unfiltered and with crypto. The issue's Record rule now says so.

  2. Class confounded with preparation. Fixed:

    • Inputs are made a block of 1,000 ahead of their measurements, and every maker draws the same random bytes for either class (secret, the scalar! draw).
    • A null control (A/A) and a leaky control run at the start and end of each program. Either failing exits nonzero.
    • Under TCG at 20k the null control stays below 2.1, the leaky control reaches 10–22, and every routine passes. The previous TCG run had flagged the short routines.
    • The bench boots at ec5a0cd read LEAKS everywhere. They are recorded as the confound, not as findings.
  3. Bounds. Fixed. Fast: 968 s on the T14 and 823 s on AMD, measured, plus half. Curves: derived from the T14's measured X25519 time × 34, the curves-to-X25519 ratio on the Mac, which is exploratory. Plus half. The comment states this, and the smoke-scaling function is gone.

  4. NIST counts. Fixed. Each CAVP file is held to a line count taken apart from the parse (Count = , Len = , the Monte MD = , Msg = , Qx = ), and ACVP to its "tcId"s.

  5. Dead files. Deleted, with their NOTICE and COMMITTED_FILES rows: the three RSA-8192 files and semi-expanded ML-KEM. S-C taken too: the GCMVS decrypt files are gone. m02's exact test is now wycheproof aead, still red.

  6. Triple runs. Fixed. The arithmetic-only differential, fuzz and vector tests run in the portable form and are ignored by name elsewhere. Cost, measured in one session: main 20 s wall / 15 s CPU across the three forms; this branch 74 s / 396 s. One number, now in the issue and the PR.

  7. fiat record. Corrected.

    • curve25519-dalek 4.1.3 comes via toyos-crypto's x25519/ed25519-dalek oracles and lib/update.
    • russh's curve25519-dalek 5.0.0-pre.6 is now named.
    • Both sit behind an unset cfg.

    New and real: russh's p384 0.14.0-rc.8 depends on fiat-crypto unless p384_backend = "bignum", so fiat code is compiled into sshserver today. That is on main, not this PR. It is recorded, and it goes with russh.

  8. Miri. Done. The heavy tests take a spread of their inputs under cfg(miri). The full run passes at eddf262, whose crate source is the head's: 25 passed, 1 ignored under Miri, provider filtered (ring is C). 2,968 s.

Suggestions:

  • A. Taken: the leaky control is a 16-byte early-exit tag comparison after a ChaCha20-Poly1305 seal, and it runs again at the end of each program.
  • B. Taken differently: CPUID serializes on both vendors, so the counter is read behind it at both ends. RDTSCP is absent on QEMU's default CPU (SIGILL in the smoke), so it is not used.
  • C. Taken: the GCMVS decrypt files are gone, and SigVer stays.
  • D. Taken.

Controls: all 15 are red at 9022dcc against their exact tests. cargo run -- --ci host passes at 9022dcc (87 steps). Both timing boots are staged again for both benches.

@Japabu

Japabu commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

Review r2 of #993: code at 9022dcc, head 7a36687

This round checks the r1 required items and reviews only the code changed since r1. I read the full diff ec5a0cd..7a36687 and the r2 evidence (ToyOSOrg/evidence@187abe77, pr-993/r2), and I ran nothing.

r1 required items

  1. Rows enter every full metal pass: fixed. metal::select takes a named-only row only by its whole name or by its boot. tests/checks/metal.rs holds this: no word, a word that is part of the name, the whole name, and a boot word. The issue now records the rows as named-only, which is the second option r1 offered. One gap remains in the mechanism; see N1.
  2. Class confounded with preparation: fixed.
    • measured builds a BLOCK of 1,000 (class, input) pairs before timing any of them.
    • secret() draws the random bytes for both classes. The curve scalar! macro runs the draw-until-valid loop and from_bytes(1) for both classes. ML-KEM decapsulation draws its ciphertext for both classes.
    • Only run sits between the two counter reads. record (the crop loop) runs after stop().
    • The null control (one distribution in both classes) and the leaky control (a 16-byte early-exit compare behind the same seal) run at the start and the end. finish exits nonzero if either fails.
    • The confounded bench boots at ec5a0cd bear r1 out: every short routine read LEAKS, including AES-GCM on the T14 (bench-t14-ec5a0cd18-confounded.txt). Under TCG at r2, the null control reads 1.2 to 2.1, the leaky control 10.1 to 22.0, and every routine is below 4.5.
    • Reading the counter behind CPUID is right for both vendors. The comment cites Intel's SDM and AMD's APM for the reason, and says why LFENCE and RDTSCP are not used.
  3. Bounds: fixed for the fast job, pending for the curves job.
    • CRYPTO_CT_MS is the measured 968 s (T14) plus half. The fixed harness adds two controls and a CPUID per read. That is a few seconds against 968, well inside the margin.
    • CRYPTO_CT_CURVES_MS (about 4.0 h) is derived from routine ratios and has not been measured. That is acceptable only until the run now on the benches finishes; see N2.
    • A correction to r1's wording: [profile.dev] is opt-level = 2, so the Mac smoke under target/debug was optimised, not a debug build. r1's conclusion stands, since a Mac ×4 is not a derivation for a bench, and it is now replaced in any case.
  4. NIST counts were tautological: fixed.
    • The CAVP files are held to raw-text line counts: Count = for GCM, Msg = for SigVer, Qx = for PKV.
    • SHAVS is held to Len = for messages and MD = for Monte Carlo checkpoints.
    • ACVP HMAC and ML-KEM are held to "tcId" occurrences.
    • None of these goes through the parser, so a merged or lost record now fails.
  5. Vector files that tested nothing: fixed. The RSA-8192 and semi-expanded ML-KEM files are gone, and so are the GCMVS decrypt files (r1's suggestion C). Their NOTICE entries and COMMITTED_FILES rows (6 × 6 lines) went with them, and git grep finds no remaining reference outside issues/. The Wycheproof module header now names the excluded classes accurately.
  6. The triple curve run: fixed. Every test with no instruction path is ignored outside portable. The cost evidence is now one session on the Mac: main 20 s wall / 15 s CPU, branch 74 s / 396 s. The issue states those numbers. See the cost judgement below.
  7. The fiat record: fixed, with a new finding. The record now names curve25519-dalek 4.1.3 through toyos-crypto's own dalek oracles, and russh's 5.0.0-pre.6. It also records the new finding: russh's p384 0.14.0-rc.8 compiles fiat-crypto into sshserver today, with its exit at stage 2 of the SSH server issue. That record is accurate. Whether a fiat build in a shipped program until then is acceptable is the owner's decision, and the orchestrator has it.
  8. Miri: fixed. miri-2.log shows --all-features --lib --skip rustls_provider, exit 0, 25 passed. One test is ignored (the 3329² exhaustive one) and 11 provider tests are filtered (ring is C). The cfg(miri) reductions in gcm, mlkem and test_arithmetic thin the inputs without removing any path. The issue records the run as done.

Host-test cost

The added cost is acceptable for routine host CI, and I would not trim further in this PR.

  • In the actual --ci host run (ci-host-3.log), the three crypto steps took about 11 s, 62 s and 9 s by their timestamps, about 82 s of a 28-minute run (about 5%).
  • Nearly all of it is the portable form's differential: 39 s wall, the 10,000 seeded curve cases per primitive that stage 4's recorded design sets. The fuzz adds 5 s and Wycheproof 9.5 s.
  • [profile.dev] is already opt-level = 2, so no profile change would buy anything.
  • The cost should fall with the speed work, which is where the curve arithmetic belongs.
  • One measurement is missing: the GitHub runner's wall time for the host job. The draft skipped CI, and the runner has far fewer cores than the M4 Pro. Record it when the PR leaves draft. If it turns out to be material, the next trim is the case count for the curve primitives alone. That changes a recorded design number, so it is the owner's decision, not a review demand.

New findings

N1. BLOCKER: the named-only list can go silently inert. tests/toyos.rs:1390, tests/common/metal.rs:174,194.

  • METAL_NAMED_ONLY is a string list apart from METAL, and select never checks that each entry names a row.
  • If either row is renamed, or an entry is mistyped, that row quietly becomes an ordinary row again, and every unfiltered metal pass takes the multi-hour boot. tests/CLAUDE.md calls this kind of field this suite's worst defect class.
  • Fix: make it a field of metal::Metal (for example named_only: bool) so the type carries it. Alternatively, have select refuse an entry that names no row, and test that refusal in words_take_rows_members_and_whole_boots.

N2. REQUIRED BEFORE LANDING: the curves bound must be measured. tests/toyos.rs:1397-1402. When the run in progress finishes, set CRYPTO_CT_CURVES_MS from the measured job time of the slower bench plus the same half, and replace the derivation comment with that measurement. If the run hits the derived bound, that is a finding about the derivation; do not just widen it. This is a number and comment update, not another design round.

Suggestion (optional). The null control draws the same 16 random bytes for both classes, so it never exercises the fixed-class data path through secret(). A second null that uses secret(class, rng, fixed) with a routine that only black_boxes its input would also cover that path. The leaky control already exercises it in one direction.

SEND BACK, for N1 alone. N2 is a value from the run already in progress. Once N1 is fixed and N2 is recorded, I expect LAND without another full round.

Japabu added a commit that referenced this pull request Oct 22, 2026
…ecorded

Review r1 of #1005. `--ci host` compiled sshserver, and so fiat-crypto,
for the host test and checked it for Linux and Windows: a build for a
triple with no `[target]` table took no flags from the config. `[build]
rustflags` is what cargo applies to exactly those builds, and the ToyOS
tables, whose flags replace it, carry the cfg as well.

The SSH track records the compromise: until stage 2, sshserver's P-384
(client `ecdsa-sha2-nistp384` verification before authentication, and an
operator's P-384 host key) runs on the backend upstream calls
experimental. The config comment no longer cites the crypto issue, whose
record of the ruling lands with #993.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q
@Japabu
Japabu force-pushed the wt/toyos-crypto-sec branch from 7a36687 to 0ce19e2 Compare October 22, 2026 10:53
Japabu added a commit that referenced this pull request Oct 22, 2026
… apart

Review r1 of #993:
- The 8192-bit RSA files' every case was the one size refusal
  `fuzz::rsa_components` already holds, and the semi-expanded ML-KEM file
  made no call at all: both are other interfaces, as the module says of
  the files never taken. GCMVS's decrypt files gave 750 judged cases in
  15,750, refusals Wycheproof and the differential already hold; the
  encrypt files stay.
- Each CAVP file's records are held to a count taken from its text apart
  from the parse (`Count = `, `Len = `, `Msg = `, `Qx = ` and the Monte
  Carlo `MD = ` lines), and each ACVP file's cases to its `"tcId"`s, so a
  parser that merged two records reds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Japabu added a commit that referenced this pull request Oct 22, 2026
Review r1 of #993: `portable` changes only the instruction paths, so the
curves, scalar25519, RSA, ML-KEM, X25519, Ed25519 and WPA2's HMAC-SHA1
are the same code in every form, and their differential, fuzz and vector
tests ran three times for one result. They now run in the `portable`
form and are ignored, by name and reason, in the other two. AES-GCM,
GHASH, ChaCha20, Poly1305 and SHA-2 stay in every form.

Under Miri the heavy unit tests take a spread of their inputs, as
`poly1305` and `test_arithmetic` already did: GCM's lengths and GHASH's
bits, one ML-KEM key generation, and fewer arithmetic edge pairs. The
readability points of the review are taken too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Japabu added a commit that referenced this pull request Oct 22, 2026
…ized counter

Review r1 of #993, and the first bench boots: every routine read LEAKS on
both benches, the leaky control at |t| ~7,000, which fits a confound of
the harness rather than of the crate.

- Inputs are made a block of 1,000 at a time before any of them is
  timed, and each maker draws the same random bytes for either class, so
  what lies between two counter reads is the routine alone. The class
  draw is still per measurement.
- A null control, both classes one distribution, runs at the start and
  the end of each program and must stay below |t| 4.5; the leaky control,
  now a 16-byte early-exit tag comparison after a ChaCha20-Poly1305 seal,
  must exceed 10 at both. A run whose controls fail exits nonzero, its
  verdicts all printed.
- The counter is read behind `CPUID` at both ends, which serializes on
  Intel and AMD alike; `LFENCE` orders execution on AMD only under DE_CFG
  bit 1, which this kernel does not set, and QEMU's default CPU has no
  `RDTSCP`.
- The two metal rows are named-only: a pass takes them by their whole
  name or their boot, never unfiltered and never by a part of the name,
  as stage 4 keeps the result out of the suite (`metal::select`).
- The bounds come from the benches: the fast job took 968 s on the T14
  and 823 s on the AMD bench; the curves job is derived from the T14's
  X25519 time.

Under TCG at 20,000 measurements both programs end cleanly, the leaky
control at |t| 10 to 22 and the null control below 2.1, and no routine
reads LEAKS; TCG's timings judge nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Japabu added a commit that referenced this pull request Oct 22, 2026
The record now states what holds and what it cost: 43 Wycheproof files and
NIST's held to independent counts, the arithmetic-only tests in one form
with main's cost beside this one's, the complete Miri run, the timing
rows named-only, and the first bench boots read as the harness confound
they were. The fiat-crypto record names every lock entry and how it is
reached: the dalek 4.1.3 and 5.0.0-pre.6 entries sit behind a `cfg`
nothing sets, but russh's `p384` 0.14.0-rc.8 compiles fiat code into
sshserver today.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Japabu added a commit that referenced this pull request Oct 22, 2026
…ured

Review r2 of #993, N1: `METAL_NAMED_ONLY` was a list of names apart from
the rows, so a renamed or mistyped row would quietly become an ordinary
one and every unfiltered pass would take its hours-long boot. The arm now
says it (`Arm::named_only`): an arm already carries what its jobs cost
the boot (`adds_ms`), and every arm is built from `once`, so no other row
changes. A row is named-only when one of its arms is.

N2: the bounds are the slower bench's measured job time plus half, from
the boots at 7a36687: the fast job 970 s (T14) and 822 s (AMD), the
curves job 7,036 s (T14) and 6,234 s (AMD).

The issue records those boots' verdicts: everything passes on AMD; on
the T14 both P-256 routines leak, at #993's base before #990.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Japabu and others added 14 commits October 22, 2026 14:07
…ctors

Stage 4's independent vectors. Wycheproof's `testvectors_v1/` at the
commit stage 1 pinned (7a325932) gains every file for an interface the
crate offers: GMAC, AES key wrap, PBKDF2- and HMAC-SHA1 (WPA2), ECDH over
SEC 1 points, ECDSA with each SHA-2 the provider pairs with a curve, RSA
PKCS #1 at 2048, 3072, 4096 and 8192 bits, PSS at each salt and MGF1 hash
the files hold, and ML-KEM-768 by seed. The ASN.1, PEM, JWK and P1363
files, other algorithms, curves and hashes, and RSA signature generation
are other interfaces and stay out.

Every case is judged against the file's result or counted outside the
interface by a named reason, and the count is held against the file's
`numberOfTests`. Outside means refused by an error where the interface
can be handed the input (a 15-byte tag, an 8192-bit modulus, a PSS
signature valid under another salt), or by a type that cannot carry it
(a 64-bit IV, AES-192). A PSS signature invalid under the file's salt but
made with TLS's ("s_len changed to 32") is counted, not judged.

NIST's: CAVP's SHAVS files for SHA-384 beside SHA-256's and SHA-512's in
`tests/cavp`, now read by this crate too; GCMVS's AES-128 and AES-256
files; FIPS 186-4's ECDSA SigVer and PKV; and ACVP's HMAC-SHA-1/256/384/512
at the ML-KEM files' commit. ML-KEM's ACVP test moves to `tests/nist.rs`.
Each file is byte for byte, pinned by sha256 in NOTICE and a
COMMITTED_FILES row, which the `tests/cavp` files now all have.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…crypto

The owner rejected fiat-crypto (2026-10-21): ToyOS keeps its own crypto
stack, and no fiat code is a dependency of any kind. Poly1305's oracle,
its one use, becomes `num-bigint`, already the crate's test-only integer
oracle, and the dev-dependency goes.

`test_arithmetic` becomes a harness over one trait, `Field`, whose
implementations alone name the crate's types: field25519 and the four
NIST Montgomery moduli today, and a new representation of the same field
by implementing it. Each is held against integers on:
- every pair of boundary values: 0, 1, 2, p - 1, p - 2, p's halves, and
  each 51- and 64-bit limb's 2^k - 1, 2^k, p - 2^k and p - 2^k + 1, with
  inputs from p up where the decoder takes them;
- seeded values against seeded partners;
- inversion as a^(p-2), zero included;
- a chain that feeds every result back as the operation left it, so an
  output's limb bounds are an input's;
- distributivity and (x - y) + y = x.
The Ed25519 group order is held the same way through `S`: reduction of
any width, S < L, and the signing operations.

ML-KEM's reduction modulo 3329 is checked over every pair of coefficients,
and Compress and Decompress over every input at the widths ML-KEM-768
uses, against FIPS 203 §4.2.1's rounding. Iteration counts drop under
Miri, which reads the same tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stage 4's differential, against ring and RustCrypto as dev-dependencies
only. Each primitive takes 10,000 cases, each drawn from a generator
seeded by its number, so a red names a case that reproduces whichever
thread took it; the cases spread over every CPU.

- SHA-2, HMAC and HKDF against ring, whole and streamed in random pieces,
  a changed tag refused.
- AES-128/256-GCM and ChaCha20-Poly1305 against ring and RustCrypto, and
  one change to ciphertext, tag or AAD that both refuse, leaving the
  ciphertext as it was.
- OpenSSH's construction against RustCrypto's ChaCha20 and Poly1305.
- X25519 against dalek on random u-coordinates, values from p up and the
  high bit included.
- Ed25519 against ring and dalek's `verify_strict`: ring's signatures
  verified, a bit flipped, S raised by the order, the message changed;
  signing against ring's; and random encodings decoded where dalek's are.
- P-256 and P-384 key derivation and ECDH against RustCrypto, and
  verification of its signatures, changed or the other S, against ring.
- RSA PKCS #1 v1.5 and PSS, 2048 and 4096 bits, against ring's verifier,
  over a pool of ring's signatures changed six ways.
- ML-KEM-768 against RustCrypto, implicit rejection included.
- WPA2's HMAC-SHA1 and 802.11 PRF against ring; the PMK, 8,192 HMAC
  computations each, at 200 cases.

The small differential tests these supersede go; the length sweeps stay.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stage 4's fuzz, 4,000 cases per decoder as `build/ssh/tests/fuzz.rs`
does: a valid encoding bent along its grammar, or random bytes, and an
independent decoder's verdict.

- SEC 1 points: the other y, coordinates at, past and below p, x + p,
  compressed and other tags; RustCrypto's decoder, uncompressed only.
- DER signatures: tags, lengths, long-form lengths, integer padding and
  sign, r or s at 0, n - 1, n and n + 1, the other S, trailing bytes;
  ring's verifier.
- RSA components: held to the interface's contract, and an accepted key
  verifies only where it is the signer's.
- Ed25519 encodings near 0 and p, each sign: dalek's decoder, canonical
  and not weak.
- ML-KEM-768 keys with coefficients past q, and ciphertexts changed or
  cut: RustCrypto's modulus check and decapsulation.
- The rustls provider's key shares for every group, against the shares'
  own decoders and the zero X25519 result; its `RSAPublicKey` DER against
  ring's; and its records cut, changed or opened at another sequence
  number.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What Miri reads of the crate's `unsafe`: `erase` over sizes around its
16-byte stores, alignments below theirs, nested arrays and none at all;
`wipe`, `wipe64` and `wipe32` over slices; a dropped `Secret`'s storage;
and `ct`'s masks, selection, `opaque` and `equal` over their ranges and a
difference at every byte.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stage 4's negative controls mutate each check a forgery would pass. Two
mutations left every test green, so two tests that would have seen them:

- SEC 1 decoding refuses a coordinate at or past p. A residue can be
  encoded above p only where it is small, below 2^256 - p, which a random
  point never is. The fuzz now takes the point with the least x, on
  P-256 and P-384, and encodes x or x + p.
- RSA verification refuses a signature that is not the modulus's length
  (RFC 8017 §8.2.2 step 1). A signature whose first byte is zero, without
  it or with another zero before it, is the same integer: refused, as
  ring refuses it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…crets

Stage 4's constant-time harness, a guest program for the benches: for
each secret routine a consumer calls, a million measurements with the
class drawn per measurement, one class holding a fixed special secret
and the other a fresh uniform one, the public inputs the same in both.
Welch's t over the raw set and dudect's 100 percentile crops of a
calibration run; a routine passes below |t| = 4.5. Each input is made
before the clock starts.

A deliberately leaky control runs first in each program: a byte-at-a-time
comparison that stops at the first difference. It must exceed |t| = 10,
or the program fails, since that run could see no leak. Every other
verdict is a printed line, evidence for the track issue rather than a
suite verdict.

Two programs, in two boots: `crypto_constant_time` (the AEADs' and
OpenSSH's sealing and refusal, HMAC, HKDF, X25519, ML-KEM-768
encapsulation and decapsulation of valid against rejected ciphertexts)
and `crypto_constant_time_curves` (P-256 and P-384 key derivation and
agreement, Ed25519 signing), which takes a bench for hours. ML-KEM key
generation is not timed: its matrix expansion depends on rho, a public
output of the secret seed, which no two classes can share.

A metal arm now says what its jobs add to its list's bound
(`Arm::adds_ms`), as a shared boot's members do; each constant-time arm
derives its bound from a smoke run's CPU time on the development Mac,
scaled to the measurement count, four times over. The shared boot skips
both programs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An AEAD case is one record rather than ten arguments, a committed RSA
key a struct, and its DER exists only where the provider reads it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A QEMU smoke run of both programs, at 2,000 measurements, timed with
`Instant` and its clock-page arithmetic, saw nothing of the leaky
control: |t| 0.01 and 4.36, so both runs failed as they should. Each
measurement now reads the time-stamp counter between `LFENCE`s
(`arch/counter.rs`, with AArch64's virtual counter beside it), and at
20,000 measurements under TCG the control reads |t| 59 and 66 and both
programs end cleanly. TCG's timings judge nothing.

The metal rows are `crypto_secret_timing` and
`crypto_secret_timing_curves`: a row and a shared member under one name
are two verdicts under one name, which the suite refuses.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The track's stage 4 replaces its fiat-crypto oracle with the integer one
the owner chose (2026-10-21) and records what the test-side items hold:
the vectors, the oracle, the differential, the fuzz, the negative
controls and Miri, with their evidence; the constant-time harness built
and waiting on the benches; ML-KEM key generation not timed, and why;
and what stays open. `tests/cavp` gains its second reader.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… apart

Review r1 of #993:
- The 8192-bit RSA files' every case was the one size refusal
  `fuzz::rsa_components` already holds, and the semi-expanded ML-KEM file
  made no call at all: both are other interfaces, as the module says of
  the files never taken. GCMVS's decrypt files gave 750 judged cases in
  15,750, refusals Wycheproof and the differential already hold; the
  encrypt files stay.
- Each CAVP file's records are held to a count taken from its text apart
  from the parse (`Count = `, `Len = `, `Msg = `, `Qx = ` and the Monte
  Carlo `MD = ` lines), and each ACVP file's cases to its `"tcId"`s, so a
  parser that merged two records reds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review r1 of #993: `portable` changes only the instruction paths, so the
curves, scalar25519, RSA, ML-KEM, X25519, Ed25519 and WPA2's HMAC-SHA1
are the same code in every form, and their differential, fuzz and vector
tests ran three times for one result. They now run in the `portable`
form and are ignored, by name and reason, in the other two. AES-GCM,
GHASH, ChaCha20, Poly1305 and SHA-2 stay in every form.

Under Miri the heavy unit tests take a spread of their inputs, as
`poly1305` and `test_arithmetic` already did: GCM's lengths and GHASH's
bits, one ML-KEM key generation, and fewer arithmetic edge pairs. The
readability points of the review are taken too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ized counter

Review r1 of #993, and the first bench boots: every routine read LEAKS on
both benches, the leaky control at |t| ~7,000, which fits a confound of
the harness rather than of the crate.

- Inputs are made a block of 1,000 at a time before any of them is
  timed, and each maker draws the same random bytes for either class, so
  what lies between two counter reads is the routine alone. The class
  draw is still per measurement.
- A null control, both classes one distribution, runs at the start and
  the end of each program and must stay below |t| 4.5; the leaky control,
  now a 16-byte early-exit tag comparison after a ChaCha20-Poly1305 seal,
  must exceed 10 at both. A run whose controls fail exits nonzero, its
  verdicts all printed.
- The counter is read behind `CPUID` at both ends, which serializes on
  Intel and AMD alike; `LFENCE` orders execution on AMD only under DE_CFG
  bit 1, which this kernel does not set, and QEMU's default CPU has no
  `RDTSCP`.
- The two metal rows are named-only: a pass takes them by their whole
  name or their boot, never unfiltered and never by a part of the name,
  as stage 4 keeps the result out of the suite (`metal::select`).
- The bounds come from the benches: the fast job took 968 s on the T14
  and 823 s on the AMD bench; the curves job is derived from the T14's
  X25519 time.

Under TCG at 20,000 measurements both programs end cleanly, the leaky
control at |t| 10 to 22 and the null control below 2.1, and no routine
reads LEAKS; TCG's timings judge nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The record now states what holds and what it cost: 43 Wycheproof files and
NIST's held to independent counts, the arithmetic-only tests in one form
with main's cost beside this one's, the complete Miri run, the timing
rows named-only, and the first bench boots read as the harness confound
they were. The fiat-crypto record names every lock entry and how it is
reached: the dalek 4.1.3 and 5.0.0-pre.6 entries sit behind a `cfg`
nothing sets, but russh's `p384` 0.14.0-rc.8 compiles fiat code into
sshserver today.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Japabu and others added 3 commits October 22, 2026 14:07
…ured

Review r2 of #993, N1: `METAL_NAMED_ONLY` was a list of names apart from
the rows, so a renamed or mistyped row would quietly become an ordinary
one and every unfiltered pass would take its hours-long boot. The arm now
says it (`Arm::named_only`): an arm already carries what its jobs cost
the boot (`adds_ms`), and every arm is built from `once`, so no other row
changes. A row is named-only when one of its arms is.

N2: the bounds are the slower bench's measured job time plus half, from
the boots at 7a36687: the fast job 970 s (T14) and 822 s (AMD), the
curves job 7,036 s (T14) and 6,234 s (AMD).

The issue records those boots' verdicts: everything passes on AMD; on
the T14 both P-256 routines leak, at #993's base before #990.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…measure

russh's `p384` is on its `bignum` backend under #1005, so the fiat record
names the dependency as one no guest build selects. The T14's P-256
finding is re-measured on #1000's arithmetic, now on main, with a boot
that sets DOITM beside the one that does not.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqpxbTfpvcbx1FJEy3Eny9
#1005 sets `p384_backend = "bignum"` for host and guest builds alike, so
the record says that and what held before it, true in either landing
order.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q
@Japabu

Japabu commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

Response to review r2. The head is 9217793, rebased onto main d7f269d (#997, #999 and #1000 included).

  • N1. Fixed. Named-only is now a field of the arm (Arm::named_only), so a renamed or mistyped row cannot fall out of a separate list. The fixture sets it on beta_row; clearing it turns metal_words_take_rows_members_and_whole_boots red (controls/n1.log). Main added its own Arm::adds_ms in the meantime, and the branch keeps main's.
  • N2. Fixed. Both bounds are now the slower bench's measured time plus half. The fast job took 970 s on the T14 and 822 s on AMD, the curves job 7,036 s and 6,234 s; the comments state the measurements.
  • Suggestion (second null). Not taken this round.

The bench verdicts from 7a36687 are recorded in the issue: everything passes except P-256 on the T14 (old code). The re-measure on #1000's code, with a DOITM probe beside it, is staged. Miri is complete again on the new crypto code (44 passed). All 15 controls are red at d79698f. Evidence: https://github.com/ToyOSOrg/evidence/tree/a58ea8eb194c15f8274383cb620f7c426a0b726e/pr-993/r4

@Japabu

Japabu commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

Review r3 of #993 at 9217793 (on main d7f269d)

This is a focused round. I checked the r2 items, the rebase onto #997, #999 and #1000, the new issue record and the evidence (ToyOSOrg/evidence@a58ea8eb, pr-993/r4, and @18e8243a, pr-993/r3). I ran nothing and did not reopen settled points.

r2 items

  • N1: fixed. Arm::named_only is a field on the arm. once defaults it to false, and select reads it from the row's arms, so there is no separate list left to fall out of sync. The fixture sets it on beta_row. controls/n1.log flips it to false and shows metal_words_take_rows_members_and_whole_boots red, with an unfiltered run taking alpha_row,beta_row. Main's adds_ms is kept, not duplicated.
  • N2: fixed. CRYPTO_CT_MS = 1_455_000 is 970 s × 1.5, and CRYPTO_CT_CURVES_MS = 10_554_000 is 7,036 s × 1.5. Both are the T14's job CPU time as the kernel's exit line records it (cpu=970142ms, cpu=7036130ms). The AMD bench is faster, at 822 s and 6,234 s. These were measured at 7a36687, on the old NIST code. crypto: P-256 special reduction, Booth windows, joint ECDSA verify (parity item 8a) #1000's Booth windows and crypto: Ed25519 signing and X25519 keys by a fixed-base comb (parity item 7) #999's comb make the curves faster, so the bound remains an upper bound for the boots now running.

Rebase

  • Oracle port: correct, and nothing was lost. test_arithmetic now uses Tight, Residue<M, N> and Modulus in place of main's own oracle tests. I compared it with main's version item by item:
    • Main's checks are all still there:
      • the M::ONE, M::R2 and N0 constants;
      • from_integer refusing p and above, now over every boundary value;
      • from_wide of all-ones (bound - 1 through from);
      • inversion;
      • neg, mul_small and pow_p58;
      • loose operands at their largest into mul, square and mul_small;
      • the reachable-limb bounds test;
      • PublicScalar::reduce, reduce32 and mul_add;
      • multiples of L near 2^512.
    • It adds the boundary-pair products and the unreduced chains.
    • The comment "below 2p for these moduli" on from_wide's input holds, because all four moduli exceed 2^(64N−1).
  • The Ed25519 scalar API is correct. PublicScalar::canonical returns the scalar itself, and the test checks its value, not just whether it is Some. Controls m10 (PublicScalar::canonical) and m11 (Point::decode) were re-derived for crypto: Ed25519 verify by one joint multiplication, scalars by Barrett (parity items 6, E3, E4) #997's code and are red by their exact tests at d79698f.
  • Fiat record: correct in either landing order. #1005 sets that for every build, host and guest; before it, fiat compiled into sshserver is true whichever of the two lands first. sshserver: russh's p384 on its own bignum backend, host and guest, so no fiat-crypto compiles in #1005's diff sets --cfg=p384_backend="bignum" in all three rustflags entries and touches a different issue, so the two do not conflict.

Evidence

  • --ci host at 9217793 (ci-host-5.log, .rev): exit 0, 93 steps. The three crypto forms now take 5, 11 and 5 s, down from 62 s for portable at r2, because crypto: Ed25519 verify by one joint multiplication, scalars by Barrett (parity items 6, E3, E4) #997 and crypto: P-256 special reduction, Booth windows, joint ECDSA verify (parity item 8a) #1000 made the curve arithmetic faster. That settles the cost question.
  • At d79698f (ci-host-4, r4-crypto-*, controls/rev, miri-4.rev):
    • all three forms exit 0;
    • all 15 controls exit 101, each failing its exact test;
    • Miri --all-features --lib --skip rustls_provider exits 0: 44 passed, 1 ignored, 11 filtered, in 3 h 08.
  • The reuse is valid, but the stated reason is too broad. I confirmed that git diff d79698fbe 9217793d2 is empty for lib/crypto, the four timing-harness files and tests/guest/rust-tests/Cargo.toml. The only Cargo.lock hunks are in osbench and the kernel. However, the README's statement "nothing under lib/crypto or tests/guest changed" is not literally true. Main's rebase brought in osbench and several rust-tests binaries. None of that can affect the crate tests, controls or Miri. Correct the sentence to name what is actually unchanged; this does not block landing.

Findings

BLOCKER 1. The issue names an agent as an owner. issues/toyos-crypto-is-in-the-tree-on-par-with-ring-and-secure.md:426: "Owner: crypto-perf". That is a coordination label, not a durable owner. Root CLAUDE.md says engineering records must stand on their own without private planning notes. Name the owner the way other issues do, for example "this track's parity stage, the NIST arithmetic (lib/crypto/src/nist.rs)".

BLOCKER 2. A failing verdict is called "marginal". Same file, line 425: "P-384 agreement reads 5.79, marginal". The pass bound is |t| < 4.5, so 5.79 is LEAKS by the harness's own rule, and the bench line says LEAKS. State it as a T14 finding next to P-256, and widen the exit to "a T14 pass of P-256 and P-384 agreement on the current NIST code".

Suggestion. secret.rs's drop test covers Secret but not main's Workspace, which carries the same erase on every return path. One more ManuallyDrop case would hold it.

Both blockers are wording edits to the issue. They invalidate no evidence and need no rerun. A commit that makes only those edits can land without another review round.

SEND BACK

…s a durable owner

Review r3 of #993. |t| 5.79 against the 4.5 bound is LEAKS by the
harness's own rule, so P-384 agreement joins P-256 in the T14 finding,
and the exit asks a T14 pass of both. The owner is this track's parity
stage, the NIST arithmetic, not a coordination label.

The drop test also holds `Workspace`, which carries the same erase on
every return path: without its `erase` it reads all ones.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q
@Japabu

Japabu commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

Response to review r3, head e37f115.

  • Blocker 1: the finding's owner is now "this track's parity stage, the NIST arithmetic (lib/crypto/src/nist.rs)".
  • Blocker 2: P-384 agreement is recorded as LEAKS (max |t| 5.79, raw t 5.77), next to P-256. The exit is "a T14 pass of P-256 and P-384 agreement on the current NIST code".
  • Suggestion: taken. a_dropped_secret_leaves_zeros also holds a Workspace. With its erase made a no-op, the test fails with all ones left. Miri passes on the secret:: tests.
  • Evidence: the README now names exactly which paths are unchanged since d79698f. https://github.com/ToyOSOrg/evidence/tree/84ce94af15503ec7cc3272ddc9d56b3462660b13/pr-993/r4

@Japabu
Japabu marked this pull request as ready for review October 22, 2026 12:44
@Japabu
Japabu added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit c2fa263 Oct 22, 2026
8 checks passed
@Japabu
Japabu deleted the wt/toyos-crypto-sec branch October 22, 2026 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant