[Auditbeat Host] Add host, packages, and processes metricsets #8436

cwurm · 2018-09-25T16:55:14Z

This adds three new metricsets to the new system module:

host collects general information about the system.
packages collects information about the installed packages and can detect changes.
processes collects all currently running processes and can detect changes.

Implements simple, initial versions of what a host, packages, and processes metricsets could look like.

x-pack/auditbeat/module/system/packages/packages.go

cwurm · 2018-09-25T16:56:50Z

This going into the feature branch the code is not terribly well tested (esp. on systems other than Mac), but there are system tests for each metricset.

I'm a bit unhappy about having copied the whole magefile.go over from auditbeat/, but it seems there's no way to import targets in mage. Happy for suggestions, I didn't look too deeply into it so far to be honest.

webmat

For any other reviewers, here's the steps I used to make this code work. Not sure if this is obvious to everyone:

Run make in x-pack/auditbeat as well. You'll be running the binary created in this dir, not the one at auditbeat/auditbeat.
Config file:

# Optional. To use the same data & log dir when running auditbeat from the x-pack directory
path.home: (your home path)/go/src/github.com/elastic/beats/auditbeat

auditbeat.modules:
- module: system
  metricsets:
    - host
    - packages
    - processes

Load the index template ./auditbeat setup --template -c ../../auditbeat/auditbeat.dev.yml
The binary must be run as root, so you'll need to set your dev config file owner to root
sudo ./auditbeat -c ../../auditbeat/auditbeat.dev.yml -e

My review

A few points that are not about the data

Perhaps we should remove the local addresses like 127.0.0.1 and ::1? I don't think they'll be useful.
Your fields for the index template are not nested under system., so they don't apply to the data you're sending (e.g. data sent: system.host.ip, template defines host.ip).
- When they are nested under system., I suspect that the IP addresses will need to be truncated not to include the subnet size. So you'd want to send 127.0.0.1 without the /8.
I would break up the initial listings of packages and processes to be one event per package/process. We can do very little with objects nested in arrays, with ElasticSearch or Kibana.
- So perhaps single package events for these full package inventories could have status:present instead of status:installed. Same for processes: system.processes.status:already running?

I've also commented the code here and there. But of course not being a proficient Go & Python dev, I can comment on very little at this time. Others will have to chime in for that :-)

One final detail: if building under x-pack remains a thing, we should add a gitignore there for "auditbeat", "logs" and "data" as well :-)

Good stuff overall!

webmat · 2018-09-27T18:59:35Z

metricbeat/tests/system/metricbeat.py

+
+        if not hasattr(self, 'beat_path'):
+            self.beat_path = os.path.abspath(os.path.join(os.path.dirname(__file__), "../../"))
+


Curious: why is this metricbeat test being modified? libbeat/tests/system/beat/beat.py doesn't appear to have been modified in this PR...

Since Auditbeat with X-Pack is in its own xpack/auditbeat directory, beat_path needs to point to that. It was being overwritten since AuditbeatXPackTest in auditbeat_xpack.py ultimately extends BaseTest from metricbeat.py.

webmat · 2018-09-27T19:54:48Z

x-pack/auditbeat/cache/cache.go

+			missing = append(missing, cacheValue)
+			delete(cache.hashMap, cacheKey)
+		}
+	}


You appear to be doing an O(n^2) operation here (n before and after is roughly equal).

Since the new state (current) is all you need to keep in memory for the next tick, perhaps you can skip the updating of the existing hash map and save current as is for later. In other words, this isn't really a cache, but more of a "previous state".

Then you can find "new" and "missing" result sets by:

looping over all keys from current and searching the cache for each item based on key. Each "not found" is added to your "new" result set

looping over all keys from cache and searching the current (which would have to be a map as well, not a slice). Each "not found" is added to your "missing" result set.

Each of these loops is an O(n), and so is the work for building a map out of current.

Note that you can check for presence by assigning to a second, optional var when searching the map:

