unkin/tomswall
4
0
Fork 0
Code Issues 5 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

Reject unknown and excluded zone names in rule, blrule and conntrack specs

Browse Source
This commit is contained in:
Unkin Agent
2026-10-03 23:48:08 +10:00
parent 3c5d23a811
commit 9432bb05c9
4 changed files with 62 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/config/blrules.go
+4 -21
View File
@@ -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
internal/config/conntrack.go
+5 -2
View File
@@ -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 != "" {
internal/config/extras_test.go
+21 -5
View File
@@ -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",
internal/config/rules.go
+32 -22
View File
@@ -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
}