@@ -102,13 +102,12 @@ func (p *v2Puller) pullV2Repository(tag string) (err error) {

 // downloadInfo is used to pass information from download to extractor

 type downloadInfo struct {

-	img      *image.Image

-	tmpFile  *os.File

-	digest   digest.Digest

-	layer    distribution.ReadSeekCloser

-	size     int64

-	err      chan error

-	verified bool

+	img     *image.Image

+	tmpFile *os.File

+	digest  digest.Digest

+	layer   distribution.ReadSeekCloser

+	size    int64

+	err     chan error

 }

 type errVerification struct{}

@@ -176,9 +175,11 @@ func (p *v2Puller) download(di *downloadInfo) {

 	out.Write(p.sf.FormatProgress(stringid.TruncateID(di.img.ID), "Verifying Checksum", nil))

-	di.verified = verifier.Verified()

-	if !di.verified {

-		logrus.Infof("Image verification failed for layer %s", di.digest)

+	if !verifier.Verified() {

+		err = fmt.Errorf("filesystem layer verification failed for digest %s", di.digest)

+		logrus.Error(err)

+		di.err <- err

+		return

 	}

 	out.Write(p.sf.FormatProgress(stringid.TruncateID(di.img.ID), "Download complete", nil))

@@ -252,7 +253,6 @@ func (p *v2Puller) pullV2Tag(tag, taggedName string) (bool, error) {

 				return false, err

 			}

 		}

-		verified = verified && d.verified

 		if d.layer != nil {

 			// if tmpFile is empty assume download and extracted elsewhere

 			defer os.Remove(d.tmpFile.Name())

@@ -368,6 +368,28 @@ func (p *v2Puller) verifyTrustedKeys(namespace string, keys []libtrust.PublicKey

 }

 func (p *v2Puller) validateManifest(m *manifest.SignedManifest, tag string) (verified bool, err error) {

+	// If pull by digest, then verify the manifest digest. NOTE: It is

+	// important to do this first, before any other content validation. If the

+	// digest cannot be verified, don't even bother with those other things.

+	if manifestDigest, err := digest.ParseDigest(tag); err == nil {

+		verifier, err := digest.NewDigestVerifier(manifestDigest)

+		if err != nil {

+			return false, err

+		}

+		payload, err := m.Payload()

+		if err != nil {

+			return false, err

+		}

+		if _, err := verifier.Write(payload); err != nil {

+			return false, err

+		}

+		if !verifier.Verified() {

+			err := fmt.Errorf("image verification failed for digest %s", manifestDigest)

+			logrus.Error(err)

+			return false, err

+		}

+	}

+

 	// TODO(tiborvass): what's the usecase for having manifest == nil and err == nil ? Shouldn't be the error be "DoesNotExist" ?

 	if m == nil {

 		return false, fmt.Errorf("image manifest does not exist for tag %q", tag)

@@ -389,21 +411,5 @@ func (p *v2Puller) validateManifest(m *manifest.SignedManifest, tag string) (ver

 	if err != nil {

 		return false, fmt.Errorf("error verifying manifest keys: %v", err)

 	}

-	localDigest, err := digest.ParseDigest(tag)

-	// if pull by digest, then verify

-	if err == nil {

-		verifier, err := digest.NewDigestVerifier(localDigest)

-		if err != nil {

-			return false, err

-		}

-		payload, err := m.Payload()

-		if err != nil {

-			return false, err

-		}

-		if _, err := verifier.Write(payload); err != nil {

-			return false, err

-		}

-		verified = verified && verifier.Verified()

-	}

 	return verified, nil

 }

@@ -1,25 +1,29 @@

 package main

 import (

+	"encoding/json"

 	"fmt"

 	"regexp"

 	"strings"

+	"github.com/docker/distribution/digest"

+	"github.com/docker/distribution/manifest"

 	"github.com/docker/docker/utils"

 	"github.com/go-check/check"

 )

 var (

-	repoName        = fmt.Sprintf("%v/dockercli/busybox-by-dgst", privateRegistryURL)

+	remoteRepoName  = "dockercli/busybox-by-dgst"

+	repoName        = fmt.Sprintf("%v/%s", privateRegistryURL, remoteRepoName)

 	pushDigestRegex = regexp.MustCompile("[\\S]+: digest: ([\\S]+) size: [0-9]+")

 	digestRegex     = regexp.MustCompile("Digest: ([\\S]+)")

 )

-func setupImage(c *check.C) (string, error) {

+func setupImage(c *check.C) (digest.Digest, error) {

 	return setupImageWithTag(c, "latest")

 }

-func setupImageWithTag(c *check.C, tag string) (string, error) {

+func setupImageWithTag(c *check.C, tag string) (digest.Digest, error) {

 	containerName := "busyboxbydigest"

 	dockerCmd(c, "run", "-d", "-e", "digest=1", "--name", containerName, "busybox")

@@ -52,7 +56,7 @@ func setupImageWithTag(c *check.C, tag string) (string, error) {

 	}

 	pushDigest := matches[1]

-	return pushDigest, nil

+	return digest.Digest(pushDigest), nil

 }

 func (s *DockerRegistrySuite) TestPullByTagDisplaysDigest(c *check.C) {

@@ -72,7 +76,7 @@ func (s *DockerRegistrySuite) TestPullByTagDisplaysDigest(c *check.C) {

 	pullDigest := matches[1]

 	// make sure the pushed and pull digests match

-	if pushDigest != pullDigest {

+	if pushDigest.String() != pullDigest {

 		c.Fatalf("push digest %q didn't match pull digest %q", pushDigest, pullDigest)

 	}

 }

@@ -95,7 +99,7 @@ func (s *DockerRegistrySuite) TestPullByDigest(c *check.C) {

 	pullDigest := matches[1]

 	// make sure the pushed and pull digests match

-	if pushDigest != pullDigest {

+	if pushDigest.String() != pullDigest {

 		c.Fatalf("push digest %q didn't match pull digest %q", pushDigest, pullDigest)

 	}

 }

@@ -291,7 +295,7 @@ func (s *DockerRegistrySuite) TestListImagesWithDigests(c *check.C) {

 	out, _ := dockerCmd(c, "images", "--digests")

 	// make sure repo shown, tag=<none>, digest = $digest1

-	re1 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest1 + `\s`)

+	re1 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest1.String() + `\s`)

 	if !re1.MatchString(out) {

 		c.Fatalf("expected %q: %s", re1.String(), out)

 	}

@@ -319,7 +323,7 @@ func (s *DockerRegistrySuite) TestListImagesWithDigests(c *check.C) {

 	}

 	// make sure repo shown, tag=<none>, digest = $digest2

-	re2 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest2 + `\s`)

+	re2 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest2.String() + `\s`)

 	if !re2.MatchString(out) {

 		c.Fatalf("expected %q: %s", re2.String(), out)

 	}

@@ -332,7 +336,7 @@ func (s *DockerRegistrySuite) TestListImagesWithDigests(c *check.C) {

 	// make sure image 1 has repo, tag, <none> AND repo, <none>, digest

 	reWithTag1 := regexp.MustCompile(`\s*` + repoName + `\s*tag1\s*<none>\s`)

-	reWithDigest1 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest1 + `\s`)

+	reWithDigest1 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest1.String() + `\s`)

 	if !reWithTag1.MatchString(out) {

 		c.Fatalf("expected %q: %s", reWithTag1.String(), out)

 	}

@@ -357,7 +361,7 @@ func (s *DockerRegistrySuite) TestListImagesWithDigests(c *check.C) {

 	// make sure image 2 has repo, tag, digest

 	reWithTag2 := regexp.MustCompile(`\s*` + repoName + `\s*tag2\s*<none>\s`)

-	reWithDigest2 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest2 + `\s`)

+	reWithDigest2 := regexp.MustCompile(`\s*` + repoName + `\s*<none>\s*` + digest2.String() + `\s`)

 	if !reWithTag2.MatchString(out) {

 		c.Fatalf("expected %q: %s", reWithTag2.String(), out)

 	}

@@ -401,3 +405,95 @@ func (s *DockerRegistrySuite) TestDeleteImageByIDOnlyPulledByDigest(c *check.C)

 	dockerCmd(c, "rmi", imageID)

 }

+

+// TestPullFailsWithAlteredManifest tests that a `docker pull` fails when

+// we have modified a manifest blob and its digest cannot be verified.

+func (s *DockerRegistrySuite) TestPullFailsWithAlteredManifest(c *check.C) {

+	manifestDigest, err := setupImage(c)

+	if err != nil {

+		c.Fatalf("error setting up image: %v", err)

+	}

+

+	// Load the target manifest blob.

+	manifestBlob := s.reg.readBlobContents(c, manifestDigest)

+

+	var imgManifest manifest.Manifest

+	if err := json.Unmarshal(manifestBlob, &imgManifest); err != nil {

+		c.Fatalf("unable to decode image manifest from blob: %s", err)

+	}

+

+	// Add a malicious layer digest to the list of layers in the manifest.

+	imgManifest.FSLayers = append(imgManifest.FSLayers, manifest.FSLayer{

+		BlobSum: digest.Digest("sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"),

+	})

+

+	// Move the existing data file aside, so that we can replace it with a

+	// malicious blob of data. NOTE: we defer the returned undo func.

+	undo := s.reg.tempMoveBlobData(c, manifestDigest)

+	defer undo()

+

+	alteredManifestBlob, err := json.Marshal(imgManifest)

+	if err != nil {

+		c.Fatalf("unable to encode altered image manifest to JSON: %s", err)

+	}

+

+	s.reg.writeBlobContents(c, manifestDigest, alteredManifestBlob)

+

+	// Now try pulling that image by digest. We should get an error about

+	// digest verification for the manifest digest.

+

+	// Pull from the registry using the <name>@<digest> reference.

+	imageReference := fmt.Sprintf("%s@%s", repoName, manifestDigest)

+	out, exitStatus, _ := dockerCmdWithError("pull", imageReference)

+	if exitStatus == 0 {

+		c.Fatalf("expected a zero exit status but got %d: %s", exitStatus, out)

+	}

+

+	expectedErrorMsg := fmt.Sprintf("image verification failed for digest %s", manifestDigest)

+	if !strings.Contains(out, expectedErrorMsg) {

+		c.Fatalf("expected error message %q in output: %s", expectedErrorMsg, out)

+	}

+}

+

+// TestPullFailsWithAlteredLayer tests that a `docker pull` fails when

+// we have modified a layer blob and its digest cannot be verified.

+func (s *DockerRegistrySuite) TestPullFailsWithAlteredLayer(c *check.C) {

+	manifestDigest, err := setupImage(c)

+	if err != nil {

+		c.Fatalf("error setting up image: %v", err)

+	}

+

+	// Load the target manifest blob.

+	manifestBlob := s.reg.readBlobContents(c, manifestDigest)

+

+	var imgManifest manifest.Manifest

+	if err := json.Unmarshal(manifestBlob, &imgManifest); err != nil {

+		c.Fatalf("unable to decode image manifest from blob: %s", err)

+	}

+

+	// Next, get the digest of one of the layers from the manifest.

+	targetLayerDigest := imgManifest.FSLayers[0].BlobSum

+

+	// Move the existing data file aside, so that we can replace it with a

+	// malicious blob of data. NOTE: we defer the returned undo func.

+	undo := s.reg.tempMoveBlobData(c, targetLayerDigest)

+	defer undo()

+

+	// Now make a fake data blob in this directory.

+	s.reg.writeBlobContents(c, targetLayerDigest, []byte("This is not the data you are looking for."))

+

+	// Now try pulling that image by digest. We should get an error about

+	// digest verification for the target layer digest.

+

+	// Pull from the registry using the <name>@<digest> reference.

+	imageReference := fmt.Sprintf("%s@%s", repoName, manifestDigest)

+	out, exitStatus, _ := dockerCmdWithError("pull", imageReference)

+	if exitStatus == 0 {

+		c.Fatalf("expected a zero exit status but got: %d", exitStatus)

+	}

+

+	expectedErrorMsg := fmt.Sprintf("filesystem layer verification failed for digest %s", targetLayerDigest)

+	if !strings.Contains(out, expectedErrorMsg) {

+		c.Fatalf("expected error message %q in output: %s", expectedErrorMsg, out)

+	}

+}

@@ -8,6 +8,7 @@ import (

 	"os/exec"

 	"path/filepath"

+	"github.com/docker/distribution/digest"

 	"github.com/go-check/check"

 )

@@ -70,3 +71,50 @@ func (t *testRegistryV2) Close() {

 	t.cmd.Process.Kill()

 	os.RemoveAll(t.dir)

 }

+

+func (t *testRegistryV2) getBlobFilename(blobDigest digest.Digest) string {

+	// Split the digest into it's algorithm and hex components.

+	dgstAlg, dgstHex := blobDigest.Algorithm(), blobDigest.Hex()

+

+	// The path to the target blob data looks something like:

+	//   baseDir + "docker/registry/v2/blobs/sha256/a3/a3ed...46d4/data"

+	return fmt.Sprintf("%s/docker/registry/v2/blobs/%s/%s/%s/data", t.dir, dgstAlg, dgstHex[:2], dgstHex)

+}

+

+func (t *testRegistryV2) readBlobContents(c *check.C, blobDigest digest.Digest) []byte {

+	// Load the target manifest blob.

+	manifestBlob, err := ioutil.ReadFile(t.getBlobFilename(blobDigest))

+	if err != nil {

+		c.Fatalf("unable to read blob: %s", err)

+	}

+

+	return manifestBlob

+}

+

+func (t *testRegistryV2) writeBlobContents(c *check.C, blobDigest digest.Digest, data []byte) {

+	if err := ioutil.WriteFile(t.getBlobFilename(blobDigest), data, os.FileMode(0644)); err != nil {

+		c.Fatalf("unable to write malicious data blob: %s", err)

+	}

+}

+

+func (t *testRegistryV2) tempMoveBlobData(c *check.C, blobDigest digest.Digest) (undo func()) {

+	tempFile, err := ioutil.TempFile("", "registry-temp-blob-")

+	if err != nil {

+		c.Fatalf("unable to get temporary blob file: %s", err)

+	}

+	tempFile.Close()

+

+	blobFilename := t.getBlobFilename(blobDigest)

+

+	// Move the existing data file aside, so that we can replace it with a

+	// another blob of data.

+	if err := os.Rename(blobFilename, tempFile.Name()); err != nil {

+		os.Remove(tempFile.Name())

+		c.Fatalf("unable to move data blob: %s", err)

+	}

+

+	return func() {

+		os.Rename(tempFile.Name(), blobFilename)

+		os.Remove(tempFile.Name())

+	}

+}