Merge pull request #38 from cruise-automation/collin/fix_filestatchec…

…k_for_links

fix link handling

Changelog.md

@@ -7,6 +7,8 @@ Always update Version in Makefile
 
 ### Fixed
 - removed `release/` folder
+- FileStatCheck for links
+- general handling for links
 
 ## [v1.4.0] - 2020-04-30
 

Readme.md

@@ -171,17 +171,28 @@ Example Output:
 }
 ```
 
+### Link Handling
+
+With links we refer to soft links.
+Links can point to files on a different filesystem, therefore, we handle links in a special way.
+
+`FileStatCheck` will handle links like you would expect it.
+However if `AllowEmpty` is `false` and the file is a link the check fails.
+
+All other checks and dataextract will fail if the file is a link.
+Those checks need to be pointed to the actual file (the file the link points to).
+
 ### File Stat Check
 
 The `FileStatCheck` can be used to model the metadata for a specific file or directory.
 Any variation of the configuration will be reported as an offender.
 
-- `AllowEmpty` : bool, (optional) defines that the file can have zero size (default: false)
+- `AllowEmpty` : bool, (optional) defines that the file can have zero size will cause error if file is link (default: false)
 - `Uid` : int, (optional) specifies the UID of the file, not specifying a UID or specifying -1 will skip the check
 - `Gid` : int, (optional) specifies the GID of the file, not specifying a GID or specifying -1 will skip the check
 - `Mode` : string, (optional) specifies the UN*X file mode/permissions in octal, not specifying a mode will skip the check
 - `SELinuxLabel` : string, (optional) the SELinux label of the file (will skip the check if not set)
-- `LinkTarget` : string, (optional) the target of a symlink, not specifying a link target will skip the check. This is currently supported for `dirfs`, `squashfs`, and `ubifs` filesystems.
+- `LinkTarget` : string, (optional) the target of a symlink, not specifying a link target will skip the check. This is currently supported for `dirfs`, `squashfs`, `cpiofs`, and `ubifs` filesystems.
 - `Capability` : string array, (optional) list of capabilities (e.g. cap_net_admin+p).
 
 - `Desc` : string, (optional) is a descriptive string that will be attached to the report if there is a failed check

pkg/analyzer/dataextract/dataextract.go

@@ -92,6 +92,10 @@ func (state *dataExtractType) Name() string {
 }
 
 func (state *dataExtractType) CheckFile(fi *fsparser.FileInfo, filepath string) error {
+	if !fi.IsFile() {
+		return nil
+	}
+
 	fn := path.Join(filepath, fi.Name)
 	if _, ok := state.config[fn]; !ok {
 		return nil
@@ -108,27 +112,37 @@ func (state *dataExtractType) CheckFile(fi *fsparser.FileInfo, filepath string)
 			continue
 		}
 
+		if fi.IsLink() {
+			state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: file is Link (extract data from actual file): %s : %s",
+				item.Name, item.Desc))
+			continue
+		}
+
 		if item.RegEx != "" {
 			reg, err := regexp.Compile(item.RegEx)
 			if err != nil {
-				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: regex compile error: %s : %s %s", item.RegEx, item.Name, item.Desc))
+				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: regex compile error: %s : %s %s",
+					item.RegEx, item.Name, item.Desc))
 				continue
 			}
 
 			tmpfn, err := state.a.FileGet(fn)
 			if err != nil {
-				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: file read error, file get: %s : %s : %s", err, item.Name, item.Desc))
+				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: file read error, file get: %s : %s : %s",
+					err, item.Name, item.Desc))
 				continue
 			}
 			fdata, err := ioutil.ReadFile(tmpfn)
 			if err != nil {
-				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: file read error, file read: %s : %s : %s", err, item.Name, item.Desc))
+				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: file read error, file read: %s : %s : %s",
+					err, item.Name, item.Desc))
 				continue
 			}
 			_ = state.a.RemoveFile(tmpfn)
 			res := reg.FindAllStringSubmatch(string(fdata), -1)
 			if len(res) < 1 {
-				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: regex match error, regex: %s : %s : %s", item.RegEx, item.Name, item.Desc))
+				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: regex match error, regex: %s : %s : %s",
+					item.RegEx, item.Name, item.Desc))
 			} else {
 				// only one match
 				if len(res) == 1 && len(res[0]) == 2 {
@@ -147,15 +161,17 @@ func (state *dataExtractType) CheckFile(fi *fsparser.FileInfo, filepath string)
 					state.a.AddData(item.Name, string(jdata))
 					nameFilled[item.Name] = true
 				} else {
-					state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: regex match error : %s : %s", item.Name, item.Desc))
+					state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: regex match error : %s : %s",
+						item.Name, item.Desc))
 				}
 			}
 		}
 
 		if item.Script != "" {
 			out, err := runScriptOnFile(state.a, item.Script, item.ScriptOptions, fi, fn)
 			if err != nil {
-				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: script error: %s : %s : %s", err, item.Name, item.Desc))
+				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: script error: %s : %s : %s",
+					err, item.Name, item.Desc))
 			} else {
 				state.a.AddData(item.Name, out)
 				nameFilled[item.Name] = true
@@ -165,19 +181,22 @@ func (state *dataExtractType) CheckFile(fi *fsparser.FileInfo, filepath string)
 		if item.Json != "" {
 			tmpfn, err := state.a.FileGet(fn)
 			if err != nil {
-				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: file read error, file get: %s : %s : %s", err, item.Name, item.Desc))
+				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: file read error, file get: %s : %s : %s",
+					err, item.Name, item.Desc))
 				continue
 			}
 			fdata, err := ioutil.ReadFile(tmpfn)
 			if err != nil {
-				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: file read error, file read: %s : %s : %s", err, item.Name, item.Desc))
+				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: file read error, file read: %s : %s : %s",
+					err, item.Name, item.Desc))
 				continue
 			}
 			_ = state.a.RemoveFile(tmpfn)
 
 			out, err := util.XtractJsonField(fdata, strings.Split(item.Json, "."))
 			if err != nil {
-				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: JSON decode error: %s : %s : %s", err, item.Name, item.Desc))
+				state.a.AddData(item.Name, fmt.Sprintf("DataExtract ERROR: JSON decode error: %s : %s : %s",
+					err, item.Name, item.Desc))
 				continue
 			}
 			state.a.AddData(item.Name, out)
@@ -194,7 +213,8 @@ func runScriptOnFile(a analyzer.AnalyzerType, script string, scriptOptions []str
 	if err != nil {
 		return "", err
 	}
-	options := []string{fname, filepath.Base(fpath), fmt.Sprintf("%d", fi.Uid), fmt.Sprintf("%d", fi.Gid), fmt.Sprintf("%o", fi.Mode), fi.SELinuxLabel}
+	options := []string{fname, filepath.Base(fpath), fmt.Sprintf("%d", fi.Uid), fmt.Sprintf("%d", fi.Gid),
+		fmt.Sprintf("%o", fi.Mode), fi.SELinuxLabel}
 	if len(scriptOptions) > 0 {
 		options = append(options, "--")
 		options = append(options, scriptOptions...)

pkg/analyzer/dataextract/dataextract_test.go

@@ -61,7 +61,7 @@ func makeFile(data string, fn string) fsparser.FileInfo {
 	if err != nil {
 		panic(err)
 	}
-	return fsparser.FileInfo{Name: fn, Size: 1}
+	return fsparser.FileInfo{Name: fn, Size: 1, Mode: 0100644}
 }
 
 func TestRegex1(t *testing.T) {

pkg/analyzer/filecmp/filecmp.go

@@ -128,16 +128,17 @@ func makeTmpFromOld(filePath string) (string, error) {
 }
 
 func (state *fileCmpType) CheckFile(fi *fsparser.FileInfo, filepath string) error {
-	if !fi.IsFile() {
-		return nil
-	}
-
 	fn := path.Join(filepath, fi.Name)
 	if _, ok := state.files[fn]; !ok {
 		return nil
 	}
 
 	for _, item := range state.files[fn] {
+		if !fi.IsFile() || fi.IsLink() {
+			state.a.AddOffender(fn, "FileCmp: is not a file or is a link")
+			continue
+		}
+
 		tmpfn, err := state.a.FileGet(fn)
 		if err != nil {
 			state.a.AddOffender(fn, fmt.Sprintf("FileCmp: error getting file: %s", err))

pkg/analyzer/filecontent/filecontent.go

@@ -129,6 +129,18 @@ func regexCompile(rx string) (*regexp.Regexp, error) {
 	return reg, err
 }
 
+func (state *fileContentType) canCheckFile(fi *fsparser.FileInfo, fn string, item contentType) bool {
+	if !fi.IsFile() {
+		state.a.AddOffender(fn, fmt.Sprintf("FileContent: '%s' file is NOT a file : %s", item.name, item.Desc))
+		return false
+	}
+	if fi.IsLink() {
+		state.a.AddOffender(fn, fmt.Sprintf("FileContent: '%s' file is a link (check actual file) : %s", item.name, item.Desc))
+		return false
+	}
+	return true
+}
+
 func (state *fileContentType) CheckFile(fi *fsparser.FileInfo, filepath string) error {
 	fn := path.Join(filepath, fi.Name)
 	if _, ok := state.files[fn]; !ok {
@@ -141,6 +153,9 @@ func (state *fileContentType) CheckFile(fi *fsparser.FileInfo, filepath string)
 		items[n].checked = true
 		//fmt.Printf("name: %s file: %s (%s)\n", item.name, item.File, fn)
 		if item.RegEx != "" {
+			if !state.canCheckFile(fi, fn, item) {
+				continue
+			}
 			reg, err := regexCompile(item.RegEx)
 			if err != nil {
 				state.a.AddOffender(fn, fmt.Sprintf("FileContent: regex compile error: %s : %s : %s", item.RegEx, item.name, item.Desc))
@@ -181,6 +196,9 @@ func (state *fileContentType) CheckFile(fi *fsparser.FileInfo, filepath string)
 		}
 
 		if item.Digest != "" {
+			if !state.canCheckFile(fi, fn, item) {
+				continue
+			}
 			digestRaw, err := state.a.FileGetSha256(fn)
 			if err != nil {
 				return err
@@ -203,11 +221,17 @@ func (state *fileContentType) CheckFile(fi *fsparser.FileInfo, filepath string)
 			if fi.IsDir() {
 				state.a.CheckAllFilesWithPath(checkFileScript, &cbd, fn)
 			} else {
+				if !state.canCheckFile(fi, fn, item) {
+					continue
+				}
 				checkFileScript(fi, filepath, &cbd)
 			}
 		}
 
 		if item.Json != "" {
+			if !state.canCheckFile(fi, fn, item) {
+				continue
+			}
 			tmpfn, err := state.a.FileGet(fn)
 			if err != nil {
 				state.a.AddOffender(fn, fmt.Sprintf("FileContent: error getting file: %s", err))
@@ -275,7 +299,7 @@ func checkFileScript(fi *fsparser.FileInfo, fullpath string, cbData analyzer.All
 	cbd := cbData.(*callbackDataType)
 
 	// skip/ignore anything but normal files
-	if !fi.IsFile() {
+	if !fi.IsFile() || fi.IsLink() {
 		return
 	}
 

pkg/analyzer/filestatcheck/filestatcheck.go

@@ -34,6 +34,7 @@ type fileexistType struct {
 	Uid               int
 	Gid               int
 	SELinuxLabel      string
+	LinkTarget        string
 	Capabilities      []string
 	Desc              string
 	InformationalOnly bool
@@ -93,13 +94,33 @@ func (state *fileExistType) Finalize() string {
 				checkMode = true
 				mode, _ = strconv.ParseUint(item.Mode, 8, 0)
 			}
+			if item.LinkTarget != "" {
+				if !fi.IsLink() {
+					state.a.AddOffender(fn, fmt.Sprintf("File State Check failed LinkTarget set but file is not a link : %s", item.Desc))
+				} else if item.LinkTarget != fi.LinkTarget {
+					if item.InformationalOnly {
+						state.a.AddInformational(fn, fmt.Sprintf("File State Check failed LinkTarget does not match '%s' found '%s' : %s", item.LinkTarget, fi.LinkTarget, item.Desc))
+					} else {
+						state.a.AddOffender(fn, fmt.Sprintf("File State Check failed LinkTarget does not match '%s' found '%s' : %s", item.LinkTarget, fi.LinkTarget, item.Desc))
+					}
+				}
+			}
+			// not allow empty with check if file size is zero
 			if !item.AllowEmpty && fi.Size == 0 {
 				if item.InformationalOnly {
 					state.a.AddInformational(fn, fmt.Sprintf("File State Check failed: size: %d AllowEmpyt=false : %s", fi.Size, item.Desc))
 				} else {
 					state.a.AddOffender(fn, fmt.Sprintf("File State Check failed: size: %d AllowEmpyt=false : %s", fi.Size, item.Desc))
 				}
 			}
+			// not allow empty with check that file is not a Link
+			if !item.AllowEmpty && fi.IsLink() {
+				if item.InformationalOnly {
+					state.a.AddInformational(fn, fmt.Sprintf("File State Check failed: AllowEmpyt=false but file is Link (check link target instead) : %s", item.Desc))
+				} else {
+					state.a.AddOffender(fn, fmt.Sprintf("File State Check failed: AllowEmpyt=false but file is Link (check link target instead) : %s", item.Desc))
+				}
+			}
 			if checkMode && fi.Mode != mode {
 				if item.InformationalOnly {
 					state.a.AddInformational(fn, fmt.Sprintf("File State Check failed: mode found %o should be %s : %s", fi.Mode, item.Mode, item.Desc))
@@ -128,7 +149,6 @@ func (state *fileExistType) Finalize() string {
 					state.a.AddOffender(fn, fmt.Sprintf("File State Check failed: selinux label found = %s should be = %s : %s", fi.SELinuxLabel, item.SELinuxLabel, item.Desc))
 				}
 			}
-
 			if len(item.Capabilities) > 0 {
 				if !capability.CapsEqual(item.Capabilities, fi.Capabilities) {
 					if item.InformationalOnly {