From c5a6eda6a221ad3812be2d653cfbb3950cd92ac5 Mon Sep 17 00:00:00 2001
From: Marius Sturm <marius@graylog.com>
Date: Tue, 11 Sep 2018 12:22:58 +0200
Subject: [PATCH] Fix collector_whitelist feature (#284)

* Fix collector_whitelist feature

* Fix collector_whitelist feature

The previous fix for #279 introduced an error,
because result.Path was not always set.

Instead of failing on non-existant files, still perform the PathMatch
but report them in result.DoesExist.

Broken symlinks are always reported as an error, because we cannot
resolve their actual path using EvalSymlinks.

Add some tests.

Fixes #279
---
 common/helper.go      | 25 +++++++++------
 common/helper_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 9 deletions(-)

diff --git a/common/helper.go b/common/helper.go
index e8e8cb5..e2724c4 100644
--- a/common/helper.go
+++ b/common/helper.go
@@ -170,25 +170,32 @@ func Sprintf(format string, values ...interface{}) (string, error) {
 }
 
 type PathMatchResult struct {
-	Path   string
-	Match  bool
-	IsLink bool
+	Path      string
+	Match     bool
+	IsLink    bool
+	DoesExist bool
 }
 
 func PathMatch(path string, patternList []string) (PathMatchResult, error) {
-	result := PathMatchResult{}
-	if _, err := os.Stat(path); os.IsExist(err) {
+	result := PathMatchResult{Path: path, DoesExist: true}
+
+	if _, err := os.Lstat(path); err == nil {
 		resolvedPath, err := filepath.EvalSymlinks(path)
 		if err != nil {
+			// error out on broken symlinks, because we cannot resolve
+			// their path with EvalSymlinks
+			result.DoesExist = false
 			return result, err
 		} else {
-			result.Path = resolvedPath
+			if resolvedPath != path {
+				result.IsLink = true
+				result.Path = resolvedPath
+			}
 		}
+	} else {
+		result.DoesExist = false
 	}
 
-	if result.Path != path {
-		result.IsLink = true
-	}
 	for _, pattern := range patternList {
 		match, err := filepath.Match(pattern, result.Path)
 		if err != nil {
diff --git a/common/helper_test.go b/common/helper_test.go
index 99b5cd2..51308ff 100644
--- a/common/helper_test.go
+++ b/common/helper_test.go
@@ -20,6 +20,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"runtime"
 	"strings"
 	"testing"
 )
@@ -98,3 +99,74 @@ func TestEncloseWithoutData(t *testing.T) {
 		t.Fail()
 	}
 }
+
+func TestPathMatch(t *testing.T) {
+	dir, err := ioutil.TempDir("", "test-path-match")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	execfile := filepath.Join(dir, "myexec")
+	patternList := []string{"/usr/bin/moo", "/sbin/bar"}
+	result, err := PathMatch(execfile, patternList)
+	if err != nil || result.Match {
+		t.Fatalf("'%s' should not match patternList %v err '%v' result '%v'", execfile, patternList, err, result)
+	}
+	if result.DoesExist {
+		t.Fatalf("DoesExist should not be set")
+	}
+
+	os.Create(execfile)
+	patternList = []string{"/usr/bin/moo", "/sbin/bar", execfile}
+	result, err = PathMatch(execfile, patternList)
+	if err != nil || !result.Match {
+		t.Fatalf("'%s' should match patternList %v err '%v' result '%v'", execfile, patternList, err, result)
+	}
+	if !result.DoesExist {
+		t.Fatalf("DoesExist should be set")
+	}
+
+	patternList = []string{"/usr/bin/moo", "/sbin/bar", dir + "/*"}
+	result, err = PathMatch(execfile, patternList)
+	if err != nil || !result.Match {
+		t.Fatalf("'%s' should match globbing patternList %v err '%v' result '%v'",
+			execfile, patternList, err, result)
+	}
+
+}
+
+func TestPathMatchSymlink(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip()
+	}
+
+	dir, err := ioutil.TempDir("", "test-path-match")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	execfile := filepath.Join(dir, "myexec")
+	symlink := filepath.Join(dir, "symlink")
+	if err = os.Symlink(execfile, symlink); err != nil {
+		t.Fatal()
+	}
+	patternList := []string{"moo", "bar", execfile}
+	result, err := PathMatch(symlink, patternList)
+	if err == nil {
+		t.Fatalf("broken symlinks should report an error")
+	}
+
+	os.Create(execfile)
+	result, err = PathMatch(symlink, patternList)
+	if err != nil || !result.Match {
+		t.Fatalf("'%s' should match patternList %v err '%v' result '%v'", execfile, patternList, err, result)
+	}
+	if !result.IsLink {
+		t.Fatalf("result.IsLink is false")
+	}
+	if result.Path != execfile {
+		t.Fatalf("result.Path did not contain resolved symlink")
+	}
+}