Add pinned offline image trust verification - #118
Conversation
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Initial review of the current head (8f7fa7f). I reviewed the actual diff against main (12 files, ~342 insertions), the surrounding policy/verifier/lockfile code, and the pinned-cosign CLI contract against upstream sigstore/cosign source.
Findings
🟡 [important] offline-bundle has undocumented hard prerequisites — it deterministically fails for legacy-format signatures and cosign < v3.1
internal/imagetrust/verify.go passes --offline=true --new-bundle-format=true --trusted-root=... to cosign verify-blob-attestation for each bundle returned by cosign download signature. Checking upstream cosign source:
download signatureonly emits new-format Sigstore bundles when the pinned verifier is cosign ≥ v3.1.0 (GetBundlesOCI-1.1-referrer fetch was added toSignatureCmdthere; v3.0.x and all v2.x only return legacysha256-<digest>.sigobjects viaFetchSignaturesForReference).- The image must actually be published with new-format referrer bundles (
cosign signdefault only on v3.x; opt-in--new-bundle-formaton v2.x), and the registry must support the OCI 1.1 referrers API. - When any of this doesn't hold, the staged file fails cosign's
checkNewBundle, the verifier drops to the legacy path, and--trusted-roottriggers the hard error--trusted-root only supported with --new-bundle-format— so every bundle fails and verification ends withno downloaded image signature bundle satisfied machine policy.
This is fail-closed (correct) and the cosign stderr is surfaced via verifierRunError, so it's diagnosable — but docs/enterprise-policy.md and README.md never state that offline-bundle requires (a) cosign ≥ v3.1 and (b) images signed/published in the new bundle format. An admin enabling offline-bundle for the common legacy-signed image today will hit a deterministic, unexplained failure. Suggest documenting these prerequisites in the NETWORK_MODE section of docs/enterprise-policy.md (and a minimum cosign version note).
🟢 [nit] Version-gate diagnostic ordering for trusted-root fields
cosign_trusted_root_path/cosign_trusted_root_sha256 set both usedImageTrustFields and usedOfflineImageTrustFields (internal/policy/policy.go:413-424), and the schema-3 check runs before the schema-4 check. A policy_version = 1 or 2 file using trusted-root fields is rejected with "image trust controls require policy_version 3" instead of the schema-4 requirement — an admin would bump to 3 and get a second rejection. Still fails closed; only the message is imprecise.
💡 [suggestion] No end-to-end qualification for the offline path
internal/cli/image_trust_e2e_windows_test.go still only exercises policy_version = 3 + online. Given the version-sensitive cosign contract above (flag availability, download signature referrer behavior, checkNewBundle gating), an optional offline leg — e.g. an E2E_TRUSTED_ROOT env var plus a referrer-bundle-signed image — would validate the real CLI path rather than only the fake-runner unit tests.
Summary
The implementation is careful and consistently fail-closed: the pinned TrustedRoot is authenticated via the same immutable-snapshot/TOCTOU pattern as the verifier and key, staged privately at 0600, re-hashed after each use; schema gating is correct (schema-3 offline rules parse but stay blocked until migrated; trusted-root fields require schema 4 + ≥1 offline rule; missing/incomplete pins are rejected); runtime freshness authorization correctly gates offline evidence on the pinned root identity via the policy fingerprint; and the minimal runner environment means no ambient TUF/credential state reaches cosign. --offline is effectively belt-and-suspenders on the new-bundle path (deprecated upstream; harmless since the trusted root makes verification local). The main gap is the undocumented cosign-version/bundle-format prerequisite, which will make the feature deterministically fail — albeit safely — for most images signed today.
Note: CI checks were still pending at review time; this review is based on source inspection plus upstream cosign source analysis, not CI results.
|
Addressed Cheryl's review in signed commit
Validation: |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The security-sensitive trust path depends on real cosign and registry interoperability covered only by opt-in qualification.
Review effort: Balanced
Findings: None
What changed in this PR
Adds schema-4, pinned Sigstore TrustedRoot support for fail-closed offline image verification.
Changes:
- Authenticates and stages pinned TrustedRoot snapshots.
- Enables offline cosign verification and lock/runtime evidence.
- Adds tests and updates security and policy documentation.
| File | Description |
|---|---|
| README.md | Documents offline verification requirements. |
| internal/policy/policy.go | Adds schema-4 trusted-root policy support. |
| internal/policy/policy_test.go | Tests parsing, authentication, and authorization. |
| internal/imagetrust/verify.go | Stages roots and invokes offline cosign verification. |
| internal/imagetrust/verify_test.go | Tests offline verifier behavior. |
| internal/cli/image_trust.go | Accepts offline verification evidence. |
| internal/cli/image_trust_test.go | Tests offline evidence generation. |
| internal/cli/image_trust_e2e_windows_test.go | Extends Windows qualification coverage. |
| docs/security-model.md | Updates the security model. |
| docs/roadmap-implementation-requirements.md | Marks offline verification implemented. |
| docs/roadmap-decisions.md | Records the schema-4 design. |
| docs/enterprise-policy.md | Documents configuration and qualification. |
| docs/architecture.md | Updates verification architecture. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Re-review of the current head (f1ce47c). I re-validated each finding from my prior round against the current code rather than the author's replies, then performed a complete fresh review of the full diff (13 files, ~410 insertions): internal/imagetrust/verify.go, the schema-4 policy changes in internal/policy/policy.go, the schema-2 evidence path in internal/cli/image_trust.go + internal/lockfile, tests, the build-tagged Windows qualification, and all docs.
Validation of previous findings
- [important] undocumented
offline-bundleprerequisites — fixed.docs/enterprise-policy.md(NETWORK_MODE section) andREADME.mdnow state the hard prerequisites: cosign ≥ 3.1.0, an image published with a new-format Sigstore bundle, and OCI 1.1 referrers support, with explicit language that legacy signature objects fail closed rather than falling back online. - [nit] version-gate diagnostic ordering — fixed.
internal/policy/policy.gonow checksusedOfflineImageTrustFields(schema 4) before the generic image-trust (schema 3) gate, andTestParseReportsSchemaFourForTrustedRootFieldspins the precise message forpolicy_version1-3. - [suggestion] no end-to-end qualification for the offline path — fixed. Commit 390291b extends the opt-in
image_trust_e2eWindows test withCONTAINERBIN_IMAGE_TRUST_E2E_TRUSTED_ROOTand exercises realLock/Updateagainst a schema-4offline-bundlepolicy with real cosign and Docker Desktop, reloading the schema-2 lockfile after each operation. It remains opt-in (not claimed by normal CI), consistent with the documented qualification model.
Fresh review of the current implementation
Verified fail-closed properties end to end:
- Schema gating is precise: trusted-root fields require schema 4 and at least one
offline-bundlerule; a missing or partial pin with an offline rule is rejected; a pinned root without any offline rule is rejected. Schema-3offline-bundlerules remain parseable (compat) but fail closed twice —AuthenticateImageTrustTrustedRoothas no pin to authenticate, andauthorizeRuntimeImageTrustrejects whencosignTrustedRoot.SHA256is not a valid pin. - The trusted root goes through the same authenticated-snapshot pipeline as the verifier/key (Lstat → open → bounded read → re-Lstat → digest match), is staged
O_EXCLat 0600 under the protected staging DACL, and is re-hashed after the download call and again after the verify loop alongside all other staged material. - Evidence binding is sound: the schema-2 record carries no explicit network mode, but the repository → rule mapping is deterministic and the complete policy fingerprint (which covers the rule's mode and the root pin) is compared at runtime, so no cross-mode or cross-root replay is possible.
- Output remains bounded (1 MiB, ≤ 32 bundles), per-bundle verification treats any cosign failure (including overflow) as a failed bundle, and evidence still requires exactly one distinct authenticated bundle.
- The runner environment stays minimal — no ambient TUF, registry credential, or HOME state reaches cosign, matching the documented private-registry exclusion.
All CI checks pass (vet, go test -race on Linux and Windows amd64/arm64, release builds, reproducibility, CodeQL, govulncheck, zizmor, dependency review).
Findings
No actionable issues found in the current revision.
Summary
offline-bundleimage-trust rules--offline=true, explicit new-bundle mode, and the staged--trusted-rootfor local bundle verificationThe implementation follows cosign's documented air-gapped contract: the bundle carries signature/proof material, while an explicit TrustedRoot prevents TUF/service-key lookup. No ambient trust or credential files are inherited.
Validation
gofmt -l .go vet ./...go test -race ./...cb.exe versionsmoke checkgit diff --checkRefs #2