Skip to content

Commit 57f9cf6

Browse files
author
Michal Minář
committed
Added manifest verification
Mostly copied from docker/distribution repo. Signed-off-by: Michal Minář <[email protected]>
1 parent 84e11a7 commit 57f9cf6

9 files changed

+247
-14
lines changed

‎pkg/dockerregistry/server/blobdescriptorservice_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func TestBlobDescriptorServiceIsApplied(t *testing.T) {
9090
}
9191
os.Setenv("DOCKER_REGISTRY_URL", serverURL.Host)
9292

93-
desc, _, err := registrytest.UploadTestBlob(serverURL, "user/app")
93+
desc, _, err := registrytest.UploadTestBlob(serverURL, nil, "user/app")
9494
if err != nil {
9595
t.Fatal(err)
9696
}

‎pkg/dockerregistry/server/manifesthandler.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ type ManifestHandler interface {
2525
// Payload returns manifest's media type, complete payload with signatures and canonical payload without
2626
// signatures or an error if the information could not be fetched.
2727
Payload() (mediaType string, payload []byte, canonical []byte, err error)
28+
29+
// Verify returns an error if the contained manifest is not valid or has missing dependencies.
30+
Verify(ctx context.Context, skipDependencyVerification bool) error
2831
}
2932

3033
// NewManifestHandler creates a manifest handler for the given manifest.

‎pkg/dockerregistry/server/manifestschema1handler.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ package server
22

33
import (
44
"encoding/json"
5+
"fmt"
6+
"path"
57

68
"github.com/docker/distribution"
79
"github.com/docker/distribution/context"
810
"github.com/docker/distribution/manifest/schema1"
11+
"github.com/docker/distribution/reference"
912
"github.com/docker/libtrust"
1013

1114
"k8s.io/kubernetes/pkg/util/sets"
@@ -103,3 +106,69 @@ func (h *manifestSchema1Handler) Payload() (mediaType string, payload []byte, ca
103106
mt, payload, err := h.manifest.Payload()
104107
return mt, payload, h.manifest.Canonical, err
105108
}
109+
110+
func (h *manifestSchema1Handler) Verify(ctx context.Context, skipDependencyVerification bool) error {
111+
var errs distribution.ErrManifestVerification
112+
113+
// we want to verify that referenced blobs exist locally - thus using upstream repository object directly
114+
repo := h.repo.Repository
115+
116+
if len(path.Join(h.repo.registryAddr, h.manifest.Name)) > reference.NameTotalLengthMax {
117+
errs = append(errs,
118+
distribution.ErrManifestNameInvalid{
119+
Name: h.manifest.Name,
120+
Reason: fmt.Errorf("<registry-host>/<manifest-name> must not be more than %d characters", reference.NameTotalLengthMax),
121+
})
122+
}
123+
124+
if !reference.NameRegexp.MatchString(h.manifest.Name) {
125+
errs = append(errs,
126+
distribution.ErrManifestNameInvalid{
127+
Name: h.manifest.Name,
128+
Reason: fmt.Errorf("invalid manifest name format"),
129+
})
130+
}
131+
132+
if len(h.manifest.History) != len(h.manifest.FSLayers) {
133+
errs = append(errs, fmt.Errorf("mismatched history and fslayer cardinality %d != %d",
134+
len(h.manifest.History), len(h.manifest.FSLayers)))
135+
}
136+
137+
if _, err := schema1.Verify(h.manifest); err != nil {
138+
switch err {
139+
case libtrust.ErrMissingSignatureKey, libtrust.ErrInvalidJSONContent, libtrust.ErrMissingSignatureKey:
140+
errs = append(errs, distribution.ErrManifestUnverified{})
141+
default:
142+
if err.Error() == "invalid signature" {
143+
errs = append(errs, distribution.ErrManifestUnverified{})
144+
} else {
145+
errs = append(errs, err)
146+
}
147+
}
148+
}
149+
150+
if skipDependencyVerification {
151+
if len(errs) > 0 {
152+
return errs
153+
}
154+
return nil
155+
}
156+
157+
for _, fsLayer := range h.manifest.References() {
158+
_, err := repo.Blobs(ctx).Stat(ctx, fsLayer.Digest)
159+
if err != nil {
160+
if err != distribution.ErrBlobUnknown {
161+
errs = append(errs, err)
162+
continue
163+
}
164+
165+
// On error here, we always append unknown blob errors.
166+
errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest})
167+
}
168+
}
169+
170+
if len(errs) > 0 {
171+
return errs
172+
}
173+
return nil
174+
}

