Skip to content

fix(ci): verify ledger-suite release artifacts against build attestations - #11324

Draft
basvandijk wants to merge 9 commits into
bas/attest-cdn-uploadsfrom
bas/verify-ledger-suite-artifacts
Draft

fix(ci): verify ledger-suite release artifacts against build attestations#11324
basvandijk wants to merge 9 commits into
bas/attest-cdn-uploadsfrom
bas/verify-ledger-suite-artifacts

Conversation

@basvandijk

Copy link
Copy Markdown
Collaborator

Second step for security finding 3618194 (MEDIUM, CWE-494), building on the producer-side attestations of #11323.

ledger-suite-release.yml downloaded canister wasms plus the CDN's own SHA256SUMS and republished a pruned copy as the release's "Files and Hashes" verification data. No downloaded byte was ever hashed and compared — the only check was that each expected filename had a SHA256SUMS line — so anyone able to write to the CDN buckets could get their artifacts published as an official DFINITY release with matching, authentic-looking checksums.

Changes

This PR introduces the shared verification primitive that the remaining consumers (tag-release.yml, mainnet_revisions.py) will reuse in follow-ups:

  • ci/scripts/fetch-attested-sums.sh — downloads a directory's SHA256SUMS from the CDN and verifies it, before anything reads it, with
    gh attestation verify SHA256SUMS --repo dfinity/ic \
      --signer-workflow <pipeline> --source-digest <commit>
    
    --source-digest pins the attestation to the exact commit, so replaying another commit's legitimately-attested SHA256SUMS also fails. Validates its inputs (40-hex commit, no ../. path segments) and fails closed.
  • .github/actions/fetch-verified-artifacts — workflow wrapper: pinned gh CLI install (house pattern from update-mainnet-canister-revisions.yaml), one attested-SHA256SUMS fetch per directory, then per-file curl + sha256sum --check against the verified sums, aborting on any missing entry or byte mismatch. Emits a SHA256SUMS of the verified hashes under the files' local names — so the published checksums are the verified build-time values, not re-hashes of the downloads.
  • ledger-suite-release.yml — the fetch()-based download step is replaced by one invocation of the action (signer-workflow: dfinity/ic/.github/workflows/ci-kickoff.yml, since ledger-suite tags point at master commits, which the ci-kickoff pipeline attests). Release body, changelog, file-verification and softprops steps are unchanged apart from reading the action's out-dir.

Verification

