diff --git a/api_test.go b/api_test.go index cc27e5b0..41f3de69 100644 --- a/api_test.go +++ b/api_test.go @@ -250,6 +250,10 @@ func TestManifestAPI(t *testing.T) { t.Fatalf("should have received two invalid digest errors: %v", respErrs) } + // TODO(stevvooe): Add a test case where we take a mostly valid registry, + // tamper with the content and ensure that we get a unverified manifest + // error. + // Push 2 random layers expectedLayers := make(map[digest.Digest]io.ReadSeeker) @@ -277,7 +281,7 @@ func TestManifestAPI(t *testing.T) { resp = putManifest(t, "putting signed manifest", manifestURL, signedManifest) - checkResponse(t, "putting manifest", resp, http.StatusOK) + checkResponse(t, "putting signed manifest", resp, http.StatusOK) resp, err = http.Get(manifestURL) if err != nil { @@ -299,9 +303,15 @@ func TestManifestAPI(t *testing.T) { } func putManifest(t *testing.T, msg, url string, v interface{}) *http.Response { - body, err := json.Marshal(v) - if err != nil { - t.Fatalf("unexpected error marshaling %v: %v", v, err) + var body []byte + if sm, ok := v.(*storage.SignedManifest); ok { + body = sm.Raw + } else { + var err error + body, err = json.MarshalIndent(v, "", " ") + if err != nil { + t.Fatalf("unexpected error marshaling %v: %v", v, err) + } } req, err := http.NewRequest("PUT", url, bytes.NewReader(body)) diff --git a/storage/manifest.go b/storage/manifest.go index 8b288625..daeaa39b 100644 --- a/storage/manifest.go +++ b/storage/manifest.go @@ -6,6 +6,8 @@ import ( "fmt" "strings" + "github.com/Sirupsen/logrus" + "github.com/docker/libtrust" "github.com/docker/docker-registry/digest" @@ -78,7 +80,7 @@ type Manifest struct { // SignedManifest. This typically won't be used within the registry, except // for testing. func (m *Manifest) Sign(pk libtrust.PrivateKey) (*SignedManifest, error) { - p, err := json.Marshal(m) + p, err := json.MarshalIndent(m, "", " ") if err != nil { return nil, err } @@ -107,7 +109,7 @@ func (m *Manifest) Sign(pk libtrust.PrivateKey) (*SignedManifest, error) { // The public key of the first element in the chain must be the public key // corresponding with the sign key. func (m *Manifest) SignWithChain(key libtrust.PrivateKey, chain []*x509.Certificate) (*SignedManifest, error) { - p, err := json.Marshal(m) + p, err := json.MarshalIndent(m, "", " ") if err != nil { return nil, err } @@ -138,8 +140,9 @@ type SignedManifest struct { Manifest // Raw is the byte representation of the ImageManifest, used for signature - // verification. The manifest byte representation cannot change or it will - // have to be re-signed. + // verification. The value of Raw must be used directly during + // serialization, or the signature check will fail. The manifest byte + // representation cannot change or it will have to be re-signed. Raw []byte `json:"-"` } @@ -148,6 +151,7 @@ type SignedManifest struct { func (sm *SignedManifest) Verify() ([]libtrust.PublicKey, error) { js, err := libtrust.ParsePrettySignature(sm.Raw, "signatures") if err != nil { + logrus.WithField("err", err).Debugf("(*SignedManifest).Verify") return nil, err } @@ -174,13 +178,16 @@ func (sm *SignedManifest) UnmarshalJSON(b []byte) error { } sm.Manifest = manifest - sm.Raw = b + sm.Raw = make([]byte, len(b), len(b)) + copy(sm.Raw, b) return nil } // MarshalJSON returns the contents of raw. If Raw is nil, marshals the inner -// contents. +// contents. Applications requiring a marshaled signed manifest should simply +// use Raw directly, since the the content produced by json.Marshal will +// compacted and will fail signature checks. func (sm *SignedManifest) MarshalJSON() ([]byte, error) { if len(sm.Raw) > 0 { return sm.Raw, nil diff --git a/storage/manifeststore.go b/storage/manifeststore.go index 707311b8..e1760dd8 100644 --- a/storage/manifeststore.go +++ b/storage/manifeststore.go @@ -111,11 +111,13 @@ func (ms *manifestStore) verifyManifest(name, tag string, manifest *SignedManife var errs ErrManifestVerification if manifest.Name != name { - return fmt.Errorf("name does not match manifest name") + // TODO(stevvooe): This needs to be an exported error + errs = append(errs, fmt.Errorf("name does not match manifest name")) } if manifest.Tag != tag { - return fmt.Errorf("tag does not match manifest tag") + // TODO(stevvooe): This needs to be an exported error. + errs = append(errs, fmt.Errorf("tag does not match manifest tag")) } // TODO(stevvooe): These pubkeys need to be checked with either Verify or @@ -127,7 +129,11 @@ func (ms *manifestStore) verifyManifest(name, tag string, manifest *SignedManife case libtrust.ErrMissingSignatureKey, libtrust.ErrInvalidJSONContent, libtrust.ErrMissingSignatureKey: errs = append(errs, ErrManifestUnverified{}) default: - errs = append(errs, err) + if err.Error() == "invalid signature" { // TODO(stevvooe): This should be exported by libtrust + errs = append(errs, ErrManifestUnverified{}) + } else { + errs = append(errs, err) + } } }