diff --git a/graph/manifest.go b/graph/manifest.go index b3f9895771..36b1adcfa1 100644 --- a/graph/manifest.go +++ b/graph/manifest.go @@ -19,19 +19,9 @@ import ( // 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") + payload, keys, err := unpackSignedManifest(manifestBytes) if err != nil { - 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) - } - - payload, err := sig.Payload() - if err != nil { - return "", nil, false, fmt.Errorf("error retrieving payload: %s", err) + return "", nil, false, fmt.Errorf("error unpacking manifest: %v", err) } // TODO(stevvooe): It would be a lot better here to build up a stack of @@ -41,59 +31,32 @@ func (s *TagStore) loadManifest(manifestBytes []byte, ref string, remoteDigest d 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) + // Verify the local digest, if present in ref. ParseDigest will validate + // that the ref is a digest and verify against that if present. Otherwize + // (on error), we simply compute the localDigest and proceed. + if dgst, err := digest.ParseDigest(ref); err == nil { + // verify the manifest against local ref + if err := verifyDigest(dgst, payload); err != nil { + return "", nil, false, fmt.Errorf("verifying local digest: %v", err) + } - // we don't have a local digest, since we are working from a tag. - // Digest the payload and return that. + localDigest = dgst + } else { + // We don't have a local digest, since we are working from a tag. + // Compute the digest of the payload and return that. + logrus.Debugf("provided manifest reference %q is not a digest: %v", ref, err) localDigest, err = digest.FromBytes(payload) if err != nil { // near impossible logrus.Errorf("error calculating local digest during tag pull: %v", err) return "", nil, false, err } - } else { - // verify the manifest against local ref - verifier, err := digest.NewDigestVerifier(dgst) - 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 local digest reference %q: %v", dgst, err) - } - - if _, err := verifier.Write(payload); err != nil { - return "", nil, false, fmt.Errorf("error writing payload to local digest reference verifier: %v", err) - } - - if !verifier.Verified() { - return "", nil, false, fmt.Errorf("verification against local digest reference %q failed", dgst) - } - - localDigest = dgst } // 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) + if err := verifyDigest(remoteDigest, payload); err != nil { + return "", nil, false, fmt.Errorf("verifying remote digest: %v", err) } } @@ -101,9 +64,6 @@ func (s *TagStore) loadManifest(manifestBytes []byte, ref string, remoteDigest d if err := json.Unmarshal(payload, &manifest); err != nil { 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) - } // validate the contents of the manifest if err := validateManifest(&manifest); err != nil { @@ -111,33 +71,101 @@ func (s *TagStore) loadManifest(manifestBytes []byte, ref string, remoteDigest d } var verified bool + verified, err = s.verifyTrustedKeys(manifest.Name, keys) + if err != nil { + return "", nil, false, fmt.Errorf("error verifying trusted keys: %v", err) + } + + return localDigest, &manifest, verified, nil +} + +// unpackSignedManifest takes the raw, signed manifest bytes, unpacks the jws +// and returns the payload and public keys used to signed the manifest. +// Signatures are verified for authenticity but not against the trust store. +func unpackSignedManifest(p []byte) ([]byte, []libtrust.PublicKey, error) { + sig, err := libtrust.ParsePrettySignature(p, "signatures") + if err != nil { + return nil, nil, fmt.Errorf("error parsing payload: %s", err) + } + + keys, err := sig.Verify() + if err != nil { + return nil, nil, fmt.Errorf("error verifying payload: %s", err) + } + + payload, err := sig.Payload() + if err != nil { + return nil, nil, fmt.Errorf("error retrieving payload: %s", err) + } + + return payload, keys, nil +} + +// verifyTrustedKeys checks the keys provided against the trust store, +// ensuring that the provided keys are trusted for the namespace. The keys +// provided from this method must come from the signatures provided as part of +// the manifest JWS package, obtained from unpackSignedManifest or libtrust. +func (s *TagStore) verifyTrustedKeys(namespace string, keys []libtrust.PublicKey) (verified bool, err error) { + if namespace[0] != '/' { + namespace = "/" + namespace + } + for _, key := range keys { - namespace := manifest.Name - if namespace[0] != '/' { - namespace = "/" + namespace - } b, err := key.MarshalJSON() if err != nil { - return "", nil, false, fmt.Errorf("error marshalling public key: %s", err) + return 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 false, fmt.Errorf("error running key check: %s", err) } logrus.Debugf("Key check result: %v", vErr) } verified = v - if verified { - logrus.Debug("Key check result: verified") - } } - return localDigest, &manifest, verified, nil + + if verified { + logrus.Debug("Key check result: verified") + } + + return +} + +// verifyDigest checks the contents of p against the provided digest. Note +// that for manifests, this is the signed payload and not the raw bytes with +// signatures. +func verifyDigest(dgst digest.Digest, p []byte) error { + if err := dgst.Validate(); err != nil { + return fmt.Errorf("error validating digest %q: %v", dgst, err) + } + + verifier, err := digest.NewDigestVerifier(dgst) + 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 fmt.Errorf("error creating verifier for digest %q: %v", dgst, err) + } + + if _, err := verifier.Write(p); err != nil { + return fmt.Errorf("error writing payload to digest verifier (verifier target %q): %v", dgst, err) + } + + if !verifier.Verified() { + return fmt.Errorf("verification against digest %q failed", dgst) + } + + return nil } func validateManifest(manifest *registry.ManifestData) error { + if manifest.SchemaVersion != 1 { + return fmt.Errorf("unsupported schema version: %d", manifest.SchemaVersion) + } + if len(manifest.FSLayers) != len(manifest.History) { return fmt.Errorf("length of history not equal to number of layers") }