From e087b47e98d4768085d3ee5c9ceb1aaa7038ec72 Mon Sep 17 00:00:00 2001
From: Claudiu Belu <cbelu@cloudbasesolutions.com>
Date: Sat, 28 Aug 2021 16:02:06 -0700
Subject: [PATCH] import: Raise error if the imported image is filtered out

During import, if an image does not match the host's platform,
it won't have any children labels set, which will result in the
Garbage Collector deleting its content later, resulting in an
unusable image. In this case, we should fail early.

This can still be bypassed by using ctr import --all-platforms.

Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>
---
 images/handlers.go                | 34 +++++++++++++++++++++++++++++++
 import.go                         |  2 +-
 integration/client/import_test.go | 21 +++++++++++++++----
 3 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/images/handlers.go b/images/handlers.go
index 05a9017bc..a228aad5b 100644
--- a/images/handlers.go
+++ b/images/handlers.go
@@ -40,6 +40,10 @@ var (
 	// This applies only to a single descriptor in a handler
 	// chain and does not apply to descendant descriptors.
 	ErrStopHandler = fmt.Errorf("stop handler")
+
+	// ErrEmptyWalk is used when the WalkNotEmpty handlers return no
+	// children (e.g.: they were filtered out).
+	ErrEmptyWalk = fmt.Errorf("image might be filtered out")
 )
 
 // Handler handles image manifests
@@ -99,6 +103,36 @@ func Walk(ctx context.Context, handler Handler, descs ...ocispec.Descriptor) err
 			}
 		}
 	}
+	return nil
+}
+
+// WalkNotEmpty works the same way Walk does, with the exception that it ensures that
+// some children are still found by Walking the descriptors (for example, not all of
+// them have been filtered out by one of the handlers). If there are no children,
+// then an ErrEmptyWalk error is returned.
+func WalkNotEmpty(ctx context.Context, handler Handler, descs ...ocispec.Descriptor) error {
+	isEmpty := true
+	var notEmptyHandler HandlerFunc = func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
+		children, err := handler.Handle(ctx, desc)
+		if err != nil {
+			return children, err
+		}
+
+		if len(children) > 0 {
+			isEmpty = false
+		}
+
+		return children, nil
+	}
+
+	err := Walk(ctx, notEmptyHandler, descs...)
+	if err != nil {
+		return err
+	}
+
+	if isEmpty {
+		return ErrEmptyWalk
+	}
 
 	return nil
 }
diff --git a/import.go b/import.go
index 2ba717448..e2288c4f1 100644
--- a/import.go
+++ b/import.go
@@ -185,7 +185,7 @@ func (c *Client) Import(ctx context.Context, reader io.Reader, opts ...ImportOpt
 
 	handler = images.FilterPlatforms(handler, platformMatcher)
 	handler = images.SetChildrenLabels(cs, handler)
-	if err := images.Walk(ctx, handler, index); err != nil {
+	if err := images.WalkNotEmpty(ctx, handler, index); err != nil {
 		return nil, err
 	}
 
diff --git a/integration/client/import_test.go b/integration/client/import_test.go
index 22d5603a7..ed81d30b4 100644
--- a/integration/client/import_test.go
+++ b/integration/client/import_test.go
@@ -25,6 +25,7 @@ import (
 	"io/ioutil"
 	"math/rand"
 	"reflect"
+	"runtime"
 	"testing"
 
 	. "github.com/containerd/containerd"
@@ -98,7 +99,8 @@ func TestImport(t *testing.T) {
 	empty := []byte("{}")
 	version := []byte("1.0")
 
-	c1, d2 := createConfig()
+	c1, d2 := createConfig(runtime.GOOS, runtime.GOARCH)
+	badConfig, _ := createConfig("foo", "lish")
 
 	m1, d3, expManifest := createManifest(c1, [][]byte{b1})
 
@@ -172,6 +174,17 @@ func TestImport(t *testing.T) {
 				checkManifest(ctx, t, imgs[0].Target, nil)
 			},
 		},
+		{
+			Name: "DockerV2.1-BadOSArch",
+			Writer: tartest.TarAll(
+				tc.Dir("bd765cd43e95212f7aa2cab51d0a", 0755),
+				tc.File("bd765cd43e95212f7aa2cab51d0a/json", empty, 0644),
+				tc.File("bd765cd43e95212f7aa2cab51d0a/layer.tar", b1, 0644),
+				tc.File("bd765cd43e95212f7aa2cab51d0a/VERSION", version, 0644),
+				tc.File("e95212f7aa2cab51d0abd765cd43.json", badConfig, 0644),
+				tc.File("manifest.json", []byte(`[{"Config":"e95212f7aa2cab51d0abd765cd43.json","RepoTags":["test-import:notlatest", "another/repo:tag"],"Layers":["bd765cd43e95212f7aa2cab51d0a/layer.tar"]}]`), 0644),
+			),
+		},
 		{
 			Name: "OCI-BadFormat",
 			Writer: tartest.TarAll(
@@ -302,10 +315,10 @@ func createContent(size int64, seed int64) ([]byte, digest.Digest) {
 	return b, digest.FromBytes(b)
 }
 
-func createConfig() ([]byte, digest.Digest) {
+func createConfig(osName, archName string) ([]byte, digest.Digest) {
 	image := ocispec.Image{
-		OS:           "any",
-		Architecture: "any",
+		OS:           osName,
+		Architecture: archName,
 		Author:       "test",
 	}
 	b, _ := json.Marshal(image)