Repository navigation
Verify every OCI digest and Helm charts before caching - #583
Merged
Merged
Conversation
Manifests and digest-addressed artifacts were only checked against sha256 digests. A sha512 digest, which the OCI spec also allows, or a digest with any other algorithm was fetched and cached unchecked: a manifest requested as sha512:<hex>, or a tag whose Docker-Content-Digest header named one, was stored under that digest whatever the body was. Manifests are now checked against any digest in the reference or the Docker-Content-Digest header with the digest's own algorithm, and one the proxy can't compute is answered with DIGEST_INVALID and not cached. GetOrFetchArtifactFromURLWithDigest verifies sha512 (and sha384) digests through the declared-integrity check, so a mismatch is never committed, and refuses other algorithms with ErrUnsupportedDigest instead of fetching unverified. Blob routes still only accept sha256.
Without artifact streaming, a chart was fetched with no expected digest, committed to the cache, and only then compared with the index's digest by serveChart, which cleared it again. Until then the unverified chart was a cache entry like any other. The index digest is now always passed to the fetch, so a mismatching chart is discarded before it is committed, as it already was when streaming. serveChart's check stays for charts that were cached before.
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.
Two places where fetched content could be cached before it was verified.
OCI digests other than sha256. Manifests and digest-addressed artifacts were only checked when the digest was
sha256:. Asha512:digest, which the OCI spec also allows, or one with any other algorithm was fetched and cached unchecked: a manifest requested assha512:<hex>, or a tag whoseDocker-Content-Digestheader named a sha512 digest, was stored under that digest whatever the body was.Docker-Content-Digestwith that digest's algorithm. A digest the proxy can't compute getsDIGEST_INVALID(502) and nothing is cached.GetOrFetchArtifactFromURLWithDigestverifies sha512 (and sha384) through the existing declared-integrity check, so a mismatch is never committed, and refuses other algorithms withErrUnsupportedDigestinstead of fetching unverified. sha256 handling is unchanged. Blob routes still accept only sha256, as before.Helm charts. Without
stream_artifacts, a chart was fetched with no expected digest, committed to the cache, and only then compared with the index digest byserveChart, which cleared it again. The index digest is now always passed to the fetch, so a mismatching chart is discarded before it is committed, as already happened when streaming.Tests:
TestContainerHandler_ManifestDigestsAreVerified(replacesTestContainerHandler_ManifestNonSHA256DigestReferenceIsProxied, which asserted the old pass-through),TestFetchWithDigestVerifiesEveryAlgorithm(stored and streamed), andTestHelmHandler_RejectsChartDigestMismatchnow expects nothing left in storage.