From f99b7a314bf6d285b92a528d1a0800c35dfdc603 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 5 Apr 2021 21:02:21 +0200 Subject: [PATCH] Fix rootlesskit port forwarder with custom slirp cidr The source ip for the rootlesskit port forwarder was hardcoded to the standard slirp4netns ip. This is incorrect since users can change the subnet used by slirp4netns with `--network slirp4netns:cidr=10.5.0.0/24`. The container interface ip is always the .100 in the subnet. Only when the rootlesskit port forwarder child ip matches the container interface ip the port forwarding will work. Fixes #9828 Signed-off-by: Paul Holzinger --- libpod/networking_linux.go | 2 +- libpod/networking_slirp4netns.go | 14 ++++++- test/system/500-networking.bats | 70 +++++++++++++++++++------------- 3 files changed, 54 insertions(+), 32 deletions(-) diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 6e2c2880f4..cfed5a1f2c 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -577,7 +577,7 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) error { // set up port forwarder for CNI-in-slirp4netns netnsPath := ctr.state.NetNS.Path() // TODO: support slirp4netns port forwarder as well - return r.setupRootlessPortMappingViaRLK(ctr, netnsPath) + return r.setupRootlessPortMappingViaRLK(ctr, netnsPath, "") } return nil } diff --git a/libpod/networking_slirp4netns.go b/libpod/networking_slirp4netns.go index 72ab3c919b..c46dc6972c 100644 --- a/libpod/networking_slirp4netns.go +++ b/libpod/networking_slirp4netns.go @@ -312,7 +312,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, netOptions.cidr) } return nil } @@ -363,7 +363,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, slirp4CIDR string) error { syncR, syncW, err := os.Pipe() if err != nil { return errors.Wrapf(err, "failed to open pipe") @@ -391,6 +391,16 @@ func (r *Runtime) setupRootlessPortMappingViaRLK(ctr *Container, netnsPath strin } childIP := slirp4netnsIP + // set the correct childIP when a custom cidr is set + if slirp4CIDR != "" { + _, cidr, err := net.ParseCIDR(slirp4CIDR) + if err != nil { + return errors.Wrap(err, "failed to parse slirp4netns cidr") + } + // the slirp container ip is always the hundredth ip in the subnet + cidr.IP[len(cidr.IP)-1] = cidr.IP[len(cidr.IP)-1] + 100 + childIP = cidr.IP.String() + } outer: for _, r := range ctr.state.NetworkStatus { for _, i := range r.IPs { diff --git a/test/system/500-networking.bats b/test/system/500-networking.bats index 8da8647980..21240e5210 100644 --- a/test/system/500-networking.bats +++ b/test/system/500-networking.bats @@ -77,35 +77,47 @@ load helpers # FIXME: randomize port, and create second random host port myport=54321 - # Container will exit as soon as 'nc' receives input - # We use '-n -v' to give us log messages showing an incoming connection - # and its IP address; the purpose of that is guaranteeing that the - # remote IP is not 127.0.0.1 (podman PR #9052). - # We could get more parseable output by using $NCAT_REMOTE_ADDR, - # but busybox nc doesn't support that. - run_podman run -d --userns=keep-id -p 127.0.0.1:$myport:$myport \ - $IMAGE nc -l -n -v -p $myport - cid="$output" - - # emit random string, and check it - teststring=$(random_string 30) - echo "$teststring" | nc 127.0.0.1 $myport - - run_podman logs $cid - # Sigh. We can't check line-by-line, because 'nc' output order is - # unreliable. We usually get the 'connect to' line before the random - # string, but sometimes we get it after. So, just do substring checks. - is "$output" ".*listening on \[::\]:$myport .*" "nc -v shows right port" - - # This is the truly important check: make sure the remote IP is - # in the 10.X range, not 127.X. - is "$output" \ - ".*connect to \[::ffff:10\..*\]:$myport from \[::ffff:10\..*\]:.*" \ - "nc -v shows remote IP address in 10.X space (not 127.0.0.1)" - is "$output" ".*${teststring}.*" "test string received on container" - - # Clean up - run_podman rm $cid + for cidr in "" "$(random_rfc1918_subnet).0/24"; do + myport=$(( myport + 1 )) + if [[ -z $cidr ]]; then + # regex to match that we are in 10.X subnet + match="10\..*" + else + # Issue #9828 make sure a custom slir4netns cidr also works + network_arg="--network slirp4netns:cidr=$cidr" + # slirp4netns interface ip is always .100 + match="${cidr%.*}.100" + fi + + # Container will exit as soon as 'nc' receives input + # We use '-n -v' to give us log messages showing an incoming connection + # and its IP address; the purpose of that is guaranteeing that the + # remote IP is not 127.0.0.1 (podman PR #9052). + # We could get more parseable output by using $NCAT_REMOTE_ADDR, + # but busybox nc doesn't support that. + run_podman run -d --userns=keep-id $network_arg -p 127.0.0.1:$myport:$myport \ + $IMAGE nc -l -n -v -p $myport + cid="$output" + + # emit random string, and check it + teststring=$(random_string 30) + echo "$teststring" | nc 127.0.0.1 $myport + + run_podman logs $cid + # Sigh. We can't check line-by-line, because 'nc' output order is + # unreliable. We usually get the 'connect to' line before the random + # string, but sometimes we get it after. So, just do substring checks. + is "$output" ".*listening on \[::\]:$myport .*" "nc -v shows right port" + + # This is the truly important check: make sure the remote IP is not 127.X. + is "$output" \ + ".*connect to \[::ffff:$match*\]:$myport from \[::ffff:$match\]:.*" \ + "nc -v shows remote IP address is not 127.0.0.1" + is "$output" ".*${teststring}.*" "test string received on container" + + # Clean up + run_podman rm $cid + done } # "network create" now works rootless, with the help of a special container