From 5e0fbd8374a97c6c6cebbc528b64c6daae2f282d Mon Sep 17 00:00:00 2001 From: Alban Bedel Date: Tue, 31 Mar 2020 23:29:25 +0200 Subject: [PATCH] portmap: Apply the DNAT hairpin to the whole subnet The DNAT hairpin rule only allow the container itself to access the ports it is exposing thru the host IP. Other containers in the same subnet might also want to access this service via the host IP, so apply this rule to the whole subnet instead of just for the container. This is particularly useful with setups using a reverse proxy for https. With such a setup connections between containers (for ex. oauth2) have to downgrade to http, or need complex dns setup to make use of the internal IP of the reverse proxy. On the other hand going thru the host IP is easy as that is probably what the service name already resolve to. Signed-off-by: Alban Bedel -- v2: Fixed the tests v3: Updated iptables rules documentation in README.md v4: Fixed the network addresses in README.md to match iptables output --- plugins/meta/portmap/README.md | 6 ++--- plugins/meta/portmap/main.go | 22 ++++++++-------- plugins/meta/portmap/portmap.go | 22 ++++++++-------- plugins/meta/portmap/portmap_test.go | 39 ++++++++++++++++------------ 4 files changed, 48 insertions(+), 41 deletions(-) diff --git a/plugins/meta/portmap/README.md b/plugins/meta/portmap/README.md index 748c50807..c879b18bc 100644 --- a/plugins/meta/portmap/README.md +++ b/plugins/meta/portmap/README.md @@ -72,7 +72,7 @@ will masquerade traffic as needed. The DNAT rule rewrites the destination port and address of new connections. There is a top-level chain, `CNI-HOSTPORT-DNAT` which is always created and never deleted. Each plugin execution creates an additional chain for ease -of cleanup. So, if a single container exists on IP 172.16.30.2 with ports +of cleanup. So, if a single container exists on IP 172.16.30.2/24 with ports 8080 and 8043 on the host forwarded to ports 80 and 443 in the container, the rules look like this: @@ -86,10 +86,10 @@ rules look like this: - `-j MARK --set-xmark 0x2000/0x2000` `CNI-DN-xxxxxx` chain: -- `-p tcp -s 172.16.30.2 --dport 8080 -j CNI-HOSTPORT-SETMARK` (masquerade hairpin traffic) +- `-p tcp -s 172.16.30.0/24 --dport 8080 -j CNI-HOSTPORT-SETMARK` (masquerade hairpin traffic) - `-p tcp -s 127.0.0.1 --dport 8080 -j CNI-HOSTPORT-SETMARK` (masquerade localhost traffic) - `-p tcp --dport 8080 -j DNAT --to-destination 172.16.30.2:80` (rewrite destination) -- `-p tcp -s 172.16.30.2 --dport 8043 -j CNI-HOSTPORT-SETMARK` +- `-p tcp -s 172.16.30.0/24 --dport 8043 -j CNI-HOSTPORT-SETMARK` - `-p tcp -s 127.0.0.1 --dport 8043 -j CNI-HOSTPORT-SETMARK` - `-p tcp --dport 8043 -j DNAT --to-destination 172.16.30.2:443` diff --git a/plugins/meta/portmap/main.go b/plugins/meta/portmap/main.go index 1df147d95..4743a3702 100644 --- a/plugins/meta/portmap/main.go +++ b/plugins/meta/portmap/main.go @@ -60,9 +60,9 @@ type PortMapConf struct { // These are fields parsed out of the config or the environment; // included here for convenience - ContainerID string `json:"-"` - ContIPv4 net.IP `json:"-"` - ContIPv6 net.IP `json:"-"` + ContainerID string `json:"-"` + ContIPv4 net.IPNet `json:"-"` + ContIPv6 net.IPNet `json:"-"` } // The default mark bit to signal that masquerading is required @@ -85,13 +85,13 @@ func cmdAdd(args *skel.CmdArgs) error { netConf.ContainerID = args.ContainerID - if netConf.ContIPv4 != nil { + if netConf.ContIPv4.IP != nil { if err := forwardPorts(netConf, netConf.ContIPv4); err != nil { return err } } - if netConf.ContIPv6 != nil { + if netConf.ContIPv6.IP != nil { if err := forwardPorts(netConf, netConf.ContIPv6); err != nil { return err } @@ -138,13 +138,13 @@ func cmdCheck(args *skel.CmdArgs) error { conf.ContainerID = args.ContainerID - if conf.ContIPv4 != nil { + if conf.ContIPv4.IP != nil { if err := checkPorts(conf, conf.ContIPv4); err != nil { return err } } - if conf.ContIPv6 != nil { + if conf.ContIPv6.IP != nil { if err := checkPorts(conf, conf.ContIPv6); err != nil { return err } @@ -205,9 +205,9 @@ func parseConfig(stdin []byte, ifName string) (*PortMapConf, *current.Result, er if conf.PrevResult != nil { for _, ip := range result.IPs { - if ip.Version == "6" && conf.ContIPv6 != nil { + if ip.Version == "6" && conf.ContIPv6.IP != nil { continue - } else if ip.Version == "4" && conf.ContIPv4 != nil { + } else if ip.Version == "4" && conf.ContIPv4.IP != nil { continue } @@ -223,9 +223,9 @@ func parseConfig(stdin []byte, ifName string) (*PortMapConf, *current.Result, er } switch ip.Version { case "6": - conf.ContIPv6 = ip.Address.IP + conf.ContIPv6 = ip.Address case "4": - conf.ContIPv4 = ip.Address.IP + conf.ContIPv4 = ip.Address } } } diff --git a/plugins/meta/portmap/portmap.go b/plugins/meta/portmap/portmap.go index 16d7d428f..738b3112b 100644 --- a/plugins/meta/portmap/portmap.go +++ b/plugins/meta/portmap/portmap.go @@ -48,9 +48,9 @@ const MarkMasqChainName = "CNI-HOSTPORT-MASQ" const OldTopLevelSNATChainName = "CNI-HOSTPORT-SNAT" // forwardPorts establishes port forwarding to a given container IP. -// containerIP can be either v4 or v6. -func forwardPorts(config *PortMapConf, containerIP net.IP) error { - isV6 := (containerIP.To4() == nil) +// containerNet.IP can be either v4 or v6. +func forwardPorts(config *PortMapConf, containerNet net.IPNet) error { + isV6 := (containerNet.IP.To4() == nil) var ipt *iptables.IPTables var err error @@ -86,7 +86,7 @@ func forwardPorts(config *PortMapConf, containerIP net.IP) error { if !isV6 { // Set the route_localnet bit on the host interface, so that // 127/8 can cross a routing boundary. - hostIfName := getRoutableHostIF(containerIP) + hostIfName := getRoutableHostIF(containerNet.IP) if hostIfName != "" { if err := enableLocalnetRouting(hostIfName); err != nil { return fmt.Errorf("unable to enable route_localnet: %v", err) @@ -104,7 +104,7 @@ func forwardPorts(config *PortMapConf, containerIP net.IP) error { dnatChain := genDnatChain(config.Name, config.ContainerID) // First, idempotently tear down this chain in case there was some // sort of collision or bad state. - fillDnatRules(&dnatChain, config, containerIP) + fillDnatRules(&dnatChain, config, containerNet) if err := dnatChain.setup(ipt); err != nil { return fmt.Errorf("unable to setup DNAT: %v", err) } @@ -112,10 +112,10 @@ func forwardPorts(config *PortMapConf, containerIP net.IP) error { return nil } -func checkPorts(config *PortMapConf, containerIP net.IP) error { +func checkPorts(config *PortMapConf, containerNet net.IPNet) error { dnatChain := genDnatChain(config.Name, config.ContainerID) - fillDnatRules(&dnatChain, config, containerIP) + fillDnatRules(&dnatChain, config, containerNet) ip4t := maybeGetIptables(false) ip6t := maybeGetIptables(true) @@ -180,8 +180,8 @@ func genDnatChain(netName, containerID string) chain { // dnatRules generates the destination NAT rules, one per port, to direct // traffic from hostip:hostport to podip:podport -func fillDnatRules(c *chain, config *PortMapConf, containerIP net.IP) { - isV6 := (containerIP.To4() == nil) +func fillDnatRules(c *chain, config *PortMapConf, containerNet net.IPNet) { + isV6 := (containerNet.IP.To4() == nil) comment := trimComment(fmt.Sprintf(`dnat name: "%s" id: "%s"`, config.Name, config.ContainerID)) entries := config.RuntimeConfig.PortMaps setMarkChainName := SetMarkChainName @@ -249,7 +249,7 @@ func fillDnatRules(c *chain, config *PortMapConf, containerIP net.IP) { copy(hpRule, ruleBase) hpRule = append(hpRule, - "-s", containerIP.String(), + "-s", containerNet.String(), "-j", setMarkChainName, ) c.rules = append(c.rules, hpRule) @@ -272,7 +272,7 @@ func fillDnatRules(c *chain, config *PortMapConf, containerIP net.IP) { copy(dnatRule, ruleBase) dnatRule = append(dnatRule, "-j", "DNAT", - "--to-destination", fmtIpPort(containerIP, entry.ContainerPort), + "--to-destination", fmtIpPort(containerNet.IP, entry.ContainerPort), ) c.rules = append(c.rules, dnatRule) } diff --git a/plugins/meta/portmap/portmap_test.go b/plugins/meta/portmap/portmap_test.go index 8b04bf78a..efd4d2f70 100644 --- a/plugins/meta/portmap/portmap_test.go +++ b/plugins/meta/portmap/portmap_test.go @@ -16,7 +16,8 @@ package main import ( "fmt" - "net" + + "github.com/containernetworking/cni/pkg/types" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -77,8 +78,10 @@ var _ = Describe("portmapping configuration", func() { Expect(c.SNAT).To(Equal(&fvar)) Expect(c.Name).To(Equal("test")) - Expect(c.ContIPv4).To(Equal(net.ParseIP("10.0.0.2"))) - Expect(c.ContIPv6).To(Equal(net.ParseIP("2001:db8:1::2"))) + n, err := types.ParseCIDR("10.0.0.2/24") + Expect(c.ContIPv4).To(Equal(*n)) + n, err = types.ParseCIDR("2001:db8:1::2/64") + Expect(c.ContIPv6).To(Equal(*n)) }) It("Correctly parses a DEL config", func() { @@ -186,7 +189,8 @@ var _ = Describe("portmapping configuration", func() { entryChains: []string{"CNI-HOSTPORT-DNAT"}, })) - fillDnatRules(&ch, conf, net.ParseIP("10.0.0.2")) + n, err := types.ParseCIDR("10.0.0.2/24") + fillDnatRules(&ch, conf, *n) Expect(ch.entryRules).To(Equal([][]string{ {"-m", "comment", "--comment", @@ -204,16 +208,16 @@ var _ = Describe("portmapping configuration", func() { })) Expect(ch.rules).To(Equal([][]string{ - {"-p", "tcp", "--dport", "8080", "-s", "10.0.0.2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-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", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-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"}, - {"-p", "udp", "--dport", "8080", "-s", "10.0.0.2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-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", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-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"}, })) @@ -221,16 +225,17 @@ var _ = Describe("portmapping configuration", func() { ch.rules = nil ch.entryRules = nil - fillDnatRules(&ch, conf, net.ParseIP("2001:db8::2")) + n, err = types.ParseCIDR("2001:db8::2/64") + fillDnatRules(&ch, conf, *n) Expect(ch.rules).To(Equal([][]string{ - {"-p", "tcp", "--dport", "8080", "-s", "2001:db8::2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-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", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-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"}, - {"-p", "udp", "--dport", "8080", "-s", "2001:db8::2", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-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", "-j", "CNI-HOSTPORT-SETMARK"}, + {"-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"}, })) @@ -240,7 +245,8 @@ var _ = Describe("portmapping configuration", func() { fvar := false conf.SNAT = &fvar - fillDnatRules(&ch, conf, net.ParseIP("10.0.0.2")) + n, err = types.ParseCIDR("10.0.0.2/24") + fillDnatRules(&ch, conf, *n) Expect(ch.rules).To(Equal([][]string{ {"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, {"-p", "tcp", "--dport", "8081", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, @@ -276,9 +282,10 @@ var _ = Describe("portmapping configuration", func() { conf.ContainerID = containerID ch = genDnatChain(conf.Name, containerID) - fillDnatRules(&ch, conf, net.ParseIP("10.0.0.2")) + n, err := types.ParseCIDR("10.0.0.2/24") + fillDnatRules(&ch, conf, *n) Expect(ch.rules).To(Equal([][]string{ - {"-p", "tcp", "--dport", "8080", "-s", "10.0.0.2", "-j", "PLZ-SET-MARK"}, + {"-p", "tcp", "--dport", "8080", "-s", "10.0.0.2/24", "-j", "PLZ-SET-MARK"}, {"-p", "tcp", "--dport", "8080", "-s", "127.0.0.1", "-j", "PLZ-SET-MARK"}, {"-p", "tcp", "--dport", "8080", "-j", "DNAT", "--to-destination", "10.0.0.2:80"}, }))