diff --git a/internal/config/blrules.go b/internal/config/blrules.go index 4118d22..1dbd02c 100644 --- a/internal/config/blrules.go +++ b/internal/config/blrules.go @@ -55,28 +55,11 @@ func (c *Config) validateBlrules() error { return fmt.Errorf("blrules[%d]: dest required", i) } - if r.Source != "all" && r.Source != "any" && r.Source != "none" && - !hasPrefix(r.Source, "all!") && !hasPrefix(r.Source, "any!") { - for _, zs := range SplitZoneList(r.Source) { - if _, ok := c.Zones[zs.Zone]; !ok { - return fmt.Errorf("blrules[%d]: source zone %q not defined", i, zs.Zone) - } - if !validAddrList(zs.Addr) { - return fmt.Errorf("blrules[%d]: source %q: '!' may only prefix the whole address list", i, zs.Addr) - } - } + if err := c.validateZoneRef(r.Source); err != nil { + return fmt.Errorf("blrules[%d]: source %w", i, err) } - - if r.Dest != "all" && r.Dest != "any" && r.Dest != "none" && - !hasPrefix(r.Dest, "all!") && !hasPrefix(r.Dest, "any!") { - for _, zs := range SplitZoneList(r.Dest) { - if _, ok := c.Zones[zs.Zone]; !ok { - return fmt.Errorf("blrules[%d]: dest zone %q not defined", i, zs.Zone) - } - if !validAddrList(zs.Addr) { - return fmt.Errorf("blrules[%d]: dest %q: '!' may only prefix the whole address list", i, zs.Addr) - } - } + if err := c.validateZoneRef(r.Dest); err != nil { + return fmt.Errorf("blrules[%d]: dest %w", i, err) } } return nil diff --git a/internal/config/conntrack.go b/internal/config/conntrack.go index 527a00f..63edb4a 100644 --- a/internal/config/conntrack.go +++ b/internal/config/conntrack.go @@ -68,8 +68,11 @@ func (c *Config) validateConntrack() error { return fmt.Errorf("conntrack[%d]: helper name required for helper action", i) } - if ct.Source == "" && ct.Dest == "" && ct.Action != ConntrackHelper { - return fmt.Errorf("conntrack[%d]: source or dest required", i) + if err := c.validateZoneRef(ct.Source); err != nil { + return fmt.Errorf("conntrack[%d]: source %w", i, err) + } + if err := c.validateZoneRef(ct.Dest); err != nil { + return fmt.Errorf("conntrack[%d]: dest %w", i, err) } if ct.User != "" { diff --git a/internal/config/extras_test.go b/internal/config/extras_test.go index f83941d..82ebd24 100644 --- a/internal/config/extras_test.go +++ b/internal/config/extras_test.go @@ -53,11 +53,27 @@ func TestValidateConntrack(t *testing.T) { }, }, { - name: "source or dest required for non-helper", - rules: []ConntrackRule{ - {Action: ConntrackDrop}, - }, - wantErr: "source or dest required", + name: "omitted source and dest is valid", + rules: []ConntrackRule{{Action: ConntrackNoTrack, Proto: "udp", DPort: PortSpec{"53"}}}, + }, + { + name: "unknown source zone", + rules: []ConntrackRule{{Action: ConntrackNoTrack, Source: "nte"}}, + wantErr: `source zone "nte" not defined`, + }, + { + name: "unknown dest zone", + rules: []ConntrackRule{{Action: ConntrackDrop, Source: "net", Dest: "nte:192.0.2.1"}}, + wantErr: `dest zone "nte" not defined`, + }, + { + name: "unknown excluded zone", + rules: []ConntrackRule{{Action: ConntrackNoTrack, Source: "all!nte"}}, + wantErr: `excluded zone "nte" not defined`, + }, + { + name: "exclusion and all forms are valid", + rules: []ConntrackRule{{Action: ConntrackNoTrack, Source: "all!net", Dest: "all:192.0.2.1"}}, }, { name: "helper without source/dest is valid", diff --git a/internal/config/rules.go b/internal/config/rules.go index fb94e31..4099b79 100644 --- a/internal/config/rules.go +++ b/internal/config/rules.go @@ -173,29 +173,13 @@ func (c *Config) validateRules() error { return fmt.Errorf("rule[%d]: dest required", i) } - if r.Source != "all" && r.Source != "any" && r.Source != "none" && - !hasPrefix(r.Source, "all+") && !hasPrefix(r.Source, "all!") && !hasPrefix(r.Source, "any!") { - for _, zs := range SplitZoneList(r.Source) { - if _, ok := c.Zones[zs.Zone]; !ok { - return fmt.Errorf("rule[%d]: source zone %q not defined", i, zs.Zone) - } - if !validAddrList(zs.Addr) { - return fmt.Errorf("rule[%d]: source %q: '!' may only prefix the whole address list", i, zs.Addr) - } - } + if err := c.validateZoneRef(r.Source); err != nil { + return fmt.Errorf("rule[%d]: source %w", i, err) } if r.Action != RuleDNAT && r.Action != RuleRedirect && r.Action != RuleNoNAT { - if r.Dest != "all" && r.Dest != "any" && r.Dest != "none" && - !hasPrefix(r.Dest, "all+") && !hasPrefix(r.Dest, "all!") && !hasPrefix(r.Dest, "any!") { - for _, zs := range SplitZoneList(r.Dest) { - if _, ok := c.Zones[zs.Zone]; !ok { - return fmt.Errorf("rule[%d]: dest zone %q not defined", i, zs.Zone) - } - if !validAddrList(zs.Addr) { - return fmt.Errorf("rule[%d]: dest %q: '!' may only prefix the whole address list", i, zs.Addr) - } - } + if err := c.validateZoneRef(r.Dest); err != nil { + return fmt.Errorf("rule[%d]: dest %w", i, err) } } @@ -264,6 +248,32 @@ func zoneFromSpec(spec string) string { return spec } -func hasPrefix(s, prefix string) bool { - return len(s) >= len(prefix) && s[:len(prefix)] == prefix +// validateZoneRef checks a SOURCE/DEST spec: all/any[+][!excluded,...][:addr], none, or a declared zone list. +func (c *Config) validateZoneRef(spec string) error { + zones, addr, _ := strings.Cut(spec, ":") + base, excl, isExcl := strings.Cut(zones, "!") + switch base { + case "", "none", "all", "all+", "any", "any+": + if base == "" && isExcl { + return fmt.Errorf("%q: exclusion needs all or any", spec) + } + for _, z := range strings.Split(excl, ",") { + if _, ok := c.Zones[strings.TrimSpace(z)]; isExcl && !ok { + return fmt.Errorf("excluded zone %q not defined", z) + } + } + if !validAddrList(addr) { + return fmt.Errorf("%q: '!' may only prefix the whole address list", addr) + } + return nil + } + for _, zs := range SplitZoneList(spec) { + if _, ok := c.Zones[zs.Zone]; !ok { + return fmt.Errorf("zone %q not defined", zs.Zone) + } + if !validAddrList(zs.Addr) { + return fmt.Errorf("%q: '!' may only prefix the whole address list", zs.Addr) + } + } + return nil }