Merge pull request #633 from RichardScothern/manifest-verification

External manifest verification

notifications/listener.go

@@ -51,11 +51,15 @@ func Listen(repo distribution.Repository, listener Listener) distribution.Reposi
 	}
 }
 
-func (rl *repositoryListener) Manifests() distribution.ManifestService {
+func (rl *repositoryListener) Manifests(ctx context.Context, options ...distribution.ManifestServiceOption) (distribution.ManifestService, error) {
-	return &manifestServiceListener{
+	manifests, err := rl.Repository.Manifests(ctx, options...)
-		ManifestService: rl.Repository.Manifests(),
+	if err != nil {
-		parent:          rl,
+		return nil, err
 	}
+	return &manifestServiceListener{
+		ManifestService: manifests,
+		parent:          rl,
+	}, nil
 }
 
 func (rl *repositoryListener) Blobs(ctx context.Context) distribution.BlobStore {

notifications/listener_test.go

@@ -146,9 +146,12 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository) {
 		t.Fatalf("unexpected error signing manifest: %v", err)
 	}
 
-	manifests := repository.Manifests()
+	manifests, err := repository.Manifests(ctx)
+	if err != nil {
+		t.Fatal(err.Error())
+	}
 
-	if err := manifests.Put(sm); err != nil {
+	if err = manifests.Put(sm); err != nil {
 		t.Fatalf("unexpected error putting the manifest: %v", err)
 	}
 

registry.go

@@ -46,7 +46,8 @@ type Repository interface {
 	Name() string
 
 	// Manifests returns a reference to this repository's manifest service.
-	Manifests() ManifestService
+	// with the supplied options applied.
+	Manifests(ctx context.Context, options ...ManifestServiceOption) (ManifestService, error)
 
 	// Blobs returns a reference to this repository's blob service.
 	Blobs(ctx context.Context) BlobStore

registry/client/repository.go

@@ -70,18 +70,20 @@ func (r *repository) Blobs(ctx context.Context) distribution.BlobStore {
 	}
 }
 
-func (r *repository) Manifests() distribution.ManifestService {
+func (r *repository) Manifests(ctx context.Context, options ...distribution.ManifestServiceOption) (distribution.ManifestService, error) {
+	// todo(richardscothern): options should be sent over the wire
 	return &manifests{
 		name:   r.Name(),
 		ub:     r.ub,
 		client: r.client,
 		etags:  make(map[string]string),
-	}
+	}, nil
 }
 
 func (r *repository) Signatures() distribution.SignatureService {
+	ms, _ := r.Manifests(r.context)
 	return &signatures{
-		manifests: r.Manifests(),
+		manifests: ms,
 	}
 }
 
@@ -236,6 +238,8 @@ func (ms *manifests) Put(m *manifest.SignedManifest) error {
 		return err
 	}
 
+	// todo(richardscothern): do something with options here when they become applicable
+
 	putRequest, err := http.NewRequest("PUT", manifestURL, bytes.NewReader(m.Raw))
 	if err != nil {
 		return err

registry/client/repository_test.go

@@ -492,6 +492,7 @@ func checkEqualManifest(m1, m2 *manifest.SignedManifest) error {
 }
 
 func TestManifestFetch(t *testing.T) {
+	ctx := context.Background()
 	repo := "test.example.com/repo"
 	m1, dgst := newRandomSchemaV1Manifest(repo, "latest", 6)
 	var m testutil.RequestResponseMap
@@ -504,7 +505,10 @@ func TestManifestFetch(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	ms := r.Manifests()
+	ms, err := r.Manifests(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	ok, err := ms.Exists(dgst)
 	if err != nil {
@@ -536,8 +540,12 @@ func TestManifestFetchWithEtag(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	ctx := context.Background()
+	ms, err := r.Manifests(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	ms := r.Manifests()
 	m2, err := ms.GetByTag("latest", AddEtagToTag("latest", d1.String()))
 	if err != nil {
 		t.Fatal(err)
@@ -572,8 +580,12 @@ func TestManifestDelete(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	ctx := context.Background()
+	ms, err := r.Manifests(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	ms := r.Manifests()
 	if err := ms.Delete(dgst1); err != nil {
 		t.Fatal(err)
 	}
@@ -609,8 +621,12 @@ func TestManifestPut(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	ctx := context.Background()
+	ms, err := r.Manifests(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	ms := r.Manifests()
 	if err := ms.Put(m1); err != nil {
 		t.Fatal(err)
 	}
@@ -653,8 +669,12 @@ func TestManifestTags(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	ctx := context.Background()
+	ms, err := r.Manifests(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	ms := r.Manifests()
 	tags, err := ms.Tags()
 	if err != nil {
 		t.Fatal(err)
@@ -691,7 +711,11 @@ func TestManifestUnauthorized(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	ms := r.Manifests()
+	ctx := context.Background()
+	ms, err := r.Manifests(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	_, err = ms.Get(dgst)
 	if err == nil {

registry/handlers/images.go

@@ -50,13 +50,13 @@ type imageManifestHandler struct {
 // GetImageManifest fetches the image manifest from the storage backend, if it exists.
 func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http.Request) {
 	ctxu.GetLogger(imh).Debug("GetImageManifest")
-	manifests := imh.Repository.Manifests()
+	manifests, err := imh.Repository.Manifests(imh)
-
+	if err != nil {
-	var (
+		imh.Errors = append(imh.Errors, err)
-		sm  *manifest.SignedManifest
+		return
-		err error
+	}
-	)
 
+	var sm *manifest.SignedManifest
 	if imh.Tag != "" {
 		sm, err = manifests.GetByTag(imh.Tag)
 	} else {
@@ -106,7 +106,12 @@ func etagMatch(r *http.Request, etag string) bool {
 // PutImageManifest validates and stores and image in the registry.
 func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http.Request) {
 	ctxu.GetLogger(imh).Debug("PutImageManifest")
-	manifests := imh.Repository.Manifests()
+	manifests, err := imh.Repository.Manifests(imh)
+	if err != nil {
+		imh.Errors = append(imh.Errors, err)
+		return
+	}
+
 	dec := json.NewDecoder(r.Body)
 
 	var manifest manifest.SignedManifest

registry/handlers/tags.go

@@ -34,7 +34,11 @@ type tagsAPIResponse struct {
 // GetTags returns a json list of tags for a specific image name.
 func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) {
 	defer r.Body.Close()
-	manifests := th.Repository.Manifests()
+	manifests, err := th.Repository.Manifests(th)
+	if err != nil {
+		th.Errors = append(th.Errors, err)
+		return
+	}
 
 	tags, err := manifests.Tags()
 	if err != nil {

registry/storage/manifeststore.go

@@ -11,10 +11,11 @@ import (
 )
 
 type manifestStore struct {
-	repository    *repository
+	repository                 *repository
-	revisionStore *revisionStore
+	revisionStore              *revisionStore
-	tagStore      *tagStore
+	tagStore                   *tagStore
-	ctx           context.Context
+	ctx                        context.Context
+	skipDependencyVerification bool
 }
 
 var _ distribution.ManifestService = &manifestStore{}
@@ -39,10 +40,19 @@ func (ms *manifestStore) Get(dgst digest.Digest) (*manifest.SignedManifest, erro
 	return ms.revisionStore.get(ms.ctx, dgst)
 }
 
+// SkipLayerVerification allows a manifest to be Put before it's
+// layers are on the filesystem
+func SkipLayerVerification(ms distribution.ManifestService) error {
+	if ms, ok := ms.(*manifestStore); ok {
+		ms.skipDependencyVerification = true
+		return nil
+	}
+	return fmt.Errorf("skip layer verification only valid for manifeststore")
+}
+
 func (ms *manifestStore) Put(manifest *manifest.SignedManifest) error {
 	context.GetLogger(ms.ctx).Debug("(*manifestStore).Put")
 
-	// Verify the manifest.
 	if err := ms.verifyManifest(ms.ctx, manifest); err != nil {
 		return err
 	}
@@ -113,18 +123,19 @@ func (ms *manifestStore) verifyManifest(ctx context.Context, mnfst *manifest.Sig
 		}
 	}
 
-	for _, fsLayer := range mnfst.FSLayers {
+	if !ms.skipDependencyVerification {
-		_, err := ms.repository.Blobs(ctx).Stat(ctx, fsLayer.BlobSum)
+		for _, fsLayer := range mnfst.FSLayers {
-		if err != nil {
+			_, err := ms.repository.Blobs(ctx).Stat(ctx, fsLayer.BlobSum)
-			if err != distribution.ErrBlobUnknown {
+			if err != nil {
-				errs = append(errs, err)
+				if err != distribution.ErrBlobUnknown {
-			}
+					errs = append(errs, err)
+				}
 
-			// On error here, we always append unknown blob errors.
+				// On error here, we always append unknown blob errors.
-			errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.BlobSum})
+				errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.BlobSum})
+			}
 		}
 	}
-
 	if len(errs) != 0 {
 		return errs
 	}

registry/storage/manifeststore_test.go

@@ -48,7 +48,11 @@ func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestE
 
 func TestManifestStorage(t *testing.T) {
 	env := newManifestStoreTestEnv(t, "foo/bar", "thetag")
-	ms := env.repository.Manifests()
+	ctx := context.Background()
+	ms, err := env.repository.Manifests(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	exists, err := ms.ExistsByTag(env.tag)
 	if err != nil {
@@ -97,14 +101,14 @@ func TestManifestStorage(t *testing.T) {
 		t.Fatalf("unexpected error generating private key: %v", err)
 	}
 
-	sm, err := manifest.Sign(&m, pk)
+	sm, merr := manifest.Sign(&m, pk)
-	if err != nil {
+	if merr != nil {
 		t.Fatalf("error signing manifest: %v", err)
 	}
 
 	err = ms.Put(sm)
 	if err == nil {
-		t.Fatalf("expected errors putting manifest")
+		t.Fatalf("expected errors putting manifest with full verification")
 	}
 
 	switch err := err.(type) {

registry/storage/registry.go

@@ -99,15 +99,15 @@ func (repo *repository) Name() string {
 // Manifests returns an instance of ManifestService. Instantiation is cheap and
 // may be context sensitive in the future. The instance should be used similar
 // to a request local.
-func (repo *repository) Manifests() distribution.ManifestService {
+func (repo *repository) Manifests(ctx context.Context, options ...distribution.ManifestServiceOption) (distribution.ManifestService, error) {
-	return &manifestStore{
+	ms := &manifestStore{
-		ctx:        repo.ctx,
+		ctx:        ctx,
 		repository: repo,
 		revisionStore: &revisionStore{
-			ctx:        repo.ctx,
+			ctx:        ctx,
 			repository: repo,
 			blobStore: &linkedBlobStore{
-				ctx:        repo.ctx,
+				ctx:        ctx,
 				blobStore:  repo.blobStore,
 				repository: repo,
 				statter: &linkedBlobStatter{
@@ -122,11 +122,21 @@ func (repo *repository) Manifests() distribution.ManifestService {
 			},
 		},
 		tagStore: &tagStore{
-			ctx:        repo.ctx,
+			ctx:        ctx,
 			repository: repo,
 			blobStore:  repo.registry.blobStore,
 		},
 	}
+
+	// Apply options
+	for _, option := range options {
+		err := option(ms)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return ms, nil
 }
 
 // Blobs returns an instance of the BlobStore. Instantiation is cheap and