Skip to content

Commit 23eb7bd

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 5b83937 commit 23eb7bd

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
@@ -10650,6 +10650,167 @@ func RunAuthorizationTests(t *testing.T, client *resty.Client, baseURL, user str
1065010650
})
1065110651
}
1065210652

10653+
func TestSupportedDigestAlgorithms(t *testing.T) {
10654+
port := test.GetFreePort()
10655+
baseURL := test.GetBaseURL(port)
10656+
10657+
conf := config.New()
10658+
conf.HTTP.Port = port
10659+
10660+
dir := t.TempDir()
10661+
10662+
ctlr := api.NewController(conf)
10663+
ctlr.Config.Storage.RootDirectory = dir
10664+
ctlr.Config.Storage.Dedupe = false
10665+
ctlr.Config.Storage.GC = false
10666+
10667+
cm := test.NewControllerManager(ctlr)
10668+
10669+
cm.StartAndWait(port)
10670+
defer cm.StopServer()
10671+
10672+
Convey("Test SHA512 single-arch image", t, func() {
10673+
image := CreateImageWithDigestAlgorithm(godigest.SHA512).
10674+
RandomLayers(1, 10).DefaultConfig().Build()
10675+
10676+
name := "algo-sha256"
10677+
tag := "singlearch"
10678+
10679+
err := UploadImage(image, baseURL, name, tag)
10680+
So(err, ShouldBeNil)
10681+
10682+
client := resty.New()
10683+
10684+
// The server picks canonical digests when tags are pushed
10685+
// See https://github.com/opencontainers/distribution-spec/issues/494
10686+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10687+
// but there is no way to specify a client preference
10688+
// so all we can do is verify the correct algorithm is returned
10689+
10690+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
10691+
10692+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10693+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10694+
})
10695+
10696+
Convey("Test SHA384 single-arch image", t, func() {
10697+
image := CreateImageWithDigestAlgorithm(godigest.SHA384).
10698+
RandomLayers(1, 10).DefaultConfig().Build()
10699+
10700+
name := "algo-sha384"
10701+
tag := "singlearch"
10702+
10703+
err := UploadImage(image, baseURL, name, tag)
10704+
So(err, ShouldBeNil)
10705+
10706+
client := resty.New()
10707+
10708+
// The server picks canonical digests when tags are pushed
10709+
// See https://github.com/opencontainers/distribution-spec/issues/494
10710+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10711+
// but there is no way to specify a client preference
10712+
// so all we can do is verify the correct algorithm is returned
10713+
10714+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
10715+
10716+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10717+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10718+
})
10719+
10720+
Convey("Test SHA512 multi-arch image", t, func() {
10721+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
10722+
DefaultConfig().Build()
10723+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
10724+
DefaultConfig().Build()
10725+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA512).
10726+
Images([]Image{subImage1, subImage2}).Build()
10727+
10728+
name := "algo-sha256"
10729+
tag := "multiarch"
10730+
10731+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
10732+
So(err, ShouldBeNil)
10733+
10734+
client := resty.New()
10735+
10736+
// The server picks canonical digests when tags are pushed
10737+
// See https://github.com/opencontainers/distribution-spec/issues/494
10738+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10739+
// but there is no way to specify a client preference
10740+
// so all we can do is verify the correct algorithm is returned
10741+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
10742+
10743+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10744+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10745+
10746+
// While the expected multiarch manifest digest is always using the canonical algorithm
10747+
// the sub-imgage manifest digest can use any algorith
10748+
verifyReturnedManifestDigest(t, client, baseURL, name,
10749+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
10750+
verifyReturnedManifestDigest(t, client, baseURL, name,
10751+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
10752+
})
10753+
10754+
Convey("Test SHA384 multi-arch image", t, func() {
10755+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
10756+
DefaultConfig().Build()
10757+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
10758+
DefaultConfig().Build()
10759+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA384).
10760+
Images([]Image{subImage1, subImage2}).Build()
10761+
10762+
name := "algo-sha384"
10763+
tag := "multiarch"
10764+
10765+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
10766+
So(err, ShouldBeNil)
10767+
10768+
client := resty.New()
10769+
10770+
// The server picks canonical digests when tags are pushed
10771+
// See https://github.com/opencontainers/distribution-spec/issues/494
10772+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10773+
// but there is no way to specify a client preference
10774+
// so all we can do is verify the correct algorithm is returned
10775+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
10776+
10777+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10778+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10779+
10780+
// While the expected multiarch manifest digest is always using the canonical algorithm
10781+
// the sub-imgage manifest digest can use any algorith
10782+
verifyReturnedManifestDigest(t, client, baseURL, name,
10783+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
10784+
verifyReturnedManifestDigest(t, client, baseURL, name,
10785+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
10786+
})
10787+
}
10788+
10789+
func verifyReturnedManifestDigest(t *testing.T, client *resty.Client, baseURL, repoName,
10790+
reference, expectedDigestStr string,
10791+
) {
10792+
t.Helper()
10793+
10794+
t.Logf("Verify Docker-Content-Digest returned for repo %s reference %s is %s",
10795+
repoName, reference, expectedDigestStr)
10796+
10797+
getResponse, err := client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, repoName, reference))
10798+
So(err, ShouldBeNil)
10799+
So(getResponse, ShouldNotBeNil)
10800+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10801+
10802+
contentDigestStr := getResponse.Header().Get("Docker-Content-Digest")
10803+
So(contentDigestStr, ShouldEqual, expectedDigestStr)
10804+
10805+
getResponse, err = client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, repoName, reference))
10806+
So(err, ShouldBeNil)
10807+
So(getResponse, ShouldNotBeNil)
10808+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10809+
10810+
contentDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10811+
So(contentDigestStr, ShouldEqual, expectedDigestStr)
10812+
}
10813+
1065310814
func getEmptyImageConfig() ([]byte, godigest.Digest) {
1065410815
config := ispec.Image{}
1065510816

‎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)