diff --git a/pkg/agent/bird.go b/pkg/agent/bird.go index c0ce126..08cc040 100644 --- a/pkg/agent/bird.go +++ b/pkg/agent/bird.go @@ -55,6 +55,12 @@ func (b *BirdManager) Render(nc *flockv1alpha1.NodeConfig, anycast6, anycast4 [] // the BGP peer. crt001 rejects IPv6 advertisements whose next-hop is // link-local-only; an explicit `source address` makes BIRD use a // global next-hop self, which Cisco accepts. + // + // Also derive the connected subnet (peer IP masked to /64 v6 / /24 v4) + // per family. Render uses it to install `import where net != ` + // on the BGP channel so the gateway can't readvertise our own connected + // /64 back to us — accepting it would override the kernel route and + // hairpin all inter-host traffic via the gateway. for _, p := range nc.Spec.BGP.Peers { fam := bird.FamilyOf(p.Address) if fam == "" { @@ -69,6 +75,14 @@ func (b *BirdManager) Render(nc *flockv1alpha1.NodeConfig, anycast6, anycast4 [] in.LocalV4 = local } } + if subnet := peerSubnet(p.Address); subnet != "" { + if fam == "v6" && in.LocalSubnetV6 == "" { + in.LocalSubnetV6 = subnet + } + if fam == "v4" && in.LocalSubnetV4 == "" { + in.LocalSubnetV4 = subnet + } + } } cfg, err := bird.Render(in) @@ -165,6 +179,25 @@ func (b *BirdManager) SummaryRoutes(nc *flockv1alpha1.NodeConfig) error { return nil } +// peerSubnet returns the canonical CIDR of the assumed connected subnet +// containing `peer` — /64 for IPv6, /24 for IPv4. Returns "" if peer +// doesn't parse. Matches the assumption already baked into +// localAddrSameSubnet: fritzlab convention is /64 v6 and /24 v4. +func peerSubnet(peer string) string { + pip := net.ParseIP(peer) + if pip == nil { + return "" + } + var mask net.IPMask + if pip.To4() != nil { + mask = net.CIDRMask(24, 32) + } else { + mask = net.CIDRMask(64, 128) + } + n := &net.IPNet{IP: pip.Mask(mask), Mask: mask} + return n.String() +} + // localAddrSameSubnet finds an IP on a local interface that's in the same // /64 (v6) or /24 (v4) as `peer`. Returns "" if none. Used to derive the // `source address` for a BGP session. diff --git a/pkg/agent/bird_test.go b/pkg/agent/bird_test.go new file mode 100644 index 0000000..aa44fe4 --- /dev/null +++ b/pkg/agent/bird_test.go @@ -0,0 +1,25 @@ +package agent + +import "testing" + +func TestPeerSubnet(t *testing.T) { + cases := []struct { + peer string + want string + }{ + {"2602:817:3000:a25::1", "2602:817:3000:a25::/64"}, + {"2602:817:3000:a25::104", "2602:817:3000:a25::/64"}, + {"172.25.25.1", "172.25.25.0/24"}, + {"172.25.25.104", "172.25.25.0/24"}, + {"", ""}, + {"not-an-ip", ""}, + } + for _, tc := range cases { + t.Run(tc.peer, func(t *testing.T) { + got := peerSubnet(tc.peer) + if got != tc.want { + t.Fatalf("peerSubnet(%q) = %q, want %q", tc.peer, got, tc.want) + } + }) + } +} diff --git a/pkg/routing/bird/config.go b/pkg/routing/bird/config.go index 57f4033..bd369c2 100644 --- a/pkg/routing/bird/config.go +++ b/pkg/routing/bird/config.go @@ -26,6 +26,14 @@ type NodeBGP struct { // hop self that crt001 accepts). LocalV6 string LocalV4 string + // LocalSubnetV6 / LocalSubnetV4 are the directly-connected subnets + // (CIDR) the BGP peers live on. When set, the per-peer ipv6 / ipv4 + // channel uses `import where net != ` so the gateway can't + // re-advertise our own connected /64 (or /24) back to us — accepting + // it would override the kernel-connected route and hairpin all + // inter-host traffic via the gateway. + LocalSubnetV6 string + LocalSubnetV4 string // CIDR6 / CIDR4 are the per-node summary aggregates the agent wants // advertised. The agent installs blackhole kernel routes for each so // BIRD's protocol kernel imports them. @@ -92,7 +100,7 @@ protocol bgp upstream6_{{$i}} { neighbor {{$p.Address}} as {{$p.ASN}}; graceful restart; ipv6 { - import all; + {{if $.LocalSubnetV6}}import where net != {{$.LocalSubnetV6}};{{else}}import all;{{end}} next hop self; export filter { {{range $cidr := $.CIDR6}}if net = {{$cidr}} then accept; @@ -107,7 +115,7 @@ protocol bgp upstream4_{{$i}} { neighbor {{$p.Address}} as {{$p.ASN}}; graceful restart; ipv4 { - import all; + {{if $.LocalSubnetV4}}import where net != {{$.LocalSubnetV4}};{{else}}import all;{{end}} next hop self; export filter { {{range $cidr := $.CIDR4}}if net = {{$cidr}} then accept; @@ -147,6 +155,12 @@ func Render(in NodeBGP) (string, error) { if err := validateLocalSource(in.LocalV4, "v4"); err != nil { return "", err } + if err := validateLocalSubnet(in.LocalSubnetV6, "v6"); err != nil { + return "", err + } + if err := validateLocalSubnet(in.LocalSubnetV4, "v4"); err != nil { + return "", err + } for i, p := range in.Peers { if err := validatePeer(p); err != nil { return "", fmt.Errorf("bird render: peer[%d]: %w", i, err) @@ -263,6 +277,31 @@ func validateLocalSource(s, fam string) error { return nil } +// validateLocalSubnet validates an optional LocalSubnetV6/LocalSubnetV4 CIDR. +// Empty is allowed (no import filter); non-empty must be a parseable CIDR of +// the matching family in canonical form (host bits zero) so the BIRD `net !=` +// comparison matches the route the gateway re-advertises. +func validateLocalSubnet(s, fam string) error { + if s == "" { + return nil + } + ip, n, err := net.ParseCIDR(s) + if err != nil { + return fmt.Errorf("bird render: LocalSubnet%s %q is not a valid CIDR: %w", strings.ToUpper(fam), s, err) + } + if !ip.Equal(n.IP) { + return fmt.Errorf("bird render: LocalSubnet%s %q has non-zero host bits (want %s)", strings.ToUpper(fam), s, n.String()) + } + isV4 := n.IP.To4() != nil + if fam == "v6" && isV4 { + return fmt.Errorf("bird render: LocalSubnetV6 %q is IPv4", s) + } + if fam == "v4" && !isV4 { + return fmt.Errorf("bird render: LocalSubnetV4 %q is IPv6", s) + } + return nil +} + func normalize(in NodeBGP) NodeBGP { cp := in cp.CIDR6 = sortedUnique(in.CIDR6) diff --git a/pkg/routing/bird/config_fuzz_test.go b/pkg/routing/bird/config_fuzz_test.go index 8701011..f958db1 100644 --- a/pkg/routing/bird/config_fuzz_test.go +++ b/pkg/routing/bird/config_fuzz_test.go @@ -24,6 +24,8 @@ func FuzzRender(f *testing.F) { anycast4 string localV6 string localV4 string + subnet6 string + subnet4 string } seeds := []seed{ {routerID: "10.0.0.1", asn: 65101, peerAddr: "2001:db8::1", peerASN: 65000, cidr6: "2001:db8:f001::/64"}, @@ -38,17 +40,23 @@ func FuzzRender(f *testing.F) { {routerID: "10.0.0.1`", asn: 65101}, // Newlines and template-meta in user-supplied addresses {routerID: "10.0.0.1", asn: 65101, peerAddr: "2001:db8::1\n{{kaboom}}", peerASN: 65000, cidr6: "2001:db8:f001::/64"}, + // LocalSubnet filters set. + {routerID: "172.25.25.104", asn: 65104, peerAddr: "2602:817:3000:a25::1", peerASN: 65000, subnet6: "2602:817:3000:a25::/64", subnet4: "172.25.25.0/24"}, + // Malformed subnet should be rejected by validation, not crash. + {routerID: "10.0.0.1", asn: 65101, subnet6: "not-a-cidr"}, } for _, s := range seeds { - f.Add(s.routerID, s.asn, s.peerAddr, s.peerASN, s.cidr6, s.cidr4, s.anycast6, s.anycast4, s.localV6, s.localV4) + f.Add(s.routerID, s.asn, s.peerAddr, s.peerASN, s.cidr6, s.cidr4, s.anycast6, s.anycast4, s.localV6, s.localV4, s.subnet6, s.subnet4) } - f.Fuzz(func(t *testing.T, routerID string, asn uint32, peerAddr string, peerASN uint32, cidr6, cidr4, anycast6, anycast4, localV6, localV4 string) { + f.Fuzz(func(t *testing.T, routerID string, asn uint32, peerAddr string, peerASN uint32, cidr6, cidr4, anycast6, anycast4, localV6, localV4, subnet6, subnet4 string) { in := NodeBGP{ - RouterID: routerID, - LocalASN: asn, - LocalV6: localV6, - LocalV4: localV4, + RouterID: routerID, + LocalASN: asn, + LocalV6: localV6, + LocalV4: localV4, + LocalSubnetV6: subnet6, + LocalSubnetV4: subnet4, } // Add the peer in whichever family it belongs to, if any. FamilyOf // returns "" for non-IPs; that test exercises the "skip unknown diff --git a/pkg/routing/bird/config_test.go b/pkg/routing/bird/config_test.go index b305985..be149df 100644 --- a/pkg/routing/bird/config_test.go +++ b/pkg/routing/bird/config_test.go @@ -75,6 +75,89 @@ func TestRender_StableOutput(t *testing.T) { } } +func TestRender_LocalSubnetImportFilter(t *testing.T) { + out, err := Render(NodeBGP{ + RouterID: "172.25.25.104", + LocalASN: 65104, + Peers: []Peer{{Family: "v6", Address: "2602:817:3000:a25::1", ASN: 65000}, {Family: "v4", Address: "172.25.25.1", ASN: 65000}}, + CIDR6: []string{"2602:817:3000:f004::/64"}, + CIDR4: []string{"172.25.214.0/24"}, + LocalSubnetV6: "2602:817:3000:a25::/64", + LocalSubnetV4: "172.25.25.0/24", + }) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + "import where net != 2602:817:3000:a25::/64;", + "import where net != 172.25.25.0/24;", + } { + if !strings.Contains(out, want) { + t.Errorf("missing %q in output:\n%s", want, out) + } + } + // Each BGP peer block should use the import filter, not import all. + // Slice out just the `protocol bgp ...` stanzas to avoid catching the + // kernel proto's legitimate `import all;`. + for _, marker := range []string{"protocol bgp upstream6_", "protocol bgp upstream4_"} { + idx := strings.Index(out, marker) + if idx < 0 { + continue + } + end := strings.Index(out[idx:], "\n}") + if end < 0 { + continue + } + stanza := out[idx : idx+end] + if strings.Contains(stanza, "import all;") { + t.Errorf("BGP stanza still has `import all;`:\n%s", stanza) + } + } +} + +func TestRender_LocalSubnetEmpty_FallsBackToImportAll(t *testing.T) { + out, err := Render(NodeBGP{ + RouterID: "10.0.0.1", + LocalASN: 65101, + Peers: []Peer{{Family: "v6", Address: "2001:db8::1", ASN: 65000}}, + CIDR6: []string{"2001:db8:f001::/64"}, + }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "import all;") { + t.Errorf("expected `import all;` when LocalSubnetV6 unset:\n%s", out) + } +} + +func TestRender_LocalSubnetValidation(t *testing.T) { + cases := []struct { + name string + v6, v4 string + wantErr string + }{ + {name: "non-canonical v6", v6: "2602:817:3000:a25::1/64", wantErr: "non-zero host bits"}, + {name: "non-canonical v4", v4: "172.25.25.1/24", wantErr: "non-zero host bits"}, + {name: "v6 family mismatch", v6: "172.25.25.0/24", wantErr: "is IPv4"}, + {name: "v4 family mismatch", v4: "2602:817:3000:a25::/64", wantErr: "is IPv6"}, + {name: "garbage", v6: "not-a-cidr", wantErr: "not a valid CIDR"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := Render(NodeBGP{ + RouterID: "10.0.0.1", + LocalASN: 65101, + Peers: []Peer{{Family: "v6", Address: "2001:db8::1", ASN: 65000}}, + LocalSubnetV6: tc.v6, + LocalSubnetV4: tc.v4, + }) + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("want error containing %q, got %v", tc.wantErr, err) + } + }) + } +} + func TestFamilyOf(t *testing.T) { if FamilyOf("2001:db8::1") != "v6" { t.Fatal("v6 detection broken") diff --git a/pkg/routing/bird/testdata/fuzz/FuzzRender/568393b92f99b2ad b/pkg/routing/bird/testdata/fuzz/FuzzRender/568393b92f99b2ad index e4398d8..39e3bb8 100644 --- a/pkg/routing/bird/testdata/fuzz/FuzzRender/568393b92f99b2ad +++ b/pkg/routing/bird/testdata/fuzz/FuzzRender/568393b92f99b2ad @@ -9,3 +9,5 @@ string("") string("}") string("") string("") +string("") +string("")