Validate manifest parent-child relationships

Since 1.8 may push bad manifests, we've added some validation to ensure that
the parent-child relationships represented by image json are correct. If the
relationship is not correct, we reject the push.

Signed-off-by: Stephen J Day <stephen.day@docker.com>

errors.go

@@ -74,6 +74,16 @@ func (ErrManifestUnverified) Error() string {
 	return fmt.Sprintf("unverified manifest")
 }
 
+// ErrManifestValidation is returned during manifest verification if a common
+// validation error is encountered.
+type ErrManifestValidation struct {
+	Reason string
+}
+
+func (err ErrManifestValidation) Error() string {
+	return fmt.Sprintf("invalid manifest: %s", err.Reason)
+}
+
 // ErrManifestVerification provides a type to collect errors encountered
 // during manifest verification. Currently, it accepts errors of all types,
 // but it may be narrowed to those involving manifest verification.

notifications/listener_test.go

@@ -1,10 +1,12 @@
 package notifications
 
 import (
+	"encoding/json"
 	"io"
 	"reflect"
 	"testing"
 
+	"code.google.com/p/go-uuid/uuid"
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
@@ -132,6 +134,8 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository) {
 		}
 	}
 
+	m.History = generateHistory(t, len(m.FSLayers))
+
 	pk, err := libtrust.GenerateECP256PrivateKey()
 	if err != nil {
 		t.Fatalf("unexpected error generating key: %v", err)
@@ -176,3 +180,37 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository) {
 		t.Fatalf("retrieved unexpected manifest: %v", err)
 	}
 }
+
+// generateHistory creates a valid history entry of length n.
+func generateHistory(t *testing.T, n int) []manifest.History {
+	var images []map[string]interface{}
+
+	// first pass: create images entries.
+	for i := 0; i < n; i++ {
+		// simulate correct id -> parent links in v1Compatibility, using uuids.
+		image := map[string]interface{}{
+			"id": uuid.New(),
+		}
+
+		images = append(images, image)
+	}
+
+	var history []manifest.History
+
+	for i, image := range images {
+		if i+1 < len(images) {
+			image["parent"] = images[i+1]["id"]
+		}
+
+		p, err := json.Marshal(image)
+		if err != nil {
+			t.Fatalf("error generating image json: %v", err)
+		}
+
+		history = append(history, manifest.History{
+			V1Compatibility: string(p),
+		})
+	}
+
+	return history
+}

registry/handlers/api_test.go

@@ -16,6 +16,7 @@ import (
 	"strings"
 	"testing"
 
+	"code.google.com/p/go-uuid/uuid"
 	"github.com/docker/distribution/configuration"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
@@ -323,10 +324,12 @@ func TestManifestAPI(t *testing.T) {
 		},
 	}
 
+	unsignedManifest.History = generateHistory(t, len(unsignedManifest.FSLayers), false)
+
 	resp = putManifest(t, "putting unsigned manifest", manifestURL, unsignedManifest)
 	defer resp.Body.Close()
 	checkResponse(t, "posting unsigned manifest", resp, http.StatusBadRequest)
