Runtime whitelist parsing improvement (#2422)

* Improve whitelist parsing * Split whitelist check into a function tied to whitelist, also since we check node debug we can make a pointer to node containing whitelist * No point passing clog as an argument since it is just a pointer to node we already know about * We should break instead of returning false, false as it may have been whitelisted by ips/cidrs * reimplement early return if expr errors * Fix lint and dont need to parse ip back to string just loop over sources * Log error with node logger as it provides context * Move getsource to a function cleanup some code * Change func name * Split out compile to a function so we can use in tests. Add a bunch of tests * spell correction * Use node logger so it has context * alternative solution * quick fixes * Use containswls * Change whitelist test to use parseipsource and only events * Make it simpler * Postoverflow tests, some basic ones to make sure it works * Use official pkg * Add @mmetc reco * Add @mmetc reco * Change if if to a switch to only evaluate once * simplify assertions --------- Co-authored-by: bui <thibault@crowdsec.net> Co-authored-by: Marco Mariani <marco@crowdsec.net>
crowdsecurity · Oct 16, 2023 · 19de3a8 · 19de3a8
1 parent e7ad3d8
commit 19de3a8
Show file tree

Hide file tree

Showing 5 changed files with 522 additions and 92 deletions.
diff --git a/pkg/parser/node.go b/pkg/parser/node.go
@@ -3,7 +3,6 @@ package parser
 import (
 	"errors"
 	"fmt"
-	"net"
 	"strings"
 	"time"
 
@@ -172,75 +171,24 @@ func (n *Node) process(p *types.Event, ctx UnixParserCtx, expressionEnv map[stri
 	if n.Name != "" {
 		NodesHits.With(prometheus.Labels{"source": p.Line.Src, "type": p.Line.Module, "name": n.Name}).Inc()
 	}
-	isWhitelisted := false
-	hasWhitelist := false
-	var srcs []net.IP
-	/*overflow and log don't hold the source ip in the same field, should be changed */
-	/* perform whitelist checks for ips, cidr accordingly */
-	/* TODO move whitelist elsewhere */
-	if p.Type == types.LOG {
-		if _, ok := p.Meta["source_ip"]; ok {
-			srcs = append(srcs, net.ParseIP(p.Meta["source_ip"]))
-		}
-	} else if p.Type == types.OVFLW {
-		for k := range p.Overflow.Sources {
-			srcs = append(srcs, net.ParseIP(k))
-		}
+	exprErr := error(nil)
+	isWhitelisted := n.CheckIPsWL(p.ParseIPSources())
+	if !isWhitelisted {
+		isWhitelisted, exprErr = n.CheckExprWL(cachedExprEnv)
 	}
-	for _, src := range srcs {
-		if isWhitelisted {
-			break
-		}
-		for _, v := range n.Whitelist.B_Ips {
-			if v.Equal(src) {
-				clog.Debugf("Event from [%s] is whitelisted by IP (%s), reason [%s]", src, v, n.Whitelist.Reason)
-				isWhitelisted = true
-			} else {
-				clog.Tracef("whitelist: %s is not eq [%s]", src, v)
-			}
-			hasWhitelist = true
-		}
-		for _, v := range n.Whitelist.B_Cidrs {
-			if v.Contains(src) {
-				clog.Debugf("Event from [%s] is whitelisted by CIDR (%s), reason [%s]", src, v, n.Whitelist.Reason)
-				isWhitelisted = true
-			} else {
-				clog.Tracef("whitelist: %s not in [%s]", src, v)
-			}
-			hasWhitelist = true
-		}
+	if exprErr != nil {
+		// Previous code returned nil if there was an error, so we keep this behavior
+		return false, nil //nolint:nilerr
 	}
 
-	/* run whitelist expression tests anyway */
-	for eidx, e := range n.Whitelist.B_Exprs {
-		output, err := expr.Run(e.Filter, cachedExprEnv)
-		if err != nil {
-			clog.Warningf("failed to run whitelist expr : %v", err)
-			clog.Debug("Event leaving node : ko")
-			return false, nil
-		}
-		switch out := output.(type) {
-		case bool:
-			if n.Debug {
-				e.ExprDebugger.Run(clog, out, cachedExprEnv)
-			}
-			if out {
-				clog.Debugf("Event is whitelisted by expr, reason [%s]", n.Whitelist.Reason)
-				isWhitelisted = true
-			}
-			hasWhitelist = true
-		default:
-			log.Errorf("unexpected type %t (%v) while running '%s'", output, output, n.Whitelist.Exprs[eidx])
-		}
-	}
 	if isWhitelisted && !p.Whitelisted {
 		p.Whitelisted = true
 		p.WhitelistReason = n.Whitelist.Reason
 		/*huglily wipe the ban order if the event is whitelisted and it's an overflow */
 		if p.Type == types.OVFLW { /*don't do this at home kids */
 			ips := []string{}
-			for _, src := range srcs {
-				ips = append(ips, src.String())
+			for k := range p.Overflow.Sources {
+				ips = append(ips, k)
 			}
 			clog.Infof("Ban for %s whitelisted, reason [%s]", strings.Join(ips, ","), n.Whitelist.Reason)
 			p.Overflow.Whitelisted = true
@@ -395,9 +343,10 @@ func (n *Node) process(p *types.Event, ctx UnixParserCtx, expressionEnv map[stri
 	}
 
 	/*
-		This is to apply statics when the node *has* whitelists that successfully matched the node.
+		This is to apply statics when the node either was whitelisted, or is not a whitelist (it has no expr/ips wl)
+		It is overconvoluted and should be simplified
 	*/
-	if len(n.Statics) > 0 && (isWhitelisted || !hasWhitelist) {
+	if len(n.Statics) > 0 && (isWhitelisted || !n.ContainsWLs()) {
 		clog.Debugf("+ Processing %d statics", len(n.Statics))
 		// if all else is good in whitelist, process node's statics
 		err := n.ProcessStatics(n.Statics, p)
@@ -610,36 +559,11 @@ func (n *Node) compile(pctx *UnixParserCtx, ectx EnricherCtx) error {
 	}
 
 	/* compile whitelists if present */
-	for _, v := range n.Whitelist.Ips {
-		n.Whitelist.B_Ips = append(n.Whitelist.B_Ips, net.ParseIP(v))
-		n.Logger.Debugf("adding ip %s to whitelists", net.ParseIP(v))
-		valid = true
-	}
-
-	for _, v := range n.Whitelist.Cidrs {
-		_, tnet, err := net.ParseCIDR(v)
-		if err != nil {
-			n.Logger.Fatalf("Unable to parse cidr whitelist '%s' : %v.", v, err)
-		}
-		n.Whitelist.B_Cidrs = append(n.Whitelist.B_Cidrs, tnet)
-		n.Logger.Debugf("adding cidr %s to whitelists", tnet)
-		valid = true
-	}
-
-	for _, filter := range n.Whitelist.Exprs {
-		expression := &ExprWhitelist{}
-		expression.Filter, err = expr.Compile(filter, exprhelpers.GetExprOptions(map[string]interface{}{"evt": &types.Event{}})...)
-		if err != nil {
-			n.Logger.Fatalf("Unable to compile whitelist expression '%s' : %v.", filter, err)
-		}
-		expression.ExprDebugger, err = exprhelpers.NewDebugger(filter, exprhelpers.GetExprOptions(map[string]interface{}{"evt": &types.Event{}})...)
-		if err != nil {
-			log.Errorf("unable to build debug filter for '%s' : %s", filter, err)
-		}
-		n.Whitelist.B_Exprs = append(n.Whitelist.B_Exprs, expression)
-		n.Logger.Debugf("adding expression %s to whitelists", filter)
-		valid = true
+	whitelistValid, err := n.CompileWLs()
+	if err != nil {
+		return err
 	}
+	valid = valid || whitelistValid
 
 	if !valid {
 		/* node is empty, error force return */

diff --git a/pkg/parser/whitelist.go b/pkg/parser/whitelist.go
@@ -1,11 +1,14 @@
 package parser
 
 import (
+	"fmt"
 	"net"
 
+	"github.com/antonmedv/expr"
 	"github.com/antonmedv/expr/vm"
 
 	"github.com/crowdsecurity/crowdsec/pkg/exprhelpers"
+	"github.com/crowdsecurity/crowdsec/pkg/types"
 )
 
 type Whitelist struct {
@@ -22,3 +25,111 @@ type ExprWhitelist struct {
 	Filter       *vm.Program
 	ExprDebugger *exprhelpers.ExprDebugger // used to debug expression by printing the content of each variable of the expression
 }
+
+func (n *Node) ContainsWLs() bool {
+	return n.ContainsIPLists() || n.ContainsExprLists()
+}
+
+func (n *Node) ContainsExprLists() bool {
+	return len(n.Whitelist.B_Exprs) > 0
+}
+
+func (n *Node) ContainsIPLists() bool {
+	return len(n.Whitelist.B_Ips) > 0 || len(n.Whitelist.B_Cidrs) > 0
+}
+
+func (n *Node) CheckIPsWL(srcs []net.IP) bool {
+	isWhitelisted := false
+	if !n.ContainsIPLists() {
+		return isWhitelisted
+	}
+	for _, src := range srcs {
+		if isWhitelisted {
+			break
+		}
+		for _, v := range n.Whitelist.B_Ips {
+			if v.Equal(src) {
+				n.Logger.Debugf("Event from [%s] is whitelisted by IP (%s), reason [%s]", src, v, n.Whitelist.Reason)
+				isWhitelisted = true
+				break
+			}
+			n.Logger.Tracef("whitelist: %s is not eq [%s]", src, v)
+		}
+		for _, v := range n.Whitelist.B_Cidrs {
+			if v.Contains(src) {
+				n.Logger.Debugf("Event from [%s] is whitelisted by CIDR (%s), reason [%s]", src, v, n.Whitelist.Reason)
+				isWhitelisted = true
+				break
+			}
+			n.Logger.Tracef("whitelist: %s not in [%s]", src, v)
+		}
+	}
+	return isWhitelisted
+}
+
+func (n *Node) CheckExprWL(cachedExprEnv map[string]interface{}) (bool, error) {
+	isWhitelisted := false
+
+	if !n.ContainsExprLists() {
+		return false, nil
+	}
+	/* run whitelist expression tests anyway */
+	for eidx, e := range n.Whitelist.B_Exprs {
+		//if we already know the event is whitelisted, skip the rest of the expressions
+		if isWhitelisted {
+			break
+		}
+		output, err := expr.Run(e.Filter, cachedExprEnv)
+		if err != nil {
+			n.Logger.Warningf("failed to run whitelist expr : %v", err)
+			n.Logger.Debug("Event leaving node : ko")
+			return isWhitelisted, err
+		}
+		switch out := output.(type) {
+		case bool:
+			if n.Debug {
+				e.ExprDebugger.Run(n.Logger, out, cachedExprEnv)
+			}
+			if out {
+				n.Logger.Debugf("Event is whitelisted by expr, reason [%s]", n.Whitelist.Reason)
+				isWhitelisted = true
+			}
+		default:
+			n.Logger.Errorf("unexpected type %t (%v) while running '%s'", output, output, n.Whitelist.Exprs[eidx])
+		}
+	}
+	return isWhitelisted, nil
+}
+
+func (n *Node) CompileWLs() (bool, error) {
+	for _, v := range n.Whitelist.Ips {
+		n.Whitelist.B_Ips = append(n.Whitelist.B_Ips, net.ParseIP(v))
+		n.Logger.Debugf("adding ip %s to whitelists", net.ParseIP(v))
+	}
+
+	for _, v := range n.Whitelist.Cidrs {
+		_, tnet, err := net.ParseCIDR(v)
+		if err != nil {
+			return false, fmt.Errorf("unable to parse cidr whitelist '%s' : %v", v, err)
+		}
+		n.Whitelist.B_Cidrs = append(n.Whitelist.B_Cidrs, tnet)
+		n.Logger.Debugf("adding cidr %s to whitelists", tnet)
+	}
+
+	for _, filter := range n.Whitelist.Exprs {
+		var err error
+		expression := &ExprWhitelist{}
+		expression.Filter, err = expr.Compile(filter, exprhelpers.GetExprOptions(map[string]interface{}{"evt": &types.Event{}})...)
+		if err != nil {
+			return false, fmt.Errorf("unable to compile whitelist expression '%s' : %v", filter, err)
+		}
+		expression.ExprDebugger, err = exprhelpers.NewDebugger(filter, exprhelpers.GetExprOptions(map[string]interface{}{"evt": &types.Event{}})...)
+		if err != nil {
+			n.Logger.Errorf("unable to build debug filter for '%s' : %s", filter, err)
+		}
+
+		n.Whitelist.B_Exprs = append(n.Whitelist.B_Exprs, expression)
+		n.Logger.Debugf("adding expression %s to whitelists", filter)
+	}
+	return n.ContainsWLs(), nil
+}