Skip to content

Commit 6e24e9a

Browse files
Callisto13Callisto13
committed
Add option to --skip-layer-validation (OCI images)
[finishes #149849148]
1 parent f97ef1f commit 6e24e9a

File tree

12 files changed

+161
-20
lines changed

12 files changed

+161
-20
lines changed

‎commands/config/builder.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ type Config struct {
2727

2828
type Create struct {
2929
ExcludeImageFromQuota bool `yaml:"exclude_image_from_quota"`
30+
SkipLayerValidation bool `yaml:"skip_layer_validation"`
3031
WithClean bool `yaml:"with_clean"`
3132
WithoutMount bool `yaml:"without_mount"`
3233
DiskLimitSizeBytes int64 `yaml:"disk_limit_size_bytes"`
@@ -175,6 +176,13 @@ func (b *Builder) WithExcludeImageFromQuota(exclude, isSet bool) *Builder {
175176
return b
176177
}
177178

179+
func (b *Builder) WithSkipLayerValidation(skip, isSet bool) *Builder {
180+
if isSet {
181+
b.config.Create.SkipLayerValidation = skip
182+
}
183+
return b
184+
}
185+
178186
func (b *Builder) WithCleanThresholdBytes(threshold int64, isSet bool) *Builder {
179187
if isSet {
180188
b.config.Clean.ThresholdBytes = threshold

‎commands/config/builder_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ var _ = Describe("Builder", func() {
2727
WithClean: false,
2828
WithoutMount: false,
2929
ExcludeImageFromQuota: true,
30+
SkipLayerValidation: true,
3031
InsecureRegistries: []string{"http://example.org"},
3132
DiskLimitSizeBytes: int64(1000),
3233
}
@@ -463,6 +464,24 @@ var _ = Describe("Builder", func() {
463464
})
464465
})
465466

467+
Describe("WithSkipLayerValidation", func() {
468+
It("overrides the config's SkipLayerValidation when the flag is set", func() {
469+
builder = builder.WithSkipLayerValidation(false, true)
470+
config, err := builder.Build()
471+
Expect(err).NotTo(HaveOccurred())
472+
Expect(config.Create.SkipLayerValidation).To(BeFalse())
473+
})
474+
475+
Context("when flag is not set", func() {
476+
It("uses the config entry", func() {
477+
builder = builder.WithSkipLayerValidation(false, false)
478+
config, err := builder.Build()
479+
Expect(err).NotTo(HaveOccurred())
480+
Expect(config.Create.SkipLayerValidation).To(BeTrue())
481+
})
482+
})
483+
})
484+
466485
Describe("WithCleanThresholdBytes", func() {
467486
It("overrides the config's CleanThresholdBytes entry when the flag is set", func() {
468487
builder = builder.WithCleanThresholdBytes(1024, true)

‎commands/create.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ var CreateCommand = cli.Command{
5151
Name: "exclude-image-from-quota",
5252
Usage: "Set disk limit to be exclusive (i.e.: excluding image layers)",
5353
},
54+
cli.BoolFlag{
55+
Name: "skip-layer-validation",
56+
Usage: "Do not validate checksums of image layers. (Can only be used with oci:/// protocol images.)",
57+
},
5458
cli.BoolFlag{
5559
Name: "with-clean",
5660
Usage: "Clean up unused layers before creating rootfs",
@@ -93,6 +97,8 @@ var CreateCommand = cli.Command{
9397
ctx.IsSet("disk-limit-size-bytes")).
9498
WithExcludeImageFromQuota(ctx.Bool("exclude-image-from-quota"),
9599
ctx.IsSet("exclude-image-from-quota")).
100+
WithSkipLayerValidation(ctx.Bool("skip-layer-validation"),
101+
ctx.IsSet("skip-layer-validation")).
96102
WithClean(ctx.IsSet("with-clean"), ctx.IsSet("without-clean")).
97103
WithMount(ctx.IsSet("with-mount"), ctx.IsSet("without-mount"))
98104

@@ -150,7 +156,7 @@ var CreateCommand = cli.Command{
150156
unpacker = unpackerpkg.NewNSIdMapperUnpacker(runner, idMapper, unpackerStrategy)
151157
}
152158

153-
layerSource := source.NewLayerSource(ctx.String("username"), ctx.String("password"), cfg.Create.InsecureRegistries)
159+
layerSource := source.NewLayerSource(ctx.String("username"), ctx.String("password"), cfg.Create.InsecureRegistries, cfg.Create.SkipLayerValidation)
154160

155161
layerFetcher := layer_fetcher.NewLayerFetcher(&layerSource)
156162
tarFetcher := tar_fetcher.NewTarFetcher()

‎fetcher/layer_fetcher/layer_fetcher.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
"github.com/containers/image/types"
1515
specsv1 "github.com/opencontainers/image-spec/specs-go/v1"
16+
errorspkg "github.com/pkg/errors"
1617
)
1718

1819
const cfBaseDirectoryAnnotation = "org.cloudfoundry.image.base-directory"
@@ -80,7 +81,7 @@ func (f *LayerFetcher) StreamBlob(logger lager.Logger, baseImageURL *url.URL, so
8081
blobReader, err := NewBlobReader(blobFilePath)
8182
if err != nil {
8283
logger.Error("blob-reader-failed", err)
83-
return nil, 0, err
84+
return nil, 0, errorspkg.Wrap(err, "opening stream from temporary blob file")
8485
}
8586

8687
return blobReader, size, nil

‎fetcher/layer_fetcher/source/layer_source.go

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,18 @@ import (
2626
const MAX_BLOB_RETRIES = 3
2727

2828
type LayerSource struct {
29-
trustedRegistries []string
30-
username string
31-
password string
29+
trustedRegistries []string
30+
username string
31+
password string
32+
skipChecksumValidation bool
3233
}
3334

34-
func NewLayerSource(username, password string, trustedRegistries []string) LayerSource {
35+
func NewLayerSource(username, password string, trustedRegistries []string, skipChecksumValidation bool) LayerSource {
3536
return LayerSource{
36-
username: username,
37-
password: password,
38-
trustedRegistries: trustedRegistries,
37+
username: username,
38+
password: password,
39+
trustedRegistries: trustedRegistries,
40+
skipChecksumValidation: skipChecksumValidation,
3941
}
4042
}
4143

@@ -82,9 +84,11 @@ func (s *LayerSource) Blob(logger lager.Logger, baseImageURL *url.URL, digest st
8284
defer func() { _ = blobTempFile.Close() }()
8385

8486
blobReader := io.TeeReader(blob, blobTempFile)
85-
if !s.checkCheckSum(logger, blobReader, digest) {
87+
88+
if !s.checkCheckSum(logger, blobReader, digest, baseImageURL) {
8689
return "", 0, errorspkg.Errorf("invalid checksum: layer is corrupted `%s`", digest)
8790
}
91+
8892
return blobTempFile.Name(), size, nil
8993
}
9094

@@ -102,7 +106,7 @@ func (s *LayerSource) getBlobWithRetries(imgSrc types.ImageSource, blobInfo type
102106
return nil, 0, err
103107
}
104108

105-
func (s *LayerSource) checkCheckSum(logger lager.Logger, data io.Reader, digest string) bool {
109+
func (s *LayerSource) checkCheckSum(logger lager.Logger, data io.Reader, digest string, baseImageURL *url.URL) bool {
106110
var (
107111
actualSize int64
108112
err error
@@ -122,7 +126,7 @@ func (s *LayerSource) checkCheckSum(logger lager.Logger, data io.Reader, digest
122126
"downloadedChecksum": blobContentsSha,
123127
})
124128

125-
return digestID == blobContentsSha
129+
return s.skipLayerCheckSumValidation(baseImageURL.Scheme) || digestID == blobContentsSha
126130
}
127131

128132
func (s *LayerSource) skipTLSValidation(baseImageURL *url.URL) bool {
@@ -262,3 +266,11 @@ func preferedMediaTypes() []string {
262266
manifestpkg.DockerV2Schema2MediaType,
263267
}
264268
}
269+
270+
func (s *LayerSource) skipLayerCheckSumValidation(scheme string) bool {
271+
if s.skipChecksumValidation && scheme == "oci" {
272+
return true
273+
}
274+
275+
return false
276+
}

‎fetcher/layer_fetcher/source/layer_source_docker_test.go

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,13 @@ var _ = Describe("Layer source: Docker", func() {
3030
configBlob string
3131
expectedLayersDigests []types.BlobInfo
3232
expectedDiffIds []digestpkg.Digest
33+
34+
skipChecksumValidation bool
3335
)
3436

3537
BeforeEach(func() {
3638
trustedRegistries = []string{}
39+
skipChecksumValidation = false
3740

3841
configBlob = "sha256:217f3b4afdf698d639f854d9c6d640903a011413bc7e7bffeabe63c7ca7e4a7d"
3942
expectedLayersDigests = []types.BlobInfo{
@@ -58,7 +61,7 @@ var _ = Describe("Layer source: Docker", func() {
5861
})
5962

6063
JustBeforeEach(func() {
61-
layerSource = source.NewLayerSource("", "", trustedRegistries)
64+
layerSource = source.NewLayerSource("", "", trustedRegistries, skipChecksumValidation)
6265
})
6366

6467
Describe("Manifest", func() {
@@ -106,7 +109,7 @@ var _ = Describe("Layer source: Docker", func() {
106109

107110
Context("when the correct credentials are provided", func() {
108111
JustBeforeEach(func() {
109-
layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries)
112+
layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries, skipChecksumValidation)
110113
})
111114

112115
It("fetches the manifest", func() {
@@ -186,7 +189,7 @@ var _ = Describe("Layer source: Docker", func() {
186189
})
187190

188191
JustBeforeEach(func() {
189-
layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries)
192+
layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries, skipChecksumValidation)
190193
var err error
191194
manifest, err = layerSource.Manifest(logger, baseImageURL)
192195
Expect(err).NotTo(HaveOccurred())
@@ -351,7 +354,7 @@ var _ = Describe("Layer source: Docker", func() {
351354
})
352355

353356
JustBeforeEach(func() {
354-
layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries)
357+
layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries, skipChecksumValidation)
355358
})
356359

357360
It("fetches the manifest", func() {
@@ -427,7 +430,7 @@ var _ = Describe("Layer source: Docker", func() {
427430

428431
Context("when the correct credentials are provided", func() {
429432
JustBeforeEach(func() {
430-
layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries)
433+
layerSource = source.NewLayerSource(RegistryUsername, RegistryPassword, trustedRegistries, skipChecksumValidation)
431434
})
432435

433436
It("fetches the config", func() {
@@ -500,6 +503,17 @@ var _ = Describe("Layer source: Docker", func() {
500503
_, _, err := layerSource.Blob(logger, baseImageURL, expectedLayersDigests[1].Digest.String())
501504
Expect(err).To(MatchError(ContainSubstring("invalid checksum: layer is corrupted")))
502505
})
506+
507+
Context("when a devious hacker tries to set skipChecksumValidation to true", func() {
508+
BeforeEach(func() {
509+
skipChecksumValidation = true
510+
})
511+
512+
It("returns an error", func() {
513+
_, _, err := layerSource.Blob(logger, baseImageURL, expectedLayersDigests[1].Digest.String())
514+
Expect(err).To(MatchError(ContainSubstring("invalid checksum: layer is corrupted")))
515+
})
516+
})
503517
})
504518
})
505519
})

‎fetcher/layer_fetcher/source/layer_source_oci_test.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@ var _ = Describe("Layer source: OCI", func() {
2727
configBlob string
2828
expectedLayersDigest []types.BlobInfo
2929
expectedDiffIds []digestpkg.Digest
30-
// manifest layer_fetcher.Manifest
31-
workDir string
30+
workDir string
31+
32+
skipChecksumValidation bool
3233
)
3334

3435
BeforeEach(func() {
3536
trustedRegistries = []string{}
37+
skipChecksumValidation = false
3638

3739
configBlob = "sha256:10c8f0eb9d1af08fe6e3b8dbd29e5aa2b6ecfa491ecd04ed90de19a4ac22de7b"
3840
expectedLayersDigest = []types.BlobInfo{
@@ -59,7 +61,7 @@ var _ = Describe("Layer source: OCI", func() {
5961
})
6062

6163
JustBeforeEach(func() {
62-
layerSource = source.NewLayerSource("", "", trustedRegistries)
64+
layerSource = source.NewLayerSource("", "", trustedRegistries, skipChecksumValidation)
6365
})
6466

6567
Describe("Manifest", func() {
@@ -169,5 +171,19 @@ var _ = Describe("Layer source: OCI", func() {
169171
Expect(err).To(MatchError(ContainSubstring("invalid checksum: layer is corrupted")))
170172
})
171173
})
174+
175+
Context("when skipChecksumValidation is set to true", func() {
176+
BeforeEach(func() {
177+
var err error
178+
baseImageURL, err = url.Parse(fmt.Sprintf("oci:///%s/../../../integration/assets/oci-test-image/corrupted:latest", workDir))
179+
Expect(err).NotTo(HaveOccurred())
180+
skipChecksumValidation = true
181+
})
182+
183+
It("does not validate against checksums and does not return an error", func() {
184+
_, _, err := layerSource.Blob(logger, baseImageURL, expectedLayersDigest[0].Digest.String())
185+
Expect(err).NotTo(HaveOccurred())
186+
})
187+
})
172188
})
173189
})

‎integration/create_docker_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -762,4 +762,41 @@ var _ = Describe("Create with remote DOCKER images", func() {
762762
})
763763
})
764764
})
765+
766+
Context("when --skip-layer-validation flag is passed", func() {
767+
var (
768+
fakeRegistry *testhelpers.FakeRegistry
769+
corruptedBlob string
770+
)
771+
772+
AfterEach(func() {
773+
fakeRegistry.Stop()
774+
})
775+
776+
BeforeEach(func() {
777+
dockerHubUrl, err := url.Parse("https://registry-1.docker.io")
778+
Expect(err).NotTo(HaveOccurred())
779+
fakeRegistry = testhelpers.NewFakeRegistry(dockerHubUrl)
780+
corruptedBlob = testhelpers.EmptyBaseImageV011.Layers[1].BlobID
781+
fakeRegistry.WhenGettingBlob(corruptedBlob, 0, func(w http.ResponseWriter, r *http.Request) {
782+
_, err := w.Write([]byte("bad-blob"))
783+
Expect(err).NotTo(HaveOccurred())
784+
})
785+
fakeRegistry.Start()
786+
baseImageURL = fmt.Sprintf("docker://%s/cfgarden/empty:v0.1.1", fakeRegistry.Addr())
787+
})
788+
789+
It("has no effect", func() {
790+
runner := Runner.WithInsecureRegistry(fakeRegistry.Addr())
791+
792+
_, err := runner.SkipLayerCheckSumValidation().Create(groot.CreateSpec{
793+
BaseImage: baseImageURL,
794+
ID: "random-id",
795+
Mount: true,
796+
})
797+
798+
Expect(err).To(HaveOccurred())
799+
Expect(err).To(MatchError(ContainSubstring("layer is corrupted")))
800+
})
801+
})
765802
})

‎integration/create_oci_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,4 +395,17 @@ var _ = Describe("Create with OCI images", func() {
395395
})
396396
})
397397
})
398+
399+
Context("when --skip-layer-validation flag is passed", func() {
400+
It("does not validate the checksums for oci image layers", func() {
401+
image, err := Runner.SkipLayerCheckSumValidation().Create(groot.CreateSpec{
402+
BaseImage: fmt.Sprintf("oci:///%s/assets/oci-test-image/corrupted:latest", workDir),
403+
ID: "random-id",
404+
Mount: true,
405+
})
406+
407+
Expect(err).NotTo(HaveOccurred())
408+
Expect(filepath.Join(image.Rootfs, "corrupted")).To(BeARegularFile())
409+
})
410+
})
398411
})

‎integration/runner/create.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ func (r Runner) makeCreateArgs(spec groot.CreateSpec) []string {
113113
args = append(args, "--password", r.RegistryPassword)
114114
}
115115

116+
if r.SkipLayerValidation {
117+
args = append(args, "--skip-layer-validation")
118+
}
119+
116120
if spec.DiskLimit != 0 {
117121
args = append(args, "--disk-limit-size-bytes",
118122
strconv.FormatInt(spec.DiskLimit, 10),

0 commit comments

Comments
 (0)