bird: per-peer import filter rejects connected subnet
Build flock Image / build (push) Successful in 2m17s
Build flock Image / build (push) Successful in 2m17s
Without a filter, crt001's `network 2602:817:3000:A25::/64` gets
re-advertised to every peer on that subnet. bird installs the BGP /64
with metric 32, beating the kernel-connected route at 256, and all
inter-host VLAN-25 traffic hairpins through the gateway — losing PMTU
9000 and ~30x throughput. Broke Plex 2026-05-04: NFS to nas002 capped
at 7 MB/s, jumbo blackholed.
Add LocalSubnetV6/V4 (CIDR) to NodeBGP. Agent populates by masking the
peer's address to /64 (v6) or /24 (v4) — same fritzlab convention
already in localAddrSameSubnet. Render emits `import where net !=
<subnet>;` per BGP channel when set, falls back to `import all;`
otherwise so existing tests stay green.
Defence in depth: with the matching outbound route-map on crt001
(ROUTE_MAP_CLUSTER_OUT_V{4,6}) the agent now refuses the leak on its
own if the router filter ever drifts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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 != <subnet>`
|
||||
// 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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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 != <subnet>` 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)
|
||||
|
||||
@@ -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,
|
||||
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
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -9,3 +9,5 @@ string("")
|
||||
string("}")
|
||||
string("")
|
||||
string("")
|
||||
string("")
|
||||
string("")
|
||||
|
||||
Reference in New Issue
Block a user