diff --git a/firewall/rules_builder.go b/firewall/rules_builder.go index 4b6eca9c..6d607de2 100644 --- a/firewall/rules_builder.go +++ b/firewall/rules_builder.go @@ -288,15 +288,13 @@ func (rr *remoteRules) add(r rule) { } if r.cidr.IsValid() { - if rr.CIDR == nil { - rr.CIDR = new(bart.Table[*localRules]) + prefix := r.cidr.Masked() + i := slices.IndexFunc(rr.CIDR, func(cr cidrRule) bool { return cr.Prefix == prefix }) + if i < 0 { + i = len(rr.CIDR) + rr.CIDR = append(rr.CIDR, cidrRule{Prefix: prefix, LocalCIDR: &localRules{}}) } - lr, ok := rr.CIDR.Get(r.cidr) - if !ok { - lr = &localRules{} - rr.CIDR.Insert(r.cidr, lr) - } - lr.add(r) + rr.CIDR[i].LocalCIDR.add(r) } } diff --git a/firewall/rules_builder_test.go b/firewall/rules_builder_test.go index cec62ab2..01ad40af 100644 --- a/firewall/rules_builder_test.go +++ b/firewall/rules_builder_test.go @@ -59,21 +59,29 @@ func TestRulesBuilder_AddRule(t *testing.T) { require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", ti.String(), "", "", "")) rules = rb.Build(nil, nil) assert.Nil(t, rules.Out.protos[ProtoAny][1].Any.Any) - _, ok := rules.Out.protos[ProtoAny][1].Any.CIDR.Get(ti) - assert.True(t, ok) + require.Len(t, rules.Out.protos[ProtoAny][1].Any.CIDR, 1) + assert.Equal(t, ti, rules.Out.protos[ProtoAny][1].Any.CIDR[0].Prefix) rb = NewRulesBuilder(l) require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", ti6.String(), "", "", "")) rules = rb.Build(nil, nil) assert.Nil(t, rules.Out.protos[ProtoAny][1].Any.Any) - _, ok = rules.Out.protos[ProtoAny][1].Any.CIDR.Get(ti6) - assert.True(t, ok) + require.Len(t, rules.Out.protos[ProtoAny][1].Any.CIDR, 1) + assert.Equal(t, ti6, rules.Out.protos[ProtoAny][1].Any.CIDR[0].Prefix) + + // The same cidr written two ways is one entry + rb = NewRulesBuilder(l) + require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", "1.2.3.4/24", "", "", "")) + require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", "1.2.3.0/24", "", "", "")) + rules = rb.Build(nil, nil) + require.Len(t, rules.Out.protos[ProtoAny][1].Any.CIDR, 1) + assert.Equal(t, netip.MustParsePrefix("1.2.3.0/24"), rules.Out.protos[ProtoAny][1].Any.CIDR[0].Prefix) rb = NewRulesBuilder(l) require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", "", ti.String(), "", "")) rules = rb.Build(nil, nil) assert.NotNil(t, rules.Out.protos[ProtoAny][1].Any.Any) - ok = rules.Out.protos[ProtoAny][1].Any.Any.LocalCIDR.Get(ti) + ok := rules.Out.protos[ProtoAny][1].Any.Any.LocalCIDR.Get(ti) assert.True(t, ok) rb = NewRulesBuilder(l) @@ -102,25 +110,23 @@ func TestRulesBuilder_AddRule(t *testing.T) { anyIp, err := netip.ParsePrefix("0.0.0.0/0") require.NoError(t, err) + // 0.0.0.0/0 is any IPv4 address, not any IPv6 address require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", anyIp.String(), "", "", "")) rules = rb.Build(nil, nil) assert.Nil(t, rules.Out.protos[ProtoAny][0].Any.Any) - table, ok := rules.Out.protos[ProtoAny][0].Any.CIDR.Lookup(netip.MustParseAddr("1.1.1.1")) - assert.True(t, table.Any) - table, ok = rules.Out.protos[ProtoAny][0].Any.CIDR.Lookup(netip.MustParseAddr("9::9")) - assert.False(t, ok) + assert.True(t, rules.Out.protos[ProtoAny][0].Any.match(&Packet{RemoteAddr: netip.MustParseAddr("1.1.1.1")}, nil)) + assert.False(t, rules.Out.protos[ProtoAny][0].Any.match(&Packet{RemoteAddr: netip.MustParseAddr("9::9")}, nil)) rb = NewRulesBuilder(l) anyIp6, err := netip.ParsePrefix("::/0") require.NoError(t, err) + // ::/0 is any IPv6 address, not any IPv4 address require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", anyIp6.String(), "", "", "")) rules = rb.Build(nil, nil) assert.Nil(t, rules.Out.protos[ProtoAny][0].Any.Any) - table, ok = rules.Out.protos[ProtoAny][0].Any.CIDR.Lookup(netip.MustParseAddr("9::9")) - assert.True(t, table.Any) - table, ok = rules.Out.protos[ProtoAny][0].Any.CIDR.Lookup(netip.MustParseAddr("1.1.1.1")) - assert.False(t, ok) + assert.True(t, rules.Out.protos[ProtoAny][0].Any.match(&Packet{RemoteAddr: netip.MustParseAddr("9::9")}, nil)) + assert.False(t, rules.Out.protos[ProtoAny][0].Any.match(&Packet{RemoteAddr: netip.MustParseAddr("1.1.1.1")}, nil)) rb = NewRulesBuilder(l) require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", "any", "", "", "")) diff --git a/firewall/table.go b/firewall/table.go index 5827c90d..39971b5b 100644 --- a/firewall/table.go +++ b/firewall/table.go @@ -46,7 +46,9 @@ type remoteRules struct { Any *localRules Hosts map[string]*localRules Groups []*groupsRule - CIDR *bart.Table[*localRules] + // CIDR is checked with Prefix.Contains, so 0.0.0.0/0 allows any IPv4 address and ::/0 any IPv6 address, + // neither allows the other family + CIDR []cidrRule } type groupsRule struct { @@ -54,6 +56,12 @@ type groupsRule struct { LocalCIDR *localRules } +type cidrRule struct { + // Prefix is masked, so a cidr written two ways is one cidrRule + Prefix netip.Prefix + LocalCIDR *localRules +} + type localRules struct { Any bool // LocalCIDR is always set when Any isn't. It's shared with other localRules, so it's never changed. @@ -164,11 +172,9 @@ func (rr *remoteRules) match(p *Packet, c *cert.CachedCertificate) bool { } } - if rr.CIDR != nil { - for _, v := range rr.CIDR.Supernets(netip.PrefixFrom(p.RemoteAddr, p.RemoteAddr.BitLen())) { - if v.match(p, c) { - return true - } + for i := range rr.CIDR { + if rr.CIDR[i].Prefix.Contains(p.RemoteAddr) && rr.CIDR[i].LocalCIDR.match(p, c) { + return true } } diff --git a/firewall_test.go b/firewall_test.go index 8db914f0..41cd81e7 100644 --- a/firewall_test.go +++ b/firewall_test.go @@ -389,8 +389,36 @@ func BenchmarkFirewallTable_match(b *testing.B) { ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 10}, true, c, cp) } }) + + // The port 10 rules have a cidr, this cert fails their group and host so only the cidr can pass them + cidrCert := &cert.CachedCertificate{ + Certificate: &dummyCert{name: "nope"}, + InvertedGroups: map[string]struct{}{"nope": {}}, + } + for _, tc := range []struct { + name string + addr netip.Addr + want bool + }{ + {"pass on cidr", pfix.Addr(), true}, + {"pass on cidr6", pfix6.Addr(), true}, + {"pass proto, port, fail group and name, fail on cidr", netip.MustParseAddr("9.254.254.245"), false}, + {"pass proto, port, fail group and name, fail on cidr6", netip.MustParseAddr("fd99::99"), false}, + } { + p := &firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 10, RemoteAddr: tc.addr} + b.Run(tc.name, func(b *testing.B) { + if ft.Match(p, true, cidrCert, cp) != tc.want { + b.Fatal("wrong verdict") + } + for b.Loop() { + benchMatchSink = ft.Match(p, true, cidrCert, cp) + } + }) + } } +var benchMatchSink bool + func TestFirewall_Drop2(t *testing.T) { ob := &bytes.Buffer{} l := test.NewLoggerWithOutput(ob)