Skip to content

Commit d62e448

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 79e1402 commit d62e448

File tree

16 files changed

+486
-130
lines changed

16 files changed

+486
-130
lines changed

‎pkg/api/controller_test.go‎

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

10536+
func TestSupportedDigestAlgorithms(t *testing.T) {
10537+
port := test.GetFreePort()
10538+
baseURL := test.GetBaseURL(port)
10539+
10540+
conf := config.New()
10541+
conf.HTTP.Port = port
10542+
10543+
dir := t.TempDir()
10544+
10545+
ctlr := api.NewController(conf)
10546+
ctlr.Config.Storage.RootDirectory = dir
10547+
ctlr.Config.Storage.Dedupe = false
10548+
ctlr.Config.Storage.GC = false
10549+
10550+
cm := test.NewControllerManager(ctlr)
10551+
10552+
cm.StartAndWait(port)
10553+
defer cm.StopServer()
10554+
10555+
Convey("Test SHA512 single-arch image", t, func() {
10556+
image := CreateImageWithDigestAlgorithm(godigest.SHA512).
10557+
RandomLayers(1, 10).DefaultConfig().Build()
10558+
10559+
name := "algo-sha256"
10560+
tag := "singlearch"
10561+
10562+
err := UploadImage(image, baseURL, name, tag)
10563+
So(err, ShouldBeNil)
10564+
10565+
client := resty.New()
10566+
10567+
// The server picks canonical digests when tags are pushed
10568+
// See https://github.com/opencontainers/distribution-spec/issues/494
10569+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10570+
// but there is no way to specify a client preference
10571+
// so all we can do is verify the correct algorithm is returned
10572+
10573+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
10574+
10575+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10576+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10577+
})
10578+
10579+
Convey("Test SHA384 single-arch image", t, func() {
10580+
image := CreateImageWithDigestAlgorithm(godigest.SHA384).
10581+
RandomLayers(1, 10).DefaultConfig().Build()
10582+
10583+
name := "algo-sha384"
10584+
tag := "singlearch"
10585+
10586+
err := UploadImage(image, baseURL, name, tag)
10587+
So(err, ShouldBeNil)
10588+
10589+
client := resty.New()
10590+
10591+
// The server picks canonical digests when tags are pushed
10592+
// See https://github.com/opencontainers/distribution-spec/issues/494
10593+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10594+
// but there is no way to specify a client preference
10595+
// so all we can do is verify the correct algorithm is returned
10596+
10597+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
10598+
10599+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10600+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10601+
})
10602+
10603+
Convey("Test SHA512 multi-arch image", t, func() {
10604+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
10605+
DefaultConfig().Build()
10606+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
10607+
DefaultConfig().Build()
10608+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA512).
10609+
Images([]Image{subImage1, subImage2}).Build()
10610+
10611+
name := "algo-sha256"
10612+
tag := "multiarch"
10613+
10614+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
10615+
So(err, ShouldBeNil)
10616+
10617+
client := resty.New()
10618+
10619+
// The server picks canonical digests when tags are pushed
10620+
// See https://github.com/opencontainers/distribution-spec/issues/494
10621+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10622+
// but there is no way to specify a client preference
10623+
// so all we can do is verify the correct algorithm is returned
10624+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
10625+
10626+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10627+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10628+
10629+
// While the expected multiarch manifest digest is always using the canonical algorithm
10630+
// the sub-imgage manifest digest can use any algorith
10631+
verifyReturnedManifestDigest(t, client, baseURL, name,
10632+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
10633+
verifyReturnedManifestDigest(t, client, baseURL, name,
10634+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
10635+
})
10636+
10637+
Convey("Test SHA384 multi-arch image", t, func() {
10638+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
10639+
DefaultConfig().Build()
10640+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
10641+
DefaultConfig().Build()
10642+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA384).
10643+
Images([]Image{subImage1, subImage2}).Build()
10644+
10645+
name := "algo-sha384"
10646+
tag := "multiarch"
10647+
10648+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
10649+
So(err, ShouldBeNil)
10650+
10651+
client := resty.New()
10652+
10653+
// The server picks canonical digests when tags are pushed
10654+
// See https://github.com/opencontainers/distribution-spec/issues/494
10655+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10656+
// but there is no way to specify a client preference
10657+
// so all we can do is verify the correct algorithm is returned
10658+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
10659+
10660+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
10661+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
10662+
10663+
// While the expected multiarch manifest digest is always using the canonical algorithm
10664+
// the sub-imgage manifest digest can use any algorith
10665+
verifyReturnedManifestDigest(t, client, baseURL, name,
10666+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
10667+
verifyReturnedManifestDigest(t, client, baseURL, name,
10668+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
10669+
})
10670+
}
10671+
10672+
func verifyReturnedManifestDigest(t *testing.T, client *resty.Client, baseURL, repoName,
10673+
reference, expectedDigestStr string,
10674+
) {
10675+
t.Helper()
10676+
10677+
t.Logf("Verify Docker-Content-Digest returned for repo %s reference %s is %s",
10678+
repoName, reference, expectedDigestStr)
10679+
10680+
getResponse, err := client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, repoName, reference))
10681+
So(err, ShouldBeNil)
10682+
So(getResponse, ShouldNotBeNil)
10683+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10684+
10685+
contentDigestStr := getResponse.Header().Get("Docker-Content-Digest")
10686+
So(contentDigestStr, ShouldEqual, expectedDigestStr)
10687+
10688+
getResponse, err = client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, repoName, reference))
10689+
So(err, ShouldBeNil)
10690+
So(getResponse, ShouldNotBeNil)
10691+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10692+
10693+
contentDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10694+
So(contentDigestStr, ShouldEqual, expectedDigestStr)
10695+
}
10696+
1053610697
func getEmptyImageConfig() ([]byte, godigest.Digest) {
1053710698
config := ispec.Image{}
1053810699

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