Skip to content

market: one release codec, and check a release before offering it - #544

Closed
eKisNonos wants to merge 5 commits into
mainfrom
market/release-index
Closed

eKisNonos wants to merge 5 commits into
mainfrom
market/release-index

Conversation

@eKisNonos

Copy link
Copy Markdown
Contributor

Release encode, decode and signing move into marketplace_abi. The tool writing the index and the capsule reading it had a codec each, over the same structure.

install_ready checks arch and readiness against the running image instead of believing the index, and gains a seventh gate for whether a release carries a zk trailer for its own measurement. The other six are signatures over the artifact, so this is a different question and counted separately.

Tooling: the catalogue generator reads what is on disk and writes JSON, the CLI encodes the binary the market ingests. Signing is a separate step with the operator seed and does not appear in a build rule. Only the public key is committed.

Release encode, decode and signing move into marketplace_abi, so the
tool that writes the index and the capsule that reads it share a codec
instead of having one each.

install_ready checks arch and readiness against the running image
rather than the index, and adds a seventh gate for whether a release
carries a zk trailer for its own measurement. The other six are
signatures over the artifact.

The catalogue generator reads what is on disk and writes JSON; the CLI
encodes the binary the market ingests. Signing is a separate step with
the operator seed, which is not in any build rule.
IconId::Store points table.rs at assets/icons/store.a8, which only
existed on the app-store branch, so every build of this branch failed
to read it. Add the mask and its SVG source here so the table stands
on its own.
nonos-data/marketplace/index.bin has no make rule, so naming it as a
hard prerequisite failed every build on a checkout without it (CI:
No rule to make target). Wrapping it in $(wildcard) keeps the rebuild
on a newer catalogue where it exists and drops the prerequisite where
it does not.
The branch's manifest predated the switch to in-process Ed25519 and
dropped the dependency while verify/crypto.rs imports it, so the
capsule failed with an unresolved import. Restore main's manifest and
add only the app_skeleton dependency boot_index.rs needs.
@senseix21

Copy link
Copy Markdown
Collaborator

@eKisNonos Review before merge

Scope: marketplace release codec v2 (zk_trailer_hash bound under domain NONOS.marketplace.release.v2), install-readiness checks, boot-time baseline catalogue (capsule_market/src/boot_index.rs), toolkit IconId::Store (48 icons).