Failure paths exercised directly (fail closed):

  • malformed commit id → rejected, exit 1; ../etc and canisters/.. subdirs → rejected, exit 1;
  • live run against the real canisters/SHA256SUMS of master commit 79c01052b5gh attestation verify fails with HTTP 404 from the attestations API (no attestations exist until feat(ci): attest CDN artifact uploads with build provenance #11323 merges) → exit 1. A flipped hex char or substituted CDN object fails the same way (digest lookup misses / sha256sum --check mismatch).

Positive-path validation: cut a ledger-suite-{icrc|icp}-YYYY-MM-DD.rc1 tag on a master commit built after #11323 merges — the draft prerelease then carries byte-verified hashes.

Rollout note

A tag push runs the workflow file at the tag's commit, so tags on commits containing both #11323 and this change are automatically attested and verified; older tags keep the old behavior. Tags pointing at commits that predate the attestation rollout can no longer be released with this workflow (documented in the workflow header — pick a newer commit).

Depends on #11323 (runtime dependency only — attestations must exist for the commits being released; no code dependency, safe to review in parallel).

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds attestation-backed integrity verification for ledger-suite release artifacts.

Changes:

  • Adds verified SHA256SUMS retrieval.
  • Adds an artifact download and checksum-verification action.
  • Integrates verification into the release workflow.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
ci/scripts/fetch-attested-sums.sh Fetches and verifies attested checksums.
.github/actions/fetch-verified-artifacts/action.yaml Downloads and validates artifacts.
.github/workflows/ledger-suite-release.yml Uses verified artifacts for releases.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/actions/fetch-verified-artifacts/action.yaml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

.github/actions/fetch-verified-artifacts/action.yaml:166

  • Two entries can resolve to the same local_name (notably files with the same basename from different architecture directories). The later download silently overwrites the earlier file while both checksum lines remain, so the output directory and emitted SHA256SUMS become inconsistent and a caller can publish the wrong artifact. Reject a local name that has already been emitted before downloading it.
          echo "Fetching $rel -> $local_name"
          curl -fsSL --retry 3 \
            "https://download.dfinity.systems/ic/$COMMIT/$rel" \
            -o "$out_dir/$local_name"

Comment thread .github/actions/fetch-verified-artifacts/action.yaml Outdated
basvandijk added a commit that referenced this pull request Aug 26, 2026
Flattening the directory separator (s|/|_|) is not injective:
"foo/bar_baz" and "foo_bar/baz" shared a cache key, so the second
directory would silently reuse the first's verified SHA256SUMS instead
of verifying its own, weakening the per-directory subject binding when
both appear in one invocation. The cache is now keyed by the sha256 of
the exact directory string.

Addresses
#11324 (comment)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@basvandijk
basvandijk requested a balanced review from Copilot August 26, 2026 21:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

.github/actions/fetch-verified-artifacts/action.yaml:155

  • Duplicate local names are accepted. On the second occurrence, the artifact overwrites the first file while SHA256SUMS retains both hashes for that same name, leaving an inconsistent output and making a glob-publishing caller release only the overwritten artifact. Reject a local name already emitted to the manifest.
          if ! [[ "$local_name" =~ ^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$ ]] || [ "$local_name" = "SHA256SUMS" ]; then
            echo "ERROR: invalid local-name: $local_name" >&2
            exit 1
          fi

ci/scripts/fetch-attested-sums.sh:5

  • This reference points to .github/actions/attest-uploads, but no such action exists; the producer described in #11323 is an attest-uploads job in the calling workflows. Update the pointer so future maintainers can find the provenance producer.
# pipeline that built <commit> (see .github/actions/attest-uploads).

Comment thread .github/workflows/ledger-suite-release.yml
Comment thread ci/scripts/fetch-attested-sums.sh
Comment thread .github/actions/fetch-verified-artifacts/action.yaml
basvandijk added a commit that referenced this pull request Aug 26, 2026
…ntract

Three review findings on the verification primitive:

- The signer-workflow pin fixes which workflow signed an attestation,
  not from which ref it ran: ci-kickoff.yml can be dispatched on
  arbitrary branches (and runs for dev-gh-* pushes and PRs), so anyone
  able to trigger a release build of an unreviewed branch could mint
  an acceptable attestation. fetch-attested-sums.sh now takes a
  required <source-ref-regex> and requires the same attestation entry
  to match it (certificate sourceRepositoryRef) AND carry the expected
  subject; ledger-suite pins refs/heads/master. Live-tested against a
  real branch-minted attestation: accepted only by its own ref regex,
  rejected by the master and release-branch pins.

- The release job's explicit permissions map zeroes everything
  unlisted; grant attestations: read for `gh attestation verify`.

- fetch-verified-artifacts now refuses a non-empty out-dir (a consumer
  globbing the directory must see exactly the verified files) and
  rejects duplicate local names.

Addresses
#11324 (comment)
#11324 (comment)
#11324 (comment)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@basvandijk
basvandijk requested a balanced review from Copilot August 26, 2026 22:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread .github/actions/fetch-verified-artifacts/action.yaml Outdated
basvandijk added a commit that referenced this pull request Aug 26, 2026
find does not dereference a command-line symlink by default, so an
out-dir that is a symlink to a non-empty directory looked empty while
the writes (and a caller's publication glob) follow the link — stale,
unverified files would survive in the advertised output directory.
find -H follows the start-point symlink only: a symlink to a non-empty
target is now rejected, a symlink to an empty one still works, and a
child symlink below the start point counts as an entry.

Addresses
#11324 (comment)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@basvandijk
basvandijk requested a balanced review from Copilot August 26, 2026 22:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@basvandijk
basvandijk changed the base branch from master to bas/attest-cdn-uploads August 27, 2026 09:43
Comment on lines 21 to +25
# 1. Pick a commit on the master branch for which the FI nightly test passed, and verify that artifacts are available in
# the CDN (e.g., verify that https://download.dfinity.systems/ic/${commit}/canisters/ic-icrc1-ledger.wasm.gz exists).
# The commit's CDN uploads must carry a build-provenance attestation (created by the attest-uploads job of the
# ci-kickoff pipeline for every master commit): this workflow verifies every downloaded artifact against it and
# fails otherwise, so tags on commits that predate the attestation rollout cannot be released with this workflow.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idea for a follow up: restructure the workflows in such a way that the attestation guarantees that the FI nightly test passed on the commit.

basvandijk added a commit that referenced this pull request Aug 27, 2026
Fourth step for security finding 3618194 (MEDIUM, CWE-494), building on
the producer-side attestations of #11323 and fetch-attested-sums.sh of
#11324. mainnet_revisions.py recorded several hashes into
mainnet-icos-revisions.json straight from unauthenticated CDN data: the
dev update-image hash was the hash of the CDN download itself, and the
setup-os and binaries hashes were read from CDN-served SHA256SUMS. The
recording PR is auto-approved and auto-merged, and
bazel/mainnet-icos-{images,binaries}.bzl then "verify" downloads against
those same values -- self-consistency, not authenticity.

All CDN-derived hashes now flow through VersionArtifactSums, which
verifies each directory's SHA256SUMS via ci/scripts/fetch-attested-sums.sh
(`gh attestation verify --repo dfinity/ic --signer-workflow
dfinity/ic/.github/workflows/release-testing.yml --source-digest
<version>` -- elected versions are always rc/hotfix builds) before any
value is read from it:

* update_img_hash_dev now comes from the verified
  {guest,host}-os/update-img-dev SHA256SUMS instead of hashing the
  multi-hundred-MB image the cron used to download every 2 hours.
* setupos hashes and all 7 MAINNET_BINARIES hashes come from verified
  SHA256SUMS.
* launch-measurements.json (dev and the prod fallback) is byte-verified
  against the attested SHA256SUMS entry before parsing.
* New cross-check: the NNS-elected update-img hash from the proposal must
  equal the build-time hash in the verified SHA256SUMS -- a mismatch
  means the CDN does not serve what the NNS elected, and nothing is
  recorded.

Cutover policy (VersionArtifactSums docstring): a PUBLIC commit without a
verifiable attestation hard-fails -- no PR is created, the cron retries;
backfill by re-running release-testing.yml on the version's branch (the
rebuild is checked byte-for-byte against the CDN by rclone --immutable
--checksum before attest-uploads runs). A commit that is NOT public
(undisclosed security patch, built in ic-private and not attested here)
falls back to the pre-fix CDN behavior with a loud warning, time-bounded
until disclosure; CDN write access cannot remove a commit from the
public repository, so the fallback is not attacker-selectable within the
finding's threat model.

The update_saved_* functions now check is_record_up_to_date BEFORE
collecting the version's artifact info, so the 2-hourly cron stays cheap
and green while recorded versions predate the attestation rollout;
verification only runs when a new version is recorded.

The updater workflow gets the pinned gh CLI install (gh attestation
verify --source-digest needs gh >= 2.61; same step as
update-mainnet-canister-revisions.yaml) and its pull_request dry-run
trigger now also fires on changes to the updater script and
fetch-attested-sums.sh.

Verification, all run against the live APIs:

* `mainnet_revisions.py --dry-run icos` passes end-to-end (all records
  up-to-date, no attestation calls needed).
* get_replica_version_info() for the currently-elected 79c0105 fails
  closed: attestation lookup 404s (none exist until #11323 merges), the
  commit is public, and the "Refusing to record CDN-served hashes"
  exception is raised.
* 9 new unit tests cover: attested path, missing-entry, hard-fail for
  public commits, warning fallback for private commits, no-fallback once
  attested, verified_json tamper rejection (flipped byte), elected-vs-
  build hash mismatch, get_binary_hashes completeness, and single-space
  SHA256SUMS parsing. 25/25 pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@basvandijk
basvandijk force-pushed the bas/verify-ledger-suite-artifacts branch from b832e4f to 98c5a1c Compare August 27, 2026 20:24
@basvandijk
basvandijk requested a balanced review from Copilot August 27, 2026 20:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@basvandijk
basvandijk force-pushed the bas/verify-ledger-suite-artifacts branch from 98c5a1c to 9ff7353 Compare August 27, 2026 22:04
basvandijk added a commit that referenced this pull request Aug 27, 2026
Fourth step for security finding 3618194 (MEDIUM, CWE-494), building on
the producer-side attestations of #11323 and fetch-attested-sums.sh of
#11324. mainnet_revisions.py recorded several hashes into
mainnet-icos-revisions.json straight from unauthenticated CDN data: the
dev update-image hash was the hash of the CDN download itself, and the
setup-os and binaries hashes were read from CDN-served SHA256SUMS. The
recording PR is auto-approved and auto-merged, and
bazel/mainnet-icos-{images,binaries}.bzl then "verify" downloads against
those same values -- self-consistency, not authenticity.

All CDN-derived hashes now flow through VersionArtifactSums, which
verifies each directory's SHA256SUMS via ci/scripts/fetch-attested-sums.sh
(`gh attestation verify --repo dfinity/ic --signer-workflow
dfinity/ic/.github/workflows/release-testing.yml --source-digest
<version>` -- elected versions are always rc/hotfix builds) before any
value is read from it:

* update_img_hash_dev now comes from the verified
  {guest,host}-os/update-img-dev SHA256SUMS instead of hashing the
  multi-hundred-MB image the cron used to download every 2 hours.
* setupos hashes and all 7 MAINNET_BINARIES hashes come from verified
  SHA256SUMS.
* launch-measurements.json (dev and the prod fallback) is byte-verified
  against the attested SHA256SUMS entry before parsing.
* New cross-check: the NNS-elected update-img hash from the proposal must
  equal the build-time hash in the verified SHA256SUMS -- a mismatch
  means the CDN does not serve what the NNS elected, and nothing is
  recorded.

Cutover policy (VersionArtifactSums docstring): a PUBLIC commit without a
verifiable attestation hard-fails -- no PR is created, the cron retries;
backfill by re-running release-testing.yml on the version's branch (the
rebuild is checked byte-for-byte against the CDN by rclone --immutable
--checksum before attest-uploads runs). A commit that is NOT public
(undisclosed security patch, built in ic-private and not attested here)
falls back to the pre-fix CDN behavior with a loud warning, time-bounded
until disclosure; CDN write access cannot remove a commit from the
public repository, so the fallback is not attacker-selectable within the
finding's threat model.

The update_saved_* functions now check is_record_up_to_date BEFORE
collecting the version's artifact info, so the 2-hourly cron stays cheap
and green while recorded versions predate the attestation rollout;
verification only runs when a new version is recorded.

The updater workflow gets the pinned gh CLI install (gh attestation
verify --source-digest needs gh >= 2.61; same step as
update-mainnet-canister-revisions.yaml) and its pull_request dry-run
trigger now also fires on changes to the updater script and
fetch-attested-sums.sh.

Verification, all run against the live APIs:

* `mainnet_revisions.py --dry-run icos` passes end-to-end (all records
  up-to-date, no attestation calls needed).
* get_replica_version_info() for the currently-elected 79c0105 fails
  closed: attestation lookup 404s (none exist until #11323 merges), the
  commit is public, and the "Refusing to record CDN-served hashes"
  exception is raised.
* 9 new unit tests cover: attested path, missing-entry, hard-fail for
  public commits, warning fallback for private commits, no-fallback once
  attested, verified_json tamper rejection (flipped byte), elected-vs-
  build hash mismatch, get_binary_hashes completeness, and single-space
  SHA256SUMS parsing. 25/25 pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@basvandijk
basvandijk requested a balanced review from Copilot August 28, 2026 11:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

basvandijk and others added 9 commits August 28, 2026 13:24
…ions

Second step for security finding 3618194 (MEDIUM, CWE-494), building on the
producer-side attestations of #11323. ledger-suite-release.yml downloaded
canister wasms plus the CDN's own SHA256SUMS and republished a pruned copy
of that file as the release's verification data. No downloaded byte was
ever hashed and compared: the only check was that each expected filename
had a SHA256SUMS line, so whoever can write to the CDN buckets could get
their artifacts published as an official release with matching checksums.

This introduces the shared verification primitive the remaining consumers
(tag-release.yml, mainnet_revisions.py) will reuse:

* ci/scripts/fetch-attested-sums.sh downloads a directory's SHA256SUMS
  from the CDN and verifies it with `gh attestation verify --repo
  dfinity/ic --signer-workflow <pipeline> --source-digest <commit>`
  before anything reads it. --source-digest pins the attestation to the
  exact commit, so replaying another commit's legitimately-attested
  SHA256SUMS also fails.
* .github/actions/fetch-verified-artifacts wraps it for workflows: pinned
  gh CLI install (house pattern), one attested-SHA256SUMS fetch per
  directory, then per file curl + `sha256sum --check` against the
  verified sums, failing closed on any missing entry or mismatch. It
  emits a SHA256SUMS of the verified hashes under the files' local names,
  so published checksums are the verified build-time values rather than
  re-hashes of the downloads.
* ledger-suite-release.yml replaces its fetch()-based download step with
  the action (signer-workflow: ci-kickoff.yml, since ledger-suite tags
  point at master commits). The release body, changelog and softprops
  steps are unchanged apart from reading the action's out-dir.

Verification: the failure paths were exercised directly. A malformed
commit id and '..' path segments are rejected; fetching the real
canisters/SHA256SUMS of master commit 79c0105 and verifying it fails
with a 404 from the attestations API (no attestations exist until #11323
merges) and a non-zero exit -- the workflow fails closed rather than
publishing unverified artifacts. Positive-path validation happens with
the first ledger-suite-*.rc1 tag on a post-#11323 master commit.

Note the enforcement/coverage propagation: a tag push runs the workflow
file at the tag's commit, so tags on commits containing both #11323 and
this change are automatically attested AND verified, while older tags
keep the old behavior. Tags on commits that predate the attestation
rollout can no longer be released with this workflow (documented in the
header).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Align with ARTIFACT_NAME_PATTERN in mainnet_revisions.py: require a
leading alphanumeric (rejecting names starting with '-', which downstream
shell globs could read as options, and with '.') and cap the length at
128 characters.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…erify failure

Addresses two review findings:

* fetch-verified-artifacts ran the verifying fetch inside a command
  substitution. Bash does not propagate errexit into $(...) subshells, so
  when fetch-attested-sums.sh failed (e.g. gh attestation verify
  rejecting a substituted file) the function continued to echo the path
  of the UNVERIFIED download and the action trusted it -- a complete
  bypass of the new check. The fetch now runs in the main shell and
  reports via a variable, the verifier writes to a temp name that is only
  moved into the cache on success, and inherit_errexit hardens the
  remaining substitutions. Reproduced the bypass and the fix with a
  minimal set -e harness.

* gh attestation verify proves "some file of this build has this digest",
  not "this digest belongs at this CDN path". One attestation covers all
  ~250 files a build uploads, so a CDN writer could serve one directory's
  legitimately-attested SHA256SUMS at another directory's URL (e.g. prod
  update-img sums at the update-img-dev path) and have consumers record a
  valid build hash for the wrong artifact. fetch-attested-sums.sh now
  additionally requires, via --format json, that a verified attestation
  records the file's digest under the subject name
  ic/<commit>/<subdir>/SHA256SUMS. The subject names are trustworthy
  because the pinned --signer-workflow's attest-uploads job generates
  them from its own upload manifest. Adds a jq dependency (present on
  ubuntu-latest runners).

Verified: the jq binding accepts the matching (name, digest) pair and
rejects a cross-directory substitution and an unknown name against a
synthetic verification result; the pre-existing failure paths (bad args,
missing attestation) still exit non-zero.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The number resolves to nothing a future reader can look up; the
comments already explain the property that matters (an integrity
anchor independent of the CDN).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Flattening the directory separator (s|/|_|) is not injective:
"foo/bar_baz" and "foo_bar/baz" shared a cache key, so the second
directory would silently reuse the first's verified SHA256SUMS instead
of verifying its own, weakening the per-directory subject binding when
both appear in one invocation. The cache is now keyed by the sha256 of
the exact directory string.

Addresses
#11324 (comment)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntract

Three review findings on the verification primitive:

- The signer-workflow pin fixes which workflow signed an attestation,
  not from which ref it ran: ci-kickoff.yml can be dispatched on
  arbitrary branches (and runs for dev-gh-* pushes and PRs), so anyone
  able to trigger a release build of an unreviewed branch could mint
  an acceptable attestation. fetch-attested-sums.sh now takes a
  required <source-ref-regex> and requires the same attestation entry
  to match it (certificate sourceRepositoryRef) AND carry the expected
  subject; ledger-suite pins refs/heads/master. Live-tested against a
  real branch-minted attestation: accepted only by its own ref regex,
  rejected by the master and release-branch pins.

- The release job's explicit permissions map zeroes everything
  unlisted; grant attestations: read for `gh attestation verify`.

- fetch-verified-artifacts now refuses a non-empty out-dir (a consumer
  globbing the directory must see exactly the verified files) and
  rejects duplicate local names.

Addresses
#11324 (comment)
#11324 (comment)
#11324 (comment)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
find does not dereference a command-line symlink by default, so an
out-dir that is a symlink to a non-empty directory looked empty while
the writes (and a caller's publication glob) follow the link — stale,
unverified files would survive in the advertised output directory.
find -H follows the start-point symlink only: a symlink to a non-empty
target is now rejected, a symlink to an empty one still works, and a
child symlink below the start point counts as an entry.

Addresses
#11324 (comment)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ples

Disclosed hotfixes are re-pushed to dfinity/ic as hotfix-* branches and
re-built there, so public-hotfix-* never mints attestations. [^/]+
instead of .+ because branch protections and workflow triggers match
hotfix-* with fnmatch, where * does not cross '/'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@basvandijk
basvandijk force-pushed the bas/verify-ledger-suite-artifacts branch from 81c0c06 to 6775080 Compare August 28, 2026 11:24
basvandijk added a commit that referenced this pull request Aug 28, 2026
Third step for security finding 3618194 (MEDIUM, CWE-494), building on the
producer-side attestations of #11323 and the fetch-verified-artifacts
action of #11324. tag-release.yml downloaded 9 tool binaries, 2 OS images
and 19 canister wasm/did pairs from the CDN with plain curl and published
sha256sums.txt computed FROM THE DOWNLOADS THEMSELVES, so whoever can
write to the CDN buckets could get their artifacts republished as an
official GitHub release with matching checksums.

Every download now goes through .github/actions/fetch-verified-artifacts:
the per-directory SHA256SUMS is verified with `gh attestation verify
--repo dfinity/ic --signer-workflow
dfinity/ic/.github/workflows/release-testing.yml --source-digest
<commit>` and each artifact is checked against the verified sums, failing
the release on any mismatch. Pinning the release-testing.yml signer also
enforces that a release-* tag can only ship commits that went through the
release-qualification pipeline. sha256sums.txt is now the verified
build-time hashes (plus the locally-assembled canisters.tar hash), not a
re-hash of the downloads.

Bugs fixed in passing:

* The OS images had silently never been attached: since 9bd0a40
  (2024-08) the download step saved setup-os-img.tar.zst /
  update-os-img.tar.zst while the publish step listed *.tar.gz, and
  softprops v0.1.15 only warned about the unmatched patterns. The image
  downloads are now dropped entirely (preserving the two-year de-facto
  release contents; the canonical image channel is the CDN + NNS election
  proposal hash, verified by ci/scripts/repro-check), and
  fail_on_unmatched_files makes any repeat of this bug fail loudly.
* sha256sums.txt claimed to cover the release but only ever hashed *.gz
  and *.tar; it now covers exactly what ships.
* On the workflow_dispatch path the softprops action defaulted tag_name
  to the dispatched ref, not the requested tag; tag_name is now passed
  explicitly.
* softprops/action-gh-release upgraded from the v0.1.15 pin to the same
  v2.4.2 pin used by ledger-suite-release.yml.

Verification: the workflow_dispatch dry-run path exercises checkout,
download and verification without publishing; run it with a tag-name
whose commit was built by release-testing.yml after #11323 merged. The
verification failure paths (missing attestation, flipped hash, wrong
--source-digest commit) were exercised in #11324 and fail closed. Note
that a push-tag trigger runs the workflow file at the tag's commit, so
this protects release-* tags on rc branches cut after this change reaches
master; the arm64/darwin binary directories additionally need the
SHA256SUMS files introduced by #11323.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
basvandijk added a commit that referenced this pull request Aug 28, 2026
Fourth step for security finding 3618194 (MEDIUM, CWE-494), building on
the producer-side attestations of #11323 and fetch-attested-sums.sh of
#11324. mainnet_revisions.py recorded several hashes into
mainnet-icos-revisions.json straight from unauthenticated CDN data: the
dev update-image hash was the hash of the CDN download itself, and the
setup-os and binaries hashes were read from CDN-served SHA256SUMS. The
recording PR is auto-approved and auto-merged, and
bazel/mainnet-icos-{images,binaries}.bzl then "verify" downloads against
those same values -- self-consistency, not authenticity.

All CDN-derived hashes now flow through VersionArtifactSums, which
verifies each directory's SHA256SUMS via ci/scripts/fetch-attested-sums.sh
(`gh attestation verify --repo dfinity/ic --signer-workflow
dfinity/ic/.github/workflows/release-testing.yml --source-digest
<version>` -- elected versions are always rc/hotfix builds) before any
value is read from it:

* update_img_hash_dev now comes from the verified
  {guest,host}-os/update-img-dev SHA256SUMS instead of hashing the
  multi-hundred-MB image the cron used to download every 2 hours.
* setupos hashes and all 7 MAINNET_BINARIES hashes come from verified
  SHA256SUMS.
* launch-measurements.json (dev and the prod fallback) is byte-verified
  against the attested SHA256SUMS entry before parsing.
* New cross-check: the NNS-elected update-img hash from the proposal must
  equal the build-time hash in the verified SHA256SUMS -- a mismatch
  means the CDN does not serve what the NNS elected, and nothing is
  recorded.

Cutover policy (VersionArtifactSums docstring): a PUBLIC commit without a
verifiable attestation hard-fails -- no PR is created, the cron retries;
backfill by re-running release-testing.yml on the version's branch (the
rebuild is checked byte-for-byte against the CDN by rclone --immutable
--checksum before attest-uploads runs). A commit that is NOT public
(undisclosed security patch, built in ic-private and not attested here)
falls back to the pre-fix CDN behavior with a loud warning, time-bounded
until disclosure; CDN write access cannot remove a commit from the
public repository, so the fallback is not attacker-selectable within the
finding's threat model.

The update_saved_* functions now check is_record_up_to_date BEFORE
collecting the version's artifact info, so the 2-hourly cron stays cheap
and green while recorded versions predate the attestation rollout;
verification only runs when a new version is recorded.

The updater workflow gets the pinned gh CLI install (gh attestation
verify --source-digest needs gh >= 2.61; same step as
update-mainnet-canister-revisions.yaml) and its pull_request dry-run
trigger now also fires on changes to the updater script and
fetch-attested-sums.sh.

Verification, all run against the live APIs:

* `mainnet_revisions.py --dry-run icos` passes end-to-end (all records
  up-to-date, no attestation calls needed).
* get_replica_version_info() for the currently-elected 79c0105 fails
  closed: attestation lookup 404s (none exist until #11323 merges), the
  commit is public, and the "Refusing to record CDN-served hashes"
  exception is raised.
* 9 new unit tests cover: attested path, missing-entry, hard-fail for
  public commits, warning fallback for private commits, no-fallback once
  attested, verified_json tamper rejection (flipped byte), elected-vs-
  build hash mismatch, get_binary_hashes completeness, and single-space
  SHA256SUMS parsing. 25/25 pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants