Skip to content

[Auditbeat] Add user metricset#8835

Merged
cwurm merged 31 commits intoelastic:feature-auditbeat-hostfrom
cwurm:users_host_metricset
Nov 16, 2018
Merged

[Auditbeat] Add user metricset#8835
cwurm merged 31 commits intoelastic:feature-auditbeat-hostfrom
cwurm:users_host_metricset

Conversation

@cwurm
Copy link
Copy Markdown
Contributor

@cwurm cwurm commented Oct 30, 2018

Adds a list of users with all their information to the host metricset. Works on Linux and macOS by using getpwent(3).

Copy link
Copy Markdown
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

My suggestion would be to add a separate metricset for host_user which sends an event for each user.

@andrewkroh FYI

@cwurm cwurm added in progress Pull request is currently in progress. and removed review labels Nov 6, 2018
@cwurm
Copy link
Copy Markdown
Contributor Author

cwurm commented Nov 6, 2018

Thanks, @andrewkroh and @ruflin. I've (hopefully) addressed all comments for now. This is now its own metricset, sending each user as its own document, and detecting added/removed/changed users.

I'm still working on adding persistence across Auditbeat restarts, and we have some outstanding naming discussions (I put event.type / event.type_detail for now, but those are TBD).

So I'm keeping the PR in in progress for now, and you don't need to do another review at this point.

@cwurm cwurm changed the title [Auditbeat] Add user information to host metricset [Auditbeat] Add user metricset Nov 6, 2018
@cwurm
Copy link
Copy Markdown
Contributor Author

cwurm commented Nov 12, 2018

I think this is ready for another review.

The user metricset now does everything that it should do. It reads (via C functions) /etc/passwd, /etc/shadow, and /etc/group to collect all relevant information about a user.

It can detect new users, deleted users, changes to users (e.g. groups), and - as a special distinct category - password changes.

Periodically, it sends all user information as state (frequency controlled by the config values state.period and user.state.period - default 12h). Otherwise, it periodically (controlled by the usual period config - very low values are possible and recommended since when no changes have occurred it only triggers three stat syscalls) checks the ctime of the above files, reads them if the ctime has changed, detects changes compared to its internal cache, and reports 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 all current user information incl. a 10-character excerpt of the password hash from /etc/shadow (necessary to detect any password changes incl. a direct edit of /etc/shadow - however, that excerpt is not output to ES or any other output).

Possible future enhancements:

  • Use inotify instead of periodic stat calls to detect possible changes.
  • Read /etc/sudoers to add sudo information to users.

@cwurm cwurm added review and removed in progress Pull request is currently in progress. labels Nov 12, 2018
Copy link
Copy Markdown
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

LGTM overall.

I notice the metricset still needs to be changed to system_audit, but other than that, looking good. I've left a few comments here and there.

Other major thing is about using user.id to save the uid (see comment about this for the full discussion)

Copy link
Copy Markdown
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

Overall this LGTM. I left a few minor comments on the latest updates.

@cwurm cwurm force-pushed the users_host_metricset branch from b1f67a1 to 0681300 Compare November 14, 2018 13:43
@cwurm
Copy link
Copy Markdown
Contributor Author

cwurm commented Nov 14, 2018

After a conversation with @andrewkroh I changed the password change detection to use a SHA-512 hash of the full encrypted password (rather than the last 10 characters) - but excluding the salt. This should make sure that we detect any possible password change.

I merged master into the feature branch and rebased this PR, so the commits are now displayed all at the end. There are two commits since the last review: Switch to new UUID library. and
Use full encrypted password for change detection, store as SHA-512 hash.
.

@andrewkroh let me know if this still looks good so I can merge.

@cwurm cwurm merged commit 6c422a4 into elastic:feature-auditbeat-host Nov 16, 2018
cwurm pushed a commit to cwurm/beats that referenced this pull request Dec 16, 2018
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants