Skip to content
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.

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

[osquery] - Add support for Fs.fs #30865

Closed
ofiriro3 opened this issue Mar 16, 2022 · 1 comment · Fixed by #30808
Closed

[osquery] - Add support for Fs.fs #30865

ofiriro3 opened this issue Mar 16, 2022 · 1 comment · Fixed by #30808

Comments

@ofiriro3
Copy link
Contributor

ofiriro3 commented Mar 16, 2022

Motivation

This PR enhances the functionality of the proc os-query package, such that it would also support file system injection.

Why is it important?

This change is important for two reasons:

  1. It lets the user expose only a part of his file system/give a relative path.
  2. It enhances the functionality such that a developer can use an in-memory file system such as fstest

This change is required by the @elastic/cloud-posture-security, since we are using this extension and would like to write tests using fstest.

In-depth

Cloudbeat allows the user to detect and analyze his Kubernetes environment.
In order to do so, the Cloudbeat fetches all types of resources from the Kubernetes along with processes data.
The processes data is being claimed out of a procfs directory that is being mounted to the pod Cloudbeats runs on.

The process fetcher, the fetcher responsible for collecting the procfs data, uses the osquery-extension.

We would like the osquery-extension to support injecting of fs.FS so that a developer can inject his own file-system to the function (that will make our tests much much tidier).

Solution proposition

  1. Duplicate only the function signatures.

Code example

// List returns all the processes in the proc folder
func List(root string) ([]string, error) {
	return ListFS(root)
}

func ListFS(sysfs fs.FS) ([]string, error) {
	var pids []string

	dirs, err := fs.ReadDir(sysfs, proc)

	if err != nil {
		return nil, err
	}

	for _, dir := range dirs {
		if !dir.IsDir() {
			continue
		}

		name := dir.Name()
		// Check if directory is number
		_, err := strconv.Atoi(name)
		if err != nil {
			err = nil
			continue
		}
		pids = append(pids, name)
	}

	return pids, nil
}
  1. Duplicate the function code
// List returns all the processes in the proc folder
func List(root string) ([]string, error) {
	var pids []string

	root = filepath.Join(root, "/proc")

	dirs, err := os.ReadDir(root)

	if err != nil {
		return nil, err
	}

	for _, dir := range dirs {
		if !dir.IsDir() {
			continue
		}

		name := dir.Name()
		// Check if directory is number
		_, err := strconv.Atoi(name)
		if err != nil {
			err = nil
			continue
		}
		pids = append(pids, name)
	}

	return pids, nil
}

func ListFS(sysfs fs.FS, root string) ([]string, error) {
	var pids []string

	root = filepath.Join(root, "/proc")

	dirs, err := fs.ReadDir(sysfs, root)

	if err != nil {
		return nil, err
	}

	for _, dir := range dirs {
		if !dir.IsDir() {
			continue
		}

		name := dir.Name()
		// Check if directory is number
		_, err := strconv.Atoi(name)
		if err != nil {
			err = nil
			continue
		}
		pids = append(pids, name)
	}

	return pids, nil
}

Risk assessment

As far as I understand, the DirFs doesn't support windows (it's not a cross-platform solution since windows do not have a root folder).
In addition, there is a small issue with using "/", it cannot appear as a prefix or a suffix of the input to

files := os.DirFS("/")
file, err := files.Open("/foo/bar.txt")

More can be found here - golang/go#44279.

Definition of done

The proc package will support the usage of an injected file system.

@ofiriro3 ofiriro3 self-assigned this Mar 16, 2022
@ofiriro3 ofiriro3 linked a pull request Mar 16, 2022 that will close this issue
6 tasks
@ofiriro3
Copy link
Contributor Author

This is a duplication of https://github.com/elastic/security-team/issues/3320.

It was copied here since we cannot transfer issues from a private repository to a public repository.

The content was merged already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant