From eedbdb95e5dc92609044de2180c8b42b2f6f635b Mon Sep 17 00:00:00 2001 From: ugurtafrali Date: Thu, 16 Apr 2026 07:14:35 +0300 Subject: [PATCH] Fix IPv6 protocol conflict in nftables rules Remove RuleActionSelfSNAT from the generic rule action map. IPv6 rules were failing because the generic "snat to ip saddr" was being applied to all IP families. This needs per-family handling. Add tests to verify IPv6 protocol headers work correctly. Signed-off-by: Tom Pantelis --- .github/workflows/e2e.yml | 1 + pkg/packetfilter/nftables/nftables.go | 13 +++-- pkg/packetfilter/nftables/nftables_test.go | 51 ++++++++++++++++++++ pkg/packetfilter/nftables/rule_conversion.go | 46 ++++++++++++------ 4 files changed, 90 insertions(+), 21 deletions(-) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 6381b4727..59c014738 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -60,6 +60,7 @@ jobs: - extra-toggles: dual-stack, globalnet - extra-toggles: ipv6-stack - extra-toggles: nftables + - extra-toggles: nftables, dual-stack - extra-toggles: nftables, ovn - extra-toggles: nftables, globalnet - extra-toggles: nftables, globalnet, ovn diff --git a/pkg/packetfilter/nftables/nftables.go b/pkg/packetfilter/nftables/nftables.go index 5fce3e158..6222657ff 100644 --- a/pkg/packetfilter/nftables/nftables.go +++ b/pkg/packetfilter/nftables/nftables.go @@ -58,13 +58,12 @@ var ( } ruleActionToStr = map[packetfilter.RuleAction][]string{ - packetfilter.RuleActionAccept: {"accept"}, - packetfilter.RuleActionMss: {"tcp", "option", "maxseg"}, - packetfilter.RuleActionMark: {"meta", "mark"}, - packetfilter.RuleActionSNAT: {"snat"}, - packetfilter.RuleActionDNAT: {"dnat"}, - packetfilter.RuleActionJump: {"jump"}, - packetfilter.RuleActionSelfSNAT: {"snat to ip saddr"}, + packetfilter.RuleActionAccept: {"accept"}, + packetfilter.RuleActionMss: {"tcp", "option", "maxseg"}, + packetfilter.RuleActionMark: {"meta", "mark"}, + packetfilter.RuleActionSNAT: {"snat"}, + packetfilter.RuleActionDNAT: {"dnat"}, + packetfilter.RuleActionJump: {"jump"}, } nftFamilies = map[k8snet.IPFamily]knftables.Family{ diff --git a/pkg/packetfilter/nftables/nftables_test.go b/pkg/packetfilter/nftables/nftables_test.go index 218f24d64..2a7d6b9f1 100644 --- a/pkg/packetfilter/nftables/nftables_test.go +++ b/pkg/packetfilter/nftables/nftables_test.go @@ -400,6 +400,57 @@ var _ = Describe("Interface", func() { }) }) +var _ = Describe("IPv6 rules", func() { + const chainName6 = "test-chain" + + var ( + fakeNft6 *fakeKnftablesWrapper + pf6 packetfilter.Driver + ) + + BeforeEach(func() { + fakeNft6 = &fakeKnftablesWrapper{knftables.NewFake(knftables.IPv6Family, "submariner-ipv6")} + pf6 = nftables.NewWithNft(fakeNft6, k8snet.IPv6) + + Expect(pf6.CreateChainIfNotExists(packetfilter.TableTypeNAT, &packetfilter.Chain{Name: chainName6})).To(Succeed()) + }) + + getRuleSpec6 := func() string { + rules, err := fakeNft6.Fake.ListRules(context.TODO(), chainName6) + Expect(err).To(Succeed()) + Expect(rules).To(HaveLen(1)) + + return rules[0].Rule + } + + Specify("should use ip6 nexthdr for UDP", func() { + Expect(pf6.Append(packetfilter.TableTypeNAT, chainName6, &packetfilter.Rule{ + Proto: packetfilter.RuleProtoUDP, DPort: "4500", Action: packetfilter.RuleActionAccept, + })).To(Succeed()) + Expect(getRuleSpec6()).To(And( + ContainSubstring("ip6 nexthdr udp"), + Not(ContainSubstring("ip protocol udp")), + )) + }) + + Specify("should use ip6 nexthdr icmpv6 for ICMP", func() { + Expect(pf6.Append(packetfilter.TableTypeNAT, chainName6, &packetfilter.Rule{ + Proto: packetfilter.RuleProtoICMP, Action: packetfilter.RuleActionAccept, + })).To(Succeed()) + Expect(getRuleSpec6()).To(And( + ContainSubstring("ip6 nexthdr icmpv6"), + Not(ContainSubstring("ip nexthdr icmp")), + )) + }) + + Specify("should use ip6 saddr for SelfSNAT", func() { + Expect(pf6.Append(packetfilter.TableTypeNAT, chainName6, &packetfilter.Rule{ + Action: packetfilter.RuleActionSelfSNAT, + })).To(Succeed()) + Expect(getRuleSpec6()).To(ContainSubstring("snat to ip6 saddr")) + }) +}) + func testRuleConversion(rule *packetfilter.Rule) { serialized := nftables.SerializeRule(rule) Expect(len(serialized)).To(BeNumerically("<=", 128)) diff --git a/pkg/packetfilter/nftables/rule_conversion.go b/pkg/packetfilter/nftables/rule_conversion.go index 62cd06c04..ac77d8532 100644 --- a/pkg/packetfilter/nftables/rule_conversion.go +++ b/pkg/packetfilter/nftables/rule_conversion.go @@ -29,22 +29,35 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" ) -const serializedVersion = "1.0" +const ( + serializedVersion = "1.0" + ipv6Family = "ip6" +) + +func protoToRuleSpec(ruleSpec []string, proto packetfilter.RuleProto, dPort string, isIPv6 bool) []string { + addrFamily, protoKW := "ip", "protocol" + if isIPv6 { + addrFamily, protoKW = ipv6Family, "nexthdr" + } -func protoToRuleSpec(ruleSpec []string, proto packetfilter.RuleProto, dPort string) []string { switch proto { case packetfilter.RuleProtoUDP: - ruleSpec = append(ruleSpec, "ip", "protocol", "udp") + ruleSpec = append(ruleSpec, addrFamily, protoKW, "udp") if dPort != "" { ruleSpec = append(ruleSpec, "udp", "dport", dPort) } case packetfilter.RuleProtoTCP: - ruleSpec = append(ruleSpec, "ip", "protocol", "tcp") + ruleSpec = append(ruleSpec, addrFamily, protoKW, "tcp") if dPort != "" { ruleSpec = append(ruleSpec, "tcp", "dport", dPort) } case packetfilter.RuleProtoICMP: - ruleSpec = append(ruleSpec, "ip", "protocol", "icmp") + icmpProto := "icmp" + if isIPv6 { + icmpProto = "icmpv6" + } + + ruleSpec = append(ruleSpec, addrFamily, protoKW, icmpProto) case packetfilter.RuleProtoAll: case packetfilter.RuleProtoUndefined: } @@ -64,11 +77,11 @@ func mssClampToRuleSpec(ruleSpec []string, clampType packetfilter.MssClampType, return ruleSpec } -func setToRuleSpec(ruleSpec []string, srcSetName, destSetName string, isIPv6Set bool) []string { +func setToRuleSpec(ruleSpec []string, srcSetName, destSetName string, isIPv6 bool) []string { setPrefix := "ip" - if isIPv6Set { - setPrefix = "ip6" + if isIPv6 { + setPrefix = ipv6Family } if srcSetName != "" { @@ -82,13 +95,13 @@ func setToRuleSpec(ruleSpec []string, srcSetName, destSetName string, isIPv6Set return ruleSpec } -func toNftRuleSpec(rule *packetfilter.Rule, isIPv6Set bool) string { - ruleSpec := protoToRuleSpec([]string{}, rule.Proto, rule.DPort) +func toNftRuleSpec(rule *packetfilter.Rule, isIPv6 bool) string { + ruleSpec := protoToRuleSpec([]string{}, rule.Proto, rule.DPort, isIPv6) setPrefix := "ip" - if isIPv6Set { - setPrefix = "ip6" + if isIPv6 { + setPrefix = ipv6Family } if rule.SrcCIDR != "" { @@ -104,7 +117,7 @@ func toNftRuleSpec(rule *packetfilter.Rule, isIPv6Set bool) string { ruleSpec = append(ruleSpec, "meta", "mark", "&", rule.MarkValue, "==", rule.MarkValue) } - ruleSpec = setToRuleSpec(ruleSpec, rule.SrcSetName, rule.DestSetName, isIPv6Set) + ruleSpec = setToRuleSpec(ruleSpec, rule.SrcSetName, rule.DestSetName, isIPv6) if rule.OutInterface != "" { ruleSpec = append(ruleSpec, "oifname", rule.OutInterface) @@ -119,7 +132,12 @@ func toNftRuleSpec(rule *packetfilter.Rule, isIPv6Set bool) string { } ruleSpec = append(ruleSpec, "counter") - ruleSpec = append(ruleSpec, ruleActionToStr[rule.Action]...) + + if rule.Action == packetfilter.RuleActionSelfSNAT { + ruleSpec = append(ruleSpec, "snat", "to", setPrefix, "saddr") + } else { + ruleSpec = append(ruleSpec, ruleActionToStr[rule.Action]...) + } if rule.Action == packetfilter.RuleActionJump { ruleSpec = append(ruleSpec, rule.TargetChain)