Skip to content

fix(portmap): correct masquerading source address handling #1235

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
l1b0k wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(portmap): correct masquerading source address handling #1235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions plugins/meta/portmap/portmap_nftables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,16 +228,32 @@ func (pmNFT *portMapperNFTables) forwardPorts(config *PortMapConf, containerNet
// In theory we should validate that the original dst IP and port are as
// expected, but *any* traffic matching one of these patterns would need
// to be masqueraded to be able to work correctly anyway.

var masqSrcAddr string
if config.MasqAll {
// MasqAll: match traffic from any source IP
if isV6 {
masqSrcAddr = "::/0"
} else {
masqSrcAddr = "0.0.0.0/0"
}
} else {
// Default: only match traffic from container's own IP (hairpin)
masqSrcAddr = containerNet.IP.String()
}

tx.Add(&knftables.Rule{
Chain: masqueradingChain,
Rule: knftables.Concat(
ipX, "saddr", containerNet.IP,
ipX, "saddr", masqSrcAddr,
ipX, "daddr", containerNet.IP,
"masquerade",
),
Comment: &config.ContainerID,
})
if !isV6 {
if !isV6 && !config.MasqAll {
// Only add localhost rule when MasqAll is false
// (when MasqAll is true, 0.0.0.0/0 already covers 127.0.0.1)
tx.Add(&knftables.Rule{
Chain: masqueradingChain,
Rule: knftables.Concat(
Expand Down Expand Up @@ -275,8 +291,12 @@ func (pmNFT *portMapperNFTables) checkPorts(config *PortMapConf, containerNet ne
}
if *config.SNAT {
masqueradings = 1
if !isV6 {
masqueradings *= 2
// When MasqAll is false and IPv4, we have 2 rules:
// 1. hairpin rule (container IP -> container IP)
// 2. localhost rule (127.0.0.1 -> container IP)
// When MasqAll is true, we only have 1 rule (0.0.0.0/0 -> container IP)
if !isV6 && !config.MasqAll {
masqueradings = 2
}
}

Expand Down
248 changes: 248 additions & 0 deletions plugins/meta/portmap/portmap_nftables_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,5 +224,253 @@ add rule ip6 cni_hostport prerouting c d fib daddr type local jump hostports_all
Expect(err).To(HaveOccurred())
})
})

Describe("MasqAll configuration", func() {
var pmNFT *portMapperNFTables
var ipv4Fake, ipv6Fake *knftables.Fake
BeforeEach(func() {
ipv4Fake = knftables.NewFake(knftables.IPv4Family, tableName)
ipv6Fake = knftables.NewFake(knftables.IPv6Family, tableName)
pmNFT = &portMapperNFTables{
ipv4: ipv4Fake,
ipv6: ipv6Fake,
}
})

It(fmt.Sprintf("[%s] generates correct rules with masqAll=true for IPv4", ver), func() {
configTmpl := `{
"name": "test",
"type": "portmap",
"cniVersion": "%s",
"backend": "nftables",
"runtimeConfig": {
"portMappings": [
{ "hostPort": 8080, "containerPort": 80, "protocol": "tcp"}
]
},
"snat": true,
"masqAll": true
}`
configBytes := []byte(fmt.Sprintf(configTmpl, ver))

conf, _, err := parseConfig(configBytes, "foo")
Expect(err).NotTo(HaveOccurred())
conf.ContainerID = containerID

err = pmNFT.forwardPorts(conf, *containerNet4)
Expect(err).NotTo(HaveOccurred())

// With masqAll=true, should have only 1 masquerading rule with saddr 0.0.0.0/0
expectedRules := strings.TrimSpace(`
add table ip cni_hostport { comment "CNI portmap plugin" ; }
add chain ip cni_hostport hostip_hostports
add chain ip cni_hostport hostports
add chain ip cni_hostport hostports_all
add chain ip cni_hostport masquerading { type nat hook postrouting priority 100 ; }
add chain ip cni_hostport output { type nat hook output priority -100 ; }
add chain ip cni_hostport prerouting { type nat hook prerouting priority -100 ; }
add rule ip cni_hostport hostports tcp dport 8080 dnat to 10.0.0.2:80 comment "icee6giejonei6so"
add rule ip cni_hostport hostports_all jump hostip_hostports
add rule ip cni_hostport hostports_all jump hostports
add rule ip cni_hostport masquerading ip saddr 0.0.0.0/0 ip daddr 10.0.0.2 masquerade comment "icee6giejonei6so"
add rule ip cni_hostport output fib daddr type local jump hostports_all
add rule ip cni_hostport prerouting fib daddr type local jump hostports_all
`)
actualRules := strings.TrimSpace(ipv4Fake.Dump())
Expect(actualRules).To(Equal(expectedRules))

// Check should pass with 1 masquerading rule
err = pmNFT.checkPorts(conf, *containerNet4)
Expect(err).NotTo(HaveOccurred())
})

It(fmt.Sprintf("[%s] generates correct rules with masqAll=false for IPv4", ver), func() {
configTmpl := `{
"name": "test",
"type": "portmap",
"cniVersion": "%s",
"backend": "nftables",
"runtimeConfig": {
"portMappings": [
{ "hostPort": 8080, "containerPort": 80, "protocol": "tcp"}
]
},
"snat": true,
"masqAll": false
}`
configBytes := []byte(fmt.Sprintf(configTmpl, ver))

conf, _, err := parseConfig(configBytes, "foo")
Expect(err).NotTo(HaveOccurred())
conf.ContainerID = containerID

err = pmNFT.forwardPorts(conf, *containerNet4)
Expect(err).NotTo(HaveOccurred())

// With masqAll=false, should have 2 masquerading rules: hairpin + localhost
expectedRules := strings.TrimSpace(`
add table ip cni_hostport { comment "CNI portmap plugin" ; }
add chain ip cni_hostport hostip_hostports
add chain ip cni_hostport hostports
add chain ip cni_hostport hostports_all
add chain ip cni_hostport masquerading { type nat hook postrouting priority 100 ; }
add chain ip cni_hostport output { type nat hook output priority -100 ; }
add chain ip cni_hostport prerouting { type nat hook prerouting priority -100 ; }
add rule ip cni_hostport hostports tcp dport 8080 dnat to 10.0.0.2:80 comment "icee6giejonei6so"
add rule ip cni_hostport hostports_all jump hostip_hostports
add rule ip cni_hostport hostports_all jump hostports
add rule ip cni_hostport masquerading ip saddr 10.0.0.2 ip daddr 10.0.0.2 masquerade comment "icee6giejonei6so"
add rule ip cni_hostport masquerading ip saddr 127.0.0.1 ip daddr 10.0.0.2 masquerade comment "icee6giejonei6so"
add rule ip cni_hostport output fib daddr type local jump hostports_all
add rule ip cni_hostport prerouting fib daddr type local jump hostports_all
`)
actualRules := strings.TrimSpace(ipv4Fake.Dump())
Expect(actualRules).To(Equal(expectedRules))

// Check should pass with 2 masquerading rules
err = pmNFT.checkPorts(conf, *containerNet4)
Expect(err).NotTo(HaveOccurred())
})

It(fmt.Sprintf("[%s] generates correct rules with masqAll=true for IPv6", ver), func() {
configTmpl := `{
"name": "test",
"type": "portmap",
"cniVersion": "%s",
"backend": "nftables",
"runtimeConfig": {
"portMappings": [
{ "hostPort": 8080, "containerPort": 80, "protocol": "tcp"}
]
},
"snat": true,
"masqAll": true
}`
configBytes := []byte(fmt.Sprintf(configTmpl, ver))

conf, _, err := parseConfig(configBytes, "foo")
Expect(err).NotTo(HaveOccurred())
conf.ContainerID = containerID

err = pmNFT.forwardPorts(conf, *containerNet6)
Expect(err).NotTo(HaveOccurred())

// With masqAll=true for IPv6, should have only 1 masquerading rule with saddr ::/0
expectedRules := strings.TrimSpace(`
add table ip6 cni_hostport { comment "CNI portmap plugin" ; }
add chain ip6 cni_hostport hostip_hostports
add chain ip6 cni_hostport hostports
add chain ip6 cni_hostport hostports_all
add chain ip6 cni_hostport masquerading { type nat hook postrouting priority 100 ; }
add chain ip6 cni_hostport output { type nat hook output priority -100 ; }
add chain ip6 cni_hostport prerouting { type nat hook prerouting priority -100 ; }
add rule ip6 cni_hostport hostports tcp dport 8080 dnat to [2001:db8::2]:80 comment "icee6giejonei6so"
add rule ip6 cni_hostport hostports_all jump hostip_hostports
add rule ip6 cni_hostport hostports_all jump hostports
add rule ip6 cni_hostport masquerading ip6 saddr ::/0 ip6 daddr 2001:db8::2 masquerade comment "icee6giejonei6so"
add rule ip6 cni_hostport output fib daddr type local jump hostports_all
add rule ip6 cni_hostport prerouting fib daddr type local jump hostports_all
`)
actualRules := strings.TrimSpace(ipv6Fake.Dump())
Expect(actualRules).To(Equal(expectedRules))

// Check should pass with 1 masquerading rule
err = pmNFT.checkPorts(conf, *containerNet6)
Expect(err).NotTo(HaveOccurred())
})

It(fmt.Sprintf("[%s] generates correct rules with masqAll=false for IPv6", ver), func() {
configTmpl := `{
"name": "test",
"type": "portmap",
"cniVersion": "%s",
"backend": "nftables",
"runtimeConfig": {
"portMappings": [
{ "hostPort": 8080, "containerPort": 80, "protocol": "tcp"}
]
},
"snat": true,
"masqAll": false
}`
configBytes := []byte(fmt.Sprintf(configTmpl, ver))

conf, _, err := parseConfig(configBytes, "foo")
Expect(err).NotTo(HaveOccurred())
conf.ContainerID = containerID

err = pmNFT.forwardPorts(conf, *containerNet6)
Expect(err).NotTo(HaveOccurred())

// With masqAll=false for IPv6, should have 1 masquerading rule (no localhost for IPv6)
expectedRules := strings.TrimSpace(`
add table ip6 cni_hostport { comment "CNI portmap plugin" ; }
add chain ip6 cni_hostport hostip_hostports
add chain ip6 cni_hostport hostports
add chain ip6 cni_hostport hostports_all
add chain ip6 cni_hostport masquerading { type nat hook postrouting priority 100 ; }
add chain ip6 cni_hostport output { type nat hook output priority -100 ; }
add chain ip6 cni_hostport prerouting { type nat hook prerouting priority -100 ; }
add rule ip6 cni_hostport hostports tcp dport 8080 dnat to [2001:db8::2]:80 comment "icee6giejonei6so"
add rule ip6 cni_hostport hostports_all jump hostip_hostports
add rule ip6 cni_hostport hostports_all jump hostports
add rule ip6 cni_hostport masquerading ip6 saddr 2001:db8::2 ip6 daddr 2001:db8::2 masquerade comment "icee6giejonei6so"
add rule ip6 cni_hostport output fib daddr type local jump hostports_all
add rule ip6 cni_hostport prerouting fib daddr type local jump hostports_all
`)
actualRules := strings.TrimSpace(ipv6Fake.Dump())
Expect(actualRules).To(Equal(expectedRules))

// Check should pass with 1 masquerading rule
err = pmNFT.checkPorts(conf, *containerNet6)
Expect(err).NotTo(HaveOccurred())
})

It(fmt.Sprintf("[%s] checkPorts validates correct rule count with masqAll", ver), func() {
configTmpl := `{
"name": "test",
"type": "portmap",
"cniVersion": "%s",
"backend": "nftables",
"runtimeConfig": {
"portMappings": [
{ "hostPort": 8080, "containerPort": 80, "protocol": "tcp"}
]
},
"snat": true,
"masqAll": %t
}`

// Test masqAll=true: expects 1 rule
configBytes := []byte(fmt.Sprintf(configTmpl, ver, true))
conf, _, err := parseConfig(configBytes, "foo")
Expect(err).NotTo(HaveOccurred())
conf.ContainerID = containerID

err = pmNFT.forwardPorts(conf, *containerNet4)
Expect(err).NotTo(HaveOccurred())

// Should pass with 1 rule
err = pmNFT.checkPorts(conf, *containerNet4)
Expect(err).NotTo(HaveOccurred())

// Clear the fake nftables
ipv4Fake = knftables.NewFake(knftables.IPv4Family, tableName)
pmNFT.ipv4 = ipv4Fake

// Test masqAll=false: expects 2 rules
configBytes = []byte(fmt.Sprintf(configTmpl, ver, false))
conf, _, err = parseConfig(configBytes, "foo")
Expect(err).NotTo(HaveOccurred())
conf.ContainerID = containerID

err = pmNFT.forwardPorts(conf, *containerNet4)
Expect(err).NotTo(HaveOccurred())

// Should pass with 2 rules
err = pmNFT.checkPorts(conf, *containerNet4)
Expect(err).NotTo(HaveOccurred())
})
})
}
})
Loading