Skip to content

Commit

Permalink
Merge pull request #2217 from buildpacks/security-fixes
Browse files Browse the repository at this point in the history
Fixes from security review
  • Loading branch information
natalieparellano authored Jul 17, 2024
2 parents cbc880a + 13ca537 commit f6b450f
Show file tree
Hide file tree
Showing 17 changed files with 625 additions and 83 deletions.
17 changes: 11 additions & 6 deletions acceptance/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"github.com/buildpacks/pack/internal/style"
"github.com/buildpacks/pack/pkg/archive"
"github.com/buildpacks/pack/pkg/cache"
"github.com/buildpacks/pack/pkg/logging"
h "github.com/buildpacks/pack/testhelpers"
)

Expand Down Expand Up @@ -1162,8 +1163,9 @@ func testAcceptance(
ref, err := name.ParseReference(repoName, name.WeakValidation)
assert.Nil(err)
cacheImage := cache.NewImageCache(ref, dockerCli)
buildCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli)
launchCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli)
logger := logging.NewSimpleLogger(&bytes.Buffer{})
buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli, logger)
launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli, logger)
cacheImage.Clear(context.TODO())
buildCacheVolume.Clear(context.TODO())
launchCacheVolume.Clear(context.TODO())
Expand Down Expand Up @@ -1282,8 +1284,9 @@ func testAcceptance(
ref, err := name.ParseReference(repoName, name.WeakValidation)
assert.Nil(err)
cacheImage := cache.NewImageCache(ref, dockerCli)
buildCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli)
launchCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli)
logger := logging.NewSimpleLogger(&bytes.Buffer{})
buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli, logger)
launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli, logger)
cacheImage.Clear(context.TODO())
buildCacheVolume.Clear(context.TODO())
launchCacheVolume.Clear(context.TODO())
Expand Down Expand Up @@ -1627,6 +1630,7 @@ func testAcceptance(

it.Before(func() {
h.SkipIf(t, os.Getenv("DOCKER_HOST") != "", "cannot mount volume when DOCKER_HOST is set")
h.SkipIf(t, imageManager.HostOS() == "windows", "These tests are broken on Windows Containers on Windows when not using the creator; see https://github.com/buildpacks/pack/issues/2147")

if imageManager.HostOS() == "windows" {
volumeRoot = `c:\`
Expand Down Expand Up @@ -3167,8 +3171,9 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ]
imageManager.CleanupImages(origID, repoName, runBefore)
ref, err := name.ParseReference(repoName, name.WeakValidation)
assert.Nil(err)
buildCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli)
launchCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli)
logger := logging.NewSimpleLogger(&bytes.Buffer{})
buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli, logger)
launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli, logger)
assert.Succeeds(buildCacheVolume.Clear(context.TODO()))
assert.Succeeds(launchCacheVolume.Clear(context.TODO()))
})
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module github.com/buildpacks/pack

require (
github.com/BurntSushi/toml v1.3.2
github.com/GoogleContainerTools/kaniko v1.22.0
github.com/Masterminds/semver v1.5.0
github.com/Microsoft/go-winio v0.6.2
github.com/apex/log v1.9.0
Expand Down Expand Up @@ -108,7 +109,7 @@ require (
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/mattn/go-runewidth v0.0.15 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/mapstructure v1.4.1 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
github.com/moby/buildkit v0.13.2 // indirect
github.com/moby/docker-image-spec v1.3.1 // indirect
github.com/moby/patternmatcher v0.6.0 // indirect
Expand Down
14 changes: 8 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ github.com/Azure/go-autorest/tracing v0.6.0 h1:TYi4+3m5t6K48TGI9AUdb+IzbnSxvnvUM
github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU=
github.com/BurntSushi/toml v1.3.2 h1:o7IhLm0Msx3BaB+n3Ag7L8EVlByGnpq14C4YWiu/gL8=
github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
github.com/GoogleContainerTools/kaniko v1.22.0 h1:WIL8Wuc+lQW8sv1R+zOZsCy4lQtTzrVJ76K2VMkB++0=
github.com/GoogleContainerTools/kaniko v1.22.0/go.mod h1:Kki7uX+HlskobmD7PRrGZvL0S9Aejf8kzfzoQUv68pQ=
github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww=
github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y=
github.com/Microsoft/go-winio v0.5.2/go.mod h1:WpS1mjBmmwHBEWmogvA2mj8546UReBk4v8QkMxJ6pZY=
Expand Down Expand Up @@ -272,8 +274,8 @@ github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG
github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
github.com/mitchellh/ioprogress v0.0.0-20180201004757-6a23b12fa88e h1:Qa6dnn8DlasdXRnacluu8HzPts0S1I9zvvUPDbBnXFI=
github.com/mitchellh/ioprogress v0.0.0-20180201004757-6a23b12fa88e/go.mod h1:waEya8ee1Ro/lgxpVhkJI4BVASzkm3UZqkx/cFJiYHM=
github.com/mitchellh/mapstructure v1.4.1 h1:CpVNEelQCZBooIPDn+AR3NpivK/TIKU8bDxdASFVQag=
github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY=
github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/moby/buildkit v0.13.2 h1:nXNszM4qD9E7QtG7bFWPnDI1teUQFQglBzon/IU3SzI=
github.com/moby/buildkit v0.13.2/go.mod h1:2cyVOv9NoHM7arphK9ZfHIWKn9YVZRFd1wXB8kKmEzY=
github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0=
Expand Down Expand Up @@ -533,8 +535,8 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
google.golang.org/genproto v0.0.0-20240213162025-012b6fc9bca9 h1:9+tzLLstTlPTRyJTh+ah5wIMsBW5c4tQwGTN3thOW9Y=
google.golang.org/genproto/googleapis/api v0.0.0-20231016165738-49dd2c1f3d0b h1:CIC2YMXmIhYw6evmhPxBKJ4fmLbOFtXQN/GV3XOZR8k=
google.golang.org/genproto/googleapis/api v0.0.0-20231016165738-49dd2c1f3d0b/go.mod h1:IBQ646DjkDkvUIsVq/cc03FUFQ9wbZu7yE396YcL870=
google.golang.org/genproto/googleapis/api v0.0.0-20240311132316-a219d84964c2 h1:rIo7ocm2roD9DcFIX67Ym8icoGCKSARAiPljFhh5suQ=
google.golang.org/genproto/googleapis/api v0.0.0-20240311132316-a219d84964c2/go.mod h1:O1cOfN1Cy6QEYr7VxtjOyP5AdAuR0aJ/MYZaaof623Y=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240314234333-6e1732d8331c h1:lfpJ/2rWPa/kJgxyyXM8PrNnfCzcmxJ265mADgwmvLI=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240314234333-6e1732d8331c/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY=
google.golang.org/grpc v1.62.1 h1:B4n+nfKzOICUXMgyrNd19h/I9oH0L1pizfk1d4zSgTk=
Expand All @@ -561,5 +563,5 @@ gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C
gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0=
gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8=
gotest.tools/v3 v3.4.0 h1:ZazjZUfuVeZGLAmlKKuyv3IKP5orXcwtOwDQH6YVr6o=
gotest.tools/v3 v3.4.0/go.mod h1:CtbdzLSsqVhDgMtKsx03ird5YTGB3ar27v0u/yKBW5g=
2 changes: 2 additions & 0 deletions internal/build/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ type DockerClient interface {
ContainerInspect(ctx context.Context, container string) (types.ContainerJSON, error)
ContainerRemove(ctx context.Context, container string, options containertypes.RemoveOptions) error
CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error
NetworkCreate(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error)
NetworkRemove(ctx context.Context, network string) error
}

var _ DockerClient = dockerClient.CommonAPIClient(nil)
46 changes: 42 additions & 4 deletions internal/build/lifecycle_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/buildpacks/lifecycle/api"
"github.com/buildpacks/lifecycle/auth"
"github.com/buildpacks/lifecycle/platform/files"
"github.com/docker/docker/api/types"
"github.com/google/go-containerregistry/pkg/name"
"github.com/pkg/errors"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -165,6 +166,7 @@ func (l *LifecycleExecution) PrevImageName() string {

func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseFactoryCreator) error {
phaseFactory := phaseFactoryCreator(l)

var buildCache Cache
if l.opts.CacheImage != "" || (l.opts.Cache.Build.Format == cache.CacheImage) {
cacheImageName := l.opts.CacheImage
Expand All @@ -179,7 +181,11 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF
} else {
switch l.opts.Cache.Build.Format {
case cache.CacheVolume:
buildCache = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Build, "build", l.docker)
var err error
buildCache, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Build, "build", l.docker, l.logger)
if err != nil {
return err
}
l.logger.Debugf("Using build cache volume %s", style.Symbol(buildCache.Name()))
case cache.CacheBind:
buildCache = cache.NewBindCache(l.opts.Cache.Build, l.docker)
Expand All @@ -194,7 +200,33 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF
l.logger.Debugf("Build cache %s cleared", style.Symbol(buildCache.Name()))
}

launchCache := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Launch, "launch", l.docker)
launchCache, err := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Launch, "launch", l.docker, l.logger)
if err != nil {
return err
}

if l.opts.Network == "" {
// start an ephemeral bridge network
driver := "bridge"
if l.os == "windows" {
driver = "nat"
}
networkName := fmt.Sprintf("pack.local/network/%x", randString(10))
resp, err := l.docker.NetworkCreate(ctx, networkName, types.NetworkCreate{
Driver: driver,
})
if err != nil {
return fmt.Errorf("failed to create ephemeral %s network: %w", driver, err)
}
defer func() {
_ = l.docker.NetworkRemove(ctx, networkName)
}()
l.logger.Debugf("Created ephemeral bridge network %s with ID %s", networkName, resp.ID)
if resp.Warning != "" {
l.logger.Warn(resp.Warning)
}
l.opts.Network = networkName
}

if !l.opts.UseCreator {
if l.platformAPI.LessThan("0.7") {
Expand Down Expand Up @@ -224,7 +256,10 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF
// lifecycle 0.17.0 (introduces support for Platform API 0.12) and above will ensure that
// this volume is owned by the CNB user,
// and hence the restorer (after dropping privileges) will be able to write to it.
kanikoCache = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker)
kanikoCache, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker, l.logger)
if err != nil {
return err
}
} else {
switch {
case buildCache.Type() == cache.Volume:
Expand All @@ -236,7 +271,10 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF
return fmt.Errorf("build cache must be volume cache when building with extensions")
default:
// The kaniko cache is unused, so it doesn't matter that it's not usable.
kanikoCache = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker)
kanikoCache, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker, l.logger)
if err != nil {
return err
}
}
}

Expand Down
65 changes: 63 additions & 2 deletions internal/build/lifecycle_execution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
ifakes "github.com/buildpacks/imgutil/fakes"
"github.com/buildpacks/lifecycle/api"
"github.com/buildpacks/lifecycle/platform/files"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/client"
"github.com/google/go-containerregistry/pkg/authn"
Expand Down Expand Up @@ -275,7 +276,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
fakeBuilder *fakes.FakeBuilder
outBuf bytes.Buffer
logger *logging.LogWithWriters
docker *client.Client
docker *fakeDockerClient
fakeTermui *fakes.FakeTermui
)

Expand All @@ -289,7 +290,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
fakeBuilder, err = fakes.NewFakeBuilder(fakes.WithSupportedPlatformAPIs([]*api.Version{api.MustParse("0.3")}))
h.AssertNil(t, err)
logger = logging.NewLogWithWriters(&outBuf, &outBuf)
docker, err = client.NewClientWithOpts(client.FromEnv, client.WithVersion("1.38"))
docker = &fakeDockerClient{}
h.AssertNil(t, err)
fakePhaseFactory = fakes.NewFakePhaseFactory()
})
Expand Down Expand Up @@ -780,6 +781,46 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
})
})

when("network is not provided", func() {
it("creates an ephemeral bridge network", func() {
beforeNetworks := func() int {
networks, err := docker.NetworkList(context.Background(), types.NetworkListOptions{})
h.AssertNil(t, err)
return len(networks)
}()

opts := build.LifecycleOptions{
Image: imageName,
Builder: fakeBuilder,
Termui: fakeTermui,
}

lifecycle, err := build.NewLifecycleExecution(logger, docker, "some-temp-dir", opts)
h.AssertNil(t, err)

err = lifecycle.Run(context.Background(), func(execution *build.LifecycleExecution) build.PhaseFactory {
return fakePhaseFactory
})
h.AssertNil(t, err)

for _, entry := range fakePhaseFactory.NewCalledWithProvider {
h.AssertContains(t, string(entry.HostConfig().NetworkMode), "pack.local/network/")
h.AssertEq(t, entry.HostConfig().NetworkMode.IsDefault(), false)
h.AssertEq(t, entry.HostConfig().NetworkMode.IsHost(), false)
h.AssertEq(t, entry.HostConfig().NetworkMode.IsNone(), false)
h.AssertEq(t, entry.HostConfig().NetworkMode.IsPrivate(), true)
h.AssertEq(t, entry.HostConfig().NetworkMode.IsUserDefined(), true)
}

afterNetworks := func() int {
networks, err := docker.NetworkList(context.Background(), types.NetworkListOptions{})
h.AssertNil(t, err)
return len(networks)
}()
h.AssertEq(t, beforeNetworks, afterNetworks)
})
})

when("Error cases", func() {
when("passed invalid", func() {
it("fails for cache-image", func() {
Expand Down Expand Up @@ -2657,6 +2698,26 @@ func (f *fakeImageFetcher) fetchRunImage(name string) error {
return nil
}

type fakeDockerClient struct {
nNetworks int
build.DockerClient
}

func (f *fakeDockerClient) NetworkList(ctx context.Context, opts types.NetworkListOptions) ([]types.NetworkResource, error) {
ret := make([]types.NetworkResource, f.nNetworks)
return ret, nil
}

func (f *fakeDockerClient) NetworkCreate(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error) {
f.nNetworks++
return types.NetworkCreateResponse{}, nil
}

func (f *fakeDockerClient) NetworkRemove(ctx context.Context, network string) error {
f.nNetworks--
return nil
}

func newTestLifecycleExecErr(t *testing.T, logVerbose bool, tmpDir string, ops ...func(*build.LifecycleOptions)) (*build.LifecycleExecution, error) {
docker, err := client.NewClientWithOpts(client.FromEnv, client.WithVersion("1.38"))
h.AssertNil(t, err)
Expand Down
7 changes: 6 additions & 1 deletion internal/build/phase_config_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,14 @@ type PhaseConfigProvider struct {
}

func NewPhaseConfigProvider(name string, lifecycleExec *LifecycleExecution, ops ...PhaseConfigProviderOperation) *PhaseConfigProvider {
hostConf := new(container.HostConfig)
hostConf.UsernsMode = "host"
if lifecycleExec.os != "windows" {
hostConf.SecurityOpt = []string{"no-new-privileges=true"}
}
provider := &PhaseConfigProvider{
ctrConf: new(container.Config),
hostConf: new(container.HostConfig),
hostConf: hostConf,
name: name,
os: lifecycleExec.os,
infoWriter: logging.GetWriterForLevel(lifecycleExec.logger, logging.InfoLevel),
Expand Down
3 changes: 3 additions & 0 deletions internal/build/phase_config_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ func testPhaseConfigProvider(t *testing.T, when spec.G, it spec.S) {
h.AssertSliceContainsMatch(t, phaseConfigProvider.HostConfig().Binds, "pack-app-.*:/workspace")

h.AssertEq(t, phaseConfigProvider.HostConfig().Isolation, container.IsolationEmpty)
h.AssertEq(t, phaseConfigProvider.HostConfig().UsernsMode, container.UsernsMode("host"))
h.AssertSliceContains(t, phaseConfigProvider.HostConfig().SecurityOpt, "no-new-privileges=true")
})

when("building for Windows", func() {
Expand All @@ -72,6 +74,7 @@ func testPhaseConfigProvider(t *testing.T, when spec.G, it spec.S) {
phaseConfigProvider := build.NewPhaseConfigProvider("some-name", lifecycle)

h.AssertEq(t, phaseConfigProvider.HostConfig().Isolation, container.IsolationProcess)
h.AssertSliceNotContains(t, phaseConfigProvider.HostConfig().SecurityOpt, "no-new-privileges=true")
})
})

Expand Down
Loading

0 comments on commit f6b450f

Please sign in to comment.