From 0f4d195bffd6b6398936a06bb820f0a23d6940ea Mon Sep 17 00:00:00 2001 From: Jayden Nyamiaka Date: Wed, 14 Aug 2024 13:43:42 -0700 Subject: [PATCH] Implement Loop Checking for Jump & Goto operations with minor rule restructure. Loop checking is done whenever a rule is registered to a chain. Includes restructuring how rules are added/registered to chains to make loop checking consistent and restrict adding operations to already registered rules. Change also performs minor code re-org, moving evaluation methods to the top. PiperOrigin-RevId: 663039449 --- pkg/tcpip/nftables/nftables.go | 448 ++++++++++++------- pkg/tcpip/nftables/nftables_test.go | 642 ++++++++++++++++++++++++++-- pkg/tcpip/nftables/nftinterp.go | 4 +- 3 files changed, 912 insertions(+), 182 deletions(-) diff --git a/pkg/tcpip/nftables/nftables.go b/pkg/tcpip/nftables/nftables.go index 07511438f..3d38dd6f9 100644 --- a/pkg/tcpip/nftables/nftables.go +++ b/pkg/tcpip/nftables/nftables.go @@ -187,7 +187,7 @@ func validateHook(hook Hook, family AddressFamily) error { return nil } - return fmt.Errorf("hook %s is not valid for address family %s", hook.String(), family.String()) + return fmt.Errorf("hook %v is not valid for address family %v", hook, family) } // NFTables represents the nftables state for all address families. @@ -415,16 +415,16 @@ func NewStandardPriority(name string, family AddressFamily, hook Hook) (Priority // Looks up standard priority name in the standard priority matrix. familyMatrix, exists := standardPriorityMatrix[family] if !exists { - return Priority{}, fmt.Errorf("standard priority names are not available for address family %s", family.String()) + return Priority{}, fmt.Errorf("standard priority names are not available for address family %v", family) } sp, exists := familyMatrix[name] if !exists { - return Priority{}, fmt.Errorf("standard priority name '%s' is not compatible with address family %s", name, family.String()) + return Priority{}, fmt.Errorf("standard priority name '%s' is not compatible with address family %v", name, family) } // Checks for hook compatibility. if !slices.Contains(sp.hooks, hook) { - return Priority{}, fmt.Errorf("standard priority %s is not compatible with hook %s", name, hook.String()) + return Priority{}, fmt.Errorf("standard priority %s is not compatible with hook %v", name, hook) } return Priority{value: sp.value, standardPriorityName: name}, nil @@ -530,10 +530,10 @@ func validateBaseChainInfo(info *BaseChainInfo, family AddressFamily) error { return fmt.Errorf("invalid base chain type: %d", int(info.BcType)) } if !slices.Contains(supportedAFsForBaseChainTypes[info.BcType], family) { - return fmt.Errorf("base chain type %s is not valid for address family %s", info.BcType.String(), family.String()) + return fmt.Errorf("base chain type %v is not valid for address family %v", info.BcType, family) } if !slices.Contains(supportedHooksForBaseChainTypes[info.BcType], info.Hook) { - return fmt.Errorf("base chain type %s is not valid for hook %s", info.BcType.String(), info.Hook.String()) + return fmt.Errorf("base chain type %v is not valid for hook %v", info.BcType, info.Hook) } // Priority assumed to be valid since it's a result of a constructor call. @@ -543,27 +543,14 @@ func validateBaseChainInfo(info *BaseChainInfo, family AddressFamily) error { // Rule represents a single rule in a chain and is represented as a list of // operations that are evaluated sequentially (on a packet). -// Note: Empty rules should be created directly (via &Rule{}) and the chain will -// be set by the AddRule function once the rule is added to a chain. +// Rules must be registered to a chain to be used and evaluated, and rules that +// have been registered to a chain cannot be modified. +// Note: Empty rules should be created directly (via &Rule{}). type Rule struct { chain *Chain ops []Operation } -// evaluate evaluates the rule on the given packet and register set, changing -// the register set and possibly the packet in place. -// The verdict in regs.Verdict() may be an nf table internal verdict or a -// netfilter terminal verdict. -func (r *Rule) evaluate(regs *RegisterSet, pkt *stack.PacketBuffer) error { - for _, op := range r.ops { - op.evaluate(regs, pkt) - if regs.Verdict().Code != VC(linux.NFT_CONTINUE) { - break - } - } - return nil -} - // Operation represents a single operation in a rule. type Operation interface { @@ -677,13 +664,7 @@ func (rd VerdictData) String() string { // Equal compares the verdict data to another RegisterData object. func (rd VerdictData) Equal(other RegisterData) bool { - if other == nil { - return false - } - if other.Type() != DataVerdict { - return false - } - return rd.data == other.(VerdictData).data + return other != nil && other.Type() == DataVerdict && rd.data == other.(VerdictData).data } // ValidateRegister ensures the register is compatible with VerdictData. @@ -869,6 +850,152 @@ func VerdictToString(v uint32) string { } } +// ----------------------------------------------------------------------------- +// ----------------------------------------------------------------------------- + +// +// Core Evaluation Functions +// + +// EvaluateHook evaluates a packet using the rules of the given hook for the +// given address family, returning a netfilter verdict and modifying the packet +// in place. +// Returns an error if address family or hook is invalid or they don't match. +// TODO(b/345684870): Consider removing error case if we never return an error. +func (nf *NFTables) EvaluateHook(family AddressFamily, hook Hook, pkt *stack.PacketBuffer) (Verdict, error) { + // Note: none of the other evaluate functions are public because they require + // jumping to different chains in the same table, so all chains, rules, and + // operations must be tied to a table. Thus, calling evaluate for standalone + // chains, rules, or operations can be misleading and dangerous. + + // Ensures address family is valid. + if err := validateAddressFamily(family); err != nil { + return Verdict{}, err + } + + // Ensures hook is valid. + if err := validateHook(hook, family); err != nil { + return Verdict{}, err + } + + // Immediately accept if there are no base chains for the specified hook. + if nf.filters[family] == nil || nf.filters[family].hfStacks[hook] == nil || + len(nf.filters[family].hfStacks[hook].baseChains) == 0 { + return Verdict{Code: VC(linux.NF_ACCEPT)}, nil + } + + regs := NewRegisterSet() + + // Evaluates packet through all base chains for given hook in priority order. + var bc *Chain + for _, bc = range nf.filters[family].hfStacks[hook].baseChains { + // Doesn't evaluate chain if it's table is flagged as dormant. + if _, dormant := bc.table.flagSet[TableFlagDormant]; dormant { + continue + } + + err := bc.evaluate(®s, pkt) + if err != nil { + return Verdict{}, err + } + + // Terminates immediately on netfilter terminal verdicts. + switch regs.Verdict().Code { + case VC(linux.NF_ACCEPT), VC(linux.NF_DROP), VC(linux.NF_STOLEN), VC(linux.NF_QUEUE): + return regs.Verdict(), nil + } + } + + // Returns policy verdict of the last base chain evaluated if no terminal + // verdict was issued. + switch regs.Verdict().Code { + case VC(linux.NFT_CONTINUE), VC(linux.NFT_RETURN): + if bc.GetBaseChainInfo().PolicyDrop { + return Verdict{Code: VC(linux.NF_DROP)}, nil + } + return Verdict{Code: VC(linux.NF_ACCEPT)}, nil + } + + panic(fmt.Sprintf("unexpected verdict from hook evaluation: %s", VerdictToString(regs.Verdict().Code))) +} + +// evaluateFromRule is a helper function for Chain.evaluate that evaluates the +// packet through the rules in the chain starting at the specified rule index. +func (c *Chain) evaluateFromRule(rIdx int, jumpDepth int, regs *RegisterSet, pkt *stack.PacketBuffer) error { + if jumpDepth >= nestedJumpLimit { + return fmt.Errorf("jump stack limit of %d exceeded", nestedJumpLimit) + } + + // Resets verdict to continue for the next rule. + regs.verdict.Code = VC(linux.NFT_CONTINUE) + + // Evaluates all rules in the chain (breaking on terminal verdicts). +evalLoop: + for ; rIdx < len(c.rules); rIdx++ { + rule := c.rules[rIdx] + if err := rule.evaluate(regs, pkt); err != nil { + return err + } + + // Continues evaluation at target chains for jump and goto verdicts. + jumped := false + switch regs.Verdict().Code { + case VC(linux.NFT_JUMP): + jumpDepth++ + jumped = true + fallthrough + case VC(linux.NFT_GOTO): + // Finds the chain named in the same table as the calling chain. + nextChain, exists := c.table.chains[regs.verdict.ChainName] + if !exists { + return fmt.Errorf("chain '%s' does not exist in table %s", regs.verdict.ChainName, c.table.GetName()) + } + if err := nextChain.evaluateFromRule(0, jumpDepth, regs, pkt); err != nil { + return err + } + // Ends evaluation for goto (and continues evaluation for jump). + if !jumped { + break evalLoop + } + jumpDepth-- + } + + // Only continues evaluation for Continue and Break verdicts. + switch regs.Verdict().Code { + case VC(linux.NFT_BREAK): + // Resets verdict for next rule (after breaking from a single operation). + regs.verdict.Code = VC(linux.NFT_CONTINUE) + case VC(linux.NFT_CONTINUE): + // Goes to next rule. + continue + default: + // Break evaluation for all the netfilter verdicts. + break evalLoop + } + } + return nil +} + +// evaluate for Chain evaluates the packet through the chain's rules and returns +// the verdict and modifies the packet in place. +func (c *Chain) evaluate(regs *RegisterSet, pkt *stack.PacketBuffer) error { + return c.evaluateFromRule(0, 0, regs, pkt) +} + +// evaluate evaluates the rule on the given packet and register set, changing +// the register set and possibly the packet in place. +// The verdict in regs.Verdict() may be an nf table internal verdict or a +// netfilter terminal verdict. +func (r *Rule) evaluate(regs *RegisterSet, pkt *stack.PacketBuffer) error { + for _, op := range r.ops { + op.evaluate(regs, pkt) + if regs.Verdict().Code != VC(linux.NFT_CONTINUE) { + break + } + } + return nil +} + // // Top-Level NFTables Functions // Note: Provides wrapper functions for the creation and deletion of tables, @@ -909,7 +1036,7 @@ func (nf *NFTables) GetTable(family AddressFamily, tableName string) (*Table, er // Checks if the table map for the address family has been initialized. if nf.filters[family] == nil || nf.filters[family].tables == nil { - return nil, fmt.Errorf("address family %s has no tables", family.String()) + return nil, fmt.Errorf("address family %v has no tables", family) } // Gets the corresponding table map for the address family. @@ -918,7 +1045,7 @@ func (nf *NFTables) GetTable(family AddressFamily, tableName string) (*Table, er // Checks if a table with the name exists. t, exists := tableMap[tableName] if !exists { - return nil, fmt.Errorf("table '%s' does not exists for address family %s", tableName, family.String()) + return nil, fmt.Errorf("table '%s' does not exists for address family %v", tableName, family) } return t, nil @@ -954,7 +1081,7 @@ func (nf *NFTables) AddTable(family AddressFamily, name string, comment string, // existing table (unless errorOnDuplicate is true). if existingTable, exists := tableMap[name]; exists { if errorOnDuplicate { - return nil, fmt.Errorf("table '%s' already exists in address family %s", name, family.String()) + return nil, fmt.Errorf("table '%s' already exists in address family %v", name, family) } return existingTable, nil } @@ -1056,66 +1183,9 @@ func (nf *NFTables) DeleteChain(family AddressFamily, tableName string, chainNam return t.DeleteChain(chainName), nil } -// EvaluateHook evaluates a packet using the rules of the given hook for the -// given address family, returning a netfilter verdict and modifying the packet -// in place. -// Returns an error if address family or hook is invalid or they don't match. -// TODO(b/345684870): Consider removing error case if we never return an error. -func (nf *NFTables) EvaluateHook(family AddressFamily, hook Hook, pkt *stack.PacketBuffer) (Verdict, error) { - // Note: none of the other evaluate functions are public because they require - // jumping to different chains in the same table, so all chains, rules, and - // operations must be tied to a table. Thus, calling evaluate for standalone - // chains, rules, or operations can be misleading and dangerous. - - // Ensures address family is valid. - if err := validateAddressFamily(family); err != nil { - return Verdict{}, err - } - - // Ensures hook is valid. - if err := validateHook(hook, family); err != nil { - return Verdict{}, err - } - - // Immediately accept if there are no base chains for the specified hook. - if nf.filters[family] == nil || nf.filters[family].hfStacks[hook] == nil || - len(nf.filters[family].hfStacks[hook].baseChains) == 0 { - return Verdict{Code: VC(linux.NF_ACCEPT)}, nil - } - - regs := NewRegisterSet() - - // Evaluates packet through all base chains for given hook in priority order. - var bc *Chain - for _, bc = range nf.filters[family].hfStacks[hook].baseChains { - // Doesn't evaluate chain if it's table is flagged as dormant. - if _, dormant := bc.table.flagSet[TableFlagDormant]; dormant { - continue - } - - err := bc.evaluate(®s, pkt) - if err != nil { - return Verdict{}, err - } - - // Terminates immediately on netfilter terminal verdicts. - switch regs.Verdict().Code { - case VC(linux.NF_ACCEPT), VC(linux.NF_DROP), VC(linux.NF_STOLEN), VC(linux.NF_QUEUE): - return regs.Verdict(), nil - } - } - - // Returns policy verdict of the last base chain evaluated if no terminal - // verdict was issued. - switch regs.Verdict().Code { - case VC(linux.NFT_CONTINUE), VC(linux.NFT_RETURN): - if bc.GetBaseChainInfo().PolicyDrop { - return Verdict{Code: VC(linux.NF_DROP)}, nil - } - return Verdict{Code: VC(linux.NF_ACCEPT)}, nil - } - - panic(fmt.Sprintf("unexpected verdict from hook evaluation: %d", regs.Verdict().Code)) +// TableCount returns the number of tables in the NFTables object. +func (nf *NFTables) TableCount() int { + return len(nf.filters) } // @@ -1214,7 +1284,7 @@ func (t *Table) DeleteChain(name string) bool { if c.baseChainInfo != nil { hfStack := t.afFilter.hfStacks[c.baseChainInfo.Hook] if err := hfStack.detachBaseChain(c.name); err != nil { - panic(fmt.Sprintf("failed to detach base chain %s from hook %s: %s", c.GetName(), c.baseChainInfo.Hook.String(), err)) + panic(fmt.Sprintf("failed to detach base chain %s from hook %v: %v", c.GetName(), c.baseChainInfo.Hook, err)) } if len(hfStack.baseChains) == 0 { delete(t.afFilter.hfStacks, c.baseChainInfo.Hook) @@ -1226,6 +1296,11 @@ func (t *Table) DeleteChain(name string) bool { return true } +// ChainCount returns the number of chains in the table. +func (t *Table) ChainCount() int { + return len(t.chains) +} + // // Chain Functions // @@ -1299,87 +1374,148 @@ func (c *Chain) SetComment(comment string) { c.comment = comment } -// AddRule adds a rule to the chain. -func (c *Chain) AddRule(rule *Rule) { - // Assigns the chain to the rule. - rule.chain = c - - c.rules = append(c.rules, rule) -} - -// evaluateFromRule is a helper function for Chain.evaluate that evaluates the -// packet through the rules in the chain starting at the specified rule index. -func (c *Chain) evaluateFromRule(rIdx int, jumpDepth int, regs *RegisterSet, pkt *stack.PacketBuffer) error { - if jumpDepth >= nestedJumpLimit { - return fmt.Errorf("jump stack limit of %d exceeded", nestedJumpLimit) +// RegisterRule assigns the chain to the rule and adds the rule to the chain's +// rule list at the given index. +// Valid indices are -1 (append) and [0, len]. Errors on invalid index. +// This also checks that the operations in the rule comply with the chain. +// Checks done: +// - All jump and goto operations have a valid target chain. +// - Loop checking for jump and goto operations. +// - TODO(b/345684870): Add more checks as more operations are supported. +func (c *Chain) RegisterRule(rule *Rule, index int) error { + if rule.chain != nil { + return fmt.Errorf("rule is already registered to a chain") } - // Resets verdict to continue for the next rule. - regs.verdict.Code = VC(linux.NFT_CONTINUE) + if index < -1 || index > c.RuleCount() { + return fmt.Errorf("invalid index %d for rule registration with %d rule(s)", index, c.RuleCount()) + } - // Evaluates all rules in the chain (breaking on terminal verdicts). -evalLoop: - for ; rIdx < len(c.rules); rIdx++ { - rule := c.rules[rIdx] - if err := rule.evaluate(regs, pkt); err != nil { + // Checks if there are loops from all jump and goto operations in the rule. + for _, op := range rule.ops { + isJumpOrGoto, targetChainName := isJumpOrGotoOperation(op) + if !isJumpOrGoto { + continue + } + nextChain, exists := c.table.chains[targetChainName] + if !exists { + return fmt.Errorf("chain '%s' does not exist in table %s", targetChainName, c.table.GetName()) + } + if err := nextChain.checkLoops(c); err != nil { return err } + } - // Continues evaluation at target chains for jump and goto verdicts. - jumped := false - switch regs.Verdict().Code { - case VC(linux.NFT_JUMP): - jumpDepth++ - jumped = true - fallthrough - case VC(linux.NFT_GOTO): - // Finds the chain named in the same table as the calling chain. - nextChain, exists := c.table.chains[regs.verdict.ChainName] - if !exists { - return fmt.Errorf("chain '%s' does not exist in table %s", regs.verdict.ChainName, c.table.GetName()) - } - if err := nextChain.evaluateFromRule(0, jumpDepth, regs, pkt); err != nil { - return err - } - // Ends evaluation for goto (and continues evaluation for jump). - if !jumped { - break evalLoop - } - jumpDepth-- - } + // Assigns chain to rule and adds rule to chain's rule list at given index. + rule.chain = c - // Only continues evaluation for Continue and Break verdicts. - switch regs.Verdict().Code { - case VC(linux.NFT_BREAK): - // Resets verdict for next rule (after breaking from a single operation). - regs.verdict.Code = VC(linux.NFT_CONTINUE) - case VC(linux.NFT_CONTINUE): - // Goes to next rule. - continue - default: - // Break evaluation for all the netfilter verdicts. - break evalLoop - } + // Adds the rule to the chain's rule list at the correct index. + if index == -1 || index == c.RuleCount() { + c.rules = append(c.rules, rule) + } else { + c.rules = slices.Insert(c.rules, index, rule) } return nil } -// evaluate for Chain evaluates the packet through the chain's rules and returns -// the verdict and modifies the packet in place. -func (c *Chain) evaluate(regs *RegisterSet, pkt *stack.PacketBuffer) error { - return c.evaluateFromRule(0, 0, regs, pkt) +// UnregisterRule removes the rule at the given index from the chain's rule list +// and unassigns the chain from the rule then returns the unregistered rule. +// Valid indices are -1 (pop) and [0, len-1]. Errors on invalid index. +func (c *Chain) UnregisterRule(index int) (*Rule, error) { + rule, err := c.GetRule(index) + if err != nil { + return nil, fmt.Errorf("invalid index %d for rule registration with %d rule(s)", index, c.RuleCount()) + } + if index == -1 { + index = c.RuleCount() - 1 + } + c.rules = append(c.rules[:index], c.rules[index+1:]...) + rule.chain = nil + return rule, nil +} + +// GetRule returns the rule at the given index in the chain's rule list. +// Valid indices are -1 (last) and [0, len-1]. Errors on invalid index. +func (c *Chain) GetRule(index int) (*Rule, error) { + if index < -1 || index > c.RuleCount()-1 || (index == -1 && c.RuleCount() == 0) { + return nil, fmt.Errorf("invalid index %d for rule retrieval with %d rule(s)", index, c.RuleCount()) + } + if index == -1 { + return c.rules[c.RuleCount()-1], nil + } + return c.rules[index], nil +} + +// RuleCount returns the number of rules in the chain. +func (c *Chain) RuleCount() int { + return len(c.rules) +} + +// +// Loop Checking Helper Functions +// + +// isJumpOrGoto returns whether the operation is an immediate operation that +// sets the verdict register to a jump or goto verdict and returns the name of +// the target chain to jump or goto if so. +func isJumpOrGotoOperation(op Operation) (bool, string) { + imm, ok := op.(*Immediate) + if !ok { + return false, "" + } + if imm.data.Type() != DataVerdict { + return false, "" + } + verdict := imm.data.(VerdictData).data + if verdict.Code != VC(linux.NFT_JUMP) && verdict.Code != VC(linux.NFT_GOTO) { + return false, "" + } + return true, verdict.ChainName +} + +// checkLoops detects if there are any loops via jumps and gotos between chains +// by tracing all immediate operations starting from the destination chain +// of a jump or goto operation and checking that no jump or goto operations lead +// back to the original source chain. +// Note: this loop checking is done whenever a rule is registered to a chain. +func (c *Chain) checkLoops(source *Chain) error { + if c == source { + return fmt.Errorf("loop detected between calling chain %s and source chain %s", c.name, source.name) + } + for _, rule := range c.rules { + for _, op := range rule.ops { + isJumpOrGoto, targetChainName := isJumpOrGotoOperation(op) + if !isJumpOrGoto { + continue + } + nextChain, exists := c.table.chains[targetChainName] + if !exists { + return fmt.Errorf("chain '%s' does not exist in table %s", targetChainName, c.table.GetName()) + } + if err := nextChain.checkLoops(source); err != nil { + return err + } + } + } + return nil } // // Rule Functions // -// AddOperation adds an operation to the rule. -func (r *Rule) AddOperation(op Operation) { +// AddOperation adds an operation to the rule. Adding operations is only allowed +// before the rule is registered to a chain. Returns an error if the operation +// is nil or if the rule is already registered to a chain. +func (r *Rule) AddOperation(op Operation) error { if op == nil { - panic("operation is nil") + return fmt.Errorf("operation is nil") + } + if r.chain != nil { + return fmt.Errorf("cannot add operation to a rule that is already registered to a chain") } r.ops = append(r.ops, op) + return nil } // @@ -1416,10 +1552,10 @@ func (hfStack *hookFunctionStack) detachBaseChain(name string) error { return chain.name == name }) if len(hfStack.baseChains) == prevLen { - return fmt.Errorf("base chain '%s' does not exist for hook %s", name, hfStack.hook.String()) + return fmt.Errorf("base chain '%s' does not exist for hook %v", name, hfStack.hook) } if len(hfStack.baseChains) < prevLen-1 { - panic(fmt.Errorf("multiple base chains with name '%s' exist for hook %s", name, hfStack.hook.String())) + panic(fmt.Errorf("multiple base chains with name '%s' exist for hook %v", name, hfStack.hook)) } return nil } diff --git a/pkg/tcpip/nftables/nftables_test.go b/pkg/tcpip/nftables/nftables_test.go index 8c139cffc..918181ba2 100644 --- a/pkg/tcpip/nftables/nftables_test.go +++ b/pkg/tcpip/nftables/nftables_test.go @@ -15,6 +15,7 @@ package nftables import ( + "fmt" "reflect" "testing" @@ -23,7 +24,26 @@ import ( "gvisor.dev/gvisor/pkg/tcpip/stack" ) -const arbitraryTargetChain = "target_chain" +const ( + arbitraryTargetChain string = "target_chain" + arbitraryHook Hook = Prerouting + arbitraryFamily AddressFamily = Inet +) + +var ( + arbitraryPriority Priority = func() Priority { + priority, err := NewStandardPriority("filter", arbitraryFamily, arbitraryHook) + if err != nil { + panic(fmt.Sprintf("unexpected error for NewStandardPriority: %v", err)) + } + return priority + }() + arbitraryInfoPolicyAccept *BaseChainInfo = &BaseChainInfo{ + BcType: BaseChainTypeFilter, + Hook: arbitraryHook, + Priority: arbitraryPriority, + } +) // makeTestingPacket creates an arbitrary packet for testing. func makeTestingPacket() *stack.PacketBuffer { @@ -40,11 +60,11 @@ func TestUnsupportedAddressFamily(t *testing.T) { for _, unsupportedFamily := range []AddressFamily{AddressFamily(NumAFs), AddressFamily(-1)} { // Note: the Prerouting hook is arbitrary (any hook would work). pkt := makeTestingPacket() - v, err := nf.EvaluateHook(unsupportedFamily, Prerouting, pkt) + v, err := nf.EvaluateHook(unsupportedFamily, arbitraryHook, pkt) if err == nil { - t.Fatalf("expecting error for EvaluateHook with unsupported address family %d; got %s verdict, %s packet, and error %v", + t.Fatalf("expecting error for EvaluateHook with unsupported address family %d; got %v verdict, %s packet, and error %v", int(unsupportedFamily), - v.String(), packetResultString(makeTestingPacket(), pkt), err) + v, packetResultString(makeTestingPacket(), pkt), err) } } } @@ -70,15 +90,15 @@ func TestAcceptAllForSupportedHooks(t *testing.T) { if supported { if err != nil || v.Code != VC(linux.NF_ACCEPT) { - t.Fatalf("expecting accept verdict for EvaluateHook with supported hook %s for family %s; got %s verdict, %s packet, and error %v", - hook.String(), family.String(), - v.String(), packetResultString(makeTestingPacket(), pkt), err) + t.Fatalf("expecting accept verdict for EvaluateHook with supported hook %v for family %v; got %v verdict, %s packet, and error %v", + hook, family, + v, packetResultString(makeTestingPacket(), pkt), err) } } else { if err == nil { - t.Fatalf("expecting error for EvaluateHook with unsupported hook %s for family %s; got %s verdict, %s packet, and error %v", - hook.String(), family.String(), - v.String(), packetResultString(makeTestingPacket(), pkt), err) + t.Fatalf("expecting error for EvaluateHook with unsupported hook %v for family %v; got %v verdict, %s packet, and error %v", + hook, family, + v, packetResultString(makeTestingPacket(), pkt), err) } } } @@ -219,7 +239,7 @@ func TestEvaluateImmediate(t *testing.T) { // Sets up an NFTables object with a base chain (for 2 rules) and another // target chain (for 1 rule). nf := NewNFTables() - tab, err := nf.AddTable(IP, "test", "test table", false) + tab, err := nf.AddTable(arbitraryFamily, "test", "test table", false) if err != nil { t.Fatalf("unexpected error for AddTable: %v", err) } @@ -227,15 +247,7 @@ func TestEvaluateImmediate(t *testing.T) { if err != nil { t.Fatalf("unexpected error for AddChain: %v", err) } - priority, err := NewStandardPriority("filter", IP, Prerouting) - if err != nil { - t.Fatalf("unexpected error for NewStandardPriority: %v", err) - } - bc.SetBaseChainInfo(&BaseChainInfo{ - BcType: BaseChainTypeFilter, - Hook: Prerouting, - Priority: priority, - }) + bc.SetBaseChainInfo(arbitraryInfoPolicyAccept) tc, err := tab.AddChain(arbitraryTargetChain, nil, "test chain", false) if err != nil { t.Fatalf("unexpected error for AddChain: %v", err) @@ -244,27 +256,607 @@ func TestEvaluateImmediate(t *testing.T) { // Adds testing rules and operations. if test.baseOp1 != nil { rule1 := &Rule{} - bc.AddRule(rule1) rule1.AddOperation(test.baseOp1) + if err := bc.RegisterRule(rule1, -1); err != nil { + t.Fatalf("unexpected error for RegisterRule for the first operation: %v", err) + } } if test.baseOp2 != nil { rule2 := &Rule{} - bc.AddRule(rule2) rule2.AddOperation(test.baseOp2) + if err := bc.RegisterRule(rule2, -1); err != nil { + t.Fatalf("unexpected error for RegisterRule for the second operation: %v", err) + } } if test.targetOp != nil { ruleTarget := &Rule{} - tc.AddRule(ruleTarget) ruleTarget.AddOperation(test.targetOp) + if err := tc.RegisterRule(ruleTarget, -1); err != nil { + t.Fatalf("unexpected error for RegisterRule for the target operation: %v", err) + } } + // Runs evaluation and checks verdict. pkt := makeTestingPacket() - v, err := nf.EvaluateHook(IP, Prerouting, pkt) + v, err := nf.EvaluateHook(arbitraryFamily, arbitraryHook, pkt) + if err != nil { t.Fatalf("unexpected error for EvaluateHook: %v", err) } if v.Code != test.verdict.Code { - t.Fatalf("expected verdict %s, got %s", test.verdict.String(), v.String()) + t.Fatalf("expected verdict %v, got %v", test.verdict, v) + } + }) + } +} + +// TestLoopCheckOnRegisterAndUnregister tests the loop checking and accompanying +// logic on registering and unregistering rules. +func TestLoopCheckOnRegisterAndUnregister(t *testing.T) { + for _, test := range []struct { + tname string + chains map[string]*Chain + verdict Verdict + shouldErr bool + }{ + { + tname: "jump to non-existent chain", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "non_existent_chain"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + tname: "goto to non-existent chain", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "non_existent_chain"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + tname: "jump to itself", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "base_chain"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + tname: "goto to itself", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "base_chain"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + tname: "simple 2-chain loop", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"}))}, + }}, + }, + "aux_chain": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "base_chain"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + tname: "2-chain loop with entry point outside loop", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"}))}, + }}, + }, + "aux_chain": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "aux_chain2"}))}, + }}, + }, + "aux_chain2": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "aux_chain"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + tname: "simple 3-chain loop", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"}))}, + }}, + }, + "aux_chain": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain2"}))}, + }}, + }, + "aux_chain2": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "base_chain"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + tname: "3-chain loop with entry point 2 points outside loop", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"}))}, + }}, + }, + "aux_chain": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "aux_chain2"}))}, + }}, + }, + "aux_chain2": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain3"}))}, + }}, + }, + "aux_chain3": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "aux_chain4"}))}, + }}, + }, + "aux_chain4": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain2"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + tname: "simple 4-chain loop", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"}))}, + }}, + }, + "aux_chain": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "aux_chain2"}))}, + }}, + }, + "aux_chain2": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain3"}))}, + }}, + }, + "aux_chain3": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "base_chain"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + tname: "simple 5-chain loop", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"}))}, + }}, + }, + "aux_chain": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "aux_chain2"}))}, + }}, + }, + "aux_chain2": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain3"}))}, + }}, + }, + "aux_chain3": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "base_chain"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + // 0 + // / \ + // v v + // 1 <- 2 <-> 3 + tname: "complex 2-3 loop", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{&Rule{ + ops: []Operation{ + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"})), + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain2"})), + }, + }}, + }, + "aux_chain": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NF_DROP)}))}, + }}, + }, + "aux_chain2": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{ + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"})), + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain3"})), + }, + }}, + }, + "aux_chain3": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "aux_chain2"}))}, + }}, + }, + }, + shouldErr: true, + }, + { + tname: "simple loop amongst other rules and operations", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{ + &Rule{ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_1, NewBytesData([]byte{0, 1, 2, 3}))}}, + &Rule{ops: []Operation{mustCreateImmediate(t, linux.NFT_REG32_14, NewBytesData([]byte{0, 1, 2, 3}))}}, + &Rule{ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"}))}}, + }, + }, + "aux_chain": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{ + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NF_DROP)})), + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "aux_chain2"})), + }, + }}, + }, + "aux_chain2": &Chain{ + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain3"}))}, + }}, + }, + "aux_chain3": &Chain{ + rules: []*Rule{ + &Rule{ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_1, NewBytesData([]byte{0, 1, 2, 3}))}}, + &Rule{ops: []Operation{mustCreateImmediate(t, linux.NFT_REG32_14, NewBytesData([]byte{0, 1, 2, 3}))}}, + &Rule{ops: []Operation{ + mustCreateImmediate(t, linux.NFT_REG_4, NewBytesData([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15})), + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_GOTO), ChainName: "aux_chain"})), + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NF_DROP)})), + }}, + }, + }, + }, + shouldErr: true, + }, + { + tname: "base chain jump to 3 other chains", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{ + &Rule{ + ops: []Operation{ + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"})), + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain2"})), + }, + }, + &Rule{ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain3"}))}}, + }, + }, + "aux_chain": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_2, NewBytesData([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}))}, + }}, + }, + "aux_chain2": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_3, NewBytesData([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}))}, + }}, + }, + "aux_chain3": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_4, NewBytesData([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}))}, + }}, + }, + }, + verdict: Verdict{Code: VC(linux.NF_ACCEPT)}, // from base chain policy + }, + { + tname: "base chain jump to 3 other chains with last chain dropping", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{ + &Rule{ + ops: []Operation{ + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"})), + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain2"})), + }, + }, + &Rule{ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain3"}))}}, + }, + }, + "aux_chain": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_2, NewBytesData([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}))}, + }}, + }, + "aux_chain2": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_3, NewBytesData([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}))}, + }}, + }, + "aux_chain3": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NF_DROP)}))}, + }}, + }, + }, + verdict: Verdict{Code: VC(linux.NF_DROP)}, // from last chain + }, + { + tname: "base chain jump to 3 other chains with last rule in base chain dropping", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{ + &Rule{ + ops: []Operation{ + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"})), + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain2"})), + }, + }, + &Rule{ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain3"}))}}, + &Rule{ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NF_DROP)}))}}, + }, + }, + "aux_chain": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_2, NewBytesData([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}))}, + }}, + }, + "aux_chain2": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_3, NewBytesData([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}))}, + }}, + }, + "aux_chain3": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{ + ops: []Operation{mustCreateImmediate(t, linux.NFT_REG_4, NewBytesData([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}))}, + }}, + }, + }, + verdict: Verdict{Code: VC(linux.NF_DROP)}, // from last rule in base chain + }, + { + tname: "jump to the same chain", + chains: map[string]*Chain{ + "base_chain": &Chain{ + baseChainInfo: arbitraryInfoPolicyAccept, + rules: []*Rule{ + &Rule{ + ops: []Operation{ + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"})), + mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NFT_JUMP), ChainName: "aux_chain"})), + }, + }, + }, + }, + "aux_chain": &Chain{ + comment: "strictly target", + rules: []*Rule{&Rule{}}, + }, + }, + verdict: Verdict{Code: VC(linux.NF_ACCEPT)}, // from base chain policy + }, + } { + t.Run(test.tname, func(t *testing.T) { + // Sets up an NFTables object based on test struct. + nf := NewNFTables() + tab, err := nf.AddTable(arbitraryFamily, "test", "test table", false) + if err != nil { + t.Fatalf("unexpected error for AddTable: %v", err) + } + // Creates all chains in the test struct first. This is necessary so the + // loop checking sees the target chains exist (otherwise it would error). + for chainName, chainInit := range test.chains { + tab.AddChain(chainName, chainInit.GetBaseChainInfo(), chainInit.GetComment(), false) + } + if len(test.chains) != tab.ChainCount() { + t.Fatalf("not all chains added to table") + } + // Registers all rules to all chains in the test struct. + for chainName, chainInit := range test.chains { + chain, err := nf.GetChain(tab.GetAddressFamily(), tab.GetName(), chainName) + if err != nil { + t.Fatalf("unexpected error for GetChain: %v", err) + } + for _, rule := range chainInit.rules { + // Note: this is where the loop checking is triggered. + if err := chain.RegisterRule(rule, -1); err != nil { + if !test.shouldErr { + t.Fatalf("unexpected error for RegisterRule: %v", err) + } + return + } + // Checks that the chain was assigned to the rule. + if rule.chain == nil { + t.Fatalf("chain is not assigned to rule after RegisterRule") + } + } + if chainInit.RuleCount() != chain.RuleCount() { + t.Fatalf("not all rules added to chain") + } + } + + // Runs evaluation and checks verdict. + pkt := makeTestingPacket() + v, err := nf.EvaluateHook(arbitraryFamily, arbitraryHook, pkt) + if err != nil { + if test.verdict.ChainName != "error" { + t.Fatalf("unexpected error for EvaluateHook: %v", err) + } + } + if v.Code != test.verdict.Code { + t.Fatalf("expected verdict %v, got %v", test.verdict, v) + } + + // Unregisters all rules from all chains and checks that the chain is + // unassigned from the rule. + for chainName, chainInit := range test.chains { + chain, err := nf.GetChain(tab.GetAddressFamily(), tab.GetName(), chainName) + if err != nil { + t.Fatalf("unexpected error for GetChain: %v", err) + } + for rIdx := chainInit.RuleCount() - 1; rIdx >= 0; rIdx-- { + rule, err := chain.UnregisterRule(rIdx) + if err != nil { + t.Fatalf("unexpected error for UnregisterRule: %v", err) + } + if rule != chainInit.rules[rIdx] { + t.Fatalf("rule returned by UnregisterRule does not match previously registered rule") + } + if rule.chain != nil { + t.Fatalf("chain is not unassigned from rule after UnregisterRule") + } + } + if chain.RuleCount() != 0 { + t.Fatalf("not all rules removed from chain") + } + } + }) + } +} + +// TestMaxNestedJumps tests the limit on nested jumps (no limit for gotos). +func TestMaxNestedJumps(t *testing.T) { + for _, test := range []struct { + tname string + useJumpOp bool + numberOfJumps int + verdict Verdict // ChainName is set to "error" if an error is expected + }{ + { + tname: "nested jump limit reached with jumps", + useJumpOp: true, + numberOfJumps: nestedJumpLimit, + verdict: Verdict{Code: VC(linux.NF_DROP)}, + }, + { + tname: "nested jump limit reached with gotos", + useJumpOp: false, + numberOfJumps: nestedJumpLimit, + verdict: Verdict{Code: VC(linux.NF_DROP)}, + }, + { + tname: "nested jump limit exceeded with jumps", + useJumpOp: true, + numberOfJumps: nestedJumpLimit + 1, + verdict: Verdict{ChainName: "error"}, + }, + { + tname: "nested jump limit exceeded with gotos", + useJumpOp: false, + numberOfJumps: nestedJumpLimit + 1, + verdict: Verdict{Code: VC(linux.NF_DROP)}, // limit only for jumps + }, + } { + t.Run(test.tname, func(t *testing.T) { + // Sets up chains of nested jumps or gotos. + nf := NewNFTables() + tab, err := nf.AddTable(arbitraryFamily, "test", "test table", false) + if err != nil { + t.Fatalf("unexpected error for AddTable: %v", err) + } + for i := test.numberOfJumps - 1; i >= 0; i-- { + name := fmt.Sprintf("chain %d", i) + c, err := tab.AddChain(name, nil, "test chain", false) + if i == 0 { + c.SetBaseChainInfo(arbitraryInfoPolicyAccept) + } + if err != nil { + t.Fatalf("unexpected error for AddChain: %v", err) + } + r := &Rule{} + if i == test.numberOfJumps-1 { + err = r.AddOperation(mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: VC(linux.NF_DROP)}))) + } else { + targetName := fmt.Sprintf("chain %d", i+1) + code := VC(linux.NFT_JUMP) + if !test.useJumpOp { + code = VC(linux.NFT_GOTO) + } + err = r.AddOperation(mustCreateImmediate(t, linux.NFT_REG_VERDICT, NewVerdictData(Verdict{Code: code, ChainName: targetName}))) + } + if err != nil { + t.Fatalf("unexpected error for AddOperation: %v", err) + } + if err := c.RegisterRule(r, -1); err != nil { + t.Fatalf("unexpected error for RegisterRule: %v", err) + } + } + + // Runs evaluation and checks verdict. + pkt := makeTestingPacket() + v, err := nf.EvaluateHook(arbitraryFamily, arbitraryHook, pkt) + if err != nil { + if test.verdict.ChainName != "error" { + t.Fatalf("unexpected error for EvaluateHook: %v", err) + } + } + if v.Code != test.verdict.Code { + t.Fatalf("expected verdict %v, got %v", test.verdict, v) } }) } diff --git a/pkg/tcpip/nftables/nftinterp.go b/pkg/tcpip/nftables/nftinterp.go index b222c9524..caa9d6ec3 100644 --- a/pkg/tcpip/nftables/nftinterp.go +++ b/pkg/tcpip/nftables/nftinterp.go @@ -116,7 +116,9 @@ func InterpretRule(ruleString string) (*Rule, error) { if err != nil { return nil, err } - r.AddOperation(op) + if err := r.AddOperation(op); err != nil { + return nil, err + } } return r, nil