From dd32fbe6152f18a6a4593c48af73d2c357fd339d Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Tue, 3 Nov 2015 11:03:17 -0800 Subject: [PATCH] Before allowing a schema1 manifest to be stored in the registry, ensure that it contains equal length History and FSLayer arrays. This is required to prevent malformed manifests being put to the registry and failing external verification checks. Signed-off-by: Richard Scothern --- manifest/schema1/manifest_test.go | 50 ++++++++++++++++++++------ notifications/listener_test.go | 3 ++ registry/handlers/api_test.go | 26 +++++++++++++- registry/storage/manifeststore.go | 5 +++ registry/storage/manifeststore_test.go | 4 +++ 5 files changed, 76 insertions(+), 12 deletions(-) diff --git a/manifest/schema1/manifest_test.go b/manifest/schema1/manifest_test.go index 16cedae31..7d0d382db 100644 --- a/manifest/schema1/manifest_test.go +++ b/manifest/schema1/manifest_test.go @@ -10,10 +10,10 @@ import ( ) type testEnv struct { - name, tag string - manifest *Manifest - signed *SignedManifest - pk libtrust.PrivateKey + name, tag string + invalidSigned *SignedManifest + signed *SignedManifest + pk libtrust.PrivateKey } func TestManifestMarshaling(t *testing.T) { @@ -42,6 +42,7 @@ func TestManifestUnmarshaling(t *testing.T) { if !reflect.DeepEqual(&signed, env.signed) { t.Fatalf("manifests are different after unmarshaling: %v != %v", signed, env.signed) } + } func TestManifestVerification(t *testing.T) { @@ -69,6 +70,12 @@ func TestManifestVerification(t *testing.T) { if !found { t.Fatalf("expected public key, %v, not found in verified keys: %v", publicKey, publicKeys) } + + // Check that an invalid manifest fails verification + _, err = Verify(env.invalidSigned) + if err != nil { + t.Fatalf("Invalid manifest should not pass Verify()") + } } func genEnv(t *testing.T) *testEnv { @@ -79,7 +86,7 @@ func genEnv(t *testing.T) *testEnv { name, tag := "foo/bar", "test" - m := Manifest{ + invalid := Manifest{ Versioned: SchemaVersion, Name: name, Tag: tag, @@ -93,16 +100,37 @@ func genEnv(t *testing.T) *testEnv { }, } - sm, err := Sign(&m, pk) + valid := Manifest{ + Versioned: SchemaVersion, + Name: name, + Tag: tag, + FSLayers: []FSLayer{ + { + BlobSum: "asdf", + }, + }, + History: []History{ + { + V1Compatibility: "", + }, + }, + } + + sm, err := Sign(&valid, pk) + if err != nil { + t.Fatalf("error signing manifest: %v", err) + } + + invalidSigned, err := Sign(&invalid, pk) if err != nil { t.Fatalf("error signing manifest: %v", err) } return &testEnv{ - name: name, - tag: tag, - manifest: &m, - signed: sm, - pk: pk, + name: name, + tag: tag, + invalidSigned: invalidSigned, + signed: sm, + pk: pk, } } diff --git a/notifications/listener_test.go b/notifications/listener_test.go index e4fa79e03..04dae1184 100644 --- a/notifications/listener_test.go +++ b/notifications/listener_test.go @@ -131,6 +131,9 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository) { m.FSLayers = append(m.FSLayers, schema1.FSLayer{ BlobSum: dgst, }) + m.History = append(m.History, schema1.History{ + V1Compatibility: "", + }) // Then fetch the blobs if rc, err := blobs.Open(ctx, dgst); err != nil { diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index c5683dfa3..7f52d13d7 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -804,6 +804,14 @@ func testManifestAPI(t *testing.T, env *testEnv, args manifestArgs) (*testEnv, m BlobSum: "qwer", }, }, + History: []schema1.History{ + { + V1Compatibility: "", + }, + { + V1Compatibility: "", + }, + }, } resp = putManifest(t, "putting unsigned manifest", manifestURL, unsignedManifest) @@ -999,6 +1007,19 @@ func testManifestAPI(t *testing.T, env *testEnv, args manifestArgs) (*testEnv, m t.Fatalf("tag not as expected: %q != %q", tagsResponse.Tags[0], tag) } + // Attempt to put a manifest with mismatching FSLayer and History array cardinalities + + unsignedManifest.History = append(unsignedManifest.History, schema1.History{ + V1Compatibility: "", + }) + invalidSigned, err := schema1.Sign(unsignedManifest, env.pk) + if err != nil { + t.Fatalf("error signing manifest") + } + + resp = putManifest(t, "putting invalid signed manifest", manifestDigestURL, invalidSigned) + checkResponse(t, "putting invalid signed manifest", resp, http.StatusBadRequest) + return env, args } @@ -1432,8 +1453,10 @@ func createRepository(env *testEnv, t *testing.T, imageName string, tag string) { BlobSum: "asdf", }, + }, + History: []schema1.History{ { - BlobSum: "qwer", + V1Compatibility: "", }, }, } @@ -1499,6 +1522,7 @@ func TestRegistryAsCacheMutationAPIs(t *testing.T) { Name: imageName, Tag: tag, FSLayers: []schema1.FSLayer{}, + History: []schema1.History{}, } sm, err := schema1.Sign(m, env.pk) diff --git a/registry/storage/manifeststore.go b/registry/storage/manifeststore.go index db49aaa43..d161fb5a5 100644 --- a/registry/storage/manifeststore.go +++ b/registry/storage/manifeststore.go @@ -110,6 +110,11 @@ func (ms *manifestStore) verifyManifest(ctx context.Context, mnfst *schema1.Sign errs = append(errs, fmt.Errorf("repository name does not match manifest name")) } + if len(mnfst.History) != len(mnfst.FSLayers) { + errs = append(errs, fmt.Errorf("mismatched history and fslayer cardinality %d != %d", + len(mnfst.History), len(mnfst.FSLayers))) + } + if _, err := schema1.Verify(mnfst); err != nil { switch err { case libtrust.ErrMissingSignatureKey, libtrust.ErrInvalidJSONContent, libtrust.ErrMissingSignatureKey: diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index 30126e4bb..51370e173 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -98,6 +98,10 @@ func TestManifestStorage(t *testing.T) { m.FSLayers = append(m.FSLayers, schema1.FSLayer{ BlobSum: dgst, }) + m.History = append(m.History, schema1.History{ + V1Compatibility: "", + }) + } pk, err := libtrust.GenerateECP256PrivateKey()