Skip to content

Commit

Permalink
Fix rootlesskit port forwarder with custom slirp cidr
Browse files Browse the repository at this point in the history
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 containers#9828

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
Paul Holzinger committed Apr 23, 2021
1 parent 8465626 commit f99b7a3
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 32 deletions.
2 changes: 1 addition & 1 deletion libpod/networking_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
14 changes: 12 additions & 2 deletions libpod/networking_slirp4netns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand Down
70 changes: 41 additions & 29 deletions test/system/500-networking.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit f99b7a3

Please sign in to comment.