_, found := cache[key]

current is not a map, so I would need to build a new map and add all current items and their hashes. Doing that every time we check the cache (e.g. every second) seems pretty expensive - most of the time nothing will have changed, or only one or two items. Am I missing something?

Just wanted to loop back here. I won't push too hard on the O(n^2), since I'm a total Go noob, so I may be missing something in your code or in the Map initialization.

But my computer currently runs 500-ish processes. This means each time it's comparing the new list of processes with the last state, it's doing a multiple of 250 000 operations (500^2) instead of doing a multiple of 500 operations... This may be irrelevant, since typically servers run way less stuff than workstations.

So I just wanted to put that out there, but we can keep things as they are for now. This may be premature optimization.

webmat · 2018-09-27T20:00:56Z

x-pack/auditbeat/module/system/host/host.go

+
+	if config.ReportChanges {
+		// TODO: Implement reporting changes?
+		ms.log.Warnw("Metricset %v/%v does not support report_changes", moduleName, metricsetName)


This doesn't seem to interpolate correctly for me. Here's what I get in the logs:

2018-09-27T14:15:10.957-0400 WARN [system] host/host.go:52 Metricset %v/%v does not support report_changes {"system": "host"}

webmat · 2018-09-27T20:06:37Z

x-pack/auditbeat/module/system/packages/packages.go

+		"package.summary":     pkg.Summary,
+		"package.url":         pkg.URL,
+	}
+}


I love how much more information we're getting here over osquery :-D Thinking of "InstallTime" and "Summary" particularly!

x-pack/auditbeat/module/system/processes/processes.go

andrewkroh

It's off to a good start. I only looked at the Auditbeat code in this pass. The main issues are

Align field names to ECS.
Figure out a secomp solution for running rpm.

andrewkroh · 2018-09-27T19:54:48Z

x-pack/auditbeat/module/system/host/host.go

