Skip to content

Commit

Permalink
Properly verify manifests and layer digests on pull
Browse files Browse the repository at this point in the history 
To ensure manifest integrity when pulling by digest, this changeset ensures
that not only the remote digest provided by the registry is verified but also
that the digest provided on the command line is checked, as well. If this check
fails, the pull is cancelled as with an error. Inspection also should that
while layers were being verified against their digests, the error was being
treated as tech preview image signing verification error. This, in fact, is not
a tech preview and opens up the docker daemon to man in the middle attacks that
can be avoided with the v2 registry protocol.

As a matter of cleanliness, the digest package from the distribution project
has been updated to latest version. There were some recent improvements in the
digest package.

Signed-off-by: Stephen J Day <[email protected]>
  • Loading branch information
@stevvooe
stevvooe committed May 29, 2015
1 parent d79654d commit 06612cc
Show file tree
Hide file tree
Showing 19 changed files with 820 additions and 178 deletions.
102 changes: 73 additions & 29 deletions graph/manifest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,62 +8,106 @@ import (
"github.com/docker/distribution/digest"
"github.com/docker/docker/registry"
"github.com/docker/docker/trust"
"github.com/docker/docker/utils"
"github.com/docker/libtrust"
)

// 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(manifestBytes []byte, dgst, ref string) (*registry.ManifestData, bool, error) {
// loadManifest loads a manifest from a byte array and verifies its content,
// returning the local digest, the manifest itself, whether or not it was
// verified. If ref is a digest, rather than a tag, this will be treated as
// the local digest. An error will be returned if the signature verification
// fails, local digest verification fails and, if provided, the remote digest
// verification fails. The boolean return will only be false without error on
// the failure of signatures trust check.
func (s *TagStore) loadManifest(manifestBytes []byte, ref string, remoteDigest digest.Digest) (digest.Digest, *registry.ManifestData, bool, error) {
sig, err := libtrust.ParsePrettySignature(manifestBytes, "signatures")
if err != nil {
return nil, false, fmt.Errorf("error parsing payload: %s", err)
return "", nil, false, fmt.Errorf("error parsing payload: %s", err)
}

keys, err := sig.Verify()
if err != nil {
return nil, false, fmt.Errorf("error verifying payload: %s", err)
return "", nil, false, fmt.Errorf("error verifying payload: %s", err)
}

payload, err := sig.Payload()
if err != nil {
return nil, false, fmt.Errorf("error retrieving payload: %s", err)
return "", nil, false, fmt.Errorf("error retrieving payload: %s", err)
}

var manifestDigest digest.Digest
// TODO(stevvooe): It would be a lot better here to build up a stack of
// verifiers, then push the bytes one time for signatures and digests, but
// the manifests are typically small, so this optimization is not worth
// hacking this code without further refactoring.

if dgst != "" {
manifestDigest, err = digest.ParseDigest(dgst)
var localDigest digest.Digest

// verify the local digest, if present in tag
if dgst, err := digest.ParseDigest(ref); err != nil {
logrus.Debugf("provided manifest reference %q is not a digest: %v", ref, err)

// we don't have a local digest, since we are working from a tag.
// Digest the payload and return that.
localDigest, err = digest.FromBytes(payload)
if err != nil {
return nil, false, fmt.Errorf("invalid manifest digest from registry: %s", err)
// near impossible
logrus.Errorf("error calculating local digest during tag pull: %v", err)
return "", nil, false, err
}

dgstVerifier, err := digest.NewDigestVerifier(manifestDigest)
} else {
// verify the manifest against local ref
verifier, err := digest.NewDigestVerifier(dgst)
if err != nil {
return nil, false, fmt.Errorf("unable to verify manifest digest from registry: %s", err)
// There are not many ways this can go wrong: if it does, its
// fatal. Likley, the cause would be poor validation of the
// incoming reference.
return "", nil, false, fmt.Errorf("error creating verifier for local digest reference %q: %v", dgst, err)
}

dgstVerifier.Write(payload)
if _, err := verifier.Write(payload); err != nil {
return "", nil, false, fmt.Errorf("error writing payload to local digest reference verifier: %v", err)
}

if !dgstVerifier.Verified() {
computedDigest, _ := digest.FromBytes(payload)
return nil, false, fmt.Errorf("unable to verify manifest digest: registry has %q, computed %q", manifestDigest, computedDigest)
if !verifier.Verified() {
return "", nil, false, fmt.Errorf("verification against local digest reference %q failed", dgst)
}

localDigest = dgst
}

if utils.DigestReference(ref) && ref != manifestDigest.String() {
return nil, false, fmt.Errorf("mismatching image manifest digest: got %q, expected %q", manifestDigest, ref)
// verify against the remote digest, if available
if remoteDigest != "" {
if err := remoteDigest.Validate(); err != nil {
return "", nil, false, fmt.Errorf("error validating remote digest %q: %v", remoteDigest, err)
}

verifier, err := digest.NewDigestVerifier(remoteDigest)
if err != nil {
// There are not many ways this can go wrong: if it does, its
// fatal. Likley, the cause would be poor validation of the
// incoming reference.
return "", nil, false, fmt.Errorf("error creating verifier for remote digest %q: %v", remoteDigest, err)
}

if _, err := verifier.Write(payload); err != nil {
return "", nil, false, fmt.Errorf("error writing payload to remote digest verifier (verifier target %q): %v", remoteDigest, err)
}

if !verifier.Verified() {
return "", nil, false, fmt.Errorf("verification against remote digest %q failed", remoteDigest)
}
}

var manifest registry.ManifestData
if err := json.Unmarshal(payload, &manifest); err != nil {
return nil, false, fmt.Errorf("error unmarshalling manifest: %s", err)
return "", nil, false, fmt.Errorf("error unmarshalling manifest: %s", err)
}
if manifest.SchemaVersion != 1 {
return nil, false, fmt.Errorf("unsupported schema version: %d", manifest.SchemaVersion)
return "", nil, false, fmt.Errorf("unsupported schema version: %d", manifest.SchemaVersion)
}

// validate the contents of the manifest
if err := validateManifest(&manifest); err != nil {
return "", nil, false, err
}

var verified bool
Expand All @@ -74,14 +118,14 @@ func (s *TagStore) loadManifest(manifestBytes []byte, dgst, ref string) (*regist
}
b, err := key.MarshalJSON()
if err != nil {
return nil, false, fmt.Errorf("error marshalling public key: %s", err)
return "", nil, false, fmt.Errorf("error marshalling public key: %s", err)
}
// Check key has read/write permission (0x03)
v, err := s.trustService.CheckKey(namespace, b, 0x03)
if err != nil {
vErr, ok := err.(trust.NotVerifiedError)
if !ok {
return nil, false, fmt.Errorf("error running key check: %s", err)
return "", nil, false, fmt.Errorf("error running key check: %s", err)
}
logrus.Debugf("Key check result: %v", vErr)
}
Expand All @@ -90,10 +134,10 @@ func (s *TagStore) loadManifest(manifestBytes []byte, dgst, ref string) (*regist
logrus.Debug("Key check result: verified")
}
}
return &manifest, verified, nil
return localDigest, &manifest, verified, nil
}

func checkValidManifest(manifest *registry.ManifestData) error {
func validateManifest(manifest *registry.ManifestData) error {
if len(manifest.FSLayers) != len(manifest.History) {
return fmt.Errorf("length of history not equal to number of layers")
}
Expand Down
59 changes: 33 additions & 26 deletions graph/pull.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,17 +458,6 @@ func WriteStatus(requestedTag string, out io.Writer, sf *streamformatter.StreamF
}
}

// downloadInfo is used to pass information from download to extractor
type downloadInfo struct {
imgJSON []byte
img *image.Image
digest digest.Digest
tmpFile *os.File
length int64
downloaded bool
err chan error
}

func (s *TagStore) pullV2Repository(r *registry.Session, out io.Writer, repoInfo *registry.RepositoryInfo, tag string, sf *streamformatter.StreamFormatter) error {
endpoint, err := r.V2RegistryEndpoint(repoInfo.Index)
if err != nil {
Expand Down Expand Up @@ -518,27 +507,34 @@ func (s *TagStore) pullV2Repository(r *registry.Session, out io.Writer, repoInfo
func (s *TagStore) pullV2Tag(r *registry.Session, out io.Writer, endpoint *registry.Endpoint, repoInfo *registry.RepositoryInfo, tag string, sf *streamformatter.StreamFormatter, auth *registry.RequestAuthorization) (bool, error) {
logrus.Debugf("Pulling tag from V2 registry: %q", tag)

manifestBytes, manifestDigest, err := r.GetV2ImageManifest(endpoint, repoInfo.RemoteName, tag, auth)
remoteDigest, manifestBytes, err := r.GetV2ImageManifest(endpoint, repoInfo.RemoteName, tag, auth)
if err != nil {
return false, err
}

// loadManifest ensures that the manifest payload has the expected digest
// if the tag is a digest reference.
manifest, verified, err := s.loadManifest(manifestBytes, manifestDigest, tag)
localDigest, manifest, verified, err := s.loadManifest(manifestBytes, tag, remoteDigest)
if err != nil {
return false, fmt.Errorf("error verifying manifest: %s", err)
}

if err := checkValidManifest(manifest); err != nil {
return false, err
}

if verified {
logrus.Printf("Image manifest for %s has been verified", utils.ImageReference(repoInfo.CanonicalName, tag))
}
out.Write(sf.FormatStatus(tag, "Pulling from %s", repoInfo.CanonicalName))

// downloadInfo is used to pass information from download to extractor
type downloadInfo struct {
imgJSON []byte
img *image.Image
digest digest.Digest
tmpFile *os.File
length int64
downloaded bool
err chan error
}

downloads := make([]downloadInfo, len(manifest.FSLayers))

for i := len(manifest.FSLayers) - 1; i >= 0; i-- {
Expand Down Expand Up @@ -611,8 +607,7 @@ func (s *TagStore) pullV2Tag(r *registry.Session, out io.Writer, endpoint *regis
out.Write(sf.FormatProgress(stringid.TruncateID(img.ID), "Verifying Checksum", nil))

if !verifier.Verified() {
logrus.Infof("Image verification failed: checksum mismatch for %q", di.digest.String())
verified = false
return fmt.Errorf("image layer digest verification failed for %q", di.digest)
}

out.Write(sf.FormatProgress(stringid.TruncateID(img.ID), "Download complete", nil))
Expand Down Expand Up @@ -689,15 +684,27 @@ func (s *TagStore) pullV2Tag(r *registry.Session, out io.Writer, endpoint *regis
out.Write(sf.FormatStatus(utils.ImageReference(repoInfo.CanonicalName, tag), "The image you are pulling has been verified. Important: image verification is a tech preview feature and should not be relied on to provide security."))
}

if manifestDigest != "" {
out.Write(sf.FormatStatus("", "Digest: %s", manifestDigest))
if localDigest != remoteDigest { // this is not a verification check.
// NOTE(stevvooe): This is a very defensive branch and should never
// happen, since all manifest digest implementations use the same
// algorithm.
logrus.WithFields(
logrus.Fields{
"local": localDigest,
"remote": remoteDigest,
}).Debugf("local digest does not match remote")

out.Write(sf.FormatStatus("", "Remote Digest: %s", remoteDigest))
}

out.Write(sf.FormatStatus("", "Digest: %s", localDigest))

// always set the digest so we can use the digest whether we pull by it or not.
if err = s.SetDigest(repoInfo.LocalName, localDigest.String(), downloads[0].img.ID); err != nil {
return false, err
}

if utils.DigestReference(tag) {
if err = s.SetDigest(repoInfo.LocalName, tag, downloads[0].img.ID); err != nil {
return false, err
}
} else {
if !utils.DigestReference(tag) {
// only set the repository/tag -> image ID mapping when pulling by tag (i.e. not by digest)
if err = s.Tag(repoInfo.LocalName, tag, downloads[0].img.ID, true); err != nil {
return false, err
Expand Down
2 changes: 1 addition & 1 deletion graph/push.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ func (s *TagStore) pushV2Repository(r *registry.Session, localRepo Repository, o
m.History[i] = &registry.ManifestHistory{V1Compatibility: string(jsonData)}
}

if err := checkValidManifest(m); err != nil {
if err := validateManifest(m); err != nil {
return fmt.Errorf("invalid manifest: %s", err)
}

Expand Down
2 changes: 1 addition & 1 deletion hack/vendor.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ clone git github.com/vishvananda/netns 008d17ae001344769b031375bdb38a86219154c6
clone git github.com/vishvananda/netlink 8eb64238879fed52fd51c5b30ad20b928fb4c36c

# get distribution packages
clone git github.com/docker/distribution d957768537c5af40e4f4cd96871f7b2bde9e2923
clone git github.com/docker/distribution b9eeb328080d367dbde850ec6e94f1e4ac2b5efe
mv src/github.com/docker/distribution/digest tmp-digest
mv src/github.com/docker/distribution/registry/api tmp-api
rm -rf src/github.com/docker/distribution
Expand Down
41 changes: 30 additions & 11 deletions registry/session_v2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,42 +68,61 @@ func (r *Session) GetV2Authorization(ep *Endpoint, imageName string, readOnly bo
// 1.c) if anything else, err
// 2) PUT the created/signed manifest
//
func (r *Session) GetV2ImageManifest(ep *Endpoint, imageName, tagName string, auth *RequestAuthorization) ([]byte, string, error) {

// GetV2ImageManifest simply fetches the bytes of a manifest and the remote
// digest, if available in the request. Note that the application shouldn't
// rely on the untrusted remoteDigest, and should also verify against a
// locally provided digest, if applicable.
func (r *Session) GetV2ImageManifest(ep *Endpoint, imageName, tagName string, auth *RequestAuthorization) (remoteDigest digest.Digest, p []byte, err error) {
routeURL, err := getV2Builder(ep).BuildManifestURL(imageName, tagName)
if err != nil {
return nil, "", err
return "", nil, err
}

method := "GET"
logrus.Debugf("[registry] Calling %q %s", method, routeURL)

req, err := http.NewRequest(method, routeURL, nil)
if err != nil {
return nil, "", err
return "", nil, err
}

if err := auth.Authorize(req); err != nil {
return nil, "", err
return "", nil, err
}

res, err := r.client.Do(req)
if err != nil {
return nil, "", err
return "", nil, err
}
defer res.Body.Close()

if res.StatusCode != 200 {
if res.StatusCode == 401 {
return nil, "", errLoginRequired
return "", nil, errLoginRequired
} else if res.StatusCode == 404 {
return nil, "", ErrDoesNotExist
return "", nil, ErrDoesNotExist
}
return nil, "", httputils.NewHTTPRequestError(fmt.Sprintf("Server error: %d trying to fetch for %s:%s", res.StatusCode, imageName, tagName), res)
return "", nil, httputils.NewHTTPRequestError(fmt.Sprintf("Server error: %d trying to fetch for %s:%s", res.StatusCode, imageName, tagName), res)
}

manifestBytes, err := ioutil.ReadAll(res.Body)
p, err = ioutil.ReadAll(res.Body)
if err != nil {
return nil, "", fmt.Errorf("Error while reading the http response: %s", err)
return "", nil, fmt.Errorf("Error while reading the http response: %s", err)
}

return manifestBytes, res.Header.Get(DockerDigestHeader), nil
dgstHdr := res.Header.Get(DockerDigestHeader)
if dgstHdr != "" {
remoteDigest, err = digest.ParseDigest(dgstHdr)
if err != nil {
// NOTE(stevvooe): Including the remote digest is optional. We
// don't need to verify against it, but it is good practice.
remoteDigest = ""
logrus.Debugf("error parsing remote digest when fetching %v: %v", routeURL, err)
}
}

return
}

// - Succeeded to head image blob (already exists)
Expand Down
Loading

0 comments on commit 06612cc

Please sign in to comment.