Skip to content

Commit 26be383

Browse files
andaaronandaaron
authored
fix(digests): do not mandate sha256 as the only algorithm used for hashing blobs (#2075)
Signed-off-by: Andrei Aaron <[email protected]>
1 parent 6421d8b commit 26be383

File tree

15 files changed

+528
-127
lines changed

15 files changed

+528
-127
lines changed

‎pkg/api/controller_test.go‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11167,6 +11167,211 @@ func RunAuthorizationTests(t *testing.T, client *resty.Client, baseURL, user str
1116711167
})
1116811168
}
1116911169

11170+
func TestSupportedDigestAlgorithms(t *testing.T) {
11171+
port := test.GetFreePort()
11172+
baseURL := test.GetBaseURL(port)
11173+
11174+
conf := config.New()
11175+
conf.HTTP.Port = port
11176+
11177+
dir := t.TempDir()
11178+
11179+
ctlr := api.NewController(conf)
11180+
ctlr.Config.Storage.RootDirectory = dir
11181+
ctlr.Config.Storage.Dedupe = false
11182+
ctlr.Config.Storage.GC = false
11183+
11184+
cm := test.NewControllerManager(ctlr)
11185+
11186+
cm.StartAndWait(port)
11187+
defer cm.StopServer()
11188+
11189+
Convey("Test SHA512 single-arch image", t, func() {
11190+
image := CreateImageWithDigestAlgorithm(godigest.SHA512).
11191+
RandomLayers(1, 10).DefaultConfig().Build()
11192+
11193+
name := "algo-sha512"
11194+
tag := "singlearch"
11195+
11196+
err := UploadImage(image, baseURL, name, tag)
11197+
So(err, ShouldBeNil)
11198+
11199+
client := resty.New()
11200+
11201+
// The server picks canonical digests when tags are pushed
11202+
// See https://github.com/opencontainers/distribution-spec/issues/494
11203+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
11204+
// but there is no way to specify a client preference
11205+
// so all we can do is verify the correct algorithm is returned
11206+
11207+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
11208+
11209+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
11210+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
11211+
})
11212+
11213+
Convey("Test SHA512 single-arch image pushed by digest", t, func() {
11214+
image := CreateImageWithDigestAlgorithm(godigest.SHA512).
11215+
RandomLayers(1, 11).DefaultConfig().Build()
11216+
11217+
name := "algo-sha512-2"
11218+
11219+
err := UploadImage(image, baseURL, name, image.DigestStr())
11220+
So(err, ShouldBeNil)
11221+
11222+
client := resty.New()
11223+
11224+
expectedDigestStr := image.DigestForAlgorithm(godigest.SHA512).String()
11225+
11226+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
11227+
})
11228+
11229+
Convey("Test SHA384 single-arch image", t, func() {
11230+
image := CreateImageWithDigestAlgorithm(godigest.SHA384).
11231+
RandomLayers(1, 10).DefaultConfig().Build()
11232+
11233+
name := "algo-sha384"
11234+
tag := "singlearch"
11235+
11236+
err := UploadImage(image, baseURL, name, tag)
11237+
So(err, ShouldBeNil)
11238+
11239+
client := resty.New()
11240+
11241+
// The server picks canonical digests when tags are pushed
11242+
// See https://github.com/opencontainers/distribution-spec/issues/494
11243+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
11244+
// but there is no way to specify a client preference
11245+
// so all we can do is verify the correct algorithm is returned
11246+
11247+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
11248+
11249+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
11250+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
11251+
})
11252+
11253+
Convey("Test SHA512 multi-arch image", t, func() {
11254+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
11255+
DefaultConfig().Build()
11256+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
11257+
DefaultConfig().Build()
11258+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA512).
11259+
Images([]Image{subImage1, subImage2}).Build()
11260+
11261+
name := "algo-sha512"
11262+
tag := "multiarch"
11263+
11264+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
11265+
So(err, ShouldBeNil)
11266+
11267+
client := resty.New()
11268+
11269+
// The server picks canonical digests when tags are pushed
11270+
// See https://github.com/opencontainers/distribution-spec/issues/494
11271+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
11272+
// but there is no way to specify a client preference
11273+
// so all we can do is verify the correct algorithm is returned
11274+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
11275+
11276+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
11277+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
11278+
11279+
// While the expected multiarch manifest digest is always using the canonical algorithm
11280+
// the sub-imgage manifest digest can use any algorith
11281+
verifyReturnedManifestDigest(t, client, baseURL, name,
11282+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
11283+
verifyReturnedManifestDigest(t, client, baseURL, name,
11284+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
11285+
})
11286+
11287+
Convey("Test SHA512 multi-arch image pushed by digest", t, func() {
11288+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
11289+
DefaultConfig().Build()
11290+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
11291+
DefaultConfig().Build()
11292+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA512).
11293+
Images([]Image{subImage1, subImage2}).Build()
11294+
11295+
name := "algo-sha512-2"
11296+
11297+
t.Log(multiarch.DigestStr())
11298+
11299+
err := UploadMultiarchImage(multiarch, baseURL, name, multiarch.DigestStr())
11300+
So(err, ShouldBeNil)
11301+
11302+
client := resty.New()
11303+
11304+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.SHA512).String()
11305+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
11306+
11307+
// While the expected multiarch manifest digest is always using the canonical algorithm
11308+
// the sub-imgage manifest digest can use any algorith
11309+
verifyReturnedManifestDigest(t, client, baseURL, name,
11310+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
11311+
verifyReturnedManifestDigest(t, client, baseURL, name,
11312+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
11313+
})
11314+
11315+
Convey("Test SHA384 multi-arch image", t, func() {
11316+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
11317+
DefaultConfig().Build()
11318+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
11319+
DefaultConfig().Build()
11320+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA384).
11321+
Images([]Image{subImage1, subImage2}).Build()
11322+
11323+
name := "algo-sha384"
11324+
tag := "multiarch"
11325+
11326+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
11327+
So(err, ShouldBeNil)
11328+
11329+
client := resty.New()
11330+
11331+
// The server picks canonical digests when tags are pushed
11332+
// See https://github.com/opencontainers/distribution-spec/issues/494
11333+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
11334+
// but there is no way to specify a client preference
11335+
// so all we can do is verify the correct algorithm is returned
11336+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
11337+
11338+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
11339+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
11340+
11341+
// While the expected multiarch manifest digest is always using the canonical algorithm
11342+
// the sub-imgage manifest digest can use any algorith
11343+
verifyReturnedManifestDigest(t, client, baseURL, name,
11344+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
11345+
verifyReturnedManifestDigest(t, client, baseURL, name,
11346+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
11347+
})
11348+
}
11349+
11350+
func verifyReturnedManifestDigest(t *testing.T, client *resty.Client, baseURL, repoName,
11351+
reference, expectedDigestStr string,
11352+
) {
11353+
t.Helper()
11354+
11355+
t.Logf("Verify Docker-Content-Digest returned for repo %s reference %s is %s",
11356+
repoName, reference, expectedDigestStr)
11357+
11358+
getResponse, err := client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, repoName, reference))
11359+
So(err, ShouldBeNil)
11360+
So(getResponse, ShouldNotBeNil)
11361+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
11362+
11363+
contentDigestStr := getResponse.Header().Get("Docker-Content-Digest")
11364+
So(contentDigestStr, ShouldEqual, expectedDigestStr)
11365+
11366+
getResponse, err = client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, repoName, reference))
11367+
So(err, ShouldBeNil)
11368+
So(getResponse, ShouldNotBeNil)
11369+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
11370+
11371+
contentDigestStr = getResponse.Header().Get("Docker-Content-Digest")
11372+
So(contentDigestStr, ShouldEqual, expectedDigestStr)
11373+
}
11374+
1117011375
func getEmptyImageConfig() ([]byte, godigest.Digest) {
1117111376
config := ispec.Image{}
1117211377

‎pkg/storage/common/common.go‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -63,19 +63,19 @@ func GetManifestDescByReference(index ispec.Index, reference string) (ispec.Desc
6363

6464
func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaType string, body []byte,
6565
log zlog.Logger,
66-
) (godigest.Digest, error) {
66+
) error {
6767
// validate the manifest
6868
if !IsSupportedMediaType(mediaType) {
6969
log.Debug().Interface("actual", mediaType).
7070
Msg("bad manifest media type")
7171

72-
return "", zerr.ErrBadManifest
72+
return zerr.ErrBadManifest
7373
}
7474

7575
if len(body) == 0 {
7676
log.Debug().Int("len", len(body)).Msg("invalid body length")
7777

78-
return "", zerr.ErrBadManifest
78+
return zerr.ErrBadManifest
7979
}
8080

8181
switch mediaType {
@@ -86,13 +86,13 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy
8686
if err := ValidateManifestSchema(body); err != nil {
8787
log.Error().Err(err).Msg("failed to validate OCIv1 image manifest schema")
8888

89-
return "", zerr.NewError(zerr.ErrBadManifest).AddDetail("jsonSchemaValidation", err.Error())
89+
return zerr.NewError(zerr.ErrBadManifest).AddDetail("jsonSchemaValidation", err.Error())
9090
}
9191

9292
if err := json.Unmarshal(body, &manifest); err != nil {
9393
log.Error().Err(err).Msg("failed to unmarshal JSON")
9494

95-
return "", zerr.ErrBadManifest
95+
return zerr.ErrBadManifest
9696
}
9797

9898
// validate blobs only for known media types
@@ -104,7 +104,7 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy
104104
log.Error().Err(err).Str("digest", manifest.Config.Digest.String()).
105105
Msg("failed to stat blob due to missing config blob")
106106

107-
return "", zerr.ErrBadManifest
107+
return zerr.ErrBadManifest
108108
}
109109

110110
// validate layers - a lightweight check if the blob is present
@@ -121,7 +121,7 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy
121121
log.Error().Err(err).Str("digest", layer.Digest.String()).
122122
Msg("failed to validate manifest due to missing layer blob")
123123

124-
return "", zerr.ErrBadManifest
124+
return zerr.ErrBadManifest
125125
}
126126
}
127127
}
@@ -130,43 +130,52 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy
130130
if err := ValidateImageIndexSchema(body); err != nil {
131131
log.Error().Err(err).Msg("failed to validate OCIv1 image index manifest schema")
132132

133-
return "", zerr.NewError(zerr.ErrBadManifest).AddDetail("jsonSchemaValidation", err.Error())
133+
return zerr.NewError(zerr.ErrBadManifest).AddDetail("jsonSchemaValidation", err.Error())
134134
}
135135

