From cabafcf84a5c271e2aa2622a9a5940b3d36a7df5 Mon Sep 17 00:00:00 2001 From: banana-monkey-tree Date: Fri, 18 Sep 2026 22:34:15 +0200 Subject: [PATCH] nftables: fixed QueueBypass for system rules System firewall queue rules always included the bypass flag, even when FwOptions.QueueBypass was false. Pass the configured flag to the verdict builder, as we already do for the interception rules. Add a system-rule test that checks the installed queue flags with bypass disabled and enabled. --- daemon/firewall/nftables/exprs/nat_test.go | 10 ++-- daemon/firewall/nftables/exprs/verdict.go | 5 +- .../firewall/nftables/exprs/verdict_test.go | 6 +-- daemon/firewall/nftables/system.go | 2 +- daemon/firewall/nftables/system_test.go | 51 +++++++++++++++++++ 5 files changed, 62 insertions(+), 12 deletions(-) diff --git a/daemon/firewall/nftables/exprs/nat_test.go b/daemon/firewall/nftables/exprs/nat_test.go index ec8ce6742a..66cdb0cb42 100644 --- a/daemon/firewall/nftables/exprs/nat_test.go +++ b/daemon/firewall/nftables/exprs/nat_test.go @@ -173,7 +173,7 @@ func TestExprVerdictSNAT(t *testing.T) { for _, test := range tests { t.Run(test.Name, func(t *testing.T) { - verdExpr := exprs.NewExprVerdict(exprs.VERDICT_SNAT, test.Parms) + verdExpr := exprs.NewExprVerdict(exprs.VERDICT_SNAT, test.Parms, expr.QueueFlagBypass) if !test.ExpectedFail && verdExpr == nil { t.Errorf("error creating snat verdict") } else if test.ExpectedFail && verdExpr == nil { @@ -358,7 +358,7 @@ func TestExprVerdictDNAT(t *testing.T) { for _, test := range tests { t.Run(test.Name, func(t *testing.T) { - verdExpr := exprs.NewExprVerdict(exprs.VERDICT_DNAT, test.Parms) + verdExpr := exprs.NewExprVerdict(exprs.VERDICT_DNAT, test.Parms, expr.QueueFlagBypass) if !test.ExpectedFail && verdExpr == nil { t.Errorf("error creating verdict") } else if test.ExpectedFail && verdExpr == nil { @@ -442,7 +442,7 @@ func TestExprVerdictMasquerade(t *testing.T) { for _, test := range tests { t.Run(test.Name, func(t *testing.T) { - verdExpr := exprs.NewExprVerdict(exprs.VERDICT_MASQUERADE, test.Parms) + verdExpr := exprs.NewExprVerdict(exprs.VERDICT_MASQUERADE, test.Parms, expr.QueueFlagBypass) if !test.ExpectedFail && verdExpr == nil { t.Errorf("error creating verdict") } else if test.ExpectedFail && verdExpr == nil { @@ -517,7 +517,7 @@ func TestExprVerdictRedirect(t *testing.T) { for _, test := range tests { t.Run(test.Name, func(t *testing.T) { - verdExpr := exprs.NewExprVerdict(exprs.VERDICT_REDIRECT, test.Parms) + verdExpr := exprs.NewExprVerdict(exprs.VERDICT_REDIRECT, test.Parms, expr.QueueFlagBypass) if !test.ExpectedFail && verdExpr == nil { t.Errorf("error creating verdict") } else if test.ExpectedFail && verdExpr == nil { @@ -602,7 +602,7 @@ func TestExprVerdictTProxy(t *testing.T) { for _, test := range tests { t.Run(test.Name, func(t *testing.T) { - verdExpr := exprs.NewExprVerdict(exprs.VERDICT_TPROXY, test.Parms) + verdExpr := exprs.NewExprVerdict(exprs.VERDICT_TPROXY, test.Parms, expr.QueueFlagBypass) if !test.ExpectedFail && verdExpr == nil { t.Errorf("error creating verdict") } else if test.ExpectedFail && verdExpr == nil { diff --git a/daemon/firewall/nftables/exprs/verdict.go b/daemon/firewall/nftables/exprs/verdict.go index 54a02327a0..17ff711864 100644 --- a/daemon/firewall/nftables/exprs/verdict.go +++ b/daemon/firewall/nftables/exprs/verdict.go @@ -10,7 +10,7 @@ import ( ) // NewExprVerdict constructs a new verdict to apply on connections. -func NewExprVerdict(verdict, parms string) *[]expr.Any { +func NewExprVerdict(verdict, parms string, queueFlag expr.QueueFlag) *[]expr.Any { switch strings.ToLower(verdict) { case VERDICT_ACCEPT: return NewExprAccept() @@ -51,7 +51,6 @@ func NewExprVerdict(verdict, parms string) *[]expr.Any { log.Warning("invalid Queue expr parameters") return nil } - // TODO: allow to configure this flag if p[0] == NFT_QUEUE_NUM { queueNum, err = strconv.Atoi(p[len(p)-1]) if err != nil { @@ -63,7 +62,7 @@ func NewExprVerdict(verdict, parms string) *[]expr.Any { return &[]expr.Any{ &expr.Queue{ Num: uint16(queueNum), - Flag: expr.QueueFlagBypass, + Flag: queueFlag, }} case VERDICT_SNAT: diff --git a/daemon/firewall/nftables/exprs/verdict_test.go b/daemon/firewall/nftables/exprs/verdict_test.go index de34b3b642..0feedfca95 100644 --- a/daemon/firewall/nftables/exprs/verdict_test.go +++ b/daemon/firewall/nftables/exprs/verdict_test.go @@ -50,7 +50,7 @@ func TestExprVerdict(t *testing.T) { for _, test := range verdictTests { t.Run(test.name, func(t *testing.T) { - verdExpr := exprs.NewExprVerdict(test.verdict, test.parms) + verdExpr := exprs.NewExprVerdict(test.verdict, test.parms, expr.QueueFlagBypass) r, _ := nftest.AddTestRule(t, conn, verdExpr) if r == nil { t.Errorf("Error adding rule with verdict expression %s", test.verdict) @@ -262,7 +262,7 @@ func TestExprVerdictReject(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - verdExpr := exprs.NewExprVerdict(exprs.VERDICT_REJECT, test.parms) + verdExpr := exprs.NewExprVerdict(exprs.VERDICT_REJECT, test.parms, expr.QueueFlagBypass) r, _ := nftest.AddTestRule(t, conn, verdExpr) if r == nil { t.Errorf("Error adding rule with reject verdict %s", "") @@ -295,7 +295,7 @@ func TestExprVerdictQueue(t *testing.T) { defer nftest.CleanupSystemConn(t, newNS) nftest.Fw.Conn = conn - verdExpr := exprs.NewExprVerdict(exprs.VERDICT_QUEUE, "num 1") + verdExpr := exprs.NewExprVerdict(exprs.VERDICT_QUEUE, "num 1", expr.QueueFlagBypass) r, _ := nftest.AddTestRule(t, conn, verdExpr) if r == nil { t.Errorf("Error adding rule with Queue verdict") diff --git a/daemon/firewall/nftables/system.go b/daemon/firewall/nftables/system.go index 08c08a2e06..08c811ad38 100644 --- a/daemon/firewall/nftables/system.go +++ b/daemon/firewall/nftables/system.go @@ -166,7 +166,7 @@ func (n *Nft) AddSystemRule(rule *config.FwRule, chain *config.FwChain) (err4, e exprList = append(exprList, *exprsOfRule...) } if len(exprList) > 0 { - exprVerdict := exprs.NewExprVerdict(rule.Target, rule.TargetParameters) + exprVerdict := exprs.NewExprVerdict(rule.Target, rule.TargetParameters, n.getBypassFlag()) if exprVerdict == nil { return fmt.Errorf("%s invalid verdict %s %s", rule.UUID, rule.Target, rule.TargetParameters), nil } diff --git a/daemon/firewall/nftables/system_test.go b/daemon/firewall/nftables/system_test.go index 871faa2ce6..d000f4d884 100644 --- a/daemon/firewall/nftables/system_test.go +++ b/daemon/firewall/nftables/system_test.go @@ -3,8 +3,10 @@ package nftables_test import ( "testing" + nftb "github.com/evilsocket/opensnitch/daemon/firewall/nftables" "github.com/evilsocket/opensnitch/daemon/firewall/nftables/exprs" "github.com/evilsocket/opensnitch/daemon/firewall/nftables/nftest" + "github.com/google/nftables/expr" ) type sysChainsListT struct { @@ -166,3 +168,52 @@ func TestDeleteSystemRules(t *testing.T) { t.Run("test-delete-system-rules+chains", func(t *testing.T) { }) } + +// Unlike the other tests in this file, which use nftest.Fw + AddSystemRules, +// this one goes through the full Init() path: bypassQueue is unexported and is +// only set there, so it is the only way to exercise the configured flag end to +// end rather than asserting on the expression builder. The thread is locked to +// a fresh network namespace for the duration, so the rules it installs and +// removes cannot touch a firewall outside the test. +func TestSystemRuleQueueBypass(t *testing.T) { + nftest.SkipIfNotPrivileged(t) + + tests := []struct { + name string + bypass bool + expectedFlag expr.QueueFlag + }{ + {"bypass-disabled", false, 0}, + {"bypass-enabled", true, expr.QueueFlagBypass}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + conn, newNS := nftest.OpenSystemConn(t) + defer nftest.CleanupSystemConn(t, newNS) + + fw, err := nftb.Fw() + if err != nil { + t.Fatalf("Error creating firewall: %s", err) + } + fw.Init(0, configFile, "0s", test.bypass) + defer fw.Stop() + + rules, _ := getRulesList(t, conn, exprs.NFT_FAMILY_INET, exprs.TABLE_OPENSNITCH, exprs.CHAIN_MANGLE_FORWARD) + if len(rules) != 1 { + t.Fatalf("mangle_forward should contain 1 system rule, got %d", len(rules)) + } + for _, e := range rules[0].Exprs { + if queue, ok := e.(*expr.Queue); ok { + if queue.Num != 0 { + t.Errorf("invalid system rule queue number: got %d, expected 0", queue.Num) + } + if queue.Flag != test.expectedFlag { + t.Errorf("QueueBypass=%t: system rule queue flags = %#x, expected %#x", test.bypass, queue.Flag, test.expectedFlag) + } + return + } + } + t.Fatal("system rule has no queue expression") + }) + } +}