diff --git a/go.mod b/go.mod index d33810fbd23..ea0a1fbfdb0 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ go 1.21.0 // ***** ATTENTION WARNING CAUTION DANGER ****** require ( github.com/containerd/containerd v1.7.18 github.com/containernetworking/cni v1.2.3 - github.com/containers/common v0.60.2 + github.com/containers/common v0.60.4 github.com/containers/image/v5 v5.32.2 github.com/containers/luksy v0.0.0-20240618143119-a8846e21c08c github.com/containers/ocicrypt v1.2.0 diff --git a/go.sum b/go.sum index e12b7d37881..1c53edc703d 100644 --- a/go.sum +++ b/go.sum @@ -61,8 +61,8 @@ github.com/containernetworking/cni v1.2.3 h1:hhOcjNVUQTnzdRJ6alC5XF+wd9mfGIUaj8F github.com/containernetworking/cni v1.2.3/go.mod h1:DuLgF+aPd3DzcTQTtp/Nvl1Kim23oFKdm2okJzBQA5M= github.com/containernetworking/plugins v1.5.1 h1:T5ji+LPYjjgW0QM+KyrigZbLsZ8jaX+E5J/EcKOE4gQ= github.com/containernetworking/plugins v1.5.1/go.mod h1:MIQfgMayGuHYs0XdNudf31cLLAC+i242hNm6KuDGqCM= -github.com/containers/common v0.60.2 h1:utcwp2YkO8c0mNlwRxsxfOiqfj157FRrBjxgjR6f+7o= -github.com/containers/common v0.60.2/go.mod h1:I0upBi1qJX3QmzGbUOBN1LVP6RvkKhd3qQpZbQT+Q54= +github.com/containers/common v0.60.4 h1:H5+LAMHPZEqX6vVNOQ+IguVsaFl8kbO/SZ/VPXjxhy0= +github.com/containers/common v0.60.4/go.mod h1:I0upBi1qJX3QmzGbUOBN1LVP6RvkKhd3qQpZbQT+Q54= github.com/containers/image/v5 v5.32.2 h1:SzNE2Y6sf9b1GJoC8qjCuMBXwQrACFp4p0RK15+4gmQ= github.com/containers/image/v5 v5.32.2/go.mod h1:v1l73VeMugfj/QtKI+jhYbwnwFCFnNGckvbST3rQ5Hk= github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 h1:Qzk5C6cYglewc+UyGf6lc8Mj2UaPTHy/iF2De0/77CA= diff --git a/internal/volumes/volumes.go b/internal/volumes/volumes.go index 515f846f349..da6b768fdc2 100644 --- a/internal/volumes/volumes.go +++ b/internal/volumes/volumes.go @@ -105,6 +105,12 @@ func GetBindMount(ctx *types.SystemContext, args []string, contextDir string, st if !hasArgValue { return newMount, "", fmt.Errorf("%v: %w", argName, errBadOptionArg) } + switch argValue { + default: + return newMount, "", fmt.Errorf("%v: %q: %w", argName, argValue, errBadMntOption) + case "shared", "rshared", "private", "rprivate", "slave", "rslave": + // this should be the relevant parts of the same list of options we accepted above + } newMount.Options = append(newMount.Options, argValue) case "src", "source": if !hasArgValue { @@ -277,6 +283,12 @@ func GetCacheMount(args []string, store storage.Store, imageMountLabel string, a if !hasArgValue { return newMount, nil, fmt.Errorf("%v: %w", argName, errBadOptionArg) } + switch argValue { + default: + return newMount, nil, fmt.Errorf("%v: %q: %w", argName, argValue, errBadMntOption) + case "shared", "rshared", "private", "rprivate", "slave", "rslave": + // this should be the relevant parts of the same list of options we accepted above + } newMount.Options = append(newMount.Options, argValue) case "id": if !hasArgValue { diff --git a/tests/bud.bats b/tests/bud.bats index f40319b6aab..b1ed89072ce 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -6892,3 +6892,28 @@ _EOF run_buildah build ${TEST_SCRATCH_DIR} expect_output --substring "\-\-platform=$platform" } + +@test "build-validates-bind-bind-propagation" { + _prefetch alpine + + cat > ${TEST_SCRATCH_DIR}/Containerfile << _EOF +FROM alpine as base +FROM alpine +RUN --mount=type=bind,from=base,source=/,destination=/var/empty,rw,bind-propagation=suid pwd +_EOF + + run_buildah 125 build $WITH_POLICY_JSON ${TEST_SCRATCH_DIR} + expect_output --substring "invalid mount option" +} + +@test "build-validates-cache-bind-propagation" { + _prefetch alpine + + cat > ${TEST_SCRATCH_DIR}/Containerfile << _EOF +FROM alpine +RUN --mount=type=cache,destination=/var/empty,rw,bind-propagation=suid pwd +_EOF + + run_buildah 125 build $WITH_POLICY_JSON ${TEST_SCRATCH_DIR} + expect_output --substring "invalid mount option" +} diff --git a/vendor/github.com/containers/common/pkg/netns/netns_linux.go b/vendor/github.com/containers/common/pkg/netns/netns_linux.go index bbcedc0f6ce..3a3372889a0 100644 --- a/vendor/github.com/containers/common/pkg/netns/netns_linux.go +++ b/vendor/github.com/containers/common/pkg/netns/netns_linux.go @@ -40,6 +40,8 @@ import ( // threadNsPath is the /proc path to the current netns handle for the current thread const threadNsPath = "/proc/thread-self/ns/net" +var errNoFreeName = errors.New("failed to find free netns path name") + // GetNSRunDir returns the dir of where to create the netNS. When running // rootless, it needs to be at a location writable by user. func GetNSRunDir() (string, error) { @@ -60,14 +62,26 @@ func NewNSAtPath(nsPath string) (ns.NetNS, error) { // NewNS creates a new persistent (bind-mounted) network namespace and returns // an object representing that namespace, without switching to it. func NewNS() (ns.NetNS, error) { + nsRunDir, err := GetNSRunDir() + if err != nil { + return nil, err + } + + // Create the directory for mounting network namespaces + // This needs to be a shared mountpoint in case it is mounted in to + // other namespaces (containers) + err = makeNetnsDir(nsRunDir) + if err != nil { + return nil, err + } + for i := 0; i < 10000; i++ { - b := make([]byte, 16) - _, err := rand.Reader.Read(b) + nsName, err := getRandomNetnsName() if err != nil { - return nil, fmt.Errorf("failed to generate random netns name: %v", err) + return nil, err } - nsName := fmt.Sprintf("netns-%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:]) - ns, err := NewNSWithName(nsName) + nsPath := path.Join(nsRunDir, nsName) + ns, err := newNSPath(nsPath) if err == nil { return ns, nil } @@ -77,62 +91,128 @@ func NewNS() (ns.NetNS, error) { } return nil, err } - return nil, errors.New("failed to find free netns path name") + return nil, errNoFreeName } -// NewNSWithName creates a new persistent (bind-mounted) network namespace and returns -// an object representing that namespace, without switching to it. -func NewNSWithName(name string) (ns.NetNS, error) { +// NewNSFrom creates a persistent (bind-mounted) network namespace from the +// given netns path, i.e. /proc//ns/net, and returns the new full path to +// the bind mounted file in the netns run dir. +func NewNSFrom(fromNetns string) (string, error) { nsRunDir, err := GetNSRunDir() if err != nil { - return nil, err + return "", err } - // Create the directory for mounting network namespaces - // This needs to be a shared mountpoint in case it is mounted in to - // other namespaces (containers) - err = os.MkdirAll(nsRunDir, 0o755) + err = makeNetnsDir(nsRunDir) if err != nil { - return nil, err + return "", err } - // Remount the namespace directory shared. This will fail if it is not - // already a mountpoint, so bind-mount it on to itself to "upgrade" it - // to a mountpoint. - err = unix.Mount("", nsRunDir, "none", unix.MS_SHARED|unix.MS_REC, "") - if err != nil { - if err != unix.EINVAL { - return nil, fmt.Errorf("mount --make-rshared %s failed: %q", nsRunDir, err) + for i := 0; i < 10000; i++ { + nsName, err := getRandomNetnsName() + if err != nil { + return "", err } + nsPath := filepath.Join(nsRunDir, nsName) - // Recursively remount /run/netns on itself. The recursive flag is - // so that any existing netns bindmounts are carried over. - err = unix.Mount(nsRunDir, nsRunDir, "none", unix.MS_BIND|unix.MS_REC, "") + // create an empty file to use as at the mount point + err = createNetnsFile(nsPath) if err != nil { - return nil, fmt.Errorf("mount --rbind %s %s failed: %q", nsRunDir, nsRunDir, err) + // retry when the name already exists + if errors.Is(err, os.ErrExist) { + continue + } + return "", err } - // Now we can make it shared - err = unix.Mount("", nsRunDir, "none", unix.MS_SHARED|unix.MS_REC, "") + err = unix.Mount(fromNetns, nsPath, "none", unix.MS_BIND|unix.MS_SHARED|unix.MS_REC, "") if err != nil { - return nil, fmt.Errorf("mount --make-rshared %s failed: %q", nsRunDir, err) + // Do not leak the ns on errors + _ = os.RemoveAll(nsPath) + return "", fmt.Errorf("failed to bind mount ns at %s: %v", nsPath, err) } + return nsPath, nil } - nsPath := path.Join(nsRunDir, name) - return newNSPath(nsPath) + return "", errNoFreeName } -func newNSPath(nsPath string) (ns.NetNS, error) { - // create an empty file at the mount point - mountPointFd, err := os.OpenFile(nsPath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o600) +func getRandomNetnsName() (string, error) { + b := make([]byte, 16) + _, err := rand.Reader.Read(b) if err != nil { - return nil, err + return "", fmt.Errorf("failed to generate random netns name: %v", err) } - if err := mountPointFd.Close(); err != nil { - return nil, err + return fmt.Sprintf("netns-%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:]), nil +} + +func makeNetnsDir(nsRunDir string) error { + err := os.MkdirAll(nsRunDir, 0o755) + if err != nil { + return err + } + // Important, the bind mount setup is racy if two process try to set it up in parallel. + // This can have very bad consequences because we end up with two duplicated mounts + // for the netns file that then might have a different parent mounts. + // Also because as root netns dir is also created by ip netns we should not race against them. + // Use a lock on the netns dir like they do, compare the iproute2 ip netns add code. + // https://github.com/iproute2/iproute2/blob/8b9d9ea42759c91d950356ca43930a975d0c352b/ip/ipnetns.c#L806-L815 + + dirFD, err := unix.Open(nsRunDir, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) + if err != nil { + return &os.PathError{Op: "open", Path: nsRunDir, Err: err} + } + // closing the fd will also unlock so we do not have to call flock(fd,LOCK_UN) + defer unix.Close(dirFD) + + err = unix.Flock(dirFD, unix.LOCK_EX) + if err != nil { + return fmt.Errorf("failed to lock %s dir: %w", nsRunDir, err) + } + + // Remount the namespace directory shared. This will fail with EINVAL + // if it is not already a mountpoint, so bind-mount it on to itself + // to "upgrade" it to a mountpoint. + err = unix.Mount("", nsRunDir, "none", unix.MS_SHARED|unix.MS_REC, "") + if err == nil { + return nil + } + if err != unix.EINVAL { + return fmt.Errorf("mount --make-rshared %s failed: %q", nsRunDir, err) + } + + // Recursively remount /run/netns on itself. The recursive flag is + // so that any existing netns bindmounts are carried over. + err = unix.Mount(nsRunDir, nsRunDir, "none", unix.MS_BIND|unix.MS_REC, "") + if err != nil { + return fmt.Errorf("mount --rbind %s %s failed: %q", nsRunDir, nsRunDir, err) } + // Now we can make it shared + err = unix.Mount("", nsRunDir, "none", unix.MS_SHARED|unix.MS_REC, "") + if err != nil { + return fmt.Errorf("mount --make-rshared %s failed: %q", nsRunDir, err) + } + + return nil +} + +// createNetnsFile created the file with O_EXCL to ensure there are no conflicts with others +// Callers should check for ErrExist and loop over it to find a free file. +func createNetnsFile(path string) error { + mountPointFd, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o600) + if err != nil { + return err + } + return mountPointFd.Close() +} + +func newNSPath(nsPath string) (ns.NetNS, error) { + // create an empty file to use as at the mount point + err := createNetnsFile(nsPath) + if err != nil { + return nil, err + } // Ensure the mount point is cleaned up on errors; if the namespace // was successfully mounted this will have no effect because the file // is in-use diff --git a/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go b/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go index ded66365bb4..a6538ffb908 100644 --- a/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go +++ b/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go @@ -11,6 +11,7 @@ import ( "github.com/containers/common/pkg/umask" "github.com/containers/storage/pkg/fileutils" "github.com/containers/storage/pkg/idtools" + securejoin "github.com/cyphar/filepath-securejoin" rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux/label" "github.com/sirupsen/logrus" @@ -346,7 +347,10 @@ func addFIPSModeSubscription(mounts *[]rspec.Mount, containerRunDir, mountPoint, srcBackendDir := "/usr/share/crypto-policies/back-ends/FIPS" destDir := "/etc/crypto-policies/back-ends" - srcOnHost := filepath.Join(mountPoint, srcBackendDir) + srcOnHost, err := securejoin.SecureJoin(mountPoint, srcBackendDir) + if err != nil { + return fmt.Errorf("resolve %s in the container: %w", srcBackendDir, err) + } if err := fileutils.Exists(srcOnHost); err != nil { if errors.Is(err, os.ErrNotExist) { return nil diff --git a/vendor/github.com/containers/common/version/version.go b/vendor/github.com/containers/common/version/version.go index 4867ade0bf9..8f30e468817 100644 --- a/vendor/github.com/containers/common/version/version.go +++ b/vendor/github.com/containers/common/version/version.go @@ -1,4 +1,4 @@ package version // Version is the version of the build. -const Version = "0.60.2" +const Version = "0.60.4" diff --git a/vendor/modules.txt b/vendor/modules.txt index 050df38631d..c5d2c828d66 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -107,7 +107,7 @@ github.com/containernetworking/cni/pkg/version # github.com/containernetworking/plugins v1.5.1 ## explicit; go 1.20 github.com/containernetworking/plugins/pkg/ns -# github.com/containers/common v0.60.2 +# github.com/containers/common v0.60.4 ## explicit; go 1.21.0 github.com/containers/common/internal github.com/containers/common/internal/attributedstring