improved rules (re)loading

- Delete lists of domains if the rule about to change is of type Lists.
- Monitor the lists of domains, and reload them if they're modified.
- Delete rules from disk when the Duration changes from
  Always (saved on disk) to !Always (temporary).
- After the above operation a fsnotify Remove event is fired. Don't
  delete the rule from memory if it's temporary.
- Rules are only compiled if they're enabled, avoiding unnecessary
  allocations.

daemon/core/core.go

@@ -1,11 +1,13 @@
 package core
 
 import (
+	"fmt"
 	"os"
 	"os/exec"
 	"os/user"
 	"path/filepath"
 	"strings"
+	"time"
 )
 
 const (
@@ -55,3 +57,12 @@ func ExpandPath(path string) (string, error) {
 	}
 	return "", nil
 }
+
+// GetFileModTime checks if a file has been modified.
+func GetFileModTime(filepath string) (time.Time, error) {
+	fi, err := os.Stat(filepath)
+	if err != nil || fi.IsDir() {
+		return time.Now(), fmt.Errorf("GetFileModTime() Invalid file")
+	}
+	return fi.ModTime(), nil
+}

daemon/rule/loader.go

@@ -54,6 +54,13 @@ func (l *Loader) NumRules() int {
 	return len(l.rules)
 }
 
