daemon: fix empty list operator matching every connection

[munzzyy]

listMatch() starts its AND fold at true:

func (o *Operator) listMatch(con *conman.Connection, hasChecksums bool) bool {
	res := true
	for i := 0; i < len(o.List); i++ {
		res = res && o.List[i].Match(con, hasChecksums)
	}
	return res
}

If o.List is empty the loop never runs and the function returns true. A rule whose operator is type list with zero sub-operators matches every connection. Give that rule allow + precedence and FindFirstMatch() returns it for the first connection it sees, so nothing below it in the ruleset gets evaluated.

Reproduced through the path the GUI uses, handleActionChangeRule() -> Loader.Replace():

=== RUN   TestScratchReproEmptyList
    repro_scratch_test.go:26: Replace() accepted the rule. sub-operators=0
    repro_scratch_test.go:44:   dest.host="telemetry.example.com"  -> matched "000-empty-list", action=allow
    repro_scratch_test.go:44:   dest.host="anything.invalid"       -> matched "000-empty-list", action=allow
    repro_scratch_test.go:44:   dest.host=""                       -> matched "000-empty-list", action=allow
--- PASS: TestScratchReproEmptyList (0.00s)

the scratch test that produced that output (not part of this PR)

package rule

import (
	"net"
	"testing"

	"github.com/evilsocket/opensnitch/daemon/conman"
	"github.com/evilsocket/opensnitch/daemon/netstat"
	"github.com/evilsocket/opensnitch/daemon/procmon"
)

func TestScratchReproEmptyList(t *testing.T) {
	l, err := NewLoader(false)
	if err != nil {
		t.Fatal(err)
	}

	op, _ := NewOperator(List, false, OpList, "", nil)
	r := Create("000-empty-list", "allow list rule with no operators", true, false, true, Allow, Always, op)

	if err := l.Replace(r, false); err != nil {
		t.Logf("Replace() rejected the rule: %s", err)
		return
	}
	t.Logf("Replace() accepted the rule. sub-operators=%d", len(r.Operator.List))

	for _, host := range []string{"telemetry.example.com", "anything.invalid", ""} {
		c := &conman.Connection{
			Protocol: "TCP",
			SrcPort:  55555,
			SrcIP:    net.ParseIP("192.168.1.111"),
			DstIP:    net.ParseIP("203.0.113.7"),
			DstPort:  uint(443),
			DstHost:  host,
			Process:  &procmon.Process{ID: 4242, Path: "/usr/bin/curl", Args: []string{"curl"}},
			Entry:    &netstat.Entry{UserId: 1000},
		}
		m := l.FindFirstMatch(c)
		if m == nil {
			t.Logf("  dest.host=%-24q -> no rule matched", host)
			continue
		}
		t.Logf("  dest.host=%-24q -> matched %q, action=%s", host, m.Name, m.Action)
	}
}

That rule would not have survived v1.6.3. replaceUserRule() unmarshalled Operator.Data unconditionally for list rules:

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)
	}

An empty Data failed there with unexpected end of JSON input and the rule was refused. On HEAD Deserialize() blanks Data for list rules and copies the sub-operators out of the protobuf message instead. When that protobuf list arrives empty, nothing between Deserialize() and listMatch() catches it.

So: length check in listMatch(), return false. That's what the other operators in the file already do. domainsListsCmp() and reListCmp() both return false when they have nothing to compare against.

Compile() looked like the tidier place for the check, but it doesn't work there. TestNewOperatorList compiles the operator before assigning opList.List, so the guard fires on a valid rule:

=== RUN   TestNewOperatorList/Operator_List_simple_case-insensitive
    operator_test.go:784: NewOperator list.regexp.err compiling: Operator of type list with an empty list of operators
--- FAIL: TestNewOperatorList (0.00s)

Test

TestNewOperatorListEmpty against unpatched operator.go:

$ go test ./rule/ -run TestNewOperatorListEmpty -v
=== RUN   TestNewOperatorListEmpty
    operator_test.go:859: Test NewOperator() List without operators
    operator_test.go:874: Test NewOperator() empty list matches every connection
--- FAIL: TestNewOperatorListEmpty (0.00s)
FAIL
FAIL	github.com/evilsocket/opensnitch/daemon/rule	0.007s

With the fix:

$ go test ./rule/ -run TestNewOperatorListEmpty -v
=== RUN   TestNewOperatorListEmpty
    operator_test.go:859: Test NewOperator() List without operators
--- PASS: TestNewOperatorListEmpty (0.00s)
PASS
ok  	github.com/evilsocket/opensnitch/daemon/rule	0.007s

Whole package, 27 tests, none failing:

$ go test ./rule/...
ok  	github.com/evilsocket/opensnitch/daemon/rule	136.178s

gofmt -l ./rule/ and go vet ./rule/... print exactly what they print on an unmodified tree: loader_test.go unformatted, four copies lock value warnings in loader.go and rule.go.