Skip to content

Commit

Permalink
Do not mount unrestricted volumes that overlap BOSH mounts
Browse files Browse the repository at this point in the history
Previous to this commit, there was de-duplication in the volumes
mounted, but only if they matched exactly.  This commit adds filtering
wherein unrestricted volumes will NOT be mounted if they overlap with
existing BOSH mounts.

BOSH mounts include:
* /var/vcap/jobs/<JOB_NAME>
* /var/vcap/packages/<JOB_NAME>
* /var/vcap/sys/log/<JOB_NAME>
among others
  • Loading branch information
selzoc committed Jun 28, 2024
1 parent 338524b commit 69dd7a4
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 3 deletions.
25 changes: 23 additions & 2 deletions src/bpm/runc/adapter/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"

"code.cloudfoundry.org/bytefmt"
"code.cloudfoundry.org/lager/v3"
Expand Down Expand Up @@ -227,14 +228,16 @@ func (a *RuncAdapter) BuildSpec(

ms := newMountDedup(logger)
ms.addMounts(systemIdentityMounts(mountResolvConf))
ms.addMounts(boshMounts(bpmCfg, procCfg.EphemeralDisk, procCfg.PersistentDisk))
boshMounts := boshMounts(bpmCfg, procCfg.EphemeralDisk, procCfg.PersistentDisk)
ms.addMounts(boshMounts)
ms.addMounts(userProvidedIdentityMounts(bpmCfg, procCfg.AdditionalVolumes))
if procCfg.Unsafe != nil && len(procCfg.Unsafe.UnrestrictedVolumes) > 0 {
expanded, err := a.globExpandVolumes(procCfg.Unsafe.UnrestrictedVolumes)
if err != nil {
return specs.Spec{}, err
}
ms.addMounts(userProvidedIdentityMounts(bpmCfg, expanded))
filteredVolumes := filterVolumesUnderBoshMounts(boshMounts, expanded)
ms.addMounts(userProvidedIdentityMounts(bpmCfg, filteredVolumes))
}

wrappedExe, wrappedArgs := wrapWithInit(bpmCfg, procCfg)
Expand Down Expand Up @@ -285,6 +288,24 @@ func (a *RuncAdapter) BuildSpec(
return *spec, nil
}

func filterVolumesUnderBoshMounts(mounts []specs.Mount, volumes []config.Volume) []config.Volume {
var filteredVolumes []config.Volume
for _, v := range volumes {
keep := true
for _, m := range mounts {
if strings.HasPrefix(v.Path, m.Destination) {
keep = false
}
}

if keep {
filteredVolumes = append(filteredVolumes, v)
}
}

return filteredVolumes
}

func wrapWithInit(bpmCfg *config.BPMConfig, procCfg *config.ProcessConfig) (string, []string) {
exe := bpmCfg.TiniPath().Internal()
args := append([]string{"-w", "-s", "--", procCfg.Executable}, procCfg.Args...)
Expand Down
46 changes: 45 additions & 1 deletion src/bpm/runc/adapter/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ var _ = Describe("RuncAdapter", func() {
Path: bpmCfg.RootFSPath(),
}))

Expect(spec.Mounts).To(HaveLen(24))
Expect(spec.Mounts).To(HaveLen(24), fmt.Sprintf("spec.Mounts is actually length %d", len(spec.Mounts)))
Expect(spec.Mounts).To(HaveMount(specs.Mount{
Destination: "/proc",
Type: "proc",
Expand Down Expand Up @@ -868,6 +868,8 @@ var _ = Describe("RuncAdapter", func() {
UnrestrictedVolumes: []config.Volume{
{Path: "/this/is/an/unrestricted/path"},
{Path: "/writable/executable/path", Writable: true, AllowExecutions: true},
{Path: "/var/vcap/jobs/example/config/config.yml", MountOnly: true},
{Path: "/var/vcap/jobs/other/config/config.yml", MountOnly: true},
},
}
})
Expand All @@ -888,13 +890,32 @@ var _ = Describe("RuncAdapter", func() {
Source: "/writable/executable/path",
Options: []string{"nodev", "nosuid", "exec", "rbind", "rw"},
}))
Expect(spec.Mounts).To(HaveMount(specs.Mount{
Destination: "/var/vcap/jobs/other/config/config.yml",
Type: "bind",
Source: "/var/vcap/jobs/other/config/config.yml",
Options: []string{"nodev", "nosuid", "noexec", "rbind", "ro"},
}))
})

It("does not add the volume if it is a subdirectory of a bosh mount for this job", func() {
spec, err := runcAdapter.BuildSpec(logger, bpmCfg, procCfg, user)
Expect(err).NotTo(HaveOccurred())

Expect(spec.Mounts).NotTo(HaveMount(specs.Mount{
Destination: "/var/vcap/jobs/example/config/config.yml",
Type: "bind",
Source: "/var/vcap/jobs/example/config/config.yml",
Options: []string{"nodev", "nosuid", "noexec", "rbind", "ro"},
}))
})

Context("when the user requests a glob", func() {
BeforeEach(func() {
procCfg.Unsafe = &config.Unsafe{
UnrestrictedVolumes: []config.Volume{
{Path: "/*/file.txt", Writable: true, AllowExecutions: true},
{Path: "/var/vcap/jobs/*/config/config.yml", MountOnly: true},
},
}
})
Expand All @@ -907,6 +928,11 @@ var _ = Describe("RuncAdapter", func() {
"/unrestricted/file.txt",
"/other/file.txt",
}, nil
case "/var/vcap/jobs/*/config/config.yml":
return []string{
"/var/vcap/jobs/example/config/config.yml",
"/var/vcap/jobs/other-job/config/config.yml",
}, nil
default:
return []string{pattern}, nil
}
Expand All @@ -930,6 +956,24 @@ var _ = Describe("RuncAdapter", func() {
Source: "/other/file.txt",
Options: []string{"nodev", "nosuid", "exec", "rbind", "rw"},
}))
Expect(spec.Mounts).To(HaveMount(specs.Mount{
Destination: "/var/vcap/jobs/other-job/config/config.yml",
Type: "bind",
Source: "/var/vcap/jobs/other-job/config/config.yml",
Options: []string{"nodev", "nosuid", "noexec", "rbind", "ro"},
}))
})

It("does not add the volume if the glob is a subdirectory of a bosh mount for this job", func() {
spec, err := runcAdapter.BuildSpec(logger, bpmCfg, procCfg, user)
Expect(err).NotTo(HaveOccurred())

Expect(spec.Mounts).NotTo(HaveMount(specs.Mount{
Destination: "/var/vcap/jobs/example/config/config.yml",
Type: "bind",
Source: "/var/vcap/jobs/example/config/config.yml",
Options: []string{"nodev", "nosuid", "noexec", "rbind", "ro"},
}))
})

Context("when the glob fails", func() {
Expand Down

0 comments on commit 69dd7a4

Please sign in to comment.