Skip to content

nftables: fixed QueueBypass for system rules #1672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
banana-monkey-tree wants to merge 1 commit into
base: master
Choose a base branch
from
+62 −12
Open

nftables: fixed QueueBypass for system rules #1672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions daemon/firewall/nftables/exprs/nat_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions daemon/firewall/nftables/exprs/verdict.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand All @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions daemon/firewall/nftables/exprs/verdict_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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", "")
Expand Down Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion daemon/firewall/nftables/system.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
51 changes: 51 additions & 0 deletions daemon/firewall/nftables/system_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
})
}
}