+
+	if config.ReportChanges {
+		// TODO: Implement reporting changes?
+		ms.log.Warnw("Metricset %v/%v does not support report_changes", moduleName, metricsetName)


I recommend using <metricset>.report_changes so this can be controlled independently for each metricset within the module config.

Makes sense, and I see the cpu and core metricsets of the system module in Metricbeat do that as well. I wonder if the configuration is counterintuitive - I myself tried at first to nest it under the metricset i.e.:

- module: system metricsets: - processes report_changes: true

Should we consider allowing something like this in the future, rather than have it apart?

For now, I'll change it to be processes.report_changes and packages.report_changes.

x-pack/auditbeat/module/system/packages/packages.go

x-pack/auditbeat/tests/system/test_metricsets.py

andrewkroh · 2018-09-27T20:50:42Z

x-pack/auditbeat/module/system/packages/packages.go

+*/
+func listRPMPackages() ([]cache.Cacheable, error) {
+	format := "%{NAME}|%{VERSION}|%{RELEASE}|%{ARCH}|%{LICENSE}|%{INSTALLTIME}|%{SIZE}|%{URL}|%{SUMMARY}\\n"
+	out, err := exec.Command("/usr/bin/rpm", "--qf", format, "-qa").Output()


This won't work on Linux unless something is changed w.r.t. the seccomp policy or config.

Right, true. Do you think it makes sense to allow Auditbeat to execute /usr/bin/rpm, or change to reading the RPM db files in /var/lib/rpm/* with the help of a library instead?

If we could directly read the database file or use librpm's database api and statically link it with Auditbeat that would be the ideal solution in my opinion because we don't have to modify Auditbeat's security posture.

Makes sense. There are some Go libraries for RPM and BerkeleyDB out there that we could use as well. I'd like to do that in a separate PR though, so this one can get some closure. I'll remove the RPM code for now.

I agree, this can it can be handled at a later date. For now you could inject config to disable seccomp for the system test case on Linux that executes rpm.

x-pack/auditbeat/module/system/packages/packages.go

x-pack/auditbeat/tests/system/auditbeat_xpack.py

x-pack/auditbeat/module/system/host/host.go

cwurm · 2018-10-04T12:32:43Z

So... I think this is ready for another review. I hope I've addressed any concerns and not missed anything - apologies if I did (and please point it out).

Follow-up 1: RPM support

RPM will be implemented in a separate PR. We'll have to figure out how to read the rpmdb - there are some Go libraries for RPM and for BerkeleyDB (which rpmdb is using) and the official rpm C library we could use.

Follow-up 2: Templates

At the moment, we cannot generate Elasticsearch templates from two directories (auditbeat and x-pack/auditbeat in this case). @andrewkroh is working on a solution.

Schema

I hope the fields are mostly ECS-compliant, though many are not in ECS.

The current schemas are:

One vs. many documents

When reporting a snapshot of the state (host info, list of currently running processes, list of installed packages) we could send one document containing all items (the code does this now) or one document for each process/package.

I like sending one document, because it makes it easy to know what the system's current status is. You just has to find the most recent snapshot and then replay any events from there. If the snapshot is in multiple documents, it's hard to get them all together - you have to know or guess how many there are and query until you have all of them, and you would need some kind of correlation field (e.g. timestamp, UUID) to know which belong to the same snapshot. I also don't see what one would do with multiple documents - it's state information, so I don't think Kibana can visualize it in a meaningful time series way.

webmat · 2018-10-15T20:39:10Z

@cwurm I agree that if we have all processes / packages (even in full checkins) we then need a way to make correlation easier. In other words, figure out what are all the docs related to a given checkin. I'd like to experiment further with the data set. You may be right that all the data in one array may be best at this time.

I initially pushed for one per document in order to get the full power of aggregations & search. However since there will be multiple indices at play and subtle links between them (e.g. one checkin reports many packages), the direct querying by users will not be as simple as it usually is for monitoring indices anyway. In other words, I'm now realizing that making the data model easy to query by end users should perhaps not be our goal at this time.

So let's leave it as is and learn more about how we'll need to query the data before making this change. 👍🏼

Is there a way to force a full process & package checkin? Right now I'm only getting start/stops on the processes.

A suggestion, as well: on host checkins, please save all IPs as an array on the ECS field host.ip. The field is already defined as an ip field via the common fields in libbeat. You can leave the array of all interface details untouched, however. I want us to keep this juicy information.

I haven't looked at the code again yet. I'll try to do this a bit tomorrow.

cwurm · 2018-10-16T16:19:21Z

Is there a way to force a full process & package checkin? Right now I'm only getting start/stops on the processes.

Everytime Auditbeat starts it should output a full list of currently running processes. Following that it will report only started/stopped ones if processes.report_changes is set to true (the default).

In full debug mode (started with -d "*") the log should look like this:

2018-10-16T17:09:11.462+0100	INFO	instance/beat.go:383	auditbeat start running.
2018-10-16T17:09:11.462+0100	INFO	[monitoring]	log/log.go:117	Starting metrics logging every 30s
2018-10-16T17:09:11.462+0100	DEBUG	[module]	module/wrapper.go:117	Starting Wrapper[name=system, len(metricSetWrappers)=1]
2018-10-16T17:09:11.463+0100	DEBUG	[module]	module/wrapper.go:179	Starting metricSetWrapper[module=system, name=processes, host=]
2018-10-16T17:09:11.515+0100	DEBUG	[publish]	pipeline/processor.go:308	Publish event: {
  "@timestamp": "2018-10-16T16:09:11.463Z",
  "@metadata": {
    "beat": "auditbeat",
    "type": "doc",
    "version": "7.0.0-alpha1"
  },
  "system": {
    "processes": {
      "process": [
        {
          "args": [
            "./auditbeat",
            "-e",
            "-d",
            "*",
            "-c",
            "auditbeat.yml.test"
          ],
          "pid": 96448,
          "ppid": 96447,
          "cwd": "/Users/cwurm/go/src/github.com/elastic/beats/x-pack/auditbeat",
          "exe": "./auditbeat",
          "starttime": "2018-10-16T16:09:10.586Z",
          "status": "running",
          "name": "auditbeat"
        },
[...]

What do you see?

A suggestion, as well: on host checkins, please save all IPs as an array on the ECS field host.ip.

👍

webmat · 2018-10-16T17:18:56Z

Gotcha. Yes, I do see the full package listing right when starting auditbeat. We'd need a deterministic way to differentiate a full listing vs an update notification. Perhaps use event.category to distinguish between full and update? This wording is generic enough that this could be used for packages, processes, groups, users, etc. Anything where we'll publish a full listing + occasional updates. WDYT?

tsg · 2018-10-18T10:09:10Z

x-pack/auditbeat/module/system/packages/_meta/docs.asciidoc

@@ -0,0 +1,8 @@
+The System `packages` metricset provides ... TODO.


Can you include the information of what packaging systems are currently supported? I think dpkg/deb and Homebrew, right? And probably the next line with the OS support is superfluous.

tsg · 2018-10-18T10:20:55Z

x-pack/auditbeat/module/system/packages/packages.go

+}
+
+func listDebPackages() ([]*Package, error) {
+	const statusFile = "/var/lib/dpkg/status"


We should leave it for a follow up PR, but I think we should test this code by injecting the status file location and providing a sample.

tsg · 2018-10-18T10:21:16Z

x-pack/auditbeat/module/system/packages/packages.go

+}
+
+func listBrewPackages() ([]*Package, error) {
+	const cellarPath = "/usr/local/Cellar"


Same comment here, we should make sure we follow up with tests.

tsg · 2018-10-18T10:37:07Z

x-pack/auditbeat/cache/cache.go

+package cache
+
+// Cache is just a map being used as a cache.
+type Cache struct {


I wonder if the Cache doesn't need a mutex? Is it always called only from a single go-routine? If yes, then it's fine.

Fetch is always called from the same goroutine. But I don’t see much of a downside to make it thread safe now.

tsg

LGTM. Left some relatively minor comments, which can be handled in follow ups.

cwurm · 2018-10-19T11:20:38Z

Merged into feature branch.

Follow up actions (feel free to add if I forgot something)

RPM support in packages
Distinguishing between snapshots and diffs - it's possible now but maybe needs to be more explicit
Fill ECS fields host.ip (and host.mac?)
Templates
More tests: Sample files for packages, integration tests
Documentation
Make Cache thread safe

tsg · 2018-10-19T13:16:01Z

@cwurm Looks good, should we make an Auditbeat system module meta ticket with that checklist?

Adds host, packages, and processes metricsets to Auditbeat. Host collects general host information, e.g. boottime, timezone, OS, network interfaces. Packages collects information about installed packages. For now, it supports debian and homebrew on darwin. Processes collects information about currently running, started, and stopped processes.

Christoph Wurm added 5 commits September 25, 2018 17:45

Add host, packages, and processes metricsets

b9de5c6

Implements simple, initial versions of what a host, packages, and processes metricsets could look like.

Add diffing to processes metricset

687f54b

Add diffing to packages metricset

0b89b90

Add fields.yml files and unit test.

4315796

Add system tests.

621bc34

cwurm added review Auditbeat labels Sep 25, 2018

houndci-bot reviewed Sep 25, 2018

View reviewed changes

x-pack/auditbeat/module/system/packages/packages.go Outdated Show resolved Hide resolved

Christoph Wurm added 6 commits September 25, 2018 17:58

Minor fixes.

ee76d3b

Elastic license check and some formatting.

066a9b0

Formatting and making autopep work from xpack/.

6085e69

Fix formatting in metricbeat.

5ad0e56

Improve error handling.

fcab22c

Skip processes system test on macOS if not root.

e99c4db

webmat reviewed Sep 27, 2018

View reviewed changes

andrewkroh requested changes Sep 27, 2018

View reviewed changes

Christoph Wurm added 3 commits September 28, 2018 12:33

Change to {processes|packages}.report_changes.

018828c

Change to xxhash.

05b8ab4

Various changes to packages.

ecfe354

cwurm added in progress Pull request is currently in progress. and removed review labels Sep 28, 2018

Christoph Wurm added 4 commits October 1, 2018 11:50

Use *{Package|ProcessInfo} instead of Cacheable.

e9452ff

Add .gitignore in x-pack/auditbeat/.

681cc68

Collect information about network interfaces.

973241c

Add TestData functions, move some fields around.

92e1d72

houndci-bot reviewed Oct 2, 2018

View reviewed changes

x-pack/auditbeat/module/system/host/host.go Show resolved Hide resolved

Christoph Wurm added 3 commits October 2, 2018 11:00

Add comment to NetworkInterface.

d6548ef

Cache Hash() in Cache.

637d3df

Remove RPM code (for now).

30ef25e

cwurm added review and removed in progress Pull request is currently in progress. labels Oct 4, 2018

tsg reviewed Oct 18, 2018

View reviewed changes

tsg approved these changes Oct 18, 2018

View reviewed changes

cwurm added the SecOps label Oct 19, 2018

cwurm merged commit b8025f8 into elastic:feature-auditbeat-host Oct 19, 2018

cwurm mentioned this pull request Oct 24, 2018

[Auditbeat] System module 6.6 #8725

Closed

21 tasks

cwurm mentioned this pull request Jan 16, 2019

[Auditbeat] System module 6.7 / 7.0 #10103

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Auditbeat Host] Add host, packages, and processes metricsets #8436

[Auditbeat Host] Add host, packages, and processes metricsets #8436

cwurm commented Sep 25, 2018

cwurm commented Sep 25, 2018

webmat left a comment

webmat Sep 27, 2018

cwurm Sep 28, 2018

webmat Sep 27, 2018

cwurm Sep 28, 2018

webmat Oct 15, 2018

webmat Sep 27, 2018

webmat Sep 27, 2018

andrewkroh left a comment

andrewkroh Sep 27, 2018

cwurm Sep 28, 2018

andrewkroh Sep 27, 2018

cwurm Oct 3, 2018

andrewkroh Oct 3, 2018

cwurm Oct 4, 2018

andrewkroh Oct 4, 2018

cwurm commented Oct 4, 2018

webmat commented Oct 15, 2018

cwurm commented Oct 16, 2018

webmat commented Oct 16, 2018

tsg Oct 18, 2018

tsg Oct 18, 2018

tsg Oct 18, 2018

tsg Oct 18, 2018

andrewkroh Oct 18, 2018 •

edited

Loading

tsg left a comment

cwurm commented Oct 19, 2018 •

edited

Loading

tsg commented Oct 19, 2018


		if not hasattr(self, 'beat_path'):
		self.beat_path = os.path.abspath(os.path.join(os.path.dirname(__file__), "../../"))

		@@ -0,0 +1,8 @@
		The System `packages` metricset provides ... TODO.

[Auditbeat Host] Add host, packages, and processes metricsets #8436

[Auditbeat Host] Add host, packages, and processes metricsets #8436

Conversation

cwurm commented Sep 25, 2018

cwurm commented Sep 25, 2018

webmat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewkroh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwurm commented Oct 4, 2018

Follow-up 1: RPM support

Follow-up 2: Templates

Schema

One vs. many documents

webmat commented Oct 15, 2018

cwurm commented Oct 16, 2018

webmat commented Oct 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewkroh Oct 18, 2018 • edited Loading

Choose a reason for hiding this comment

tsg left a comment

Choose a reason for hiding this comment

cwurm commented Oct 19, 2018 • edited Loading

Follow up actions (feel free to add if I forgot something)

tsg commented Oct 19, 2018

andrewkroh Oct 18, 2018 •

edited

Loading

cwurm commented Oct 19, 2018 •

edited

Loading