Skip to content

Commit 22f7136

Browse files
andaaronandaaron
committed
fix(digests): do not mandate sha256 as the only algorithm used for hashing blobs
Signed-off-by: Andrei Aaron <[email protected]>
1 parent 3f97f87 commit 22f7136

File tree

15 files changed

+485
-129
lines changed

15 files changed

+485
-129
lines changed

‎pkg/api/controller_test.go‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10466,6 +10466,167 @@ func RunAuthorizationTests(t *testing.T, client *resty.Client, baseURL, user str
1046610466
})
1046710467
}
1046810468

10469+
func TestSupportedDigestAlgorithms(t *testing.T) {
10470+
port := test.GetFreePort()
10471+
baseURL := test.GetBaseURL(port)
10472+
10473+
conf := config.New()
10474+
conf.HTTP.Port = port
10475+
10476+
dir := t.TempDir()
10477+
10478+
ctlr := api.NewController(conf)
10479+
ctlr.Config.Storage.RootDirectory = dir
10480+
ctlr.Config.Storage.Dedupe = false
10481+
ctlr.Config.Storage.GC = false
10482+
10483+
cm := test.NewControllerManager(ctlr)
10484+
10485+
cm.StartAndWait(port)
10486+
defer cm.StopServer()
10487+
10488+
Convey("Test SHA512 single-arch image", t, func() {
10489+
image := CreateImageWithDigestAlgorithm(godigest.SHA512).
10490+
RandomLayers(1, 10).DefaultConfig().Build()
10491+
10492+
name := "algo-sha256"
10493+
tag := "singlearch"
10494+
10495+
err := UploadImage(image, baseURL, name, tag)
10496+
So(err, ShouldBeNil)
10497+
10498+
client := resty.New()
10499+
10500+
// The server picks canonical digests when tags are pushed
10501+
// See https://github.com/opencontainers/distribution-spec/issues/494
10502+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10503+
// but there is no way to specify a client preference
10504+
// so all we can do is verify the correct algorithm is returned
10505+
10506+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
10507+
10508+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10509+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10510+
})
10511+
10512+
Convey("Test SHA384 single-arch image", t, func() {
10513+
image := CreateImageWithDigestAlgorithm(godigest.SHA384).
10514+
RandomLayers(1, 10).DefaultConfig().Build()
10515+
10516+
name := "algo-sha384"
10517+
tag := "singlearch"
10518+
10519+
err := UploadImage(image, baseURL, name, tag)
10520+
So(err, ShouldBeNil)
10521+
10522+
client := resty.New()
10523+
10524+
// The server picks canonical digests when tags are pushed
10525+
// See https://github.com/opencontainers/distribution-spec/issues/494
10526+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10527+
// but there is no way to specify a client preference
10528+
// so all we can do is verify the correct algorithm is returned
10529+
10530+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
10531+
10532+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10533+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10534+
})
10535+
10536+
Convey("Test SHA512 multi-arch image", t, func() {
10537+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
10538+
DefaultConfig().Build()
10539+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
10540+
DefaultConfig().Build()
10541+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA512).
10542+
Images([]Image{subImage1, subImage2}).Build()
10543+
10544+
name := "algo-sha256"
10545+
tag := "multiarch"
10546+
10547+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
10548+
So(err, ShouldBeNil)
10549+
10550+
client := resty.New()
10551+
10552+
// The server picks canonical digests when tags are pushed
10553+
// See https://github.com/opencontainers/distribution-spec/issues/494
10554+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10555+
// but there is no way to specify a client preference
10556+
// so all we can do is verify the correct algorithm is returned
10557+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
10558+
10559+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10560+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10561+
10562+
// While the expected multiarch manifest digest is always using the canonical algorithm
10563+
// the sub-imgage manifest digest can use any algorith
10564+
verifyReturnedManifestDigest(t, client, baseURL, name,
10565+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
10566+
verifyReturnedManifestDigest(t, client, baseURL, name,
10567+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
10568+
})
10569+
10570+
Convey("Test SHA384 multi-arch image", t, func() {
10571+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
10572+
DefaultConfig().Build()
10573+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
10574+
DefaultConfig().Build()
10575+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA384).
10576+
Images([]Image{subImage1, subImage2}).Build()
10577+
10578+
name := "algo-sha384"
10579+
tag := "multiarch"
10580+
10581+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
10582+
So(err, ShouldBeNil)
10583+
10584+
client := resty.New()
10585+
10586+
// The server picks canonical digests when tags are pushed
10587+
// See https://github.com/opencontainers/distribution-spec/issues/494
10588+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10589+
// but there is no way to specify a client preference
10590+
// so all we can do is verify the correct algorithm is returned
10591+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
10592+
10593+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10594+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10595+
10596+
// While the expected multiarch manifest digest is always using the canonical algorithm
10597+
// the sub-imgage manifest digest can use any algorith
10598+
verifyReturnedManifestDigest(t, client, baseURL, name,
10599+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
10600+
verifyReturnedManifestDigest(t, client, baseURL, name,
10601+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
10602+
})
10603+
}
10604+
10605+
func verifyReturnedManifestDigest(t *testing.T, client *resty.Client, baseURL, repoName,
10606+
reference, expectedDigestStr string,
10607+
) {
10608+
t.Helper()
10609+
10610+
t.Logf("Verify Docker-Content-Digest returned for repo %s reference %s is %s",
10611+
repoName, reference, expectedDigestStr)
10612+
10613+
getResponse, err := client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, repoName, reference))
10614+
So(err, ShouldBeNil)
10615+
So(getResponse, ShouldNotBeNil)
10616+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10617+
10618+
contentDigestStr := getResponse.Header().Get("Docker-Content-Digest")
10619+
So(contentDigestStr, ShouldEqual, expectedDigestStr)
10620+
10621+
getResponse, err = client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, repoName, reference))
10622+
So(err, ShouldBeNil)
10623+
So(getResponse, ShouldNotBeNil)
10624+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10625+
10626+
contentDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10627+
So(contentDigestStr, ShouldEqual, expectedDigestStr)
10628+
}
10629+
1046910630
func getEmptyImageConfig() ([]byte, godigest.Digest) {
1047010631
config := ispec.Image{}
1047110632

‎pkg/storage/common/common.go‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -64,19 +64,19 @@ func GetManifestDescByReference(index ispec.Index, reference string) (ispec.Desc
6464

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

73-
return "", zerr.ErrBadManifest
73+
return zerr.ErrBadManifest
7474
}
7575

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

79-
return "", zerr.ErrBadManifest
79+
return zerr.ErrBadManifest
8080
}
8181

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

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

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

