Merge pull request #10456 from dmcgowan/revert-client-signature

Revert client signature

api/client/commands.go

@@ -46,7 +46,6 @@ import (
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/runconfig"
 	"github.com/docker/docker/utils"
-	"github.com/docker/libtrust"
 )
 
 const (
@@ -1191,10 +1190,6 @@ func (cli *DockerCli) CmdPush(args ...string) error {
 	name := cmd.Arg(0)
 
 	cli.LoadConfigFile()
-	trustKey, err := api.LoadOrCreateTrustKey(cli.keyFile)
-	if err != nil {
-		log.Fatal(err)
-	}
 
 	remote, tag := parsers.ParseRepositoryTag(name)
 
@@ -1220,25 +1215,6 @@ func (cli *DockerCli) CmdPush(args ...string) error {
 	v := url.Values{}
 	v.Set("tag", tag)
 
-	body, _, err := readBody(cli.call("GET", "/images/"+remote+"/manifest?"+v.Encode(), nil, false))
-	if err != nil {
-		return err
-	}
-
-	js, err := libtrust.NewJSONSignature(body)
-	if err != nil {
-		return err
-	}
-	err = js.Sign(trustKey)
-	if err != nil {
-		return err
-	}
-
-	signedBody, err := js.PrettySignature("signatures")
-	if err != nil {
-		return err
-	}
-
 	push := func(authConfig registry.AuthConfig) error {
 		buf, err := json.Marshal(authConfig)
 		if err != nil {
@@ -1248,7 +1224,7 @@ func (cli *DockerCli) CmdPush(args ...string) error {
 			base64.URLEncoding.EncodeToString(buf),
 		}
 
-		return cli.stream("POST", "/images/"+remote+"/push?"+v.Encode(), bytes.NewReader(signedBody), cli.out, map[string][]string{
+		return cli.stream("POST", "/images/"+remote+"/push?"+v.Encode(), nil, cli.out, map[string][]string{
 			"X-Registry-Auth": registryAuthHeader,
 		})
 	}

api/server/server.go

@@ -621,18 +621,6 @@ func getImagesSearch(eng *engine.Engine, version version.Version, w http.Respons
 	return job.Run()
 }
 
-func getImageManifest(eng *engine.Engine, version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
-	if err := parseForm(r); err != nil {
-		return err
-	}
-
-	job := eng.Job("image_manifest", vars["name"])
-	job.Setenv("tag", r.Form.Get("tag"))
-	job.Stdout.Add(utils.NewWriteFlusher(w))
-
-	return job.Run()
-}
-
 func postImagesPush(eng *engine.Engine, version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	if vars == nil {
 		return fmt.Errorf("Missing parameter")
@@ -664,15 +652,9 @@ func postImagesPush(eng *engine.Engine, version version.Version, w http.Response
 		}
 	}
 
-	manifest, err := ioutil.ReadAll(r.Body)
-	if err != nil {
-		return err
-	}
-
 	job := eng.Job("push", vars["name"])
 	job.SetenvJson("metaHeaders", metaHeaders)
 	job.SetenvJson("authConfig", authConfig)
-	job.Setenv("manifest", string(manifest))
 	job.Setenv("tag", r.Form.Get("tag"))
 	if version.GreaterThan("1.0") {
 		job.SetenvBool("json", true)
@@ -1325,7 +1307,6 @@ func createRouter(eng *engine.Engine, logging, enableCors bool, dockerVersion st
 			"/images/viz":                     getImagesViz,
 			"/images/search":                  getImagesSearch,
 			"/images/get":                     getImagesGet,
-			"/images/{name:.*}/manifest":      getImageManifest,
 			"/images/{name:.*}/get":           getImagesGet,
 			"/images/{name:.*}/history":       getImagesHistory,
 			"/images/{name:.*}/json":          getImagesByName,

graph/manifest.go

@@ -15,35 +15,6 @@ import (
 	"github.com/docker/libtrust"
 )
 
-func (s *TagStore) CmdManifest(job *engine.Job) engine.Status {
-	if len(job.Args) != 1 {
-		return job.Errorf("usage: %s NAME", job.Name)
-	}
-	name := job.Args[0]
-	tag := job.Getenv("tag")
-	if tag == "" {
-		tag = "latest"
-	}
-
-	// Resolve the Repository name from fqn to endpoint + name
-	repoInfo, err := registry.ParseRepositoryInfo(name)
-	if err != nil {
-		return job.Error(err)
-	}
-
-	manifestBytes, err := s.newManifest(name, repoInfo.RemoteName, tag)
-	if err != nil {
-		return job.Error(err)
-	}
-
-	_, err = job.Stdout.Write(manifestBytes)
-	if err != nil {
-		return job.Error(err)
-	}
-
-	return engine.StatusOK
-}
-
 func (s *TagStore) newManifest(localName, remoteName, tag string) ([]byte, error) {
 	manifest := &registry.ManifestData{
 		Name:          remoteName,
@@ -130,7 +101,12 @@ func (s *TagStore) newManifest(localName, remoteName, tag string) ([]byte, error
 	return manifestBytes, nil
 }
 
-func (s *TagStore) verifyManifest(eng *engine.Engine, manifestBytes []byte) (*registry.ManifestData, bool, error) {
+// loadManifest loads a manifest from a byte array and verifies its content.
+// The signature must be verified or an error is returned. If the manifest
+// contains no signatures by a trusted key for the name in the manifest, the
+// image is not considered verified. The parsed manifest object and a boolean
+// for whether the manifest is verified is returned.
+func (s *TagStore) loadManifest(eng *engine.Engine, manifestBytes []byte) (*registry.ManifestData, bool, error) {
 	sig, err := libtrust.ParsePrettySignature(manifestBytes, "signatures")
 	if err != nil {
 		return nil, false, fmt.Errorf("error parsing payload: %s", err)

graph/pull.go

@@ -417,7 +417,7 @@ func (s *TagStore) pullV2Tag(eng *engine.Engine, r *registry.Session, out io.Wri
 		return false, err
 	}
 
-	manifest, verified, err := s.verifyManifest(eng, manifestBytes)
+	manifest, verified, err := s.loadManifest(eng, manifestBytes)
 	if err != nil {
 		return false, fmt.Errorf("error verifying manifest: %s", err)
 	}

graph/push.go

@@ -65,6 +65,25 @@ func (s *TagStore) getImageList(localRepo map[string]string, requestedTag string
 	return imageList, tagsByImage, nil
 }
 
+func (s *TagStore) getImageTags(localName, askedTag string) ([]string, error) {
+	localRepo, err := s.Get(localName)
+	if err != nil {
+		return nil, err
+	}
+	log.Debugf("Checking %s against %#v", askedTag, localRepo)
+	if len(askedTag) > 0 {
+		if _, ok := localRepo[askedTag]; !ok {
+			return nil, fmt.Errorf("Tag does not exist for %s:%s", localName, askedTag)
+		}
+		return []string{askedTag}, nil
+	}
+	var tags []string
+	for tag := range localRepo {
+		tags = append(tags, tag)
+	}
+	return tags, nil
+}
+
 // createImageIndex returns an index of an image's layer IDs and tags.
 func (s *TagStore) createImageIndex(images []string, tags map[string][]string) []*registry.ImgData {
 	var imageIndex []*registry.ImgData
@@ -251,7 +270,7 @@ func (s *TagStore) pushImage(r *registry.Session, out io.Writer, imgID, ep strin
 	return imgData.Checksum, nil
 }
 
-func (s *TagStore) pushV2Repository(r *registry.Session, eng *engine.Engine, out io.Writer, repoInfo *registry.RepositoryInfo, manifestBytes, tag string, sf *utils.StreamFormatter) error {
+func (s *TagStore) pushV2Repository(r *registry.Session, eng *engine.Engine, out io.Writer, repoInfo *registry.RepositoryInfo, tag string, sf *utils.StreamFormatter) error {
 	if repoInfo.Official {
 		j := eng.Job("trust_update_base")
 		if err := j.Run(); err != nil {
@@ -263,13 +282,22 @@ func (s *TagStore) pushV2Repository(r *registry.Session, eng *engine.Engine, out
 	if err != nil {
 		return fmt.Errorf("error getting registry endpoint: %s", err)
 	}
+
+	tags, err := s.getImageTags(repoInfo.LocalName, tag)
+	if err != nil {
+		return err
+	}
+	if len(tags) == 0 {
+		return fmt.Errorf("No tags to push for %s", repoInfo.LocalName)
+	}
+
 	auth, err := r.GetV2Authorization(endpoint, repoInfo.RemoteName, false)
 	if err != nil {
 		return fmt.Errorf("error getting authorization: %s", err)
 	}
 
-	// if no manifest is given, generate and sign with the key associated with the local tag store
-	if len(manifestBytes) == 0 {
+	for _, tag := range tags {
+		log.Debugf("Pushing %s:%s to v2 repository", repoInfo.LocalName, tag)
 		mBytes, err := s.newManifest(repoInfo.LocalName, repoInfo.RemoteName, tag)
 		if err != nil {
 			return err
@@ -287,63 +315,66 @@ func (s *TagStore) pushV2Repository(r *registry.Session, eng *engine.Engine, out
 		if err != nil {
 			return err
 		}
-		log.Infof("Signed manifest using daemon's key: %s", s.trustKey.KeyID())
-
-		manifestBytes = string(signedBody)
-	}
-
-	manifest, verified, err := s.verifyManifest(eng, []byte(manifestBytes))
-	if err != nil {
-		return fmt.Errorf("error verifying manifest: %s", err)
-	}
-
-	if err := checkValidManifest(manifest); err != nil {
-		return fmt.Errorf("invalid manifest: %s", err)
-	}
-
-	if !verified {
-		log.Debugf("Pushing unverified image")
-	}
+		log.Infof("Signed manifest for %s:%s using daemon's key: %s", repoInfo.LocalName, tag, s.trustKey.KeyID())
 
-	for i := len(manifest.FSLayers) - 1; i >= 0; i-- {
-		var (
-			sumStr  = manifest.FSLayers[i].BlobSum
-			imgJSON = []byte(manifest.History[i].V1Compatibility)
-		)
+		manifestBytes := string(signedBody)
 
-		sumParts := strings.SplitN(sumStr, ":", 2)
-		if len(sumParts) < 2 {
-			return fmt.Errorf("Invalid checksum: %s", sumStr)
+		manifest, verified, err := s.loadManifest(eng, signedBody)
+		if err != nil {
+			return fmt.Errorf("error verifying manifest: %s", err)
 		}
-		manifestSum := sumParts[1]
 
-		img, err := image.NewImgJSON(imgJSON)
-		if err != nil {
-			return fmt.Errorf("Failed to parse json: %s", err)
+		if err := checkValidManifest(manifest); err != nil {
+			return fmt.Errorf("invalid manifest: %s", err)
 		}
 
-		// Call mount blob
-		exists, err := r.HeadV2ImageBlob(endpoint, repoInfo.RemoteName, sumParts[0], manifestSum, auth)
-		if err != nil {
-			out.Write(sf.FormatProgress(utils.TruncateID(img.ID), "Image push failed", nil))
-			return err
+		if verified {
+			log.Infof("Pushing verified image, key %s is registered for %q", s.trustKey.KeyID(), repoInfo.RemoteName)
 		}
 
-		if !exists {
-			if err := s.PushV2Image(r, img, endpoint, repoInfo.RemoteName, sumParts[0], manifestSum, sf, out, auth); err != nil {
+		for i := len(manifest.FSLayers) - 1; i >= 0; i-- {
+			var (
+				sumStr  = manifest.FSLayers[i].BlobSum
+				imgJSON = []byte(manifest.History[i].V1Compatibility)
+			)
+
+			sumParts := strings.SplitN(sumStr, ":", 2)
+			if len(sumParts) < 2 {
+				return fmt.Errorf("Invalid checksum: %s", sumStr)
+			}
+			manifestSum := sumParts[1]
+
+			img, err := image.NewImgJSON(imgJSON)
+			if err != nil {
+				return fmt.Errorf("Failed to parse json: %s", err)
+			}
+
+			// Call mount blob
+			exists, err := r.HeadV2ImageBlob(endpoint, repoInfo.RemoteName, sumParts[0], manifestSum, auth)
+			if err != nil {
+				out.Write(sf.FormatProgress(utils.TruncateID(img.ID), "Image push failed", nil))
 				return err
 			}
-		} else {
-			out.Write(sf.FormatProgress(utils.TruncateID(img.ID), "Image already exists", nil))
+
+			if !exists {
+				if err := s.pushV2Image(r, img, endpoint, repoInfo.RemoteName, sumParts[0], manifestSum, sf, out, auth); err != nil {
+					return err
+				}
+			} else {
+				out.Write(sf.FormatProgress(utils.TruncateID(img.ID), "Image already exists", nil))
+			}
 		}
-	}
 
-	// push the manifest
-	return r.PutV2ImageManifest(endpoint, repoInfo.RemoteName, tag, bytes.NewReader([]byte(manifestBytes)), auth)
+		// push the manifest
+		if err := r.PutV2ImageManifest(endpoint, repoInfo.RemoteName, tag, bytes.NewReader([]byte(manifestBytes)), auth); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 // PushV2Image pushes the image content to the v2 registry, first buffering the contents to disk
-func (s *TagStore) PushV2Image(r *registry.Session, img *image.Image, endpoint *registry.Endpoint, imageName, sumType, sumStr string, sf *utils.StreamFormatter, out io.Writer, auth *registry.RequestAuthorization) error {
+func (s *TagStore) pushV2Image(r *registry.Session, img *image.Image, endpoint *registry.Endpoint, imageName, sumType, sumStr string, sf *utils.StreamFormatter, out io.Writer, auth *registry.RequestAuthorization) error {
 	out.Write(sf.FormatProgress(utils.TruncateID(img.ID), "Buffering to Disk", nil))
 
 	image, err := s.graph.Get(img.ID)
@@ -398,7 +429,6 @@ func (s *TagStore) CmdPush(job *engine.Job) engine.Status {
 	}
 
 	tag := job.Getenv("tag")
-	manifestBytes := job.Getenv("manifest")
 	job.GetenvJson("authConfig", authConfig)
 	job.GetenvJson("metaHeaders", &metaHeaders)
 
@@ -418,12 +448,8 @@ func (s *TagStore) CmdPush(job *engine.Job) engine.Status {
 		return job.Error(err2)
 	}
 
-	if len(tag) == 0 {
-		tag = DEFAULTTAG
-	}
-
 	if repoInfo.Index.Official || endpoint.Version == registry.APIVersion2 {
-		err := s.pushV2Repository(r, job.Eng, job.Stdout, repoInfo, manifestBytes, tag, sf)
+		err := s.pushV2Repository(r, job.Eng, job.Stdout, repoInfo, tag, sf)
 		if err == nil {
 			return engine.StatusOK
 		}

graph/service.go

@@ -25,7 +25,6 @@ func (s *TagStore) Install(eng *engine.Engine) error {
 		"import":         s.CmdImport,
 		"pull":           s.CmdPull,
 		"push":           s.CmdPush,
-		"image_manifest": s.CmdManifest,
 	} {
 		if err := eng.Register(name, handler); err != nil {
 			return fmt.Errorf("Could not register %q: %v", name, err)

integration-cli/docker_cli_pull_test.go

@@ -24,6 +24,7 @@ func TestPullImageWithAliases(t *testing.T) {
 		if out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "tag", "busybox", repo)); err != nil {
 			t.Fatalf("Failed to tag image %v: error %v, output %q", repos, err, out)
 		}
+		defer deleteImages(repo)
 		if out, err := exec.Command(dockerBinary, "push", repo).CombinedOutput(); err != nil {
 			t.Fatalf("Failed to push image %v: error %v, output %q", err, string(out))
 		}
@@ -40,7 +41,6 @@ func TestPullImageWithAliases(t *testing.T) {
 	if out, _, err := runCommandWithOutput(pullCmd); err != nil {
 		t.Fatalf("Failed to pull %v: error %v, output %q", repoName, err, out)
 	}
-	defer deleteImages(repos[0])
 	if err := exec.Command(dockerBinary, "inspect", repos[0]).Run(); err != nil {
 		t.Fatalf("Image %v was not pulled down", repos[0])
 	}

integration-cli/docker_cli_push_test.go

@@ -45,7 +45,7 @@ func TestPushUntagged(t *testing.T) {
 
 	repoName := fmt.Sprintf("%v/dockercli/busybox", privateRegistryURL)
 
-	expected := "does not exist"
+	expected := "No tags to push"
 	pushCmd := exec.Command(dockerBinary, "push", repoName)
 	if out, _, err := runCommandWithOutput(pushCmd); err == nil {
 		t.Fatalf("pushing the image to the private registry should have failed: outuput %q", out)
@@ -55,6 +55,46 @@ func TestPushUntagged(t *testing.T) {
 	logDone("push - untagged image")
 }
 
+func TestPushBadTag(t *testing.T) {
+	defer setupRegistry(t)()
+
+	repoName := fmt.Sprintf("%v/dockercli/busybox:latest", privateRegistryURL)
+
+	expected := "does not exist"
+	pushCmd := exec.Command(dockerBinary, "push", repoName)
+	if out, _, err := runCommandWithOutput(pushCmd); err == nil {
+		t.Fatalf("pushing the image to the private registry should have failed: outuput %q", out)
+	} else if !strings.Contains(out, expected) {
+		t.Fatalf("pushing the image failed with an unexpected message: expected %q, got %q", expected, out)
+	}
+	logDone("push - image with bad tag")
+}
+
+func TestPushMultipleTags(t *testing.T) {
+	defer setupRegistry(t)()
+
+	repoName := fmt.Sprintf("%v/dockercli/busybox", privateRegistryURL)
+	repoTag1 := fmt.Sprintf("%v/dockercli/busybox:t1", privateRegistryURL)
+	repoTag2 := fmt.Sprintf("%v/dockercli/busybox:t2", privateRegistryURL)
+	// tag the image to upload it tot he private registry
+	tagCmd1 := exec.Command(dockerBinary, "tag", "busybox", repoTag1)
+	if out, _, err := runCommandWithOutput(tagCmd1); err != nil {
+		t.Fatalf("image tagging failed: %s, %v", out, err)
+	}
+	defer deleteImages(repoTag1)
+	tagCmd2 := exec.Command(dockerBinary, "tag", "busybox", repoTag2)
+	if out, _, err := runCommandWithOutput(tagCmd2); err != nil {
+		t.Fatalf("image tagging failed: %s, %v", out, err)
+	}
+	defer deleteImages(repoTag2)
+
+	pushCmd := exec.Command(dockerBinary, "push", repoName)
+	if out, _, err := runCommandWithOutput(pushCmd); err != nil {
+		t.Fatalf("pushing the image to the private registry has failed: %s, %v", out, err)
+	}
+	logDone("push - multiple tags to private registry")
+}
+
 func TestPushInterrupt(t *testing.T) {
 	defer setupRegistry(t)()