Skip to content

Commit eeb7795

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 9f53510 commit eeb7795

File tree

15 files changed

+481
-125
lines changed

15 files changed

+481
-125
lines changed

‎pkg/api/controller_test.go‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11167,6 +11167,167 @@ 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-sha256"
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 SHA384 single-arch image", t, func() {
11214+
image := CreateImageWithDigestAlgorithm(godigest.SHA384).
11215+
RandomLayers(1, 10).DefaultConfig().Build()
11216+
11217+
name := "algo-sha384"
11218+
tag := "singlearch"
11219+
11220+
err := UploadImage(image, baseURL, name, tag)
11221+
So(err, ShouldBeNil)
11222+
11223+
client := resty.New()
11224+
11225+
// The server picks canonical digests when tags are pushed
11226+
// See https://github.com/opencontainers/distribution-spec/issues/494
11227+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
11228+
// but there is no way to specify a client preference
11229+
// so all we can do is verify the correct algorithm is returned
11230+
11231+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
11232+
11233+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
11234+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
11235+
})
11236+
11237+
Convey("Test SHA512 multi-arch image", t, func() {
11238+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
11239+
DefaultConfig().Build()
11240+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
11241+
DefaultConfig().Build()
11242+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA512).
11243+
Images([]Image{subImage1, subImage2}).Build()
11244+
11245+
name := "algo-sha256"
11246+
tag := "multiarch"
11247+
11248+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
11249+
So(err, ShouldBeNil)
11250+
11251+
client := resty.New()
11252+
11253+
// The server picks canonical digests when tags are pushed
11254+
// See https://github.com/opencontainers/distribution-spec/issues/494
11255+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
11256+
// but there is no way to specify a client preference
11257+
// so all we can do is verify the correct algorithm is returned
11258+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
11259+
11260+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
11261+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
11262+
11263+
// While the expected multiarch manifest digest is always using the canonical algorithm
11264+
// the sub-imgage manifest digest can use any algorith
11265+
verifyReturnedManifestDigest(t, client, baseURL, name,
11266+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
11267+
verifyReturnedManifestDigest(t, client, baseURL, name,
11268+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
11269+
})
11270+
11271+
Convey("Test SHA384 multi-arch image", t, func() {
11272+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
11273+
DefaultConfig().Build()
11274+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
11275+
DefaultConfig().Build()
11276+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA384).
11277+
Images([]Image{subImage1, subImage2}).Build()
11278+
11279+
name := "algo-sha384"
11280+
tag := "multiarch"
11281+
11282+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
11283+
So(err, ShouldBeNil)
11284+
11285+
client := resty.New()
11286+
11287+
// The server picks canonical digests when tags are pushed
11288+
// See https://github.com/opencontainers/distribution-spec/issues/494
11289+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
11290+
// but there is no way to specify a client preference
11291+
// so all we can do is verify the correct algorithm is returned
11292+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
11293+
11294+
verifyReturnedManifestDigest(t, client, baseURL, name, tag, expectedDigestStr)
11295+
verifyReturnedManifestDigest(t, client, baseURL, name, expectedDigestStr, expectedDigestStr)
11296+
11297+
// While the expected multiarch manifest digest is always using the canonical algorithm
11298+
// the sub-imgage manifest digest can use any algorith
11299+
verifyReturnedManifestDigest(t, client, baseURL, name,
11300+
subImage1.ManifestDescriptor.Digest.String(), subImage1.ManifestDescriptor.Digest.String())
11301+
verifyReturnedManifestDigest(t, client, baseURL, name,
11302+
subImage2.ManifestDescriptor.Digest.String(), subImage2.ManifestDescriptor.Digest.String())
11303+
})
11304+
}
11305+
11306+
func verifyReturnedManifestDigest(t *testing.T, client *resty.Client, baseURL, repoName,
11307+
reference, expectedDigestStr string,
11308+
) {
11309+
t.Helper()
11310+
11311+
t.Logf("Verify Docker-Content-Digest returned for repo %s reference %s is %s",
11312+
repoName, reference, expectedDigestStr)
11313+
11314+
getResponse, err := client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, repoName, reference))
11315+
So(err, ShouldBeNil)
11316+
So(getResponse, ShouldNotBeNil)
11317+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
11318+
11319+
contentDigestStr := getResponse.Header().Get("Docker-Content-Digest")
11320+
So(contentDigestStr, ShouldEqual, expectedDigestStr)
11321+
11322+
getResponse, err = client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, repoName, reference))
11323+
So(err, ShouldBeNil)
11324+
So(getResponse, ShouldNotBeNil)
11325+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
11326+
11327+
contentDigestStr = getResponse.Header().Get("Docker-Content-Digest")
11328+
So(contentDigestStr, ShouldEqual, expectedDigestStr)
11329+
}
11330+
1117011331
func getEmptyImageConfig() ([]byte, godigest.Digest) {
1117111332
config := ispec.Image{}
1117211333

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

‎pkg/storage/gc/gc_internal_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,8 +427,8 @@ func TestGarbageCollectWithMockedImageStore(t *testing.T) {
427427
GetIndexContentFn: func(repo string) ([]byte, error) {
428428
return returnedIndexJSONBuf, nil
429429
},
430-
GetAllBlobsFn: func(repo string) ([]string, error) {
431-
return []string{}, errGC
430+
GetAllBlobsFn: func(repo string) ([]godigest.Digest, error) {
431+
return []godigest.Digest{}, errGC
432432
},
433433
}
434434

0 commit comments

Comments
 (0)