sys fw: report errors to the GUI after reloading

 - Send errors to the server (GUI) if there's any error when reloading
   the system fw rules (far from being perfect/optimal, needs a
   rewrite).
 - Don't load the configuration after saving it, let the watcher reload
   it on write change to avoid double reload/duplicated errors.

daemon/firewall/common/common.go

@@ -26,15 +26,47 @@ type (
 	// iptables and nftables.
 	Common struct {
 		RulesChecker *time.Ticker
-		stopChecker  chan bool
+		ErrChan      chan string
 		QueueNum     uint16
+		stopChecker  chan bool
 		Running      bool
 		Intercepting bool
 		FwEnabled    bool
 		sync.RWMutex
 	}
 )
 
+// 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)
+
+	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")
+	}
+}
+
 // SetQueueNum sets the queue number used by the firewall.
 // It's the queue where all intercepted connections will be sent.
 func (c *Common) SetQueueNum(qNum *int) {

daemon/firewall/config/config.go

@@ -215,8 +215,6 @@ func (c *Config) SaveConfiguration(rawConfig string) error {
 		return err
 	}
 
-	c.loadConfiguration([]byte(rawConfig))
-
 	if err = os.Chmod(c.file, 0600); err != nil {
 		log.Warning("unable to set system-fw.json permissions: %s", err)
 	}

daemon/firewall/iptables/iptables.go

@@ -101,6 +101,7 @@ func (ipt *Iptables) Init(qNum *int) {
 		return
 	}
 	ipt.SetQueueNum(qNum)
+	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

daemon/firewall/nftables/nftables.go

@@ -75,6 +75,7 @@ func (n *Nft) Init(qNum *int) {
 	if n.IsRunning() {
 		return
 	}
+	n.ErrChan = make(chan string, 100)
 	InitMapsStore()
 	n.SetQueueNum(qNum)
 	n.Conn = NewNft()

daemon/firewall/nftables/rules.go

@@ -151,7 +151,7 @@ func (n *Nft) InsertRule(chain, table, family string, position uint64, exprs *[]
 	}
 	n.Conn.InsertRule(rule)
 	if !n.Commit() {
-		return fmt.Errorf("%s rule not added", logTag)
+		return fmt.Errorf("rule not added")
 	}
 
 	return nil
@@ -161,13 +161,13 @@ func (n *Nft) InsertRule(chain, table, family string, position uint64, exprs *[]
 func (n *Nft) AddRule(chain, table, family string, position uint64, key string, exprs *[]expr.Any) (*nftables.Rule, error) {
 	tbl := n.GetTable(table, family)
 	if tbl == nil {
-		return nil, fmt.Errorf("%s addRule, Error getting table: %s, %s", logTag, table, family)
+		return nil, fmt.Errorf("getting %s table: %s, %s", logTag, table, family)
 	}
 
 	chainKey := getChainKey(chain, tbl)
 	chn, chok := sysChains.Load(chainKey)
 	if !chok {
-		return nil, fmt.Errorf("%s addRule, Error getting table: %s, %s", logTag, table, family)
+		return nil, fmt.Errorf("getting table: %s, %s", table, family)
 	}
 
 	rule := &nftables.Rule{
@@ -179,7 +179,7 @@ func (n *Nft) AddRule(chain, table, family string, position uint64, key string,
 	}
 	n.Conn.AddRule(rule)
 	if !n.Commit() {
-		return nil, fmt.Errorf("%s Error adding rule", logTag)
+		return nil, fmt.Errorf("adding %s rule", logTag)
 	}
 
 	return rule, nil

daemon/firewall/nftables/system.go

@@ -125,7 +125,7 @@ func (n *Nft) AddSystemRules(reload, backupExistingChains bool) {
 				}
 				if chain.Rules[i].Enabled {
 					if err4, _ := n.AddSystemRule(chain.Rules[i], chain); err4 != nil {
-						log.Warning("rule not added: %s", err4)
+						n.SendError(fmt.Sprintf("%s (%s)", err4, chain.Rules[i].UUID))
 					}
 				}
 			}

daemon/firewall/rules.go

@@ -31,6 +31,9 @@ type Firewall interface {
 
 	Serialize() (*protocol.SysFirewall, error)
 	Deserialize(sysfw *protocol.SysFirewall) ([]byte, error)
+
+	ErrorsChan() <-chan string
+	ErrChanEmpty() bool
 }
 
 var (
@@ -78,6 +81,16 @@ func IsRunning() bool {
 	return fw != nil && fw.IsRunning()
 }
 
+// ErrorsChan returns the channel where the errors are sent to.
+func ErrorsChan() <-chan string {
+	return fw.ErrorsChan()
+}
+
+// ErrChanEmpty checks if the errors channel is empty.
+func ErrChanEmpty() bool {
+	return fw.ErrChanEmpty()
+}
+
 // CleanRules deletes the rules we added.
 func CleanRules(logErrors bool) {
 	if fw == nil {

daemon/ui/notifications.go

@@ -194,6 +194,47 @@ func (c *Client) handleActionStopMonitorProcess(stream protocol.UI_Notifications
 	c.sendNotificationReply(stream, notification.Id, "", nil)
 }
 
+func (c *Client) handleActionReloadFw(stream protocol.UI_NotificationsClient, notification *protocol.Notification) {
+	log.Info("[notification] reloading firewall")
+
+	sysfw, err := firewall.Deserialize(notification.SysFirewall)
+	if err != nil {
+		log.Warning("firewall.Deserialize() error: %s", err)
+		c.sendNotificationReply(stream, notification.Id, "", fmt.Errorf("Error reloading firewall, invalid rules"))
+		return
+	}
+	if err := firewall.SaveConfiguration(sysfw); err != nil {
+		c.sendNotificationReply(stream, notification.Id, "", fmt.Errorf("Error saving system firewall rules: %s", err))
+		return
+	}
+	// TODO:
+	// - add new API endpoints to delete, add or change rules atomically.
+	// - a global goroutine where errors can be sent to the server (GUI).
+	go func(c *Client) {
+		var errors string
+		for {
+			select {
+			case fwerr := <-firewall.ErrorsChan():
+				errors = fmt.Sprint(errors, fwerr, ",")
+				if firewall.ErrChanEmpty() {
+					goto ExitWithError
+				}
+
+			// FIXME: can this operation last longer than 2s? if there're more than.. 100...10000 rules?
+			case <-time.After(2 * time.Second):
+				log.Debug("[notification] reload firewall. timeout fired, no errors?")
+				c.sendNotificationReply(stream, notification.Id, "", nil)
+				goto Exit
+
+			}
+		}
+	ExitWithError:
+		c.sendNotificationReply(stream, notification.Id, "", fmt.Errorf("%s", errors))
+	Exit:
+	}(c)
+
+}
+
 func (c *Client) handleNotification(stream protocol.UI_NotificationsClient, notification *protocol.Notification) {
 	switch {
 	case notification.Type == protocol.Action_MONITOR_PROCESS:
@@ -224,19 +265,7 @@ func (c *Client) handleNotification(stream protocol.UI_NotificationsClient, noti
 		c.sendNotificationReply(stream, notification.Id, "", nil)
 
 	case notification.Type == protocol.Action_RELOAD_FW_RULES:
-		log.Info("[notification] reloading firewall")
-
-		sysfw, err := firewall.Deserialize(notification.SysFirewall)
-		if err != nil {
-			log.Warning("firewall.Deserialize() error: %s", err)
-			c.sendNotificationReply(stream, notification.Id, "", fmt.Errorf("Error reloading firewall, invalid rules"))
-			return
-		}
-		if err := firewall.SaveConfiguration(sysfw); err != nil {
-			c.sendNotificationReply(stream, notification.Id, "", fmt.Errorf("Error saving system firewall rules: %s", err))
-			return
-		}
-		c.sendNotificationReply(stream, notification.Id, "", nil)
+		c.handleActionReloadFw(stream, notification)
 
 	// ENABLE_RULE just replaces the rule on disk
 	case notification.Type == protocol.Action_ENABLE_RULE: