chore: merge upstream changes into mjholub/firewall-errors-wrap

Author: mjholub

daemon/firewall/common/common.go

@@ -17,26 +17,26 @@ var (
 	RestoreChains  = true
 	BackupChains   = true
 	ReloadConf     = true
-
-	DefaultCheckInterval = 10 * time.Second
-	RulesCheckerDisabled = "0s"
 )
 
 type (
 	callback     func()
 	callbackBool func() bool
 
+	stopChecker struct {
+		ch chan bool
+		sync.RWMutex
+	}
+
 	// Common holds common fields and functionality of both firewalls,
 	// iptables and nftables.
 	Common struct {
-		RulesChecker       *time.Ticker
-		ErrChan            chan string
-		stopChecker        chan bool
-		RulesCheckInterval time.Duration
-		QueueNum           uint16
-		Running            bool
-		Intercepting       bool
-		FwEnabled          bool
+		RulesChecker    *time.Ticker
+		stopCheckerChan *stopChecker
+		QueueNum        uint16
+		Running         bool
+		Intercepting    bool
+		FwEnabled       bool
 		sync.RWMutex
 	}
 	// FirewallError is a type that holds both IPv4 and IPv6 errors.
@@ -77,46 +77,15 @@ func (s *stopChecker) exit() <-chan bool {
 	return s.ch
 }
 
-// ErrorsChan returns the channel where the errors are sent to.
-func (c *Common) ErrorsChan() <-chan string {
-	return c.ErrChan
-}
-
-// ErrChanEmpty checks if the errors channel is empty.
-func (c *Common) ErrChanEmpty() bool {
-	return len(c.ErrChan) == 0
-}
-
-// SendError sends an error to the channel of errors.
-func (c *Common) SendError(err string) {
-	log.Warning("%s", err)
+func (s *stopChecker) stop() {
+	s.Lock()
+	defer s.Unlock()
 
-	if len(c.ErrChan) >= cap(c.ErrChan) {
-		log.Debug("fw errors channel full, emptying errChan")
-		for e := range c.ErrChan {
-			log.Warning("%s", e)
-			if c.ErrChanEmpty() {
-				break
-			}
-		}
-		return
-	}
-	select {
-	case c.ErrChan <- err:
-	case <-time.After(100 * time.Millisecond):
-		log.Warning("SendError() channel locked? REVIEW")
-	}
-}
-
-func (c *Common) SetRulesCheckerInterval(interval string) {
-	dur, err := time.ParseDuration(interval)
-	if err != nil {
-		log.Warning("Invalid rules checker interval (falling back to %s): %s", DefaultCheckInterval, err)
-		c.RulesCheckInterval = DefaultCheckInterval
-		return
+	if s.ch != nil {
+		s.ch <- true
+		close(s.ch)
+		s.ch = nil
 	}
-
-	c.RulesCheckInterval = dur
 }
 
 // SetQueueNum sets the queue number used by the firewall.
@@ -128,6 +97,7 @@ func (c *Common) SetQueueNum(qNum *int) {
 	if qNum != nil {
 		c.QueueNum = uint16(*qNum)
 	}
+
 }
 
 // IsRunning returns if the firewall is running or not.
@@ -160,38 +130,25 @@ func (c *Common) IsIntercepting() bool {
 func (c *Common) NewRulesChecker(areRulesLoaded callbackBool, reloadRules callback) {
 	c.Lock()
 	defer c.Unlock()
-	if c.RulesCheckInterval.String() == RulesCheckerDisabled {
-		log.Info("Fw rules checker disabled ...")
-		return
+	if c.stopCheckerChan != nil {
+		c.stopCheckerChan.stop()
+		c.stopCheckerChan = nil
 	}
 
-	if c.RulesChecker != nil {
-		c.RulesChecker.Stop()
-		select {
-		case c.stopChecker <- true:
-		case <-time.After(5 * time.Millisecond):
-			log.Error("NewRulesChecker: timed out stopping monitor rules")
-		}
-	}
-	c.stopChecker = make(chan bool, 1)
-	log.Info("Starting new fw checker every %s ...", c.RulesCheckInterval)
-	c.RulesChecker = time.NewTicker(c.RulesCheckInterval)
+	c.stopCheckerChan = &stopChecker{ch: make(chan bool, 1)}
+	c.RulesChecker = time.NewTicker(time.Second * 15)
 
-	go startCheckingRules(c.stopChecker, c.RulesChecker, areRulesLoaded, reloadRules)
+	go c.startCheckingRules(areRulesLoaded, reloadRules)
 }
 
 // StartCheckingRules monitors if our rules are loaded.
 // If the rules to intercept traffic are not loaded, we'll try to insert them again.
-func startCheckingRules(exitChan <-chan bool, rulesChecker *time.Ticker, areRulesLoaded callbackBool, reloadRules callback) {
+func (c *Common) startCheckingRules(areRulesLoaded callbackBool, reloadRules callback) {
 	for {
 		select {
-		case <-exitChan:
+		case <-c.stopCheckerChan.exit():
 			goto Exit
-		case _, active := <-rulesChecker.C:
-			if !active {
-				goto Exit
-			}
-
+		case <-c.RulesChecker.C:
 			if areRulesLoaded() == false {
 				reloadRules()
 			}
@@ -204,20 +161,14 @@ Exit:
 
 // StopCheckingRules stops checking if firewall rules are loaded.
 func (c *Common) StopCheckingRules() {
-	c.Lock()
-	defer c.Unlock()
+	c.RLock()
+	defer c.RUnlock()
 
 	if c.RulesChecker != nil {
-		select {
-		case c.stopChecker <- true:
-			close(c.stopChecker)
-		case <-time.After(5 * time.Millisecond):
-			// We should not arrive here
-			log.Error("StopCheckingRules: timed out stopping monitor rules")
-		}
-
 		c.RulesChecker.Stop()
-		c.RulesChecker = nil
+	}
+	if c.stopCheckerChan != nil {
+		c.stopCheckerChan.stop()
 	}
 }
 

daemon/firewall/iptables/iptables.go

@@ -41,9 +41,9 @@ const (
 
 // SystemRule blabla
 type SystemRule struct {
-	Rule  *config.FwRule
 	Table string
 	Chain string
+	Rule  *config.FwRule
 }
 
 // SystemChains keeps track of the fw rules that have been added to the system.
@@ -54,13 +54,16 @@ type SystemChains struct {
 
 // Iptables struct holds the fields of the iptables fw
 type Iptables struct {
+	config.Config
+	common.Common
+
+	bin  string
+	bin6 string
+
 	regexRulesQuery       *regexp.Regexp
 	regexSystemRulesQuery *regexp.Regexp
-	bin                   string
-	bin6                  string
-	chains                SystemChains
-	common.Common
-	config.Config
+
+	chains SystemChains
 
 	sync.Mutex
 }
@@ -93,18 +96,16 @@ func (ipt *Iptables) Name() string {
 
 // Init inserts the firewall rules and starts monitoring for firewall
 // changes.
-func (ipt *Iptables) Init(qNum *int, configPath, monitorInterval string) {
+func (ipt *Iptables) Init(qNum *int) {
 	if ipt.IsRunning() {
 		return
 	}
 	ipt.SetQueueNum(qNum)
-	ipt.SetRulesCheckerInterval(monitorInterval)
-	ipt.ErrChan = make(chan string, 100)
 
 	// In order to clean up any existing firewall rule before start,
 	// we need to load the fw configuration first to know what rules
 	// were configured.
-	ipt.NewSystemFwConfig(configPath, ipt.preloadConfCallback, ipt.reloadRulesCallback)
+	ipt.NewSystemFwConfig(ipt.preloadConfCallback, ipt.reloadRulesCallback)
 	ipt.LoadDiskConfiguration(!common.ReloadConf)
 
 	// start from a clean state
@@ -117,7 +118,6 @@ func (ipt *Iptables) Init(qNum *int, configPath, monitorInterval string) {
 
 // Stop deletes the firewall rules, allowing network traffic.
 func (ipt *Iptables) Stop() {
-	ipt.ErrChan = make(chan string, 100)
 	if ipt.Running == false {
 		return
 	}
@@ -141,9 +141,9 @@ func IsAvailable() error {
 // EnableInterception adds fw rules to intercept connections.
 func (ipt *Iptables) EnableInterception() {
 	if err := ipt.QueueConnections(common.EnableRule, true); err != nil {
-		log.Fatal("Error while running conntrack firewall rule: %s %s", err)
+		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: %s %s", err)
+		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)