+// GetAll returns the loaded rules.
+func (l *Loader) GetAll() map[string]*Rule {
+	l.RLock()
+	defer l.RUnlock()
+	return l.rules
+}
+
 // Load loads rules files from disk.
 func (l *Loader) Load(path string) error {
 	if core.Exists(path) == false {
@@ -66,9 +73,6 @@ func (l *Loader) Load(path string) error {
 		return fmt.Errorf("Error globbing '%s': %s", expr, err)
 	}
 
-	l.Lock()
-	defer l.Unlock()
-
 	l.path = path
 	if len(l.rules) == 0 {
 		l.rules = make(map[string]*Rule)
@@ -83,8 +87,6 @@ func (l *Loader) Load(path string) error {
 		}
 	}
 
-	l.sortRules()
-
 	if l.liveReload && l.liveReloadRunning == false {
 		go l.liveReloadWorker()
 	}
@@ -97,15 +99,24 @@ func (l *Loader) loadRule(fileName string) error {
 	if err != nil {
 		return fmt.Errorf("Error while reading %s: %s", fileName, err)
 	}
+	l.Lock()
+	defer l.Unlock()
 
 	var r Rule
 	err = json.Unmarshal(raw, &r)
 	if err != nil {
 		return fmt.Errorf("Error parsing rule from %s: %s", fileName, err)
 	}
+	raw = nil
+
+	if oldRule, found := l.rules[r.Name]; found {
+		l.cleanListsRule(oldRule)
+	}
 
 	if r.Enabled {
-		r.Operator.Compile()
+		if err := r.Operator.Compile(); err != nil {
+			log.Warning("Operator.Compile() error: %s: %s", err, r.Operator.Data)
+		}
 		if r.Operator.Type == List {
 			for i := 0; i < len(r.Operator.List); i++ {
 				if err := r.Operator.List[i].Compile(); err != nil {
@@ -113,34 +124,33 @@ func (l *Loader) loadRule(fileName string) error {
 				}
 			}
 		}
-	} else {
-		// if we're reloading the list of rules (due to changes on disk),
-		// we need to delete any possible loaded lists.
-		if r.Operator.Type == Lists {
-			r.Operator.ClearLists()
-		} else if r.Operator.Type == List {
-			for i := 0; i < len(r.Operator.List); i++ {
-				if r.Operator.List[i].Type == Lists {
-					r.Operator.ClearLists()
-				}
-			}
-		}
 	}
-	// FIXME: if a rule file is changed manually on disk from Always to !Always,
-	// the file is deleted from disk, but the Remove event deletes it
-	// also from memory.
-	l.deleteOldRuleFromDisk(&r)
+	if oldRule, found := l.rules[r.Name]; found {
+		l.deleteOldRuleFromDisk(oldRule, &r)
+	}
 
 	log.Debug("Loaded rule from %s: %s", fileName, r.String())
+	l.Lock()
 	l.rules[r.Name] = &r
+	l.sortRules()
+	l.Unlock()
+
+	if l.isTemporary(&r) {
+		err = l.scheduleTemporaryRule(&r)
+	}
 
 	return nil
 }
 
-// deleteRule deletes a rule from memory and from disk if the Duration is Always
+// deleteRule deletes a rule from memory if it has been deleted from disk.
+// This is only called if fsnotify's Remove event is fired, thus it doesn't
+// have to delete temporary rules (!Always).
 func (l *Loader) deleteRule(filePath string) {
 	fileName := filepath.Base(filePath)
-	l.Delete(fileName[:len(fileName)-5])
+	ruleName := fileName[:len(fileName)-5]
+	if rule, found := l.rules[ruleName]; found && rule.Duration == Always {
+		l.Delete(ruleName)
+	}
 }
 
 func (l *Loader) deleteRuleFromDisk(ruleName string) error {
@@ -150,11 +160,23 @@ func (l *Loader) deleteRuleFromDisk(ruleName string) error {
 
 // deleteOldRuleFromDisk deletes a rule from disk if the Duration changes
 // from Always (saved on disk), to !Always (temporary).
-func (l *Loader) deleteOldRuleFromDisk(newRule *Rule) {
-	if oldRule, found := l.rules[newRule.Name]; found {
-		if oldRule.Duration == Always && newRule.Duration != Always {
-			if err := l.deleteRuleFromDisk(oldRule.Name); err != nil {
-				log.Error("Error deleting old rule from disk: %s", oldRule.Name)
+func (l *Loader) deleteOldRuleFromDisk(oldRule, newRule *Rule) {
+	if oldRule.Duration == Always && newRule.Duration != Always {
+		if err := l.deleteRuleFromDisk(oldRule.Name); err != nil {
+			log.Error("Error deleting old rule from disk: %s", oldRule.Name)
+		}
+	}
+}
+
+// cleanListsRule erases the list of domains of an Operator of type Lists
+func (l *Loader) cleanListsRule(oldRule *Rule) {
+	if oldRule.Operator.Type == Lists {
+		oldRule.Operator.StopMonitoringLists()
+	} else if oldRule.Operator.Type == List {
+		for i := 0; i < len(oldRule.Operator.List); i++ {
+			if oldRule.Operator.List[i].Type == Lists {
+				oldRule.Operator.List[i].StopMonitoringLists()
+				break
 			}
 		}
 	}
@@ -184,6 +206,8 @@ func (l *Loader) liveReloadWorker() {
 			} else if event.Op&fsnotify.Remove == fsnotify.Remove {
 				if strings.HasSuffix(event.Name, ".json") {
 					log.Important("Rule deleted %s", path.Base(event.Name))
+					// we only need to delete from memory rules of type Always,
+					// because the Remove event is of a file, i.e.: Duration == Always
 					l.deleteRule(event.Name)
 				}
 			}
@@ -193,11 +217,8 @@ func (l *Loader) liveReloadWorker() {
 	}
 }
 
-// GetAll returns the loaded rules.
-func (l *Loader) GetAll() map[string]*Rule {
-	l.RLock()
-	defer l.RUnlock()
-	return l.rules
+func (l *Loader) isTemporary(r *Rule) bool {
+	return r.Duration != Restart && r.Duration != Always && r.Duration != Once
 }
 
 func (l *Loader) isUniqueName(name string) bool {
@@ -235,50 +256,47 @@ func (l *Loader) addUserRule(rule *Rule) {
 }
 
 func (l *Loader) replaceUserRule(rule *Rule) (err error) {
-	// If the rule has changed from Always (saved on disk) to !Always (temporary),
-	// we need to delete the rule from disk and keep it in memory.
-	l.deleteOldRuleFromDisk(rule)
+	if oldRule, found := l.rules[rule.Name]; found {
+		// If the rule has changed from Always (saved on disk) to !Always (temporary),
+		// we need to delete the rule from disk and keep it in memory.
+		l.deleteOldRuleFromDisk(oldRule, rule)
 
+		// delete loaded lists, if this is a rule of type Lists
+		l.cleanListsRule(oldRule)
+	}
+
+	if rule.Enabled {
+		if err := rule.Operator.Compile(); err != nil {
+			log.Warning("Operator.Compile() error: %s: %s", err, rule.Operator.Data)
+		}
+
+		if rule.Operator.Type == List {
+			// TODO: use List protobuf object instead of un/marshalling to/from json
+			if err = json.Unmarshal([]byte(rule.Operator.Data), &rule.Operator.List); err != nil {
+				return fmt.Errorf("Error loading rule of type list: %s", err)
+			}
+
+			for i := 0; i < len(rule.Operator.List); i++ {
+				if err := rule.Operator.List[i].Compile(); err != nil {
+					log.Warning("Operator.Compile() error: %s: ", err)
+				}
+			}
+		}
+	}
 	l.Lock()
 	l.rules[rule.Name] = rule
 	l.sortRules()
 	l.Unlock()
 
-	if rule.Enabled == false && rule.Operator.Type == Lists {
-		rule.Operator.ClearLists()
-	} else {
-		rule.Operator.isCompiled = false
-		if err := rule.Operator.Compile(); err != nil {
-			log.Warning("Operator.Compile() error: %s: %s", err, rule.Operator.Data)
-		}
+	if l.isTemporary(rule) {
+		err = l.scheduleTemporaryRule(rule)
 	}
 
-	if rule.Operator.Type == List {
-		// TODO: use List protobuf object instead of un/marshalling to/from json
-		if err = json.Unmarshal([]byte(rule.Operator.Data), &rule.Operator.List); err != nil {
-			return fmt.Errorf("Error loading rule of type list: %s", err)
-		}
+	return err
+}
 
-		// TODO handle the situation where the field Lists has been unchecked: delete lists
-		for i := 0; i < len(rule.Operator.List); i++ {
-			if rule.Enabled == false && rule.Operator.List[i].Type == Lists {
-				rule.Operator.ClearLists()
-				continue
-			}
-			// force re-Compile() changed rule
-			rule.Operator.List[i].isCompiled = false
-			if err := rule.Operator.List[i].Compile(); err != nil {
-				log.Warning("Operator.Compile() error: %s: ", err)
-			}
-		}
-	}
-
-	if rule.Duration == Restart || rule.Duration == Always || rule.Duration == Once {
-		return err
-	}
-
-	var tTime time.Duration
-	tTime, err = time.ParseDuration(string(rule.Duration))
+func (l *Loader) scheduleTemporaryRule(rule *Rule) error {
+	tTime, err := time.ParseDuration(string(rule.Duration))
 	if err != nil {
 		return err
 	}
@@ -290,8 +308,7 @@ func (l *Loader) replaceUserRule(rule *Rule) (err error) {
 		l.sortRules()
 		l.Unlock()
 	})
-
-	return err
+	return nil
 }
 
 // Add adds a rule to the list of rules, and optionally saves it to disk.

daemon/rule/operator.go

@@ -6,6 +6,7 @@ import (
 	"reflect"
 	"regexp"
 	"strings"
+	"sync"
 
 	"github.com/evilsocket/opensnitch/daemon/conman"
 	"github.com/evilsocket/opensnitch/daemon/core"
@@ -60,11 +61,14 @@ type Operator struct {
 	Data      string     `json:"data"`
 	List      []Operator `json:"list"`
 
-	cb         opCallback
-	re         *regexp.Regexp
-	netMask    *net.IPNet
-	isCompiled bool
-	lists      map[string]string
+	sync.RWMutex
+	cb                  opCallback
+	re                  *regexp.Regexp
+	netMask             *net.IPNet
+	isCompiled          bool
+	lists               map[string]string
+	listsMonitorRunning bool
+	exitMonitorChan     chan (bool)
 }
 
 // NewOperator returns a new operator object
@@ -96,13 +100,11 @@ func (o *Operator) Compile() error {
 			return err
 		}
 		o.re = re
-	} else if o.Type == Lists && o.Operand == OpDomainsLists {
+	} else if o.Operand == OpDomainsLists {
 		if o.Data == "" {
 			return fmt.Errorf("Operand lists is empty, nothing to load: %s", o)
 		}
-		if err := o.loadLists(); err != nil {
-			return err
-		}
+		o.loadLists()
 		o.cb = o.domainsListCmp
 	} else if o.Type == List {
 		o.Operand = OpList
@@ -160,6 +162,9 @@ func (o *Operator) domainsListCmp(v interface{}) bool {
 	if dstHost == "" {
 		return false
 	}
+	o.Lock()
+	defer o.Unlock()
+
 	if _, found := o.lists[dstHost]; found {
 		log.Debug("%s: %s, %s", log.Red("domain list match"), dstHost, o.lists[dstHost])
 		return true

daemon/rule/operator_lists.go

@@ -2,31 +2,145 @@ package rule
 
 import (
 	"fmt"
+	"github.com/evilsocket/opensnitch/daemon/core"
 	"github.com/evilsocket/opensnitch/daemon/log"
 	"io/ioutil"
 	"path/filepath"
 	"runtime/debug"
 	"strings"
+	"time"
 )
 
+var (
+	count = 0
+)
+
+func (o *Operator) monitorLists() {
+	count++
+	log.Info("monitor lists started: %s, %d", o.Data, count)
+	modTimes := make(map[string]time.Time)
+	totalFiles := 0
+	needReload := false
+
+	expr := filepath.Join(o.Data, "/*.*")
+	for {
+		select {
+		case <-o.exitMonitorChan:
+			goto Exit
+		default:
+			fileList, err := filepath.Glob(expr)
+			if err != nil {
+				needReload = false
+				continue
+			}
+			if len(fileList) != totalFiles {
+				needReload = true
+			}
+			totalFiles = len(fileList)
+
+			for _, filename := range fileList {
+				// an overwrite operation performs two tasks: truncate the file and save the new content,
+				// causing the file time to be modified twice.
+				modTime, err := core.GetFileModTime(filename)
+				if err != nil {
+					log.Debug("deleting saved mod time due to error reading the list, %s", filename)
+					delete(modTimes, filename)
+				} else if lastModTime, found := modTimes[filename]; found {
+					if lastModTime.Equal(modTime) == false {
+						log.Debug("list changed: %s, %s, %s", lastModTime, modTime, filename)
+						needReload = true
+					}
+				}
+				modTimes[filename] = modTime
+			}
+			fileList = nil
+
+			if needReload {
+				// we can't reload a single list, because the domains of all lists are added to the same map.
+				// we could have the domains separated by lists/files, but then we'd need to iterate the map in order
+				// to match a domain. Reloading the lists shoud only occur once a day.
+				if err := o.readLists(); err != nil {
+					log.Warning("%s", err)
+				}
+				needReload = false
+			}
+			time.Sleep(4 * time.Second)
+		}
+	}
+
+Exit:
+	modTimes = nil
+	o.ClearLists()
+	log.Info("lists monitor stopped")
+}
+
 // ClearLists deletes all the entries of a list
 func (o *Operator) ClearLists() {
-	log.Debug("clearing domains lists: %d - %s", len(o.lists), o.Data)
+	log.Info("clearing domains lists: %d - %s", len(o.lists), o.Data)
 	for k := range o.lists {
 		delete(o.lists, k)
 	}
-	o.lists = nil
 	debug.FreeOSMemory()
 }
 
-func (o *Operator) loadLists() error {
-	log.Info("loading domains lists: %s, %s, %s", o.Type, o.Operand, o.Data)
+// StopMonitoringLists stops the monitoring lists goroutine.
+func (o *Operator) StopMonitoringLists() {
+	if o.listsMonitorRunning == true {
+		o.exitMonitorChan <- true
+		o.exitMonitorChan = nil
+		o.listsMonitorRunning = false
+	}
+}
 
+func (o *Operator) readList(fileName string) (dups uint64) {
+	log.Debug("Loading domains list: %s", fileName)
+	raw, err := ioutil.ReadFile(fileName)
+	if err != nil {
+		log.Warning("Error reading list of domains (%s): %s", fileName, err)
+		return
+	}
+
+	log.Debug("domains list size: %d", len(raw))
+	lines := strings.Split(string(raw), "\n")
+	for _, domain := range lines {
+		if len(domain) < 9 {
+			continue
+		}
+		// exclude not valid lines
+		if domain[:7] != "0.0.0.0" && domain[:9] != "127.0.0.1" {
+			continue
+		}
+		host := domain[8:]
+		// exclude localhost entries
+		if domain[:9] == "127.0.0.1" {
+			host = domain[10:]
+		}
+		if host == "local" || host == "localhost" || host == "localhost.localdomain" || host == "broadcasthost" {
+			continue
+		}
+		if _, found := o.lists[host]; found {
+			dups++
+			continue
+		}
+		o.Lock()
+		o.lists[host] = fileName
+		o.Unlock()
+	}
+	raw = nil
+	lines = nil
+	log.Info("%d domains loaded, %s", len(o.lists), fileName)
+
+	return dups
+}
+
+func (o *Operator) readLists() error {
 	o.ClearLists()
-	var dups uint64
 
-	// this list is particular to this operator/rule
+	var dups uint64
+	// this list is particular to this operator and rule
+	o.Lock()
 	o.lists = make(map[string]string)
+	o.Unlock()
 
 	expr := filepath.Join(o.Data, "/*.*")
 	fileList, err := filepath.Glob(expr)
@@ -35,39 +149,19 @@ func (o *Operator) loadLists() error {
 	}
 
 	for _, fileName := range fileList {
-		log.Debug("Loading domains list: %s", fileName)
-		raw, err := ioutil.ReadFile(fileName)
-		log.Debug("domains list size: %d", len(raw))
-		if err != nil {
-			log.Warning("Error reading list of domains (%s): %s", fileName, err)
-			continue
-		}
-		for _, domain := range strings.Split(string(raw), "\n") {
-			if len(domain) < 9 {
-				continue
-			}
-			// exclude not valid lines
-			if domain[:7] != "0.0.0.0" && domain[:9] != "127.0.0.1" {
-				continue
-			}
-			host := domain[8:]
-			// exclude localhost entries
-			if domain[:9] == "127.0.0.1" {
-				host = domain[10:]
-			}
-			if host == "local" || host == "localhost" || host == "localhost.localdomain" || host == "broadcasthost" {
-				continue
-			}
-			if _, found := o.lists[host]; found {
-				dups++
-				continue
-			}
-			o.lists[host] = fileName
-		}
-		raw = nil
-		log.Info("domains loaded: %d, %s", len(o.lists), fileName)
+		dups += o.readList(fileName)
 	}
-	log.Info("Total domains loaded: %d - Duplicated: %d", len(o.lists), dups)
-
+	log.Info("%d lists loaded, %d domains, %d duplicated", len(fileList), len(o.lists), dups)
 	return nil
 }
+
+func (o *Operator) loadLists() {
+	log.Info("loading domains lists: %s, %s, %s", o.Type, o.Operand, o.Data)
+
+	// when loading from disk, we don't use the Operator's constructor, so we need to create this channel
+	if o.exitMonitorChan == nil {
+		o.exitMonitorChan = make(chan bool)
+		o.listsMonitorRunning = true
+		go o.monitorLists()
+	}
+}