From 30e0c42e901aac75bf2c5d7f26cea8cfd4bfdb0f Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 9 Feb 2016 11:38:37 -0800 Subject: [PATCH 1/2] Verify layer tarstream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds verification for getting layer data out of layerstore. These failures should only be possible if layer metadata files have been manually changed of if something is wrong with tar-split algorithm. Failing early makes sure we don’t upload invalid data to the registries where it would fail after someone tries to pull it. Signed-off-by: Tonis Tiigi (cherry picked from commit e29e580f7fe628e936925681a4885d0b655bb151) Upstream-commit: 50a498ea1c49cc3caa81bb9fb1417de387117e89 Component: engine --- components/engine/layer/layer_test.go | 81 ++++++++++++++++++++-- components/engine/layer/layer_unix_test.go | 2 +- components/engine/layer/migration_test.go | 2 +- components/engine/layer/mount_test.go | 6 +- components/engine/layer/ro_layer.go | 49 ++++++++++++- 5 files changed, 126 insertions(+), 14 deletions(-) diff --git a/components/engine/layer/layer_test.go b/components/engine/layer/layer_test.go index b17c35c7bf..c8e9c28cf7 100644 --- a/components/engine/layer/layer_test.go +++ b/components/engine/layer/layer_test.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" "github.com/docker/distribution/digest" @@ -56,7 +57,7 @@ func newTestGraphDriver(t *testing.T) (graphdriver.Driver, func()) { } } -func newTestStore(t *testing.T) (Store, func()) { +func newTestStore(t *testing.T) (Store, string, func()) { td, err := ioutil.TempDir("", "layerstore-") if err != nil { t.Fatal(err) @@ -72,7 +73,7 @@ func newTestStore(t *testing.T) (Store, func()) { t.Fatal(err) } - return ls, func() { + return ls, td, func() { graphcleanup() os.RemoveAll(td) } @@ -265,7 +266,7 @@ func assertLayerEqual(t *testing.T, l1, l2 Layer) { } func TestMountAndRegister(t *testing.T) { - ls, cleanup := newTestStore(t) + ls, _, cleanup := newTestStore(t) defer cleanup() li := initWithFiles(newTestFile("testfile.txt", []byte("some test data"), 0644)) @@ -306,7 +307,7 @@ func TestMountAndRegister(t *testing.T) { } func TestLayerRelease(t *testing.T) { - ls, cleanup := newTestStore(t) + ls, _, cleanup := newTestStore(t) defer cleanup() layer1, err := createLayer(ls, "", initWithFiles(newTestFile("layer1.txt", []byte("layer 1 file"), 0644))) @@ -351,7 +352,7 @@ func TestLayerRelease(t *testing.T) { } func TestStoreRestore(t *testing.T) { - ls, cleanup := newTestStore(t) + ls, _, cleanup := newTestStore(t) defer cleanup() layer1, err := createLayer(ls, "", initWithFiles(newTestFile("layer1.txt", []byte("layer 1 file"), 0644))) @@ -472,7 +473,7 @@ func TestStoreRestore(t *testing.T) { } func TestTarStreamStability(t *testing.T) { - ls, cleanup := newTestStore(t) + ls, _, cleanup := newTestStore(t) defer cleanup() files1 := []FileApplier{ @@ -668,7 +669,7 @@ func assertActivityCount(t *testing.T, l RWLayer, expected int) { } func TestRegisterExistingLayer(t *testing.T) { - ls, cleanup := newTestStore(t) + ls, _, cleanup := newTestStore(t) defer cleanup() baseFiles := []FileApplier{ @@ -702,3 +703,69 @@ func TestRegisterExistingLayer(t *testing.T) { assertReferences(t, layer2a, layer2b) } + +func TestTarStreamVerification(t *testing.T) { + ls, tmpdir, cleanup := newTestStore(t) + defer cleanup() + + files1 := []FileApplier{ + newTestFile("/foo", []byte("abc"), 0644), + newTestFile("/bar", []byte("def"), 0644), + } + files2 := []FileApplier{ + newTestFile("/foo", []byte("abc"), 0644), + newTestFile("/bar", []byte("def"), 0600), // different perm + } + + tar1, err := tarFromFiles(files1...) + if err != nil { + t.Fatal(err) + } + + tar2, err := tarFromFiles(files2...) + if err != nil { + t.Fatal(err) + } + + layer1, err := ls.Register(bytes.NewReader(tar1), "") + if err != nil { + t.Fatal(err) + } + + layer2, err := ls.Register(bytes.NewReader(tar2), "") + if err != nil { + t.Fatal(err) + } + id1 := digest.Digest(layer1.ChainID()) + id2 := digest.Digest(layer2.ChainID()) + + // Replace tar data files + src, err := os.Open(filepath.Join(tmpdir, id1.Algorithm().String(), id1.Hex(), "tar-split.json.gz")) + if err != nil { + t.Fatal(err) + } + + dst, err := os.Create(filepath.Join(tmpdir, id2.Algorithm().String(), id2.Hex(), "tar-split.json.gz")) + if err != nil { + t.Fatal(err) + } + + if _, err := io.Copy(dst, src); err != nil { + t.Fatal(err) + } + + src.Close() + dst.Close() + + ts, err := layer2.TarStream() + if err != nil { + t.Fatal(err) + } + _, err = io.Copy(ioutil.Discard, ts) + if err == nil { + t.Fatal("expected data verification to fail") + } + if !strings.Contains(err.Error(), "could not verify layer data") { + t.Fatalf("wrong error returned from tarstream: %q", err) + } +} diff --git a/components/engine/layer/layer_unix_test.go b/components/engine/layer/layer_unix_test.go index 75373411ea..9aa1afd597 100644 --- a/components/engine/layer/layer_unix_test.go +++ b/components/engine/layer/layer_unix_test.go @@ -16,7 +16,7 @@ func graphDiffSize(ls Store, l Layer) (int64, error) { // Unix as Windows graph driver does not support Changes which is indirectly // invoked by calling DiffSize on the driver func TestLayerSize(t *testing.T) { - ls, cleanup := newTestStore(t) + ls, _, cleanup := newTestStore(t) defer cleanup() content1 := []byte("Base contents") diff --git a/components/engine/layer/migration_test.go b/components/engine/layer/migration_test.go index cd7ffc6e06..df0f869bbf 100644 --- a/components/engine/layer/migration_test.go +++ b/components/engine/layer/migration_test.go @@ -268,7 +268,7 @@ func TestLayerMigrationNoTarsplit(t *testing.T) { } func TestMountMigration(t *testing.T) { - ls, cleanup := newTestStore(t) + ls, _, cleanup := newTestStore(t) defer cleanup() baseFiles := []FileApplier{ diff --git a/components/engine/layer/mount_test.go b/components/engine/layer/mount_test.go index 6889912e6d..a1e86ae95d 100644 --- a/components/engine/layer/mount_test.go +++ b/components/engine/layer/mount_test.go @@ -11,7 +11,7 @@ import ( ) func TestMountInit(t *testing.T) { - ls, cleanup := newTestStore(t) + ls, _, cleanup := newTestStore(t) defer cleanup() basefile := newTestFile("testfile.txt", []byte("base data!"), 0644) @@ -63,7 +63,7 @@ func TestMountInit(t *testing.T) { } func TestMountSize(t *testing.T) { - ls, cleanup := newTestStore(t) + ls, _, cleanup := newTestStore(t) defer cleanup() content1 := []byte("Base contents") @@ -105,7 +105,7 @@ func TestMountSize(t *testing.T) { } func TestMountChanges(t *testing.T) { - ls, cleanup := newTestStore(t) + ls, _, cleanup := newTestStore(t) defer cleanup() basefiles := []FileApplier{ diff --git a/components/engine/layer/ro_layer.go b/components/engine/layer/ro_layer.go index 51e0921dd1..92b0ea0ee2 100644 --- a/components/engine/layer/ro_layer.go +++ b/components/engine/layer/ro_layer.go @@ -1,6 +1,11 @@ package layer -import "io" +import ( + "fmt" + "io" + + "github.com/docker/distribution/digest" +) type roLayer struct { chainID ChainID @@ -29,7 +34,11 @@ func (rl *roLayer) TarStream() (io.ReadCloser, error) { pw.Close() } }() - return pr, nil + rc, err := newVerifiedReadCloser(pr, digest.Digest(rl.diffID)) + if err != nil { + return nil, err + } + return rc, nil } func (rl *roLayer) ChainID() ChainID { @@ -117,3 +126,39 @@ func storeLayer(tx MetadataTransaction, layer *roLayer) error { return nil } + +func newVerifiedReadCloser(rc io.ReadCloser, dgst digest.Digest) (io.ReadCloser, error) { + verifier, err := digest.NewDigestVerifier(dgst) + if err != nil { + return nil, err + } + return &verifiedReadCloser{ + rc: rc, + dgst: dgst, + verifier: verifier, + }, nil +} + +type verifiedReadCloser struct { + rc io.ReadCloser + dgst digest.Digest + verifier digest.Verifier +} + +func (vrc *verifiedReadCloser) Read(p []byte) (n int, err error) { + n, err = vrc.rc.Read(p) + if n > 0 { + if n, err := vrc.verifier.Write(p[:n]); err != nil { + return n, err + } + } + if err == io.EOF { + if !vrc.verifier.Verified() { + err = fmt.Errorf("could not verify layer data for: %s. This may be because internal files in the layer store were modified. Re-pulling or rebuilding this image may resolve the issue", vrc.dgst) + } + } + return +} +func (vrc *verifiedReadCloser) Close() error { + return vrc.rc.Close() +} From ea1de57ceb7ae08c25d8343a4fc870acef0c9847 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 9 Feb 2016 09:44:33 -0800 Subject: [PATCH 2/2] =?UTF-8?q?Don=E2=80=99t=20stop=20daemon=20on=20migrat?= =?UTF-8?q?ion=20hard=20failure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also changes missing storage layer for container RWLayer to a soft failure. Fixes #20147 Signed-off-by: Tonis Tiigi (cherry picked from commit 2798d7a6a681aee8995e87c9b68128e54876d2b5) Upstream-commit: 55080fc03bed4dba85d5c5760363b92851ff728f Component: engine --- components/engine/daemon/daemon.go | 2 +- components/engine/layer/migration.go | 2 +- components/engine/migrate/v1/migratev1.go | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/components/engine/daemon/daemon.go b/components/engine/daemon/daemon.go index 33c3f10caf..dfe6f0aeb6 100644 --- a/components/engine/daemon/daemon.go +++ b/components/engine/daemon/daemon.go @@ -760,7 +760,7 @@ func NewDaemon(config *Config, registryService *registry.Service) (daemon *Daemo migrationStart := time.Now() if err := v1.Migrate(config.Root, graphDriver, d.layerStore, d.imageStore, referenceStore, distributionMetadataStore); err != nil { - return nil, err + logrus.Errorf("Graph migration failed: %q. Your old graph data was found to be too inconsistent for upgrading to content-addressable storage. Some of the old data was probably not upgraded. We recommend starting over with a clean storage directory if possible.", err) } logrus.Infof("Graph migration to content-addressability took %.2f seconds", time.Since(migrationStart).Seconds()) diff --git a/components/engine/layer/migration.go b/components/engine/layer/migration.go index 9779ab7984..ac0f0065f2 100644 --- a/components/engine/layer/migration.go +++ b/components/engine/layer/migration.go @@ -32,7 +32,7 @@ func (ls *layerStore) CreateRWLayerByGraphID(name string, graphID string, parent } if !ls.driver.Exists(graphID) { - return errors.New("graph ID does not exist") + return fmt.Errorf("graph ID does not exist: %q", graphID) } var p *roLayer diff --git a/components/engine/migrate/v1/migratev1.go b/components/engine/migrate/v1/migratev1.go index 9243c5a42a..b7ce75b1c0 100644 --- a/components/engine/migrate/v1/migratev1.go +++ b/components/engine/migrate/v1/migratev1.go @@ -282,7 +282,8 @@ func migrateContainers(root string, ls graphIDMounter, is image.Store, imageMapp } if err := ls.CreateRWLayerByGraphID(id, id, img.RootFS.ChainID()); err != nil { - return err + logrus.Errorf("migrate container error: %v", err) + continue } logrus.Infof("migrated container %s to point to %s", id, imageID)