GRMrGecko/go-firewall
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Route inexpressible csf/apf shapes to the hook instead of rejecting them

Browse source 
csf and apf both back a native config with a raw-iptables pre-hook, but a rule
their config could not express was reported as unsupported even when iptables
applies it directly. Route those shapes to the hook, and share the routing
predicates between the two backends.

Shared hook layer (hooks_linux.go): extract parseAddrFamily, listRow/listRows,
bareHostShape, hostNeedsHook, advRuleNeedsHook and bareProtoNeedsHook so both
backends route on the same predicates. MarshalAdvRule no longer returns an error
for shapes its caller has already routed away.

csf: replace checkConnLimit, checkICMP and checkPortProto with a single needsHook
gate mirroring apf's. A connection limit CONNLIMIT cannot express, an ICMP rule
the advanced-line format cannot carry (see nativeICMP), and a port on a protocol
its TCP_IN/UDP_IN lists cannot hold now reach the hook. A port on ProtocolAny has
no form in iptables either, so it stays on the native path and iptablesRuleValid
rejects it with ErrUnsupported.

csf: remove a deny from csf.deny whatever action the caller names, as apf does.
The file encodes no action of its own, so the deny of an address is a single
entry; the old early return reported success while csf kept enforcing it.

IPv6: ipv6Unavailable only rejects concrete-IPv6 rules, so a FamilyAny rule
passed the gate and then fanned out into an IPv6 row. With csf.conf's IPV6 or
conf.apf's USE_IPV6 off that row is never enforced, and in the hook it is worse
than inert: the pre-hook runs on every reload while neither backend flushes
ip6tables, so an injected ip6tables line is re-appended each time and outlives
its removal from the hook. Narrow both write-side fan-outs to the families the
backend enforces (hookScript.writeFamilies, writeFamilyRows). Removals stay wide
so a v6 line written while IPv6 was on is still swept once it is off.

Integration: the shared probes take the first variant a backend accepts, so
widening csf's accepted shapes silently moved icmp/icmptype/connlimit onto its
hook path. Add csf-gated subtests to keep the native csf.conf CONNLIMIT and
csf.allow advanced-rule paths covered, alongside one for action-agnostic
csf.deny removal.
This commit is contained in:
James Coleman 2026-07-09 21:30:27 -05:00
parent a036c8e6e9
commit 060d667e93
7 changed files with 1104 additions and 735 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

373
apf_linux.go
View file

