From 60e8d6371276af717d2c7d36a3c2274c715fa85f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 16 May 2016 17:40:16 +0200 Subject: [PATCH] Compute the digest in (skopeo inspect) instead of trusting the registry Compute the digest ourselves, the registry is in general untrusted and computing it ourserlves is easy enough. The stop passing the unverifiedCanonicalDigest value around, simplifying ImageSource.GetManifest and related code. In particular, remove retrieveRawManifest and have internal users just call Manifest() now that we don't need the digest. --- cmd/skopeo/copy.go | 2 +- cmd/skopeo/inspect.go | 17 +++++++++++------ directory/directory.go | 9 ++------- docker/docker_image.go | 36 +++++++++++++----------------------- docker/docker_image_src.go | 11 ++++++----- openshift/openshift.go | 4 ++-- types/types.go | 3 +-- 7 files changed, 36 insertions(+), 46 deletions(-) diff --git a/cmd/skopeo/copy.go b/cmd/skopeo/copy.go index 5fe78ce6..ffa2c2e1 100644 --- a/cmd/skopeo/copy.go +++ b/cmd/skopeo/copy.go @@ -54,7 +54,7 @@ func copyHandler(context *cli.Context) { } signBy := context.String("sign-by") - manifest, _, err := src.GetManifest() + manifest, err := src.GetManifest() if err != nil { logrus.Fatalf("Error reading manifest: %s", err.Error()) } diff --git a/cmd/skopeo/inspect.go b/cmd/skopeo/inspect.go index 790a2017..02e3f2d6 100644 --- a/cmd/skopeo/inspect.go +++ b/cmd/skopeo/inspect.go @@ -7,6 +7,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/codegangsta/cli" + "github.com/projectatomic/skopeo/dockerutils" ) // inspectOutput is the output format of (skopeo inspect), primarily so that we can format it with a simple json.MarshalIndent. @@ -37,22 +38,26 @@ var inspectCmd = cli.Command{ if err != nil { logrus.Fatal(err) } + rawManifest, err := img.Manifest() + if err != nil { + logrus.Fatal(err) + } if c.Bool("raw") { - b, err := img.Manifest() - if err != nil { - logrus.Fatal(err) - } - fmt.Println(string(b)) + fmt.Println(string(rawManifest)) return } imgInspect, err := img.Inspect() if err != nil { logrus.Fatal(err) } + manifestDigest, err := dockerutils.ManifestDigest(rawManifest) + if err != nil { + logrus.Fatalf("Error computing manifest digest: %s", err.Error()) + } outputData := inspectOutput{ Name: imgInspect.Name, Tag: imgInspect.Tag, - Digest: imgInspect.Digest, + Digest: manifestDigest, RepoTags: imgInspect.RepoTags, Created: imgInspect.Created, DockerVersion: imgInspect.DockerVersion, diff --git a/directory/directory.go b/directory/directory.go index e41854ec..a399ef6f 100644 --- a/directory/directory.go +++ b/directory/directory.go @@ -84,13 +84,8 @@ func (s *dirImageSource) IntendedDockerReference() string { return "" } -func (s *dirImageSource) GetManifest() ([]byte, string, error) { - manifest, err := ioutil.ReadFile(manifestPath(s.dir)) - if err != nil { - return nil, "", err - } - - return manifest, "", nil // FIXME? unverifiedCanonicalDigest value - really primarily used by dockerImage +func (s *dirImageSource) GetManifest() ([]byte, error) { + return ioutil.ReadFile(manifestPath(s.dir)) } func (s *dirImageSource) GetLayer(digest string) (io.ReadCloser, error) { diff --git a/docker/docker_image.go b/docker/docker_image.go index 97fad461..47c16454 100644 --- a/docker/docker_image.go +++ b/docker/docker_image.go @@ -20,8 +20,7 @@ var ( type dockerImage struct { src *dockerImageSource - digest string - rawManifest []byte + cachedManifest []byte // Private cache for Manifest(); nil if not yet known. cachedSignatures [][]byte // Private cache for Signatures(); nil if not yet known. } @@ -44,10 +43,14 @@ func (i *dockerImage) IntendedDockerReference() string { // Manifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need. func (i *dockerImage) Manifest() ([]byte, error) { - if err := i.retrieveRawManifest(); err != nil { - return nil, err + if i.cachedManifest == nil { + m, err := i.src.GetManifest() + if err != nil { + return nil, err + } + i.cachedManifest = m } - return i.rawManifest, nil + return i.cachedManifest, nil } // Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need. @@ -76,7 +79,7 @@ func (i *dockerImage) Inspect() (*types.DockerImageManifest, error) { if err != nil { return nil, err } - imgManifest, err := makeImageManifest(i.src.ref.FullName(), ms1, i.digest, tags) + imgManifest, err := makeImageManifest(i.src.ref.FullName(), ms1, tags) if err != nil { return nil, err } @@ -122,7 +125,7 @@ type v1Image struct { OS string `json:"os,omitempty"` } -func makeImageManifest(name string, m *manifestSchema1, dgst string, tagList []string) (*types.DockerImageManifest, error) { +func makeImageManifest(name string, m *manifestSchema1, tagList []string) (*types.DockerImageManifest, error) { v1 := &v1Image{} if err := json.Unmarshal([]byte(m.History[0].V1Compatibility), v1); err != nil { return nil, err @@ -130,7 +133,6 @@ func makeImageManifest(name string, m *manifestSchema1, dgst string, tagList []s return &types.DockerImageManifest{ Name: name, Tag: m.Tag, - Digest: dgst, RepoTags: tagList, DockerVersion: v1.DockerVersion, Created: v1.Created, @@ -181,25 +183,13 @@ func sanitize(s string) string { return strings.Replace(s, "/", "-", -1) } -func (i *dockerImage) retrieveRawManifest() error { - if i.rawManifest != nil { - return nil - } - manblob, unverifiedCanonicalDigest, err := i.src.GetManifest() - if err != nil { - return err - } - i.rawManifest = manblob - i.digest = unverifiedCanonicalDigest - return nil -} - func (i *dockerImage) getSchema1Manifest() (manifest, error) { - if err := i.retrieveRawManifest(); err != nil { + manblob, err := i.Manifest() + if err != nil { return nil, err } mschema1 := &manifestSchema1{} - if err := json.Unmarshal(i.rawManifest, mschema1); err != nil { + if err := json.Unmarshal(manblob, mschema1); err != nil { return nil, err } if err := fixManifestLayers(mschema1); err != nil { diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index 4277be8b..cfaa81a7 100644 --- a/docker/docker_image_src.go +++ b/docker/docker_image_src.go @@ -55,23 +55,24 @@ func (s *dockerImageSource) IntendedDockerReference() string { return fmt.Sprintf("%s:%s", s.ref.Name(), s.tag) } -func (s *dockerImageSource) GetManifest() (manifest []byte, unverifiedCanonicalDigest string, err error) { +func (s *dockerImageSource) GetManifest() ([]byte, error) { url := fmt.Sprintf(manifestURL, s.ref.RemoteName(), s.tag) // TODO(runcom) set manifest version header! schema1 for now - then schema2 etc etc and v1 // TODO(runcom) NO, switch on the resulter manifest like Docker is doing res, err := s.c.makeRequest("GET", url, nil, nil) if err != nil { - return nil, "", err + return nil, err } defer res.Body.Close() manblob, err := ioutil.ReadAll(res.Body) if err != nil { - return nil, "", err + return nil, err } if res.StatusCode != http.StatusOK { - return nil, "", errFetchManifest{res.StatusCode, manblob} + return nil, errFetchManifest{res.StatusCode, manblob} } - return manblob, res.Header.Get("Docker-Content-Digest"), nil + // We might validate manblob against the Docker-Content-Digest header here to protect against transport errors. + return manblob, nil } func (s *dockerImageSource) GetLayer(digest string) (io.ReadCloser, error) { diff --git a/openshift/openshift.go b/openshift/openshift.go index 6425a928..f351bf46 100644 --- a/openshift/openshift.go +++ b/openshift/openshift.go @@ -193,9 +193,9 @@ func (s *openshiftImageSource) IntendedDockerReference() string { return s.client.canonicalDockerReference() } -func (s *openshiftImageSource) GetManifest() (manifest []byte, unverifiedCanonicalDigest string, err error) { +func (s *openshiftImageSource) GetManifest() ([]byte, error) { if err := s.ensureImageIsResolved(); err != nil { - return nil, "", err + return nil, err } return s.docker.GetManifest() } diff --git a/types/types.go b/types/types.go index 4bc37138..a8a87b77 100644 --- a/types/types.go +++ b/types/types.go @@ -35,7 +35,7 @@ type ImageSource interface { // May be "" if unknown. IntendedDockerReference() string // GetManifest returns the image's manifest. It may use a remote (= slow) service. - GetManifest() (manifest []byte, unverifiedCanonicalDigest string, err error) + GetManifest() ([]byte, error) GetLayer(digest string) (io.ReadCloser, error) // GetSignatures returns the image's signatures. It may use a remote (= slow) service. GetSignatures() ([][]byte, error) @@ -71,7 +71,6 @@ type Image interface { type DockerImageManifest struct { Name string Tag string - Digest string RepoTags []string Created time.Time DockerVersion string