From 216e2cb36679abfcca869bed110b73e816ff9bf4 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 8 Nov 2021 20:35:22 +0100 Subject: Fix rootless networking with userns and ports A rootless container created with a custom userns and forwarded ports did not work. I refactored the network setup to make the setup logic more clear. Signed-off-by: Paul Holzinger --- libpod/container_internal.go | 7 ++---- libpod/container_internal_linux.go | 9 -------- libpod/networking_linux.go | 45 +++++++++++++++++++------------------- libpod/networking_slirp4netns.go | 21 +++++++++--------- test/system/500-networking.bats | 3 +++ 5 files changed, 37 insertions(+), 48 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index fbc2c1f38..64fe99132 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -290,7 +290,7 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (_ bool, retErr err // setup slirp4netns again because slirp4netns will die when conmon exits if c.config.NetMode.IsSlirp4netns() { - err := c.runtime.setupSlirp4netns(c) + err := c.runtime.setupSlirp4netns(c, c.state.NetNS) if err != nil { return false, err } @@ -299,7 +299,7 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (_ bool, retErr err // setup rootlesskit port forwarder again since it dies when conmon exits // we use rootlesskit port forwarder only as rootless and when bridge network is used if rootless.IsRootless() && c.config.NetMode.IsBridge() && len(c.config.PortMappings) > 0 { - err := c.runtime.setupRootlessPortMappingViaRLK(c, c.state.NetNS.Path()) + err := c.runtime.setupRootlessPortMappingViaRLK(c, c.state.NetNS.Path(), c.state.NetworkStatus) if err != nil { return false, err } @@ -999,9 +999,6 @@ func (c *Container) completeNetworkSetup() error { if err := c.syncContainer(); err != nil { return err } - if c.config.NetMode.IsSlirp4netns() { - return c.runtime.setupSlirp4netns(c) - } if err := c.runtime.setupNetNS(c); err != nil { return err } diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 3187724ca..91453574e 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -108,15 +108,6 @@ func (c *Container) prepare() error { c.state.NetNS = netNS c.state.NetworkStatus = networkStatus } - - // handle rootless network namespace setup - if noNetNS && !c.config.PostConfigureNetNS { - if rootless.IsRootless() { - createNetNSErr = c.runtime.setupRootlessNetNS(c) - } else if c.config.NetMode.IsSlirp4netns() { - createNetNSErr = c.runtime.setupSlirp4netns(c) - } - } }() // Mount storage if not mounted go func() { diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index ef261a438..fc91155fa 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -651,6 +651,9 @@ func getCNIPodName(c *Container) string { // Create and configure a new network namespace for a container func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) (map[string]types.StatusBlock, error) { + if ctr.config.NetMode.IsSlirp4netns() { + return nil, r.setupSlirp4netns(ctr, ctrNS) + } networks, _, err := ctr.networks() if err != nil { return nil, err @@ -665,7 +668,24 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) (map[string]typ if err != nil { return nil, err } - return r.setUpNetwork(ctrNS.Path(), netOpts) + netStatus, err := r.setUpNetwork(ctrNS.Path(), netOpts) + if err != nil { + return nil, err + } + + // setup rootless port forwarder when rootless with ports and the network status is empty, + // if this is called from network reload the network status will not be empty and we should + // not setup port because they are still active + if rootless.IsRootless() && len(ctr.config.PortMappings) > 0 && ctr.getNetworkStatus() == nil { + // set up port forwarder for rootless netns + netnsPath := ctrNS.Path() + // TODO: support slirp4netns port forwarder as well + // make sure to fix this in container.handleRestartPolicy() as well + // Important we have to call this after r.setUpNetwork() so that + // we can use the proper netStatus + err = r.setupRootlessPortMappingViaRLK(ctr, netnsPath, netStatus) + } + return netStatus, err } // Create and configure a new network namespace for a container @@ -688,31 +708,10 @@ func (r *Runtime) createNetNS(ctr *Container) (n ns.NetNS, q map[string]types.St logrus.Debugf("Made network namespace at %s for container %s", ctrNS.Path(), ctr.ID()) var networkStatus map[string]types.StatusBlock - if !ctr.config.NetMode.IsSlirp4netns() { - networkStatus, err = r.configureNetNS(ctr, ctrNS) - } + networkStatus, err = r.configureNetNS(ctr, ctrNS) return ctrNS, networkStatus, err } -// Configure the network namespace for a rootless container -func (r *Runtime) setupRootlessNetNS(ctr *Container) error { - if ctr.config.NetMode.IsSlirp4netns() { - return r.setupSlirp4netns(ctr) - } - networks, _, err := ctr.networks() - if err != nil { - return err - } - if len(networks) > 0 && len(ctr.config.PortMappings) > 0 { - // set up port forwarder for rootless netns - netnsPath := ctr.state.NetNS.Path() - // TODO: support slirp4netns port forwarder as well - // make sure to fix this in container.handleRestartPolicy() as well - return r.setupRootlessPortMappingViaRLK(ctr, netnsPath) - } - return nil -} - // 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) diff --git a/libpod/networking_slirp4netns.go b/libpod/networking_slirp4netns.go index 760427f22..9da94fb44 100644 --- a/libpod/networking_slirp4netns.go +++ b/libpod/networking_slirp4netns.go @@ -17,6 +17,7 @@ import ( "time" "github.com/containernetworking/plugins/pkg/ns" + "github.com/containers/podman/v3/libpod/network/types" "github.com/containers/podman/v3/pkg/errorhandling" "github.com/containers/podman/v3/pkg/rootless" "github.com/containers/podman/v3/pkg/rootlessport" @@ -207,7 +208,7 @@ func createBasicSlirp4netnsCmdArgs(options *slirp4netnsNetworkOptions, features } // setupSlirp4netns can be called in rootful as well as in rootless -func (r *Runtime) setupSlirp4netns(ctr *Container) error { +func (r *Runtime) setupSlirp4netns(ctr *Container, netns ns.NetNS) error { path := r.config.Engine.NetworkCmdPath if path == "" { var err error @@ -263,7 +264,7 @@ func (r *Runtime) setupSlirp4netns(ctr *Container) error { if err != nil { return errors.Wrapf(err, "failed to create rootless network sync pipe") } - netnsPath = ctr.state.NetNS.Path() + netnsPath = netns.Path() cmdArgs = append(cmdArgs, "--netns-type=path", netnsPath, "tap0") } else { defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncR) @@ -366,7 +367,7 @@ func (r *Runtime) setupSlirp4netns(ctr *Container) error { if netOptions.isSlirpHostForward { return r.setupRootlessPortMappingViaSlirp(ctr, cmd, apiSocket) } - return r.setupRootlessPortMappingViaRLK(ctr, netnsPath) + return r.setupRootlessPortMappingViaRLK(ctr, netnsPath, nil) } return nil @@ -479,7 +480,7 @@ func waitForSync(syncR *os.File, cmd *exec.Cmd, logFile io.ReadSeeker, timeout t return nil } -func (r *Runtime) setupRootlessPortMappingViaRLK(ctr *Container, netnsPath string) error { +func (r *Runtime) setupRootlessPortMappingViaRLK(ctr *Container, netnsPath string, netStatus map[string]types.StatusBlock) error { syncR, syncW, err := os.Pipe() if err != nil { return errors.Wrapf(err, "failed to open pipe") @@ -506,7 +507,7 @@ func (r *Runtime) setupRootlessPortMappingViaRLK(ctr *Container, netnsPath strin } } - childIP := getRootlessPortChildIP(ctr) + childIP := getRootlessPortChildIP(ctr, netStatus) cfg := rootlessport.Config{ Mappings: ctr.config.PortMappings, NetNSPath: netnsPath, @@ -531,9 +532,7 @@ func (r *Runtime) setupRootlessPortMappingViaRLK(ctr *Container, netnsPath strin cmd.Args = []string{rootlessport.BinaryName} // Leak one end of the pipe in rootlessport process, the other will be sent to conmon - if ctr.rootlessPortSyncR != nil { - defer errorhandling.CloseQuiet(ctr.rootlessPortSyncR) - } + defer errorhandling.CloseQuiet(ctr.rootlessPortSyncR) cmd.ExtraFiles = append(cmd.ExtraFiles, ctr.rootlessPortSyncR, syncW) cmd.Stdin = cfgR @@ -649,7 +648,7 @@ func (r *Runtime) setupRootlessPortMappingViaSlirp(ctr *Container, cmd *exec.Cmd return nil } -func getRootlessPortChildIP(c *Container) string { +func getRootlessPortChildIP(c *Container, netStatus map[string]types.StatusBlock) string { if c.config.NetMode.IsSlirp4netns() { slirp4netnsIP, err := GetSlirp4netnsIP(c.slirp4netnsSubnet) if err != nil { @@ -659,7 +658,7 @@ func getRootlessPortChildIP(c *Container) string { } var ipv6 net.IP - for _, status := range c.getNetworkStatus() { + for _, status := range netStatus { for _, netInt := range status.Interfaces { for _, netAddress := range netInt.Networks { ipv4 := netAddress.Subnet.IP.To4() @@ -679,7 +678,7 @@ func getRootlessPortChildIP(c *Container) string { // reloadRootlessRLKPortMapping will trigger a reload for the port mappings in the rootlessport process. // This should only be called by network connect/disconnect and only as rootless. func (c *Container) reloadRootlessRLKPortMapping() error { - childIP := getRootlessPortChildIP(c) + childIP := getRootlessPortChildIP(c, c.state.NetworkStatus) logrus.Debugf("reloading rootless ports for container %s, childIP is %s", c.config.ID, childIP) conn, err := openUnixSocket(filepath.Join(c.runtime.config.Engine.TmpDir, "rp", c.config.ID)) diff --git a/test/system/500-networking.bats b/test/system/500-networking.bats index b3471b425..815c77609 100644 --- a/test/system/500-networking.bats +++ b/test/system/500-networking.bats @@ -78,6 +78,9 @@ load helpers if [[ -z $cidr ]]; then # regex to match that we are in 10.X subnet match="10\..*" + # force bridge networking also for rootless + # this ensures that rootless + bridge + userns + ports works + network_arg="--network bridge" else # Issue #9828 make sure a custom slir4netns cidr also works network_arg="--network slirp4netns:cidr=$cidr" -- cgit v1.2.3-54-g00ecf