From 317388e37d428bcb676c58d5bea4120e60b90a99 Mon Sep 17 00:00:00 2001 From: JackDoan Date: Tue, 29 Sep 2026 18:36:48 -0500 Subject: [PATCH] refactor checkpt --- firewall.go | 800 ++------------------------------------ firewall/config.go | 333 ++++++++++++++++ firewall/config_test.go | 423 ++++++++++++++++++++ firewall/rules_builder.go | 328 ++++++++++++++++ firewall/table.go | 164 ++++++++ firewall_test.go | 743 +++++++++-------------------------- 6 files changed, 1461 insertions(+), 1330 deletions(-) create mode 100644 firewall/config.go create mode 100644 firewall/config_test.go create mode 100644 firewall/rules_builder.go create mode 100644 firewall/table.go diff --git a/firewall.go b/firewall.go index 6776b033..787b0488 100644 --- a/firewall.go +++ b/firewall.go @@ -2,17 +2,10 @@ package nebula import ( "context" - "crypto/sha256" - "encoding/hex" "errors" - "fmt" - "hash/fnv" "log/slog" "net/netip" - "reflect" - "slices" "strconv" - "strings" "sync" "time" @@ -24,10 +17,6 @@ import ( "github.com/slackhq/nebula/iputil" ) -type FirewallInterface interface { - AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, cidr, localCidr string, caName string, caSha string) error -} - type conn struct { Expires time.Time // Time when this conntrack entry will expire @@ -42,12 +31,8 @@ type conn struct { type Firewall struct { Conntrack *FirewallConntrack - InRules *FirewallTable - OutRules *FirewallTable - - // inBuilder and outBuilder collect the rules from AddRule until buildRules turns them into InRules and OutRules - inBuilder *FirewallTableBuilder - outBuilder *FirewallTableBuilder + InRules *firewall.Table + OutRules *firewall.Table InboundSendReject bool OutboundSendReject bool @@ -67,12 +52,13 @@ type Firewall struct { // unsafeNetworks is the list of unsafe networks issued to us in the certificate unsafeNetworks []netip.Prefix - rules string + // ruleHash and ruleHashFNV identify the rules, see firewall.Rules + ruleHash string + ruleHashFNV uint32 rulesVersion uint16 - defaultLocalCIDRAny bool - incomingMetrics firewallMetrics - outgoingMetrics firewallMetrics + incomingMetrics firewallMetrics + outgoingMetrics firewallMetrics l *slog.Logger } @@ -90,160 +76,6 @@ type FirewallConntrack struct { TimerWheel *TimerWheel[firewall.Packet] } -// FirewallTable is the entry point for a rule, the evaluation order is: -// Proto AND port AND (CA SHA or CA name) AND local CIDR AND (group OR groups OR name OR remote CIDR) -// A FirewallTable doesn't change once built, see FirewallTableBuilder. -type FirewallTable struct { - // Protos holds the rules for each IP protocol number, so a packet is only checked against one set of rules. - // A protocol with rules of its own also holds a copy of every proto `any` rule, the rest share the proto `any` - // rules, which are at Protos[firewall.ProtoAny]. nil when a protocol has no rules at all. - // ICMP and ICMPv6 share one set of rules. - Protos [256]firewallPort -} - -// FirewallTableBuilder collects the rules for a FirewallTable, build turns them into one -type FirewallTableBuilder struct { - anyRules []firewallPortRule - // protoRules holds the rules for each protocol besides proto `any`, ICMPv6 rules are kept with ICMP - protoRules [256][]firewallPortRule -} - -// firewallPortRule is a rule for a firewallPort, parsed so adding it can't fail -type firewallPortRule struct { - startPort, endPort int32 - groups []string - host string - caName, caSha string - - // cidr is the remote cidr, not valid when the rule doesn't have one - cidr netip.Prefix - anyCIDR bool - - // localCIDR is not valid when the rule doesn't have one, which means the vpn networks if there are unsafe networks - localCIDR netip.Prefix - anyLocalCIDR bool -} - -func parseFirewallPortRule(startPort, endPort int32, groups []string, host, cidr, localCidr, caName, caSha string) (firewallPortRule, error) { - if startPort > endPort { - return firewallPortRule{}, fmt.Errorf("start port was lower than end port") - } - - r := firewallPortRule{ - startPort: startPort, - endPort: endPort, - groups: groups, - host: host, - caName: caName, - caSha: caSha, - } - - var err error - r.cidr, r.anyCIDR, err = parseFirewallCIDR(cidr) - if err != nil { - return firewallPortRule{}, err - } - r.localCIDR, r.anyLocalCIDR, err = parseFirewallCIDR(localCidr) - if err != nil { - return firewallPortRule{}, err - } - - return r, nil -} - -// parseFirewallCIDR parses a rule's cidr, which is empty, `any`, or a prefix -func parseFirewallCIDR(s string) (prefix netip.Prefix, isAny bool, err error) { - switch s { - case "": - return netip.Prefix{}, false, nil - case "any": - return netip.Prefix{}, true, nil - } - - prefix, err = netip.ParsePrefix(s) - return prefix, false, err -} - -// isAny reports whether r matches any host, making its groups, host, and cidr irrelevant -func (r firewallPortRule) isAny() bool { - if len(r.groups) == 0 && r.host == "" && !r.cidr.IsValid() { - return true - } - - return slices.Contains(r.groups, "any") || r.host == "any" || r.anyCIDR -} - -// addRule keeps r for proto, or for every protocol when proto is firewall.ProtoAny -func (b *FirewallTableBuilder) addRule(proto uint8, r firewallPortRule) { - switch proto { - case firewall.ProtoAny: - b.anyRules = append(b.anyRules, r) - case iputil.IPProtocolICMPv6: - b.protoRules[iputil.IPProtocolICMP] = append(b.protoRules[iputil.IPProtocolICMP], r) - default: - b.protoRules[proto] = append(b.protoRules[proto], r) - } -} - -func (b *FirewallTableBuilder) build(f *Firewall) *FirewallTable { - var anyProto firewallPort - if len(b.anyRules) > 0 { - anyProto = firewallPort{} - for _, r := range b.anyRules { - anyProto.addRule(f, r) - } - } - - ft := &FirewallTable{} - for proto := range ft.Protos { - rules := b.protoRules[proto] - if len(rules) == 0 { - ft.Protos[proto] = anyProto - continue - } - - fp := firewallPort{} - for _, r := range b.anyRules { - fp.addRule(f, r) - } - for _, r := range rules { - fp.addRule(f, r) - } - ft.Protos[proto] = fp - } - ft.Protos[iputil.IPProtocolICMPv6] = ft.Protos[iputil.IPProtocolICMP] - - return ft -} - -type FirewallCA struct { - Any *FirewallRule - CANames map[string]*FirewallRule - CAShas map[string]*FirewallRule -} - -type FirewallRule struct { - // Any makes Hosts, Groups, and CIDR irrelevant - Any *firewallLocalCIDR - Hosts map[string]*firewallLocalCIDR - Groups []*firewallGroups - CIDR *bart.Table[*firewallLocalCIDR] -} - -type firewallGroups struct { - Groups []string - LocalCIDR *firewallLocalCIDR -} - -// Even though ports are uint16, int32 maps are faster for lookup -// Plus we can use `-1` for fragment rules -type firewallPort map[int32]*FirewallCA - -type firewallLocalCIDR struct { - Any bool - LocalCIDR *bart.Lite -} - // NewFirewall creates a new Firewall object. A TimerWheel is created for you from the provided timeouts. // The certificate provided should be the highest version loaded in memory. func NewFirewall(l *slog.Logger, tcpTimeout, UDPTimeout, defaultTimeout time.Duration, c cert.Certificate) *Firewall { @@ -277,15 +109,11 @@ func NewFirewall(l *slog.Logger, tcpTimeout, UDPTimeout, defaultTimeout time.Dur routableNetworks.Insert(n) } - return &Firewall{ + fw := &Firewall{ Conntrack: &FirewallConntrack{ Conns: make(map[firewall.Packet]*conn), TimerWheel: NewTimerWheel[firewall.Packet](tmin, tmax), }, - InRules: &FirewallTable{}, - OutRules: &FirewallTable{}, - inBuilder: &FirewallTableBuilder{}, - outBuilder: &FirewallTableBuilder{}, TCPTimeout: tcpTimeout, UDPTimeout: UDPTimeout, DefaultTimeout: defaultTimeout, @@ -305,6 +133,10 @@ func NewFirewall(l *slog.Logger, tcpTimeout, UDPTimeout, defaultTimeout time.Dur droppedNoRule: metrics.GetOrRegisterCounter("firewall.outgoing.dropped.no_rule", nil), }, } + + // Nothing is allowed until there are rules + fw.setRules(firewall.NewRulesBuilder(l)) + return fw } func NewFirewallFromConfig(l *slog.Logger, cs *CertState, c *config.C) (*Firewall, error) { @@ -326,8 +158,6 @@ func NewFirewallFromConfig(l *slog.Logger, cs *CertState, c *config.C) (*Firewal //TODO: max_connections ) - fw.defaultLocalCIDRAny = c.GetBool("firewall.default_local_cidr_any", false) - inboundAction := c.GetString("firewall.inbound_action", "drop") switch inboundAction { case "reject": @@ -350,89 +180,30 @@ func NewFirewallFromConfig(l *slog.Logger, cs *CertState, c *config.C) (*Firewal fw.OutboundSendReject = false } - err := AddFirewallRulesFromConfig(l, false, c, fw) - if err != nil { - return nil, err - } - - err = AddFirewallRulesFromConfig(l, true, c, fw) - if err != nil { - return nil, err - } - - err = fw.buildRules() + rules, err := firewall.RulesFromConfig(l, c) if err != nil { return nil, err } + fw.setRules(rules) return fw, nil } -// AddRule properly creates the in memory rule structure for a firewall table. -func (f *Firewall) AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, cidr, localCidr, caName string, caSha string) error { - b := f.outBuilder - if incoming { - b = f.inBuilder - } - if b == nil { - return errors.New("firewall rules can't be added once they're built") - } - - if proto == iputil.IPProtocolICMP || proto == iputil.IPProtocolICMPv6 { - //ICMP traffic doesn't have ports, so we always coerce to "any", even if a value is provided - if startPort != firewall.PortAny { - f.l.Warn("ignoring port specification for ICMP firewall rule", "startPort", startPort) - } - startPort = firewall.PortAny - endPort = firewall.PortAny - } - - r, err := parseFirewallPortRule(startPort, endPort, groups, host, cidr, localCidr, caName, caSha) - if err != nil { - return err - } - - // We need this rule string because we generate a hash. Removing this will break firewall reload. - ruleString := fmt.Sprintf( - "incoming: %v, proto: %v, startPort: %v, endPort: %v, groups: %v, host: %v, ip: %v, localIp: %v, caName: %v, caSha: %s", - incoming, proto, startPort, endPort, groups, host, cidr, localCidr, caName, caSha, - ) - f.rules += ruleString + "\n" - - direction := "incoming" - if !incoming { - direction = "outgoing" - } - f.l.Info("Firewall rule added", - "firewallRule", m{"direction": direction, "proto": proto, "startPort": startPort, "endPort": endPort, "groups": groups, "host": host, "cidr": cidr, "localCidr": localCidr, "caName": caName, "caSha": caSha}, - ) - - b.addRule(proto, r) - return nil -} - -// buildRules turns the rules from AddRule into InRules and OutRules, no rules can be added after -func (f *Firewall) buildRules() error { - if f.inBuilder == nil || f.outBuilder == nil { - return errors.New("firewall rules are already built") - } - - f.InRules, f.OutRules = f.inBuilder.build(f), f.outBuilder.build(f) - f.inBuilder, f.outBuilder = nil, nil - return nil +// setRules builds rules into InRules and OutRules +func (f *Firewall) setRules(rb *firewall.RulesBuilder) { + rules := rb.Build(f.assignedNetworks, f.unsafeNetworks) + f.InRules, f.OutRules = rules.In, rules.Out + f.ruleHash, f.ruleHashFNV = rules.Hash, rules.HashFNV } // GetRuleHash returns a hash representation of all inbound and outbound rules func (f *Firewall) GetRuleHash() string { - sum := sha256.Sum256([]byte(f.rules)) - return hex.EncodeToString(sum[:]) + return f.ruleHash } // GetRuleHashFNV returns a uint32 FNV-1 hash representation the rules, for use as a metric value func (f *Firewall) GetRuleHashFNV() uint32 { - h := fnv.New32a() - h.Write([]byte(f.rules)) - return h.Sum32() + return f.ruleHashFNV } // GetRuleHashes returns both the sha256 and FNV-1 hashes, suitable for logging @@ -440,117 +211,6 @@ func (f *Firewall) GetRuleHashes() string { return "SHA:" + f.GetRuleHash() + ",FNV:" + strconv.FormatUint(uint64(f.GetRuleHashFNV()), 10) } -func AddFirewallRulesFromConfig(l *slog.Logger, inbound bool, c *config.C, fw FirewallInterface) error { - var table string - if inbound { - table = "firewall.inbound" - } else { - table = "firewall.outbound" - } - - r := c.Get(table) - if r == nil { - return nil - } - - rs, ok := r.([]any) - if !ok { - return fmt.Errorf("%s failed to parse, should be an array of rules", table) - } - - for i, t := range rs { - r, err := convertRule(l, t, table, i) - if err != nil { - return fmt.Errorf("%s rule #%v; %s", table, i, err) - } - - if r.Code != "" && r.Port != "" { - return fmt.Errorf("%s rule #%v; only one of port or code should be provided", table, i) - } - - if r.Host == "" && len(r.Groups) == 0 && r.Cidr == "" && r.LocalCidr == "" && r.CAName == "" && r.CASha == "" { - return fmt.Errorf("%s rule #%v; at least one of host, group, cidr, local_cidr, ca_name, or ca_sha must be provided", table, i) - } - - var sPort, errPort string - if r.Code != "" { - errPort = "code" - sPort = r.Code - } else { - errPort = "port" - sPort = r.Port - } - - var proto uint8 - switch r.Proto { - case "any": - proto = firewall.ProtoAny - case "tcp": - proto = iputil.IPProtocolTCP - case "udp": - proto = iputil.IPProtocolUDP - case "udplite": - proto = iputil.IPProtocolUDPLite - case "dccp": - proto = iputil.IPProtocolDCCP - case "sctp": - proto = iputil.IPProtocolSCTP - case "icmp": - proto = iputil.IPProtocolICMP - default: - // Any other protocol by number. 0 is reserved for `any`. - n, perr := strconv.ParseUint(r.Proto, 10, 8) - if perr != nil || n == 0 { - return fmt.Errorf("%s rule #%v; proto was not understood; `%s`", table, i, r.Proto) - } - proto = uint8(n) - } - - var startPort, endPort int32 - if proto == iputil.IPProtocolICMP || proto == iputil.IPProtocolICMPv6 { - startPort = firewall.PortAny - endPort = firewall.PortAny - if sPort != "" { - l.Warn("ignoring port specification for ICMP firewall rule", "port", sPort) - } - } else { - startPort, endPort, err = parsePort(sPort) - } - if err != nil { - return fmt.Errorf("%s rule #%v; %s %s", table, i, errPort, err) - } - - if r.Cidr != "" && r.Cidr != "any" { - _, err = netip.ParsePrefix(r.Cidr) - if err != nil { - return fmt.Errorf("%s rule #%v; cidr did not parse; %s", table, i, err) - } - } - - if r.LocalCidr != "" && r.LocalCidr != "any" { - _, err = netip.ParsePrefix(r.LocalCidr) - if err != nil { - return fmt.Errorf("%s rule #%v; local_cidr did not parse; %s", table, i, err) - } - } - - if warning := r.sanity(); warning != nil { - l.Warn("firewall rule sanity check", - "table", table, - "rule", i, - "warning", warning, - ) - } - - err = fw.AddRule(inbound, proto, startPort, endPort, r.Groups, r.Host, r.Cidr, r.LocalCidr, r.CAName, r.CASha) - if err != nil { - return fmt.Errorf("%s rule #%v; `%s`", table, i, err) - } - } - - return nil -} - var ErrUnknownNetworkType = errors.New("unknown network type") var ErrPeerRejected = errors.New("remote address is not within a network that we handle") var ErrInvalidRemoteIP = errors.New("remote address is not in remote certificate networks") @@ -604,7 +264,7 @@ func (f *Firewall) Drop(fp firewall.Packet, incoming bool, h *HostInfo, caPool * } // We now know which firewall table to check against - if !table.match(&fp, incoming, h.ConnectionState.peerCert, caPool) { + if !table.Match(&fp, incoming, h.ConnectionState.peerCert, caPool) { f.metrics(incoming).droppedNoRule.Inc(1) return ErrNoMatchingRule } @@ -670,7 +330,7 @@ func (f *Firewall) inConns(fp *firewall.Packet, h *HostInfo, caPool *cert.CAPool } // We now know which firewall table to check against - if !table.match(fp, c.incoming, h.ConnectionState.peerCert, caPool) { + if !table.Match(fp, c.incoming, h.ConnectionState.peerCert, caPool) { if f.l.Enabled(context.Background(), slog.LevelDebug) { h.logger(f.l).Debug("dropping old conntrack entry, does not match new ruleset", "fwPacket", *fp, @@ -762,417 +422,3 @@ func (f *Firewall) evict(p firewall.Packet) { // This conn is done delete(conntrack.Conns, p) } - -func (ft *FirewallTable) match(p *firewall.Packet, incoming bool, c *cert.CachedCertificate, caPool *cert.CAPool) bool { - return ft.Protos[p.Protocol].match(p, incoming, c, caPool) -} - -func (fp firewallPort) addRule(f *Firewall, r firewallPortRule) { - for i := r.startPort; i <= r.endPort; i++ { - if _, ok := fp[i]; !ok { - fp[i] = &FirewallCA{ - CANames: make(map[string]*FirewallRule), - CAShas: make(map[string]*FirewallRule), - } - } - - fp[i].addRule(f, r) - } -} - -func (fp firewallPort) match(p *firewall.Packet, incoming bool, c *cert.CachedCertificate, caPool *cert.CAPool) bool { - // We don't have any allowed ports, bail - if fp == nil { - return false - } - - // ICMP has no ports, only the port `any` rules apply, including the ones copied in from proto `any` rules - if p.Protocol == iputil.IPProtocolICMP || p.Protocol == iputil.IPProtocolICMPv6 { - // port numbers are re-used for connection tracking of ICMP, - // but we don't want to actually filter on them. - return fp[firewall.PortAny].match(p, c, caPool) - } - - var port int32 - - if p.Fragment { - port = firewall.PortFragment - } else if incoming { - port = int32(p.LocalPort) - } else { - port = int32(p.RemotePort) - } - - // Packets without ports (gre, esp, etc) have port 0, which is PortAny, don't check those rules twice - if port != firewall.PortAny && fp[port].match(p, c, caPool) { - return true - } - - return fp[firewall.PortAny].match(p, c, caPool) -} - -func (fc *FirewallCA) addRule(f *Firewall, r firewallPortRule) { - fr := func() *FirewallRule { - return &FirewallRule{ - Hosts: make(map[string]*firewallLocalCIDR), - Groups: make([]*firewallGroups, 0), - CIDR: new(bart.Table[*firewallLocalCIDR]), - } - } - - if r.caSha == "" && r.caName == "" { - if fc.Any == nil { - fc.Any = fr() - } - - fc.Any.addRule(f, r) - return - } - - if r.caSha != "" { - if _, ok := fc.CAShas[r.caSha]; !ok { - fc.CAShas[r.caSha] = fr() - } - fc.CAShas[r.caSha].addRule(f, r) - } - - if r.caName != "" { - if _, ok := fc.CANames[r.caName]; !ok { - fc.CANames[r.caName] = fr() - } - fc.CANames[r.caName].addRule(f, r) - } -} - -func (fc *FirewallCA) match(p *firewall.Packet, c *cert.CachedCertificate, caPool *cert.CAPool) bool { - if fc == nil { - return false - } - - if fc.Any.match(p, c) { - return true - } - - if t, ok := fc.CAShas[c.Certificate.Issuer()]; ok { - if t.match(p, c) { - return true - } - } - - s, err := caPool.GetCAForCert(c.Certificate) - if err != nil { - return false - } - - return fc.CANames[s.Certificate.Name()].match(p, c) -} - -func (fr *FirewallRule) addRule(f *Firewall, r firewallPortRule) { - flc := func() *firewallLocalCIDR { - return &firewallLocalCIDR{ - LocalCIDR: new(bart.Lite), - } - } - - if r.isAny() { - if fr.Any == nil { - fr.Any = flc() - } - - fr.Any.addRule(f, r) - return - } - - if len(r.groups) > 0 { - nlc := flc() - nlc.addRule(f, r) - - fr.Groups = append(fr.Groups, &firewallGroups{ - Groups: r.groups, - LocalCIDR: nlc, - }) - } - - if r.host != "" { - nlc := fr.Hosts[r.host] - if nlc == nil { - nlc = flc() - } - nlc.addRule(f, r) - fr.Hosts[r.host] = nlc - } - - if r.cidr.IsValid() { - nlc, _ := fr.CIDR.Get(r.cidr) - if nlc == nil { - nlc = flc() - } - nlc.addRule(f, r) - fr.CIDR.Insert(r.cidr, nlc) - } -} - -func (fr *FirewallRule) match(p *firewall.Packet, c *cert.CachedCertificate) bool { - if fr == nil { - return false - } - - // Shortcut path for if groups, hosts, or cidr contained an `any` - if fr.Any.match(p, c) { - return true - } - - // Need any of group, host, or cidr to match - for _, sg := range fr.Groups { - found := false - - for _, g := range sg.Groups { - if _, ok := c.InvertedGroups[g]; !ok { - found = false - break - } - - found = true - } - - if found && sg.LocalCIDR.match(p, c) { - return true - } - } - - if fr.Hosts != nil { - if flc, ok := fr.Hosts[c.Certificate.Name()]; ok { - if flc.match(p, c) { - return true - } - } - } - - for _, v := range fr.CIDR.Supernets(netip.PrefixFrom(p.RemoteAddr, p.RemoteAddr.BitLen())) { - if v.match(p, c) { - return true - } - } - - return false -} - -func (flc *firewallLocalCIDR) addRule(f *Firewall, r firewallPortRule) { - if r.anyLocalCIDR { - flc.Any = true - return - } - - if !r.localCIDR.IsValid() { - if len(f.unsafeNetworks) == 0 || f.defaultLocalCIDRAny { - flc.Any = true - return - } - - for _, network := range f.assignedNetworks { - flc.LocalCIDR.Insert(network) - } - return - } - - flc.LocalCIDR.Insert(r.localCIDR) -} - -func (flc *firewallLocalCIDR) match(p *firewall.Packet, c *cert.CachedCertificate) bool { - if flc == nil { - return false - } - - if flc.Any { - return true - } - - return flc.LocalCIDR.Contains(p.LocalAddr) -} - -type rule struct { - Port string - Code string - Proto string - Host string - Groups []string - Cidr string - LocalCidr string - CAName string - CASha string -} - -func convertRule(l *slog.Logger, p any, table string, i int) (rule, error) { - r := rule{} - - m, ok := p.(map[string]any) - if !ok { - return r, errors.New("could not parse rule") - } - - toString := func(k string, m map[string]any) string { - v, ok := m[k] - if !ok { - return "" - } - return fmt.Sprintf("%v", v) - } - - r.Port = toString("port", m) - r.Code = toString("code", m) - r.Proto = toString("proto", m) - r.Host = toString("host", m) - r.Cidr = toString("cidr", m) - r.LocalCidr = toString("local_cidr", m) - r.CAName = toString("ca_name", m) - r.CASha = toString("ca_sha", m) - - // Make sure group isn't an array - if v, ok := m["group"].([]any); ok { - if len(v) > 1 { - return r, errors.New("group should contain a single value, an array with more than one entry was provided") - } - - l.Warn("group was an array with a single value, converting to simple value", - "table", table, - "rule", i, - ) - m["group"] = v[0] - } - - singleGroup := toString("group", m) - - if rg, ok := m["groups"]; ok { - switch reflect.TypeOf(rg).Kind() { - case reflect.Slice: - v := reflect.ValueOf(rg) - r.Groups = make([]string, v.Len()) - for i := 0; i < v.Len(); i++ { - r.Groups[i] = v.Index(i).Interface().(string) - } - case reflect.String: - r.Groups = []string{rg.(string)} - default: - r.Groups = []string{fmt.Sprintf("%v", rg)} - } - } - - //flatten group vs groups - if singleGroup != "" { - // Check if we have both groups and group provided in the rule config - if len(r.Groups) > 0 { - return r, fmt.Errorf("only one of group or groups should be defined, both provided") - } - r.Groups = []string{singleGroup} - } - - return r, nil -} - -// sanity returns an error if the rule would be evaluated in a way that would short-circuit a configured check on a wildcard value -// rules are evaluated as "port AND proto AND (ca_sha OR ca_name) AND (host OR group OR groups OR cidr) AND local_cidr" -func (r *rule) sanity() error { - //port, proto, local_cidr are AND, no need to check here - //ca_sha and ca_name don't have a wildcard value, no need to check here - groupsEmpty := len(r.Groups) == 0 - hostEmpty := r.Host == "" - cidrEmpty := r.Cidr == "" - - if (groupsEmpty && hostEmpty && cidrEmpty) == true { - return nil //no content! - } - - groupsHasAny := slices.Contains(r.Groups, "any") - if groupsHasAny && len(r.Groups) > 1 { - return fmt.Errorf("groups spec [%s] contains the group '\"any\". This rule will ignore the other groups specified", r.Groups) - } - - if r.Host == "any" { - if !groupsEmpty { - return fmt.Errorf("groups specified as %s, but host=any will match any host, regardless of groups", r.Groups) - } - - if !cidrEmpty { - return fmt.Errorf("cidr specified as %s, but host=any will match any host, regardless of cidr", r.Cidr) - } - } - - if groupsHasAny { - if !hostEmpty && r.Host != "any" { - return fmt.Errorf("groups spec [%s] contains the group '\"any\". This rule will ignore the specified host %s", r.Groups, r.Host) - } - if !cidrEmpty { - return fmt.Errorf("groups spec [%s] contains the group '\"any\". This rule will ignore the specified cidr %s", r.Groups, r.Cidr) - } - } - - if r.Code != "" { - return fmt.Errorf("code specified as [%s]. Support for 'code' will be dropped in a future release, as it has never been functional", r.Code) - } - - //todo alert on cidr-any - - return nil -} - -func parsePort(s string) (int32, int32, error) { - const notAPort int32 = -2 - if s == "any" { - return firewall.PortAny, firewall.PortAny, nil - } - if s == "fragment" { - return firewall.PortFragment, firewall.PortFragment, nil - } - if !strings.Contains(s, `-`) { - rPort, err := parsePortValue("", s) - if err != nil { - return notAPort, notAPort, err - } - return rPort, rPort, nil - } - - sPorts := strings.SplitN(s, `-`, 2) - for i := range sPorts { - sPorts[i] = strings.Trim(sPorts[i], " ") - } - if len(sPorts) != 2 || sPorts[0] == "" || sPorts[1] == "" { - return notAPort, notAPort, fmt.Errorf("appears to be a range but could not be parsed; `%s`", s) - } - - startPort, err := parsePortValue("beginning range ", sPorts[0]) - if err != nil { - return notAPort, notAPort, err - } - - endPort, err := parsePortValue("ending range ", sPorts[1]) - if err != nil { - return notAPort, notAPort, err - } - - if startPort == firewall.PortAny { - endPort = firewall.PortAny - } - - return startPort, endPort, nil -} - -// parsePortValue accepts a base-10 decimal in [0, 65535] and returns it -// widened to int32. Using strconv.ParseUint with bitSize 16 rejects -// negative input, out-of-range input (>65535), and any non-decimal byte -// by construction, so the int32 widening that follows is provably safe -// and cannot collide with firewall.PortAny (0) or firewall.PortFragment -// (-1) via integer truncation. -// -// prefix is prepended to both error messages so callers can disambiguate -// the single-port path (prefix="") from the range bounds (prefix="beginning -// range " / "ending range "), preserving the historical error strings. -func parsePortValue(prefix, s string) (int32, error) { - n, err := strconv.ParseUint(s, 10, 16) - if err == nil { - return int32(n), nil - } - if errors.Is(err, strconv.ErrRange) { - return 0, fmt.Errorf("%sout of range [0,65535]; `%s`", prefix, s) - } - return 0, fmt.Errorf("%swas not a number; `%s`", prefix, s) -} diff --git a/firewall/config.go b/firewall/config.go new file mode 100644 index 00000000..968d2aeb --- /dev/null +++ b/firewall/config.go @@ -0,0 +1,333 @@ +package firewall + +import ( + "errors" + "fmt" + "log/slog" + "net/netip" + "reflect" + "slices" + "strconv" + "strings" + + "github.com/slackhq/nebula/config" + "github.com/slackhq/nebula/iputil" +) + +// ruleAdder takes firewall rules, it lets tests see what addRulesFromConfig adds +type ruleAdder interface { + AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, cidr, localCidr string, caName string, caSha string) error +} + +// RulesFromConfig reads the inbound and outbound firewall rules from c +func RulesFromConfig(l *slog.Logger, c *config.C) (*RulesBuilder, error) { + b := NewRulesBuilder(l) + b.defaultLocalCIDRAny = c.GetBool("firewall.default_local_cidr_any", false) + + if err := addRulesFromConfig(l, false, c, b); err != nil { + return nil, err + } + if err := addRulesFromConfig(l, true, c, b); err != nil { + return nil, err + } + + return b, nil +} + +func addRulesFromConfig(l *slog.Logger, inbound bool, c *config.C, ra ruleAdder) error { + var table string + if inbound { + table = "firewall.inbound" + } else { + table = "firewall.outbound" + } + + r := c.Get(table) + if r == nil { + return nil + } + + rs, ok := r.([]any) + if !ok { + return fmt.Errorf("%s failed to parse, should be an array of rules", table) + } + + for i, t := range rs { + r, err := convertRule(l, t, table, i) + if err != nil { + return fmt.Errorf("%s rule #%v; %s", table, i, err) + } + + if r.Code != "" && r.Port != "" { + return fmt.Errorf("%s rule #%v; only one of port or code should be provided", table, i) + } + + if r.Host == "" && len(r.Groups) == 0 && r.Cidr == "" && r.LocalCidr == "" && r.CAName == "" && r.CASha == "" { + return fmt.Errorf("%s rule #%v; at least one of host, group, cidr, local_cidr, ca_name, or ca_sha must be provided", table, i) + } + + var sPort, errPort string + if r.Code != "" { + errPort = "code" + sPort = r.Code + } else { + errPort = "port" + sPort = r.Port + } + + var proto uint8 + switch r.Proto { + case "any": + proto = ProtoAny + case "tcp": + proto = iputil.IPProtocolTCP + case "udp": + proto = iputil.IPProtocolUDP + case "udplite": + proto = iputil.IPProtocolUDPLite + case "dccp": + proto = iputil.IPProtocolDCCP + case "sctp": + proto = iputil.IPProtocolSCTP + case "icmp": + proto = iputil.IPProtocolICMP + default: + // Any other protocol by number. 0 is reserved for `any`. + n, perr := strconv.ParseUint(r.Proto, 10, 8) + if perr != nil || n == 0 { + return fmt.Errorf("%s rule #%v; proto was not understood; `%s`", table, i, r.Proto) + } + proto = uint8(n) + } + + var startPort, endPort int32 + if proto == iputil.IPProtocolICMP || proto == iputil.IPProtocolICMPv6 { + startPort = PortAny + endPort = PortAny + if sPort != "" { + l.Warn("ignoring port specification for ICMP firewall rule", "port", sPort) + } + } else { + startPort, endPort, err = parsePort(sPort) + } + if err != nil { + return fmt.Errorf("%s rule #%v; %s %s", table, i, errPort, err) + } + + if r.Cidr != "" && r.Cidr != "any" { + _, err = netip.ParsePrefix(r.Cidr) + if err != nil { + return fmt.Errorf("%s rule #%v; cidr did not parse; %s", table, i, err) + } + } + + if r.LocalCidr != "" && r.LocalCidr != "any" { + _, err = netip.ParsePrefix(r.LocalCidr) + if err != nil { + return fmt.Errorf("%s rule #%v; local_cidr did not parse; %s", table, i, err) + } + } + + if warning := r.sanity(); warning != nil { + l.Warn("firewall rule sanity check", + "table", table, + "rule", i, + "warning", warning, + ) + } + + err = ra.AddRule(inbound, proto, startPort, endPort, r.Groups, r.Host, r.Cidr, r.LocalCidr, r.CAName, r.CASha) + if err != nil { + return fmt.Errorf("%s rule #%v; `%s`", table, i, err) + } + } + + return nil +} + +type configRule struct { + Port string + Code string + Proto string + Host string + Groups []string + Cidr string + LocalCidr string + CAName string + CASha string +} + +func convertRule(l *slog.Logger, p any, table string, i int) (configRule, error) { + r := configRule{} + + m, ok := p.(map[string]any) + if !ok { + return r, errors.New("could not parse rule") + } + + toString := func(k string, m map[string]any) string { + v, ok := m[k] + if !ok { + return "" + } + return fmt.Sprintf("%v", v) + } + + r.Port = toString("port", m) + r.Code = toString("code", m) + r.Proto = toString("proto", m) + r.Host = toString("host", m) + r.Cidr = toString("cidr", m) + r.LocalCidr = toString("local_cidr", m) + r.CAName = toString("ca_name", m) + r.CASha = toString("ca_sha", m) + + // Make sure group isn't an array + if v, ok := m["group"].([]any); ok { + if len(v) > 1 { + return r, errors.New("group should contain a single value, an array with more than one entry was provided") + } + + l.Warn("group was an array with a single value, converting to simple value", + "table", table, + "rule", i, + ) + m["group"] = v[0] + } + + singleGroup := toString("group", m) + + if rg, ok := m["groups"]; ok { + switch reflect.TypeOf(rg).Kind() { + case reflect.Slice: + v := reflect.ValueOf(rg) + r.Groups = make([]string, v.Len()) + for i := 0; i < v.Len(); i++ { + r.Groups[i] = v.Index(i).Interface().(string) + } + case reflect.String: + r.Groups = []string{rg.(string)} + default: + r.Groups = []string{fmt.Sprintf("%v", rg)} + } + } + + //flatten group vs groups + if singleGroup != "" { + // Check if we have both groups and group provided in the rule config + if len(r.Groups) > 0 { + return r, fmt.Errorf("only one of group or groups should be defined, both provided") + } + r.Groups = []string{singleGroup} + } + + return r, nil +} + +// sanity returns an error if the rule would be evaluated in a way that would short-circuit a configured check on a wildcard value +// rules are evaluated as "port AND proto AND (ca_sha OR ca_name) AND (host OR group OR groups OR cidr) AND local_cidr" +func (r *configRule) sanity() error { + //port, proto, local_cidr are AND, no need to check here + //ca_sha and ca_name don't have a wildcard value, no need to check here + groupsEmpty := len(r.Groups) == 0 + hostEmpty := r.Host == "" + cidrEmpty := r.Cidr == "" + + if (groupsEmpty && hostEmpty && cidrEmpty) == true { + return nil //no content! + } + + groupsHasAny := slices.Contains(r.Groups, "any") + if groupsHasAny && len(r.Groups) > 1 { + return fmt.Errorf("groups spec [%s] contains the group '\"any\". This rule will ignore the other groups specified", r.Groups) + } + + if r.Host == "any" { + if !groupsEmpty { + return fmt.Errorf("groups specified as %s, but host=any will match any host, regardless of groups", r.Groups) + } + + if !cidrEmpty { + return fmt.Errorf("cidr specified as %s, but host=any will match any host, regardless of cidr", r.Cidr) + } + } + + if groupsHasAny { + if !hostEmpty && r.Host != "any" { + return fmt.Errorf("groups spec [%s] contains the group '\"any\". This rule will ignore the specified host %s", r.Groups, r.Host) + } + if !cidrEmpty { + return fmt.Errorf("groups spec [%s] contains the group '\"any\". This rule will ignore the specified cidr %s", r.Groups, r.Cidr) + } + } + + if r.Code != "" { + return fmt.Errorf("code specified as [%s]. Support for 'code' will be dropped in a future release, as it has never been functional", r.Code) + } + + //todo alert on cidr-any + + return nil +} + +func parsePort(s string) (int32, int32, error) { + const notAPort int32 = -2 + if s == "any" { + return PortAny, PortAny, nil + } + if s == "fragment" { + return PortFragment, PortFragment, nil + } + if !strings.Contains(s, `-`) { + rPort, err := parsePortValue("", s) + if err != nil { + return notAPort, notAPort, err + } + return rPort, rPort, nil + } + + sPorts := strings.SplitN(s, `-`, 2) + for i := range sPorts { + sPorts[i] = strings.Trim(sPorts[i], " ") + } + if len(sPorts) != 2 || sPorts[0] == "" || sPorts[1] == "" { + return notAPort, notAPort, fmt.Errorf("appears to be a range but could not be parsed; `%s`", s) + } + + startPort, err := parsePortValue("beginning range ", sPorts[0]) + if err != nil { + return notAPort, notAPort, err + } + + endPort, err := parsePortValue("ending range ", sPorts[1]) + if err != nil { + return notAPort, notAPort, err + } + + if startPort == PortAny { + endPort = PortAny + } + + return startPort, endPort, nil +} + +// parsePortValue accepts a base-10 decimal in [0, 65535] and returns it +// widened to int32. Using strconv.ParseUint with bitSize 16 rejects +// negative input, out-of-range input (>65535), and any non-decimal byte +// by construction, so the int32 widening that follows is provably safe +// and cannot collide with PortAny (0) or PortFragment +// (-1) via integer truncation. +// +// prefix is prepended to both error messages so callers can disambiguate +// the single-port path (prefix="") from the range bounds (prefix="beginning +// range " / "ending range "), preserving the historical error strings. +func parsePortValue(prefix, s string) (int32, error) { + n, err := strconv.ParseUint(s, 10, 16) + if err == nil { + return int32(n), nil + } + if errors.Is(err, strconv.ErrRange) { + return 0, fmt.Errorf("%sout of range [0,65535]; `%s`", prefix, s) + } + return 0, fmt.Errorf("%swas not a number; `%s`", prefix, s) +} diff --git a/firewall/config_test.go b/firewall/config_test.go new file mode 100644 index 00000000..a9a8cd6b --- /dev/null +++ b/firewall/config_test.go @@ -0,0 +1,423 @@ +package firewall + +import ( + "bytes" + "errors" + "net/netip" + "testing" + + "github.com/slackhq/nebula/config" + "github.com/slackhq/nebula/iputil" + "github.com/slackhq/nebula/test" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_parsePort(t *testing.T) { + _, _, err := parsePort("") + require.EqualError(t, err, "was not a number; ``") + + _, _, err = parsePort(" ") + require.EqualError(t, err, "was not a number; ` `") + + _, _, err = parsePort("-") + require.EqualError(t, err, "appears to be a range but could not be parsed; `-`") + + _, _, err = parsePort(" - ") + require.EqualError(t, err, "appears to be a range but could not be parsed; ` - `") + + _, _, err = parsePort("a-b") + require.EqualError(t, err, "beginning range was not a number; `a`") + + _, _, err = parsePort("1-b") + require.EqualError(t, err, "ending range was not a number; `b`") + + s, e, err := parsePort(" 1 - 2 ") + assert.Equal(t, int32(1), s) + assert.Equal(t, int32(2), e) + require.NoError(t, err) + + s, e, err = parsePort("0-1") + assert.Equal(t, int32(0), s) + assert.Equal(t, int32(0), e) + require.NoError(t, err) + + s, e, err = parsePort("9919") + assert.Equal(t, int32(9919), s) + assert.Equal(t, int32(9919), e) + require.NoError(t, err) + + s, e, err = parsePort("any") + assert.Equal(t, int32(0), s) + assert.Equal(t, int32(0), e) + require.NoError(t, err) +} + +// Test_parsePort_invalid covers inputs that must error. The named bug is +// that int32(strconv.Atoi("4294967296")) truncates to 0 == PortAny, +// silently turning a typo into a match-all-ports rule; the rest are +// representative syntax/range probes. +func Test_parsePort_invalid(t *testing.T) { + tests := []struct { + name string + input string + wantErrContains string + }{ + // Numeric overflow (the named bug + boundary). + {"named bug: 2^32 truncates to PortAny", "4294967296", "out of range"}, + {"just above max real port", "65536", "out of range"}, + + // Negatives route through the range branch and hit the empty-half + // guard; included as defense in depth so a future refactor cannot + // accidentally reach the int32 cast. + {"negative", "-1", "could not be parsed"}, + + // Syntax probes. + {"NUL between digits", "4\x002", "was not a number"}, + {"hex notation", "0x10", "was not a number"}, + {"scientific notation", "1e3", "was not a number"}, + {"leading whitespace", " 42", "was not a number"}, + {"fullwidth digits", "４２", "was not a number"}, + + // Range branch. + {"range upper out of range", "1-65536", "ending range out of range"}, + {"range lower out of range", "65536-65537", "beginning range out of range"}, + {"range with negative upper", "1--1", "ending range was not a number"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, _, err := parsePort(tc.input) + require.Error(t, err, "input %q must error", tc.input) + require.ErrorContains(t, err, tc.wantErrContains) + }) + } +} + +// Test_parsePort_valid_boundaries locks in success cases at 0, 1, and 65535 +// so a future refactor cannot regress the boundaries. +func Test_parsePort_valid_boundaries(t *testing.T) { + tests := []struct { + name string + input string + wantStart int32 + wantEnd int32 + }{ + {"zero is PortAny", "0", 0, 0}, + {"min real port", "1", 1, 1}, + {"max real port", "65535", 65535, 65535}, + {"range zero to max forces end to zero", "0-65535", 0, 0}, + {"range max to max", "65535-65535", 65535, 65535}, + {"range one to max", "1-65535", 1, 65535}, + {"range with whitespace inside", " 1 - 2 ", 1, 2}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + s, e, err := parsePort(tc.input) + require.NoError(t, err) + assert.Equal(t, tc.wantStart, s, "start port") + assert.Equal(t, tc.wantEnd, e, "end port") + }) + } +} + +func TestAddRulesFromConfig(t *testing.T) { + l := test.NewLogger() + // Test adding tcp rule + conf := config.NewC(test.NewLogger()) + mf := &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": "tcp", "host": "a"}}} + require.NoError(t, addRulesFromConfig(l, false, conf, mf)) + assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolTCP, startPort: 1, endPort: 1, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) + + // Test adding udp rule + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": "udp", "host": "a"}}} + require.NoError(t, addRulesFromConfig(l, false, conf, mf)) + assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolUDP, startPort: 1, endPort: 1, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) + + // Test adding udplite, dccp, and sctp rules + for name, proto := range map[string]uint8{"udplite": iputil.IPProtocolUDPLite, "dccp": iputil.IPProtocolDCCP, "sctp": iputil.IPProtocolSCTP} { + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1-2", "proto": name, "host": "a"}}} + require.NoError(t, addRulesFromConfig(l, false, conf, mf), name) + assert.Equal(t, addRuleCall{incoming: false, proto: proto, startPort: 1, endPort: 2, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall, name) + } + + // Test adding icmp rule + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": "icmp", "host": "a"}}} + require.NoError(t, addRulesFromConfig(l, false, conf, mf)) + assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolICMP, startPort: PortAny, endPort: PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) + + // Test adding icmp rule no port + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"proto": "icmp", "host": "a"}}} + require.NoError(t, addRulesFromConfig(l, false, conf, mf)) + assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolICMP, startPort: PortAny, endPort: PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) + + // Test adding rules by protocol number, as yaml would give a string or an int + for _, proto := range []any{"47", 47} { + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "any", "proto": proto, "host": "a"}}} + require.NoError(t, addRulesFromConfig(l, false, conf, mf), proto) + assert.Equal(t, addRuleCall{incoming: false, proto: 47, startPort: PortAny, endPort: PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall, proto) + } + + // Test ICMP and ICMPv6 by number still ignore ports + for _, proto := range []uint8{iputil.IPProtocolICMP, iputil.IPProtocolICMPv6} { + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": proto, "host": "a"}}} + require.NoError(t, addRulesFromConfig(l, false, conf, mf), proto) + assert.Equal(t, addRuleCall{incoming: false, proto: proto, startPort: PortAny, endPort: PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall, proto) + } + + // Test protocols that aren't a known name or a number from 1 to 255 + for _, proto := range []string{"0", "256", "-1", "gre"} { + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "any", "proto": proto, "host": "a"}}} + require.EqualError(t, addRulesFromConfig(l, false, conf, mf), "firewall.outbound rule #0; proto was not understood; `"+proto+"`") + } + + // Test adding any rule + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "host": "a"}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) + + // Test adding rule with cidr + cidr := netip.MustParsePrefix("10.0.0.0/8") + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": cidr.String()}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: cidr.String(), localIp: ""}, mf.lastCall) + + // Test adding rule with local_cidr + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": cidr.String()}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: cidr.String()}, mf.lastCall) + + // Test adding rule with cidr ipv6 + cidr6 := netip.MustParsePrefix("fd00::/8") + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": cidr6.String()}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: cidr6.String(), localIp: ""}, mf.lastCall) + + // Test adding rule with any cidr + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": "any"}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "any", localIp: ""}, mf.lastCall) + + // Test adding rule with junk cidr + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": "junk/junk"}}} + require.EqualError(t, addRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; cidr did not parse; netip.ParsePrefix(\"junk/junk\"): ParseAddr(\"junk\"): unable to parse IP") + + // Test adding rule with local_cidr ipv6 + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": cidr6.String()}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: cidr6.String()}, mf.lastCall) + + // Test adding rule with any local_cidr + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": "any"}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, localIp: "any"}, mf.lastCall) + + // Test adding rule with junk local_cidr + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": "junk/junk"}}} + require.EqualError(t, addRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; local_cidr did not parse; netip.ParsePrefix(\"junk/junk\"): ParseAddr(\"junk\"): unable to parse IP") + + // Test adding rule with ca_sha + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "ca_sha": "12312313123"}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: "", caSha: "12312313123"}, mf.lastCall) + + // Test adding rule with ca_name + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "ca_name": "root01"}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: "", caName: "root01"}, mf.lastCall) + + // Test single group + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "group": "a"}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: []string{"a"}, ip: "", localIp: ""}, mf.lastCall) + + // Test single groups + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "groups": "a"}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: []string{"a"}, ip: "", localIp: ""}, mf.lastCall) + + // Test multiple AND groups + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "groups": []string{"a", "b"}}}} + require.NoError(t, addRulesFromConfig(l, true, conf, mf)) + assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: []string{"a", "b"}, ip: "", localIp: ""}, mf.lastCall) + + // Test Add error + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + mf.nextCallReturn = errors.New("test error") + conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "host": "a"}}} + require.EqualError(t, addRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; `test error`") +} + +func Test_convertRule(t *testing.T) { + ob := &bytes.Buffer{} + l := test.NewLoggerWithOutput(ob) + + // Ensure group array of 1 is converted and a warning is printed + c := map[string]any{ + "group": []any{"group1"}, + } + + r, err := convertRule(l, c, "test", 1) + assert.Contains(t, ob.String(), "group was an array with a single value, converting to simple value") + assert.Contains(t, ob.String(), "table=test") + assert.Contains(t, ob.String(), "rule=1") + require.NoError(t, err) + assert.Equal(t, []string{"group1"}, r.Groups) + + // Ensure group array of > 1 is errord + ob.Reset() + c = map[string]any{ + "group": []any{"group1", "group2"}, + } + + r, err = convertRule(l, c, "test", 1) + assert.Empty(t, ob.String()) + require.Error(t, err, "group should contain a single value, an array with more than one entry was provided") + + // Make sure a well formed group is alright + ob.Reset() + c = map[string]any{ + "group": "group1", + } + + r, err = convertRule(l, c, "test", 1) + require.NoError(t, err) + assert.Equal(t, []string{"group1"}, r.Groups) +} + +func Test_convertRuleSanity(t *testing.T) { + ob := &bytes.Buffer{} + l := test.NewLoggerWithOutput(ob) + + noWarningPlease := []map[string]any{ + {"group": "group1"}, + {"groups": []any{"group2"}}, + {"host": "bob"}, + {"cidr": "1.1.1.1/1"}, + {"groups": []any{"group2"}, "host": "bob"}, + {"cidr": "1.1.1.1/1", "host": "bob"}, + {"groups": []any{"group2"}, "cidr": "1.1.1.1/1"}, + {"groups": []any{"group2"}, "cidr": "1.1.1.1/1", "host": "bob"}, + } + for _, c := range noWarningPlease { + r, err := convertRule(l, c, "test", 1) + require.NoError(t, err) + require.NoError(t, r.sanity(), "should not generate a sanity warning, %+v", c) + } + + yesWarningPlease := []map[string]any{ + {"group": "group1"}, + {"groups": []any{"group2"}}, + {"cidr": "1.1.1.1/1"}, + {"groups": []any{"group2"}, "host": "bob"}, + {"cidr": "1.1.1.1/1", "host": "bob"}, + {"groups": []any{"group2"}, "cidr": "1.1.1.1/1"}, + {"groups": []any{"group2"}, "cidr": "1.1.1.1/1", "host": "bob"}, + } + for _, c := range yesWarningPlease { + c["host"] = "any" + r, err := convertRule(l, c, "test", 1) + require.NoError(t, err) + err = r.sanity() + require.Error(t, err, "I wanted a warning: %+v", c) + } + //reset the list + yesWarningPlease = []map[string]any{ + {"group": "group1"}, + {"groups": []any{"group2"}}, + {"cidr": "1.1.1.1/1"}, + {"groups": []any{"group2"}, "host": "bob"}, + {"cidr": "1.1.1.1/1", "host": "bob"}, + {"groups": []any{"group2"}, "cidr": "1.1.1.1/1"}, + {"groups": []any{"group2"}, "cidr": "1.1.1.1/1", "host": "bob"}, + } + for _, c := range yesWarningPlease { + r, err := convertRule(l, c, "test", 1) + require.NoError(t, err) + r.Groups = append(r.Groups, "any") + err = r.sanity() + require.Error(t, err, "I wanted a warning: %+v", c) + } +} + +type addRuleCall struct { + incoming bool + proto uint8 + startPort int32 + endPort int32 + groups []string + host string + ip string + localIp string + caName string + caSha string +} + +type mockFirewall struct { + lastCall addRuleCall + nextCallReturn error +} + +func (mf *mockFirewall) AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, ip, localIp, caName string, caSha string) error { + mf.lastCall = addRuleCall{ + incoming: incoming, + proto: proto, + startPort: startPort, + endPort: endPort, + groups: groups, + host: host, + ip: ip, + localIp: localIp, + caName: caName, + caSha: caSha, + } + + err := mf.nextCallReturn + mf.nextCallReturn = nil + return err +} diff --git a/firewall/rules_builder.go b/firewall/rules_builder.go new file mode 100644 index 00000000..f33b2ff2 --- /dev/null +++ b/firewall/rules_builder.go @@ -0,0 +1,328 @@ +package firewall + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "hash" + "hash/fnv" + "io" + "log/slog" + "net/netip" + "slices" + + "github.com/gaissmai/bart" + "github.com/slackhq/nebula/iputil" +) + +// RulesBuilder collects the inbound and outbound rules, Build turns them into a Table for each direction +type RulesBuilder struct { + in, out tableRules + + // defaultLocalCIDRAny makes rules without a local cidr allow any local address, even with unsafe networks + defaultLocalCIDRAny bool + + // ruleSHA and ruleFNV hash every rule as it's added, see Rules + ruleSHA hash.Hash + ruleFNV hash.Hash32 + + l *slog.Logger +} + +// Rules are the Tables built for each direction, with hashes of the rules they were built from +type Rules struct { + In, Out *Table + + // Hash is a sha256 of the rules, HashFNV is an FNV-1a of them for use as a metric value. + // Rule sets with the same rules have the same hashes. + Hash string + HashFNV uint32 +} + +// tableRules are one direction's rules, waiting to be built into a Table +type tableRules struct { + anyRules []rule + // protoRules holds the rules for each protocol besides proto `any`, ICMPv6 rules are kept with ICMP + protoRules [256][]rule +} + +// rule is a parsed rule, so adding it to a Table can't fail +type rule struct { + startPort, endPort int32 + groups []string + host string + caName, caSha string + + // cidr is the remote cidr, not valid when the rule doesn't have one + cidr netip.Prefix + anyCIDR bool + + // localCIDRs are the local addresses the rule allows, anyLocalCIDR allows all of them. + // Neither is set for a rule without a local cidr until the Table is built and it gets the default. + localCIDRs []netip.Prefix + anyLocalCIDR bool +} + +func NewRulesBuilder(l *slog.Logger) *RulesBuilder { + return &RulesBuilder{ + ruleSHA: sha256.New(), + ruleFNV: fnv.New32a(), + l: l, + } +} + +// AddRule adds a rule for incoming or outgoing traffic, on proto or on every protocol when proto is ProtoAny. +// The rule is parsed now, so a bad rule is reported when it's added rather than when the Tables are built. +func (b *RulesBuilder) AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, cidr, localCidr, caName string, caSha string) error { + if proto == iputil.IPProtocolICMP || proto == iputil.IPProtocolICMPv6 { + //ICMP traffic doesn't have ports, so we always coerce to "any", even if a value is provided + if startPort != PortAny { + b.l.Warn("ignoring port specification for ICMP firewall rule", "startPort", startPort) + } + startPort = PortAny + endPort = PortAny + } + + r, err := parseRule(startPort, endPort, groups, host, cidr, localCidr, caName, caSha) + if err != nil { + return err + } + + if incoming { + b.in.add(proto, r) + } else { + b.out.add(proto, r) + } + + // The rule hashes are of these lines, changing them changes every hash + fmt.Fprintf(io.MultiWriter(b.ruleSHA, b.ruleFNV), + "incoming: %v, proto: %v, startPort: %v, endPort: %v, groups: %v, host: %v, ip: %v, localIp: %v, caName: %v, caSha: %s\n", + incoming, proto, startPort, endPort, groups, host, cidr, localCidr, caName, caSha, + ) + + direction := "incoming" + if !incoming { + direction = "outgoing" + } + b.l.Info("Firewall rule added", + "firewallRule", m{"direction": direction, "proto": proto, "startPort": startPort, "endPort": endPort, "groups": groups, "host": host, "cidr": cidr, "localCidr": localCidr, "caName": caName, "caSha": caSha}, + ) + + return nil +} + +// Build turns the rules into a Table for each direction, and hashes them. Rules without a local cidr allow +// vpnNetworks when there are unsafe networks, otherwise any local address. +func (b *RulesBuilder) Build(vpnNetworks, unsafeNetworks []netip.Prefix) Rules { + anyLocal := len(unsafeNetworks) == 0 || b.defaultLocalCIDRAny + return Rules{ + In: b.in.build(anyLocal, vpnNetworks), + Out: b.out.build(anyLocal, vpnNetworks), + Hash: hex.EncodeToString(b.ruleSHA.Sum(nil)), + HashFNV: b.ruleFNV.Sum32(), + } +} + +func parseRule(startPort, endPort int32, groups []string, host, cidr, localCidr, caName, caSha string) (rule, error) { + if startPort > endPort { + return rule{}, fmt.Errorf("start port was lower than end port") + } + + r := rule{ + startPort: startPort, + endPort: endPort, + groups: groups, + host: host, + caName: caName, + caSha: caSha, + } + + var err error + r.cidr, r.anyCIDR, err = parseCIDR(cidr) + if err != nil { + return rule{}, err + } + + var localCIDR netip.Prefix + localCIDR, r.anyLocalCIDR, err = parseCIDR(localCidr) + if err != nil { + return rule{}, err + } + if localCIDR.IsValid() { + r.localCIDRs = []netip.Prefix{localCIDR} + } + + return r, nil +} + +// add keeps r for proto, or for every protocol when proto is ProtoAny +func (tr *tableRules) add(proto uint8, r rule) { + switch proto { + case ProtoAny: + tr.anyRules = append(tr.anyRules, r) + case iputil.IPProtocolICMPv6: + tr.protoRules[iputil.IPProtocolICMP] = append(tr.protoRules[iputil.IPProtocolICMP], r) + default: + tr.protoRules[proto] = append(tr.protoRules[proto], r) + } +} + +// build turns the rules into a Table. Rules without a local cidr allow defaultLocalCIDRs, +// or any local address when defaultLocalAny is set. +func (tr *tableRules) build(defaultLocalAny bool, defaultLocalCIDRs []netip.Prefix) *Table { + add := func(pr portRules, rules []rule) { + for _, r := range rules { + if !r.anyLocalCIDR && r.localCIDRs == nil { + r.anyLocalCIDR, r.localCIDRs = defaultLocalAny, defaultLocalCIDRs + } + pr.add(r) + } + } + + var anyProto portRules + if len(tr.anyRules) > 0 { + anyProto = portRules{} + add(anyProto, tr.anyRules) + } + + t := &Table{} + for proto := range t.Protos { + rules := tr.protoRules[proto] + if len(rules) == 0 { + t.Protos[proto] = anyProto + continue + } + + pr := portRules{} + add(pr, tr.anyRules) + add(pr, rules) + t.Protos[proto] = pr + } + t.Protos[iputil.IPProtocolICMPv6] = t.Protos[iputil.IPProtocolICMP] + + return t +} + +// parseCIDR parses a rule's cidr, which is empty, `any`, or a prefix +func parseCIDR(s string) (prefix netip.Prefix, isAny bool, err error) { + switch s { + case "": + return netip.Prefix{}, false, nil + case "any": + return netip.Prefix{}, true, nil + } + + prefix, err = netip.ParsePrefix(s) + return prefix, false, err +} + +// isAny reports whether r matches any host, making its groups, host, and cidr irrelevant +func (r rule) isAny() bool { + if len(r.groups) == 0 && r.host == "" && !r.cidr.IsValid() { + return true + } + + return slices.Contains(r.groups, "any") || r.host == "any" || r.anyCIDR +} + +func (pr portRules) add(r rule) { + for i := r.startPort; i <= r.endPort; i++ { + if _, ok := pr[i]; !ok { + pr[i] = &caRules{ + CANames: make(map[string]*remoteRules), + CAShas: make(map[string]*remoteRules), + } + } + + pr[i].add(r) + } +} + +func (cr *caRules) add(r rule) { + newRemoteRules := func() *remoteRules { + return &remoteRules{ + Hosts: make(map[string]*localRules), + Groups: make([]*groupsRule, 0), + CIDR: new(bart.Table[*localRules]), + } + } + + if r.caSha == "" && r.caName == "" { + if cr.Any == nil { + cr.Any = newRemoteRules() + } + + cr.Any.add(r) + return + } + + if r.caSha != "" { + if _, ok := cr.CAShas[r.caSha]; !ok { + cr.CAShas[r.caSha] = newRemoteRules() + } + cr.CAShas[r.caSha].add(r) + } + + if r.caName != "" { + if _, ok := cr.CANames[r.caName]; !ok { + cr.CANames[r.caName] = newRemoteRules() + } + cr.CANames[r.caName].add(r) + } +} + +func (rr *remoteRules) add(r rule) { + newLocalRules := func() *localRules { + return &localRules{ + LocalCIDR: new(bart.Lite), + } + } + + if r.isAny() { + if rr.Any == nil { + rr.Any = newLocalRules() + } + + rr.Any.add(r) + return + } + + if len(r.groups) > 0 { + lr := newLocalRules() + lr.add(r) + + rr.Groups = append(rr.Groups, &groupsRule{ + Groups: r.groups, + LocalCIDR: lr, + }) + } + + if r.host != "" { + lr := rr.Hosts[r.host] + if lr == nil { + lr = newLocalRules() + } + lr.add(r) + rr.Hosts[r.host] = lr + } + + if r.cidr.IsValid() { + lr, _ := rr.CIDR.Get(r.cidr) + if lr == nil { + lr = newLocalRules() + } + lr.add(r) + rr.CIDR.Insert(r.cidr, lr) + } +} + +func (lr *localRules) add(r rule) { + if r.anyLocalCIDR { + lr.Any = true + return + } + + for _, prefix := range r.localCIDRs { + lr.LocalCIDR.Insert(prefix) + } +} diff --git a/firewall/table.go b/firewall/table.go new file mode 100644 index 00000000..a626bbb3 --- /dev/null +++ b/firewall/table.go @@ -0,0 +1,164 @@ +package firewall + +import ( + "net/netip" + + "github.com/gaissmai/bart" + "github.com/slackhq/nebula/cert" + "github.com/slackhq/nebula/iputil" +) + +// Table holds the rules for one direction, the evaluation order is: +// Proto AND port AND (CA SHA or CA name) AND local CIDR AND (group OR groups OR name OR remote CIDR) +// A Table doesn't change once built, see RulesBuilder. +type Table struct { + // Protos holds the rules for each IP protocol number, so a packet is only checked against one set of rules. + // A protocol with rules of its own also holds a copy of every proto `any` rule, the rest share the proto `any` + // rules, which are at Protos[ProtoAny]. nil when a protocol has no rules at all. + // ICMP and ICMPv6 share one set of rules. + Protos [256]portRules +} + +// Even though ports are uint16, int32 maps are faster for lookup +// Plus we can use `-1` for fragment rules +type portRules map[int32]*caRules + +type caRules struct { + Any *remoteRules + CANames map[string]*remoteRules + CAShas map[string]*remoteRules +} + +type remoteRules struct { + // Any makes Hosts, Groups, and CIDR irrelevant + Any *localRules + Hosts map[string]*localRules + Groups []*groupsRule + CIDR *bart.Table[*localRules] +} + +type groupsRule struct { + Groups []string + LocalCIDR *localRules +} + +type localRules struct { + Any bool + LocalCIDR *bart.Lite +} + +// Match reports whether a rule allows p +func (t *Table) Match(p *Packet, incoming bool, c *cert.CachedCertificate, caPool *cert.CAPool) bool { + return t.Protos[p.Protocol].match(p, incoming, c, caPool) +} + +func (pr portRules) match(p *Packet, incoming bool, c *cert.CachedCertificate, caPool *cert.CAPool) bool { + // We don't have any allowed ports, bail + if pr == nil { + return false + } + + // ICMP has no ports, only the port `any` rules apply, including the ones copied in from proto `any` rules + if p.Protocol == iputil.IPProtocolICMP || p.Protocol == iputil.IPProtocolICMPv6 { + // port numbers are re-used for connection tracking of ICMP, + // but we don't want to actually filter on them. + return pr[PortAny].match(p, c, caPool) + } + + var port int32 + + if p.Fragment { + port = PortFragment + } else if incoming { + port = int32(p.LocalPort) + } else { + port = int32(p.RemotePort) + } + + // Packets without ports (gre, esp, etc) have port 0, which is PortAny, don't check those rules twice + if port != PortAny && pr[port].match(p, c, caPool) { + return true + } + + return pr[PortAny].match(p, c, caPool) +} + +func (cr *caRules) match(p *Packet, c *cert.CachedCertificate, caPool *cert.CAPool) bool { + if cr == nil { + return false + } + + if cr.Any.match(p, c) { + return true + } + + if t, ok := cr.CAShas[c.Certificate.Issuer()]; ok { + if t.match(p, c) { + return true + } + } + + s, err := caPool.GetCAForCert(c.Certificate) + if err != nil { + return false + } + + return cr.CANames[s.Certificate.Name()].match(p, c) +} + +func (rr *remoteRules) match(p *Packet, c *cert.CachedCertificate) bool { + if rr == nil { + return false + } + + // Shortcut path for if groups, hosts, or cidr contained an `any` + if rr.Any.match(p, c) { + return true + } + + // Need any of group, host, or cidr to match + for _, sg := range rr.Groups { + found := false + + for _, g := range sg.Groups { + if _, ok := c.InvertedGroups[g]; !ok { + found = false + break + } + + found = true + } + + if found && sg.LocalCIDR.match(p, c) { + return true + } + } + + if rr.Hosts != nil { + if lr, ok := rr.Hosts[c.Certificate.Name()]; ok { + if lr.match(p, c) { + return true + } + } + } + + for _, v := range rr.CIDR.Supernets(netip.PrefixFrom(p.RemoteAddr, p.RemoteAddr.BitLen())) { + if v.match(p, c) { + return true + } + } + + return false +} + +func (lr *localRules) match(p *Packet, c *cert.CachedCertificate) bool { + if lr == nil { + return false + } + + if lr.Any { + return true + } + + return lr.LocalCIDR.Contains(p.LocalAddr) +} diff --git a/firewall_test.go b/firewall_test.go index a77bf374..3206e9f5 100644 --- a/firewall_test.go +++ b/firewall_test.go @@ -2,7 +2,6 @@ package nebula import ( "bytes" - "errors" "log/slog" "math" "net/netip" @@ -72,6 +71,7 @@ func TestFirewall_AddRule(t *testing.T) { c := &dummyCert{} fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c) + rb := firewall.NewRulesBuilder(l) assert.NotNil(t, fw.InRules) assert.NotNil(t, fw.OutRules) @@ -81,24 +81,26 @@ func TestFirewall_AddRule(t *testing.T) { ti6, err := netip.ParsePrefix("fd12::34/128") require.NoError(t, err) - require.NoError(t, fw.AddRule(true, iputil.IPProtocolTCP, 1, 1, []string{}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolTCP, 1, 1, []string{}, "", "", "", "", "")) + fw.setRules(rb) // An empty rule is any assert.True(t, fw.InRules.Protos[iputil.IPProtocolTCP][1].Any.Any.Any) assert.Empty(t, fw.InRules.Protos[iputil.IPProtocolTCP][1].Any.Groups) assert.Empty(t, fw.InRules.Protos[iputil.IPProtocolTCP][1].Any.Hosts) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "", "")) + fw.setRules(rb) assert.Nil(t, fw.InRules.Protos[iputil.IPProtocolUDP][1].Any.Any) assert.Contains(t, fw.InRules.Protos[iputil.IPProtocolUDP][1].Any.Groups[0].Groups, "g1") assert.Empty(t, fw.InRules.Protos[iputil.IPProtocolUDP][1].Any.Hosts) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(true, iputil.IPProtocolICMP, 1, 1, []string{}, "h1", "", "", "", "")) - require.NoError(t, fw.AddRule(true, iputil.IPProtocolICMPv6, 1, 1, []string{}, "h2", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolICMP, 1, 1, []string{}, "h1", "", "", "", "")) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolICMPv6, 1, 1, []string{}, "h2", "", "", "", "")) + fw.setRules(rb) //no matter what port is given for icmp, it should end up as "any" assert.Nil(t, fw.InRules.Protos[iputil.IPProtocolICMP][firewall.PortAny].Any.Any) assert.Empty(t, fw.InRules.Protos[iputil.IPProtocolICMP][firewall.PortAny].Any.Groups) @@ -110,60 +112,69 @@ func TestFirewall_AddRule(t *testing.T) { // Any other protocol number gets its own rules fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(true, 47, 0, 0, []string{"g1"}, "", "", "", "", "")) // GRE - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, 47, 0, 0, []string{"g1"}, "", "", "", "", "")) // GRE + fw.setRules(rb) assert.Contains(t, fw.InRules.Protos[47][firewall.PortAny].Any.Groups[0].Groups, "g1") assert.Nil(t, fw.InRules.Protos[iputil.IPProtocolTCP]) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", ti.String(), "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", ti.String(), "", "", "")) + fw.setRules(rb) assert.Nil(t, fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any) _, ok := fw.OutRules.Protos[firewall.ProtoAny][1].Any.CIDR.Get(ti) assert.True(t, ok) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", ti6.String(), "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", ti6.String(), "", "", "")) + fw.setRules(rb) assert.Nil(t, fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any) _, ok = fw.OutRules.Protos[firewall.ProtoAny][1].Any.CIDR.Get(ti6) assert.True(t, ok) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", "", ti.String(), "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", "", ti.String(), "", "")) + fw.setRules(rb) assert.NotNil(t, fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any) ok = fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any.LocalCIDR.Get(ti) assert.True(t, ok) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", "", ti6.String(), "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", "", ti6.String(), "", "")) + fw.setRules(rb) assert.NotNil(t, fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any) ok = fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any.LocalCIDR.Get(ti6) assert.True(t, ok) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "ca-name", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "ca-name", "")) + fw.setRules(rb) assert.Contains(t, fw.InRules.Protos[iputil.IPProtocolUDP][1].CANames, "ca-name") fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "", "ca-sha")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "", "ca-sha")) + fw.setRules(rb) assert.Contains(t, fw.InRules.Protos[iputil.IPProtocolUDP][1].CAShas, "ca-sha") fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "any", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "any", "", "", "", "")) + fw.setRules(rb) assert.True(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.Any) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) + rb = firewall.NewRulesBuilder(l) anyIp, err := netip.ParsePrefix("0.0.0.0/0") require.NoError(t, err) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", anyIp.String(), "", "", "")) - require.NoError(t, fw.buildRules()) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", anyIp.String(), "", "", "")) + fw.setRules(rb) assert.Nil(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any) table, ok := fw.OutRules.Protos[firewall.ProtoAny][0].Any.CIDR.Lookup(netip.MustParseAddr("1.1.1.1")) assert.True(t, table.Any) @@ -171,11 +182,12 @@ func TestFirewall_AddRule(t *testing.T) { assert.False(t, ok) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) + rb = firewall.NewRulesBuilder(l) anyIp6, err := netip.ParsePrefix("::/0") require.NoError(t, err) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", anyIp6.String(), "", "", "")) - require.NoError(t, fw.buildRules()) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", anyIp6.String(), "", "", "")) + fw.setRules(rb) assert.Nil(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any) table, ok = fw.OutRules.Protos[firewall.ProtoAny][0].Any.CIDR.Lookup(netip.MustParseAddr("9::9")) assert.True(t, table.Any) @@ -183,50 +195,53 @@ func TestFirewall_AddRule(t *testing.T) { assert.False(t, ok) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "any", "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "any", "", "", "")) + fw.setRules(rb) assert.True(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.Any) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "", anyIp.String(), "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "", anyIp.String(), "", "")) + fw.setRules(rb) assert.False(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.Any) assert.True(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("1.1.1.1"))) assert.False(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("9::9"))) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "", anyIp6.String(), "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "", anyIp6.String(), "", "")) + fw.setRules(rb) assert.False(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.Any) assert.True(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("9::9"))) assert.False(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("1.1.1.1"))) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "", "any", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "", "any", "", "")) + fw.setRules(rb) assert.True(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.Any) // Every protocol number is accepted fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(true, math.MaxUint8, 0, 0, []string{}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, math.MaxUint8, 0, 0, []string{}, "", "", "", "", "")) + fw.setRules(rb) assert.True(t, fw.InRules.Protos[math.MaxUint8][firewall.PortAny].Any.Any.Any) // Test error conditions, a bad rule is reported when it's added - fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.Error(t, fw.AddRule(true, firewall.ProtoAny, 10, 0, []string{}, "", "", "", "", "")) - require.Error(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{}, "", "junk", "", "", "")) - require.Error(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{}, "", "", "junk", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.Error(t, rb.AddRule(true, firewall.ProtoAny, 10, 0, []string{}, "", "", "", "", "")) + require.Error(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{}, "", "junk", "", "", "")) + require.Error(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{}, "", "", "junk", "", "")) - // Rules take effect once built, and can't change after + // A new firewall allows nothing until its rules are set fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) assert.Nil(t, fw.InRules.Protos[iputil.IPProtocolTCP]) - require.NoError(t, fw.buildRules()) + fw.setRules(rb) assert.NotNil(t, fw.InRules.Protos[iputil.IPProtocolTCP]) - require.Error(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) - require.Error(t, fw.buildRules()) } func TestFirewall_Drop(t *testing.T) { @@ -261,8 +276,9 @@ func TestFirewall_Drop(t *testing.T) { h.buildNetworks(myVpnNetworksTable, &c) fw := NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) cp := cert.NewCAPool() // Drop outbound @@ -281,32 +297,36 @@ func TestFirewall_Drop(t *testing.T) { // ensure signer doesn't get in the way of group checks fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum-bad")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum-bad")) + fw.setRules(rb) assert.Equal(t, fw.Drop(p, true, &h, cp, nil), ErrNoMatchingRule) // test caSha doesn't drop on match fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum-bad")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum-bad")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum")) + fw.setRules(rb) require.NoError(t, fw.Drop(p, true, &h, cp, nil)) // ensure ca name doesn't get in the way of group checks cp.CAs["signer-shasum"] = &cert.CachedCertificate{Certificate: &dummyCert{name: "ca-good"}} fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good", "")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good-bad", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good", "")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good-bad", "")) + fw.setRules(rb) assert.Equal(t, fw.Drop(p, true, &h, cp, nil), ErrNoMatchingRule) // test caName doesn't drop on match cp.CAs["signer-shasum"] = &cert.CachedCertificate{Certificate: &dummyCert{name: "ca-good"}} fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good-bad", "")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good-bad", "")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good", "")) + fw.setRules(rb) require.NoError(t, fw.Drop(p, true, &h, cp, nil)) } @@ -344,8 +364,9 @@ func TestFirewall_DropV6(t *testing.T) { h.buildNetworks(myVpnNetworksTable, &c) fw := NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) cp := cert.NewCAPool() // Drop outbound @@ -364,47 +385,50 @@ func TestFirewall_DropV6(t *testing.T) { // ensure signer doesn't get in the way of group checks fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum-bad")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum-bad")) + fw.setRules(rb) assert.Equal(t, fw.Drop(p, true, &h, cp, nil), ErrNoMatchingRule) // test caSha doesn't drop on match fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum-bad")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum-bad")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum")) + fw.setRules(rb) require.NoError(t, fw.Drop(p, true, &h, cp, nil)) // ensure ca name doesn't get in the way of group checks cp.CAs["signer-shasum"] = &cert.CachedCertificate{Certificate: &dummyCert{name: "ca-good"}} fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good", "")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good-bad", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good", "")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good-bad", "")) + fw.setRules(rb) assert.Equal(t, fw.Drop(p, true, &h, cp, nil), ErrNoMatchingRule) // test caName doesn't drop on match cp.CAs["signer-shasum"] = &cert.CachedCertificate{Certificate: &dummyCert{name: "ca-good"}} fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good-bad", "")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good-bad", "")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good", "")) + fw.setRules(rb) require.NoError(t, fw.Drop(p, true, &h, cp, nil)) } func BenchmarkFirewallTable_match(b *testing.B) { - f := &Firewall{} - ftb := &FirewallTableBuilder{} + rb := firewall.NewRulesBuilder(test.NewLogger()) pfix := netip.MustParsePrefix("172.1.1.1/32") - ftb.addRule(iputil.IPProtocolTCP, firewallPortRule{startPort: 10, endPort: 10, groups: []string{"good-group"}, host: "good-host", cidr: pfix}) - ftb.addRule(iputil.IPProtocolTCP, firewallPortRule{startPort: 100, endPort: 100, groups: []string{"good-group"}, host: "good-host", localCIDR: pfix}) + require.NoError(b, rb.AddRule(true, iputil.IPProtocolTCP, 10, 10, []string{"good-group"}, "good-host", pfix.String(), "", "", "")) + require.NoError(b, rb.AddRule(true, iputil.IPProtocolTCP, 100, 100, []string{"good-group"}, "good-host", "", pfix.String(), "", "")) pfix6 := netip.MustParsePrefix("fd11::11/128") - ftb.addRule(iputil.IPProtocolTCP, firewallPortRule{startPort: 10, endPort: 10, groups: []string{"good-group"}, host: "good-host", cidr: pfix6}) - ftb.addRule(iputil.IPProtocolTCP, firewallPortRule{startPort: 100, endPort: 100, groups: []string{"good-group"}, host: "good-host", localCIDR: pfix6}) - ft := ftb.build(f) + require.NoError(b, rb.AddRule(true, iputil.IPProtocolTCP, 10, 10, []string{"good-group"}, "good-host", pfix6.String(), "", "", "")) + require.NoError(b, rb.AddRule(true, iputil.IPProtocolTCP, 100, 100, []string{"good-group"}, "good-host", "", pfix6.String(), "", "")) + ft := rb.Build(nil, nil).In cp := cert.NewCAPool() b.Run("fail on proto", func(b *testing.B) { @@ -413,7 +437,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { Certificate: &dummyCert{}, } for n := 0; n < b.N; n++ { - assert.False(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolUDP}, true, c, cp)) + assert.False(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolUDP}, true, c, cp)) } }) @@ -423,7 +447,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { Certificate: &dummyCert{}, } for n := 0; n < b.N; n++ { - assert.False(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 1}, true, c, cp)) + assert.False(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 1}, true, c, cp)) } }) @@ -433,7 +457,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { } ip := netip.MustParsePrefix("9.254.254.254/32") for n := 0; n < b.N; n++ { - assert.False(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: ip.Addr()}, true, c, cp)) + assert.False(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: ip.Addr()}, true, c, cp)) } }) b.Run("pass proto, port, fail on local CIDRv6", func(b *testing.B) { @@ -442,7 +466,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { } ip := netip.MustParsePrefix("fd99::99/128") for n := 0; n < b.N; n++ { - assert.False(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: ip.Addr()}, true, c, cp)) + assert.False(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: ip.Addr()}, true, c, cp)) } }) @@ -455,7 +479,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { InvertedGroups: map[string]struct{}{"nope": {}}, } for n := 0; n < b.N; n++ { - assert.False(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 10}, true, c, cp)) + assert.False(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 10}, true, c, cp)) } }) b.Run("pass proto, port, any local CIDRv6, fail all group, name, and cidr", func(b *testing.B) { @@ -467,7 +491,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { InvertedGroups: map[string]struct{}{"nope": {}}, } for n := 0; n < b.N; n++ { - assert.False(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 10}, true, c, cp)) + assert.False(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 10}, true, c, cp)) } }) @@ -480,7 +504,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { InvertedGroups: map[string]struct{}{"nope": {}}, } for n := 0; n < b.N; n++ { - assert.False(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: pfix.Addr()}, true, c, cp)) + assert.False(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: pfix.Addr()}, true, c, cp)) } }) b.Run("pass proto, port, specific local CIDRv6, fail all group, name, and cidr", func(b *testing.B) { @@ -492,7 +516,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { InvertedGroups: map[string]struct{}{"nope": {}}, } for n := 0; n < b.N; n++ { - assert.False(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: pfix6.Addr()}, true, c, cp)) + assert.False(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: pfix6.Addr()}, true, c, cp)) } }) @@ -504,7 +528,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { InvertedGroups: map[string]struct{}{"good-group": {}}, } for n := 0; n < b.N; n++ { - assert.True(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 10}, true, c, cp)) + assert.True(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 10}, true, c, cp)) } }) @@ -516,7 +540,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { InvertedGroups: map[string]struct{}{"good-group": {}}, } for n := 0; n < b.N; n++ { - assert.True(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: pfix.Addr()}, true, c, cp)) + assert.True(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: pfix.Addr()}, true, c, cp)) } }) b.Run("pass on group on specific local cidr6", func(b *testing.B) { @@ -527,7 +551,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { InvertedGroups: map[string]struct{}{"good-group": {}}, } for n := 0; n < b.N; n++ { - assert.True(b, ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: pfix6.Addr()}, true, c, cp)) + assert.True(b, ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 100, LocalAddr: pfix6.Addr()}, true, c, cp)) } }) @@ -539,7 +563,7 @@ func BenchmarkFirewallTable_match(b *testing.B) { InvertedGroups: map[string]struct{}{"nope": {}}, } for n := 0; n < b.N; n++ { - ft.match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 10}, true, c, cp) + ft.Match(&firewall.Packet{Protocol: iputil.IPProtocolTCP, LocalPort: 10}, true, c, cp) } }) } @@ -592,8 +616,9 @@ func TestFirewall_Drop2(t *testing.T) { h1.buildNetworks(myVpnNetworksTable, c1.Certificate) fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group", "test-group"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group", "test-group"}, "", "", "", "", "")) + fw.setRules(rb) cp := cert.NewCAPool() // h1/c1 lacks the proper groups @@ -672,9 +697,10 @@ func TestFirewall_Drop3(t *testing.T) { h3.buildNetworks(myVpnNetworksTable, c3.Certificate) fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "host1", "", "", "", "")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "", "", "", "signer-sha")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "host1", "", "", "", "")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "", "", "", "signer-sha")) + fw.setRules(rb) cp := cert.NewCAPool() // c1 should pass because host match @@ -688,8 +714,9 @@ func TestFirewall_Drop3(t *testing.T) { // Test a remote address match fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "1.2.3.4/24", "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "1.2.3.4/24", "", "", "")) + fw.setRules(rb) require.NoError(t, fw.Drop(p, true, &h1, cp, nil)) } @@ -725,9 +752,10 @@ func TestFirewall_Drop3V6(t *testing.T) { // Test a remote address match fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) + rb := firewall.NewRulesBuilder(l) cp := cert.NewCAPool() - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "fd12::34/120", "", "", "")) - require.NoError(t, fw.buildRules()) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "fd12::34/120", "", "", "")) + fw.setRules(rb) require.NoError(t, fw.Drop(p, true, &h, cp, nil)) } @@ -765,8 +793,9 @@ func TestFirewall_DropConntrackReload(t *testing.T) { h.buildNetworks(myVpnNetworksTable, c.Certificate) fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) cp := cert.NewCAPool() // Drop outbound @@ -779,8 +808,9 @@ func TestFirewall_DropConntrackReload(t *testing.T) { oldFw := fw fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 10, 10, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 10, 10, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) fw.Conntrack = oldFw.Conntrack fw.rulesVersion = oldFw.rulesVersion + 1 @@ -789,8 +819,9 @@ func TestFirewall_DropConntrackReload(t *testing.T) { oldFw = fw fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 11, 11, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 11, 11, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) fw.Conntrack = oldFw.Conntrack fw.rulesVersion = oldFw.rulesVersion + 1 @@ -834,8 +865,9 @@ func TestFirewall_ICMPPortBehavior(t *testing.T) { t.Run("ICMP allowed", func(t *testing.T) { fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, iputil.IPProtocolICMP, 0, 0, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolICMP, 0, 0, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) t.Run("zero ports", func(t *testing.T) { p := templ.Copy() p.LocalPort = 0 @@ -865,8 +897,9 @@ func TestFirewall_ICMPPortBehavior(t *testing.T) { t.Run("Any proto, some ports allowed", func(t *testing.T) { fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 80, 444, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 80, 444, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) t.Run("zero ports, still blocked", func(t *testing.T) { p := templ.Copy() p.LocalPort = 0 @@ -908,8 +941,9 @@ func TestFirewall_ICMPPortBehavior(t *testing.T) { }) t.Run("Any proto, any port", func(t *testing.T) { fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) t.Run("zero ports, allowed", func(t *testing.T) { resetConntrack(fw) p := templ.Copy() @@ -973,8 +1007,9 @@ func TestFirewall_ProtocolRules(t *testing.T) { protos := []uint8{iputil.IPProtocolTCP, iputil.IPProtocolUDP, iputil.IPProtocolUDPLite, iputil.IPProtocolDCCP, iputil.IPProtocolSCTP} for _, ruleProto := range protos { fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, ruleProto, 80, 80, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, ruleProto, 80, 80, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) for _, pktProto := range protos { p := firewall.Packet{ @@ -1004,8 +1039,9 @@ func TestFirewall_ProtocolRules(t *testing.T) { // A rule on any other protocol number only matches that protocol fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, 47, firewall.PortAny, firewall.PortAny, []string{"any"}, "", "", "", "", "")) // GRE - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, 47, firewall.PortAny, firewall.PortAny, []string{"any"}, "", "", "", "", "")) // GRE + fw.setRules(rb) require.NoError(t, fw.Drop(pkt(47), true, &h, cp, nil)) assert.Equal(t, ErrNoMatchingRule, fw.Drop(pkt(50), true, &h, cp, nil)) // ESP assert.Equal(t, ErrNoMatchingRule, fw.Drop(pkt(iputil.IPProtocolTCP), true, &h, cp, nil)) @@ -1013,8 +1049,9 @@ func TestFirewall_ProtocolRules(t *testing.T) { // A rule on either ICMP or ICMPv6 matches both for _, ruleProto := range []uint8{iputil.IPProtocolICMP, iputil.IPProtocolICMPv6} { fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, ruleProto, firewall.PortAny, firewall.PortAny, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, ruleProto, firewall.PortAny, firewall.PortAny, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) require.NoError(t, fw.Drop(pkt(iputil.IPProtocolICMP), true, &h, cp, nil), "rule %d", ruleProto) require.NoError(t, fw.Drop(pkt(iputil.IPProtocolICMPv6), true, &h, cp, nil), "rule %d", ruleProto) assert.Equal(t, ErrNoMatchingRule, fw.Drop(pkt(47), true, &h, cp, nil), "rule %d", ruleProto) @@ -1023,17 +1060,18 @@ func TestFirewall_ProtocolRules(t *testing.T) { // proto `any` rules apply to protocols with rules of their own, whichever rule was added first for _, anyFirst := range []bool{true, false} { fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) + rb := firewall.NewRulesBuilder(l) addAny := func() { - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 80, 80, []string{"any"}, "", "", "", "", "")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 80, 80, []string{"any"}, "", "", "", "", "")) } if anyFirst { addAny() } - require.NoError(t, fw.AddRule(true, iputil.IPProtocolTCP, 22, 22, []string{"any"}, "", "", "", "", "")) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolTCP, 22, 22, []string{"any"}, "", "", "", "", "")) if !anyFirst { addAny() } - require.NoError(t, fw.buildRules()) + fw.setRules(rb) for _, tc := range []struct { proto uint8 @@ -1055,17 +1093,18 @@ func TestFirewall_ProtocolRules(t *testing.T) { // ICMP and ICMPv6 share one copy of each proto `any` rule, whichever rule was added first for _, anyFirst := range []bool{true, false} { fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) + rb := firewall.NewRulesBuilder(l) addAny := func() { - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, firewall.PortAny, firewall.PortAny, []string{"default-group"}, "", "", "", "", "")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, firewall.PortAny, firewall.PortAny, []string{"default-group"}, "", "", "", "", "")) } if anyFirst { addAny() } - require.NoError(t, fw.AddRule(true, iputil.IPProtocolICMPv6, firewall.PortAny, firewall.PortAny, []string{"nope"}, "", "", "", "", "")) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolICMPv6, firewall.PortAny, firewall.PortAny, []string{"nope"}, "", "", "", "", "")) if !anyFirst { addAny() } - require.NoError(t, fw.buildRules()) + fw.setRules(rb) require.NoError(t, fw.Drop(pkt(iputil.IPProtocolICMP), true, &h, cp, nil), "anyFirst %v", anyFirst) require.NoError(t, fw.Drop(pkt(iputil.IPProtocolICMPv6), true, &h, cp, nil), "anyFirst %v", anyFirst) @@ -1074,15 +1113,17 @@ func TestFirewall_ProtocolRules(t *testing.T) { // Protocols without rules of their own have no rules to check until there is a proto `any` rule to share fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, iputil.IPProtocolTCP, 22, 22, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolTCP, 22, 22, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) assert.Nil(t, fw.InRules.Protos[47]) assert.Equal(t, ErrNoMatchingRule, fw.Drop(pkt(47), true, &h, cp, nil)) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) - require.NoError(t, fw.AddRule(true, iputil.IPProtocolTCP, 22, 22, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, firewall.PortAny, firewall.PortAny, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb = firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, iputil.IPProtocolTCP, 22, 22, []string{"any"}, "", "", "", "", "")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, firewall.PortAny, firewall.PortAny, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) require.NoError(t, fw.Drop(pkt(47), true, &h, cp, nil)) } @@ -1115,9 +1156,10 @@ func TestFirewall_DropIPSpoofing(t *testing.T) { h1.buildNetworks(myVpnNetworksTable, c1.Certificate) fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) + rb := firewall.NewRulesBuilder(l) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "", "", "", "")) + fw.setRules(rb) cp := cert.NewCAPool() // Packet spoofed by `c1`. Note that the remote addr is not a valid one. @@ -1168,9 +1210,10 @@ func TestFirewall_ConntrackSourceSpoofingAcrossPeers(t *testing.T) { attackerHI.buildNetworks(myVpnNetworksTable, attacker.Certificate) fw := NewFirewall(l, time.Second, time.Minute, time.Hour, owner) + rb := firewall.NewRulesBuilder(l) // Allow any inbound traffic that passes the cert / source-IP checks. - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) cp := cert.NewCAPool() flow := firewall.Packet{ @@ -1265,8 +1308,9 @@ func BenchmarkFirewallDropConntrackHit(b *testing.B) { for _, tc := range cases { b.Run(tc.name, func(b *testing.B) { fw := NewFirewall(l, time.Second, time.Minute, time.Hour, owner) - require.NoError(b, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) - require.NoError(b, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(b, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) // Establish the conntrack entry so every benchmarked Drop is a hit. require.NoError(b, fw.Drop(flow, true, tc.host, cp, nil)) @@ -1360,115 +1404,6 @@ func BenchmarkLookup(b *testing.B) { }) } -func Test_parsePort(t *testing.T) { - _, _, err := parsePort("") - require.EqualError(t, err, "was not a number; ``") - - _, _, err = parsePort(" ") - require.EqualError(t, err, "was not a number; ` `") - - _, _, err = parsePort("-") - require.EqualError(t, err, "appears to be a range but could not be parsed; `-`") - - _, _, err = parsePort(" - ") - require.EqualError(t, err, "appears to be a range but could not be parsed; ` - `") - - _, _, err = parsePort("a-b") - require.EqualError(t, err, "beginning range was not a number; `a`") - - _, _, err = parsePort("1-b") - require.EqualError(t, err, "ending range was not a number; `b`") - - s, e, err := parsePort(" 1 - 2 ") - assert.Equal(t, int32(1), s) - assert.Equal(t, int32(2), e) - require.NoError(t, err) - - s, e, err = parsePort("0-1") - assert.Equal(t, int32(0), s) - assert.Equal(t, int32(0), e) - require.NoError(t, err) - - s, e, err = parsePort("9919") - assert.Equal(t, int32(9919), s) - assert.Equal(t, int32(9919), e) - require.NoError(t, err) - - s, e, err = parsePort("any") - assert.Equal(t, int32(0), s) - assert.Equal(t, int32(0), e) - require.NoError(t, err) -} - -// Test_parsePort_invalid covers inputs that must error. The named bug is -// that int32(strconv.Atoi("4294967296")) truncates to 0 == firewall.PortAny, -// silently turning a typo into a match-all-ports rule; the rest are -// representative syntax/range probes. -func Test_parsePort_invalid(t *testing.T) { - tests := []struct { - name string - input string - wantErrContains string - }{ - // Numeric overflow (the named bug + boundary). - {"named bug: 2^32 truncates to PortAny", "4294967296", "out of range"}, - {"just above max real port", "65536", "out of range"}, - - // Negatives route through the range branch and hit the empty-half - // guard; included as defense in depth so a future refactor cannot - // accidentally reach the int32 cast. - {"negative", "-1", "could not be parsed"}, - - // Syntax probes. - {"NUL between digits", "4\x002", "was not a number"}, - {"hex notation", "0x10", "was not a number"}, - {"scientific notation", "1e3", "was not a number"}, - {"leading whitespace", " 42", "was not a number"}, - {"fullwidth digits", "４２", "was not a number"}, - - // Range branch. - {"range upper out of range", "1-65536", "ending range out of range"}, - {"range lower out of range", "65536-65537", "beginning range out of range"}, - {"range with negative upper", "1--1", "ending range was not a number"}, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - _, _, err := parsePort(tc.input) - require.Error(t, err, "input %q must error", tc.input) - require.ErrorContains(t, err, tc.wantErrContains) - }) - } -} - -// Test_parsePort_valid_boundaries locks in success cases at 0, 1, and 65535 -// so a future refactor cannot regress the boundaries. -func Test_parsePort_valid_boundaries(t *testing.T) { - tests := []struct { - name string - input string - wantStart int32 - wantEnd int32 - }{ - {"zero is PortAny", "0", 0, 0}, - {"min real port", "1", 1, 1}, - {"max real port", "65535", 65535, 65535}, - {"range zero to max forces end to zero", "0-65535", 0, 0}, - {"range max to max", "65535-65535", 65535, 65535}, - {"range one to max", "1-65535", 1, 65535}, - {"range with whitespace inside", " 1 - 2 ", 1, 2}, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - s, e, err := parsePort(tc.input) - require.NoError(t, err) - assert.Equal(t, tc.wantStart, s, "start port") - assert.Equal(t, tc.wantEnd, e, "end port") - }) - } -} - func TestNewFirewallFromConfig(t *testing.T) { l := test.NewLogger() // Test a bad rule definition @@ -1528,269 +1463,6 @@ func TestNewFirewallFromConfig(t *testing.T) { require.EqualError(t, err, "firewall.inbound rule #0; only one of group or groups should be defined, both provided") } -func TestAddFirewallRulesFromConfig(t *testing.T) { - l := test.NewLogger() - // Test adding tcp rule - conf := config.NewC(test.NewLogger()) - mf := &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": "tcp", "host": "a"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf)) - assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolTCP, startPort: 1, endPort: 1, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) - - // Test adding udp rule - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": "udp", "host": "a"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf)) - assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolUDP, startPort: 1, endPort: 1, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) - - // Test adding udplite, dccp, and sctp rules - for name, proto := range map[string]uint8{"udplite": iputil.IPProtocolUDPLite, "dccp": iputil.IPProtocolDCCP, "sctp": iputil.IPProtocolSCTP} { - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1-2", "proto": name, "host": "a"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf), name) - assert.Equal(t, addRuleCall{incoming: false, proto: proto, startPort: 1, endPort: 2, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall, name) - } - - // Test adding icmp rule - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": "icmp", "host": "a"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf)) - assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolICMP, startPort: firewall.PortAny, endPort: firewall.PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) - - // Test adding icmp rule no port - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"proto": "icmp", "host": "a"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf)) - assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolICMP, startPort: firewall.PortAny, endPort: firewall.PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) - - // Test adding rules by protocol number, as yaml would give a string or an int - for _, proto := range []any{"47", 47} { - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "any", "proto": proto, "host": "a"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf), proto) - assert.Equal(t, addRuleCall{incoming: false, proto: 47, startPort: firewall.PortAny, endPort: firewall.PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall, proto) - } - - // Test ICMP and ICMPv6 by number still ignore ports - for _, proto := range []uint8{iputil.IPProtocolICMP, iputil.IPProtocolICMPv6} { - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": proto, "host": "a"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf), proto) - assert.Equal(t, addRuleCall{incoming: false, proto: proto, startPort: firewall.PortAny, endPort: firewall.PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall, proto) - } - - // Test protocols that aren't a known name or a number from 1 to 255 - for _, proto := range []string{"0", "256", "-1", "gre"} { - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "any", "proto": proto, "host": "a"}}} - require.EqualError(t, AddFirewallRulesFromConfig(l, false, conf, mf), "firewall.outbound rule #0; proto was not understood; `"+proto+"`") - } - - // Test adding any rule - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "host": "a"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) - - // Test adding rule with cidr - cidr := netip.MustParsePrefix("10.0.0.0/8") - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": cidr.String()}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: cidr.String(), localIp: ""}, mf.lastCall) - - // Test adding rule with local_cidr - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": cidr.String()}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: cidr.String()}, mf.lastCall) - - // Test adding rule with cidr ipv6 - cidr6 := netip.MustParsePrefix("fd00::/8") - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": cidr6.String()}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: cidr6.String(), localIp: ""}, mf.lastCall) - - // Test adding rule with any cidr - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": "any"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "any", localIp: ""}, mf.lastCall) - - // Test adding rule with junk cidr - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": "junk/junk"}}} - require.EqualError(t, AddFirewallRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; cidr did not parse; netip.ParsePrefix(\"junk/junk\"): ParseAddr(\"junk\"): unable to parse IP") - - // Test adding rule with local_cidr ipv6 - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": cidr6.String()}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: cidr6.String()}, mf.lastCall) - - // Test adding rule with any local_cidr - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": "any"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: nil, localIp: "any"}, mf.lastCall) - - // Test adding rule with junk local_cidr - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": "junk/junk"}}} - require.EqualError(t, AddFirewallRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; local_cidr did not parse; netip.ParsePrefix(\"junk/junk\"): ParseAddr(\"junk\"): unable to parse IP") - - // Test adding rule with ca_sha - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "ca_sha": "12312313123"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: "", caSha: "12312313123"}, mf.lastCall) - - // Test adding rule with ca_name - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "ca_name": "root01"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: "", caName: "root01"}, mf.lastCall) - - // Test single group - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "group": "a"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: []string{"a"}, ip: "", localIp: ""}, mf.lastCall) - - // Test single groups - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "groups": "a"}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: []string{"a"}, ip: "", localIp: ""}, mf.lastCall) - - // Test multiple AND groups - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "groups": []string{"a", "b"}}}} - require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf)) - assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: []string{"a", "b"}, ip: "", localIp: ""}, mf.lastCall) - - // Test Add error - conf = config.NewC(test.NewLogger()) - mf = &mockFirewall{} - mf.nextCallReturn = errors.New("test error") - conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "host": "a"}}} - require.EqualError(t, AddFirewallRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; `test error`") -} - -func TestFirewall_convertRule(t *testing.T) { - ob := &bytes.Buffer{} - l := test.NewLoggerWithOutput(ob) - - // Ensure group array of 1 is converted and a warning is printed - c := map[string]any{ - "group": []any{"group1"}, - } - - r, err := convertRule(l, c, "test", 1) - assert.Contains(t, ob.String(), "group was an array with a single value, converting to simple value") - assert.Contains(t, ob.String(), "table=test") - assert.Contains(t, ob.String(), "rule=1") - require.NoError(t, err) - assert.Equal(t, []string{"group1"}, r.Groups) - - // Ensure group array of > 1 is errord - ob.Reset() - c = map[string]any{ - "group": []any{"group1", "group2"}, - } - - r, err = convertRule(l, c, "test", 1) - assert.Empty(t, ob.String()) - require.Error(t, err, "group should contain a single value, an array with more than one entry was provided") - - // Make sure a well formed group is alright - ob.Reset() - c = map[string]any{ - "group": "group1", - } - - r, err = convertRule(l, c, "test", 1) - require.NoError(t, err) - assert.Equal(t, []string{"group1"}, r.Groups) -} - -func TestFirewall_convertRuleSanity(t *testing.T) { - ob := &bytes.Buffer{} - l := test.NewLoggerWithOutput(ob) - - noWarningPlease := []map[string]any{ - {"group": "group1"}, - {"groups": []any{"group2"}}, - {"host": "bob"}, - {"cidr": "1.1.1.1/1"}, - {"groups": []any{"group2"}, "host": "bob"}, - {"cidr": "1.1.1.1/1", "host": "bob"}, - {"groups": []any{"group2"}, "cidr": "1.1.1.1/1"}, - {"groups": []any{"group2"}, "cidr": "1.1.1.1/1", "host": "bob"}, - } - for _, c := range noWarningPlease { - r, err := convertRule(l, c, "test", 1) - require.NoError(t, err) - require.NoError(t, r.sanity(), "should not generate a sanity warning, %+v", c) - } - - yesWarningPlease := []map[string]any{ - {"group": "group1"}, - {"groups": []any{"group2"}}, - {"cidr": "1.1.1.1/1"}, - {"groups": []any{"group2"}, "host": "bob"}, - {"cidr": "1.1.1.1/1", "host": "bob"}, - {"groups": []any{"group2"}, "cidr": "1.1.1.1/1"}, - {"groups": []any{"group2"}, "cidr": "1.1.1.1/1", "host": "bob"}, - } - for _, c := range yesWarningPlease { - c["host"] = "any" - r, err := convertRule(l, c, "test", 1) - require.NoError(t, err) - err = r.sanity() - require.Error(t, err, "I wanted a warning: %+v", c) - } - //reset the list - yesWarningPlease = []map[string]any{ - {"group": "group1"}, - {"groups": []any{"group2"}}, - {"cidr": "1.1.1.1/1"}, - {"groups": []any{"group2"}, "host": "bob"}, - {"cidr": "1.1.1.1/1", "host": "bob"}, - {"groups": []any{"group2"}, "cidr": "1.1.1.1/1"}, - {"groups": []any{"group2"}, "cidr": "1.1.1.1/1", "host": "bob"}, - } - for _, c := range yesWarningPlease { - r, err := convertRule(l, c, "test", 1) - require.NoError(t, err) - r.Groups = append(r.Groups, "any") - err = r.sanity() - require.Error(t, err, "I wanted a warning: %+v", c) - } -} - type testcase struct { h *HostInfo p firewall.Packet @@ -1869,8 +1541,9 @@ func newSetupFromCert(t *testing.T, l *slog.Logger, c dummyCert) testsetup { myVpnNetworksTable.Insert(prefix) } fw := NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) + fw.setRules(rb) return testsetup{ c: c, @@ -1953,51 +1626,15 @@ func TestFirewall_Drop_EnforceIPMatch(t *testing.T) { // The same rules plus one for the unsafe route fw := NewFirewall(l, time.Second, time.Minute, time.Hour, &c) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) - require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", unsafePrefix.String(), "", "")) - require.NoError(t, fw.buildRules()) + rb := firewall.NewRulesBuilder(l) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) + require.NoError(t, rb.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", unsafePrefix.String(), "", "")) + fw.setRules(rb) tc.err = nil tc.Test(t, fw) //should pass }) } -type addRuleCall struct { - incoming bool - proto uint8 - startPort int32 - endPort int32 - groups []string - host string - ip string - localIp string - caName string - caSha string -} - -type mockFirewall struct { - lastCall addRuleCall - nextCallReturn error -} - -func (mf *mockFirewall) AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, ip, localIp, caName string, caSha string) error { - mf.lastCall = addRuleCall{ - incoming: incoming, - proto: proto, - startPort: startPort, - endPort: endPort, - groups: groups, - host: host, - ip: ip, - localIp: localIp, - caName: caName, - caSha: caSha, - } - - err := mf.nextCallReturn - mf.nextCallReturn = nil - return err -} - func resetConntrack(fw *Firewall) { fw.Conntrack.Lock() fw.Conntrack.Conns = map[firewall.Packet]*conn{}