-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Auditbeat] Add system module #9546
Conversation
This adds an skeleton x-pack module to Auditbeat. The module is only included in the Elastic licensed Auditbeat binary. The config and fields.yml data are not yet included in the packaging. Additional updates are required.
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.
Changes necessary to make `mage fields` and `make system-tests` work again after merging master. (And deleting a misplaced parenthesis.)
Collects (via C functions) user information from /etc/passwd, /etc/shadow, and /etc/group on Linux. Detects new users, deleted users, changes to users (e.g. groups), and - as a special distinct category - password changes. Sends periodic state information about all users (frequency can be controlled). Otherwise, periodically checks the ctime of the above files, reads them if the ctime has changed, detects changes compared to its internal cache, and reports any changes. The cache is persisted to disk in a `beat.db` file (already used by the `file_integrity` module) after every `Fetch` and on `Close`. It contains a copy of all current user information incl. a SHA-512 hash of the password hash from /etc/shadow (to detect password changes between Auditbeat restarts - this hash is not sent to any output).
Collects information about open sockets (Linux only). Uses netlink to query for all currently open sockets. Sends information about all sockets on start, and periodically as determined by `state.period`. Otherwise, sends only newly opened or closed sockets. The sockets are enriched with process and user information.
Adds additional build flags to the new user metricset to prevent build failures on non-Linux systems. If it is configured, it will now throw an error and abort the launch.
Updates the `process` metricset to follow newest conventions: - Rename from `processes` to `process` - Change to single documents instead of arrays - Scheduled state reporting - Use top-level ECS fields
* Refactor Auditbeat build logic Update auditbeat and x-pack/auditbeat to share logic for generating config and packages. This makes auditbeat and x-pack/auditbeat have independent `package` targets where auditbeat generates only OSS packages and x-pack/auditbeat generates Elastic licensed packages. And x-pack/auditbeat will now be tested on Travis CI. * Skip failing Auditbeat system module test * Skip failing system/process test * Add temporary target alias for Windows CI * Fix file permission issues caused by Docker usage * Optimize chown by checking if UID/GID need changed
Updates the `host` metricset to be in line with the other metricsets in the `system` module: 1. Adds regular state reporting based on `state.period`/`host.state.period` 2. Persists state between restarts in `beat.db` 3. Detects changes in host information 4. Changes to using `system.host.ip`/`system.host.mac` instead of `system.host.network.interfaces`
Introduces a `user.detect_password_changes` config parameter that defaults to true in the config, but false in the code. Only if it is set to true will the code read the password field in /etc/passwd and /etc/shadow to detect password changes. The read password field values are put through 10 round of SHA-512 hashing before being locally stored.
To be compatible with ECS, changes the `event.type` field to `event.kind` throughout the system module.
The `packages` metricset is not yet ready. This disables the metricset's code, tests, fields, and docs until we have time to finish the work.
Namespaces all Auditbeat system module metricsets to `system.audit` to avoid any potential field clashes with Metricbeat.
…stic#9500) Setting `auditbeat.max_start_delay: 0` for system tests greatly reduces their execution time. (cherry picked from commit 7a2cf0f)
This adds a top-level `message` field to the `host`, `process`, `socket`, and `user` metricsets.
Adds config and asciidoc documentation for the four metricsets of the system module that are ready today: host, process, socket, user. Also adjusts the doc generation to include files from x-pack/auditbeat.
Allow the `process` metricset to run as any user by catching permission errors when trying to read other user's private process information.
Pinging @elastic/secops |
Ok, so some things that need fixing:
Plus whatever is making the build fail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were some of the files more problematic when rebasing on top of master?
I think this looks good overall. I'm in favor of merging sooner than later, then following up with smaller PRs where needed.
- I noticed two unexpected files with extension
.disabled
(see the review). These two files are the only thing I'd like to get an answer about, to understand if it's a problem with this PR. - Left a few comments about ECS & field naming. I think we should address those in a separate PR.
- I didn't remember you had modified Metricbeat in this branch, but I realized that this was from [Auditbeat] Socket metricset #8834. Makes sense.
LGTM
}, | ||
"system": { | ||
"audit": { | ||
"host": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a blocker for this PR, I think we should merge it sooner than later.
But we'll need to revisit this wrt ECS. Almost all keys under system.audit.host
map perfectly to ECS at the top level. So they should be moved there.
The exceptions I see are system.audit.host.uptime
and system.audit.host.timezone.offset.sec
. And lastly, the system.audit.host.timezone.name
should become event.timezone
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The top level host
object is populated by the add_host_metadata
processor. That processor is using a cache and so will not report the most up-to-date information. What would happen if we didn't include our own data here is that a change event would not actually have any changed data - it would still report the old state because of the cache.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very interesting edge case. It's way outside of the scope of this PR to fix, of course.
But the end goal with these field names is to have this data in the same fields, because the we want to allow for correlation across indices, and for things to be named as expected, and so on.
The fact that at this stage, the processor would override this is a very real problem that we'll need to work around, however. Not sure what the best course of action would be here. Modify Auditbeat to prevent this from happening? Or modify the processor to consider Auditbeat differently and never override the fields?
cc @ruflin
"working_directory": "/home/elastic" | ||
}, | ||
"service": { | ||
"type": "system" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also add this to a follow-up PR for mapping to ECS. But service.type
is meant to describe the name of a concrete application being monitored by the agent. The fact that this is a system monitoring event is already captured by event.module:system
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this comes from the Metricbeat framework we're using in Auditbeat. I think it's set for all of Metricbeat and Auditbeat.
I think we should address it separately and more broadly - I assume all other metricsets have an event.module
as well (e.g. event.module: mysql
) that already says what it's monitoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right about event.module
: #8879
"@timestamp": "2017-10-12T08:05:34.853Z", | ||
"agent": { | ||
"hostname": "host.example.com", | ||
"name": "host.example.com" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agent.type:auditbeat
, with agent.name
defaulting to the same value, or absent. I'd go for absent in the case of Auditbeat. This pattern around .name
needs to be better documented (see elastic/ecs#269).
agent.hostname
should instead be host.hostname
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are standard values that the testing framework puts in. The actual data has:
"agent": {
"hostname": "5efeb8ba7b09",
"version": "7.0.0",
"type": "auditbeat"
},
We should probably adjust the testing framework.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha.
This snippet you pasted is what live Auditbeat generates, though? If that's the case, agent.hostname
still needs to move to host.hostname
.
The definition of agent
is that it's a monitor that runs on the host, so there's no need for hostname there. This is in contrast to observer
, which is defined as a separate host or device that monitors a system from the outside.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. This is in master
today (introduced last month with #8873) and affects all Beats - we should address it separately.
I started a packaging test at If that’s taking too long to get a macos worker then you could also test it locally with
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn’t worry about the removed newline in auditbeat.yml. That’s not going to cause any problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reiterating my LGTM after recent discussion.
return filepath.Walk(pkgFile.Source, func(path string, info os.FileInfo, err error) error { | ||
// filepath.Walk() does not resolve symlinks, but pkgFile.Source might be one, | ||
// see mage.KibanaDashboards(). | ||
resolvedSource, err := filepath.EvalSymlinks(pkgFile.Source) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will prevent the usage of symlinks in packages. I’d rather have packages declare exactly want they include. There is a helper method to update the package manifest for build/kibana. Eventually we won’t need that at all because all beats will be the same and use build/kibana.
https://github.com/elastic/beats/blob/feature-auditbeat-host/dev-tools/mage/kibana.go#L95
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will prevent the usage of symlinks in packages.
Sorry, I don't follow. This was meant to allow PackageFile.Source
to be a symlink to a directory. This is never the case at the moment - when I changed it to create a symlink from _meta/kibana.generated
to build/kibana
the build failed and I had to make the change here to allow it. So I'm not sure how it prevents symlinks?
The problem is that make snapshot
depends on make beats-dashboards
(via make release
) which is implemented as mage packageBeatDashboards
which assumes all Beats have a _meta/kibana.generated
directory:
https://github.com/elastic/beats/blob/feature-auditbeat-host/magefile.go#L64
for _, beat := range Beats {
spec.Files[beat] = mage.PackageFile{
Source: filepath.Join(beat, "_meta/kibana.generated"),
}
}
return mage.PackageZip(spec.Evaluate())
This would have to be changed to take Beats specifics into account. E.g. Auditbeat produces a build/kibana
directory, while Filebeat still has _meta/kibana.generated
. I believe PackageKibanaDashboardsFromBuildDir()
just changes everything, regardless of what each Beat needs. So I'm not sure if we can currently do this without a bigger effort? Symlinks seemed like the simpler way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn’t realize this was solving for the beats dashboards zip case. (one the challenges of reviewing from a phone)
No one is adding symlinks via packages afaik (e.g. like installing a symlink on the system that is owned by an rpm/deb package) and I’m not sure that works anyways with our cade. So don’t consider this a problem. And once all Beats are writing dashboards to build/kibana this won’t be necessary anyways.
However this does make me wonder if dashboards from x-pack/auditbeat (or x-pack/filebeat) are being bundled in the beat dashboard zip. I suspect not, but that’s also not a problem for this PR to solve.
@andrewkroh I've started another job, but it ran out of disk space ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's one merge conflict (probably due to a Go version change).
Adds the system module to Auditbeat, with four metricsets: host, process, socket, and user. Host collects general host information, e.g. boottime, timezone, OS, network interfaces. Processes collects information about currently running, started, and stopped processes. Socket collects information about open sockets. User detects new users, deleted users, changes to users (e.g. groups), and - as a special distinct category - password changes.
Fixes a bug introduced with elastic#9546 where a symlink from `_meta/kibana.generated` to `build/kibana` would cause objects to be included in the dashboard ZIP file with the wrong path. This removes the symlink in favor of a conditional. Fixes elastic#9785.
Fixes a bug introduced with elastic#9546 where a symlink from `_meta/kibana.generated` to `build/kibana` would cause objects to be included in the dashboard ZIP file with the wrong path. This removes the symlink in favor of a conditional. Fixes elastic#9785. (cherry picked from commit 3a51720)
Fixes a bug introduced with elastic#9546 where a symlink from `_meta/kibana.generated` to `build/kibana` would cause objects to be included in the dashboard ZIP file with the wrong path. This removes the symlink in favor of a conditional. Fixes elastic#9785. (cherry picked from commit 23f1ea2)
Merges the
feature-auditbeat-host
branch intomaster
.Note: This is going off
cwurm:feature-auditbeat-host
simply so that I don't have to push any further commits directly into upstream.cwurm:feature-auditbeat-host
andelastic:feature-auditbeat-host
are exactly the same at this point in time.