diff --git a/CLAUDE.md b/CLAUDE.md index 2c10445..521f85c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,9 +127,9 @@ Variables: (`cfg/c64-https-$(BACKEND).cfg`; default ip65). Changing it requires `make clean` — see above. - `USE_X25519_SIBLING=1` — swap the in-tree X25519 for the - `libs/x25519@v0.6.0` sibling. **Does not link - on either backend at present** — both - overflow, differently; see "Known issues" + `libs/x25519@v0.11.0` sibling. **UCI links; + ip65 overflows CRYPTO_OVERLAY** — see + "Known issues". Off by default either way. - `EMBED_P256_OVERLAY=1` — stage the P-256 verify image into the CRYPTO_OVERLAY slot at PRG-load (UCI; mutually exclusive with USE_X25519_SIBLING / @@ -176,10 +176,9 @@ buffers in the crypto BSS — see per-module headers for details): X25519 / field arithmetic Default: in-tree `src/crypto/{x25519,fe25519}.s`. - Opt-in: sibling `libs/x25519@v0.6.0` via `make USE_X25519_SIBLING=1` - — **currently unbuildable on both backends**, and the pinned - v0.6.0 additionally carries an upstream correctness bug; see - Known issues before relying on either fact. + Opt-in: sibling `libs/x25519@v0.11.0` via `make USE_X25519_SIBLING=1` + — links under UCI from the v0.10.1/v0.11.0 wave pins, still + overflows under ip65, and ships in nothing; see Known issues. The v0.6.0 pin is c64-lib-contract-aligned (SPEC §8.1) and adds the bank-2 drop + RAM-reclaim work; older v0.4.0 pin is historical only. Sibling and in-tree both expose the same ABI: @@ -194,7 +193,7 @@ buffers in the crypto BSS — see per-module headers for details): SHA-256 (in-tree; no sibling) sha256_init, sha256_update, sha256_final - ECDSA P-256 (`libs/nistcurves@v0.9.1` sibling, + ECDSA P-256 (`libs/nistcurves@v0.10.1` sibling, c64-lib-contract SPEC §1-§8.1 aligned) ecdsa_verify_256 — TLS dispatcher in src/crypto/ecdsa_verify.s packs the BE struct + calls the sibling entry @@ -766,63 +765,91 @@ Five latent bugs and three new ones were cleared to get here: overflow and is fixed (see the CRYPTO_COLD_SHADOW entry under "Memory layout"). - **`USE_X25519_SIBLING=1` links on NEITHER backend** (re-measured - 2026-08-13 at f0127a0, fresh submodules, cc65 from homebrew). The - two failures are different and independent: + **`USE_X25519_SIBLING=1` LINKS UNDER UCI from the v0.10.1 / + v0.11.0 wave pins** (issue #112), and still does not under ip65. + Both backends previously died on the same symbol collision; that + half is closed, and what remains on ip65 is a fit problem: + make BACKEND=uci USE_X25519_SIBLING=1 # 62,977 B PRG, links make USE_X25519_SIBLING=1 # ip65 - X25519_RODATA overflows CRYPTO_OVERLAY by 2048 bytes - LIB_NISTCURVES_P256_CODE overflows CRYPTO_RESIDENT by 103 bytes - - make BACKEND=uci USE_X25519_SIBLING=1 # UCI - LIB_NISTCURVES_P256_CODE overflows CRYPTO_HOT by 381 bytes - - make BACKEND=uci # control: links clean - - Earlier revisions of this file called UCI "the supported - sibling-on path"; that was wrong — nothing links the sibling - today, and no shipped artifact contains it. The UCI overflow is - simply the sibling's larger code claim: sibling CRYPTO_CODE is - 4,207 B (fe25519 2,711 + x25519 698 + x25519_init 798) against the - in-tree pair's 2,769 B (fe25519 2,093 + x25519 676), i.e. +1,438 B - into a CRYPTO_HOT with ~1,057 B of slack. ip65 additionally has - only a 4,212 B `CRYPTO_OVERLAY` (vs UCI's 7.5 KB) to hold - `X25519_RODATA` (2,304 B) + `X25519_BSS` (1,536 B) on top of - TLS_CODE + CRYPTO_AUX_CODE. - - **The pinned v0.6.0 also carries an upstream correctness bug.** - c64-x25519 #64 (fixed in v0.7.0): `x25519_scalarmult` returns - deterministically wrong results for a peer u-coordinate with bit - 255 set, across v0.4.0-v0.6.0. v0.4.0 stopped writing the RFC 7748 - `decodeUCoordinate` mask back into `x25_u` but left the ladder's - `z_3 = x_1 * (DA-CB)^2` site reading the unmasked buffer, so - x1 = x3 + 19 (mod p). **The in-tree implementation is NOT - affected** — `src/crypto/x25519.s` still writes the mask back - (`sta x25_u+31`), so its x_1 read sees the masked value; - `tools/test_x25519.py --slow` RFC 7748 vector 2 (whose u ends - `0x93`, bit 255 set) PASSes on the in-tree build, 73/73. - That same vector would catch the sibling bug the moment the - sibling links, so **do not flip the sibling default at the v0.6.0 - pin** — bump to >= v0.7.0 first. - - Upstream is at v0.8.0. A bump is not a one-line submodule move: - v0.8.0 renames `CODE`/`DATA` to `LIB_X25519_CODE`/`LIB_X25519_DATA` - and adds `LIB_X25519_INIT_CODE`, all three of which the consumer - cfgs must declare (`LIB_X25519_INIT_CODE` must be the last - file-emitting segment before any bss-type segment in a file-backed - area), and v0.7.0's #64 fix adds an `x25_x1` buffer that - `build_x25519.sh`'s hand-written BSS stub does not yet export. - v0.8.0 also ships an `X25519_ONCHIP_MUL` profile with no REU - surface at all (`LIB_X25519_REU_BANKS_USED = 0`, no - `reu_fetch_mul_row` export) — which would dissolve the - `USE_NISTCURVES_ONCHIP` / `USE_X25519_SIBLING` mutual exclusion at - `Makefile:90`, whose stated reason is that both archives export - that symbol. Note that v0.8.0's headline - `LIB_X25519_RESIDENT_BYTES` drop (9224 -> 8383) is **not** a - shrink: 826 B of it simply moved to the reclaimable - `LIB_X25519_COLD_BYTES`, and our wrapper stages only three of the - sibling's sources anyway, so upstream manifest deltas do not - subtract from the overflows above. + Segment 'X25519_RODATA' overflows memory area + 'CRYPTO_OVERLAY' by 3584 bytes + + The collision was `ld65: Error: Duplicate external identifier: + 'reu_mul_tables_init'` — both libraries shipped a SPEC §8.2 + `reu_mul` provider, and c64-https pulled both: under + `USE_X25519_SIBLING=1` `src/boot.s` *imports* `reu_mul_init` + (the sibling owns the table), so ld65 pulls nistcurves' + `reu_mul_init.o` to satisfy it because `nistcurves-p256.a` + precedes `x25519.a` on the link line, and then the sibling's own + provider arrives anyway via `reu_clear_wide`. The remedy is one + line in `tools/integration/build_nistcurves_p256.sh`: drop + `reu_mul_init.o` alongside the `mul_8x8.o` / `data_shared.o` + drops already there. c64-https is the §8.0 APP_OWNED case for + that primitive, so the member is surplus in every configuration — + and provably inert in the shipped ones, since all four REU-profile + PRGs are byte-identical with and without the drop. + + Note the near miss: had ld65 resolved instead of erroring, + `reu_mul_init` would have bound to nistcurves' table builder + rather than the sibling's — a different routine writing through a + different buffer set, with no diagnostic. + + **This was a semantic merge collision, not a regression in either + change, and PR #102 should not be read as having been wrong.** Its + evidence that the sibling links was honest on its own branch: at + 76d876c the nistcurves pin was still v0.6.0 + (`git ls-tree 76d876c libs/nistcurves` -> 00d2626), and at v0.6.0 + no *archive* carries `reu_mul_tables_init`. The symbol exists in + that tree — `src/main.s:254` exports it — but `main.o` is + deliberately excluded from every `lib-*` archive target + (Makefile:223), and c64-https links archives only. Upstream #81 + later moved the provider out of the never-archived driver into + `src/reu_mul_init.s` precisely so it would ship to consumers, + which is what put it in our link at v0.9.1. Meanwhile the + nistcurves bump (f87d76d) landed on a parallel branch that is not + an ancestor of 76d876c + (`git merge-base --is-ancestor f87d76d 76d876c` -> false), so + neither branch could see the collision; it existed only on merged + master. + + (Archaeology contributed by the deferred-followups lane. The + "v0.6.0 does not export it at all" phrasing that came with it is + too strong and would not survive a grep — the export is right + there in main.s. The load-bearing fact is *never archived*.) + + ip65's remaining overflow is structural: its `CRYPTO_OVERLAY` is + 4,212 B against UCI's 7,680 B, and it already holds TLS_CODE + + CRYPTO_AUX_CODE before the sibling's `X25519_RODATA` (2,304 B) + + `X25519_BSS` (1,536 B) + the ladder/init code the Makefile routes + there arrive. Fixing it is a cfg restructure, not a flag. + + **The flag stays OFF by default.** Nothing ships the sibling, and + flipping the default is a separate decision that wants a hardware + handshake behind it, not just a link and a KAT. (The old warning + against flipping at the v0.6.0 pin — upstream #64, wrong result + for a peer u with bit 255 set — is moot at v0.11.0, which is well + past the v0.7.0 fix. The in-tree implementation never had that + bug: `src/crypto/x25519.s` writes the RFC 7748 mask back to + `x25_u+31`, so its x_1 read sees the masked value, and + `tools/test_x25519.py` RFC 7748 vector 2 — whose u ends `0x93` — + passes 73/73 on the in-tree build.) + + The pin is now **v0.11.0**. The v0.6.0 -> v0.10.0 migrations the + wrapper had to absorb (the §4 `LIB_X25519_CODE` / + `LIB_X25519_INIT_CODE` segment renames, the `x25_x1` buffer added + by v0.7.0's #64 fix) are done and documented in + `tools/integration/build_x25519.sh`'s header. v0.11.0 itself is a + pure export-surface change — upstream states the library PRG is + byte-identical to v0.8.0 — so it needed no wrapper work at all. + + The `USE_NISTCURVES_ONCHIP` / `USE_X25519_SIBLING` mutual + exclusion at `Makefile:90` still stands, and its stated reason is + still true: both archives export `reu_fetch_mul_row`. Upstream's + `X25519_ONCHIP_MUL` profile (`LIB_X25519_REU_BANKS_USED = 0`, no + `reu_fetch_mul_row`) would dissolve it, but our wrapper stages + sources directly rather than calling `make -C libs/x25519`, so + adopting that profile is wrapper work, not a flag. `tools/integration/build_x25519.sh` is the integration wrapper. It does **not** call `make -C libs/x25519` — it stages three sibling @@ -902,6 +929,49 @@ Five latent bugs and three new ones were cleared to get here: proven rather than assumed. Verify from the link if you ever doubt it: `build/labels.txt` must show `fp_mul_i=$39`, `fp_mul_j=$3A`, `zp_ptr2=$3D`. + + **The slot is spelled `nistcurves_zp_ptr2` from v0.10.0.** The + §6.5 rename window made the four general-purpose scratch slots + canonically `nistcurves_zp_{tmp1,tmp2,ptr1,ptr2}` and left the + bare `zp_*` names as aliases — but the alias *assignment* is not + `.ifndef`-guarded, so the old `-D zp_ptr2=$3d` stopped being an + override and became `zp_config.s(56): Error: Symbol 'zp_ptr2' is + already defined`. That is the good outcome for once: this class + of mistake is normally silent. The od65 post-check now reads the + canonical name, because the bare one disappears under + `-D LIB_NO_BARE_EXPORTS=1` and a guard that can go vacuous under a + build-tightening flag is worse than no guard. + - **`poly_prod_lo` / `poly_prod_hi` are a RENDEZVOUS, not private + scratch — and under `USE_NISTCURVES_ONCHIP` the sibling owns + them.** Two bytes, and getting them wrong is silent wrong crypto + with no link error anywhere. The onchip row generator + `og_common` (in the sibling's `mul_8x8_onchip.o`) does + `jsr ct_mul_8x8` — which under `SHARED_CT_MUL_8X8` resolves to + c64-https's body in `src/crypto/poly1305.s` — and then reads the + product back out of `poly_prod_lo/hi`. Writer and reader must + address the *same* two bytes. + + Through the v0.9.1 pin upstream gated those bytes with the + `ct_mul_8x8` body, so the sibling had none and the wrapper's glue + TU `.import`ed c64-https's pair. libs/nistcurves v0.10.0 moved + them **outside** the gate (contract v0.9.1 adopter-private-buffer + rule: `fp_sqr`'s diagonal path writes them with no `ct_mul_8x8` + involved), which surfaced as + `mul_8x8.s(223): Error: Symbol 'poly_prod_lo' is already an + import`. The fix inverts ownership under this profile only: the + glue TU drops the import, and `poly1305.s` imports the sibling's + pair instead of defining its own. Had the glue simply dropped the + import without the consumer-side change, the link would have + SUCCEEDED with two disjoint pairs, `og_common` would have read + two zero bytes for every product, and every on-chip-generated + multiply row would have been wrong. + + Guarded behaviourally, not structurally: `tools/test_ecdsa_kat_oracle.py` + on an onchip build is the test that would catch it (6/6 including + 3 negative CAVP vectors; a zeroed row set fails the 3 positive + ones). Run it against **an onchip PRG specifically** after any + change in this area — the REU-profile build does not link + `mul_8x8_onchip.o` at all and will pass regardless. - **The v0.7.0/v0.8.0 UCI `CRYPTO_HOT` overflow is RESOLVED at v0.9.1 — and no cfg change was needed.** Recorded here because the obvious remedy was nearly taken and would have been the wrong diff --git a/README.md b/README.md index 3db2d76..92b650b 100644 --- a/README.md +++ b/README.md @@ -276,7 +276,7 @@ Progress: - **The handshake is slow, and the ECDSA P-256 verify dominates it.** Every figure here is quoted from the measurement record in `CLAUDE.md` and was taken at the **`libs/nistcurves` v0.6.0 pin**; HEAD is v0.9.1 and nobody has re-run the sweep, so treat them as a baseline rather than as HEAD. End-to-end handshake + GET against the local listener, U64E, master 2ceb5b1: **80.8 s** (REU profile, 48 MHz), **45.5 s** (onchip profile, 48 MHz), **1,157.7 s** (REU, stock 1 MHz). On the REU-less stock-C64 path (ip65 + onchip, no REU, honest 1 MHz in VICE) the whole run measured **2,159.7 s = 36.0 min**, of which the verify stretch alone was 1,416.7 s. That is fine for the local listener, which holds the connection open; it exceeds a typical 10-30 s real-world server handshake window. - **P-384** ECDSA is stubbed at the TLS layer. The dispatcher advertises `ecdsa_secp384r1_sha384` (0x0503) and routes to `src/crypto/ecdsa_verify_384.s`, but no P-384 build target completes. Measured 2026-08-14: `make p384-overlay` from a clean tree stops at `No rule to make target 'build/labels.txt'`, and after a main build has produced that file it stops at `Segment 'LIB_NISTCURVES_SHA384_TABLES' overflows memory area 'OVERLAY_REGION' by 1536 bytes`. Cert chains requiring P-384 will not verify. -- **`USE_X25519_SIBLING=1` does not link on either backend** at the current `libs/x25519` pin. Measured 2026-08-14: `make clean && make USE_X25519_SIBLING=1` and the `BACKEND=uci` equivalent both die with `ld65: Error: Duplicate external identifier: 'reu_mul_tables_init'`. The in-tree X25519 in `src/crypto/{x25519,fe25519}.s` is what every shipped artifact contains. +- **`USE_X25519_SIBLING=1` now links under UCI, and still does not under ip65.** The duplicate-symbol failure this entry used to record — `ld65: Error: Duplicate external identifier: 'reu_mul_tables_init'`, on **both** backends — was closed by the `libs/nistcurves` v0.10.1 / `libs/x25519` v0.11.0 bump plus one line of archive surgery: `tools/integration/build_nistcurves_p256.sh` now also drops `reu_mul_init.o`, the SPEC §8.2 `reu_mul` provider that `src/boot.s` supplies itself. Measured at those pins: `make clean && make BACKEND=uci USE_X25519_SIBLING=1` produces a 62,977 B PRG, and ip65 stops instead at `Segment 'X25519_RODATA' overflows memory area 'CRYPTO_OVERLAY' by 3584 bytes` — a placement problem (ip65's overlay slot is 4,212 B against UCI's 7,680 B), not a symbol collision. The flag remains **off by default** and no shipped artifact contains the sibling; the in-tree X25519 in `src/crypto/{x25519,fe25519}.s` is what every release PRG is built from. Flipping the default is a separate decision that wants a hardware handshake behind it. - **Live internet HTTP GET (UCI backend)** has not been re-verified since the FPGA-fence rework; only the local multi-segment listener is exercised regularly. - **VICE 3.9** previously appeared to crash on chained HMAC-SHA256 calls (backend-independent — affects the crypto-only test suites), but this was caused by hardcoded port numbers bypassing the test harness port allocator. With proper `ViceInstanceManager` usage (no hardcoded ports), all N=1..10 chained calls succeed reliably. diff --git a/libs/nistcurves b/libs/nistcurves index f9701e1..1edd634 160000 --- a/libs/nistcurves +++ b/libs/nistcurves @@ -1 +1 @@ -Subproject commit f9701e1d35c664d494139db4cc7d7a3f62bd3c50 +Subproject commit 1edd6348a2d30db5db239ed835d7610b777421d9 diff --git a/libs/x25519 b/libs/x25519 index 68ae0ef..e9af04e 160000 --- a/libs/x25519 +++ b/libs/x25519 @@ -1 +1 @@ -Subproject commit 68ae0eff065cb02324742265c466cbb4f7801c69 +Subproject commit e9af04ec1a69c6b25a1c938fb0967bcfb38319c3 diff --git a/src/crypto/poly1305.s b/src/crypto/poly1305.s index 992c471..7e8f026 100644 --- a/src/crypto/poly1305.s +++ b/src/crypto/poly1305.s @@ -15,12 +15,45 @@ .import poly_h, poly_r, poly_s, poly_product, poly1305_tag .import aead_scratch +; --- poly_prod_lo / poly_prod_hi ownership (c64-lib-contract v0.9.1 +; "adopter-private buffer" rule) --- +; +; These two bytes are the a*b -> 16-bit output of the §8.3 `ct_mul_8x8` +; body below, and ordinary scratch for anything else that wants a 16-bit +; product slot. Exactly ONE module in the link may define them, and every +; writer and reader must agree on which — they are a rendezvous, not +; private state, so a second copy is not a duplicate-symbol error but a +; silent wrong answer. +; +; Under USE_NISTCURVES_ONCHIP the owner is the sibling's +; `mul_8x8_onchip.o`, and it must be: from libs/nistcurves v0.10.0 that +; object defines poly_prod_lo/hi OUTSIDE the SHARED_CT_MUL_8X8 gate, +; because its own `fp_sqr` diagonal-squaring path writes them +; independently of ct_mul_8x8 (upstream's comment at +; libs/nistcurves/src/mul_8x8.s:207-222). Its `og_common` row generator +; then does `jsr ct_mul_8x8` — resolving to the body below — and reads +; the result back out of poly_prod_lo/hi. If this file kept its own pair, +; og_common would read two zero bytes on every product and every +; on-chip-generated multiply row would be wrong, with no link diagnostic +; anywhere. So we import instead: one pair of bytes, in +; LIB_NISTCURVES_MUL_CODE, written and read by both sides. +; +; Under every other profile the sibling's mul_8x8 object is dropped from +; the archive entirely, nothing else defines them, and this file owns +; them as before. +.ifdef USE_NISTCURVES_ONCHIP +.import poly_prod_lo +.import poly_prod_hi +.endif + ; --- Exports --- .export poly1305_init .export poly1305_clamp .export sqtab_init +.ifndef USE_NISTCURVES_ONCHIP .export poly_prod_lo .export poly_prod_hi +.endif .export mul_8x8 .export poly1305_multiply .export poly1305_reduce @@ -33,8 +66,10 @@ ; ============================================================================= .segment "CRYPTO_BSS" +.ifndef USE_NISTCURVES_ONCHIP poly_prod_lo: .res 1 poly_prod_hi: .res 1 +.endif mul_a: .res 1 mul_b: .res 1 diff --git a/src/crypto/shared/mul_tables.s b/src/crypto/shared/mul_tables.s index 803db77..3468af4 100644 --- a/src/crypto/shared/mul_tables.s +++ b/src/crypto/shared/mul_tables.s @@ -53,3 +53,42 @@ mul_tables_init: sqtab_lo: .res 512 sqtab_hi: .res 512 .endif + +; ============================================================================= +; c64-lib-contract SPEC §6.5 — canonical names for the shared multiply +; buffers c64-https provides. +; +; c64-https owns these four buffers (src/data.s under the default build; +; the sibling's data module under USE_X25519_SIBLING=1) and the +; integration wrapper therefore DROPS libs/nistcurves' `data_shared.o` +; from the archive — two providers of the same RAM would be a duplicate +; symbol at best and two disjoint copies at worst. +; +; From libs/nistcurves v0.10.0 the library's own objects reference these +; buffers by their §6.5 canonical names (`nistcurves_mul_*`), because +; `mul_` is registered to c64-x25519 in the §2 prefix registry. Upstream +; keeps the bare `mul_*` spellings as same-address aliases *inside* +; data_shared.o — which is exactly the object we drop — so at the v0.10.1 +; pin the link fails with four unresolved externals: +; +; Unresolved external 'nistcurves_mul_cached_a' referenced in: +; src/fp256.s(188) ... (likewise _dma_hi, _dma_lo, _src2_buf) +; +; The aliases below close that. They are deliberately *here* rather than +; beside either definition site: `src/data.s` declares the buffers only +; under `.ifndef USE_X25519_SIBLING`, and the sibling's generated data +; module declares them otherwise, so a definition-site alias would have to +; be written twice and kept in step. Importing the bare name binds to +; whichever provider the link selected, with no duplication and no gating. +; +; When upstream drops the bare `mul_*` aliases at its next MAJOR, the +; migration is to rename the definitions and delete this block — not to +; add a second set of buffers. + .import mul_dma_lo, mul_dma_hi, mul_cached_a, mul_src2_buf + .export nistcurves_mul_dma_lo, nistcurves_mul_dma_hi + .export nistcurves_mul_cached_a, nistcurves_mul_src2_buf + +nistcurves_mul_dma_lo = mul_dma_lo +nistcurves_mul_dma_hi = mul_dma_hi +nistcurves_mul_cached_a = mul_cached_a +nistcurves_mul_src2_buf = mul_src2_buf diff --git a/src/lib_contract_asserts.s b/src/lib_contract_asserts.s index f00636b..a5f5189 100644 --- a/src/lib_contract_asserts.s +++ b/src/lib_contract_asserts.s @@ -136,13 +136,47 @@ APP_OWNED = LIB_SHARED_PRIMITIVES_SQTAB | LIB_SHARED_PRIMITIVES_REU_MUL | LIB_SH ; load-bearing breakage gate — pre-1.0 libraries take breaking changes ; on MINOR bumps, so MAJOR carries no signal. ; -; nistcurves shipped 0 from v0.3.0 through v0.8.0 and bumped to 1 at -; v0.9.0, where it also REMOVED 17 exported symbols (c64-nist-curves -; #90/#91). If a submodule bump makes this assert fire, that is the gate -; working: re-check the integration against the new export surface, then -; update the expected value on the next line — do not delete the assert. +; nistcurves shipped 0 from v0.3.0 through v0.8.0, bumped to 1 at v0.9.0 +; (17 exported symbols REMOVED, c64-nist-curves #90/#91), and to 2 at +; v0.10.0 (the lib-contract phase-3 namespace wave, c64-nist-curves #103). +; If a submodule bump makes this assert fire, that is the gate working: +; re-check the integration against the new export surface, then update the +; expected value on the next line — do not delete the assert. +; +; THE v0.9.1 -> v0.10.1 RE-CHECK, so the next person can audit the audit +; rather than re-run it blind. Method: `git diff v0.9.1 v0.10.1 -- src/` +; filtered to `.export`/`.exportzp` lines, cross-checked against every +; import of c64-https's own objects (od65 --dump-imports over build/*.o, +; build/crypto/**, build/net/**, excluding build/lib staging). +; +; Removed, unconditionally: LIB_SHARED_REU_MUL_BANK, +; LIB_SHARED_REU_MUL_OFFSET, LIB_SHARED_REU_MUL_BANKS_USED — the three +; unprefixed §8.2 consumer-INPUT equates. c64-https imports **none** of +; them (grep count 0 over the import dump), and could not usefully have: +; every §8.2 consumer defines the same three, which is precisely why +; exporting them produced `ld65: Duplicate external identifier` in any +; two-library link. Replaced by the prefixed OUTPUT counterparts +; LIB_NISTCURVES_SHARED_REU_MUL_* (#105), which c64-https also does not +; import — it owns its REU layout in src/crypto/shared/reu_layout.inc. +; +; Everything else in the wave is ADDITIVE at the default gate: the §6.5 +; rename window (#107) adds canonical `nistcurves_zp_*` / +; `nistcurves_mul_*` names while KEEPING the bare forms as same-address +; aliases, export-gated behind -D LIB_NO_BARE_EXPORTS=1. c64-https links +; ungated, so `mul_dma_lo` / `mul_dma_hi` / `mul_cached_a` / +; `mul_src2_buf` — which src/data.s provides and boot.s imports — are +; untouched. The functional surface c64-https actually consumes is the +; same four symbols as at v0.9.1: ec_base_x, ec_gx256, ec_scalar_mul_var, +; ecdsa_verify_256, all still exported. +; +; One INPUT-side break is real and was migrated: `-D zp_ptr2=$3d` in +; tools/integration/build_nistcurves_p256.sh now hard-errors, because +; the bare alias assignment is no longer `.ifndef`-guarded. The override +; is spelled `-D nistcurves_zp_ptr2=$3d` from this pin; the wrapper's +; od65 post-check was retargeted to the canonical name so it cannot go +; vacuous. See that script's ZP_OVERRIDES block. .import LIB_ABI_VERSION -.assert LIB_ABI_VERSION = 1, lderror, "libs/nistcurves: exported-surface generation changed (LIB_ABI_VERSION != 1) — re-check the integration, then bump the expected value in src/lib_contract_asserts.s" +.assert LIB_ABI_VERSION = 2, lderror, "libs/nistcurves: exported-surface generation changed (LIB_ABI_VERSION != 2) — re-check the integration, then bump the expected value in src/lib_contract_asserts.s" ; ===================================================================== diff --git a/tools/integration/build_nistcurves_p256.sh b/tools/integration/build_nistcurves_p256.sh index 488179d..1ddcebc 100755 --- a/tools/integration/build_nistcurves_p256.sh +++ b/tools/integration/build_nistcurves_p256.sh @@ -88,14 +88,31 @@ CA65="${CA65:-ca65}" AR65="${AR65:-ar65}" # --- ZP-slot overrides (c64-https canonical map) --- -# zp_ptr2 = $3D : library default $fd collides with c64-https zp_temp/zp_count -# used by der_decode.s during cert parsing. +# nistcurves_zp_ptr2 = $3D : library default $fd collides with c64-https +# zp_temp/zp_count used by der_decode.s during cert parsing. # fp_mul_i = $39, fp_mul_j = $3A : library defaults $2c/$2d collide with # c64-https fe25519 ZP claim ($2c-$37). $39-$3a is otherwise # unused inside ZP_CRYPTO. # Other slots match upstream defaults — see libs/nistcurves/src/zp_config.s. +# +# THE POINTER SLOT IS SPELLED `nistcurves_zp_ptr2` FROM v0.10.0, AND THE +# OLD SPELLING NOW HARD-ERRORS. The §6.5 rename window (upstream #107) +# made the four general-purpose scratch slots canonically +# `nistcurves_zp_{tmp1,tmp2,ptr1,ptr2}` and left the bare `zp_*` names as +# unconditional same-address aliases: +# +# .ifndef nistcurves_zp_ptr2 +# nistcurves_zp_ptr2 = $fd +# .endif +# zp_ptr2 = nistcurves_zp_ptr2 <- NOT .ifndef-guarded +# +# so `-D zp_ptr2=$3d` no longer suppresses a guarded definition; it +# collides with the alias assignment and stops the build with +# `zp_config.s(56): Error: Symbol 'zp_ptr2' is already defined`. That is +# the good case — a loud failure, not a silently-dropped override. The +# `fp_*` names are documented override knobs and were not renamed. ZP_OVERRIDES=( - '-D' 'zp_ptr2=$3d' + '-D' 'nistcurves_zp_ptr2=$3d' '-D' 'fp_mul_i=$39' '-D' 'fp_mul_j=$3a' ) @@ -172,12 +189,40 @@ check_zp_slot() { exit 1 fi } -check_zp_slot zp_ptr2 61 # $3d -check_zp_slot fp_mul_i 57 # $39 -check_zp_slot fp_mul_j 58 # $3a +# Check the CANONICAL name. The bare `zp_ptr2` alias is also exported in a +# default build and carries the same value, but it disappears under +# `-D LIB_NO_BARE_EXPORTS=1` (SPEC §6.5) and is removed outright at the +# next MAJOR — checking it would make this guard silently vacuous exactly +# when a consumer tightens the build. +check_zp_slot nistcurves_zp_ptr2 61 # $3d +check_zp_slot fp_mul_i 57 # $39 +check_zp_slot fp_mul_j 58 # $3a # --- 4. Drop conflicting members / rebuild the onchip mul object --- -rm -f "$STAGING/mul_8x8.o" "$STAGING/data_shared.o" +# `reu_mul_init.o` is the SPEC §8.2 `reu_mul` provider. c64-https is the +# §8.0 APP_OWNED case for that primitive — src/boot.s::reu_mul_init builds +# the 128 KB REU multiply table itself — so the library's provider is +# surplus in every configuration. Dropping it is the archive-surgery +# spelling of `-D SHARED_REU_MUL_INIT` (the wrapper cannot pass that +# define: upstream's Makefile builds every module with one recipe, which +# is why step 3 rebuilds rather than reconfigures). +# +# It is a NO-OP for the shipped builds and load-bearing for one that is +# not shipped yet. Default build: boot.o defines `reu_mul_init` and +# `reu_fetch_mul_row` itself, so ld65 has no undefined symbol this member +# could satisfy and never pulls it — the four REU-profile PRGs are +# byte-identical with and without the drop (measured). Onchip archives +# never contained it. But under `USE_X25519_SIBLING=1`, boot.o *imports* +# `reu_mul_init` (the sibling owns the table), ld65 pulls this member to +# satisfy it because nistcurves-p256.a precedes x25519.a on the link line, +# and then the sibling's own provider arrives via `reu_clear_wide` — +# `ld65: Error: Duplicate external identifier: 'reu_mul_tables_init'`, +# the failure README.md and CLAUDE.md both recorded as unconditional. +# Note the near miss: had ld65 resolved instead of erroring, `reu_mul_init` +# would have bound to the library's table builder rather than the +# sibling's, which is a different routine writing through a different +# buffer set. +rm -f "$STAGING/mul_8x8.o" "$STAGING/data_shared.o" "$STAGING/reu_mul_init.o" # 4b. onchip-comb: the full onchip archive carries both curves + SHA-384 + # reference-inverse extras; keep only the P-256 comb verify set. if [ "$PROFILE" = "onchip-comb" ]; then @@ -194,17 +239,39 @@ if [ "$PROFILE" = "onchip" ] || [ "$PROFILE" = "onchip-comb" ]; then # would collide with the in-tree providers (see header comment #3). # # Upstream gap (candidate c64-nist-curves issue): the og_common block - # references ct_mul_8x8 / smc_* / poly_prod_* as same-TU symbols, so + # references ct_mul_8x8 / smc_* as same-TU symbols, so # SHARED_CT_MUL_8X8 alone leaves them undefined — the guard combo was # never exercised upstream. Bridge it with a generated glue TU that # declares the .imports and then .includes the PRISTINE library source # (composition, not a source patch — libs/ stays untouched). + # + # THE IMPORT LIST SHRANK AT v0.10.1, AND SHRINKING IT WAS MANDATORY. + # Upstream moved three of the five symbols this glue used to supply + # into its own source, so re-declaring them is now a hard assemble + # error rather than a harmless duplicate: + # + # poly_prod_lo / poly_prod_hi defined unconditionally at + # mul_8x8.s:223-225, deliberately OUTSIDE the SHARED_CT_MUL_8X8 + # gate (contract v0.9.1 adopter-private-buffer rule: fp_sqr's + # diagonal path writes them with no ct_mul_8x8 involved). + # Importing them here now yields + # `mul_8x8.s(223): Error: Symbol 'poly_prod_lo' is already an + # import`. The consumer side moved to match: under + # USE_NISTCURVES_ONCHIP, src/crypto/poly1305.s IMPORTS this + # object's pair instead of defining its own, so og_common's + # `jsr ct_mul_8x8` and its poly_prod read-back address the same + # two bytes. See the ownership comment in poly1305.s — getting + # this wrong is silent wrong crypto, not a link error. + # mul_cached_a upstream imports the §6.5 canonical + # `nistcurves_mul_cached_a` itself (mul_8x8.s:30); the bare name + # is aliased for it in src/crypto/shared/mul_tables.s. + # + # ct_mul_8x8 and the two SMC bake sites are still ours to supply: + # upstream declares no import for them under SHARED_CT_MUL_8X8. cat > "$STAGING/mul_8x8_onchip_glue.s" <<'EOF' ; generated by build_nistcurves_p256.sh (onchip profile) — do not edit .import ct_mul_8x8 .import smc_sum_a_imm, smc_diff_a_imm -.import poly_prod_lo, poly_prod_hi -.import mul_cached_a .include "mul_8x8.s" EOF "$CA65" \ diff --git a/tools/test_x25519.py b/tools/test_x25519.py index d7b3ac3..7381365 100644 --- a/tools/test_x25519.py +++ b/tools/test_x25519.py @@ -632,7 +632,13 @@ def run_tests(transport, labels, seed): total_passed = 0 total_failed = 0 - test_groups = [ + # A USE_X25519_SIBLING=1 link contains no src/crypto/fe25519.s, so its + # private fe_* entry points are absent and these nine groups cannot + # run. main() has already verified that the absence is total (see the + # required-label split there) rather than a partial link. + sibling_build = labels.address("fe_copy") is None + + fe_groups = [ ("fe_copy/zero/one", lambda: test_fe_copy_zero_one(transport, labels)), ("fe_add", @@ -651,11 +657,19 @@ def run_tests(transport, labels, seed): lambda: test_fe_cswap(transport, labels, rng)), ("fe_inv", lambda: test_fe_inv(transport, labels, rng)), - ("x25519_clamp", - lambda: test_x25519_clamp(transport, labels, rng)), ] skipped_groups = [] + if sibling_build: + skipped_groups += [name for name, _ in fe_groups] + test_groups = [] + else: + test_groups = list(fe_groups) + + # x25519_clamp is public: both implementations export it. + test_groups.append( + ("x25519_clamp", + lambda: test_x25519_clamp(transport, labels, rng))) scalarmult_groups = [ ("x25519 RFC 7748 vector 1", lambda: test_x25519_rfc7748_vector1(transport, labels)), @@ -740,21 +754,74 @@ def main(): # Load labels labels = Labels.from_file(LABELS_PATH) - required = [ - "fe_src1", "fe_src2", "fe_dst", + # The fe_* unit-test groups drive the IN-TREE src/crypto/fe25519.s by + # its private symbol names. A `USE_X25519_SIBLING=1` build evicts that + # file from the link (Makefile CRYPTO_SRCS_EFFECTIVE) and the sibling + # exports the contract's `fe25519_*` surface instead, so none of these + # labels exist there and the script used to abort at + # `FATAL: 'fe_copy' label not found` before launching VICE — i.e. the + # sibling had NO runnable coverage at all, which is a bad thing to + # discover only after deciding to flip the default. + # + # The public path is spelled identically by both implementations, so + # the RFC 7748 vectors — the only end-to-end scalarmult coverage in + # this file — run against either. Split the requirement list + # accordingly: the public set is mandatory always, the fe_* set is + # mandatory only when the build claims to contain it. + # + # Detection is by ABSENCE OF THE WHOLE FAMILY, never by one probe + # label: a partially-linked in-tree build must still fail loudly + # rather than quietly downgrade itself to two vectors. + required_public = [ + "x25519_clamp", "x25519_scalarmult", + "x25_scalar", "x25_u", "x25_result", + "input_buffer", + ] + # Detection reads the ROUTINE entry points only. `fe_src1/2/dst` are ZP + # equates from src/constants.inc and are present in every link, + # sibling or not (measured: those three are exactly what survives), so + # including them in the probe set makes it never fire. + required_intree_fe_routines = [ "fe_copy", "fe_zero", "fe_one", "fe_add", "fe_sub", "fe_mul", "fe_sqr", "fe_inv", "fe_cswap", "fe_mul_a24", + ] + required_intree_fe_data = [ + "fe_src1", "fe_src2", "fe_dst", "fe_tmp1", "fe_tmp2", "fe_tmp3", - "x25519_clamp", "x25519_scalarmult", - "x25_scalar", "x25_u", "x25_result", - "input_buffer", ] + required_intree_fe = required_intree_fe_routines + required_intree_fe_data + + fe_present = [n for n in required_intree_fe_routines + if labels.address(n) is not None] + sibling_build = len(fe_present) == 0 + if fe_present and len(fe_present) != len(required_intree_fe_routines): + # Partial presence is neither an in-tree build nor a sibling one. + # Fail rather than guess: silently downgrading to two vectors here + # is how a broken link passes as a green run. + missing = [n for n in required_intree_fe_routines + if labels.address(n) is None] + print("FATAL: in-tree fe25519 is only partially linked — present " + f"{fe_present}, missing {missing}. Neither an in-tree nor a " + "USE_X25519_SIBLING build; not guessing.") + sys.exit(1) + + required = list(required_public) + if not sibling_build: + required += required_intree_fe + for name in required: if labels.address(name) is None: print(f"FATAL: '{name}' label not found in {LABELS_PATH}") sys.exit(1) + if sibling_build: + print(" NOTE: no in-tree fe_* symbols in this link — treating it as a") + print(" USE_X25519_SIBLING=1 build. The fe_* unit groups are") + print(" SKIPPED (they test src/crypto/fe25519.s internals, which") + print(" this PRG does not contain); the RFC 7748 end-to-end") + print(" vectors still run and are the whole of the coverage.") + print(f" Labels loaded: {len(required)} required labels verified") # Launch VICE @@ -791,9 +858,21 @@ def main(): summary += (f" -- {len(skipped_groups)} group(s) SKIPPED: " + ", ".join(skipped_groups)) print(summary) - if skipped_groups: + # Two different skips are possible and they mean opposite things, so + # the warning must name the one that actually happened rather than + # firing on any skip at all. --fast drops the only end-to-end + # coverage; a sibling build drops the field-arithmetic units but + # KEEPS the end-to-end vectors. + scalarmult_skipped = any(g.startswith("x25519 RFC 7748") + for g in skipped_groups) + fe_skipped = any(g.startswith("fe_") for g in skipped_groups) + if scalarmult_skipped: print("WARNING: end-to-end x25519_scalarmult coverage did NOT run; " "this run does not certify X25519.") + elif fe_skipped: + print("NOTE: field-arithmetic unit coverage did NOT run (no in-tree " + "fe25519 in this link). The RFC 7748 end-to-end vectors did, " + "and are the whole of this run's evidence.") print(f"{'='*60}") sys.exit(0 if failed == 0 else 1)