‎pkg/dockerregistry/server/manifestschema2handler.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package server
22

33
import (
44
"encoding/json"
5+
"errors"
56

67
"github.com/docker/distribution"
78
"github.com/docker/distribution/context"
@@ -10,6 +11,11 @@ import (
1011
imageapi "github.com/openshift/origin/pkg/image/api"
1112
)
1213

14+
var (
15+
errMissingURL = errors.New("missing URL on layer")
16+
errUnexpectedURL = errors.New("unexpected URL on layer")
17+
)
18+
1319
func unmarshalManifestSchema2(content []byte) (distribution.Manifest, error) {
1420
var deserializedManifest schema2.DeserializedManifest
1521
if err := json.Unmarshal(content, &deserializedManifest); err != nil {
@@ -51,3 +57,56 @@ func (h *manifestSchema2Handler) Payload() (mediaType string, payload []byte, ca
5157
mt, p, err := h.manifest.Payload()
5258
return mt, p, p, err
5359
}
60+
61+
func (h *manifestSchema2Handler) Verify(ctx context.Context, skipDependencyVerification bool) error {
62+
var errs distribution.ErrManifestVerification
63+
64+
if skipDependencyVerification {
65+
return nil
66+
}
67+
68+
// we want to verify that referenced blobs exist locally - thus using upstream repository object directly
69+
repo := h.repo.Repository
70+
71+
target := h.manifest.Target()
72+
_, err := repo.Blobs(ctx).Stat(ctx, target.Digest)
73+
if err != nil {
74+
if err != distribution.ErrBlobUnknown {
75+
errs = append(errs, err)
76+
}
77+
78+
// On error here, we always append unknown blob errors.
79+
errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: target.Digest})
80+
}
81+
82+
for _, fsLayer := range h.manifest.References() {
83+
var err error
84+
if fsLayer.MediaType != schema2.MediaTypeForeignLayer {
85+
if len(fsLayer.URLs) == 0 {
86+
_, err = repo.Blobs(ctx).Stat(ctx, fsLayer.Digest)
87+
} else {
88+
err = errUnexpectedURL
89+
}
90+
} else {
91+
// Clients download this layer from an external URL, so do not check for
92+
// its presense.
93+
if len(fsLayer.URLs) == 0 {
94+
err = errMissingURL
95+
}
96+
}
97+
if err != nil {
98+
if err != distribution.ErrBlobUnknown {
99+
errs = append(errs, err)
100+
continue
101+
}
102+
103+
// On error here, we always append unknown blob errors.
104+
errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest})
105+
}
106+
}
107+
108+
if len(errs) > 0 {
109+
return errs
110+
}
111+
return nil
112+
}

