Add option to --skip-layer-validation (OCI images)

Callisto13 · Callisto13 · commit 6e24e9a64cec · 2017-08-18T15:06:45.000Z
[finishes #149849148]
diff --git a/commands/config/builder.go b/commands/config/builder.go
@@ -27,6 +27,7 @@ type Config struct {
 
 type Create struct {
 	ExcludeImageFromQuota bool     `yaml:"exclude_image_from_quota"`
+	SkipLayerValidation   bool     `yaml:"skip_layer_validation"`
 	WithClean             bool     `yaml:"with_clean"`
 	WithoutMount          bool     `yaml:"without_mount"`
 	DiskLimitSizeBytes    int64    `yaml:"disk_limit_size_bytes"`
@@ -175,6 +176,13 @@ func (b *Builder) WithExcludeImageFromQuota(exclude, isSet bool) *Builder {
 	return b
 }
 
+func (b *Builder) WithSkipLayerValidation(skip, isSet bool) *Builder {
+	if isSet {
+		b.config.Create.SkipLayerValidation = skip
+	}
+	return b
+}
+
 func (b *Builder) WithCleanThresholdBytes(threshold int64, isSet bool) *Builder {
 	if isSet {
 		b.config.Clean.ThresholdBytes = threshold
diff --git a/commands/config/builder_test.go b/commands/config/builder_test.go
@@ -27,6 +27,7 @@ var _ = Describe("Builder", func() {
 			WithClean:             false,
 			WithoutMount:          false,
 			ExcludeImageFromQuota: true,
+			SkipLayerValidation:   true,
 			InsecureRegistries:    []string{"http://example.org"},
 			DiskLimitSizeBytes:    int64(1000),
 		}
@@ -463,6 +464,24 @@ var _ = Describe("Builder", func() {
 		})
 	})
 
+	Describe("WithSkipLayerValidation", func() {
+		It("overrides the config's SkipLayerValidation when the flag is set", func() {
+			builder = builder.WithSkipLayerValidation(false, true)
+			config, err := builder.Build()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(config.Create.SkipLayerValidation).To(BeFalse())
+		})
+
+		Context("when flag is not set", func() {
+			It("uses the config entry", func() {
+				builder = builder.WithSkipLayerValidation(false, false)
+				config, err := builder.Build()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(config.Create.SkipLayerValidation).To(BeTrue())
+			})
+		})
+	})
+
 	Describe("WithCleanThresholdBytes", func() {
 		It("overrides the config's CleanThresholdBytes entry when the flag is set", func() {
 			builder = builder.WithCleanThresholdBytes(1024, true)
diff --git a/commands/create.go b/commands/create.go
@@ -51,6 +51,10 @@ var CreateCommand = cli.Command{
 			Name:  "exclude-image-from-quota",
 			Usage: "Set disk limit to be exclusive (i.e.: excluding image layers)",
 		},
+		cli.BoolFlag{
+			Name:  "skip-layer-validation",
+			Usage: "Do not validate checksums of image layers. (Can only be used with oci:/// protocol images.)",
+		},
 		cli.BoolFlag{
 			Name:  "with-clean",
 			Usage: "Clean up unused layers before creating rootfs",
@@ -93,6 +97,8 @@ var CreateCommand = cli.Command{
 				ctx.IsSet("disk-limit-size-bytes")).
 			WithExcludeImageFromQuota(ctx.Bool("exclude-image-from-quota"),
 				ctx.IsSet("exclude-image-from-quota")).
+			WithSkipLayerValidation(ctx.Bool("skip-layer-validation"),
+				ctx.IsSet("skip-layer-validation")).
 			WithClean(ctx.IsSet("with-clean"), ctx.IsSet("without-clean")).
 			WithMount(ctx.IsSet("with-mount"), ctx.IsSet("without-mount"))
 
@@ -150,7 +156,7 @@ var CreateCommand = cli.Command{
 			unpacker = unpackerpkg.NewNSIdMapperUnpacker(runner, idMapper, unpackerStrategy)
 		}
 
-		layerSource := source.NewLayerSource(ctx.String("username"), ctx.String("password"), cfg.Create.InsecureRegistries)
+		layerSource := source.NewLayerSource(ctx.String("username"), ctx.String("password"), cfg.Create.InsecureRegistries, cfg.Create.SkipLayerValidation)
 
 		layerFetcher := layer_fetcher.NewLayerFetcher(&layerSource)
 		tarFetcher := tar_fetcher.NewTarFetcher()
diff --git a/fetcher/layer_fetcher/layer_fetcher.go b/fetcher/layer_fetcher/layer_fetcher.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/containers/image/types"
 	specsv1 "github.com/opencontainers/image-spec/specs-go/v1"
+	errorspkg "github.com/pkg/errors"
 )
 
 const cfBaseDirectoryAnnotation = "org.cloudfoundry.image.base-directory"
@@ -80,7 +81,7 @@ func (f *LayerFetcher) StreamBlob(logger lager.Logger, baseImageURL *url.URL, so
 	blobReader, err := NewBlobReader(blobFilePath)
 	if err != nil {
 		logger.Error("blob-reader-failed", err)
-		return nil, 0, err
+		return nil, 0, errorspkg.Wrap(err, "opening stream from temporary blob file")
 	}
 
 	return blobReader, size, nil
diff --git a/fetcher/layer_fetcher/source/layer_source.go b/fetcher/layer_fetcher/source/layer_source.go
@@ -26,16 +26,18 @@ import (
 const MAX_BLOB_RETRIES = 3
 
 type LayerSource struct {
-	trustedRegistries []string
-	username          string
-	password          string
+	trustedRegistries      []string
+	username               string
+	password               string
+	skipChecksumValidation bool
 }
 
-func NewLayerSource(username, password string, trustedRegistries []string) LayerSource {
+func NewLayerSource(username, password string, trustedRegistries []string, skipChecksumValidation bool) LayerSource {
 	return LayerSource{
-		username:          username,
-		password:          password,
-		trustedRegistries: trustedRegistries,
+		username:               username,
+		password:               password,
+		trustedRegistries:      trustedRegistries,
+		skipChecksumValidation: skipChecksumValidation,
 	}
 }
 
@@ -82,9 +84,11 @@ func (s *LayerSource) Blob(logger lager.Logger, baseImageURL *url.URL, digest st
 	defer func() { _ = blobTempFile.Close() }()
 
 	blobReader := io.TeeReader(blob, blobTempFile)
-	if !s.checkCheckSum(logger, blobReader, digest) {
+
+	if !s.checkCheckSum(logger, blobReader, digest, baseImageURL) {
 		return "", 0, errorspkg.Errorf("invalid checksum: layer is corrupted `%s`", digest)
 	}
+
 	return blobTempFile.Name(), size, nil
 }
 
@@ -102,7 +106,7 @@ func (s *LayerSource) getBlobWithRetries(imgSrc types.ImageSource, blobInfo type
 	return nil, 0, err
 }
 
-func (s *LayerSource) checkCheckSum(logger lager.Logger, data io.Reader, digest string) bool {
+func (s *LayerSource) checkCheckSum(logger lager.Logger, data io.Reader, digest string, baseImageURL *url.URL) bool {
 	var (
 		actualSize int64
 		err        error
@@ -122,7 +126,7 @@ func (s *LayerSource) checkCheckSum(logger lager.Logger, data io.Reader, digest
 		"downloadedChecksum": blobContentsSha,
 	})
 
-	return digestID == blobContentsSha
+	return s.skipLayerCheckSumValidation(baseImageURL.Scheme) || digestID == blobContentsSha
 }
 
 func (s *LayerSource) skipTLSValidation(baseImageURL *url.URL) bool {
@@ -262,3 +266,11 @@ func preferedMediaTypes() []string {
 		manifestpkg.DockerV2Schema2MediaType,
 	}
 }
+
+func (s *LayerSource) skipLayerCheckSumValidation(scheme string) bool {
+	if s.skipChecksumValidation && scheme == "oci" {
+		return true
+	}
+
+	return false
+}
diff --git a/fetcher/layer_fetcher/source/layer_source_docker_test.go b/fetcher/layer_fetcher/source/layer_source_docker_test.go
@@ -30,10 +30,13 @@ var _ = Describe("Layer source: Docker", func() {
 		configBlob            string
 		expectedLayersDigests []types.BlobInfo
 		expectedDiffIds       []digestpkg.Digest
+
+		skipChecksumValidation bool
 	)
 
 	BeforeEach(func() {
 		trustedRegistries = []string{}
+		skipChecksumValidation = false
 
 		configBlob = "sha256:217f3b4afdf698d639f854d9c6d640903a011413bc7e7bffeabe63c7ca7e4a7d"
 		expectedLayersDigests = []types.BlobInfo{
@@ -58,7 +61,7 @@ var _ = Describe("Layer source: Docker", func() {
 	})
 
 	JustBeforeEach(func() {
-		layerSource = source.NewLayerSource("", "", trustedRegistries)
+		layerSource = source.NewLayerSource("", "", trustedRegistries, skipChecksumValidation)
 	})
 
 	Describe("Manifest", func() {
@@ -106,7 +109,7 @@ var _ = Describe("Layer source: Docker", func() {
 
 			Context("when the correct credentials are provided", func() {
 				JustBeforeEach(func() {
-					layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries)
+					layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries, skipChecksumValidation)
 				})
 
 				It("fetches the manifest", func() {
@@ -186,7 +189,7 @@ var _ = Describe("Layer source: Docker", func() {
 			})
 
 			JustBeforeEach(func() {
-				layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries)
+				layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries, skipChecksumValidation)
 				var err error
 				manifest, err = layerSource.Manifest(logger, baseImageURL)
 				Expect(err).NotTo(HaveOccurred())
@@ -351,7 +354,7 @@ var _ = Describe("Layer source: Docker", func() {
 				})
 
 				JustBeforeEach(func() {
-					layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries)
+					layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries, skipChecksumValidation)
 				})
 
 				It("fetches the manifest", func() {
@@ -427,7 +430,7 @@ var _ = Describe("Layer source: Docker", func() {
 
 			Context("when the correct credentials are provided", func() {
 				JustBeforeEach(func() {
-					layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries)
+					layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries, skipChecksumValidation)
 				})
 
 				It("fetches the config", func() {
@@ -500,6 +503,17 @@ var _ = Describe("Layer source: Docker", func() {
 				_, _, err := layerSource.Blob(logger, baseImageURL, expectedLayersDigests[1].Digest.String())
 				Expect(err).To(MatchError(ContainSubstring("invalid checksum: layer is corrupted")))
 			})
+
+			Context("when a devious hacker tries to set skipChecksumValidation to true", func() {
+				BeforeEach(func() {
+					skipChecksumValidation = true
+				})
+
+				It("returns an error", func() {
+					_, _, err := layerSource.Blob(logger, baseImageURL, expectedLayersDigests[1].Digest.String())
+					Expect(err).To(MatchError(ContainSubstring("invalid checksum: layer is corrupted")))
+				})
+			})
 		})
 	})
 })
diff --git a/fetcher/layer_fetcher/source/layer_source_oci_test.go b/fetcher/layer_fetcher/source/layer_source_oci_test.go
@@ -27,12 +27,14 @@ var _ = Describe("Layer source: OCI", func() {
 		configBlob           string
 		expectedLayersDigest []types.BlobInfo
 		expectedDiffIds      []digestpkg.Digest
-		// manifest             layer_fetcher.Manifest
-		workDir string
+		workDir              string
+
+		skipChecksumValidation bool
 	)
 
 	BeforeEach(func() {
 		trustedRegistries = []string{}
+		skipChecksumValidation = false
 
 		configBlob = "sha256:10c8f0eb9d1af08fe6e3b8dbd29e5aa2b6ecfa491ecd04ed90de19a4ac22de7b"
 		expectedLayersDigest = []types.BlobInfo{
@@ -59,7 +61,7 @@ var _ = Describe("Layer source: OCI", func() {
 	})
 
 	JustBeforeEach(func() {
-		layerSource = source.NewLayerSource("", "", trustedRegistries)
+		layerSource = source.NewLayerSource("", "", trustedRegistries, skipChecksumValidation)
 	})
 
 	Describe("Manifest", func() {
@@ -169,5 +171,19 @@ var _ = Describe("Layer source: OCI", func() {
 				Expect(err).To(MatchError(ContainSubstring("invalid checksum: layer is corrupted")))
 			})
 		})
+
+		Context("when skipChecksumValidation is set to true", func() {
+			BeforeEach(func() {
+				var err error
+				baseImageURL, err = url.Parse(fmt.Sprintf("oci:///%s/../../../integration/assets/oci-test-image/corrupted:latest", workDir))
+				Expect(err).NotTo(HaveOccurred())
+				skipChecksumValidation = true
+			})
+
+			It("does not validate against checksums and does not return an error", func() {
+				_, _, err := layerSource.Blob(logger, baseImageURL, expectedLayersDigest[0].Digest.String())
+				Expect(err).NotTo(HaveOccurred())
+			})
+		})
 	})
 })
diff --git a/integration/create_docker_test.go b/integration/create_docker_test.go
@@ -762,4 +762,41 @@ var _ = Describe("Create with remote DOCKER images", func() {
 			})
 		})
 	})
+
+	Context("when --skip-layer-validation flag is passed", func() {
+		var (
+			fakeRegistry  *testhelpers.FakeRegistry
+			corruptedBlob string
+		)
+
+		AfterEach(func() {
+			fakeRegistry.Stop()
+		})
+
+		BeforeEach(func() {
+			dockerHubUrl, err := url.Parse("https://registry-1.docker.io")
+			Expect(err).NotTo(HaveOccurred())
+			fakeRegistry = testhelpers.NewFakeRegistry(dockerHubUrl)
+			corruptedBlob = testhelpers.EmptyBaseImageV011.Layers[1].BlobID
+			fakeRegistry.WhenGettingBlob(corruptedBlob, 0, func(w http.ResponseWriter, r *http.Request) {
+				_, err := w.Write([]byte("bad-blob"))
+				Expect(err).NotTo(HaveOccurred())
+			})
+			fakeRegistry.Start()
+			baseImageURL = fmt.Sprintf("docker://%s/cfgarden/empty:v0.1.1", fakeRegistry.Addr())
+		})
+
+		It("has no effect", func() {
+			runner := Runner.WithInsecureRegistry(fakeRegistry.Addr())
+
+			_, err := runner.SkipLayerCheckSumValidation().Create(groot.CreateSpec{
+				BaseImage: baseImageURL,
+				ID:        "random-id",
+				Mount:     true,
+			})
+
+			Expect(err).To(HaveOccurred())
+			Expect(err).To(MatchError(ContainSubstring("layer is corrupted")))
+		})
+	})
 })
diff --git a/integration/create_oci_test.go b/integration/create_oci_test.go
@@ -395,4 +395,17 @@ var _ = Describe("Create with OCI images", func() {
 			})
 		})
 	})
+
+	Context("when --skip-layer-validation flag is passed", func() {
+		It("does not validate the checksums for oci image layers", func() {
+			image, err := Runner.SkipLayerCheckSumValidation().Create(groot.CreateSpec{
+				BaseImage: fmt.Sprintf("oci:///%s/assets/oci-test-image/corrupted:latest", workDir),
+				ID:        "random-id",
+				Mount:     true,
+			})
+
+			Expect(err).NotTo(HaveOccurred())
+			Expect(filepath.Join(image.Rootfs, "corrupted")).To(BeARegularFile())
+		})
+	})
 })
diff --git a/integration/runner/create.go b/integration/runner/create.go
@@ -113,6 +113,10 @@ func (r Runner) makeCreateArgs(spec groot.CreateSpec) []string {
 		args = append(args, "--password", r.RegistryPassword)
 	}
 
+	if r.SkipLayerValidation {
+		args = append(args, "--skip-layer-validation")
+	}
+
 	if spec.DiskLimit != 0 {
 		args = append(args, "--disk-limit-size-bytes",
 			strconv.FormatInt(spec.DiskLimit, 10),
diff --git a/integration/runner/options.go b/integration/runner/options.go
@@ -218,3 +218,12 @@ func (r Runner) RunningAsUser(uid, gid int) Runner {
 	}
 	return r
 }
+
+///////////////////////////////////////////////////////////////////////////////
+// OCI Checksum Validation
+///////////////////////////////////////////////////////////////////////////////
+
+func (r Runner) SkipLayerCheckSumValidation() Runner {
+	r.SkipLayerValidation = true
+	return r
+}
diff --git a/integration/runner/runner.go b/integration/runner/runner.go
@@ -49,6 +49,8 @@ type Runner struct {
 	// Clean on Create
 	CleanOnCreate   bool
 	NoCleanOnCreate bool
+	// Layer Checksum Validation
+	SkipLayerValidation bool
 
 	SysCredential syscall.Credential
 

Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,7 @@ import (`
`13`	`13`
`14`	`14`	`"github.com/containers/image/types"`
`15`	`15`	`specsv1 "github.com/opencontainers/image-spec/specs-go/v1"`
	`16`	`+ errorspkg "github.com/pkg/errors"`
`16`	`17`	`)`
`17`	`18`
`18`	`19`	`const cfBaseDirectoryAnnotation = "org.cloudfoundry.image.base-directory"`
`@@ -80,7 +81,7 @@ func (f LayerFetcher) StreamBlob(logger lager.Logger, baseImageURL url.URL, so`
`80`	`81`	`blobReader, err := NewBlobReader(blobFilePath)`
`81`	`82`	`if err != nil {`
`82`	`83`	`logger.Error("blob-reader-failed", err)`
`83`		`- return nil, 0, err`
	`84`	`+ return nil, 0, errorspkg.Wrap(err, "opening stream from temporary blob file")`
`84`	`85`	`}`
`85`	`86`
`86`	`87`	`return blobReader, size, nil`