From 5cb3a5e89704ec26f72ab69d670aaf83f02b9205 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Thu, 21 May 2020 12:55:45 +0200 Subject: [PATCH] portmap: don't use unspecified address as destination It may happen that you want to map a port only in one IP family. It can be achieved using the unspecified IP address of the corresponding IP family as HostIP i.e.: podman run --rm --name some-nginx -d -p 0.0.0.0:8080:80 nginx The problem is that current implementation considers the unspecified address valid and appends it to the iptables rule: -A CNI-DN-60380cb3197c5457ed6ba -s 10.88.0.0/16 -d 0.0.0.0/32 -p tcp -m tcp --dport 8080 -j CNI-HOSTPORT-SETMARK This rule is not forwarding the traffic to the mapped port. We should use the unspecified address only to discriminate the IP family of the port mapping, but not use it to filter the dst. Signed-off-by: Antonio Ojea --- plugins/meta/portmap/portmap.go | 9 ++++++++- plugins/meta/portmap/portmap_test.go | 28 ++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/plugins/meta/portmap/portmap.go b/plugins/meta/portmap/portmap.go index 738b3112..f4e28774 100644 --- a/plugins/meta/portmap/portmap.go +++ b/plugins/meta/portmap/portmap.go @@ -225,6 +225,8 @@ func fillDnatRules(c *chain, config *PortMapConf, containerNet net.IPNet) { c.rules = make([][]string, 0, 3*len(entries)) for _, entry := range entries { // If a HostIP is given, only process the entry if host and container address families match + // and append it to the iptables rules + addRuleBaseDst := false if entry.HostIP != "" { hostIP := net.ParseIP(entry.HostIP) isHostV6 := (hostIP.To4() == nil) @@ -232,12 +234,17 @@ func fillDnatRules(c *chain, config *PortMapConf, containerNet net.IPNet) { if isV6 != isHostV6 { continue } + + // Unspecified addresses can not be used as destination + if !hostIP.IsUnspecified() { + addRuleBaseDst = true + } } ruleBase := []string{ "-p", entry.Protocol, "--dport", strconv.Itoa(entry.HostPort)} - if entry.HostIP != "" { + if addRuleBaseDst { ruleBase = append(ruleBase, "-d", entry.HostIP) } diff --git a/plugins/meta/portmap/portmap_test.go b/plugins/meta/portmap/portmap_test.go index efd4d2f7..12df96c5 100644 --- a/plugins/meta/portmap/portmap_test.go +++ b/plugins/meta/portmap/portmap_test.go @@ -170,7 +170,11 @@ var _ = Describe("portmapping configuration", func() { { "hostPort": 8080, "containerPort": 80, "protocol": "tcp"}, { "hostPort": 8081, "containerPort": 80, "protocol": "tcp"}, { "hostPort": 8080, "containerPort": 81, "protocol": "udp"}, - { "hostPort": 8082, "containerPort": 82, "protocol": "udp"} + { "hostPort": 8082, "containerPort": 82, "protocol": "udp"}, + { "hostPort": 8083, "containerPort": 83, "protocol": "tcp", "hostIP": "192.168.0.2"}, + { "hostPort": 8084, "containerPort": 84, "protocol": "tcp", "hostIP": "0.0.0.0"}, + { "hostPort": 8085, "containerPort": 85, "protocol": "tcp", "hostIP": "2001:db8:a::1"}, + { "hostPort": 8086, "containerPort": 86, "protocol": "tcp", "hostIP": "::"} ] }, "snat": true, @@ -197,7 +201,7 @@ var _ = Describe("portmapping configuration", func() { fmt.Sprintf("dnat name: \"test\" id: \"%s\"", containerID), "-m", "multiport", "-p", "tcp", - "--destination-ports", "8080,8081", + "--destination-ports", "8080,8081,8083,8084,8085,8086", "a", "b"}, {"-m", "comment", "--comment", fmt.Sprintf("dnat name: \"test\" id: \"%s\"", containerID), @@ -208,18 +212,28 @@ var _ = Describe("portmapping configuration", func() { })) Expect(ch.rules).To(Equal([][]string{ + // tcp rules and not hostIP {"-p", "tcp", "--dport", "8080", "-s", "10.0.0.2/24", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "tcp", "--dport", "8080", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, {"-p", "tcp", "--dport", "8081", "-s", "10.0.0.2/24", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "tcp", "--dport", "8081", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "tcp", "--dport", "8081", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, + // udp rules and not hostIP {"-p", "udp", "--dport", "8080", "-s", "10.0.0.2/24", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "udp", "--dport", "8080", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "udp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:81"}, {"-p", "udp", "--dport", "8082", "-s", "10.0.0.2/24", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "udp", "--dport", "8082", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "udp", "--dport", "8082", "-j", "DNAT", "--to-destination", "10.0.0.2:82"}, + // tcp rules and hostIP + {"-p", "tcp", "--dport", "8083", "-d", "192.168.0.2", "-s", "10.0.0.2/24", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8083", "-d", "192.168.0.2", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8083", "-d", "192.168.0.2", "-j", "DNAT", "--to-destination", "10.0.0.2:83"}, + // tcp rules and hostIP = "0.0.0.0" + {"-p", "tcp", "--dport", "8084", "-s", "10.0.0.2/24", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8084", "-s", "127.0.0.1", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8084", "-j", "DNAT", "--to-destination", "10.0.0.2:84"}, })) ch.rules = nil @@ -229,14 +243,22 @@ var _ = Describe("portmapping configuration", func() { fillDnatRules(&ch, conf, *n) Expect(ch.rules).To(Equal([][]string{ + // tcp rules and not hostIP {"-p", "tcp", "--dport", "8080", "-s", "2001:db8::2/64", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "[2001:db8::2]:80"}, {"-p", "tcp", "--dport", "8081", "-s", "2001:db8::2/64", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "tcp", "--dport", "8081", "-j", "DNAT", "--to-destination", "[2001:db8::2]:80"}, + // udp rules and not hostIP {"-p", "udp", "--dport", "8080", "-s", "2001:db8::2/64", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "udp", "--dport", "8080", "-j", "DNAT", "--to-destination", "[2001:db8::2]:81"}, {"-p", "udp", "--dport", "8082", "-s", "2001:db8::2/64", "-j", "CNI-HOSTPORT-SETMARK"}, {"-p", "udp", "--dport", "8082", "-j", "DNAT", "--to-destination", "[2001:db8::2]:82"}, + // tcp rules and hostIP + {"-p", "tcp", "--dport", "8085", "-d", "2001:db8:a::1", "-s", "2001:db8::2/64", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8085", "-d", "2001:db8:a::1", "-j", "DNAT", "--to-destination", "[2001:db8::2]:85"}, + // tcp rules and hostIP = "::" + {"-p", "tcp", "--dport", "8086", "-s", "2001:db8::2/64", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-p", "tcp", "--dport", "8086", "-j", "DNAT", "--to-destination", "[2001:db8::2]:86"}, })) // Disable snat, generate rules @@ -252,6 +274,8 @@ var _ = Describe("portmapping configuration", func() { {"-p", "tcp", "--dport", "8081", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, {"-p", "udp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:81"}, {"-p", "udp", "--dport", "8082", "-j", "DNAT", "--to-destination", "10.0.0.2:82"}, + {"-p", "tcp", "--dport", "8083", "-d", "192.168.0.2", "-j", "DNAT", "--to-destination", "10.0.0.2:83"}, + {"-p", "tcp", "--dport", "8084", "-j", "DNAT", "--to-destination", "10.0.0.2:84"}, })) })