Skip to content

Commit 9eed9c5

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 0de2210 commit 9eed9c5

File tree

14 files changed

+482
-133
lines changed

14 files changed

+482
-133
lines changed

‎pkg/api/controller_test.go

Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10533,6 +10533,245 @@ 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+
cm.StartAndWait(port)
10552+
defer cm.StopServer()
10553+
10554+
Convey("Test SHA512 single-arch image", t, func() {
10555+
image := CreateImageWithDigestAlgorithm(godigest.SHA512).
10556+
RandomLayers(1, 10).DefaultConfig().Build()
10557+
10558+
name := "algo-sha256"
10559+
tag := "singlearch"
10560+
10561+
err := UploadImage(image, baseURL, name, tag)
10562+
So(err, ShouldBeNil)
10563+
10564+
client := resty.New()
10565+
10566+
// The server picks canonical digests when tags are pushed
10567+
// See https://github.com/opencontainers/distribution-spec/issues/494
10568+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10569+
// but there is no way to specify a client preference
10570+
// so all we can do is verify the correct algorithm is returned
10571+
10572+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
10573+
10574+
headResponse, err := client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, tag))
10575+
So(err, ShouldBeNil)
10576+
So(headResponse, ShouldNotBeNil)
10577+
So(headResponse.StatusCode(), ShouldEqual, http.StatusOK)
10578+
10579+
canonicalDigestStr := headResponse.Header().Get("Docker-Content-Digest")
10580+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10581+
10582+
getResponse, err := client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, tag))
10583+
So(err, ShouldBeNil)
10584+
So(getResponse, ShouldNotBeNil)
10585+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10586+
10587+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10588+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10589+
10590+
getResponse, err = client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, canonicalDigestStr))
10591+
So(err, ShouldBeNil)
10592+
So(getResponse, ShouldNotBeNil)
10593+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10594+
10595+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10596+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10597+
10598+
getResponse, err = client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, canonicalDigestStr))
10599+
So(err, ShouldBeNil)
10600+
So(getResponse, ShouldNotBeNil)
10601+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10602+
10603+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10604+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10605+
})
10606+
10607+
Convey("Test SHA384 single-arch image", t, func() {
10608+
image := CreateImageWithDigestAlgorithm(godigest.SHA384).
10609+
RandomLayers(1, 10).DefaultConfig().Build()
10610+
10611+
name := "algo-sha384"
10612+
tag := "singlearch"
10613+
10614+
err := UploadImage(image, 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+
10625+
expectedDigestStr := image.DigestForAlgorithm(godigest.Canonical).String()
10626+
10627+
headResponse, err := client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, tag))
10628+
So(err, ShouldBeNil)
10629+
So(headResponse, ShouldNotBeNil)
10630+
So(headResponse.StatusCode(), ShouldEqual, http.StatusOK)
10631+
10632+
canonicalDigestStr := headResponse.Header().Get("Docker-Content-Digest")
10633+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10634+
10635+
getResponse, err := client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, tag))
10636+
So(err, ShouldBeNil)
10637+
So(getResponse, ShouldNotBeNil)
10638+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10639+
10640+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10641+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10642+
10643+
getResponse, err = client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, canonicalDigestStr))
10644+
So(err, ShouldBeNil)
10645+
So(getResponse, ShouldNotBeNil)
10646+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10647+
10648+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10649+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10650+
10651+
getResponse, err = client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, canonicalDigestStr))
10652+
So(err, ShouldBeNil)
10653+
So(getResponse, ShouldNotBeNil)
10654+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10655+
10656+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10657+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10658+
})
10659+
10660+
Convey("Test SHA512 multi-arch image", t, func() {
10661+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
10662+
DefaultConfig().Build()
10663+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA512).RandomLayers(1, 10).
10664+
DefaultConfig().Build()
10665+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA512).
10666+
Images([]Image{subImage1, subImage2}).Build()
10667+
10668+
name := "algo-sha256"
10669+
tag := "multiarch"
10670+
10671+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
10672+
So(err, ShouldBeNil)
10673+
10674+
client := resty.New()
10675+
10676+
// The server picks canonical digests when tags are pushed
10677+
// See https://github.com/opencontainers/distribution-spec/issues/494
10678+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10679+
// but there is no way to specify a client preference
10680+
// so all we can do is verify the correct algorithm is returned
10681+
10682+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
10683+
10684+
headResponse, err := client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, tag))
10685+
So(err, ShouldBeNil)
10686+
So(headResponse, ShouldNotBeNil)
10687+
So(headResponse.StatusCode(), ShouldEqual, http.StatusOK)
10688+
10689+
canonicalDigestStr := headResponse.Header().Get("Docker-Content-Digest")
10690+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10691+
10692+
getResponse, err := client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, tag))
10693+
So(err, ShouldBeNil)
10694+
So(getResponse, ShouldNotBeNil)
10695+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10696+
10697+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10698+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10699+
10700+
getResponse, err = client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, canonicalDigestStr))
10701+
So(err, ShouldBeNil)
10702+
So(getResponse, ShouldNotBeNil)
10703+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10704+
10705+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10706+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10707+
10708+
getResponse, err = client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, canonicalDigestStr))
10709+
So(err, ShouldBeNil)
10710+
So(getResponse, ShouldNotBeNil)
10711+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10712+
10713+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10714+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10715+
})
10716+
10717+
Convey("Test SHA384 multi-arch image", t, func() {
10718+
subImage1 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
10719+
DefaultConfig().Build()
10720+
subImage2 := CreateImageWithDigestAlgorithm(godigest.SHA384).RandomLayers(1, 10).
10721+
DefaultConfig().Build()
10722+
multiarch := CreateMultiarchWithDigestAlgorithm(godigest.SHA384).
10723+
Images([]Image{subImage1, subImage2}).Build()
10724+
10725+
name := "algo-sha384"
10726+
tag := "multiarch"
10727+
10728+
err := UploadMultiarchImage(multiarch, baseURL, name, tag)
10729+
So(err, ShouldBeNil)
10730+
10731+
client := resty.New()
10732+
10733+
// The server picks canonical digests when tags are pushed
10734+
// See https://github.com/opencontainers/distribution-spec/issues/494
10735+
// It would be nice to be able to push tags with other digest algorithms and verify those are returned
10736+
// but there is no way to specify a client preference
10737+
// so all we can do is verify the correct algorithm is returned
10738+
10739+
expectedDigestStr := multiarch.DigestForAlgorithm(godigest.Canonical).String()
10740+
10741+
headResponse, err := client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, tag))
10742+
So(err, ShouldBeNil)
10743+
So(headResponse, ShouldNotBeNil)
10744+
So(headResponse.StatusCode(), ShouldEqual, http.StatusOK)
10745+
10746+
canonicalDigestStr := headResponse.Header().Get("Docker-Content-Digest")
10747+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10748+
10749+
getResponse, err := client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, tag))
10750+
So(err, ShouldBeNil)
10751+
So(getResponse, ShouldNotBeNil)
10752+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10753+
10754+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10755+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10756+
10757+
getResponse, err = client.R().Get(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, canonicalDigestStr))
10758+
So(err, ShouldBeNil)
10759+
So(getResponse, ShouldNotBeNil)
10760+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10761+
10762+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10763+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10764+
10765+
getResponse, err = client.R().Head(fmt.Sprintf("%s/v2/%s/manifests/%s", baseURL, name, canonicalDigestStr))
10766+
So(err, ShouldBeNil)
10767+
So(getResponse, ShouldNotBeNil)
10768+
So(getResponse.StatusCode(), ShouldEqual, http.StatusOK)
10769+
10770+
canonicalDigestStr = getResponse.Header().Get("Docker-Content-Digest")
10771+
So(canonicalDigestStr, ShouldEqual, expectedDigestStr)
10772+
})
10773+
}
10774+
1053610775
func getEmptyImageConfig() ([]byte, godigest.Digest) {
1053710776
config := ispec.Image{}
1053810777

‎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("OCIv1 image manifest schema validation failed")
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("unable to unmarshal JSON")
9595

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

9999
// validate blobs only for known media types
@@ -104,7 +104,7 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy
104104
if !ok || err != nil {
105105
log.Error().Err(err).Str("digest", manifest.Config.Digest.String()).Msg("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
@@ -120,7 +120,7 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy
120120
if !ok || err != nil {
121121
log.Error().Err(err).Str("digest", layer.Digest.String()).Msg("missing layer blob")
122122

123-
return "", zerr.ErrBadManifest
123+
return zerr.ErrBadManifest
124124
}
125125
}
126126
}
@@ -129,49 +129,58 @@ func ValidateManifest(imgStore storageTypes.ImageStore, repo, reference, mediaTy
129129
if err := json.Unmarshal(body, &m); err != nil {
130130
log.Error().Err(err).Msg("unable to unmarshal JSON")
131131

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

139-
return "", zerr.NewError(zerr.ErrBadManifest).AddDetail("jsonSchemaValidation", err.Error())
139+
return zerr.NewError(zerr.ErrBadManifest).AddDetail("jsonSchemaValidation", err.Error())
140140
}
141141

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

146-
return "", zerr.ErrBadManifest
146+
return zerr.ErrBadManifest
147147
}
148148

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

153-
return "", zerr.ErrBadManifest
153+
return zerr.ErrBadManifest
154154
}
155155
}
156156
}
157157

158-
return "", nil
158+
return nil
159159
}
160160

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

164-
d, err := godigest.Parse(digestStr)
165-
if err == nil {
166-
if d.String() != bodyDigest.String() {
167-
log.Error().Str("actual", bodyDigest.String()).Str("expected", d.String()).
168-
Msg("manifest digest is not valid")
176+
if expectedDigest.String() != actualDigest.String() {
177+
log.Error().Str("actual", actualDigest.String()).Str("expected", expectedDigest.String()).
178+
Msg("manifest digest is not valid")
169179

170-
return "", zerr.ErrBadManifest
171-
}
180+
return actualDigest, zerr.ErrBadManifest
172181
}
173182

174-
return bodyDigest, err
183+
return actualDigest, nil
175184
}
176185

177186
/*

0 commit comments

Comments
 (0)