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 <pholzing@redhat.com>

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"