diff --git a/docs/source/conf.py b/docs/source/conf.py index 2ea3e4f327..a5a271d8d5 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -18,6 +18,15 @@ import re import os import subprocess +# Define the canonical URL for our custom docs.podman.io domain configured on Read the Docs +html_baseurl = os.environ.get("READTHEDOCS_CANONICAL_URL", "") + +# Tell Jinja2 templates the build is running on Read the Docs +if os.environ.get("READTHEDOCS", "") == "True": + if "html_context" not in globals(): + html_context = {} + html_context["READTHEDOCS"] = True + # We have to run the preprocessor to create the actual markdown files from .in files. # Do it here so the it can work on readthedocs as well. path = os.path.join(os.path.abspath(os.path.dirname( diff --git a/go.mod b/go.mod index 22a06ceb17..5024121123 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/checkpoint-restore/go-criu/v7 v7.1.0 github.com/containernetworking/plugins v1.5.1 github.com/containers/buildah v1.37.2 - github.com/containers/common v0.60.2 + github.com/containers/common v0.60.3 github.com/containers/conmon v2.0.20+incompatible github.com/containers/gvisor-tap-vsock v0.7.4 github.com/containers/image/v5 v5.32.2 diff --git a/go.sum b/go.sum index f5f406fd67..963da52776 100644 --- a/go.sum +++ b/go.sum @@ -79,8 +79,8 @@ github.com/containernetworking/plugins v1.5.1 h1:T5ji+LPYjjgW0QM+KyrigZbLsZ8jaX+ github.com/containernetworking/plugins v1.5.1/go.mod h1:MIQfgMayGuHYs0XdNudf31cLLAC+i242hNm6KuDGqCM= github.com/containers/buildah v1.37.2 h1:KiJ3jVNUvdtGORxDz8fjjLkR81ZHQZIfnGWJWavks40= github.com/containers/buildah v1.37.2/go.mod h1:alFCM3X0xfhE6ZjsFQkUlOMyKzOnbv9FL9fe1Ho48PA= -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.3 h1:pToT7gtFx/KWyMtWw98g4pIbW54i9KfGH2QrdN2s1io= +github.com/containers/common v0.60.3/go.mod h1:I0upBi1qJX3QmzGbUOBN1LVP6RvkKhd3qQpZbQT+Q54= github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg= github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I= github.com/containers/gvisor-tap-vsock v0.7.4 h1:iOtr/KEi+r599OOx1+9Qbss91jD5yxh1HO35MKTdths= diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 47f2401c4c..f95644a8a3 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -2900,8 +2900,10 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error { uid := int(c.config.Spec.Process.User.UID) gid := int(c.config.Spec.Process.User.GID) + idmapped := hasIdmapOption(v.Options) + // if the volume is mounted with "idmap", leave the IDs in from the current environment. - if c.config.IDMappings.UIDMap != nil && !hasIdmapOption(v.Options) { + if c.config.IDMappings.UIDMap != nil && !idmapped { p := idtools.IDPair{ UID: uid, GID: gid, @@ -2947,7 +2949,8 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error { if stat, ok := st.Sys().(*syscall.Stat_t); ok { uid, gid := int(stat.Uid), int(stat.Gid) - if c.config.IDMappings.UIDMap != nil { + // If the volume is idmapped then undo the conversion to obtain the desired UID/GID in the container + if c.config.IDMappings.UIDMap != nil && idmapped { p := idtools.IDPair{ UID: uid, GID: gid, diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 3ffc9d7159..9ffdc1d0c1 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -3,11 +3,8 @@ package libpod import ( - "crypto/rand" "fmt" "net" - "os" - "path/filepath" "github.com/containernetworking/plugins/pkg/ns" "github.com/containers/common/libnetwork/types" @@ -17,7 +14,6 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" "github.com/vishvananda/netlink" - "golang.org/x/sys/unix" ) // Create and configure a new network namespace for a container @@ -104,33 +100,10 @@ func (r *Runtime) createNetNS(ctr *Container) (n string, q map[string]types.Stat // Configure the network namespace using the container process func (r *Runtime) setupNetNS(ctr *Container) error { nsProcess := fmt.Sprintf("/proc/%d/ns/net", ctr.state.PID) - - b := make([]byte, 16) - - if _, err := rand.Reader.Read(b); err != nil { - return fmt.Errorf("failed to generate random netns name: %w", err) - } - nsPath, err := netns.GetNSRunDir() + nsPath, err := netns.NewNSFrom(nsProcess) if err != nil { return err } - nsPath = filepath.Join(nsPath, fmt.Sprintf("netns-%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:])) - - if err := os.MkdirAll(filepath.Dir(nsPath), 0711); err != nil { - return err - } - - mountPointFd, err := os.Create(nsPath) - if err != nil { - return err - } - if err := mountPointFd.Close(); err != nil { - return err - } - - if err := unix.Mount(nsProcess, nsPath, "none", unix.MS_BIND, ""); err != nil { - return fmt.Errorf("cannot mount %s: %w", nsPath, err) - } networkStatus, err := r.configureNetNS(ctr, nsPath) diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 3db6d6aada..a3f26cc527 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -513,16 +513,11 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai volOptions = append(volOptions, withSetAnon()) } - needsChown := true - // If volume-opts are set, parse and add driver opts. if len(vol.Options) > 0 { isDriverOpts := false driverOpts := make(map[string]string) for _, opts := range vol.Options { - if opts == "idmap" { - needsChown = false - } if strings.HasPrefix(opts, "volume-opt") { isDriverOpts = true driverOptKey, driverOptValue, err := util.ParseDriverOpts(opts) @@ -538,11 +533,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai } } - if needsChown { - volOptions = append(volOptions, WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID())) - } else { - volOptions = append(volOptions, WithVolumeNoChown()) - } + volOptions = append(volOptions, WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID())) _, err = r.newVolume(ctx, false, volOptions...) if err != nil { diff --git a/pkg/specgen/generate/ports.go b/pkg/specgen/generate/ports.go index 0575fc7c5e..a67419d861 100644 --- a/pkg/specgen/generate/ports.go +++ b/pkg/specgen/generate/ports.go @@ -158,7 +158,7 @@ func ParsePortMapping(portMappings []types.PortMapping, exposePorts map[uint16][ // First, we need to validate the ports passed in the specgen for _, port := range portMappings { // First, check proto - protocols, err := checkProtocol(port.Protocol, true) + protocols, err := checkProtocol(port.Protocol) if err != nil { return nil, err } @@ -356,7 +356,7 @@ func createPortMappings(s *specgen.SpecGenerator, imageData *libimage.ImageData) if port == 0 { return nil, nil, fmt.Errorf("cannot expose 0 as it is not a valid port number") } - protocols, err := checkProtocol(proto, false) + protocols, err := checkProtocol(proto) if err != nil { return nil, nil, fmt.Errorf("validating protocols for exposed port %d: %w", port, err) } @@ -377,7 +377,7 @@ func createPortMappings(s *specgen.SpecGenerator, imageData *libimage.ImageData) } // Check a string to ensure it is a comma-separated set of valid protocols -func checkProtocol(protocol string, allowSCTP bool) ([]string, error) { +func checkProtocol(protocol string) ([]string, error) { protocols := make(map[string]struct{}) splitProto := strings.Split(protocol, ",") // Don't error on duplicates - just deduplicate @@ -389,9 +389,6 @@ func checkProtocol(protocol string, allowSCTP bool) ([]string, error) { case protoUDP: protocols[protoUDP] = struct{}{} case protoSCTP: - if !allowSCTP { - return nil, fmt.Errorf("protocol SCTP is not allowed for exposed ports") - } protocols[protoSCTP] = struct{}{} default: return nil, fmt.Errorf("unrecognized protocol %q in port mapping", p) diff --git a/pkg/specgen/namespaces.go b/pkg/specgen/namespaces.go index f685ab3e8c..e5b99f89d9 100644 --- a/pkg/specgen/namespaces.go +++ b/pkg/specgen/namespaces.go @@ -11,9 +11,9 @@ import ( "github.com/containers/common/pkg/cgroups" "github.com/containers/podman/v5/libpod/define" "github.com/containers/podman/v5/pkg/namespaces" - "github.com/containers/podman/v5/pkg/rootless" "github.com/containers/podman/v5/pkg/util" "github.com/containers/storage/pkg/fileutils" + "github.com/containers/storage/pkg/unshare" storageTypes "github.com/containers/storage/types" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" @@ -160,10 +160,15 @@ func validateNetNS(n *Namespace) error { case Slirp: break case Pasta: - if rootless.IsRootless() { + // Check if we run rootless/in a userns. Do not use rootless.IsRootless() here. + // Pasta switches to nobody when running as root which causes it to fail while + // opening the netns owned by root. However when pasta is already in a userns + // it doesn't switch to nobody so it works there. + // https://github.com/containers/podman/issues/17840 + if unshare.IsRootless() { break } - return fmt.Errorf("pasta networking is only supported for rootless mode") + return fmt.Errorf("pasta networking is only supported for rootless mode or when inside a nested userns") case "", Default, Host, Path, FromContainer, FromPod, Private, NoNetwork, Bridge: break default: diff --git a/test/e2e/container_inspect_test.go b/test/e2e/container_inspect_test.go index 8e26eb0f25..37a682b4e6 100644 --- a/test/e2e/container_inspect_test.go +++ b/test/e2e/container_inspect_test.go @@ -28,19 +28,19 @@ var _ = Describe("Podman container inspect", func() { It("podman inspect shows exposed ports", func() { name := "testcon" - session := podmanTest.Podman([]string{"run", "-d", "--stop-timeout", "0", "--expose", "8787/udp", "--name", name, ALPINE, "sleep", "100"}) + session := podmanTest.Podman([]string{"run", "-d", "--stop-timeout", "0", "--expose", "8787/udp", "--expose", "99/sctp", "--name", name, ALPINE, "sleep", "100"}) session.WaitWithDefaultTimeout() Expect(session).Should(ExitCleanly()) data := podmanTest.InspectContainer(name) Expect(data).To(HaveLen(1)) Expect(data[0].NetworkSettings.Ports). - To(Equal(map[string][]define.InspectHostPort{"8787/udp": nil})) + To(Equal(map[string][]define.InspectHostPort{"8787/udp": nil, "99/sctp": nil})) session = podmanTest.Podman([]string{"ps", "--format", "{{.Ports}}"}) session.WaitWithDefaultTimeout() Expect(session).Should(ExitCleanly()) - Expect(session.OutputToString()).To(Equal("8787/udp")) + Expect(session.OutputToString()).To(Equal("99/sctp, 8787/udp")) }) It("podman inspect shows exposed ports on image", func() { diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 4080a15792..3a8ab9d5c5 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -1267,11 +1267,23 @@ EOF run_podman run --uidmap=0:1000:200 --rm --rootfs "$romount:idmap=uids=@2000-1-1;gids=@2000-1-1" stat -c %u:%g /testfile is "$output" "1:1" + # verify that copyup with an empty idmap volume maintains the original ownership with different mappings and --rootfs myvolume=my-volume-$(safename) run_podman volume create $myvolume mkdir $romount/volume - run_podman run --rm --uidmap=0:1000:10000 -v volume:/volume:idmap --rootfs $romount stat -c %u:%g /volume - is "$output" "0:0" + chown 1000:1000 $romount/volume + for FROM in 1000 2000; do + run_podman run --security-opt label=disable --rm --uidmap=0:$FROM:10000 -v $myvolume:/volume:idmap --rootfs $romount stat -c %u:%g /volume + is "$output" "0:0" + done + run_podman volume rm $myvolume + + # verify that copyup with an empty idmap volume maintains the original ownership with different mappings + myvolume=my-volume-$(safename) + for FROM in 1000 2000; do + run_podman run --rm --uidmap=0:$FROM:10000 -v $myvolume:/etc:idmap $IMAGE stat -c %u:%g /etc/passwd + is "$output" "0:0" + done run_podman volume rm $myvolume rm -rf $romount diff --git a/test/system/550-pause-process.bats b/test/system/550-pause-process.bats index cd38609306..69da5a1d8b 100644 --- a/test/system/550-pause-process.bats +++ b/test/system/550-pause-process.bats @@ -149,3 +149,22 @@ function _check_pause_process() { run_podman rm -f -t0 $cname1 } + +# regression test for https://issues.redhat.com/browse/RHEL-59620 +@test "rootless userns can unmount netns properly" { + skip_if_not_rootless "pause process is only used as rootless" + skip_if_remote "system migrate not supported via remote" + + # Use podman system migrate to stop the currently running pause process + run_podman system migrate + + # First run a container with a custom userns as this uses different netns setup logic. + local cname=c-$(safename) + run_podman run --userns keep-id --name $cname -d $IMAGE sleep 100 + + # Now run a "normal" container without userns + run_podman run --rm $IMAGE true + + # This used to hang trying to unmount the netns. + run_podman rm -f -t0 $cname +} 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 bbcedc0f6c..3a3372889a 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. + for i := 0; i < 10000; i++ { + nsName, err := getRandomNetnsName() + if err != nil { + return "", err + } + nsPath := filepath.Join(nsRunDir, nsName) + + // create an empty file to use as at the mount point + err = createNetnsFile(nsPath) + if err != nil { + // retry when the name already exists + if errors.Is(err, os.ErrExist) { + continue + } + return "", err + } + + err = unix.Mount(fromNetns, nsPath, "none", unix.MS_BIND|unix.MS_SHARED|unix.MS_REC, "") + if err != nil { + // 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 + } + + return "", errNoFreeName +} + +func getRandomNetnsName() (string, error) { + b := make([]byte, 16) + _, err := rand.Reader.Read(b) + if err != nil { + return "", fmt.Errorf("failed to generate random netns name: %v", 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 { - if err != unix.EINVAL { - return nil, 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 nil, 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 nil, fmt.Errorf("mount --make-rshared %s failed: %q", nsRunDir, err) - } + return fmt.Errorf("mount --make-rshared %s failed: %q", nsRunDir, err) } - nsPath := path.Join(nsRunDir, name) - return newNSPath(nsPath) + 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 at the mount point - mountPointFd, err := os.OpenFile(nsPath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o600) + // create an empty file to use as at the mount point + err := createNetnsFile(nsPath) if err != nil { return nil, err } - if err := mountPointFd.Close(); 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/version/version.go b/vendor/github.com/containers/common/version/version.go index 4867ade0bf..3703dc8d93 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.3" diff --git a/vendor/modules.txt b/vendor/modules.txt index 69255b2804..a0618d5e9e 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -170,7 +170,7 @@ github.com/containers/buildah/pkg/sshagent github.com/containers/buildah/pkg/util github.com/containers/buildah/pkg/volumes github.com/containers/buildah/util -# github.com/containers/common v0.60.2 +# github.com/containers/common v0.60.3 ## explicit; go 1.21.0 github.com/containers/common/internal github.com/containers/common/internal/attributedstring