Skip to content

fix(verifier): check the image manifest with one strict sha256sum grammar, in-process - #1251

Merged
kvinwang merged 2 commits into
nextfrom
fix/verifier-measurement-fidelity
Sep 24, 2026
Merged

kvinwang merged 2 commits into
nextfrom
fix/verifier-measurement-fidelity

Conversation

@kvinwang

@kvinwang kvinwang commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

The downloaded image's entire content binding is sha256sum -c sha256sum.txt plus its exit status. Reproduced against coreutils 9.4:

ca978112...  a.txt
0000000000000000000000000000000000000000000000000000000000000000 b.txt   <- one space
$ sha256sum -c sha256sum.txt
a.txt: OK
sha256sum: WARNING: 1 line is improperly formatted
exit=0

b.txt is never checked and the command succeeds. validate_image_manifest_paths uses split_whitespace(), so it accepts the single-space line, and prune_unlisted_image_files also uses split_whitespace().nth(1), so it treats b.txt as listed and keeps it. The file is then measured — bzImage, ovmf.fd, initrd — with no content check at all.

More divergences between the two grammars, all reproduced: <hash> *bzImage makes GNU check bzImage (binary-mode marker) while the whitespace split sees *bzImage, so the file that is checked is the one that gets pruned; <hash> name junk and \<hash> name diverge; CRLF and uppercase hex are accepted by GNU while str::lines() strips the \r.

And a third grammar over the same file: dstack_types::sha256sum_entry_hash matched the second whitespace token and ignored the rest, so trailing tokens were dropped and near-duplicates evaded its duplicate-entry check.

accepted "239f59ed...  measurement.tdx.cbor  junk\n"            (dstack-types)
accepted "44136fa3...  metadata.json\n0000...0000 bzImage\n"     (verifier)

Unified into one grammar rather than just adding --strict. We generate the file ourselves with sha256sum <files> (os/image/assemble.sh), so the new dstack_types::sha256sum module accepts only that output: newline-terminated <lowercase sha256> <name> lines, no blank lines, no duplicates, names from [A-Za-z0-9._-] starting with an alphanumeric. sha256sum::entry_hash goes through the same parser. The verifier now hashes each listed file in-process instead of shelling out, which removes the subprocess and the parse differential.

Overlaps with #1248, which fixes sha256sum_entry_hash more narrowly (take the whole remainder of the line as the filename). This supersedes it; take one or the other, not both.

Verification

cargo test -p dstack-verifier -p dstack-types -p dstack-mr -p dstack-attest -p dstack-kms -p dstack-vmm green; clippy clean under the CI invocation. All 43 meta-dstack release manifests from v0.5.1 on parse; the older ./name manifests stay rejected, as they already are on next.

The other verifier fixes originally bundled here were split out into #1332, #1333, #1334 and #1335.

@kvinwang
kvinwang force-pushed the fix/verifier-measurement-fidelity branch from fabe6d5 to 9589a1d Compare September 23, 2026 04:56
@kvinwang kvinwang changed the title fix(verifier): the image manifest check accepts lines sha256sum silently skipped, and the --debug RTMR diff is dead code fix(verifier): check the image manifest with one strict sha256sum grammar, in-process Sep 23, 2026
@kvinwang
kvinwang force-pushed the fix/verifier-measurement-fidelity branch from 9589a1d to 2b9e074 Compare September 23, 2026 06:54
@kvinwang
kvinwang force-pushed the fix/verifier-measurement-fidelity branch from 2b9e074 to 13589f5 Compare September 24, 2026 02:34
@kvinwang
kvinwang added this pull request to stack #1342 September 24, 2026 02:37
sha256sum.txt is what os_image_hash commits to, and it was read by two
different grammars. sha256sum_entry_hash split on whitespace and matched the
second token, so `<hash>  name  junk` matched a name GNU sha256sum resolves as
`name  junk`, while `<hash> *name` -- valid binary-mode syntax naming `name` --
matched nothing and made the entry look missing.

We generate the file ourselves with `sha256sum <files>` in
os/image/assemble.sh, so accept only that output: newline-terminated
`<lowercase sha256>  <name>` lines, no blank lines, no duplicates, and names
made of [A-Za-z0-9._-] starting with an alphanumeric. The parser lives in its
own dstack_types::sha256sum module.
@kvinwang
kvinwang force-pushed the fix/verifier-measurement-fidelity branch from 13589f5 to 348ce13 Compare September 24, 2026 02:54
…g sha256sum -c

The whole content binding for a downloaded OS image was `sha256sum -c
sha256sum.txt` plus its exit status. GNU coreutils skips an improperly
formatted line with a warning and still exits 0:

    5891b5...  a.txt
    0000...0000 b.txt        <- one space
    $ sha256sum -c sha256sum.txt
    a.txt: OK
    sha256sum: WARNING: 1 line is improperly formatted
    exit=0

validate_image_manifest_paths split on whitespace, so it accepted that line,
and prune_unlisted_image_files did too, so b.txt survived the prune. bzImage,
ovmf.fd and the initrd were then measured with no content check at all.
`<hash> *bzImage` went wrong the other way: coreutils checks `bzImage`, the
whitespace split sees `*bzImage`, and the file that was checked is the one
that gets pruned.

Parse the manifest once with dstack_types::sha256sum::parse and hash each
listed file here. One parser, one grammar, and a missing or mismatched file
is an error rather than a warning on someone else's stderr.
@kvinwang
kvinwang force-pushed the fix/verifier-measurement-fidelity branch from 348ce13 to 41467d5 Compare September 24, 2026 03:03
@kvinwang
kvinwang merged commit 3d86cf8 into next Sep 24, 2026
11 checks passed
@kvinwang
kvinwang deleted the fix/verifier-measurement-fidelity branch September 24, 2026 03:16
kvinwang added a commit that referenced this pull request Sep 25, 2026
The verifier now retries a transient image download failure (#1388), so a
two-second download timeout expires before the download error is reported.
Give the one-shot rows enough time to observe the retries, and assert that
a refused connection is retried, a 404 is not, and a 503 recovers.

Also assert the in-process manifest digest check on a tampered archive
(#1251, #1337) and that the removed debug request field is ignored and the
debug response fields are gone (#1332).

Signed-off-by: Kevin Wang <wy721@qq.com>
kvinwang added a commit that referenced this pull request Sep 25, 2026
…bounds

#1251 renamed the manifest confinement test the image-download case ran by
name. Run its replacement plus the tests that pin the sha256sum grammar,
the end-to-end image binding (#1337) and the truncated-download retry
(#1388), and extend the measurement cache case with shape-only keying
(#1334) and the bounded cache directory (#1369).

Signed-off-by: Kevin Wang <wy721@qq.com>
kvinwang added a commit that referenced this pull request Sep 25, 2026
#1251 replaced image_paths_must_be_confined_and_manifest_paths_must_be_flat
with every_manifest_entry_is_checked_before_the_image_is_accepted, so the
evidence compatibility matrix no longer found its image-manifest row.

Signed-off-by: Kevin Wang <wy721@qq.com>
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.

1 participant