pull/32687/merge
Ashvin 4 days ago committed by GitHub
commit bb702a3b0f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -19,6 +19,7 @@ import (
"bytes" "bytes"
"crypto/sha256" "crypto/sha256"
"encoding/hex" "encoding/hex"
"encoding/json"
"errors" "errors"
"fmt" "fmt"
"io" "io"
@ -29,6 +30,8 @@ import (
"path/filepath" "path/filepath"
"strings" "strings"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"helm.sh/helm/v4/internal/fileutil" "helm.sh/helm/v4/internal/fileutil"
ifs "helm.sh/helm/v4/internal/third_party/dep/fs" ifs "helm.sh/helm/v4/internal/third_party/dep/fs"
"helm.sh/helm/v4/internal/urlutil" "helm.sh/helm/v4/internal/urlutil"
@ -106,7 +109,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven
c.Cache = &DiskCache{Root: c.ContentCache} c.Cache = &DiskCache{Root: c.ContentCache}
slog.Debug("set up default downloader cache") slog.Debug("set up default downloader cache")
} }
hash, u, err := c.ResolveChartVersion(ref, version) hash, u, err := c.resolveCacheDigest(ref, version)
if err != nil { if err != nil {
return "", nil, err return "", nil, err
} }
@ -138,12 +141,20 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven
if pth, err := c.Cache.Get(digest32, CacheChart); err == nil { if pth, err := c.Cache.Get(digest32, CacheChart); err == nil {
fdata, err := os.ReadFile(pth) fdata, err := os.ReadFile(pth)
if err == nil { if err == nil {
if verr := verifyIndexDigest(ref, hash, digest32, fdata); verr != nil {
// An entry that does not hash to the digest it is filed
// under cannot be trusted, whoever wrote it. Drop it and
// download the chart again rather than serving it.
slog.Debug("discarding cache entry that does not match its digest", "id", hash)
_ = os.Remove(pth)
} else {
found = true found = true
data = bytes.NewBuffer(fdata) data = bytes.NewBuffer(fdata)
slog.Debug("found chart in cache", "id", hash) slog.Debug("found chart in cache", "id", hash)
} }
} }
} }
}
if !found { if !found {
c.Options = append(c.Options, getter.WithAcceptHeader("application/gzip,application/octet-stream")) c.Options = append(c.Options, getter.WithAcceptHeader("application/gzip,application/octet-stream"))
@ -152,6 +163,9 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven
if err != nil { if err != nil {
return "", nil, err return "", nil, err
} }
if err := verifyIndexDigest(ref, hash, digest32, data.Bytes()); err != nil {
return "", nil, err
}
} }
name := filepath.Base(u.Path) name := filepath.Base(u.Path)
@ -223,7 +237,7 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena
slog.Debug("set up default downloader cache") slog.Debug("set up default downloader cache")
} }
digestString, u, err := c.ResolveChartVersion(ref, version) digestString, u, err := c.resolveCacheDigest(ref, version)
if err != nil { if err != nil {
return "", nil, err return "", nil, err
} }
@ -248,18 +262,29 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena
copy(digest32[:], digest) copy(digest32[:], digest)
var pth string var pth string
var cached bool
// only fetch from the cache if we have a digest // only fetch from the cache if we have a digest
if len(digest) > 0 { if len(digest) > 0 {
pth, err = c.Cache.Get(digest32, CacheChart) cachePath, cerr := c.Cache.Get(digest32, CacheChart)
if err == nil { switch {
case cerr == nil:
// The cache is content addressed, but nothing has been enforcing
// that, so an entry written by an older version of Helm may not
// hash to the name it is filed under. Check before trusting it.
if verr := verifyCachedChart(ref, digestString, digest32, cachePath); verr != nil {
slog.Debug("discarding cache entry that does not match its digest", "id", digestString)
_ = os.Remove(cachePath)
} else {
pth = cachePath
cached = true
slog.Debug("found chart in cache", "id", digestString) slog.Debug("found chart in cache", "id", digestString)
} }
case !os.IsNotExist(cerr):
return "", nil, cerr
} }
if len(digest) == 0 || err != nil {
slog.Debug("attempting to download chart", "ref", ref, "version", version)
if err != nil && !os.IsNotExist(err) {
return "", nil, err
} }
if !cached {
slog.Debug("attempting to download chart", "ref", ref, "version", version)
// Get file not in the cache // Get file not in the cache
data, gerr := g.Get(u.String(), c.Options...) data, gerr := g.Get(u.String(), c.Options...)
@ -267,6 +292,12 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena
return "", nil, gerr return "", nil, gerr
} }
// Check the bytes against the digest the index published for them
// before they are written into the content cache under that digest.
if verr := verifyIndexDigest(ref, digestString, digest32, data.Bytes()); verr != nil {
return "", nil, verr
}
// Generate the digest // Generate the digest
if len(digest) == 0 { if len(digest) == 0 {
digest32 = sha256.Sum256(data.Bytes()) digest32 = sha256.Sum256(data.Bytes())
@ -592,6 +623,116 @@ func loadRepoConfig(file string) (*repo.File, error) {
return r, nil return r, nil
} }
// resolveCacheDigest resolves ref like ResolveChartVersion, but returns the
// digest the content cache should use for the chart archive.
//
// For a repository chart that is the index digest, which is already the sha256
// of the archive. For an OCI reference pinned to a digest it is not: that digest
// names the manifest, so an archive cached under it could never be checked
// against its key. In that case the manifest is fetched (the registry client
// verifies it against the pinned digest) and the digest of its chart layer is
// returned instead, which keeps every cache entry keyed by its own content.
//
// If the pinned digest names something other than an image manifest, such as
// an image index, no cache digest is returned. The chart is then downloaded
// rather than read from the cache, as it already is for a digest-only ref.
func (c *ChartDownloader) resolveCacheDigest(ref, version string) (string, *url.URL, error) {
d, u, err := c.ResolveChartVersion(ref, version)
if err != nil || d == "" || u.Scheme != registry.OCIScheme {
return d, u, err
}
layer, err := c.ociChartLayerDigest(u)
if err != nil {
return "", nil, fmt.Errorf("unable to resolve chart layer for %s: %w", ref, err)
}
return layer, u, nil
}
// ociChartLayerDigest fetches only the manifest u points at and returns the
// sha256 digest of its chart layer, or "" if u does not point at an image
// manifest.
func (c *ChartDownloader) ociChartLayerDigest(u *url.URL) (string, error) {
generic := c.RegistryClient.Generic()
result, err := generic.PullGeneric(strings.TrimPrefix(u.String(), registry.OCIScheme+"://"), registry.GenericPullOptions{
AllowedMediaTypes: []string{ocispec.MediaTypeImageManifest},
})
if err != nil {
return "", err
}
if result.Manifest.MediaType != ocispec.MediaTypeImageManifest {
return "", nil
}
data, err := generic.GetDescriptorData(result.MemoryStore, result.Manifest)
if err != nil {
return "", err
}
var manifest ocispec.Manifest
if err := json.Unmarshal(data, &manifest); err != nil {
return "", err
}
for _, layer := range manifest.Layers {
if layer.MediaType != registry.ChartLayerMediaType && layer.MediaType != registry.LegacyChartLayerMediaType {
continue
}
if layer.Digest.Algorithm() != "sha256" {
return "", fmt.Errorf("unsupported chart layer digest algorithm %q", layer.Digest.Algorithm())
}
if err := layer.Digest.Validate(); err != nil {
return "", err
}
return layer.Digest.String(), nil
}
return "", fmt.Errorf("manifest does not contain a layer with mediatype %s", registry.ChartLayerMediaType)
}
// verifyIndexDigest checks chart archive bytes against the sha256 digest the
// repository index publishes for them.
//
// An index and the archives it points at are routinely served from different
// hosts, so for an HTTP repository this digest is the only thing binding the
// index a user trusts to the bytes they actually receive. It used to be read
// only as a cache key, so a repository that served an archive not matching its
// own index was accepted without complaint.
//
// It is a no-op when the index carries no digest, which is the case for a chart
// referenced by a bare URL, so those keep working as before. For an OCI
// reference the digest is the chart layer digest from resolveCacheDigest.
func verifyIndexDigest(ref, digestString string, want [sha256.Size]byte, data []byte) error {
if digestString == "" {
return nil
}
return compareChartDigest(ref, sha256.Sum256(data), want)
}
// verifyCachedChart checks an archive already in the content cache against the
// digest it is filed under, hashing it as a stream so a large chart is not held
// in memory twice.
func verifyCachedChart(ref, digestString string, want [sha256.Size]byte, path string) error {
if digestString == "" {
return nil
}
f, err := os.Open(path)
if err != nil {
return err
}
defer f.Close()
h := sha256.New()
if _, err := io.Copy(h, f); err != nil {
return err
}
var got [sha256.Size]byte
copy(got[:], h.Sum(nil))
return compareChartDigest(ref, got, want)
}
func compareChartDigest(ref string, got, want [sha256.Size]byte) error {
if got != want {
return fmt.Errorf("chart %q does not match the digest recorded for it in the repository index: expected sha256:%s, got sha256:%s",
ref, hex.EncodeToString(want[:]), hex.EncodeToString(got[:]))
}
return nil
}
// stripDigestAlgorithm removes the algorithm prefix (e.g., "sha256:") from a digest string. // stripDigestAlgorithm removes the algorithm prefix (e.g., "sha256:") from a digest string.
// If no prefix is present, the original string is returned unchanged. // If no prefix is present, the original string is returned unchanged.
func stripDigestAlgorithm(digest string) string { func stripDigestAlgorithm(digest string) string {

@ -16,16 +16,26 @@ limitations under the License.
package downloader package downloader
import ( import (
"bytes"
"context"
"crypto/sha256" "crypto/sha256"
"encoding/hex" "encoding/hex"
"encoding/json"
"net"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"os" "os"
"path/filepath" "path/filepath"
"testing" "testing"
"time"
godigest "github.com/opencontainers/go-digest"
specs "github.com/opencontainers/image-spec/specs-go"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"oras.land/oras-go/v2/registry/remote"
"oras.land/oras-go/v2/registry/remote/auth"
"helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/internal/test/ensure"
"helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/cli"
@ -511,3 +521,318 @@ func TestStripDigestAlgorithm(t *testing.T) {
}) })
} }
} }
// writeRepoCacheIndex publishes the generated index under the name the repo
// cache looks for. repotest.Server.LinkIndices symlinks instead of copying,
// which needs a privilege Windows does not grant by default.
func writeRepoCacheIndex(t *testing.T, root string) {
t.Helper()
idx, err := os.ReadFile(filepath.Join(root, "index.yaml"))
require.NoError(t, err)
require.NoError(t, os.WriteFile(filepath.Join(root, "test-index.yaml"), idx, 0o644))
}
// tamperedChartServer serves a repository whose index records the real digest
// of signtest-0.1.0.tgz while the archive itself has been replaced. It returns
// the server, a downloader pointed at it, and the bytes now being served.
func tamperedChartServer(t *testing.T, contentCache string) (*repotest.Server, *ChartDownloader, []byte) {
t.Helper()
srv := repotest.NewTempServer(t, repotest.WithChartSourceGlob("testdata/*.tgz*"))
t.Cleanup(srv.Stop)
require.NoError(t, srv.CreateIndex())
writeRepoCacheIndex(t, srv.Root())
served := filepath.Join(srv.Root(), "signtest-0.1.0.tgz")
original, err := os.ReadFile(served)
require.NoError(t, err)
tampered := append(append([]byte(nil), original...), []byte("appended by a rewritten mirror")...)
require.NoError(t, os.WriteFile(served, tampered, 0o644))
repoFile := filepath.Join(srv.Root(), "repositories.yaml")
c := &ChartDownloader{
Out: os.Stderr,
Verify: VerifyNever,
RepositoryConfig: repoFile,
RepositoryCache: srv.Root(),
ContentCache: contentCache,
Getters: getter.All(&cli.EnvSettings{
RepositoryConfig: repoFile,
RepositoryCache: srv.Root(),
ContentCache: contentCache,
}),
Cache: &DiskCache{Root: contentCache},
}
return srv, c, tampered
}
func TestDownloadTo_RejectsChartNotMatchingIndexDigest(t *testing.T) {
contentCache := t.TempDir()
dest := t.TempDir()
_, c, _ := tamperedChartServer(t, contentCache)
_, _, err := c.DownloadTo("test/signtest", "0.1.0", dest)
require.Error(t, err, "a chart that does not match the index digest must not be accepted")
assert.Contains(t, err.Error(), "does not match the digest recorded for it in the repository index")
// Nothing may be left behind in dest for a later step to pick up.
entries, err := os.ReadDir(dest)
require.NoError(t, err)
assert.Empty(t, entries, "rejected chart must not be written to the destination")
}
func TestDownloadToCache_RejectsChartNotMatchingIndexDigest(t *testing.T) {
contentCache := t.TempDir()
_, c, _ := tamperedChartServer(t, contentCache)
digestString, _, err := c.ResolveChartVersion("test/signtest", "0.1.0")
require.NoError(t, err)
digestBytes, err := hex.DecodeString(stripDigestAlgorithm(digestString))
require.NoError(t, err)
var want [sha256.Size]byte
copy(want[:], digestBytes)
_, _, err = c.DownloadToCache("test/signtest", "0.1.0")
require.Error(t, err, "a chart that does not match the index digest must not be accepted")
assert.Contains(t, err.Error(), "does not match the digest recorded for it in the repository index")
// The rejected bytes must not have been filed in the content cache under
// the digest they failed to match.
_, err = c.Cache.Get(want, CacheChart)
assert.Error(t, err, "rejected chart must not be written to the content cache")
}
func TestDownloadToCache_DiscardsCacheEntryNotMatchingItsDigest(t *testing.T) {
srv := repotest.NewTempServer(t, repotest.WithChartSourceGlob("testdata/*.tgz*"))
defer srv.Stop()
require.NoError(t, srv.CreateIndex())
writeRepoCacheIndex(t, srv.Root())
repoFile := filepath.Join(srv.Root(), "repositories.yaml")
contentCache := t.TempDir()
c := ChartDownloader{
Out: os.Stderr,
Verify: VerifyNever,
RepositoryConfig: repoFile,
RepositoryCache: srv.Root(),
ContentCache: contentCache,
Getters: getter.All(&cli.EnvSettings{
RepositoryConfig: repoFile,
RepositoryCache: srv.Root(),
ContentCache: contentCache,
}),
Cache: &DiskCache{Root: contentCache},
}
digestString, _, err := c.ResolveChartVersion("test/signtest", "0.1.0")
require.NoError(t, err)
digestBytes, err := hex.DecodeString(stripDigestAlgorithm(digestString))
require.NoError(t, err)
var want [sha256.Size]byte
copy(want[:], digestBytes)
// Poison the content cache the way an older Helm could have: content that
// does not hash to the key it is stored under.
poison := []byte("not the chart this digest names")
_, err = c.Cache.Put(want, bytes.NewBuffer(poison), CacheChart)
require.NoError(t, err)
pth, _, err := c.DownloadToCache("test/signtest", "0.1.0")
require.NoError(t, err, "a bad cache entry should be replaced by a fresh download, not returned")
got, err := os.ReadFile(pth)
require.NoError(t, err)
assert.NotEqual(t, poison, got, "poisoned cache entry must not be served")
assert.Equal(t, want, sha256.Sum256(got), "served chart must hash to the index digest")
}
func TestIndexDigestVerificationScope(t *testing.T) {
good := []byte("chart bytes")
want := sha256.Sum256(good)
t.Run("mismatch is rejected", func(t *testing.T) {
err := verifyIndexDigest("ref", hex.EncodeToString(want[:]), want, []byte("other bytes"))
assert.Error(t, err)
})
t.Run("match is accepted", func(t *testing.T) {
err := verifyIndexDigest("ref", hex.EncodeToString(want[:]), want, good)
assert.NoError(t, err)
})
t.Run("no index digest is a no-op", func(t *testing.T) {
// A chart referenced by a bare URL has no index entry to check against.
err := verifyIndexDigest("ref", "", [sha256.Size]byte{}, []byte("anything"))
assert.NoError(t, err)
})
}
// ociChartDownloader pushes testdata/signtest-0.1.0.tgz to an in-process
// registry and returns a downloader wired to it, the chart reference pinned to
// the pushed manifest digest, the push result and the registry.
func ociChartDownloader(t *testing.T, contentCache string) (*ChartDownloader, string, *registry.PushResult, *repotest.OCIServer) {
t.Helper()
dir := t.TempDir()
srv, err := repotest.NewOCIServer(t, dir)
require.NoError(t, err)
go srv.ListenAndServe()
dialer := &net.Dialer{Timeout: time.Second}
require.Eventually(t, func() bool {
conn, err := dialer.DialContext(t.Context(), "tcp", srv.RegistryURL)
if err != nil {
return false
}
conn.Close()
return true
}, 30*time.Second, 20*time.Millisecond)
client, err := registry.NewClient(
registry.ClientOptCredentialsFile(filepath.Join(dir, "config.json")),
registry.ClientOptPlainHTTP(),
)
require.NoError(t, err)
require.NoError(t, client.Login(srv.RegistryURL,
registry.LoginOptBasicAuth(srv.TestUsername, srv.TestPassword),
registry.LoginOptInsecure(true),
registry.LoginOptPlainText(true)))
archive, err := os.ReadFile("testdata/signtest-0.1.0.tgz")
require.NoError(t, err)
pushed, err := client.Push(archive, srv.RegistryURL+"/u/ocitestuser/signtest:0.1.0")
require.NoError(t, err)
settings := &cli.EnvSettings{ContentCache: contentCache}
c := &ChartDownloader{
Out: os.Stderr,
Verify: VerifyNever,
ContentCache: contentCache,
Getters: getter.All(settings),
Options: []getter.Option{getter.WithRegistryClient(client)},
RegistryClient: client,
Cache: &DiskCache{Root: contentCache},
}
ref := "oci://" + srv.RegistryURL + "/u/ocitestuser/signtest@" + pushed.Manifest.Digest
return c, ref, pushed, srv
}
func digestKey(t *testing.T, d string) [sha256.Size]byte {
t.Helper()
b, err := hex.DecodeString(stripDigestAlgorithm(d))
require.NoError(t, err)
require.Len(t, b, sha256.Size)
var k [sha256.Size]byte
copy(k[:], b)
return k
}
func TestDownloadToCache_OCIKeyedByChartLayerDigest(t *testing.T) {
contentCache := t.TempDir()
c, ref, pushed, _ := ociChartDownloader(t, contentCache)
layer := digestKey(t, pushed.Chart.Digest)
pth, _, err := c.DownloadToCache(ref, "0.1.0")
require.NoError(t, err)
got, err := os.ReadFile(pth)
require.NoError(t, err)
assert.Equal(t, layer, sha256.Sum256(got), "cached chart must hash to its chart layer digest")
// The entry must be filed under the chart layer digest, where it can be
// checked, and not under the manifest digest, where it could not.
layerPath, err := c.Cache.Get(layer, CacheChart)
require.NoError(t, err)
assert.Equal(t, layerPath, pth)
_, err = c.Cache.Get(digestKey(t, pushed.Manifest.Digest), CacheChart)
assert.ErrorIs(t, err, os.ErrNotExist)
}
func TestDownloadToCache_OCIDiscardsCacheEntryNotMatchingItsDigest(t *testing.T) {
contentCache := t.TempDir()
c, ref, pushed, _ := ociChartDownloader(t, contentCache)
layer := digestKey(t, pushed.Chart.Digest)
poison := []byte("not the chart this digest names")
_, err := c.Cache.Put(layer, bytes.NewBuffer(poison), CacheChart)
require.NoError(t, err)
pth, _, err := c.DownloadToCache(ref, "0.1.0")
require.NoError(t, err, "a bad cache entry should be replaced by a fresh download, not returned")
got, err := os.ReadFile(pth)
require.NoError(t, err)
assert.Equal(t, layer, sha256.Sum256(got), "served chart must hash to its chart layer digest")
}
func TestDownloadTo_OCIIgnoresCacheEntryNotMatchingItsDigest(t *testing.T) {
contentCache := t.TempDir()
dest := t.TempDir()
c, ref, pushed, _ := ociChartDownloader(t, contentCache)
layer := digestKey(t, pushed.Chart.Digest)
// Before this change an OCI chart was cached under its manifest digest.
// Neither an entry like that nor a bad one under the layer digest may be
// served in place of the chart.
poison := []byte("not the chart this digest names")
_, err := c.Cache.Put(digestKey(t, pushed.Manifest.Digest), bytes.NewBuffer(poison), CacheChart)
require.NoError(t, err)
_, err = c.Cache.Put(layer, bytes.NewBuffer(poison), CacheChart)
require.NoError(t, err)
saved, _, err := c.DownloadTo(ref, "0.1.0", dest)
require.NoError(t, err)
got, err := os.ReadFile(saved)
require.NoError(t, err)
assert.Equal(t, layer, sha256.Sum256(got), "saved chart must hash to its chart layer digest")
}
func TestDownloadTo_OCIDigestOnlyRef(t *testing.T) {
c, ref, pushed, _ := ociChartDownloader(t, t.TempDir())
// With no version the registry client resolves no digest for the ref, so
// the cache is not consulted and the chart is downloaded directly.
saved, _, err := c.DownloadTo(ref, "", t.TempDir())
require.NoError(t, err)
got, err := os.ReadFile(saved)
require.NoError(t, err)
assert.Equal(t, digestKey(t, pushed.Chart.Digest), sha256.Sum256(got))
}
func TestDownloadToCache_OCIImageIndexRoot(t *testing.T) {
contentCache := t.TempDir()
c, _, pushed, srv := ociChartDownloader(t, contentCache)
layer := digestKey(t, pushed.Chart.Digest)
// Wrap the chart manifest in an image index and move the tag onto it.
repo, err := remote.NewRepository(srv.RegistryURL + "/u/ocitestuser/signtest")
require.NoError(t, err)
repo.PlainHTTP = true
repo.Client = &auth.Client{Credential: auth.StaticCredential(srv.RegistryURL, auth.Credential{
Username: srv.TestUsername,
Password: srv.TestPassword,
})}
ctx := context.Background()
manifest, err := repo.Resolve(ctx, "0.1.0")
require.NoError(t, err)
index, err := json.Marshal(ocispec.Index{
Versioned: specs.Versioned{SchemaVersion: 2},
MediaType: ocispec.MediaTypeImageIndex,
Manifests: []ocispec.Descriptor{manifest},
})
require.NoError(t, err)
indexDesc := ocispec.Descriptor{
MediaType: ocispec.MediaTypeImageIndex,
Digest: godigest.FromBytes(index),
Size: int64(len(index)),
}
require.NoError(t, repo.PushReference(ctx, indexDesc, bytes.NewReader(index), "0.1.0"))
ref := "oci://" + srv.RegistryURL + "/u/ocitestuser/signtest@" + indexDesc.Digest.String()
pth, _, err := c.DownloadToCache(ref, "0.1.0")
require.NoError(t, err, "a chart behind an image index must still download")
got, err := os.ReadFile(pth)
require.NoError(t, err)
assert.Equal(t, layer, sha256.Sum256(got))
_, err = c.Cache.Get(digestKey(t, indexDesc.Digest.String()), CacheChart)
require.ErrorIs(t, err, os.ErrNotExist, "chart must not be cached under the index digest")
saved, _, err := c.DownloadTo(ref, "0.1.0", t.TempDir())
require.NoError(t, err)
got, err = os.ReadFile(saved)
require.NoError(t, err)
assert.Equal(t, layer, sha256.Sum256(got))
}

Loading…
Cancel
Save