96-
return "", zerr.ErrBadManifest
96+
return zerr.ErrBadManifest
9797
}
9898

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

108-
return "", zerr.ErrBadManifest
108+
return zerr.ErrBadManifest
109109
}
110110

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

125-
return "", zerr.ErrBadManifest
125+
return zerr.ErrBadManifest
126126
}
127127
}
128128
}
@@ -131,50 +131,59 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy
131131
if err := json.Unmarshal(body, &m); err != nil {
132132
log.Error().Err(err).Msg("failed to unmarshal JSON")
133133

134-
return "", zerr.ErrBadManifest
134+
return zerr.ErrBadManifest
135135
}
136136
case ispec.MediaTypeImageIndex:
137137
// validate manifest
138138
if err := ValidateImageIndexSchema(body); err != nil {
139139
log.Error().Err(err).Msg("failed to validate OCIv1 image index manifest schema")
140140

141-
return "", zerr.NewError(zerr.ErrBadManifest).AddDetail("jsonSchemaValidation", err.Error())
141+
return zerr.NewError(zerr.ErrBadManifest).AddDetail("jsonSchemaValidation", err.Error())
142142
}
143143

144144
var indexManifest ispec.Index
145145
if err := json.Unmarshal(body, &indexManifest); err != nil {
146146
log.Error().Err(err).Msg("failed to unmarshal JSON")
147147

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

151151
for _, manifest := range indexManifest.Manifests {
152152
if ok, _, _, err := imgStore.StatBlob(repo, manifest.Digest); !ok || err != nil {
153153
log.Error().Err(err).Str("digest", manifest.Digest.String()).
154154
Msg("failed to stat manifest due to missing manifest blob")
155155

156-
return "", zerr.ErrBadManifest
156+
return zerr.ErrBadManifest
157157
}
158158
}
159159
}
160160

161-
return "", nil
161+
return nil
162162
}
163163

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

167-
d, err := godigest.Parse(digestStr)
168-
if err == nil {
169-
if d.String() != bodyDigest.String() {
170-
log.Error().Str("actual", bodyDigest.String()).Str("expected", d.String()).
171-
Msg("failed to validate manifest digest")
179+
if expectedDigest.String() != actualDigest.String() {
180+
log.Error().Str("actual", actualDigest.String()).Str("expected", expectedDigest.String()).
181+
Msg("failed to validate manifest digest")
172182

173-
return "", zerr.ErrBadManifest
174-
}
183+
return actualDigest, zerr.ErrBadManifest
175184
}
176185

177-
return bodyDigest, err
186+
return actualDigest, nil
178187
}
179188

180189
/*

‎pkg/storage/common/common_test.go‎

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

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

‎pkg/storage/gc/gc.go‎

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

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

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

591590
return err
592591
}
593592

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

600599
return err
601600
}

‎pkg/storage/gc/gc_internal_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,8 +440,8 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) {
440440
GetIndexContentFn: func(repo string) ([]byte, error) {
441441
return returnedIndexJSONBuf, nil
442442
},
443-
GetAllBlobsFn: func(repo string) ([]string, error) {
444-
return []string{}, errGC
443+
GetAllBlobsFn: func(repo string) ([]godigest.Digest, error) {
444+
return []godigest.Digest{}, errGC
445445
},
446446
}
447447

0 commit comments

Comments
 (0)