-	_, p, counts := checkBodyHasErrorCodes(t, "getting unknown manifest tags", resp,
+	_, p, counts := checkBodyHasErrorCodes(t, "putting unsigned manifest", resp,
 		v2.ErrorCodeManifestUnverified, v2.ErrorCodeBlobUnknown, v2.ErrorCodeDigestInvalid)
 
 	expectedCounts := map[v2.ErrorCode]int{
@@ -361,8 +364,10 @@ func TestManifestAPI(t *testing.T) {
 		pushLayer(t, env.builder, imageName, dgst, uploadURLBase, rs)
 	}
 
-	// -------------------
-	// Push the signed manifest with all layers pushed.
+	// -----------------------
+	// mostly valid, but we have an extra parent point in history.
+	unsignedManifest.History = generateHistory(t, len(unsignedManifest.FSLayers), true)
+
 	signedManifest, err := manifest.Sign(unsignedManifest, env.pk)
 	if err != nil {
 		t.Fatalf("unexpected error signing manifest: %v", err)
@@ -377,6 +382,36 @@ func TestManifestAPI(t *testing.T) {
 	manifestDigestURL, err := env.builder.BuildManifestURL(imageName, dgst.String())
 	checkErr(t, err, "building manifest url")
 
+	resp = putManifest(t, "putting signed manifest with bad parent", manifestURL, signedManifest)
+	checkResponse(t, "putting signed manifest with bad parent", resp, http.StatusBadRequest)
+	_, p, counts = checkBodyHasErrorCodes(t, "putting unsigned manifest with bad parent", resp, v2.ErrorCodeManifestInvalid)
+
+	expectedCounts = map[v2.ErrorCode]int{
+		v2.ErrorCodeManifestInvalid: 1,
+	}
+
+	if !reflect.DeepEqual(counts, expectedCounts) {
+		t.Fatalf("unexpected number of error codes encountered: %v\n!=\n%v\n---\n%s", counts, expectedCounts, string(p))
+	}
+
+	// -------------------
+	// Push the signed manifest with all layers pushed.
+	unsignedManifest.History = generateHistory(t, len(unsignedManifest.FSLayers), false)
+
+	signedManifest, err = manifest.Sign(unsignedManifest, env.pk)
+	if err != nil {
+		t.Fatalf("unexpected error signing manifest: %v", err)
+	}
+
+	payload, err = signedManifest.Payload()
+	checkErr(t, err, "getting manifest payload")
+
+	dgst, err = digest.FromBytes(payload)
+	checkErr(t, err, "digesting manifest")
+
+	manifestDigestURL, err = env.builder.BuildManifestURL(imageName, dgst.String())
+	checkErr(t, err, "building manifest url")
+
 	resp = putManifest(t, "putting signed manifest", manifestURL, signedManifest)
 	checkResponse(t, "putting signed manifest", resp, http.StatusAccepted)
 	checkHeaders(t, resp, http.Header{
@@ -791,3 +826,41 @@ func checkErr(t *testing.T, err error, msg string) {
 		t.Fatalf("unexpected error %s: %v", msg, err)
 	}
 }
+
+// generateHistory creates a valid history entry of length n.
+func generateHistory(t *testing.T, n int, extraParent bool) []manifest.History {
+	var images []map[string]interface{}
+
+	// first pass: create images entries.
+	for i := 0; i < n; i++ {
+		// simulate correct id -> parent links in v1Compatibility, using uuids.
+		image := map[string]interface{}{
+			"id": uuid.New(),
+		}
+
+		images = append(images, image)
+	}
+
+	var history []manifest.History
+
+	for i, image := range images {
+		if i+1 < len(images) {
+			image["parent"] = images[i+1]["id"]
+		}
+
+		if extraParent && i == len(images)-1 {
+			image["parent"] = uuid.New()
+		}
+
+		p, err := json.Marshal(image)
+		if err != nil {
+			t.Fatalf("error generating image json: %v", err)
+		}
+
+		history = append(history, manifest.History{
+			V1Compatibility: string(p),
+		})
+	}
+
+	return history
+}

registry/handlers/images.go

@@ -140,6 +140,8 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http
 					imh.Errors.Push(v2.ErrorCodeBlobUnknown, verificationError.FSLayer)
 				case distribution.ErrManifestUnverified:
 					imh.Errors.Push(v2.ErrorCodeManifestUnverified)
+				case distribution.ErrManifestValidation:
+					imh.Errors.Push(v2.ErrorCodeManifestInvalid, verificationError.Error())
 				default:
 					if verificationError == digest.ErrDigestInvalidFormat {
 						// TODO(stevvooe): We need to really need to move all

registry/storage/manifeststore.go

@@ -1,6 +1,7 @@
 package storage
 
 import (
+	"encoding/json"
 	"fmt"
 
 	"github.com/docker/distribution"
@@ -102,6 +103,48 @@ func (ms *manifestStore) verifyManifest(mnfst *manifest.SignedManifest) error {
 		}
 	}
 
+	if len(mnfst.FSLayers) == 0 || len(mnfst.History) == 0 {
+		errs = append(errs, distribution.ErrManifestValidation{
+			Reason: "no layers present"})
+	}
+
+	if len(mnfst.FSLayers) != len(mnfst.History) {
+		errs = append(errs, distribution.ErrManifestValidation{
+			Reason: "mismatched layers and history"})
+	}
+
+	// image provides a local type for validating the image relationship.
+	type image struct {
+		ID     string `json:"id"`
+		Parent string `json:"parent"`
+	}
+
+	// Process the history portion to ensure that the parent links are
+	// correctly represented. We serialize the image json, then walk the
+	// entries, checking the parent link.
+	var images []image
+	for _, entry := range mnfst.History {
+		var im image
+		if err := json.Unmarshal([]byte(entry.V1Compatibility), &im); err != nil {
+			errs = append(errs, err)
+		}
+
+		images = append(images, im)
+	}
+
+	// go back through each image, checking the parent link and rank
+	var parentID string
+	for i := len(images) - 1; i >= 0; i-- {
+		// ensure that the parent id matches but only if there is a parent.
+		// There are cases where successive layers don't fill in the parents.
+		if images[i].Parent != parentID {
+			errs = append(errs, distribution.ErrManifestValidation{
+				Reason: "parent not adjacent in manifest"})
+		}
+
+		parentID = images[i].ID
+	}
+
 	for _, fsLayer := range mnfst.FSLayers {
 		exists, err := ms.repository.Layers().Exists(fsLayer.BlobSum)
 		if err != nil {

registry/storage/manifeststore_test.go

@@ -2,15 +2,16 @@ package storage
 
 import (
 	"bytes"
+	"encoding/json"
 	"io"
 	"reflect"
 	"testing"
 
-	"github.com/docker/distribution/registry/storage/cache"
-
+	"code.google.com/p/go-uuid/uuid"
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
+	"github.com/docker/distribution/registry/storage/cache"
 	"github.com/docker/distribution/registry/storage/driver"
 	"github.com/docker/distribution/registry/storage/driver/inmemory"
 	"github.com/docker/distribution/testutil"
@@ -103,6 +104,38 @@ func TestManifestStorage(t *testing.T) {
 		t.Fatalf("error signing manifest: %v", err)
 	}
 
+	// try to put the manifest initially. this will fail since we have not
+	// included history or pushed any layers.
+	err = ms.Put(sm)
+	if err == nil {
+		t.Fatalf("expected errors putting manifest with full verification")
+	}
+
+	switch err := err.(type) {
+	case distribution.ErrManifestVerification:
+		if len(err) != 4 {
+			t.Fatalf("expected 4 verification errors: %#v", err)
+		}
+
+		for _, err := range err {
+			switch err := err.(type) {
+			case distribution.ErrUnknownLayer, distribution.ErrManifestValidation:
+				// noop: we expect these errors
+			default:
+				t.Fatalf("unexpected error type: %v", err)
+			}
+		}
+	default:
+		t.Fatalf("unexpected error verifying manifest: %v", err)
+	}
+
+	m.History = generateHistory(t, len(m.FSLayers))
+	sm, err = manifest.Sign(&m, pk)
+	if err != nil {
+		t.Fatalf("unexpected error signing manfiest with history: %v", err)
+	}
+
+	// we've fixed the missing history, try the push and fail on layer checks.
 	err = ms.Put(sm)
 	if err == nil {
 		t.Fatalf("expected errors putting manifest")
@@ -288,3 +321,37 @@ func TestManifestStorage(t *testing.T) {
 		t.Fatalf("unexpected an error deleting manifest by digest: %v", err)
 	}
 }
+
+// generateHistory creates a valid history entry of length n.
+func generateHistory(t *testing.T, n int) []manifest.History {
+	var images []map[string]interface{}
+
+	// first pass: create images entries.
+	for i := 0; i < n; i++ {
+		// simulate correct id -> parent links in v1Compatibility, using uuids.
+		image := map[string]interface{}{
+			"id": uuid.New(),
+		}
+
+		images = append(images, image)
+	}
+
+	var history []manifest.History
+
+	for i, image := range images {
+		if i+1 < len(images) {
+			image["parent"] = images[i+1]["id"]
+		}
+
+		p, err := json.Marshal(image)
+		if err != nil {
+			t.Fatalf("error generating image json: %v", err)
+		}
+
+		history = append(history, manifest.History{
+			V1Compatibility: string(p),
+		})
+	}
+
+	return history
+}