Verify OCI blob content against its descriptor - #688
Open
Philip Lombardi (plombardi89) wants to merge 1 commit into
Open
Verify OCI blob content against its descriptor#688Philip Lombardi (plombardi89) wants to merge 1 commit into
Philip Lombardi (plombardi89) wants to merge 1 commit into
Conversation
Blobs fetched from an OCI registry were never checked against their descriptor digest. oras-go's Repository.Fetch only compares the Docker-Content-Digest response header, and returns success when the registry omits that header entirely, so the bytes themselves were never hashed. Content of the declared size but the wrong bytes was accepted. This affects every oci:// artifact source, including offline bootstrap bundles, and it silently weakened the integrity story for anything distributed that way. Wrap the fetched body so the stream is hashed as it is read and the digest is checked once the content is complete. The failure is reported through Read rather than Close. Callers reliably check read errors and frequently ignore the error from Close, so reporting it there would let corrupt content through unnoticed. A mismatch therefore cannot be mistaken for a clean end of stream, which is pinned by a test. Found while evaluating whether a checksum sidecar could be dropped for oci:// sources on the grounds that OCI blobs are content-addressed. They are, but nothing was enforcing it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Blobs fetched from an OCI registry were never checked against their descriptor
digest.
oras-go'sRepository.FetchcallsverifyContentDigest, which compares onlythe
Docker-Content-Digestresponse header against the expected digest, andreturns success when the registry omits that header:
The bytes themselves are never hashed. Content of the declared size but the
wrong bytes was accepted, and a registry that omits the header got no integrity
check at all.
Impact
This affects every
oci://artifact source reachingpkg/agent/internal/ociartifact.Open, including offline bootstrap bundlesresolved through
bootstrapartifacts. Those are binaries the agent installsinto the node root filesystem, so the content is executed.
Some callers layer their own protection:
DownloadWithSHA256Verificationhashesthe stream, and Kubernetes binaries, CoreDNS and container image archives fetch
a sibling
.sha256. Callers usingOpenorReadAlldirectly, and thecomponents fetched without a checksum sidecar, had none.
Fix
Wrap the fetched body so the stream is hashed as it is read, and check the
digest once the content is complete, using
oras-go'scontent.VerifyReader.The failure is reported through
Read, notClose. Callers reliably checkread errors and frequently ignore the error from
Close, so reporting amismatch there would let corrupt content through unnoticed. A digest mismatch
therefore cannot be mistaken for a clean end of stream, which is pinned by a
test.
Tests
Five cases on the wrapper, including the one
oras-godoes not catch on itsown: content of exactly the declared size with substituted bytes. Also covers
truncated and overlong streams, and asserts that the failure arrives via
Readwhile
Closestays clean.How this was found
While evaluating whether a checksum sidecar could be dropped for
oci://sources on the grounds that OCI blobs are content-addressed. They are addressed
by digest, but nothing was enforcing it.
Split out of #687 because it is a correctness issue in a shared code path,
independent of the Azure Container Linux work it was found during.