Wrap firewall errors in a common struct

daemon/firewall/common/common.go

@@ -1,6 +1,7 @@
 package common
 
 import (
+	"fmt"
 	"sync"
 	"time"
 
@@ -38,8 +39,38 @@ type (
 		FwEnabled          bool
 		sync.RWMutex
 	}
+	// FirewallError is a type that holds both IPv4 and IPv6 errors.
+	FirewallError struct {
+		Err4 error
+		Err6 error
+	}
 )
 
+// Error formats the errors for both IPv4 and IPv6 errors.
+func (e *FirewallError) Error() string {
+	return fmt.Sprintf("IPv4 error: %v, IPv6 error: %v", e.Err4, e.Err6)
+}
+
+// AsError returns combined standard error for both IPv4 and IPv6
+// Only the non-nil errors are formatted into the error
+func (e *FirewallError) AsError() error {
+	switch {
+	case e.Err4 == nil && e.Err6 != nil:
+		return e.Err6
+	case e.Err4 != nil && e.Err6 == nil:
+		return e.Err4
+	case e == &FirewallError{} || e == nil:
+		return nil
+	default:
+		return fmt.Errorf("%v", e.Error())
+	}
+}
+
+// HasError simplifies error handling of the FirewallError type.
+func (e *FirewallError) HasError() bool {
+	return e.Err4 != nil || e.Err6 != nil
+}
+
 // ErrorsChan returns the channel where the errors are sent to.
 func (c *Common) ErrorsChan() <-chan string {
 	return c.ErrChan
@@ -91,7 +122,6 @@ func (c *Common) SetQueueNum(qNum *int) {
 	if qNum != nil {
 		c.QueueNum = uint16(*qNum)
 	}
-
 }
 
 // IsRunning returns if the firewall is running or not.

daemon/firewall/iptables/iptables.go

@@ -140,10 +140,10 @@ func IsAvailable() error {
 
 // EnableInterception adds fw rules to intercept connections.
 func (ipt *Iptables) EnableInterception() {
-	if err4, err6 := ipt.QueueConnections(common.EnableRule, true); err4 != nil || err6 != nil {
-		log.Fatal("Error while running conntrack firewall rule: %s %s", err4, err6)
-	} else if err4, err6 = ipt.QueueDNSResponses(common.EnableRule, true); err4 != nil || err6 != nil {
-		log.Error("Error while running DNS firewall rule: %s %s", err4, err6)
+	if err := ipt.QueueConnections(common.EnableRule, true); err != nil {
+		log.Fatal("Error while running conntrack firewall rule: %v", err.AsError())
+	} else if err = ipt.QueueDNSResponses(common.EnableRule, true); err != nil {
+		log.Error("Error while running DNS firewall rule: %v", err.AsError())
 	}
 	// start monitoring firewall rules to intercept network traffic
 	ipt.NewRulesChecker(ipt.AreRulesLoaded, ipt.reloadRulesCallback)

daemon/firewall/iptables/rules.go

@@ -4,12 +4,14 @@ import (
 	"fmt"
 
 	"github.com/evilsocket/opensnitch/daemon/core"
+	"github.com/evilsocket/opensnitch/daemon/firewall/common"
 	"github.com/evilsocket/opensnitch/daemon/log"
 	"github.com/vishvananda/netlink"
 )
 
 // RunRule inserts or deletes a firewall rule.
-func (ipt *Iptables) RunRule(action Action, enable bool, logError bool, rule []string) (err4, err6 error) {
+func (ipt *Iptables) RunRule(action Action, enable bool, logError bool, rule []string) (err *common.FirewallError) {
+	err = &common.FirewallError{}
 	if enable == false {
 		action = "-D"
 	}
@@ -19,30 +21,30 @@ func (ipt *Iptables) RunRule(action Action, enable bool, logError bool, rule []s
 	ipt.Lock()
 	defer ipt.Unlock()
 
-	if _, err4 = core.Exec(ipt.bin, rule); err4 != nil {
+	if _, err.Err4 = core.Exec(ipt.bin, rule); err != nil {
 		if logError {
-			log.Error("Error while running firewall rule, ipv4 err: %s", err4)
+			log.Error("Error while running firewall rule, ipv4 err: %s", err.Err4)
 			log.Error("rule: %s", rule)
 		}
 	}
 
 	// On some systems IPv6 is disabled
 	if core.IPv6Enabled {
-		if _, err6 = core.Exec(ipt.bin6, rule); err6 != nil {
+		if _, err.Err6 = core.Exec(ipt.bin6, rule); err.Err6 != nil {
 			if logError {
-				log.Error("Error while running firewall rule, ipv6 err: %s", err6)
+				log.Error("Error while running firewall rule, ipv6 err: %s", err.Err6)
 				log.Error("rule: %s", rule)
 			}
 		}
 	}
 
-	return
+	return err
 }
 
 // QueueDNSResponses redirects DNS responses to us, in order to keep a cache
 // of resolved domains.
 // INPUT --protocol udp --sport 53 -j NFQUEUE --queue-num 0 --queue-bypass
-func (ipt *Iptables) QueueDNSResponses(enable bool, logError bool) (err4, err6 error) {
+func (ipt *Iptables) QueueDNSResponses(enable bool, logError bool) *common.FirewallError {
 	return ipt.RunRule(INSERT, enable, logError, []string{
 		"INPUT",
 		"--protocol", "udp",
@@ -56,8 +58,8 @@ func (ipt *Iptables) QueueDNSResponses(enable bool, logError bool) (err4, err6 e
 // QueueConnections inserts the firewall rule which redirects connections to us.
 // Connections are queued until the user denies/accept them, or reaches a timeout.
 // OUTPUT -t mangle -m conntrack --ctstate NEW,RELATED -j NFQUEUE --queue-num 0 --queue-bypass
-func (ipt *Iptables) QueueConnections(enable bool, logError bool) (error, error) {
-	err4, err6 := ipt.RunRule(ADD, enable, logError, []string{
+func (ipt *Iptables) QueueConnections(enable bool, logError bool) *common.FirewallError {
+	err := ipt.RunRule(ADD, enable, logError, []string{
 		"OUTPUT",
 		"-t", "mangle",
 		"-m", "conntrack",
@@ -73,5 +75,5 @@ func (ipt *Iptables) QueueConnections(enable bool, logError bool) (error, error)
 			log.Error("error in ConntrackTableFlush %s", err)
 		}
 	}
-	return err4, err6
+	return err
 }

daemon/firewall/iptables/system.go

@@ -28,7 +28,7 @@ func (ipt *Iptables) CreateSystemRule(rule *config.FwRule, table, chain, hook st
 	ipt.RunRule(NEWCHAIN, common.EnableRule, logErrors, []string{chainName, "-t", table})
 
 	// Insert the rule at the top of the chain
-	if err4, err6 := ipt.RunRule(INSERT, common.EnableRule, logErrors, []string{hook, "-t", table, "-j", chainName}); err4 == nil && err6 == nil {
+	if err := ipt.RunRule(INSERT, common.EnableRule, logErrors, []string{hook, "-t", table, "-j", chainName}); err == nil {
 		ipt.chains.Rules[table+"-"+chainName] = &SystemRule{
 			Table: table,
 			Chain: chain,
@@ -102,7 +102,7 @@ func (ipt *Iptables) DeleteSystemRules(force, backupExistingChains, logErrors bo
 }
 
 // DeleteSystemRule deletes a new rule.
-func (ipt *Iptables) DeleteSystemRule(action Action, rule *config.FwRule, table, chain string, enable bool) (err4, err6 error) {
+func (ipt *Iptables) DeleteSystemRule(action Action, rule *config.FwRule, table, chain string, enable bool) *common.FirewallError {
 	chainName := SystemRulePrefix + "-" + chain
 	if table == "" {
 		table = "filter"
@@ -120,9 +120,9 @@ func (ipt *Iptables) DeleteSystemRule(action Action, rule *config.FwRule, table,
 }
 
 // AddSystemRule inserts a new rule.
-func (ipt *Iptables) AddSystemRule(action Action, rule *config.FwRule, table, chain string, enable bool) (err4, err6 error) {
+func (ipt *Iptables) AddSystemRule(action Action, rule *config.FwRule, table, chain string, enable bool) *common.FirewallError {
 	if rule == nil {
-		return nil, nil
+		return nil
 	}
 	ipt.RLock()
 	defer ipt.RUnlock()

daemon/firewall/nftables/monitor_test.go

@@ -22,10 +22,10 @@ func addInterceptionRules(nft *nftb.Nft, t *testing.T) {
 		return
 	}
 
-	if err, _ := nft.QueueDNSResponses(common.EnableRule, common.EnableRule); err != nil {
+	if err := nft.QueueDNSResponses(common.EnableRule, common.EnableRule); err != nil {
 		t.Errorf("Error while running DNS nftables rule: %s", err)
 	}
-	if err, _ := nft.QueueConnections(common.EnableRule, common.EnableRule); err != nil {
+	if err := nft.QueueConnections(common.EnableRule, common.EnableRule); err != nil {
 		t.Errorf("Error while running conntrack nftables rule: %s", err)
 	}
 }

daemon/firewall/nftables/nftables.go

@@ -123,11 +123,11 @@ func (n *Nft) EnableInterception() {
 		return
 	}
 
-	if err, _ := n.QueueDNSResponses(common.EnableRule, common.EnableRule); err != nil {
-		log.Error("Error while running DNS nftables rule: %s", err)
+	if err := n.QueueDNSResponses(common.EnableRule, common.EnableRule); err != nil {
+		log.Error("Error while running DNS nftables rule: %v", err.AsError())
 	}
-	if err, _ := n.QueueConnections(common.EnableRule, common.EnableRule); err != nil {
-		log.Error("Error while running conntrack nftables rule: %s", err)
+	if err := n.QueueConnections(common.EnableRule, common.EnableRule); err != nil {
+		log.Error("Error while running conntrack nftables rule: %v", err.AsError())
 	}
 	// start monitoring firewall rules to intercept network traffic.
 	n.NewRulesChecker(n.AreRulesLoaded, n.ReloadRulesCallback)

daemon/firewall/nftables/rules.go

@@ -3,6 +3,7 @@ package nftables
 import (
 	"fmt"
 
+	"github.com/evilsocket/opensnitch/daemon/firewall/common"
 	"github.com/evilsocket/opensnitch/daemon/firewall/nftables/exprs"
 	"github.com/evilsocket/opensnitch/daemon/log"
 	daemonNetlink "github.com/evilsocket/opensnitch/daemon/netlink"
@@ -17,9 +18,9 @@ import (
 // of resolved domains.
 // This rule must be added in top of the system rules, otherwise it may get bypassed.
 // nft insert rule ip filter input udp sport 53 queue num 0 bypass
-func (n *Nft) QueueDNSResponses(enable bool, logError bool) (error, error) {
+func (n *Nft) QueueDNSResponses(enable, logError bool) *common.FirewallError {
 	if n.Conn == nil {
-		return nil, nil
+		return nil
 	}
 	families := []string{exprs.NFT_FAMILY_INET}
 	for _, fam := range families {
@@ -68,28 +69,28 @@ func (n *Nft) QueueDNSResponses(enable bool, logError bool) (error, error) {
 	}
 	// apply changes
 	if !n.Commit() {
-		return fmt.Errorf("Error adding DNS interception rules"), nil
+		return &common.FirewallError{Err4: fmt.Errorf("Error adding DNS interception rules"), Err6: nil}
 	}
 
-	return nil, nil
+	return nil
 }
 
 // QueueConnections inserts the firewall rule which redirects connections to us.
 // Connections are queued until the user denies/accept them, or reaches a timeout.
 // This rule must be added at the end of all the other rules, that way we can add
 // rules above this one to exclude a service/app from being intercepted.
 // nft insert rule ip mangle OUTPUT ct state new queue num 0 bypass
-func (n *Nft) QueueConnections(enable bool, logError bool) (error, error) {
+func (n *Nft) QueueConnections(enable, logError bool) *common.FirewallError {
 	if n.Conn == nil {
-		return nil, fmt.Errorf("nftables QueueConnections: netlink connection not active")
+		return &common.FirewallError{Err4: fmt.Errorf("nftables QueueConnections: netlink connection not active"), Err6: nil}
 	}
 	table := n.GetTable(exprs.NFT_CHAIN_MANGLE, exprs.NFT_FAMILY_INET)
 	if table == nil {
-		return nil, fmt.Errorf("QueueConnections() Error getting table mangle-inet")
+		return &common.FirewallError{Err4: fmt.Errorf("QueueConnections() Error getting table mangle-inet"), Err6: nil}
 	}
 	chain := GetChain(exprs.NFT_HOOK_OUTPUT, table)
 	if chain == nil {
-		return nil, fmt.Errorf("QueueConnections() Error getting outputChain: output-%s", table.Name)
+		return &common.FirewallError{Err4: fmt.Errorf("QueueConnections() Error getting outputChain: output-%s", table.Name), Err6: nil}
 	}
 
 	n.Conn.AddRule(&nftables.Rule{
@@ -174,7 +175,7 @@ func (n *Nft) QueueConnections(enable bool, logError bool) (error, error) {
 
 	// apply changes
 	if !n.Commit() {
-		return fmt.Errorf("Error adding interception rule "), nil
+		return &common.FirewallError{Err4: fmt.Errorf("Error adding interception rule "), Err6: nil}
 	}
 
 	if enable {
@@ -191,7 +192,7 @@ func (n *Nft) QueueConnections(enable bool, logError bool) (error, error) {
 		daemonNetlink.KillAllSockets()
 	}
 
-	return nil, nil
+	return nil
 }
 
 // InsertRule inserts a rule at the top of rules list.

daemon/firewall/nftables/rules_test.go

@@ -155,8 +155,8 @@ func TestQueueConnections(t *testing.T) {
 		t.Error("pre step add_chain() output-mangle-inet failed")
 	}
 
-	if err1, err2 := nftest.Fw.QueueConnections(true, true); err1 != nil && err2 != nil {
-		t.Errorf("rule to queue connections not added: %s, %s", err1, err2)
+	if err := nftest.Fw.QueueConnections(true, true); err != nil {
+		t.Errorf("rule to queue connections not added: %v", err.AsError())
 	}
 
 	r, _ := getRule(t, conn, exprs.NFT_CHAIN_MANGLE, exprs.NFT_HOOK_OUTPUT, nftb.InterceptionRuleKey, 0)
@@ -189,8 +189,8 @@ func TestQueueDNSResponses(t *testing.T) {
 		t.Error("pre step add_chain() input-filter-inet failed")
 	}
 
-	if err1, err2 := nftest.Fw.QueueDNSResponses(true, true); err1 != nil && err2 != nil {
-		t.Errorf("rule to queue DNS responses not added: %s, %s", err1, err2)
+	if err := nftest.Fw.QueueDNSResponses(true, true); err != nil {
+		t.Errorf("rule to queue DNS responses not added: %v", err.AsError())
 	}
 
 	r, _ := getRule(t, conn, exprs.NFT_CHAIN_FILTER, exprs.NFT_HOOK_INPUT, nftb.InterceptionRuleKey, 0)

daemon/firewall/nftables/system.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/evilsocket/opensnitch/daemon/firewall/common"
 	"github.com/evilsocket/opensnitch/daemon/firewall/config"
 	"github.com/evilsocket/opensnitch/daemon/firewall/iptables"
 	"github.com/evilsocket/opensnitch/daemon/firewall/nftables/exprs"
@@ -124,8 +125,8 @@ func (n *Nft) AddSystemRules(reload, backupExistingChains bool) {
 					chain.Rules[i].UUID = uuid.String()
 				}
 				if chain.Rules[i].Enabled {
-					if err4, _ := n.AddSystemRule(chain.Rules[i], chain); err4 != nil {
-						n.SendError(fmt.Sprintf("%s (%s)", err4, chain.Rules[i].UUID))
+					if err := n.AddSystemRule(chain.Rules[i], chain); err != nil {
+						n.SendError(fmt.Sprintf("%s (%s)", err.Err4, chain.Rules[i].UUID))
 					}
 				}
 			}
@@ -153,28 +154,28 @@ func (n *Nft) DeleteSystemRules(force, restoreExistingChains, logErrors bool) {
 }
 
 // AddSystemRule inserts a new rule.
-func (n *Nft) AddSystemRule(rule *config.FwRule, chain *config.FwChain) (err4, err6 error) {
+func (n *Nft) AddSystemRule(rule *config.FwRule, chain *config.FwChain) *common.FirewallError {
 	n.Lock()
 	defer n.Unlock()
 	exprList := []expr.Any{}
 
 	for _, expression := range rule.Expressions {
 		exprsOfRule := n.parseExpression(chain.Table, chain.Name, chain.Family, expression)
 		if exprsOfRule == nil {
-			return fmt.Errorf("%s invalid rule parameters: %v", rule.UUID, expression), nil
+			return &common.FirewallError{Err4: fmt.Errorf("%s invalid rule parameters: %v", rule.UUID, expression)}
 		}
 		exprList = append(exprList, *exprsOfRule...)
 	}
 	if len(exprList) > 0 {
 		exprVerdict := exprs.NewExprVerdict(rule.Target, rule.TargetParameters)
 		if exprVerdict == nil {
-			return fmt.Errorf("%s invalid verdict %s %s", rule.UUID, rule.Target, rule.TargetParameters), nil
+			return &common.FirewallError{Err4: fmt.Errorf("%s invalid verdict %s %s", rule.UUID, rule.Target, rule.TargetParameters)}
 		}
 		exprList = append(exprList, *exprVerdict...)
 		if err := n.InsertRule(chain.Name, chain.Table, chain.Family, rule.Position, &exprList); err != nil {
-			return err, nil
+			return &common.FirewallError{Err4: err}
 		}
 	}
 
-	return nil, nil
+	return nil
 }

daemon/firewall/rules.go

@@ -22,8 +22,8 @@ type Firewall interface {
 
 	EnableInterception()
 	DisableInterception(bool)
-	QueueDNSResponses(bool, bool) (error, error)
-	QueueConnections(bool, bool) (error, error)
+	QueueDNSResponses(bool, bool) *common.FirewallError
+	QueueConnections(bool, bool) *common.FirewallError
 	CleanRules(bool)
 
 	AddSystemRules(bool, bool)

daemon/go.mod

@@ -4,6 +4,7 @@ go 1.17
 
 require (
 	github.com/fsnotify/fsnotify v1.4.7
+	github.com/fvbommel/sortorder v1.1.0
 	github.com/golang/protobuf v1.5.0
 	github.com/google/gopacket v1.1.19
 	github.com/google/nftables v0.1.0
@@ -15,5 +16,18 @@ require (
 	golang.org/x/net v0.17.0
 	golang.org/x/sys v0.13.0
 	google.golang.org/grpc v1.32.0
-	google.golang.org/protobuf v1.26.0 // indirect
+	google.golang.org/protobuf v1.26.0
+)
+
+require (
+	github.com/BurntSushi/toml v0.4.1 // indirect
+	github.com/google/go-cmp v0.5.6 // indirect
+	github.com/josharian/native v0.0.0-20200817173448-b6b71def0850 // indirect
+	github.com/mdlayher/netlink v1.4.2 // indirect
+	github.com/mdlayher/socket v0.0.0-20211102153432-57e3fa563ecb // indirect
+	golang.org/x/mod v0.8.0 // indirect
+	golang.org/x/text v0.13.0 // indirect
+	golang.org/x/tools v0.6.0 // indirect
+	google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 // indirect
+	honnef.co/go/tools v0.2.2 // indirect
 )