Our changes: merged origin/main (131fb24cd); b1533e872 adds assets/icons/store.{a8,svg} (the table's include_bytes! target — was only on #545; host icon_table test PASS, desktop_shell user-target check green); 5d9e2d6ff CAPSULE_EXTRA_DEPS := $(wildcard …/index.bin) so make no longer dies with "No rule to make target"; d221a0d7d restores nonos_ed25519 in capsule_market/Cargo.toml (branch had dropped it while verify/crypto.rs:17 imports it).

Findings:

  • userland/capsule_market/src/boot_index.rs:35 include_bytes!("../../../nonos-data/marketplace/index.bin") — the file exists in no tree: not in the pinned nonos-data submodule, and no make rule produces it. Every build/boot-smoke fails to compile the market capsule. Needs either the signed index committed to nonos-data (signing — owner action) plus a submodule bump, or a build-time fallback when absent. — blocker
  • Release signing domain bumped to v2 — old v1-signed releases now reject by design; confirm the publisher tooling was updated in lockstep. — medium
  • userland/toolkit/tests/host/icon_table.rs adds an in-body // comment — minor.

Verdict: hold — blocked on the missing signed baseline index.

@senseix21

Copy link
Copy Markdown
Collaborator

Reviewed at d221a0d7d (not a draft, base main, 28 files, +664/−49, 213 commits behind main, 5 commits of its own, last pushed 2026-09-24). New tooling in tools/nonos-market-catalogue (+352) and tools/nonos-market-sign-releases (+84), capsule_market +183/−14, marketplace_abi +25/−21, and a 7-line kernel change.

Verdict: Comment. The release-check work is the right idea and the wire change is done correctly on both sides. One gate is weaker than its name suggests, and the CI on this head is too old to tell anyone anything.

The new gate is wired into the verdict, not just reported

bd5704eb5 adds a ninth install gate and actually ANDs it. userland/capsule_market/src/install_ready/checks.rs:

let install_ready = index_signature_valid
    && validation_passed
    && package_url_present
    && package_hash_present
    && manifest_hash_present
    && publisher_signature_verified
    && arch_match
    && kernel_abi_compatible
    && attestation_present;

That is the part that is easy to get wrong — adding a field to a readiness struct, surfacing it in the UI, and forgetting to make it block. It blocks.

And the 6→7 byte wire change is coherent across both sides of the boundary, which in this codebase is where these things usually drift. Userland READINESS_LEN: 6 → 7 with slot[6] = verdict.attestation_present as u8; kernel READINESS_LEN: 6 → 7 with attestation_present: resp.body[6] != 0. Checked both, they agree.

Important

1. attestation_present is satisfied by a metadata string, and the variable name says something stronger.

// Everything above this line is somebody's word.
let minted_locally = release.supported_arches.iter().any(|a| a.as_str() == LOCAL_ARCH);
let ships_proof = release.zk_trailer_hash.iter().any(|&b| b != 0);
let attestation_present = ships_proof || minted_locally;

LOCAL_ARCH is HOSTED_ARCH, which is "x86_64-linux" (install_ready/arch.rs:22). So the second disjunct reads: the release's supported_arches list contains the string "x86_64-linux".

Two things follow.

The comment places a trust boundary and then crosses it. "Everything above this line is somebody's word" implies the two lines below it are not — but minted_locally is computed from release.supported_arches, the same index-supplied field that arch_match uses four lines above the line. ships_proof is genuinely different in kind: a non-zero zk_trailer_hash is a commitment to the artifact. minted_locally is a self-description.

And the name is doing work the code does not. minted_locally suggests "we built this ourselves"; what it tests is "the index says this is a Linux-arch package". A later reader deciding whether this gate is strong enough will read the name, not the expression.

Net effect: any release whose index entry declares x86_64-linux passes the attestation gate with no proof, and the only thing behind that claim is index_signature_valid — the index signer's word. For a gate whose purpose is to avoid having to trust the index signer about the artifact, that is a meaningful exemption.

I suspect it is deliberate and necessary: a Debian or Alpine package cannot carry a NONOS zk trailer, so hosted-arch releases need some exemption or the gate blocks the whole personality use case. If so, the ask is narrow — rename it to what it tests (declares_hosted_arch, say), move it above the trust-boundary comment where it belongs, and say in a comment why an unattested hosted-arch package is acceptable. If it is not deliberate, then the gate is bypassable by a string in an index entry.

2. The CI on this head predates the gates it would now have to pass.

build / build is red, and the report explains why that is uninformative. ci-reports-build/build/build-report.json from run 36030502546 (2026-09-24) has six checks:

pass  rustfmt            pass  clippy-nonos-sign
pass  build-x86_64-capsules   pass  section-size
pass  symbol-scan        gap   build-aarch64

Status gap, with every real check passing. There is no tcb-budget and no proof-coverage entry at all — those exist in every current build report in this queue. So this run is from an older nonos-verify against an older main, and the lane's red comes from the build-aarch64 gap being treated as failure rather than from anything in this diff.

The same applies to the rest: boot-smoke, attestation-attack, boot-proofs, benchmark, crypto-proofs, abi-contracts and every dark-features-compile variant are all nine days old against a main that has moved 213 commits — and since pull_request CI builds refs/pull/N/merge, those runs tested this head merged into a main that no longer exists.

Nothing to fix in the diff. But no verdict on this PR's CI means anything until it is rebased and re-run, and with 213 commits of drift the rebase is the first piece of work, not the last.

Minor

  1. bd5704eb5 replaces the install_ready.rs module doc — which described the protocol as "a hard AND of nine install gates… six bytes: one for the AND-result followed by the per-check bits… so a caller can short-circuit on the AND-result while still being able to tell which gate refused" — with //! OP_INSTALL_READY. That documentation was deleted in the same commit that changed the wire format from six bytes to seven. It was the only place the layout was written down, and it was describing a protocol with two independent implementations. Correcting "six bytes" to "seven" would have been the smaller edit; I count nine gates in evaluate, so the rest of it was accurate.
  2. Three of the five commits are fixes to the other two (b1533e872 ships an icon mask the icon table already referenced, 5d9e2d6ff tracks the signed index only when the tree has one, d221a0d7d keeps nonos_ed25519 because verify/crypto.rs still uses it). All sound, but it means the feature commit bd5704eb5 did not stand on its own — worth a squash before landing so the history reads as one change.

Questions

  1. Is the || minted_locally exemption intended to cover distribution packages that cannot carry a zk trailer? If so, is supported_arches the right signal, or is there something closer to the installer's own knowledge — the path a package came in through, or a publisher identity — that does not rely on the release describing itself?
  2. tools/nonos-market-sign-releases (+84) and tools/nonos-market-catalogue (+352) are new host-side tools. Does anything verify that a catalogue they produce round-trips through capsule_market's decoder, or is the codec agreement checked only by the two implementations being written together?

Verified correct

  • The new gate blocks rather than merely reporting. Confirmed in the install_ready AND, not inferred from the struct field.
  • The readiness wire change is consistent across the kernel/userland boundary. READINESS_LEN and the byte index both moved on both sides. This codebase has a history of hand-synced cross-component constants drifting; this one did not.
  • ships_proof is the right test for the thing it names. A non-zero zk_trailer_hash is a commitment to the measured artifact, so that half of attestation_present is exactly what the gate is for. Item 1 is only about the other half.
  • The arch triples fail closed on a new port. arch.rs ends in a compile_error! for any target_arch without a canonical NONOS triple, with the remedy in the message — "update arch.rs alongside the new arch port". That is the right way to make a table that must grow with the targets refuse to be forgotten.
  • HOSTED_ARCH is empty on non-x86_64, so runs_here's !HOSTED_ARCH.is_empty() guard and minted_locally both degrade to false rather than matching an empty string. Checked because an empty-string arch match would have made item 1 considerably worse.

CI at this head

Fifteen lanes pass, the rest are nine days stale against a main that has moved 213 commits, and the build report shows an older gate set than the one this PR would actually face. Item 2 is the first thing to do; item 1 is the only thing in the diff I would want answered before it lands.

@eKisNonos

Copy link
Copy Markdown
Contributor Author

Superseded. This work is integrated into the 0.9.2 release and ships in the current tree. Closing as part of the 0.9.2 consolidation.

@eKisNonos eKisNonos closed this Oct 2, 2026
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.

2 participants