‎pkg/dockerregistry/server/pullthroughblobstore_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,11 @@ func TestPullthroughServeBlob(t *testing.T) {
8888

8989
client.AddReactor("get", "imagestreams", imagetest.GetFakeImageStreamGetHandler(t, *testImageStream))
9090

91-
blob1Desc, blob1Content, err := registrytest.UploadTestBlob(serverURL, "user/app")
91+
blob1Desc, blob1Content, err := registrytest.UploadTestBlob(serverURL, nil, "user/app")
9292
if err != nil {
9393
t.Fatal(err)
9494
}
95-
blob2Desc, blob2Content, err := registrytest.UploadTestBlob(serverURL, "user/app")
95+
blob2Desc, blob2Content, err := registrytest.UploadTestBlob(serverURL, nil, "user/app")
9696
if err != nil {
9797
t.Fatal(err)
9898
}

‎pkg/dockerregistry/server/repositorymiddleware.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,18 @@ func (r *repository) Put(ctx context.Context, manifest distribution.Manifest, op
326326
return "", regapi.ErrorCodeManifestInvalid.WithDetail(err)
327327
}
328328

329+
// this is fast to check, let's do it before verification
329330
if !r.acceptschema2 && mediaType == schema2.MediaTypeManifest {
330331
err = fmt.Errorf("manifest V2 schema 2 not allowed")
331332
return "", regapi.ErrorCodeManifestInvalid.WithDetail(err)
332333
}
333334

335+
// in order to stat the referenced blobs, repository need to be set on the context
336+
ctx = WithRepository(ctx, r)
337+
if err := mh.Verify(ctx, false); err != nil {
338+
return "", err
339+
}
340+
334341
// Calculate digest
335342
dgst := digest.FromBytes(canonical)
336343

‎pkg/dockerregistry/testutil/util.go

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"io"
1010
"io/ioutil"
1111
mrand "math/rand"
12+
"net/http"
1213
"net/url"
1314
"testing"
1415
"time"
@@ -19,6 +20,8 @@ import (
1920
"github.com/docker/distribution/manifest"
2021
"github.com/docker/distribution/reference"
2122
distclient "github.com/docker/distribution/registry/client"
23+
"github.com/docker/distribution/registry/client/auth"
24+
"github.com/docker/distribution/registry/client/transport"
2225

2326
kapi "k8s.io/kubernetes/pkg/api"
2427
kerrors "k8s.io/kubernetes/pkg/api/errors"
@@ -61,7 +64,7 @@ func NewImageForManifest(repoName string, rawManifest string, managedByOpenShift
6164
}
6265

6366
// UploadTestBlob generates a random tar file and uploads it to the given repository.
64-
func UploadTestBlob(serverURL *url.URL, repoName string) (distribution.Descriptor, []byte, error) {
67+
func UploadTestBlob(serverURL *url.URL, creds auth.CredentialStore, repoName string) (distribution.Descriptor, []byte, error) {
6568
rs, ds, err := CreateRandomTarFile()
6669
if err != nil {
6770
return distribution.Descriptor{}, nil, fmt.Errorf("unexpected error generating test layer file: %v", err)
@@ -73,7 +76,22 @@ func UploadTestBlob(serverURL *url.URL, repoName string) (distribution.Descripto
7376
if err != nil {
7477
return distribution.Descriptor{}, nil, err
7578
}
76-
repo, err := distclient.NewRepository(ctx, ref, serverURL.String(), nil)
79+
80+
var rt http.RoundTripper
81+
if creds != nil {
82+
challengeManager := auth.NewSimpleChallengeManager()
83+
_, err := ping(challengeManager, serverURL.String()+"/v2/", "")
84+
if err != nil {
85+
return distribution.Descriptor{}, nil, err
86+
}
87+
rt = transport.NewTransport(
88+
nil,
89+
auth.NewAuthorizer(
90+
challengeManager,
91+
auth.NewTokenHandler(nil, creds, repoName, "pull", "push"),
92+
auth.NewBasicHandler(creds)))
93+
}
94+
repo, err := distclient.NewRepository(ctx, ref, serverURL.String(), rt)
7795
if err != nil {
7896
return distribution.Descriptor{}, nil, fmt.Errorf("failed to get repository %q: %v", repoName, err)
7997
}
@@ -247,3 +265,50 @@ func TestNewImageStreamObject(namespace, name, tag, imageName, dockerImageRefere
247265
},
248266
}
249267
}
268+
269+
type testCredentialStore struct {
270+
username string
271+
password string
272+
refreshTokens map[string]string
273+
}
274+
275+
var _ auth.CredentialStore = &testCredentialStore{}
276+
277+
// NewBasicCredentialStore returns a test credential store for use with registry token handler and/or basic
278+
// handler.
279+
func NewBasicCredentialStore(username, password string) auth.CredentialStore {
280+
return &testCredentialStore{
281+
username: username,
282+
password: password,
283+
}
284+
}
285+
286+
func (tcs *testCredentialStore) Basic(*url.URL) (string, string) {
287+
return tcs.username, tcs.password
288+
}
289+
290+
func (tcs *testCredentialStore) RefreshToken(u *url.URL, service string) string {
291+
return tcs.refreshTokens[service]
292+
}
293+
294+
func (tcs *testCredentialStore) SetRefreshToken(u *url.URL, service string, token string) {
295+
if tcs.refreshTokens != nil {
296+
tcs.refreshTokens[service] = token
297+
}
298+
}
299+
300+
// ping pings the provided endpoint to determine its required authorization challenges.
301+
// If a version header is provided, the versions will be returned.
302+
func ping(manager auth.ChallengeManager, endpoint, versionHeader string) ([]auth.APIVersion, error) {
303+
resp, err := http.Get(endpoint)
304+
if err != nil {
305+
return nil, err
306+
}
307+
defer resp.Body.Close()
308+
309+
if err := manager.AddResponse(resp); err != nil {
310+
return nil, err
311+
}
312+
313+
return auth.APIVersions(resp, versionHeader), err
314+
}

‎test/end-to-end/core.sh

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,10 @@ os::log::info "Pushed ruby-22-centos7"
139139

140140
# get image's digest
141141
rubyimagedigest="$(oc get -o jsonpath='{.status.tags[?(@.tag=="latest")].items[0].image}' is/ruby-22-centos7)"
142-
os::log::info "Ruby image digest: $rubyimagedigest"
142+
os::log::info "Ruby image digest: ${rubyimagedigest}"
143143
# get a random, non-empty blob
144144
rubyimageblob="$(oc get isimage -o go-template='{{range .image.dockerImageLayers}}{{if gt .size 1024.}}{{.name}},{{end}}{{end}}' "ruby-22-centos7@${rubyimagedigest}" | cut -d , -f 1)"
145-
os::log::info "Ruby's testing blob digest: $rubyimageblob"
145+
os::log::info "Ruby's testing blob digest: ${rubyimageblob}"
146146

147147
# verify remote images can be pulled directly from the local registry
148148
os::log::info "Docker pullthrough"
@@ -154,6 +154,24 @@ os::cmd::expect_success "curl -H 'Authorization: bearer $(oc whoami -t)' 'http:/
154154
# verify the blob exists on disk in the registry due to mirroring under .../blobs/sha256/<2 char prefix>/<sha value>
155155
os::cmd::try_until_success "oc exec --context='${CLUSTER_ADMIN_CONTEXT}' -n default -p '${registry_pod}' du /registry | tee '${LOG_DIR}/registry-images.txt' | grep '${mysqlblob:7:100}' | grep blobs"
156156

157+
os::log::info "Docker registry refuses manifest with missing dependencies"
158+
os::cmd::expect_success 'oc new-project verify-manifest'
159+
os::cmd::expect_success "oc get -o jsonpath=$'{.dockerImageManifest}\n' 'image/${rubyimagedigest}' --context="${CLUSTER_ADMIN_CONTEXT}" >'${ARTIFACT_DIR}/rubyimagemanifest.json'"
160+
os::cmd::expect_success "curl --head \
161+
--silent \
162+
--request PUT \
163+
--header 'Content-Type: application/vnd.docker.distribution.manifest.v1+json' \
164+
--user 'e2e_user:${e2e_user_token}' \
165+
--upload-file '${ARTIFACT_DIR}/rubyimagemanifest.json' \
166+
'http://${DOCKER_REGISTRY}/v2/verify-manifest/ruby-22-centos7/manifests/latest' \
167+
>'${ARTIFACT_DIR}/curl-ruby-manifest-put.txt'"
168+
os::cmd::expect_success_and_text "cat '${ARTIFACT_DIR}/curl-ruby-manifest-put.txt'" '400 Bad Request'
169+
os::cmd::expect_success_and_text "cat '${ARTIFACT_DIR}/curl-ruby-manifest-put.txt'" '"errors":.*MANIFEST_BLOB_UNKNOWN'
170+
os::cmd::expect_success_and_text "cat '${ARTIFACT_DIR}/curl-ruby-manifest-put.txt'" '"errors":.*blob unknown to registry'
171+
os::log::info "Docker registry successfuly refused manifest with missing dependencies"
172+
173+
os::cmd::expect_success 'oc project cache'
174+
157175
os::log::info "Docker registry start with GCS"
158176
os::cmd::expect_failure_and_text "docker run -e REGISTRY_STORAGE=\"gcs: {}\" openshift/origin-docker-registry:${TAG}" "No bucket parameter provided"
159177

0 commit comments

Comments
 (0)