@ -486,7 +486,7 @@ func (f *APF) ParseIPList(filePath string, action Action) (rules []*Rule, err er
rules = append(rules, rule)
} else {
// Try to parse IP.
fam, ok := csfAddrFamily(trimmed)
fam, ok := parseAddrFamily(trimmed)
if !ok {
continue
}
@ -543,9 +543,10 @@ func (f *APF) ParsePorts(val string, proto Protocol, out bool) (rules []*Rule) {
// features apf's native config cannot express.
func (f *APF) hook() *hookScript {
return &hookScript{
rulePrefix: f.rulePrefix,
hookPath: APFHook,
hookPerm: 0750,
rulePrefix: f.rulePrefix,
hookPath: APFHook,
hookPerm: 0750,
ipv6Enabled: f.ipv6Enabled,
}
}
@ -956,37 +957,14 @@ func (f *APF) addrField(addr string) string {
return addr
}
// MarshalAdvRule encodes a rule as an apf advanced allow/deny line. An apf
// advanced rule is tcp/udp only and must carry a source or destination address.
func (f *APF) MarshalAdvRule(r *Rule) (string, error) {
if r.Proto.IsICMP() {
return "", fmt.Errorf("apf advanced rules do not support icmp")
}
if r.Source == "" && r.Destination == "" {
return "", fmt.Errorf("an apf advanced rule requires a source or destination address")
}
// apf's advanced rule holds a single address field, so a rule matching both a
// source and a destination cannot be expressed; reject it rather than silently
// dropping the destination (which would install a broader rule than asked and
// leave the rule unremovable). Mirrors the dual-port guard below.
if r.Source != "" && r.Destination != "" {
return "", fmt.Errorf("apf advanced rules cannot match both a source and destination address")
}
// apf's port field takes a single port or an underscore range (no comma list),
// and there is a single port-flow field, so a rule cannot match both a source
// and a destination port at once.
if len(r.PortSpecs()) > 1 || len(r.SourcePortSpecs()) > 1 {
return "", fmt.Errorf("apf advanced rules do not support a port list in this model")
}
if r.HasPorts() && r.HasSourcePorts() {
return "", fmt.Errorf("apf advanced rules cannot match both a source and destination port")
}
// MarshalAdvRule encodes a rule as an apf advanced allow/deny line: an optional
// protocol field, a direction, one port-flow field (a source or a destination port)
// and one address field, joined by ":". It validates nothing; addRule/RemoveRule
// route every shape the line cannot carry elsewhere first (see needsHook).
func (f *APF) MarshalAdvRule(r *Rule) string {
// apf's advanced rule carries an optional protocol field, and treats a line
// without one as both transports: its trust parser emits a `-p tcp` rule and a
// `-p udp` rule for it. So TCPUDP is written by omitting the field, and no other
// protocol may be: ProtocolAny would mean every IP protocol, which the omitted
// field does not express.
// `-p udp` rule for it. So TCPUDP is written by omitting the field.
var parts []string
switch r.Proto {
case TCP:
@ -995,15 +973,15 @@ func (f *APF) MarshalAdvRule(r *Rule) (string, error) {
parts = append(parts, "udp")
case TCPUDP:
// No protocol field: apf reads that as tcp plus udp.
default:
return "", fmt.Errorf("an apf advanced rule carries tcp, udp or both: %w", ErrUnsupported)
}
if r.IsOutput() {
parts = append(parts, "out")
} else {
parts = append(parts, "in")
}
// The port-flow field: a source port or a destination port.
// The port-flow field: a source port or a destination port. apf's field holds a
// single port or one underscore range; needsHook routes a multi-port list to the
// hook, so only one spec ever arrives.
if specs := r.SourcePortSpecs(); len(specs) == 1 {
parts = append(parts, "s="+f.portToken(specs[0]))
} else if specs := r.PortSpecs(); len(specs) == 1 {
@ -1014,10 +992,99 @@ func (f *APF) MarshalAdvRule(r *Rule) (string, error) {
} else if r.Destination != "" {
parts = append(parts, "d="+f.addrField(r.Destination))
}
return strings.Join(parts, ":"), nil
return strings.Join(parts, ":")
}
// EditIPList adds or removes a rule in an apf allow_hosts/deny_hosts file, rewriting it in place.
// parseListLine parses one apf allow_hosts/deny_hosts rule line into the rule it
// holds: an advanced rule, or a plain address line, which is a single bidirectional
// DirAny rule matching every protocol. It returns nil for a line that is neither,
// which the caller passes through untouched. The action comes from the file
// (allow_hosts is accept, deny_hosts takes the drop/reject action conf.apf sets);
// the line encodes none of its own.
func (f *APF) parseListLine(line string, action Action) *Rule {
if strings.Contains(line, "=") {
return f.ParseAdvRule(line, action)
}
fam, ok := parseAddrFamily(line)
if !ok {
return nil
}
return &Rule{Direction: DirAny, Family: fam, Source: line, Action: action}
}
// listRows returns the allow_hosts/deny_hosts rows a rule materializes into, in write
// order, or none for a shape the trust files cannot hold. The rule must already carry
// the action its file implies (see EditIPList's match), since each row's read-back
// form is compared against lines stamped with it.
//
// A rule never fans out across transports: apf's trust parser reads a protocol-less
// advanced line as a `-p tcp` rule plus a `-p udp` rule, so MarshalAdvRule writes a
// TCPUDP rule as a single line. A port-only deny does fan out per family: apf requires
// an address in the advanced line's field position, and the "any" network placeholder
// it uses there is family-specific, so a family-neutral rule would otherwise silently
// become IPv4-only. The fan-out covers the families apf actually enforces (see
// writeFamilyRows; with conf.apf's USE_IPV6 off that is IPv4 alone). parseAddr
// normalizes the placeholder back to an empty address, so each row reads back as the
// address-less rule it stands for.
func (f *APF) listRows(action Action, match *Rule) []listRow {
hasIP := match.Source != "" || match.Destination != ""
var rows []listRow
switch {
case hasIP && (match.HasPorts() || match.HasSourcePorts()):
// A port rule with an address is an advanced rule.
rows = append(rows, listRow{line: f.MarshalAdvRule(match), read: match})
case hasIP:
// A bare all-protocol host allow/deny: a single address matching every
// protocol. apf's trust files hold no other portless address shape — a
// concrete-protocol host or a source+destination pair — so AddRule diverts
// those to the raw-iptables hook (hostNeedsHook) and never reaches here with
// one. A direct caller of this exported writer that supplies such a shape gets
// a best-effort single-address write, not a guard.
addr := match.Source
if addr == "" {
addr = match.Destination
}
// The plain line is bidirectional and names its address as the source, which is
// the frame the scan reads it back in.
read := *match
read.Direction = DirAny
read.Source, read.Destination = addr, ""
rows = append(rows, listRow{line: addr, read: &read})
case action != Accept && match.HasPorts():
specs := match.PortSpecs()
for _, fam := range writeFamilyRows(f.ipv6Enabled, match) {
placeholder := "0.0.0.0/0"
if fam.impliedFamily() == IPv6 {
// MarshalAdvRule brackets an IPv6 literal for the colon-separated format.
placeholder = "::/0"
}
// Pin the row to the one shape apf's single port-flow field holds: one
// destination port. needsHook routes a multi-port list and a source-port match
// to the hook before AddRule reaches here, so this only bounds a direct caller
// of this exported writer — without it MarshalAdvRule would emit a portless
// line (denying the whole protocol) for a list, or prefer the source port over
// the destination one.
read := *fam
read.Port, read.Ports = 0, specs[:1]
read.SourcePort, read.SourcePorts = 0, nil
// The row reads back address-less; only the line carries the placeholder.
row := read
if row.IsOutput() {
row.Destination = placeholder
} else {
row.Source = placeholder
}
rows = append(rows, listRow{line: f.MarshalAdvRule(&row), read: &read})
}
}
return rows
}
// EditIPList adds or removes a rule in an apf allow_hosts/deny_hosts file, rewriting
// it in place. An add expands the rule into the rows it materializes into (see
// listRows), notes which of them the file already holds, and appends only the rest,
// so a rule that fans out across families is completed rather than duplicated on
// every reconcile. A removal drops every line the target covers.
func (f *APF) EditIPList(ctx context.Context, filePath string, action Action, r *Rule, remove bool) error {
// Read the allow_hosts/deny_hosts rule list.
fd, err := os.Open(filePath)
@ -1034,13 +1101,20 @@ func (f *APF) EditIPList(ctx context.Context, filePath string, action Action, r
defer af.Abort()
scanner := bufio.NewScanner(fd)
exists := false
// A deny_hosts entry takes on the drop/reject action set in apf.conf (and
// allow_hosts is accept), so a rule read back is stamped with the file's
// action. Resolve the incoming rule's action the same way via resolveAction
// before matching.
match := *r
match.Action = f.resolveAction(action, r.Proto)
// The rows an add must end up with, and which of them the scan finds already in
// the file. A removal wants no rows: it matches the target against each line
// directly, since a line it must drop need not be one this library would write.
var rows []listRow
if !remove {
rows = f.listRows(action, &match)
}
present := make([]bool, len(rows))
// pending holds the full-line comments seen immediately above a rule, so they
// can be dropped together with a removed rule (they are its comment) or written
@ -1089,149 +1163,60 @@ func (f *APF) EditIPList(ctx context.Context, filePath string, action Action, r
continue
}
if strings.Contains(line, "=") {
rule := f.ParseAdvRule(line, action)
if rule == nil {
flush()
_, _ = fmt.Fprintln(af, orig)
continue
}
// Match family-aware: the address-less "port-only deny" form parses to
// Source="" for both families, so a bare EqualBase would let an IPv4 line
// stand in for its IPv6 twin. EqualForDedup/EqualForRemoval fold in the
// family coverage the add and remove paths respectively need.
var famMatch bool
if remove {
famMatch = rule.EqualForRemoval(&match, true)
} else {
famMatch = rule.EqualForDedup(&match, true)
}
if famMatch {
exists = true
if !remove {
flush()
_, _ = fmt.Fprintln(af, orig)
} else {
drop()
f.ConfigChanged = true
}
} else {
flush()
_, _ = fmt.Fprintln(af, orig)
}
} else {
// Try to parse IP.
fam, ok := csfAddrFamily(line)
if !ok {
flush()
_, _ = fmt.Fprintln(af, orig)
continue
}
// A line neither form parses is not a rule; pass it through untouched.
cur := f.parseListLine(line, action)
if cur == nil {
flush()
_, _ = fmt.Fprintln(af, orig)
continue
}
// A plain IP line is one bidirectional DirAny rule; match the target
// against it in the inbound frame (canonicalMatch), so a DirAny or a
// concrete-direction input/output target that names this host lines up.
plainRule := &Rule{
Direction: DirAny,
Family: fam,
Source: line,
Action: action,
// A removal drops every line the target covers, along with its comment. A
// family-neutral target touches each of the concrete lines it was written as;
// that coverage is folded into EqualForRemoval.
if remove {
if cur.EqualForRemoval(&match, true) {
drop()
f.ConfigChanged = true
continue
}
if plainRule.EqualBase(match.canonicalMatch(), false) {
exists = true
if !remove {
flush()
_, _ = fmt.Fprintln(af, orig)
} else {
drop()
f.ConfigChanged = true
}
} else {
flush()
_, _ = fmt.Fprintln(af, orig)
flush()
_, _ = fmt.Fprintln(af, orig)
continue
}
// An add keeps every line and only notes which wanted rows the file already
// covers, so the tail writes the rest. Coverage rather than a text compare, so
// a row is satisfied by an existing line that spans it (a protocol-less TCPUDP
// line absorbing a tcp row) and by one spelled differently but meaning the same.
for i := range rows {
if !present[i] && cur.EqualForDedup(rows[i].read, true) {
present[i] = true
}
}
flush()
_, _ = fmt.Fprintln(af, orig)
}
// Write any trailing comments that followed the last rule.
flush()
// If not exists and not remove, try adding the rule.
if !exists && !remove {
writeComment := func() {
if c := combineComment(f.rulePrefix, r.Comment); c != "" {
_, _ = fmt.Fprintln(af, "# "+c)
}
// Append the wanted rows the file does not already hold. A rule that fans out is
// completed row by row: when only a subset is present (the IPv4 line but not its
// IPv6 twin, from a prior single-family add or a manual edit) the missing rows
// must still be written, or that family stays open while the library reports the
// rule in force.
writeComment := func() {
if c := combineComment(f.rulePrefix, r.Comment); c != "" {
_, _ = fmt.Fprintln(af, "# "+c)
}
hasIP := r.Source != "" || r.Destination != ""
switch {
case hasIP && (r.HasPorts() || r.HasSourcePorts()):
// A port rule with an address is an advanced rule.
line, err := f.MarshalAdvRule(r)
if err != nil {
_ = fd.Close()
return err
}
f.ConfigChanged = true
writeComment()
_, _ = fmt.Fprintln(af, line)
case hasIP:
// A bare all-protocol host allow/deny: a single address matching every
// protocol. apf's trust files hold no other portless address shape — a
// concrete-protocol host or a source+destination pair — so AddRule diverts
// those to the raw-iptables hook (hostNeedsHook) and never reaches here
// with one. A direct caller of this exported writer that supplies such a
// shape gets a best-effort single-address write, not a guard.
f.ConfigChanged = true
writeComment()
if r.Source != "" {
_, _ = fmt.Fprintln(af, r.Source)
} else {
_, _ = fmt.Fprintln(af, r.Destination)
}
case action != Accept && r.HasPorts():
// A port-only deny is written as an advanced rule against any address,
// which apf requires in the field position. apf's port field holds a
// single port or underscore range only; a multi-port list has no advanced-
// rule form, so AddRule diverts it to the hook (needsHook) and this
// branch only sees a single port/range. A direct caller supplying a list
// gets a best-effort single-port write.
specs := r.PortSpecs()
// apf requires an address in the field position, so a port-only deny
// uses the "any" network as a placeholder. That literal is family-
// specific (0.0.0.0/0 vs ::/0) and read back with its family, so emit
// the placeholder matching the rule's family. A family-neutral rule has
// no single literal, so emit a line per family rather than let the rule
// silently become IPv4-only; each line reads back as its own rule.
writeAny := func(placeholder string) {
var tokens []string
if r.Proto != TCPUDP {
tokens = append(tokens, r.Proto.String())
}
if r.IsOutput() {
tokens = append(tokens, "out")
} else {
tokens = append(tokens, "in")
}
tokens = append(tokens, "d="+f.portToken(specs[0]))
if r.IsOutput() {
tokens = append(tokens, "d="+placeholder)
} else {
tokens = append(tokens, "s="+placeholder)
}
writeComment()
_, _ = fmt.Fprintln(af, strings.Join(tokens, ":"))
}
switch r.impliedFamily() {
case IPv6:
writeAny("[::/0]")
case IPv4:
writeAny("0.0.0.0/0")
default:
writeAny("0.0.0.0/0")
writeAny("[::/0]")
}
f.ConfigChanged = true
}
for i, row := range rows {
if present[i] {
continue
}
f.ConfigChanged = true
writeComment()
_, _ = fmt.Fprintln(af, row.line)
}
_ = fd.Close()
@ -1258,9 +1243,9 @@ func (f *APF) isConfRule(r *Rule) bool {
// nativeICMPv6 reports whether an ICMPv6 rule can be carried by apf's native
// IG_ICMPV6_TYPES/EG_ICMPV6_TYPES lists (an address-less accept, optionally typed)
// and so belongs in conf.apf rather than the raw-iptables hook. The shared
// ruleNeedsHook diverts every ICMPv6 rule to the hook — correct for csf, which has
// no native v6 type list — so apf overrides that only for the rules its config can
// actually express, leaving an ICMPv6 rule that also needs state/interface/log/
// ruleNeedsHook diverts every ICMPv6 rule to the hook, since not every backend it
// serves has a native v6 type list; apf overrides that only for the rules its config
// can actually express, leaving an ICMPv6 rule that also needs state/interface/log/
// rate matching (which conf.apf cannot carry) on the hook path.
func (f *APF) nativeICMPv6(r *Rule) bool {
return r.Proto == ICMPv6 && r.State == 0 && r.InInterface == "" && r.OutInterface == "" &&
@ -1281,8 +1266,8 @@ func (f *APF) barePortAccept(r *Rule) bool {
// dualStackPortNeedsHook reports whether a bare tcp/udp port accept pinned to a
// single family must be injected through the hook. apf's IG_*_CPORTS/EG_*_CPORTS
// lists are dual-stack — one list applied to both the ip and ip6 tables — so they
// express only a FamilyAny port (unlike csf, whose TCP_IN/TCP6_IN split the
// families). A single-family one is written per-family through the hook instead,
// express only a FamilyAny port; the lists have no per-family form to pin one to.
// A single-family port accept is written per-family through the hook instead,
// whose iptables (or ip6tables) rule carries just that family; removing one family
// of a FamilyAny CPORTS entry splits it (see removeDualStackPort). ICMP keeps its
// concrete family (its type lists are per-family), so this gates on a port match.
@ -1295,10 +1280,10 @@ func (f *APF) dualStackPortNeedsHook(r *Rule) bool {
// gate between the hook path and apf's config files: everything it rejects (returns
// true) is written to the hook, everything it accepts (returns false) maps onto
// conf.apf or the allow_hosts/deny_hosts trust files. The shared shapes
// (ruleNeedsHook, bareHostOneWay, hostNeedsHook) and the two apf shapes RemoveRule
// reuses to route a split (dualStackPortNeedsHook, nativeICMPv6) keep their own
// predicates; the apf-only, single-use port/source-port/connlimit/icmp tests are
// inlined here as their sole caller.
// (ruleNeedsHook, bareHostOneWay, hostNeedsHook, advRuleNeedsHook) and the two apf
// shapes RemoveRule reuses to route a split (dualStackPortNeedsHook, nativeICMPv6)
// keep their own predicates; the apf-only, single-use port/source-port/connlimit/icmp
// tests are inlined here as their sole caller.
func (f *APF) needsHook(r *Rule) bool {
// Features apf's native config cannot model — connection state, per-rule
// interface, logging, rate limiting, forward-chain routing, ICMPv6, or a
@ -1314,6 +1299,13 @@ func (f *APF) needsHook(r *Rule) bool {
if bareHostOneWay(r) || hostNeedsHook(r) {
return true
}
// A ported source+destination pair, a source port matched with a destination port,
// and an address-less source-port match all overflow the advanced line's single
// address and single port-flow field (see advRuleNeedsHook); iptables matches each
// directly, so they go to the hook.
if advRuleNeedsHook(r) {
return true
}
// A multi-port tcp/udp list apf's config cannot carry: its advanced rule holds a
// single port or one underscore range, and the only native multi-port shape is an
// address-less accept (isConfRule, carried by the IG_*_CPORTS comma lists);
@ -1322,12 +1314,6 @@ func (f *APF) needsHook(r *Rule) bool {
(len(r.PortSpecs()) > 1 || len(r.SourcePortSpecs()) > 1) && !f.isConfRule(r) {
return true
}
// A source-port match with no address has no advanced-rule form (advanced rules
// require an address); iptables matches --sport directly, so it goes to the hook.
if r.HasSourcePorts() && r.Source == "" && r.Destination == "" &&
onProtocolAxis(r.Proto) {
return true
}
// A connection limit conf.apf's IG_*_CLIMIT cannot express — anything but a
// per-source cap on a single address-less inbound tcp/udp port rejecting the
// excess (isConnLimitRule) — goes to the hook's `-m connlimit` match.
@ -1381,10 +1367,16 @@ func (f *APF) addRule(ctx context.Context, zoneName string, r *Rule, enforceIPv6
return nil
}
// Verify the rule is valid with iptables.
if err := iptablesRuleValid(r); err != nil {
return fmt.Errorf("%v: %w", err, ErrUnsupported)
}
// Any shape apf's native config cannot express (a stateful/interface/logged/
// rate-limited rule, a one-way or concrete-protocol host, a source+destination
// pair, a multi-port list, an address-less source-port match, a non-native
// connection limit, a non-native ICMPv4 rule, or a single-family port accept) is
// pair, a source-and-destination port match, a multi-port list, an address-less
// source-port match, a non-native connection limit, a non-native ICMPv4 rule, or
// a single-family port accept) is
// injected as a raw iptables rule through the apf pre-hook. See needsHook for
// each clause; everything past this gate maps onto apf's own config files.
if f.needsHook(r) {
@ -1392,9 +1384,6 @@ func (f *APF) addRule(ctx context.Context, zoneName string, r *Rule, enforceIPv6
f.ConfigChanged = f.ConfigChanged || changed
return err
}
if err := iptablesRuleValid(r); err != nil {
return fmt.Errorf("%v: %w", err, ErrUnsupported)
}
// A native connection-limit rule maps onto conf.apf's IG_*_CLIMIT lists (a
// non-native one was diverted to the hook above by needsHook).

156
apf_linux_test.go
View file

@ -4,6 +4,7 @@ import (
"context"
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/require"
@ -23,8 +24,7 @@ func TestAPFAdvRules(t *testing.T) {
{&Rule{Direction: DirOutput, Proto: UDP, Port: 53, Destination: "192.0.2.1", Family: IPv4, Action: Accept}, "udp:out:d=53:d=192.0.2.1"},
}
for _, c := range cases {
got, err := fw.MarshalAdvRule(c.rule)
require.NoError(t, err, "failed to marshal %+v", *c.rule)
got := fw.MarshalAdvRule(c.rule)
require.Equal(t, c.want, got, "marshal %+v", *c.rule)
parsed := fw.ParseAdvRule(got, c.rule.Action)
@ -40,16 +40,18 @@ func TestAPFAdvRules(t *testing.T) {
require.Equal(t, IPv6, r.Family)
require.EqualValues(t, 443, r.Port)
// MarshalAdvRule rejects rules apf cannot express as advanced rules.
bad := []*Rule{
// The shapes an advanced line cannot hold never reach MarshalAdvRule: needsHook
// routes an addressed ICMP rule and a multi-port list to the raw-iptables hook,
// and an address-less port accept lands in conf.apf's CPORTS lists instead.
routed := []*Rule{
{Proto: ICMP, ICMPType: Ptr[uint8](8), Source: "1.2.3.4", Action: Accept},
{Proto: TCP, Port: 80, Action: Accept},
{Proto: TCP, Ports: []PortRange{{Start: 80}, {Start: 443}}, Source: "1.2.3.4", Action: Accept},
}
for _, r := range bad {
_, err := fw.MarshalAdvRule(r)
require.Error(t, err, "expected error marshalling %+v", *r)
for _, r := range routed {
require.True(t, fw.needsHook(r), "expected %+v to be routed to the hook, not marshalled", *r)
}
require.True(t, fw.isConfRule(&Rule{Proto: TCP, Port: 80, Action: Accept}),
"an address-less port accept belongs in conf.apf, not an advanced line")
}
// A port-only deny whose action is Drop (conf.apf's default ALL_STOP=DROP) must
@ -85,8 +87,7 @@ func TestAPFSourcePorts(t *testing.T) {
{&Rule{Proto: UDP, SourcePorts: []PortRange{{Start: 1024, End: 2048}}, Source: "192.0.2.1", Family: IPv4, Action: Accept}, "udp:in:s=1024_2048:s=192.0.2.1"},
}
for _, c := range cases {
got, err := fw.MarshalAdvRule(c.rule)
require.NoError(t, err, "failed to marshal %+v", *c.rule)
got := fw.MarshalAdvRule(c.rule)
require.Equal(t, c.want, got, "marshal %+v", *c.rule)
parsed := fw.ParseAdvRule(got, c.rule.Action)
@ -95,12 +96,14 @@ func TestAPFSourcePorts(t *testing.T) {
"round-trip mismatch: input %+v, line %q, output %+v", *c.rule, got, parsed)
}
// Matching both a source and a destination port is not representable.
_, err := fw.MarshalAdvRule(&Rule{Proto: TCP, Port: 22, SourcePort: 1234, Source: "1.2.3.4", Action: Accept})
require.Error(t, err, "expected error matching both source and destination ports")
// The single port-flow field holds one port match, so a source port matched with
// a destination port has no advanced-line form and routes to the hook instead.
require.True(t, fw.needsHook(&Rule{Proto: TCP, Port: 22, SourcePort: 1234, Source: "1.2.3.4", Action: Accept}),
"a dual-port rule must route to the hook")
// A source-port rule with no address has no advanced-rule form, so it routes to
// the hook; one carrying an address stays on the native advanced-rule path.
// A source-port rule with no address has no advanced-rule form either (an advanced
// line requires an address), so it routes to the hook; one carrying an address
// stays on the native advanced-rule path.
require.True(t, fw.needsHook(&Rule{Proto: TCP, SourcePort: 1234, Action: Accept}),
"an address-less source-port rule must route to the hook")
require.False(t, fw.needsHook(&Rule{Proto: TCP, SourcePort: 1234, Source: "1.2.3.4", Action: Accept}))
@ -588,14 +591,13 @@ func TestAPFDropRuleRemovable(t *testing.T) {
}
// A ported apf advanced rule that matches both a source and a destination address
// cannot be expressed (apf's advanced rule holds a single address field) and must
// be rejected rather than silently dropping the destination. The portless bare
// form is not tested here: AddRule diverts it to the raw-iptables hook
// (hostNeedsHook), so the writer never sees it.
func TestAPFDualAddressRejected(t *testing.T) {
// cannot be expressed (apf's advanced rule holds a single address field), so AddRule
// routes it to the raw-iptables hook rather than silently dropping the destination.
// The portless bare form takes the same path one predicate earlier (hostNeedsHook).
func TestAPFDualAddressRouted(t *testing.T) {
fw := new(APF)
_, err := fw.MarshalAdvRule(&Rule{Proto: TCP, Port: 22, Source: "1.2.3.4", Destination: "5.6.7.8", Action: Accept})
require.Error(t, err, "a dual-address apf advanced rule must be rejected")
require.True(t, fw.needsHook(&Rule{Proto: TCP, Port: 22, Source: "1.2.3.4", Destination: "5.6.7.8", Action: Accept}),
"a ported dual-address rule must be routed to the hook, never marshalled as an advanced line")
}
// APF's IG_*_CPORTS lists are dual-stack (one list applied to both v4 and v6), so
@ -628,11 +630,11 @@ func TestAPFConnLimitFamilyIsAny(t *testing.T) {
"FamilyAny connlimit must equal the APF read-back or Sync churns")
}
// TestAPFBareProtocolRoutesToHook is the apf analogue of the csf case: a bare
// protocol match (no port, no address) has no native apf construct but iptables
// expresses it directly, so AddRule diverts it to the pre-hook via needsHook
// rather than rejecting it. A native address-less ICMP accept stays out of the
// hook (it lives in conf.apf's type lists).
// TestAPFBareProtocolRoutesToHook covers a bare protocol match (no port, no
// address): it has no native apf construct but iptables expresses it directly, so
// AddRule diverts it to the pre-hook via needsHook rather than rejecting it. A
// native address-less ICMP accept stays out of the hook (it lives in conf.apf's
// type lists).
func TestAPFBareProtocolRoutesToHook(t *testing.T) {
fw := new(APF)
for _, r := range []*Rule{
@ -660,7 +662,8 @@ func TestAPFBareProtocolRoutesToHook(t *testing.T) {
// to here since no real conf.apf exists in this test environment. Using an action
// apf would not actually apply to this entry would make EditIPList reject it.
func TestAPFPortOnlyRejectFamily(t *testing.T) {
fw := new(APF)
// IPv6 enabled, so a family-neutral deny fans out to both families (see writeFamilyRows).
fw := &APF{ipv6Enabled: true}
ctx := context.Background()
for _, rule := range []*Rule{
{Action: Drop, Proto: TCP, Port: 80},
@ -693,6 +696,48 @@ func TestAPFPortOnlyRejectFamily(t *testing.T) {
}
}
// A port-only deny fans out across family, but the file may already hold a subset of
// those lines — a prior single-family add, a manual edit, or the same rule added
// twice by a reconcile. The add must note each fan-out line present and write only
// the rest. Regression for the single-"exists" gate: no one line covers a
// family-neutral target, so re-adding it duplicated the whole fan-out, and a target
// whose IPv4 line already existed had its IPv6 twin left unwritten.
func TestAPFPortOnlyDenyHealsAndDoesNotDuplicate(t *testing.T) {
ctx := context.Background()
// IPv6 enabled, so the deny fans out to an IPv4 and an IPv6 line.
fw := &APF{ipv6Enabled: true}
dir := t.TempDir()
// Adding the same family-neutral deny twice must leave one line per family.
path := filepath.Join(dir, "deny_hosts.rules")
require.NoError(t, os.WriteFile(path, nil, 0o644))
deny := &Rule{Family: FamilyAny, Proto: TCP, Port: 80, Action: Drop}
require.NoError(t, fw.EditIPList(ctx, path, Drop, deny, false))
require.NoError(t, fw.EditIPList(ctx, path, Drop, deny, false))
data, err := os.ReadFile(path)
require.NoError(t, err)
text := string(data)
require.Equal(t, 1, strings.Count(text, "tcp:in:d=80:s=0.0.0.0/0"),
"re-adding the rule must not duplicate the IPv4 line")
require.Equal(t, 1, strings.Count(text, "tcp:in:d=80:s=[::/0]"),
"re-adding the rule must not duplicate the IPv6 line")
// A file holding only the IPv4 line must gain the missing IPv6 one, so IPv6:80 is
// actually blocked rather than reported blocked while open.
path2 := filepath.Join(dir, "deny_hosts2.rules")
require.NoError(t, os.WriteFile(path2, []byte("tcp:in:d=80:s=0.0.0.0/0\n"), 0o644))
require.NoError(t, fw.EditIPList(ctx, path2, Drop, deny, false))
data2, err := os.ReadFile(path2)
require.NoError(t, err)
text2 := string(data2)
require.Equal(t, 1, strings.Count(text2, "tcp:in:d=80:s=0.0.0.0/0"),
"the pre-existing IPv4 line must be preserved, not duplicated")
require.Equal(t, 1, strings.Count(text2, "tcp:in:d=80:s=[::/0]"),
"the missing IPv6 line must be added")
}
// A bare all-protocol host rule (address, no port) is the one portless address
// shape apf's trust files express, written as the plain address line. The
// inexpressible shapes — a concrete-protocol host or a source+destination pair —
@ -894,8 +939,7 @@ func TestAPFTCPUDPAdvRule(t *testing.T) {
fw := new(APF)
r := &Rule{Proto: TCPUDP, Port: 80, Source: "192.0.2.1", Action: Accept, Direction: DirInput}
line, err := fw.MarshalAdvRule(r)
require.NoError(t, err)
line := fw.MarshalAdvRule(r)
require.Equal(t, "in:d=80:s=192.0.2.1", line, "the protocol field is omitted for both transports")
back := fw.ParseAdvRule(line, Accept)
@ -904,15 +948,18 @@ func TestAPFTCPUDPAdvRule(t *testing.T) {
require.True(t, back.EqualBase(r, true))
// A concrete transport names itself and round-trips unchanged.
line, err = fw.MarshalAdvRule(&Rule{Proto: TCP, Port: 80, Source: "192.0.2.1", Action: Accept})
require.NoError(t, err)
line = fw.MarshalAdvRule(&Rule{Proto: TCP, Port: 80, Source: "192.0.2.1", Action: Accept})
require.Equal(t, "tcp:in:d=80:s=192.0.2.1", line)
require.Equal(t, TCP, fw.ParseAdvRule(line, Accept).Proto)
// ProtocolAny has no advanced-rule form: the omitted field means tcp+udp, not
// every protocol, so emitting it would under-apply the rule.
_, err = fw.MarshalAdvRule(&Rule{Proto: ProtocolAny, Port: 80, Source: "192.0.2.1", Action: Accept})
require.ErrorIs(t, err, ErrUnsupported)
// every protocol, so emitting it would under-apply the rule. It never reaches the
// marshaller — a port on ProtocolAny is inexpressible in iptables too, so addRule
// rejects it (iptablesRuleValid) before EditIPList runs, and needsHook leaves it
// alone rather than sending an unmarshallable rule to the hook.
anyProto := &Rule{Proto: ProtocolAny, Port: 80, Source: "192.0.2.1", Action: Accept}
require.False(t, fw.needsHook(anyProto), "a port on ProtocolAny must not reach the hook")
require.Error(t, iptablesRuleValid(anyProto), "a port on ProtocolAny must be rejected before marshalling")
}
// TestAPFTCPUDPRouting pins where a both-transports rule goes. A FamilyAny bare port
@ -937,3 +984,42 @@ func TestAPFTCPUDPRouting(t *testing.T) {
// A portless tcpudp host has no plain-line form (a plain line is all-protocol).
require.True(t, hostNeedsHook(&Rule{Proto: TCPUDP, Source: "192.0.2.1", Action: Accept}))
}
// With conf.apf's USE_IPV6 off, apf installs no IPv6 rule from its config, so a
// family-neutral port-only deny must be written as the IPv4 line alone (see
// writeFamilyRows). Removal still sweeps both families.
func TestAPFPortOnlyDenyIPv6DisabledWritesV4Only(t *testing.T) {
ctx := context.Background()
dir := t.TempDir()
off := new(APF)
path := filepath.Join(dir, "deny_hosts.rules")
require.NoError(t, os.WriteFile(path, nil, 0o644))
deny := &Rule{Family: FamilyAny, Proto: TCP, Port: 80, Action: Drop}
require.NoError(t, off.EditIPList(ctx, path, Drop, deny, false))
data, err := os.ReadFile(path)
require.NoError(t, err)
require.Contains(t, string(data), "tcp:in:d=80:s=0.0.0.0/0", "the IPv4 line must be written")
require.NotContains(t, string(data), "[::/0]",
"no IPv6 line may be written while apf's IPv6 handling is off")
got, err := off.ParseIPList(path, Drop)
require.NoError(t, err)
require.Len(t, got, 1)
require.Equal(t, IPv4, got[0].impliedFamily())
// Switching IPv6 off must not strand the v6 line written while it was on.
on := &APF{ipv6Enabled: true}
bothPath := filepath.Join(dir, "deny_hosts.both.rules")
require.NoError(t, os.WriteFile(bothPath, nil, 0o644))
require.NoError(t, on.EditIPList(ctx, bothPath, Drop, deny, false))
data, err = os.ReadFile(bothPath)
require.NoError(t, err)
require.Contains(t, string(data), "[::/0]")
require.NoError(t, off.EditIPList(ctx, bothPath, Drop, deny, true))
data, err = os.ReadFile(bothPath)
require.NoError(t, err)
require.NotContains(t, string(data), "d=80",
"removal must sweep the stale IPv6 line even with IPv6 off")
}

632
csf_linux.go
View file

@ -165,27 +165,13 @@ func (f *CSF) ParseConnLimit(val string) (rules []*Rule) {
return
}
// csfAddrFamily parses an address (IP or CIDR) and reports its family, or false
// when the value is not a valid address.
func csfAddrFamily(v string) (Family, bool) {
cidrIP, _, err := net.ParseCIDR(v)
ip := net.ParseIP(v)
if err != nil && ip == nil {
return FamilyAny, false
}
if (cidrIP != nil && cidrIP.To4() == nil) || (ip != nil && ip.To4() == nil) {
return IPv6, true
}
return IPv4, true
}
// parseAddr classifies a csf advanced-rule address field. It returns the
// address, its family, and whether the value is an address at all (a non-address
// value is a port list or ICMP type). A zero "any" network (0.0.0.0/0 or ::/0)
// is normalized to an empty address so a port-only rule written with the "any"
// placeholder round-trips against a rule that carries no address.
func (f *CSF) parseAddr(v string) (addr string, fam Family, ok bool) {
family, ok := csfAddrFamily(v)
family, ok := parseAddrFamily(v)
if !ok {
return "", FamilyAny, false
}
@ -368,7 +354,7 @@ func (f *CSF) ParseIPList(filePath string, action Action) (rules []*Rule, err er
rules = append(rules, rule)
} else {
// Try to parse IP.
family, ok := csfAddrFamily(trimmed)
family, ok := parseAddrFamily(trimmed)
if !ok {
continue
}
@ -469,9 +455,10 @@ func (f *CSF) dropActions() (dropIn, dropOut Action) {
// features csf's native config cannot express.
func (f *CSF) hook() *hookScript {
return &hookScript{
rulePrefix: f.rulePrefix,
hookPath: CSFHook,
hookPerm: 0700,
rulePrefix: f.rulePrefix,
hookPath: CSFHook,
hookPerm: 0700,
ipv6Enabled: f.ipv6Enabled,
}
}
@ -796,53 +783,11 @@ func (f *CSF) advPortValue(specs []PortRange) string {
return strings.Join(parts, ",")
}
// MarshalAdvRule encodes a rule as a csf advanced allow/deny line. A csf
// advanced rule must carry a source or destination address.
func (f *CSF) MarshalAdvRule(r *Rule) (string, error) {
if r.Proto == ICMPv6 {
return "", fmt.Errorf("csf advanced rules do not support icmpv6")
}
if r.Source == "" && r.Destination == "" {
return "", fmt.Errorf("a csf advanced rule requires a source or destination address")
}
// A csf advanced rule holds a single address field, so a rule matching both a
// source and a destination cannot be expressed; reject it rather than silently
// dropping the destination (installing a broader rule and leaving it
// unremovable). Mirrors the dual-port guard below.
if r.Source != "" && r.Destination != "" {
return "", fmt.Errorf("csf advanced rules cannot match both a source and destination address")
}
// A csf advanced rule carries a single port-flow field, so a rule cannot match
// both a source and a destination port at once.
if r.HasPorts() && r.HasSourcePorts() {
return "", fmt.Errorf("csf advanced rules cannot match both a source and destination port")
}
// An ICMP advanced rule needs a concrete type for the port-flow field: without
// one the address would land there and csf would parse it as `--icmp-type <ip>`
// and drop the rule (see checkICMP). Refuse to emit such a line.
if r.Proto == ICMP && r.ICMPType == nil {
return "", fmt.Errorf("a csf icmp advanced rule requires a concrete icmp type")
}
// A single advanced line carries a single protocol token, so a TCPUDP port rule
// with an address cannot be expressed here: a protocol-less line is defaulted to
// `-p tcp` by csf.pl's linefilter, applying the rule to TCP only while the
// library reads it back as both transports — leaving UDP open on a deny (or
// unallowed on an accept). The address-less form is fanned into a tcp and a udp
// line by the caller (portOnlyDenyLines); one line cannot fan, so reject it here
// rather than under-apply it.
if r.Proto == TCPUDP {
return "", fmt.Errorf("a csf advanced rule cannot carry both transports; expand it to tcp and udp lines: %w", ErrUnsupported)
}
// csf.pl's linefilter reads an advanced line by fixed field position, not by
// tag: it always looks for the port-flow field (d=/s=) before the address
// field. A tcp/udp rule with an address but no port shifts the address into
// the port-flow slot instead, where it is parsed as a garbage --sport/--dport
// value and the address field is left empty — linefilter then requires both
// an address and a port to install anything, so the rule is silently dropped.
if r.Proto != ICMP && !r.HasPorts() && !r.HasSourcePorts() {
return "", fmt.Errorf("a csf tcp/udp advanced rule with an address requires a port: %w", ErrUnsupported)
}
// MarshalAdvRule encodes a rule as a csf advanced allow/deny line: a protocol
// token, a direction, one port-flow field (an icmp type, a source port or a
// destination port) and one address field, joined by "|". It validates nothing;
// addRule/RemoveRule route every shape the line cannot carry elsewhere first.
func (f *CSF) MarshalAdvRule(r *Rule) string {
var parts []string
switch r.Proto {
case TCP:
@ -877,66 +822,94 @@ func (f *CSF) MarshalAdvRule(r *Rule) (string, error) {
parts = append(parts, "d="+r.Destination)
}
return strings.Join(parts, "|"), nil
return strings.Join(parts, "|")
}
// advMatch reports whether a parsed advanced-rule line (the existing row) matches
// target. A TCPUDP port rule is written as a separate tcp line and udp line, so a
// TCPUDP target matches either concrete-transport line; that coverage, like the
// family and direction coverage the add and remove paths need, is folded into
// EqualForDedup / EqualForRemoval.
func (f *CSF) advMatch(parsed, target *Rule, remove bool) bool {
if remove {
return parsed.EqualForRemoval(target, true)
// parseListLine parses one csf.allow/csf.deny rule line into the rule it holds: an
// advanced rule, or a plain address line, which is a single bidirectional DirAny
// rule matching every protocol. It returns nil for a line that is neither, which the
// caller passes through untouched. The action comes from the file (csf.allow is
// accept, csf.deny is a deny); the line encodes none of its own.
func (f *CSF) parseListLine(line string, action Action) *Rule {
if strings.Contains(line, "|") {
return f.ParseAdvRule(line, action)
}
return parsed.EqualForDedup(target, true)
family, ok := parseAddrFamily(line)
if !ok {
return nil
}
return &Rule{Direction: DirAny, Family: family, Source: line, Action: action}
}
// portOnlyDenyLines returns the advanced csf.deny lines a port-only deny (no
// address) fans out to: one per transport (tcp and udp for a TCPUDP rule) and per
// family placeholder (0.0.0.0/0 for IPv4, ::/0 for IPv6, both for a family-neutral
// rule). It mirrors the emit path so the add logic can compare against the lines
// already in the file and write only the missing ones — a single "exists" flag
// would skip the whole fan-out when just one family/protocol line was already
// present, leaving the other family/protocol open.
func (f *CSF) portOnlyDenyLines(r *Rule) []string {
dir := "in"
if r.IsOutput() {
dir = "out"
}
port := f.advPortValue(r.PortSpecs())
var protos []string
for _, sub := range expandProtocols(r) {
protos = append(protos, sub.Proto.String())
}
var placeholders []string
switch r.impliedFamily() {
case IPv6:
placeholders = []string{"::/0"}
case IPv4:
placeholders = []string{"0.0.0.0/0"}
default:
placeholders = []string{"0.0.0.0/0", "::/0"}
}
var lines []string
for _, ph := range placeholders {
for _, p := range protos {
tokens := []string{p, dir, "d=" + port}
if r.IsOutput() {
tokens = append(tokens, "d="+ph)
} else {
tokens = append(tokens, "s="+ph)
// listRows returns the csf.allow/csf.deny rows a rule materializes into, in write
// order, or none for a shape the lists cannot hold. The rule must already carry the
// action its file implies (see EditIPList's match), since each row's read-back form
// is compared against lines stamped with it.
//
// csf has no both-transports line anywhere — csf.pl's linefilter silently reads a
// protocol-less advanced line as `-p tcp` — so a TCPUDP rule fans out into a tcp row
// and a udp row. A port-only deny additionally carries no address of its own, and
// csf's advanced-rule handler only emits an iptables rule for a line that has one, so
// each of its rows takes the "any" network as a placeholder address. That literal is
// family-specific, so a family-neutral rule fans out per family too rather than
// silently becoming IPv4-only — across the families csf actually enforces (see
// writeFamilyRows; with csf.conf's IPV6 off that is IPv4 alone). parseAddr normalizes
// the placeholder back to an empty address, so each row reads back as the address-less
// rule it stands for.
func (f *CSF) listRows(action Action, match *Rule) []listRow {
hasIP := match.Source != "" || match.Destination != ""
var rows []listRow
switch {
case hasIP && (match.HasPorts() || match.HasSourcePorts() || match.Proto.IsICMP()):
// A port/ICMP rule with an address is an advanced rule.
for _, sub := range expandProtocols(match) {
rows = append(rows, listRow{line: f.MarshalAdvRule(sub), read: sub})
}
case hasIP:
// A bare all-protocol host allow/deny: a single address matching every
// protocol. csf.allow/csf.deny hold no other portless address shape — a
// concrete-protocol host or a source+destination pair — so AddRule diverts
// those to the raw-iptables hook (hostNeedsHook) and never reaches here with
// one. A direct caller of this exported writer that supplies such a shape gets
// a best-effort single-address write, not a guard.
addr := match.Source
if addr == "" {
addr = match.Destination
}
// The plain line is bidirectional and names its address as the source, which is
// the frame the scan reads it back in.
read := *match
read.Direction = DirAny
read.Source, read.Destination = addr, ""
rows = append(rows, listRow{line: addr, read: &read})
case action != Accept && match.HasPorts():
for _, fam := range writeFamilyRows(f.ipv6Enabled, match) {
placeholder := "0.0.0.0/0"
if fam.impliedFamily() == IPv6 {
placeholder = "::/0"
}
for _, sub := range expandProtocols(fam) {
// The row reads back address-less; only the line carries the placeholder.
// advRuleNeedsHook has already routed an address-less source-port match to
// the hook, so the address field being filled in here is free.
row := *sub
if row.IsOutput() {
row.Destination = placeholder
} else {
row.Source = placeholder
}
rows = append(rows, listRow{line: f.MarshalAdvRule(&row), read: sub})
}
lines = append(lines, strings.Join(tokens, "|"))
}
}
return lines
return rows
}
// EditIPList adds or removes a rule in a csf.allow/csf.deny list, rewriting it in
// place. A port-only deny fans out per transport and family placeholder, so a
// TCPUDP rule reads back as the rule that was written rather than being re-added on
// every reconcile and never matched for removal.
// place. An add expands the rule into the rows it materializes into (see listRows),
// notes which of them the file already holds, and appends only the rest, so a rule
// that fans out across families or transports is completed rather than duplicated on
// every reconcile. A removal drops every line the target covers.
func (f *CSF) EditIPList(ctx context.Context, filePath string, action Action, r *Rule, remove bool) error {
// Read the allow/deny IP rule list.
fd, err := os.Open(filePath)
@ -953,20 +926,6 @@ func (f *CSF) EditIPList(ctx context.Context, filePath string, action Action, r
defer af.Abort()
scanner := bufio.NewScanner(fd)
exists := false
// A port-only deny (no address) fans out into several csf.deny lines across
// family and protocol. Track which of those lines are already in the file so the
// add path can write only the missing ones: the single "exists" flag below marks
// the whole rule present as soon as any one fan-out line matches, which would
// otherwise leave the other family/protocol open. wantDeny is empty for every
// other rule shape, so this tracking is inert unless the fan-out applies.
wantDeny := map[string]bool{}
if !remove && action != Accept && r.HasPorts() && r.Source == "" && r.Destination == "" {
for _, l := range f.portOnlyDenyLines(r) {
wantDeny[l] = true
}
}
presentDeny := map[string]bool{}
// csf.allow/csf.deny encode no action of their own — the file decides it
// (csf.allow is accept, csf.deny is a deny). A rule read from a file is stamped
// with that file's action, so match an incoming rule with its action coerced
@ -974,6 +933,14 @@ func (f *CSF) EditIPList(ctx context.Context, filePath string, action Action, r
// as the deny action) could never be found and removed.
match := *r
match.Action = action
// The rows an add must end up with, and which of them the scan finds already in
// the file. A removal wants no rows: it matches the target against each line
// directly, since a line it must drop need not be one this library would write.
var rows []listRow
if !remove {
rows = f.listRows(action, &match)
}
present := make([]bool, len(rows))
// Full-line comments immediately above a rule are held back so they can be
// dropped together with a removed rule (they are its comment) and written
// ahead of a kept one. A blank line detaches them.
@ -1019,129 +986,58 @@ func (f *CSF) EditIPList(ctx context.Context, filePath string, action Action, r
continue
}
// Note a fan-out line that is already present so the add path below skips it
// and writes only the missing family/protocol lines. Every such line is
// preserved (written back) by the pass-through branches below.
if wantDeny[line] {
presentDeny[line] = true
// A line neither form parses is not a rule; pass it through untouched.
cur := f.parseListLine(line, action)
if cur == nil {
flush()
_, _ = fmt.Fprintln(af, orig)
continue
}
if strings.Contains(line, "|") {
rule := f.ParseAdvRule(line, action)
if rule == nil {
flush()
_, _ = fmt.Fprintln(af, orig)
continue
}
if f.advMatch(rule, &match, remove) {
exists = true
if !remove {
flush()
_, _ = fmt.Fprintln(af, orig)
} else {
drop()
}
} else {
flush()
_, _ = fmt.Fprintln(af, orig)
}
} else {
// Try to parse IP.
family, ok := csfAddrFamily(line)
if !ok {
flush()
_, _ = fmt.Fprintln(af, orig)
// A removal drops every line the target covers, along with its comment. A
// TCPUDP or family-neutral target touches each of the concrete lines it was
// written as; that coverage is folded into EqualForRemoval.
if remove {
if cur.EqualForRemoval(&match, true) {
drop()
continue
}
flush()
_, _ = fmt.Fprintln(af, orig)
continue
}
// A plain IP line is one bidirectional DirAny rule; match the target
// against it in the inbound frame (canonicalMatch), so a DirAny or a
// concrete-direction input/output target that names this host lines up.
plainRule := &Rule{
Direction: DirAny,
Family: family,
Source: line,
Action: action,
}
if plainRule.EqualBase(match.canonicalMatch(), false) {
exists = true
if !remove {
flush()
_, _ = fmt.Fprintln(af, orig)
} else {
drop()
}
} else {
flush()
_, _ = fmt.Fprintln(af, orig)
// An add keeps every line and only notes which wanted rows the file already
// covers, so the tail writes the rest. Coverage rather than a text compare, so
// a row is satisfied by an existing line that spans it (a TCPUDP line absorbing
// a tcp row) and by one spelled differently but meaning the same.
for i := range rows {
if !present[i] && cur.EqualForDedup(rows[i].read, true) {
present[i] = true
}
}
flush()
_, _ = fmt.Fprintln(af, orig)
}
// Write any trailing comments that followed the last rule.
flush()
// If not exists and not remove, try adding the rule.
if !exists && !remove {
writeComment := func() {
if c := combineComment(f.rulePrefix, r.Comment); c != "" {
_, _ = fmt.Fprintln(af, "# "+c)
}
}
hasIP := r.Source != "" || r.Destination != ""
switch {
case hasIP && (r.HasPorts() || r.HasSourcePorts() || r.Proto.IsICMP()):
// A port/ICMP rule with an address is an advanced rule.
line, err := f.MarshalAdvRule(r)
if err != nil {
_ = fd.Close()
return err
}
writeComment()
_, _ = fmt.Fprintln(af, line)
case hasIP:
// A bare all-protocol host allow/deny: a single address matching every
// protocol. csf.allow/csf.deny hold no other portless address shape — a
// concrete-protocol host or a source+destination pair — so AddRule diverts
// those to the raw-iptables hook (hostNeedsHook) and never reaches here
// with one. A direct caller of this exported writer that supplies such a
// shape gets a best-effort single-address write, not a guard.
writeComment()
if r.Source != "" {
_, _ = fmt.Fprintln(af, r.Source)
} else {
_, _ = fmt.Fprintln(af, r.Destination)
}
// Append the wanted rows the file does not already hold. A rule that fans out is
// completed row by row: when only a subset is present (the IPv4 line but not its
// IPv6 twin, from a prior single-family add or a manual edit) the missing rows
// must still be written, or that family stays open while the library reports the
// rule in force.
writeComment := func() {
if c := combineComment(f.rulePrefix, r.Comment); c != "" {
_, _ = fmt.Fprintln(af, "# "+c)
}
}
// A port-only deny (no address) fans out into a csf.deny line per family and
// protocol. Unlike the single-line cases above it must NOT be gated on the
// whole-rule "exists" flag: when only a subset of the fan-out lines is already
// present (e.g. the IPv4 line but not the IPv6 one, from a prior single-family
// add or a manual edit), the missing lines must still be written or that
// family/protocol stays open while the library reports the port blocked. Emit
// only the lines not already present (presentDeny, noted during the scan).
//
// csf's advanced-rule handler only emits an iptables rule when the line carries
// an address, so each fan-out line uses the "any" network placeholder (0.0.0.0/0
// or ::/0) as the address; parseAddr normalizes it back to an empty address on
// read, so each family's line reads back as its own rule. The transport is named
// explicitly (a
// protocol-less line defaults to tcp in csf's linefilter), and a TCPUDP deny fans
// to both tcp and udp.
if len(wantDeny) > 0 {
writeComment := func() {
if c := combineComment(f.rulePrefix, r.Comment); c != "" {
_, _ = fmt.Fprintln(af, "# "+c)
}
}
for _, line := range f.portOnlyDenyLines(r) {
if presentDeny[line] {
continue
}
writeComment()
_, _ = fmt.Fprintln(af, line)
for i, row := range rows {
if present[i] {
continue
}
writeComment()
_, _ = fmt.Fprintln(af, row.line)
}
_ = fd.Close()
@ -1155,62 +1051,62 @@ func (f *CSF) EditIPList(ctx context.Context, filePath string, action Action, r
return af.Commit()
}
// checkConnLimit rejects a connection-limit request csf's CONNLIMIT cannot
// express, so an inexpressible one is reported rather than silently dropped.
func (f *CSF) checkConnLimit(r *Rule) error {
if r.ConnLimit == nil || f.isConnLimitRule(r) {
return nil
}
return fmt.Errorf("csf connection limiting (CONNLIMIT) applies only to a single inbound tcp port with no address, per source, rejecting the excess with a tcp reset: %w", ErrUnsupportedConnLimit)
// nativeICMP reports whether an ICMPv4 rule maps onto a csf advanced rule and so
// belongs in csf.allow/csf.deny rather than the raw-iptables hook. An advanced line
// requires an address, and its single port-flow field carries the icmp type, so only
// a rule with exactly one address and a concrete type has a faithful form: csf.pl's
// linefilter reads that field by position, and an address with no type would land the
// address there (`--icmp-type <ip>`), which csf fails to parse and drops silently. A
// source+destination pair overflows the single address field and is routed by
// advRuleNeedsHook. ICMPv6 never reaches this test — ruleNeedsHook sends it to the
// hook, or the IPv6 gate rejects it.
func (f *CSF) nativeICMP(r *Rule) bool {
return r.Proto == ICMP && r.ICMPType != nil && (r.Source != "") != (r.Destination != "")
}
// checkICMP rejects ICMP rules csf cannot express: csf advanced rules are
// built on iptables ICMP (IPv4) and require an address, and must also carry a
// concrete type: csf's linefilter treats the single port-flow field as the
// icmp-type, so an address with no type would put the address in that field
// (`--icmp-type <ip>`), which csf then fails to parse and drops silently (csf.pl
// linefilter). There is no csf advanced-rule encoding for "any icmp type from an
// address" — a bare host rule already covers all protocols — so reject it rather
// than emit a dropped line. ICMPv6 never reaches this check: ruleNeedsHook routes it
// to the pre-hook (or the IPv6 gate rejects it) before addRule/RemoveRule call checkICMP.
func (f *CSF) checkICMP(r *Rule) error {
if r.Proto == ICMP {
if r.Source == "" && r.Destination == "" {
return fmt.Errorf("a csf icmp rule requires a source or destination address: %w", ErrUnsupported)
}
if r.ICMPType == nil {
return fmt.Errorf("a csf icmp rule with an address requires a concrete icmp type: %w", ErrUnsupported)
}
// needsHook reports whether a rule must be injected through the csf pre-hook as a
// raw iptables rule because csf's native config cannot express it. It is the single
// gate between the hook path and csf's config files: everything it rejects (returns
// true) is written to the hook, everything it accepts (returns false) maps onto
// csf.conf or the csf.allow/csf.deny lists. The shared shapes (ruleNeedsHook,
// bareHostOneWay, hostNeedsHook, advRuleNeedsHook, bareProtoNeedsHook) keep their own
// predicates, since RemoveRule routes on ruleNeedsHook and bareHostOneWay directly.
func (f *CSF) needsHook(r *Rule) bool {
// Features csf's native config cannot express (connection state, per-rule
// interface, logging, rate limiting, forward-chain routing, icmpv6, a transport
// csf does not carry, an address set) go to the hook.
if ruleNeedsHook(r) {
return true
}
return nil
}
// checkPortProto rejects a port match on a protocol csf cannot express as a port.
// csf's port lists are TCP_IN/UDP_IN only, so a port on a concrete non-tcp/udp
// protocol (e.g. sctp) would otherwise be wrongly written into BOTH lists, and a
// port on ProtocolAny — which matches every IP protocol, not just the two csf can
// carry — has no faithful form at all. TCPUDP is allowed: it fans out to both lists
// on an address-less accept, and to a tcp and a udp csf.deny advanced line
// otherwise.
func (f *CSF) checkPortProto(r *Rule) error {
switch r.Proto {
case TCP, UDP, TCPUDP:
return nil
// A one-way bare host has no csf.allow/csf.deny form (a plain line is
// bidirectional, an advanced rule needs a port), and a concrete-protocol host or a
// source+destination pair likewise has none (see hostNeedsHook); all go to the hook.
if bareHostOneWay(r) || hostNeedsHook(r) {
return true
}
if r.HasPorts() || r.HasSourcePorts() {
return fmt.Errorf("csf requires a tcp or udp protocol for a port match: %w", ErrUnsupported)
// A ported source+destination pair, a source port matched with a destination port,
// and an address-less source-port match all overflow the advanced line's single
// address and single port-flow field (see advRuleNeedsHook); iptables matches each
// directly, so they go to the hook.
if advRuleNeedsHook(r) {
return true
}
return nil
}
// checkSourcePort rejects a source-port match csf cannot express. csf source
// ports live in an advanced rule, which requires an address, so a source-port
// rule without one has nowhere to go.
func (f *CSF) checkSourcePort(r *Rule) error {
if r.HasSourcePorts() && r.Source == "" && r.Destination == "" {
return fmt.Errorf("a csf source-port rule requires a source or destination address: %w", ErrUnsupportedSourcePort)
// A connection limit csf.conf's CONNLIMIT cannot express — anything but a
// per-source cap on a single address-less inbound tcp port rejecting the excess
// (isConnLimitRule) — goes to the hook's `-m connlimit` match.
if r.ConnLimit != nil && !f.isConnLimitRule(r) {
return true
}
return nil
// An ICMPv4 rule csf's advanced-rule format cannot carry (see nativeICMP) goes to
// the hook's `iptables -p icmp` match, which needs neither an address nor a type.
if r.Proto == ICMP && !f.nativeICMP(r) {
return true
}
// A bare protocol match with no address and no port has no native csf construct —
// csf.conf keys on a port, an advanced rule on address+port, and csf.allow/csf.deny
// on an address — but iptables expresses it directly, so it goes to the hook (see
// bareProtoNeedsHook). ICMP is excluded there and routed above.
return bareProtoNeedsHook(r)
}
// denyAction returns the action a csf.deny entry takes in the given direction,
@ -1251,12 +1147,11 @@ func (f *CSF) addRule(ctx context.Context, zoneName string, r *Rule, enforceIPv6
return nil
}
// csf has no both-transports construct: its port lists are a TCP list and a UDP
// list, and csf.pl's linefilter silently reads a protocol-less advanced line as
// `-p tcp`. So a TCPUDP rule fans out into a tcp rule and a udp rule, each routed
// independently, and each reads back as its own rule. Unlike apf — whose trust
// parser derives both transports from one protocol-less line — csf cannot express
// the pair in a single line anywhere.
// csf has no both-transports construct anywhere: its port lists are a TCP list and
// a UDP list, and csf.pl's linefilter silently reads a protocol-less advanced line
// as `-p tcp` rather than as both transports. So a TCPUDP rule fans out into a tcp
// rule and a udp rule, each routed independently, and each reads back as its own
// rule.
if r.Proto == TCPUDP {
for _, sub := range expandProtocols(r) {
if err := f.addRule(ctx, zoneName, sub, enforceIPv6Gate); err != nil {
@ -1266,49 +1161,23 @@ func (f *CSF) addRule(ctx context.Context, zoneName string, r *Rule, enforceIPv6
return nil
}
// Features csf's native config cannot express (connection-state, per-rule
// interface, logging, rate limiting, icmpv6) are injected as iptables rules
// through the csf pre-hook.
if ruleNeedsHook(r) {
_, err := f.hook().edit(r, false)
return err
}
// A one-way bare host allow/deny cannot be a plain csf line (bidirectional) nor
// an advanced rule (needs a port), so it is written to the hook. A DirAny bare
// host takes the plain-line path below.
if bareHostOneWay(r) {
_, err := f.hook().edit(r, false)
return err
}
// A concrete-protocol host or a source+destination pair likewise has no csf.allow/
// csf.deny form (see hostNeedsHook), so it too is injected as a raw iptables rule.
if hostNeedsHook(r) {
_, err := f.hook().edit(r, false)
return err
}
// A bare protocol match with no address and no port has no native csf construct —
// csf.conf keys on a port, an advanced rule on address+port, and csf.allow/csf.deny
// on an address — but iptables expresses it directly, so it is injected through the
// pre-hook (see bareProtoNeedsHook). ICMP keeps its own handling (checkICMP) and is
// excluded there.
if bareProtoNeedsHook(r) {
_, err := f.hook().edit(r, false)
return err
}
if err := f.checkSourcePort(r); err != nil {
return err
}
if err := f.checkConnLimit(r); err != nil {
return err
}
if err := f.checkICMP(r); err != nil {
return err
}
if err := f.checkPortProto(r); err != nil {
return err
// Verify the rule is valid with iptables.
if err := iptablesRuleValid(r); err != nil {
return fmt.Errorf("%v: %w", err, ErrUnsupported)
}
// A connection-limit rule maps onto the csf.conf CONNLIMIT list.
// Any shape csf's native config cannot express (a stateful/interface/logged/
// rate-limited rule, a one-way or concrete-protocol host, a source+destination
// pair, a source-and-destination port match, an address-less source-port match, a
// non-native connection limit, a non-native ICMPv4 rule, or a bare protocol match)
// is injected as a raw iptables rule through the csf pre-hook. See needsHook for
// each clause; everything past this gate maps onto csf's own config files.
if f.needsHook(r) {
_, err := f.hook().edit(r, false)
return err
}
// A native connection-limit rule maps onto the csf.conf CONNLIMIT list (a
// non-native one was diverted to the hook above by needsHook).
if r.ConnLimit != nil {
return f.EditConf(ctx, r, false)
}
@ -1432,56 +1301,44 @@ func (f *CSF) RemoveRule(ctx context.Context, zoneName string, r *Rule) error {
return nil
}
// Hook-injected rules (see AddRule) are removed from the managed script.
if ruleNeedsHook(r) {
_, err := f.hook().edit(r, true)
// Clear any hook copy of the rule first, no matter how csf stores it. A rule csf
// carries only in the hook (see needsHook) lives nowhere else, so this is its
// entire removal; a natively-expressible rule may still have a stray hook copy —
// the library's own (a deny whose action differs from csf.conf's is stored there,
// see AddRule) or one a customer added by hand for a shape csf can also express
// natively — that must be cleared before the native entry below. DirAny is expanded
// so both one-way hook lines are matched; a rule with no hook copy makes this a
// harmless no-op.
var err error
for _, sub := range expandDirections(r) {
if _, e := f.hook().edit(sub, true); e != nil {
err = e
break
}
}
// A rule csf carries only in the hook has no native entry to fall through to, so
// return once its hook copy is cleared (or on any hook error). Returning here also
// keeps such a rule out of the plain-line split scan below, which could wrongly
// split an unrelated coexisting native entry.
if ruleNeedsHook(r) || err != nil {
return err
}
// A one-way bare host rule is stored either as its own hook rule or as one
// direction of a bidirectional plain line; removing it may need to split the
// A one-way bare host rule is stored either as its own hook rule (cleared above) or
// as one direction of a bidirectional plain line; removing it may need to split the
// plain line (see removeBareHostOneWay).
if bareHostOneWay(r) {
return f.removeBareHostOneWay(ctx, zoneName, r)
}
// A concrete-protocol host or source+destination pair is stored only as its own
// hook rule (it has no plain-line form to split), so remove it directly.
if hostNeedsHook(r) {
_, err := f.hook().edit(r, true)
return err
// Every other shape csf's native config cannot express (see needsHook) has already
// had its hook copy cleared above and has no native entry to split, so it is done.
if f.needsHook(r) {
return nil
}
// A bare protocol match is stored only as its own hook rule (see AddRule), so
// remove it from the managed script directly. ICMP keeps its own handling below.
if bareProtoNeedsHook(r) {
_, err := f.hook().edit(r, true)
return err
}
if err := f.checkSourcePort(r); err != nil {
return err
}
if err := f.checkConnLimit(r); err != nil {
return err
}
if err := f.checkICMP(r); err != nil {
return err
}
if err := f.checkPortProto(r); err != nil {
return err
if err := iptablesRuleValid(r); err != nil {
return fmt.Errorf("%v: %w", err, ErrUnsupported)
}
// This rule is natively expressible, but a copy of it may nonetheless live in the
// hook — the library's own (a deny whose action differs from csf.conf's is stored
// there, see AddRule) or one a customer added by hand for a shape csf can also
// express natively. Clear any hook copy before removing the native entry, so a rule
// present in both csf's config and the hook is removed from both and a hook-only
// deny is fully removed here. DirAny is expanded so both one-way hook lines are
// matched; a rule with no hook copy makes this a harmless no-op.
for _, sub := range expandDirections(r) {
if _, err := f.hook().edit(sub, true); err != nil {
return err
}
}
// A connection-limit rule maps onto the csf.conf CONNLIMIT list.
// A native connection-limit rule maps onto the csf.conf CONNLIMIT list.
if r.ConnLimit != nil {
return f.EditConf(ctx, r, true)
}
@ -1494,22 +1351,21 @@ func (f *CSF) RemoveRule(ctx context.Context, zoneName string, r *Rule) error {
}
}
// Edit csf.allow if accept is the action, otherwise edit csf.deny. A deny whose
// action differs from csf.conf's action for its direction was only ever in the hook
// (cleared above), so return rather than match it against csf.deny — where it could
// coincide with a genuine matching-action entry. A matching-action deny lives in
// csf.deny.
// Edit csf.allow if accept is the action, otherwise edit csf.deny. A csf.deny entry
// carries no action of its own — csf applies csf.conf's action by direction — so the
// deny of an address is a single entry there, and it is removed whatever action the
// caller named: asking to stop denying something means the entry goes, or RemoveRule
// would report success while csf kept enforcing it. EditIPList coerces the target's
// action to the file's, so a differing-action deny still matches the line. The hook
// copy such a deny was added as (see AddRule) was already cleared above, exactly as
// the hook copy of a matching-action deny is, so both backings are swept either way.
if r.Action == Accept {
err := f.EditIPList(ctx, CSFAllow, Accept, r, true)
if err != nil {
return err
}
} else {
denyAction := f.denyAction(r.IsOutput())
if r.Action != denyAction {
return nil
}
err := f.EditIPList(ctx, CSFDeny, denyAction, r, true)
err := f.EditIPList(ctx, CSFDeny, f.denyAction(r.IsOutput()), r, true)
if err != nil {
return err
}
@ -1541,7 +1397,7 @@ func (f *CSF) UnmarshalNATRule(line string) *NATRule {
return nil
}
if ipx != "*" && ipx != "" {
if _, ok := csfAddrFamily(ipx); !ok {
if _, ok := parseAddrFamily(ipx); !ok {
return nil
}
r.Destination = ipx
@ -1563,7 +1419,7 @@ func (f *CSF) UnmarshalNATRule(line string) *NATRule {
return nil
}
} else {
fam, ok := csfAddrFamily(ipy)
fam, ok := parseAddrFamily(ipy)
if !ok {
return nil
}

272
csf_linux_test.go
View file

@ -110,7 +110,8 @@ func TestCSFTCPUDPPortDenyRoundTrip(t *testing.T) {
// gate that skipped the whole fan-out.
func TestCSFPortOnlyDenyHealsMissingFamily(t *testing.T) {
ctx := context.Background()
fw := new(CSF)
// IPv6 enabled, so the deny fans out across families and the missing v6 line heals.
fw := &CSF{ipv6Enabled: true}
dir := t.TempDir()
path := filepath.Join(dir, "csf.deny")
@ -143,41 +144,44 @@ func TestCSFPortOnlyDenyHealsMissingFamily(t *testing.T) {
"the missing tcp line must be added so tcp:53 is actually blocked")
}
// A csf advanced rule with an address, a port, and TCPUDP cannot be
// expressed as a single line: csf.pl defaults a protocol-less line to tcp, so
// udp would be silently left open. MarshalAdvRule must reject it rather than
// under-apply it (the port-only form fans out instead, but that path has no
// address to place).
func TestCSFAdvRuleTCPUDPWithAddressRejected(t *testing.T) {
// A csf advanced rule with an address, a port, and TCPUDP cannot be expressed as a
// single line: csf.pl defaults a protocol-less line to tcp, so udp would be silently
// left open. AddRule must therefore fan the rule into a tcp rule and a udp rule
// before it reaches MarshalAdvRule, each of which marshals to its own line.
func TestCSFAdvRuleTCPUDPWithAddressFansOut(t *testing.T) {
fw := new(CSF)
_, err := fw.MarshalAdvRule(&Rule{Family: IPv4, Proto: TCPUDP, Port: 443, Source: "192.0.2.10", Action: Drop})
require.ErrorIs(t, err, ErrUnsupported,
"an address+port rule with TCPUDP must be rejected, not written tcp-only")
both := &Rule{Family: IPv4, Proto: TCPUDP, Port: 443, Source: "192.0.2.10", Action: Drop}
subs := expandProtocols(both)
require.Len(t, subs, 2, "a TCPUDP rule must fan out before it is marshalled")
// A concrete protocol on the same shape is fine.
_, err = fw.MarshalAdvRule(&Rule{Family: IPv4, Proto: TCP, Port: 443, Source: "192.0.2.10", Action: Drop})
require.NoError(t, err)
var lines []string
for _, sub := range subs {
lines = append(lines, fw.MarshalAdvRule(sub))
}
require.Equal(t, []string{"tcp|in|d=443|s=192.0.2.10", "udp|in|d=443|s=192.0.2.10"}, lines,
"each transport must get its own line so udp is not left open")
}
// A csf tcp/udp advanced rule with an address but no port cannot be expressed:
// csf.pl's linefilter reads the port-flow field by position before the address
// field, so the address shifts into the port slot, gets parsed as a garbage
// --sport/--dport value, and the rule is silently dropped (linefilter requires
// both an address and a port match to install anything). MarshalAdvRule must
// reject this shape rather than emit an unenforceable line.
func TestCSFAdvRuleAddressWithoutPortRejected(t *testing.T) {
// both an address and a port match to install anything). AddRule must route this
// shape to the raw-iptables hook rather than let MarshalAdvRule emit an
// unenforceable line.
func TestCSFAdvRuleAddressWithoutPortRouted(t *testing.T) {
fw := new(CSF)
_, err := fw.MarshalAdvRule(&Rule{Family: IPv4, Proto: TCP, Source: "192.0.2.10", Action: Drop})
require.ErrorIs(t, err, ErrUnsupported,
"a tcp advanced rule with an address and no port must be rejected")
_, err = fw.MarshalAdvRule(&Rule{Family: IPv4, Proto: UDP, Destination: "192.0.2.10", Action: Accept})
require.ErrorIs(t, err, ErrUnsupported,
"a udp advanced rule with an address and no port must be rejected")
require.True(t, hostNeedsHook(&Rule{Family: IPv4, Proto: TCP, Source: "192.0.2.10", Action: Drop}),
"a tcp host with no port must go to the hook, not an advanced line")
require.True(t, hostNeedsHook(&Rule{Family: IPv4, Proto: UDP, Destination: "192.0.2.10", Action: Accept}),
"a udp host with no port must go to the hook, not an advanced line")
// An ICMP rule with an address and no port is unaffected by this guard (it
// already requires a concrete ICMP type via a separate check above).
_, err = fw.MarshalAdvRule(&Rule{Family: IPv4, Proto: ICMP, Source: "192.0.2.10", Action: Accept})
require.Error(t, err, "expected the existing icmp-without-type guard to fire, not this one")
// An ICMP rule with an address and no type takes its own route: csf's linefilter
// would consume the address as the icmp-type field, so it goes to the hook, whose
// iptables rule needs no type.
icmp := &Rule{Family: IPv4, Proto: ICMP, Source: "192.0.2.10", Action: Accept}
require.False(t, hostNeedsHook(icmp), "icmp keeps its own handling")
require.True(t, fw.needsHook(icmp), "a typeless icmp host must be routed to the hook")
}
func TestCSFParseAdvRuleIPv6(t *testing.T) {
@ -224,8 +228,7 @@ func TestCSFFeatureRules(t *testing.T) {
{&Rule{Direction: DirOutput, Proto: UDP, Port: 53, Destination: "192.0.2.1", Family: IPv4, Action: Accept}, "udp|out|d=53|d=192.0.2.1"},
}
for _, c := range cases {
got, err := fw.MarshalAdvRule(c.rule)
require.NoError(t, err, "failed to marshal %+v", *c.rule)
got := fw.MarshalAdvRule(c.rule)
require.Equal(t, c.want, got, "marshal %+v", *c.rule)
parsed := fw.ParseAdvRule(got, c.rule.Action)
@ -268,11 +271,12 @@ func TestCSFFeatureRules(t *testing.T) {
&Rule{Proto: TCP, Ports: []PortRange{{Start: 2000, End: 3000}}, Action: Accept}, false),
"unexpected csf.conf port edit")
// MarshalAdvRule rejects rules csf cannot express as advanced rules.
_, err := fw.MarshalAdvRule(&Rule{Proto: ICMPv6, Source: "2001:db8::1", Action: Accept})
require.Error(t, err, "expected error marshalling an icmpv6 advanced rule")
_, err = fw.MarshalAdvRule(&Rule{Proto: TCP, Port: 80, Action: Accept})
require.Error(t, err, "expected error marshalling an advanced rule without an address")
// An ICMPv6 rule has no advanced-line form at all and never reaches the
// marshaller: ruleNeedsHook routes it to the raw-iptables hook. (An address-less
// port rule has no advanced form either; it lands in a csf.conf port list or the
// csf.deny fan-out, covered by TestCSFTCPUDPRejectFansOut.)
require.True(t, ruleNeedsHook(&Rule{Proto: ICMPv6, Source: "2001:db8::1", Action: Accept}),
"an icmpv6 rule must be routed to the hook, never marshalled as an advanced line")
}
func TestCSFSourcePorts(t *testing.T) {
@ -289,8 +293,7 @@ func TestCSFSourcePorts(t *testing.T) {
{&Rule{Proto: TCP, SourcePorts: []PortRange{{Start: 2000, End: 3000}}, Source: "1.2.3.4", Family: IPv4, Action: Accept}, "tcp|in|s=2000_3000|s=1.2.3.4"},
}
for _, c := range cases {
got, err := fw.MarshalAdvRule(c.rule)
require.NoError(t, err, "failed to marshal %+v", *c.rule)
got := fw.MarshalAdvRule(c.rule)
require.Equal(t, c.want, got, "marshal %+v", *c.rule)
parsed := fw.ParseAdvRule(got, c.rule.Action)
@ -299,38 +302,51 @@ func TestCSFSourcePorts(t *testing.T) {
"round-trip mismatch: input %+v, line %q, output %+v", *c.rule, got, parsed)
}
// Matching both a source and a destination port is not representable.
_, err := fw.MarshalAdvRule(&Rule{Proto: TCP, Port: 22, SourcePort: 1234, Source: "1.2.3.4", Action: Accept})
require.Error(t, err, "expected error matching both source and destination ports")
// A source-port rule with no address has nowhere to go.
require.Error(t, fw.checkSourcePort(&Rule{Proto: TCP, SourcePort: 1234, Action: Accept}))
// The single port-flow field holds one port match, so a source port matched with
// a destination port has no advanced-line form; neither does an address-less
// source port, since an advanced line requires an address. AddRule routes both to
// the raw-iptables hook rather than marshalling them.
require.True(t, advRuleNeedsHook(&Rule{Proto: TCP, Port: 22, SourcePort: 1234, Source: "1.2.3.4", Action: Accept}),
"a dual-port rule must be routed to the hook")
require.True(t, advRuleNeedsHook(&Rule{Proto: TCP, SourcePort: 1234, Action: Accept}),
"an address-less source-port rule must be routed to the hook")
}
// An ICMP advanced rule that carries an address but no concrete type must be
// rejected: csf's linefilter would consume the address as the icmp-type field
// and silently drop the rule, while the library reported it enforced.
func TestCSFICMPRequiresType(t *testing.T) {
// Only an ICMP rule with exactly one address and a concrete type is a csf advanced
// rule; every other ICMP shape is routed to the raw-iptables hook, which needs
// neither. csf's linefilter would otherwise consume the address as the icmp-type
// field and silently drop the rule, while the library reported it enforced.
func TestCSFICMPRouting(t *testing.T) {
fw := new(CSF)
// Address but no type: rejected by both the validator and the marshaller.
noType := &Rule{Proto: ICMP, Source: "1.2.3.4", Action: Accept}
require.Error(t, fw.checkICMP(noType))
_, err := fw.MarshalAdvRule(noType)
require.Error(t, err, "must not emit an ICMP advanced line with the address in the type field")
// With a concrete type it is a valid advanced rule and round-trips.
// With one address and a concrete type it is a valid advanced rule and round-trips.
typed := &Rule{Proto: ICMP, ICMPType: Ptr[uint8](8), Source: "1.2.3.4", Action: Accept}
require.NoError(t, fw.checkICMP(typed))
line, err := fw.MarshalAdvRule(typed)
require.NoError(t, err)
require.True(t, fw.nativeICMP(typed))
require.False(t, fw.needsHook(typed), "a typed icmp host is a native advanced rule")
line := fw.MarshalAdvRule(typed)
require.Equal(t, "icmp|in|d=8|s=1.2.3.4", line)
parsed := fw.ParseAdvRule(line, Accept)
require.NotNil(t, parsed)
require.True(t, parsed.Equal(typed, true), "round-trip mismatch: %q -> %+v", line, parsed)
// An address-less ICMP rule is still rejected (advanced rules need an address).
require.Error(t, fw.checkICMP(&Rule{Proto: ICMP, ICMPType: Ptr[uint8](8), Action: Accept}))
// Every other ICMP shape has no advanced-line form, so it is written to the hook
// rather than rejected: iptables expresses each directly.
hooked := []*Rule{
{Proto: ICMP, Source: "1.2.3.4", Action: Accept}, // address, no type
{Proto: ICMP, ICMPType: Ptr[uint8](8), Action: Accept}, // type, no address
{Proto: ICMP, Action: Drop}, // neither
{Proto: ICMP, ICMPType: Ptr[uint8](8), Source: "1.2.3.4", Destination: "5.6.7.8", Action: Accept}, // both addresses
}
h := &hookScript{}
for _, r := range hooked {
require.False(t, fw.nativeICMP(r), "expected non-native icmp for %+v", *r)
require.True(t, fw.needsHook(r), "expected hook routing for %+v", *r)
lines, err := h.rulesToLines(r)
require.NoError(t, err, "the hook must be able to render %+v", *r)
require.Len(t, lines, 1)
require.Contains(t, lines[0], "iptables -A ")
require.Contains(t, lines[0], "-p icmp")
}
}
func TestCSFConnLimit(t *testing.T) {
@ -353,17 +369,31 @@ func TestCSFConnLimit(t *testing.T) {
require.Equal(t, `CONNLIMIT = "80;20"`, fw.editConnLimit("22;5,80;20", 22, 5, true))
require.Equal(t, `CONNLIMIT = "80;50"`, fw.editConnLimit("80;20", 80, 50, false))
// Only a single inbound tcp port, address-less, per-source, reject rule maps.
require.True(t, fw.isConnLimitRule(&Rule{Proto: TCP, Port: 80, Action: Reject, ConnLimit: &ConnLimit{Count: 5, PerSource: true}}))
bad := []*Rule{
// Only a single inbound tcp port, address-less, per-source, reject rule maps onto
// CONNLIMIT. Every other connection limit is written to the hook's `-m connlimit`
// match instead of being rejected, since iptables expresses each directly.
native := &Rule{Proto: TCP, Port: 80, Action: Reject, ConnLimit: &ConnLimit{Count: 5, PerSource: true}}
require.True(t, fw.isConnLimitRule(native))
require.False(t, fw.needsHook(native), "a native connlimit belongs in csf.conf")
hooked := []*Rule{
{Proto: TCP, Port: 80, Action: Drop, ConnLimit: &ConnLimit{Count: 5, PerSource: true}}, // wrong action (csf rejects, not drops)
{Proto: UDP, Port: 80, Action: Reject, ConnLimit: &ConnLimit{Count: 5, PerSource: true}}, // udp
{Proto: TCP, Port: 80, Source: "1.2.3.4", Action: Reject, ConnLimit: &ConnLimit{Count: 5, PerSource: true}}, // address
{Proto: TCP, Ports: []PortRange{{Start: 80, End: 90}}, Action: Reject, ConnLimit: &ConnLimit{Count: 5, PerSource: true}}, // range
{Proto: TCP, Port: 80, Action: Reject, ConnLimit: &ConnLimit{Count: 5, PerSource: false}}, // global
{Proto: TCP, Port: 80, Direction: DirOutput, Action: Reject, ConnLimit: &ConnLimit{Count: 5, PerSource: true}}, // outbound
}
for _, r := range bad {
require.Error(t, fw.checkConnLimit(r), "expected reject for %+v", *r)
h := &hookScript{}
for _, r := range hooked {
require.False(t, fw.isConnLimitRule(r), "expected non-native connlimit for %+v", *r)
require.True(t, fw.needsHook(r), "expected hook routing for %+v", *r)
lines, err := h.rulesToLines(r)
require.NoError(t, err, "the hook must be able to render %+v", *r)
require.NotEmpty(t, lines)
for _, line := range lines {
require.Contains(t, line, "-m connlimit --connlimit-above 5")
}
}
}
@ -546,27 +576,41 @@ func TestCSFDropRuleRemovable(t *testing.T) {
}
// A ported csf advanced rule that matches both a source and a destination address
// cannot be expressed (a csf advanced rule holds a single address field) and must
// be rejected rather than silently dropping the destination. The portless bare
// form is not tested here: AddRule diverts it to the raw-iptables hook
// (hostNeedsHook), so the writer never sees it.
func TestCSFDualAddressRejected(t *testing.T) {
fw := new(CSF)
_, err := fw.MarshalAdvRule(&Rule{Proto: TCP, Port: 80, Source: "1.2.3.4", Destination: "5.6.7.8", Action: Accept})
require.Error(t, err, "a dual-address csf advanced rule must be rejected")
// cannot be expressed (a csf advanced rule holds a single address field), so AddRule
// routes it to the raw-iptables hook rather than silently dropping the destination.
// The portless bare form takes the same path one predicate earlier (hostNeedsHook).
func TestCSFDualAddressRouted(t *testing.T) {
require.True(t, advRuleNeedsHook(&Rule{Proto: TCP, Port: 80, Source: "1.2.3.4", Destination: "5.6.7.8", Action: Accept}),
"a ported dual-address rule must be routed to the hook, never marshalled as an advanced line")
}
// csf's port lists are TCP_IN/UDP_IN; a concrete non-tcp/udp protocol (sctp) would
// wrongly be written into both, so it must error. ProtocolAny carries a port on no
// protocol csf can express — it matches every IP protocol — so it errors too. Only
// TCPUDP maps to both lists.
// csf's port lists are TCP_IN/UDP_IN. A port on a concrete protocol they cannot hold
// (sctp) is written to the hook, whose iptables rule matches it directly. A port on
// ProtocolAny is expressible nowhere — it matches every IP protocol, and iptables has
// no protocol-less port match — so it is the one shape csf still rejects.
func TestCSFPortProtoGuard(t *testing.T) {
fw := new(CSF)
require.Error(t, fw.checkPortProto(&Rule{Proto: SCTP, Port: 80, Action: Accept}), "sctp port must be rejected")
require.Error(t, fw.checkPortProto(&Rule{Port: 80, Action: Accept}), "an any-protocol port must be rejected")
require.NoError(t, fw.checkPortProto(&Rule{Proto: TCPUDP, Port: 80, Action: Accept}), "tcpudp port fans to both lists")
require.NoError(t, fw.checkPortProto(&Rule{Proto: TCP, Port: 80, Action: Accept}))
require.NoError(t, fw.checkPortProto(&Rule{Proto: ICMP, ICMPType: Ptr[uint8](8), Action: Accept}))
// An sctp port goes to the hook rather than into both csf.conf lists.
sctp := &Rule{Proto: SCTP, Port: 80, Action: Accept}
require.True(t, fw.needsHook(sctp), "an sctp port must be routed to the hook")
lines, err := (&hookScript{}).rulesToLines(sctp)
require.NoError(t, err)
for _, line := range lines {
require.Contains(t, line, "-p sctp")
require.Contains(t, line, "--dport 80")
}
// An any-protocol port has no form in csf's config nor in iptables, so the native
// path rejects it as unsupported.
anyProto := &Rule{Port: 80, Action: Accept}
require.False(t, fw.needsHook(anyProto), "an any-protocol port has no hook form either")
require.ErrorIs(t, fw.AddRule(context.Background(), "", anyProto), ErrUnsupported,
"an any-protocol port must be rejected")
// The shapes csf's own config carries stay off the hook.
require.False(t, fw.needsHook(&Rule{Proto: TCP, Port: 80, Action: Accept}))
require.False(t, fw.needsHook(&Rule{Proto: ICMP, ICMPType: Ptr[uint8](8), Source: "1.2.3.4", Action: Accept}))
}
// csf.conf's CONNLIMIT is a single dual-stack config key (it caps both v4 and v6
@ -574,7 +618,7 @@ func TestCSFPortProtoGuard(t *testing.T) {
// from it must be FamilyAny — not IPv4 — or a FamilyAny desired connlimit rule
// (the natural shape: no address, so no family is implied) never matches its own
// read-back and Sync removes-and-re-adds it every reconcile, firing csf -r each
// time. This mirrors the APF behavior above.
// time.
func TestCSFConnLimitFamilyIsAny(t *testing.T) {
// csf.pl only installs the ip6tables CONNLIMIT rule when csf.conf's IPV6 is
// enabled; only then does a dual-stack FamilyAny read-back (and the
@ -651,7 +695,8 @@ func TestCSFBareProtocolRoutesToHook(t *testing.T) {
// normalizes the "any" network back to an empty address, and a family-neutral rule
// writes one line per family, which cover it between them.
func TestCSFPortOnlyRejectRoundTrip(t *testing.T) {
fw := new(CSF)
// IPv6 enabled, so a family-neutral reject writes a line per family.
fw := &CSF{ipv6Enabled: true}
ctx := context.Background()
for _, rule := range []*Rule{
@ -711,9 +756,9 @@ func TestCSFBareHostWritten(t *testing.T) {
// A port-only "any"-source deny is written to csf.deny as a family-specific
// placeholder line (0.0.0.0/0 for IPv4, ::/0 for IPv6). The two lines cover
// different families, so adding the IPv6 twin while the IPv4 line already exists
// must write it — advMatch matches with EqualForDedup, so without the family
// coverage gate the IPv6 add was silently dropped as a false duplicate, leaving
// IPv6 open and making Sync churn forever.
// must write it — EditIPList matches an existing line with EqualForDedup, so without
// the family coverage gate the IPv6 add was silently dropped as a false duplicate,
// leaving IPv6 open and making Sync churn forever.
func TestCSFCrossFamilyAdvDenyBothWritten(t *testing.T) {
ctx := context.Background()
fw := new(CSF)
@ -744,11 +789,12 @@ func TestCSFCrossFamilyAdvDenyBothWritten(t *testing.T) {
}
// A FamilyAny port-only deny writes both placeholder lines and must still be
// idempotent on re-add and fully removable — the EqualForDedup/EqualForRemoval
// gate in advMatch must not disturb the FamilyAny case.
// idempotent on re-add and fully removable — EditIPList's EqualForDedup/
// EqualForRemoval gate must not disturb the FamilyAny case.
func TestCSFFamilyAnyAdvDenyRoundTrip(t *testing.T) {
ctx := context.Background()
fw := new(CSF)
// IPv6 enabled, so a FamilyAny deny fans out to both placeholder lines.
fw := &CSF{ipv6Enabled: true}
dir := t.TempDir()
path := filepath.Join(dir, "csf.deny")
require.NoError(t, os.WriteFile(path, nil, 0644))
@ -827,3 +873,55 @@ func TestCSFHasPrefixFlag(t *testing.T) {
require.Equal(t, "hand-added by an admin", rules[1].Comment)
require.False(t, rules[1].HasPrefix, "a comment without the prefix is not flagged")
}
// With csf.conf's IPV6 off, csf installs no IPv6 rule from its config, so a
// family-neutral port-only deny must be written as the IPv4 line alone. An IPv6
// placeholder line would sit inert in csf.deny and read back as an IPv6 rule csf does
// not enforce and AddRule would reject. Removal still matches the target against every
// line, so a v6 line written while IPv6 was on is swept regardless.
func TestCSFPortOnlyDenyIPv6DisabledWritesV4Only(t *testing.T) {
ctx := context.Background()
dir := t.TempDir()
off := new(CSF)
path := filepath.Join(dir, "csf.deny")
require.NoError(t, os.WriteFile(path, nil, 0644))
deny := &Rule{Family: FamilyAny, Proto: TCP, Port: 80, Action: Drop}
require.NoError(t, off.EditIPList(ctx, path, Drop, deny, false))
data, err := os.ReadFile(path)
require.NoError(t, err)
require.Contains(t, string(data), "tcp|in|d=80|s=0.0.0.0/0", "the IPv4 line must be written")
require.NotContains(t, string(data), "::/0",
"no IPv6 line may be written while csf's IPv6 handling is off")
// The rows read back cover the rule, and only for the family csf enforces.
got, err := off.ParseIPList(path, Drop)
require.NoError(t, err)
require.Len(t, got, 1)
require.Equal(t, IPv4, got[0].impliedFamily())
// A rule pinned to IPv6 still writes its line: the ipv6Unavailable gate stops a
// fresh add, and Restore bypasses that gate on purpose to reproduce a snapshot.
v6path := filepath.Join(dir, "csf.deny.v6")
require.NoError(t, os.WriteFile(v6path, nil, 0644))
v6 := &Rule{Family: IPv6, Proto: TCP, Port: 80, Action: Drop}
require.NoError(t, off.EditIPList(ctx, v6path, Drop, v6, false))
data, err = os.ReadFile(v6path)
require.NoError(t, err)
require.Contains(t, string(data), "tcp|in|d=80|s=::/0")
// Switching IPv6 off must not strand the v6 line written while it was on.
on := &CSF{ipv6Enabled: true}
bothPath := filepath.Join(dir, "csf.deny.both")
require.NoError(t, os.WriteFile(bothPath, nil, 0644))
require.NoError(t, on.EditIPList(ctx, bothPath, Drop, deny, false))
data, err = os.ReadFile(bothPath)
require.NoError(t, err)
require.Contains(t, string(data), "::/0")
require.NoError(t, off.EditIPList(ctx, bothPath, Drop, deny, true))
data, err = os.ReadFile(bothPath)
require.NoError(t, err)
require.NotContains(t, string(data), "d=80",
"removal must sweep the stale IPv6 line even with IPv6 off")
}

172
hooks_linux.go
View file

@ -2,6 +2,7 @@ package firewall
import (
"fmt"
"net"
"os"
"strings"
@ -33,6 +34,10 @@ type hookScript struct {
// hookPerm is the mode the hook file must carry to be executed (0700 for CSF,
// 0750 for APF).
hookPerm os.FileMode
// ipv6Enabled mirrors the backend's own IPv6 handling (csf.conf IPV6, conf.apf
// USE_IPV6). With it off, a family-agnostic rule is written for IPv4 only (see
// writeFamilies).
ipv6Enabled bool
}
// ruleNeedsHook reports whether a rule requires a feature that CSF/APF cannot
@ -64,6 +69,66 @@ func ipv6Unavailable(ipv6Enabled bool, r *Rule) bool {
return !ipv6Enabled && r.impliedFamily() == IPv6
}
// parseAddrFamily parses an address (IP or CIDR) and reports its family, or false
// when the value is not a valid address. The boolean is what distinguishes it from
// familyOfAddr, which reports FamilyAny for both an unset and an unparseable address:
// a config line is classified by whether it is an address at all. Shared by CSF and
// APF.
func parseAddrFamily(v string) (Family, bool) {
cidrIP, _, err := net.ParseCIDR(v)
ip := net.ParseIP(v)
if err != nil && ip == nil {
return FamilyAny, false
}
if (cidrIP != nil && cidrIP.To4() == nil) || (ip != nil && ip.To4() == nil) {
return IPv6, true
}
return IPv4, true
}
// listRow is one line a rule materializes into in a csf.allow/csf.deny or apf
// allow_hosts/deny_hosts file, paired with the rule that line reads back as. A rule
// spanning a family or transport axis the native line cannot carry has no single
// form, so it fans out into one row per cell. EditIPList marks the rows the file
// already holds as it scans and writes only the rest, so a partially present fan-out
// — one family written by an earlier single-family add, or a line lost to a manual
// edit — is completed rather than left half open or duplicated wholesale. Shared by
// CSF and APF.
type listRow struct {
// line is the exact text written to the list file.
line string
// read is the rule that line parses back to, which is what an existing line in
// the file is compared against to decide whether the row is already present.
read *Rule
}
// writeFamilyRows returns the concrete-family rows a rule is written as in a csf/apf
// list file, narrowed to the families the backend enforces. With its own IPv6 handling
// off, neither backend installs any IPv6 rule from its config, so a family-agnostic
// rule is written for IPv4 only: the IPv6 row would sit inert in the file and read back
// as an IPv6 rule the backend does not enforce and AddRule would reject. It is the
// list-file analog of hookScript.writeFamilies, and enabling IPv6 later heals the
// missing row (EditIPList completes a partial fan-out).
//
// A row already pinned to a concrete family keeps it, IPv6 included: a fresh
// concrete-IPv6 add is stopped earlier by the ipv6Unavailable gate, and Restore
// deliberately bypasses that gate to reproduce a snapshot's entries verbatim. Removal
// needs no such narrowing — it matches each line against the target directly, so a
// family-agnostic target already sweeps both rows. Shared by CSF and APF.
func writeFamilyRows(ipv6Enabled bool, r *Rule) []*Rule {
rows := expandFamilies(r)
if ipv6Enabled || len(rows) == 1 {
return rows
}
kept := make([]*Rule, 0, 1)
for _, row := range rows {
if row.Family != IPv6 {
kept = append(kept, row)
}
}
return kept
}
// bareHostShape reports whether a rule has the shape a plain csf.allow/apf
// allow_hosts line expresses: exactly one source or destination address, no ports,
// and the any-protocol match. Its direction is not considered — a DirAny bare host
@ -106,8 +171,8 @@ func dirAnyPlainLine(r *Rule) bool {
// advanced rule requires a port). Both are expressed natively as an iptables
// rule. A single-address all-protocol host is not covered here — a one-way one is
// routed by bareHostOneWay, a bidirectional one is a native plain line — and ICMP
// keeps its own handling (checkICMP/apfCheckICMP), so it is excluded. Shared by
// CSF and APF.
// keeps its own handling (CSF.nativeICMP, APF.isConfRule), so it is excluded. Shared
// by CSF and APF.
func hostNeedsHook(r *Rule) bool {
if r.Proto.IsICMP() || r.HasPorts() || r.HasSourcePorts() {
return false
@ -125,13 +190,52 @@ func hostNeedsHook(r *Rule) bool {
return false
}
// advRuleNeedsHook reports whether a rule that would otherwise be written as a
// csf/apf advanced rule must instead be injected through the raw-iptables hook,
// because the advanced-line format cannot carry its shape. An advanced line holds
// exactly one address field and exactly one port-flow field, and requires an
// address, so three shapes overflow it: a source+destination pair (the second
// address has nowhere to go), a source port matched together with a destination
// port, and an address-less source-port match. iptables expresses all three directly
// (`-s`/`-d`, `--sport`/`--dport`), so they are hooked rather than rejected — the
// alternative is failing on a rule the firewall can enforce.
//
// hostNeedsHook covers the portless address shapes and bows out once a rule carries
// a port, so this predicate is what routes their ported counterparts. Only the
// protocols an iptables port or icmp match accepts are routed here: a port on
// ProtocolAny is inexpressible in iptables too, so it stays on the native path and is
// rejected there by iptablesRuleValid rather than reaching the hook and failing there.
// Shared by CSF and APF.
func advRuleNeedsHook(r *Rule) bool {
if !onProtocolAxis(r.Proto) && !r.Proto.IsICMP() {
return false
}
// One port-flow field: a source port and a destination port cannot share it.
if r.HasPorts() && r.HasSourcePorts() {
return true
}
// An advanced rule requires an address, so a bare source-port match has no
// advanced form at all; iptables matches --sport on its own.
if r.HasSourcePorts() && r.Source == "" && r.Destination == "" {
return true
}
// One address field: a source+destination pair cannot share it. Only the advanced
// form is at stake, so a rule with neither a port nor an icmp match — which never
// reaches MarshalAdvRule — is left to hostNeedsHook.
if r.Source != "" && r.Destination != "" {
return r.HasPorts() || r.HasSourcePorts() || r.Proto.IsICMP()
}
return false
}
// bareProtoNeedsHook reports whether a rule is a bare protocol match — a non-ICMP
// transport with no address and no port — that CSF/APF cannot express in their
// native config (the trust files key on an address and the conf lists key on a
// port or icmp type) but iptables applies directly (`-p tcp -j ACCEPT`, or a bare
// `-j ACCEPT`). Such a rule is injected through the raw-iptables hook rather than
// rejected. ICMP/ICMPv6 keep their own native/hook handling (checkICMP/apfCheckICMP,
// nativeICMPv6, ruleNeedsHook), so they are excluded here. Shared by CSF and APF.
// rejected. ICMP/ICMPv6 keep their own native/hook handling (CSF.nativeICMP,
// APF.isConfRule, APF.nativeICMPv6, ruleNeedsHook), so they are excluded here. Shared
// by CSF and APF.
func bareProtoNeedsHook(r *Rule) bool {
return r.Source == "" && r.Destination == "" && !r.HasPorts() && !r.HasSourcePorts() && !r.Proto.IsICMP()
}
@ -158,9 +262,11 @@ func hookRuleProtos(r *Rule) []Protocol {
return protos
}
// hookRuleFamilies lists the address families a rule is written for: a rule
// pinned to a family (by address or an ICMP protocol) touches only that command,
// a family-agnostic rule (e.g. a bare state match) is written for both v4 and v6.
// hookRuleFamilies lists every address family a rule's hook lines can occupy: a
// rule pinned to a family (by address or an ICMP protocol) touches only that
// command, a family-agnostic rule (e.g. a bare state match) spans both v4 and v6.
// It is the full set, which is what removal must sweep; writeFamilies narrows it to
// the families the backend actually enforces.
func hookRuleFamilies(r *Rule) []Family {
switch r.impliedFamily() {
case IPv4:
@ -172,6 +278,26 @@ func hookRuleFamilies(r *Rule) []Family {
}
}
// writeFamilies lists the address families a rule's hook lines are written for.
// With the backend's own IPv6 handling off, a family-agnostic rule is written for
// IPv4 only: the pre-hook runs on every (re)load regardless, but neither csf nor apf
// flushes ip6tables while IPv6 is disabled (csf.pl guards the v6 flush behind IPV6;
// apf's ipt6() is a no-op unless USE_IPV6=1), so an injected ip6tables line would be
// re-appended on each reload and would outlive its own removal from the hook. That is
// the same hazard ipv6Unavailable rejects a concrete-IPv6 rule for; a family-agnostic
// rule is not rejected, it is simply written for the family the backend enforces.
//
// A rule already pinned to a concrete family keeps it, IPv6 included: a fresh
// concrete-IPv6 add is stopped earlier by the ipv6Unavailable gate, and Restore
// deliberately bypasses that gate to reproduce a snapshot's entries verbatim.
func (h *hookScript) writeFamilies(r *Rule) []Family {
fams := hookRuleFamilies(r)
if h.ipv6Enabled || len(fams) == 1 {
return fams
}
return []Family{IPv4}
}
// hookCommand returns the iptables command name for a family.
func hookCommand(fam Family) string {
if fam == IPv6 {
@ -204,15 +330,29 @@ func shellSafeToken(tok string) string {
return "'" + strings.ReplaceAll(tok, "'", `'\''`) + "'"
}
// rulesToLines encodes a rule as the raw command line(s) to inject: one iptables
// (or ip6tables) command per underlying iptables line and per family. A logged
// rule yields a LOG line followed by its action line, as with the iptables
// rulesToLines encodes a rule as the raw command line(s) to inject, for the
// families the backend enforces (see writeFamilies).
func (h *hookScript) rulesToLines(r *Rule) ([]string, error) {
return h.linesForFamilies(r, h.writeFamilies(r))
}
// removalLines encodes every hook line a rule could occupy, across both families
// regardless of the backend's IPv6 setting. Removal sweeps the wider set so an
// ip6tables line written while IPv6 was enabled — or added by hand — is still
// cleared once it is switched off, rather than stranded in the hook.
func (h *hookScript) removalLines(r *Rule) ([]string, error) {
return h.linesForFamilies(r, hookRuleFamilies(r))
}
// linesForFamilies encodes a rule as the raw command line(s) to inject: one iptables
// (or ip6tables) command per underlying iptables line and per requested family. A
// logged rule yields a LOG line followed by its action line, as with the iptables
// backend. Each marshalled line is re-tokenized and re-quoted shell-safely,
// because the hook script is sourced by /bin/sh rather than exec'd argv-style.
func (h *hookScript) rulesToLines(r *Rule) ([]string, error) {
func (h *hookScript) linesForFamilies(r *Rule, fams []Family) ([]string, error) {
var out []string
for _, proto := range hookRuleProtos(r) {
for _, fam := range hookRuleFamilies(r) {
for _, fam := range fams {
rc := *r
rc.Proto = proto
rc.Family = fam
@ -362,7 +502,13 @@ func (h *hookScript) writeHook(lines []string, existed bool) error {
// line — user-authored shell and rules alike — and reports whether the hook
// changed. Adding to an absent hook creates it; removing from one is a no-op.
func (h *hookScript) edit(r *Rule, remove bool) (bool, error) {
desired, err := h.rulesToLines(r)
// An add writes only the families the backend enforces; a removal sweeps both, so
// a stale ip6tables line does not outlive an IPv6 switch-off (see writeFamilies).
linesFor := h.rulesToLines
if remove {
linesFor = h.removalLines
}
desired, err := linesFor(r)
if err != nil {
return false, err
}

122
hooks_linux_test.go
View file

@ -40,6 +40,51 @@ func TestRuleNeedsHook(t *testing.T) {
}
}
// A csf/apf advanced line holds one address field and one port-flow field. A rule
// that overflows either must reach the raw-iptables hook, which matches -s/-d and
// --sport/--dport together: before advRuleNeedsHook existed, AddRule fell through to
// MarshalAdvRule and failed on a rule the firewall can enforce.
func TestAdvRuleNeedsHook(t *testing.T) {
// Shapes the single address / single port-flow field cannot hold.
needs := []*Rule{
// A source+destination pair carrying a port: hostNeedsHook bows out once a
// rule has a port, so this predicate is what routes it.
{Proto: TCP, Port: 80, Source: "192.0.2.1", Destination: "198.51.100.1", Action: Accept},
{Proto: TCPUDP, Port: 80, Source: "192.0.2.1", Destination: "198.51.100.1", Action: Accept},
{Proto: TCP, SourcePort: 1234, Source: "192.0.2.1", Destination: "198.51.100.1", Action: Accept},
// A source+destination pair on an ICMP match, typed or not.
{Proto: ICMP, ICMPType: Ptr[uint8](8), Source: "192.0.2.1", Destination: "198.51.100.1", Action: Accept},
{Proto: ICMP, Source: "192.0.2.1", Destination: "198.51.100.1", Action: Accept},
// A source port matched together with a destination port, addressed or not.
{Proto: TCP, Port: 80, SourcePort: 1234, Source: "192.0.2.1", Action: Accept},
{Proto: UDP, Port: 53, SourcePort: 53, Destination: "192.0.2.1", Action: Accept},
{Proto: TCP, Port: 80, SourcePort: 1234, Action: Accept},
// An advanced rule requires an address, so a bare source-port match has no
// advanced form of any kind.
{Proto: TCP, SourcePort: 1234, Action: Accept},
{Proto: TCPUDP, SourcePort: 1234, Action: Drop},
}
for _, r := range needs {
require.True(t, advRuleNeedsHook(r), "expected %+v to need the hook", *r)
}
// Shapes the advanced line holds, or that another predicate routes.
native := []*Rule{
// One address, one port-flow field: the advanced rule itself.
{Proto: TCP, Port: 22, Source: "192.0.2.1", Action: Accept},
{Proto: TCP, SourcePort: 1234, Source: "192.0.2.1", Action: Accept},
{Proto: ICMP, ICMPType: Ptr[uint8](8), Source: "192.0.2.1", Action: Accept},
// A portless source+destination pair belongs to hostNeedsHook.
{Source: "192.0.2.1", Destination: "198.51.100.1", Action: Accept},
// A port on ProtocolAny is inexpressible in iptables too, so it must keep its
// own rejection rather than reach the hook and fail there.
{Proto: ProtocolAny, Port: 80, SourcePort: 1234, Source: "192.0.2.1", Action: Accept},
{Proto: ProtocolAny, Port: 80, Source: "192.0.2.1", Destination: "198.51.100.1", Action: Accept},
}
for _, r := range native {
require.False(t, advRuleNeedsHook(r), "expected %+v not to route to the hook", *r)
}
}
// With csf.conf's IPV6 or conf.apf's USE_IPV6 off (the shipped default in both),
// neither backend enforces any IPv6, and the raw-iptables hook is no escape hatch:
// neither firewall flushes ip6tables on reload, so a hook-injected v6 line is
@ -260,12 +305,14 @@ func TestHookScriptRemoveIgnoresComment(t *testing.T) {
func TestHookScriptRoundTrip(t *testing.T) {
dir := t.TempDir()
h := &hookScript{
rulePrefix: "go_firewall",
hookPath: filepath.Join(dir, "csfpre.sh"),
hookPerm: 0700,
rulePrefix: "go_firewall",
hookPath: filepath.Join(dir, "csfpre.sh"),
hookPerm: 0700,
ipv6Enabled: true,
}
// A family-agnostic rule is injected for both v4 and v6.
// A family-agnostic rule is injected for both v4 and v6 when the backend enforces
// IPv6.
lines, err := h.rulesToLines(&Rule{Proto: TCP, Port: 8080, Action: Accept, State: StateNew})
require.NoError(t, err)
require.Len(t, lines, 2)
@ -623,3 +670,70 @@ func TestHookAddressSetEntryEdits(t *testing.T) {
_, err = h.editAddressSetEntry("missing", "1.2.3.4", false)
require.Error(t, err, "editing an entry in a set that does not exist must fail")
}
// With the backend's own IPv6 handling off, a family-agnostic rule must be injected
// as an IPv4 line only. The pre-hook runs on every (re)load regardless, but csf/apf
// never flush ip6tables while IPv6 is disabled, so an injected ip6tables line would
// be re-appended on each reload and would outlive its removal from the hook. The
// ipv6Unavailable gate only stops a *concrete* IPv6 rule; a FamilyAny rule reaches
// the hook and must be narrowed here instead.
func TestHookScriptIPv6DisabledSkipsV6Family(t *testing.T) {
dir := t.TempDir()
h := &hookScript{
rulePrefix: "go_firewall",
hookPath: filepath.Join(dir, "csfpre.sh"),
hookPerm: 0700,
}
anyFam := &Rule{Proto: TCP, Port: 8080, Action: Accept, State: StateNew}
lines, err := h.rulesToLines(anyFam)
require.NoError(t, err)
require.Len(t, lines, 1, "a family-agnostic rule must not be written for ipv6 when ipv6 is off")
require.True(t, strings.HasPrefix(lines[0], "iptables "), "want an iptables line, got %q", lines[0])
// It is written to the hook the same way, so no ip6tables command is ever injected.
changed, err := h.edit(anyFam, false)
require.NoError(t, err)
require.True(t, changed)
data, err := os.ReadFile(h.hookPath)
require.NoError(t, err)
require.NotContains(t, string(data), "ip6tables ",
"an ip6tables line csf/apf never flush must not be injected while ipv6 is off")
// A rule pinned to a concrete family keeps it: the ipv6Unavailable gate stops a
// fresh concrete-IPv6 add, and Restore bypasses that gate on purpose to reproduce a
// snapshot's entries verbatim, so the hook must still be able to render one.
v6 := &Rule{Family: IPv6, Proto: TCP, Port: 8080, Action: Accept, State: StateNew}
lines, err = h.rulesToLines(v6)
require.NoError(t, err)
require.Len(t, lines, 1)
require.True(t, strings.HasPrefix(lines[0], "ip6tables "), "want an ip6tables line, got %q", lines[0])
}
// Switching IPv6 off must not strand the ip6tables lines written while it was on:
// removal sweeps both families even though an add only writes the enforced one.
func TestHookScriptRemoveSweepsV6AfterIPv6Disabled(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "csfpre.sh")
// Written while the backend's IPv6 handling was on: an iptables and an ip6tables line.
on := &hookScript{rulePrefix: "go_firewall", hookPath: path, hookPerm: 0700, ipv6Enabled: true}
rule := &Rule{Proto: TCP, Port: 8080, Action: Accept, State: StateNew}
changed, err := on.edit(rule, false)
require.NoError(t, err)
require.True(t, changed)
data, err := os.ReadFile(path)
require.NoError(t, err)
require.Contains(t, string(data), "ip6tables ")
// IPv6 is now off. Removing the same rule must still clear the stale ip6tables line.
off := &hookScript{rulePrefix: "go_firewall", hookPath: path, hookPerm: 0700}
changed, err = off.edit(rule, true)
require.NoError(t, err)
require.True(t, changed)
data, err = os.ReadFile(path)
require.NoError(t, err)
require.NotContains(t, string(data), "ip6tables ",
"a stale ip6tables line must be swept on removal, not stranded in the hook")
require.NotContains(t, string(data), "iptables -A ")
}

112
integration_test.go
View file

@ -227,6 +227,30 @@ func runManagerSuite(t *testing.T, mgr Manager, reconstruct func(context.Context
roundTripRule(t, ctx, mgr, zone, &Rule{Proto: TCP, Ports: []PortRange{{Start: 6001, End: 6001}, {Start: 6002, End: 6002}}, Source: "192.0.2.43/32", Action: Accept})
})
t.Run("advhookoverflow", func(t *testing.T) {
// A csf/apf advanced line holds one address field and one port-flow field, so a
// ported source+destination pair and a source-plus-destination port match both
// overflow it. Neither backend may reject them: iptables matches -s with -d and
// --sport with --dport directly, so AddRule routes both to the raw-iptables hook
// (advRuleNeedsHook). Gated to csf/apf; the chain backends express these
// natively and cover them elsewhere.
switch mgr.Type() {
case CSFType, APFType:
default:
t.Skip("only csf/apf route these shapes to the hook")
}
// A source+destination pair carrying a destination port. hostNeedsHook stops at
// the port, so this is the shape that used to reach MarshalAdvRule and fail.
roundTripRule(t, ctx, mgr, zone, &Rule{Proto: TCP, Port: 8443, Source: "192.0.2.50/32", Destination: "198.51.100.50/32", Action: Accept})
// The same pair on a source port, and on a typed ICMP match.
roundTripRule(t, ctx, mgr, zone, &Rule{Proto: UDP, SourcePort: 5353, Source: "192.0.2.51/32", Destination: "198.51.100.51/32", Action: Accept})
roundTripRule(t, ctx, mgr, zone, &Rule{Proto: ICMP, ICMPType: Ptr[uint8](13), Source: "192.0.2.52/32", Destination: "198.51.100.52/32", Action: Accept})
// A source port matched together with a destination port, with and without an
// address. Avoid port 22 in either field so an SSH-driven run keeps its session.
roundTripRule(t, ctx, mgr, zone, &Rule{Proto: TCP, Port: 8444, SourcePort: 5354, Source: "192.0.2.53/32", Action: Accept})
roundTripRule(t, ctx, mgr, zone, &Rule{Proto: TCP, Port: 8445, SourcePort: 5355, Action: Accept})
})
t.Run("negatedaddress", func(t *testing.T) {
// ufw's tuple grammar cannot negate an address, so a negated source is routed
// to the before.rules raw path (iptables `! -s`) rather than rejected, and
@ -921,11 +945,13 @@ func runManagerSuite(t *testing.T, mgr Manager, reconstruct func(context.Context
t.Run("icmp", func(t *testing.T) {
// ICMP has genuinely different shapes per backend: nft/iptables/ufw accept a
// bare rule; apf models ICMP as a list of allowed types so it needs a type
// (and forbids an address); csf builds ICMP on host-based rules so it needs
// an address AND a type (its advanced-rule format carries the type in the
// single port-flow field, so an address with no type is silently dropped by
// csf). Offer all forms and use the first the backend accepts.
// bare rule; apf models ICMP as a list of allowed types, so a rule with an
// address or a non-accept action goes to its hook; csf builds native ICMP on
// host-based advanced rules needing an address AND a type (the advanced-rule
// format carries the type in the single port-flow field), so every other shape
// goes to its hook. Both hook forms are plain iptables rules, so the bare
// variant is what csf and apf match here. Offer all forms and use the first the
// backend accepts.
roundTripVariants(t, ctx, mgr, zone,
&Rule{Family: IPv4, Proto: ICMP, Action: Accept},
&Rule{Family: IPv4, Proto: ICMP, ICMPType: Ptr[uint8](8), Action: Accept},
@ -970,17 +996,29 @@ func runManagerSuite(t *testing.T, mgr Manager, reconstruct func(context.Context
})
t.Run("icmptype", func(t *testing.T) {
// A specific ICMP type, with an address fallback for csf (which requires one
// on any ICMP rule). Use type 13 (Timestamp) rather than the more common
// echo-request type 8 to avoid colliding with Windows' built-in echo-request
// rules, which cannot be removed and would make the round-trip assertions
// match the wrong rule.
// A specific ICMP type, with an addressed fallback for a backend whose native
// type list cannot carry the bare form. Use type 13 (Timestamp) rather than the
// more common echo-request type 8 to avoid colliding with Windows' built-in
// echo-request rules, which cannot be removed and would make the round-trip
// assertions match the wrong rule.
roundTripVariants(t, ctx, mgr, zone,
&Rule{Family: IPv4, Proto: ICMP, ICMPType: Ptr[uint8](13), Action: Accept},
&Rule{Family: IPv4, Proto: ICMP, ICMPType: Ptr[uint8](13), Source: "192.0.2.0/24", Action: Accept},
)
})
t.Run("csfnativeicmp", func(t *testing.T) {
// csf's only native ICMP form is an advanced rule carrying exactly one address
// and a concrete type; every other shape routes to the hook (nativeICMP), so
// the shared icmp/icmptype probes match csf on its bare hook form first.
// Round-trip the native form here to keep the csf.allow advanced-rule path
// covered. Gated to csf; type 13 (Timestamp) avoids echo-request collisions.
if mgr.Type() != CSFType {
t.Skip("csf-specific advanced-rule ICMP path")
}
roundTripRule(t, ctx, mgr, zone, &Rule{Family: IPv4, Proto: ICMP, ICMPType: Ptr[uint8](13), Source: "192.0.2.71/32", Action: Accept})
})
t.Run("apficmphook", func(t *testing.T) {
// apf carries ICMP as a zone-wide allowed-type list (IG_ICMP_TYPES), so an
// ICMP rule with an address or a non-accept action has no native form and
@ -1018,8 +1056,9 @@ func runManagerSuite(t *testing.T, mgr Manager, reconstruct func(context.Context
t.Run("sourceport", func(t *testing.T) {
// Source-port alone where supported (note firewalld rich rules can match a
// source port but not together with a destination port, so the probe never
// combines the two); apf requires an address on a source-port rule, so fall
// back to a form carrying a source address.
// combines the two). csf/apf have no address-less advanced-rule form, so they
// route this through the raw-iptables hook; the addressed fallback remains for
// any backend that needs an address on a source-port rule.
roundTripVariants(t, ctx, mgr, zone,
&Rule{Proto: TCP, SourcePort: 1234, Action: Accept},
&Rule{Proto: TCP, SourcePort: 1234, Source: "192.0.2.0/24", Action: Accept},
@ -1054,10 +1093,11 @@ func runManagerSuite(t *testing.T, mgr Manager, reconstruct func(context.Context
t.Run("connlimit", func(t *testing.T) {
requireCap(t, caps.ConnLimit)
// Connection limiting splits across backends: nft expresses only a global
// cap; apf a per-source one that rejects the excess; csf a per-source one
// that drops it; pf a per-source one only on an accept rule (single inbound
// tcp port, no address). iptables does the global form. Try each and
// round-trip the first the backend accepts.
// cap; pf a per-source one only on an accept rule (single inbound tcp port, no
// address); iptables does the global form. csf and apf carry one native shape
// each in their config and route every other shape to their hook, whose
// iptables rule does the global form too. Try each and round-trip the first the
// backend accepts.
roundTripVariants(t, ctx, mgr, zone,
&Rule{Proto: TCP, Port: 80, Action: Drop, ConnLimit: &ConnLimit{Count: 20}},
&Rule{Proto: TCP, Port: 80, Action: Reject, ConnLimit: &ConnLimit{Count: 20, PerSource: true}},
@ -1079,6 +1119,46 @@ func runManagerSuite(t *testing.T, mgr Manager, reconstruct func(context.Context
roundTripRule(t, ctx, mgr, zone, &Rule{Proto: TCP, Port: 8081, Action: Reject, ConnLimit: &ConnLimit{Count: 15, PerSource: true}})
})
t.Run("csfdenyremovedanyaction", func(t *testing.T) {
// A csf.deny entry carries no action of its own — csf applies csf.conf's action
// by direction — so the deny of an address is a single entry there and must be
// removable whatever action the caller names, as apf's deny_hosts entry is.
// Otherwise RemoveRule reports success while csf keeps enforcing the entry.
// Gated to csf; the stock csf.conf DROP is "DROP", so Drop is the native inbound
// deny action and Reject is the differing one.
if mgr.Type() != CSFType {
t.Skip("csf-specific csf.deny action-agnostic removal")
}
host := "192.0.2.72/32"
added := &Rule{Family: IPv4, Proto: TCP, Port: 8084, Source: host, Action: Drop}
require.NoError(t, mgr.AddRule(ctx, zone, added))
t.Cleanup(func() { _ = mgr.RemoveRule(ctx, zone, added) })
require.True(t, containsRule(rulesOf(t, ctx, mgr, zone), added, mgr.Capabilities().Output),
"the native deny must be present before removal")
// The same rule, named with the other deny action, must still clear the entry.
differing := *added
differing.Action = Reject
require.NoError(t, mgr.RemoveRule(ctx, zone, &differing))
for _, r := range rulesOf(t, ctx, mgr, zone) {
require.Falsef(t, addrEqual(r.Source, host) && r.Port == 8084,
"the csf.deny entry must be removed whatever action the target names: %+v", r)
}
})
t.Run("csfnativeconnlimit", func(t *testing.T) {
// csf writes a native connection limit — a single inbound tcp port, no address,
// per-source, rejecting the excess — to csf.conf's CONNLIMIT list rather than
// the hook (isConnLimitRule). Every other connection-limit shape now routes to
// the hook (needsHook), so the shared connlimit probe matches csf on its global
// hook form first; round-trip the native form here to keep the CONNLIMIT path
// covered. Gated to csf.
if mgr.Type() != CSFType {
t.Skip("csf-specific csf.conf CONNLIMIT path")
}
roundTripRule(t, ctx, mgr, zone, &Rule{Proto: TCP, Port: 8082, Action: Reject, ConnLimit: &ConnLimit{Count: 15, PerSource: true}})
})
t.Run("comment", func(t *testing.T) {
requireCap(t, caps.Comments)
// Most backends carry a comment on a bare port rule; CSF and APF can only