10000 NVSHAS-7554 - Added a catch to block identical filter paths from bein… by jaimeyu-suse · Pull Request #934 · neuvector/neuvector · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

NVSHAS-7554 - Added a catch to block identical filter paths from bein… #934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

NVSHAS-7554 - Added a catch to block identical filter paths from bein… #934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11a6a62
NVSHAS-7554 - Added a catch to block identical filter paths from bein…
jaimeyu-suse Jul 27, 2023
30a02e6
added /tmp and /tmp/ to the unit test.
jaimeyu-suse Aug 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8000
25 changes: 18 additions & 7 deletions controller/rest/file_monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,28 @@ var fileAccessOptionList []string = []string{
}
var fileAccessOptionSet utils.Set = utils.NewSetFromSliceKind(fileAccessOptionList)

// parseFileFilter - takes a filter name and returns the directory, the regex expression, and bool if successful
// It is also important to note that this function returns:
// * the directory path the filter is to operate on
// * the regular expression to append the path (eg, adding .*)
// * boolean to signify if an error occurred parsing the filter.
// only support simple wildcard
// 1. /dir/xxx
// 2. /dir/xxx.*
// 3. /dir/*.xxx
// 4. /dir/*/abc/*
// not support [] () regexp
// NVSHAS-7554 notes - If a trailing `/` is found in the path, then we will treat the filter to work on _directories_.
// This means we will add the `/.*` suffix to the path for the regex so it can pick up files in the directory.
// Otherwise, omitting the trailling `/` means that we will treat the filter to work only on _files_.
// So `/tmp` will mean "watch the existing file /tmp for changes"
// And `/tmp/` will mean "watch the directory /tmp for changes such as new files or modified existing files"

func parseFileFilter(filter string) (string, string, bool) {
if strings.HasSuffix(filter, "/") {
filter += "*"
}
var base string
var directory string
var regxStr string
filter = filepath.Clean(filter)
if strings.ContainsAny(filter, "[]()<>") ||
Expand All @@ -49,7 +60,7 @@ func parseFileFilter(filter string) (string, string, bool) {
filter = strings.Replace(filter, ".", "\\.", -1)
filter = strings.Replace(filter, "*", ".*", -1)
if a := strings.LastIndex(filter, "/"); a >= 0 {
base = filter[:a]
directory = filter[:a]
regxStr = filter[a+1:]
} else {
return "", "", false
Expand All @@ -58,16 +69,16 @@ func parseFileFilter(filter string) (string, string, bool) {
return "", "", false
} else if !strings.Contains(regxStr, "*") {
// single file
base += "/" + regxStr
directory += "/" + regxStr
regxStr = ""
}
if _, err := regexp.Compile(base); err != nil {
if _, err := regexp.Compile(directory); err != nil {
return "", "", false
}
if _, err := regexp.Compile(regxStr); err != nil {
return "", "", false
}
return base, regxStr, true
return directory, regxStr, true
}

// caller has been verified for federal admin access right, no CRD rules
Expand Down Expand Up @@ -227,7 +238,7 @@ func handlerFileMonitorConfig(w http.ResponseWriter, r *http.Request, ps httprou
}

for i, cfilter := range profConf.Filters {
if cfilter.Filter == filter.Filter {
if (cfilter.Filter == filter.Filter) || (cfilter.Path == base) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that Filter is not the path, this is the pseudo regex expression. eg: /tmp/.*

While the additional check if Path is the same as base will throw an error if the path is the same. The reason is that the monitoring logic can only store one filter per directory.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some test cases and make sure this logic does help.

// conflict, delete predefined
if !cfilter.CustomerAdd {
profConf.Filters = append(profConf.Filters[:i], profConf.Filters[i+1:]...)
Expand All @@ -237,7 +248,7 @@ func handlerFileMonitorConfig(w http.ResponseWriter, r *http.Request, ps httprou
break
} else {
restRespErrorMessage(w, http.StatusBadRequest, api.RESTErrInvalidRequest,
fmt.Sprintf("duplicate filter: %s", filter.Filter))
fmt.Sprintf("duplicate filter path: Filter %s path already exists", filter.Filter))
return
}
}
Expand Down
2 changes: 2 additions & 0 deletions controller/rest/file_monitor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ func TestNormalizeForURL(t *testing.T) {
"/test/../ab/*": share.CLUSFileMonitorFilter{Path: "/ab", Regex: ".*"},
"/test/./ab/*": share.CLUSFileMonitorFilter{Path: "/test/ab", Regex: ".*"},
"/lib/": share.CLUSFileMonitorFilter{Path: "/lib", Regex: ".*"},
"/tmp/": share.CLUSFileMonitorFilter{Path: "/tmp", Regex: ".*"},
"/tmp": share.CLUSFileMonitorFilter{Path: "/tmp", Regex: ""},
}
badFilters := map[string]share.CLUSFileMonitorFilter{
"/test/./<ab>/*": share.CLUSFileMonitorFilter{Path: "", Regex: ""},
Expand Down
23 changes: 7 additions & 16 deletions share/fsmon/monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,8 +575,7 @@ func (w *FileWatch) getCoreFile(cid string, pid int, profile *share.CLUSFileMoni
dirList := make(map[string]*osutil.FileInfoExt)
singleFiles := make([]*osutil.FileInfoExt, 0)

// get files and dirs from all filters
for _, filter := range profile.Filters {
addFilterFn := func(filter share.CLUSFileMonitorFilter){
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cosmetic to reduce the copy paste code. No changes in the logic was done and will make it easier to make changes in the future.

flt := &filterRegex{path: filterIndexKey(filter), recursive: filter.Recursive}
flt.regex, _ = regexp.Compile(fmt.Sprintf("^%s$", flt.path))
bBlockAccess := filter.Behavior == share.FileAccessBehaviorBlock
Expand All @@ -592,24 +591,16 @@ func (w *FileWatch) getCoreFile(cid string, pid int, profile *share.CLUSFileMoni
singleFiles = append(singleFiles, singles...)
}
}
// get files and dirs from all filters
for _, filter := range profile.Filters {
addFilterFn(filter)
}

// get files and dirs from all filters
for _, filter := range profile.FiltersCRD {
flt := &filterRegex{path: filterIndexKey(filter), recursive: filter.Recursive}
flt.regex, _ = regexp.Compile(fmt.Sprintf("^%s$", flt.path))
bBlockAccess := filter.Behavior == share.FileAccessBehaviorBlock
bUserAdded := filter.CustomerAdd
if strings.Contains(filter.Path, "*") {
subDirs := w.getSubDirList(pid, filter.Path, cid)
for _, sub := range subDirs {
singles := w.getDirAndFileList(pid, sub, filter.Regex, cid, flt, filter.Recursive, bBlockAccess, bUserAdded, dirList)
singleFiles = append(singleFiles, singles...)
}
} else {
singles := w.getDirAndFileList(pid, filter.Path, filter.Regex, cid, flt, filter.Recursive, bBlockAccess, bUserAdded, dirList)
singleFiles = append(singleFiles, singles...)
}
addFilterFn(filter)
}

return dirList, singleFiles
}

Expand Down
0