136136
var indexManifest ispec.Index
137137
if err := json.Unmarshal(body, &indexManifest); err != nil {
138138
log.Error().Err(err).Msg("failed to unmarshal JSON")
139139

140-
return "", zerr.ErrBadManifest
140+
return zerr.ErrBadManifest
141141
}
142142

143143
for _, manifest := range indexManifest.Manifests {
144144
if ok, _, _, err := imgStore.StatBlob(repo, manifest.Digest); !ok || err != nil {
145145
log.Error().Err(err).Str("digest", manifest.Digest.String()).
146146
Msg("failed to stat manifest due to missing manifest blob")
147147

148-
return "", zerr.ErrBadManifest
148+
return zerr.ErrBadManifest
149149
}
150150
}
151151
}
152152

153-
return "", nil
153+
return nil
154154
}
155155

156-
func GetAndValidateRequestDigest(body []byte, digestStr string, log zlog.Logger) (godigest.Digest, error) {
157-
bodyDigest := godigest.FromBytes(body)
156+
// Returns the canonical digest or the digest provided by the reference if any
157+
// Per spec, the canonical digest would always be returned to the client in
158+
// request headers, but that does not make sense if the client requested a different digest algorithm
159+
// See https://github.com/opencontainers/distribution-spec/issues/494
160+
func GetAndValidateRequestDigest(body []byte, reference string, log zlog.Logger) (
161+
godigest.Digest, error,
162+
) {
163+
expectedDigest, err := godigest.Parse(reference)
164+
if err != nil {
165+
// This is a non-digest reference
166+
return godigest.Canonical.FromBytes(body), err
167+
}
168+
169+
actualDigest := expectedDigest.Algorithm().FromBytes(body)
158170

159-
d, err := godigest.Parse(digestStr)
160-
if err == nil {
161-
if d.String() != bodyDigest.String() {
162-
log.Error().Str("actual", bodyDigest.String()).Str("expected", d.String()).
163-
Msg("failed to validate manifest digest")
171+
if expectedDigest.String() != actualDigest.String() {
172+
log.Error().Str("actual", actualDigest.String()).Str("expected", expectedDigest.String()).
173+
Msg("failed to validate manifest digest")
164174

165-
return "", zerr.ErrBadManifest
166-
}
175+
return actualDigest, zerr.ErrBadManifest
167176
}
168177

169-
return bodyDigest, err
178+
return actualDigest, nil
170179
}
171180

172181
/*

‎pkg/storage/common/common_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,29 @@ func TestValidateManifest(t *testing.T) {
5252
So(err, ShouldBeNil)
5353
So(clen, ShouldEqual, len(cblob))
5454

55+
Convey("bad manifest mediatype", func() {
56+
manifest := ispec.Manifest{}
57+
58+
body, err := json.Marshal(manifest)
59+
So(err, ShouldBeNil)
60+
61+
_, _, err = imgStore.PutImageManifest("test", "1.0", ispec.MediaTypeImageConfig, body)
62+
So(err, ShouldNotBeNil)
63+
So(err, ShouldEqual, zerr.ErrBadManifest)
64+
})
65+
66+
Convey("empty manifest with bad media type", func() {
67+
_, _, err = imgStore.PutImageManifest("test", "1.0", ispec.MediaTypeImageConfig, []byte(""))
68+
So(err, ShouldNotBeNil)
69+
So(err, ShouldEqual, zerr.ErrBadManifest)
70+
})
71+
72+
Convey("empty manifest with correct media type", func() {
73+
_, _, err = imgStore.PutImageManifest("test", "1.0", ispec.MediaTypeImageManifest, []byte(""))
74+
So(err, ShouldNotBeNil)
75+
So(err, ShouldEqual, zerr.ErrBadManifest)
76+
})
77+
5578
Convey("bad manifest schema version", func() {
5679
manifest := ispec.Manifest{
5780
Config: ispec.Descriptor{

‎pkg/storage/gc/gc.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -581,20 +581,19 @@ func (gc GarbageCollect) removeUnreferencedBlobs(repo string, delay time.Duratio
581581

582582
gcBlobs := make([]godigest.Digest, 0)
583583

584-
for _, blob := range allBlobs {
585-
digest := godigest.NewDigestFromEncoded(godigest.SHA256, blob)
584+
for _, digest := range allBlobs {
586585
if err = digest.Validate(); err != nil {
587-
log.Error().Err(err).Str("module", "gc").Str("repository", repo).Str("digest", blob).
588-
Msg("failed to parse digest")
586+
log.Error().Err(err).Str("module", "gc").Str("repository", repo).
587+
Str("digest", digest.String()).Msg("failed to parse digest")
589588

590589
return err
591590
}
592591

593592
if _, ok := refBlobs[digest.String()]; !ok {
594593
canGC, err := isBlobOlderThan(gc.imgStore, repo, digest, delay, log)
595594
if err != nil {
596-
log.Error().Err(err).Str("module", "gc").Str("repository", repo).Str("digest", blob).
597-
Msg("failed to determine GC delay")
595+
log.Error().Err(err).Str("module", "gc").Str("repository", repo).
596+
Str("digest", digest.String()).Msg("failed to determine GC delay")
598597

599598
return err
600599
}

0 commit comments

Comments
 (0)