Skip to content

Add an upload-hop verifier and a generation-scoped trust reader (#138) - #139

Merged
AcoPiper merged 3 commits into
mainfrom
AcoPiper/issue-138
Sep 28, 2026
Merged

AcoPiper merged 3 commits into
mainfrom
AcoPiper/issue-138

Conversation

@AcoPiper

@AcoPiper AcoPiper commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds what an upload hop, such as REView's module store, needs in order to verify a package through this crate's one verifier. Before this change, every entry point required the component, version, commit and architecture up front, but an upload hop learns those only from the package itself, after the signature has been checked. This PR adds no second verifier. It changes no verdict, order or limit of verify_package, verify_contents or active_trust_set.

  • UploadRequest (src/verify.rs): holds only the deployment namespace. UploadRequest::new refuses an invalid namespace with the existing InputError::InvalidNamespace, using the same rule as VerifyRequest::for_namespaced_package.
  • Authentication split from the statement checks (src/verify.rs): the new crate-private authenticate_manifest runs the signature check, the format floor and the typed parse. It returns an Authenticated holding the parsed manifest, the key_id of the anchor that verified the signature, and the SHA-256 of the raw manifest block. verify_signature now returns that key_id instead of (), with its order and verdicts unchanged. authenticate and verify_package_bounded are now authenticate_manifest followed by the unchanged check_statements.
  • package::verify_upload (new src/package/upload.rs):
    • It measures the source, opens one RetentionScope with a budget of upload_staging_bound, takes a snapshot, and authenticates.
    • It then takes the build and architecture from the first artifact entry of the authenticated manifest and runs check_statements with the namespaced request built from them.
    • The rest runs through a post-authentication body, verify_authenticated, which verify_contents now shares instead of keeping a copy.
    • It returns a VerifiedUpload holding only metadata: the manifest, the derived build and architecture, key_id, manifest_sha256, package_sha256 and package_len. Every snapshot is released and the private directory is removed before it returns.
    • A manifest with no artifacts, or whose first entry names TRUST_TARGET, is refused as ContentError::Upload(UploadRefusal::NoArtifacts | ReservedTarget). Both refusals happen only after the signature verifies. A package mixing builds is still TargetMismatch, and one mixing architectures is still ArchitectureMismatch.
  • package::upload_staging_bound: returns min(RetainedDisk, 2 × package_len + OuterUncompressedTotal) with saturating arithmetic. It is pure, so a caller can compute it and reserve that much disk before the call, and verify_upload enforces exactly this budget. The rustdoc explains where the formula comes from.
  • release_trust::generation_trust_set and parse_generation_name: generation_trust_set reads gen-<n>/ directly and never looks at active. It shares one body (read_generation_material) with read_active_generation. A pruned generation is reported as Io with kind NotFound. parse_generation_name exposes the generation engine's rule for canonical generation names.
  • TrustSet::is_withdrawn is now public, and TrustSet::epoch is new. Both are #[must_use].

Tests cover each item in the issue's test plan:

  • the signature verdict taking precedence over every upload refusal;
  • table-driven parity with verify_contents;
  • the derived identity and key_id, including when the container's key hint names another anchor or is unusable;
  • the upload refusals and the namespace check;
  • staging_parent being left unchanged, the high-water mark staying within the bound, and the bound being enforced;
  • the bound formula and its saturation;
  • the two-generation reader, generation names, and the TrustSet accessors.

One existing test changed: every_error_variant_and_operation_is_nameable in tests/verify_contents.rs has an exhaustive match on ContentError, so it now also names the new Upload variant.

cargo fmt, both clippy runs and both cargo test runs listed in AGENTS.md pass. There is no new dependency and no CHANGELOG.md entry.

Closes #138

Deviations from the issue

  • The derived request is built without VerifyRequest::for_namespaced_package. Step 3 of item 3 asks for the request to be built with VerifyRequest::for_namespaced_package(component, version, commit, request.namespace()). Instead, verify_upload calls a new crate-private VerifyRequest::for_upload(component, version, commit, &UploadRequest), which calls VerifyRequest::for_package and then sets the namespace from the UploadRequest. for_package has only one refusal, InputError::ReservedTarget for TRUST_TARGET, and for_upload returns None for exactly that case, which verify_upload reports as UploadRefusal::ReservedTarget. The namespace was already checked with is_valid_segment in UploadRequest::new, so checking it again could never fail. Calling for_namespaced_package would have left a namespace-error branch that can never run, and the reserved-target case would have had to be picked out of an InputError. The request is the same one for_namespaced_package would build.
  • verify_package_bounded does not go through authenticate. Item 2 says authenticate becomes authenticate_manifest followed by check_statements, and that verify_package, verify_package_bounded and verify_contents keep their behaviour through it. authenticate does become those two calls, and verify_package still uses it. verify_package_bounded is now a new crate-private authenticate_bounded, which does the bounded container read and then authenticate_manifest, followed by the same check_statements call. verify_contents reaches the same sequence through verify_retained → verify_package_bounded. This was done because verify_upload needs the bounded read and authentication, including the archive block's offset and length, without the statement checks. Putting that step in its own function lets verify_package_bounded and verify_upload share one body instead of keeping two copies. The steps run in the same order and give the same verdicts.
  • One existing test was edited. The acceptance criteria say every existing test passes unchanged. every_error_variant_and_operation_is_nameable in tests/verify_contents.rs matches exhaustively on the public ContentError, so the new ContentError::Upload(UploadRefusal) variant that item 3 requires would not compile without a match arm for it. The test now names Upload(NoArtifacts) and Upload(ReservedTarget). No existing assertion or verdict changed.

Test plan

  • Signature first: an unsigned package, a flipped signature bit, an unknown key and a revoked key each give BadSignature, BadSignature, UnknownKeyId and RevokedKey. Each is paired with a manifest that would otherwise be refused as NoArtifacts, ReservedTarget, TargetMismatch or ArchitectureMismatch, and the signature verdict wins every time.
  • Parity, table-driven over the native, v6-image and legacy-image fixtures and the verify_contents refusal fixtures: verify_upload and verify_contents give the same variant. verify_contents is called with the fixture's true build, the namespace, its true architecture, and RetainedDisk set to upload_staging_bound.
  • Derived identity, for a native and a v6 fixture on x86_64 and on aarch64: component, version, commit, target_arch, key_id, manifest_sha256, package_sha256 and package_len equal the fixture's build, architecture, signing key, raw-manifest digest, package digest and length.
  • key_id() names the anchor that verified the signature, both when the container's key hint names another anchor that does not verify and when the hint is unusable.
  • Upload refusals, each signed validly:
    • an empty-artifact manifest → Upload(NoArtifacts);
    • a trust-generation container signed by a trust-set anchor → Upload(ReservedTarget);
    • a two-build manifest → TargetMismatch;
    • a two-architecture manifest → ArchitectureMismatch whose expected is the first entry's architecture.
  • Namespace:
    • an image declaring clumit-other, verified under UploadRequest::new("clumit-security"), is Image(NamespaceMismatch), and the same package verified under its own namespace passes;
    • a package declaring no image verifies under any namespace;
    • UploadRequest::new("") and UploadRequest::new("a/b"), among other invalid namespaces, are refused with InputError::InvalidNamespace.
  • Staging:
    • after every success and every refusal above, staging_parent holds exactly the entries it held before;
    • the scope's high-water mark for the largest image fixture stays within upload_staging_bound;
    • with RetainedDisk one byte below that high-water mark, the call is LimitExceeded naming RetainedDisk;
    • a group-writable staging_parent is refused as Io with InspectStagingParent.
  • Bound: upload_staging_bound at package_len 0, 1 and u64::MAX, with default limits and with OuterUncompressedTotal and RetainedDisk lowered, matches the formula and saturates instead of overflowing.
  • Generation reader:
    • a tree is seeded with admit_seed_generation and extended with replace_generation, and each generation read by number returns its own epoch, anchors and withdrawn list, whichever generation active names;
    • a read never touches active: a generation still reads correctly when active is a dangling symlink;
    • a missing generation directory or material file is Io with kind NotFound;
    • a corrupted epoch or trust-set.json gives the same refusal active_trust_set gives.
  • Names: parse_generation_name returns Some for gen-1, gen-42, gen-0 and gen-18446744073709551615 (u64::MAX). It returns None for gen-01, gen-+1, gen-1.tmp, gen-, active, gen-retention.tmp, an overflowing index, gen--1, a name with a trailing space, and multi-component paths.
  • Accessors: TrustSet::is_withdrawn is true for each listed triple and false when any part differs or two listed triples are mixed. TrustSet::epoch equals the constructor argument, including 0 and u64::MAX.
  • Unchanged behaviour: the full existing suite passes. The only edit to an existing test is the new ContentError::Upload variant added to every_error_variant_and_operation_is_nameable, as described under Deviations.
  • cargo fmt -- --check --config group_imports=StdExternalCrate passes.
  • cargo clippy --all-targets -- -D warnings and cargo clippy --all-targets --features test-support -- -D warnings pass.
  • cargo test and cargo test --features test-support pass.

An upload hop learns a package's build and architecture only from the
package, and only once its signature has verified, but every verifier
entry point took them in advance. Split authentication (signature,
format floor, typed parse) from the statement checks so a caller can
derive the request from the authenticated manifest, and add
verify_upload on top of the existing contents pipeline: one shared
post-authentication body, the verifying anchor's key_id and the raw
manifest digest reported, and a disk budget, upload_staging_bound, a
caller can compute and reserve before the call.

active_trust_set reads two files through the active link, so an
activation between the reads can pair two generations. Add
generation_trust_set, which reads one gen-<n> directly through the
same body, and parse_generation_name, so a caller resolving active
once can decode its target with the engine's own rule. Expose
TrustSet::is_withdrawn and TrustSet::epoch for re-checks against the
same generation.

The describe() helper in tests/verify_contents.rs matches ContentError
exhaustively by design, so it gains an arm for the new Upload variant.

Closes #138
The README's module summaries still presented verify_contents as the
package module's only full-content entry point and active_trust_set
as the only way to rebuild a trust set from the tree, so a reader
would not learn that an upload hop and a caller resolving `active`
itself have their own entry points.

Part of #138
The release-trust module doc explained the two readers that go through
`active` but not the new one that never does, and the README paragraph
the previous commit touched was left with a ragged wrap.

Part of #138
@AcoPiper

Copy link
Copy Markdown
Contributor Author

[Reviewer Round 1]

Review verdict: Approve, with documentation cleanup requested. The upload path authenticates before deriving identity, then shares the statement checks and content verification path with verify_contents. The numbered trust reader uses the same document validation and trust-set assembly as the active reader. The three declared deviations are accurately described and reasonable: for_upload builds the same validated request, authenticate_bounded preserves the check sequence, and the exhaustive test match needed a new arm. The new tests exercise signature precedence, derived metadata, staging cleanup, and generation-specific reads.

One documentation finding: TrustSet::epoch calls its value the active generation’s epoch, although generation_trust_set can return an inactive generation. The VerifyRequest invariant comment also says only for_namespaced_package carries a namespace, and the active_trust_set documentation calls itself the sole tree constructor. Those claims should be updated for the new APIs.

The PR body also repeats Closes #138; one occurrence is sufficient.

@AcoPiper

Copy link
Copy Markdown
Contributor Author

[Review Verdict Round 1: APPROVED]

@AcoPiper

Copy link
Copy Markdown
Contributor Author

Suggested squash commit

Title

Add an upload-hop verifier and a generation reader

Body

An upload hop learns a package's build and architecture only from the
package, and only once its signature has verified, but every verifier
entry point took them in advance. Split authentication (signature,
format floor, typed parse) from the statement checks so a caller can
derive the request from the authenticated manifest, and add
verify_upload on top of the existing contents pipeline: one shared
post-authentication body, the verifying anchor's key_id and the raw
manifest digest reported, and a disk budget, upload_staging_bound, a
caller can compute and reserve before the call.

active_trust_set reads two files through the active link, so an
activation between the reads can pair two generations. Add
generation_trust_set, which reads one gen-<n> directly through the
same body, and parse_generation_name, so a caller resolving active
once can decode its target with the engine's own rule. Expose
TrustSet::is_withdrawn and TrustSet::epoch for re-checks against the
same generation.

The README and the release-trust module doc describe the new entry
points, so a reader learns that an upload hop and a caller resolving
active itself each have their own.

The describe() helper in tests/verify_contents.rs matches ContentError
exhaustively by design, so it gains an arm for the new Upload variant.

Closes #138

@AcoPiper
AcoPiper merged commit a6e02e7 into main Sep 28, 2026
5 checks passed
@AcoPiper
AcoPiper deleted the AcoPiper/issue-138 branch September 28, 2026 05:07
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.

Add an upload-hop verifier and a generation-scoped trust reader

1 participant