[WIP][Auditbeat] auditd ECS categorization update by andrewstucki · Pull Request #18028 · elastic/beats

andrewstucki · 2020-04-27T20:50:26Z

What does this PR do?

Putting this up here to get some eyes on it. This is some of the work around initial categorization changes to support ECS 1.5 for the auditd module. Some things to note:

We need to continue shipping invalid ECS categorization fields as part of the array of event.category and event.type values until 8.x so we don't make a breaking change.
It appears like we might have some misclassification of event categories aucoalesce.AuditEventType based off of the mapping code here, and the corresponding header values from the linux kernel/audit userspace headers. Namely: AUDIT_USER_CHAUTHTOK, AUDIT_ROLE_ASSIGN, and AUDIT_ROLE_REMOVE. This could be a bug in the original code that was ported for the go-libaudit library, but I'm wondering if it's worth fixing as a bug on our side.
I tried to expand the aucoalesce.AuditEventType values to explicitly enumerate what auditd types each encompassed and copied the comments from the headers documenting what each value is. This isn't all-inclusive of everything that we'd expect to be picked up, but should be informative for what sort of events we're actually working with for categorization purposes.
I threw some commented proposed categorization fields in the yml file that we can talk about adopting into the ECS spec if we deem them useful. Specifically: config, device, policy, anomaly, and check.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

Relates [Auditbeat] Update OSS Audibeat auditd module to ECS 1.5 #17519

elasticmachine · 2020-04-27T20:50:47Z

Pinging @elastic/siem (Team:SIEM)

andrewkroh · 2020-04-27T23:32:56Z

I haven't taken a deep look at this yet, but my initial thought is that maybe this will be easier to maintain over the long run if the data from go-libaudit came with all of this information. This way all the logic and mappings are in one place.

andrewstucki · 2020-04-28T15:44:50Z

@andrewkroh agree that things would be a bit cleaner if we put some of this in go-libaudit--main concern there is whether or not we're introducing too much ECS-specific domain knowledge to what seems to otherwise be a general-purpose library.

webmat

I think @MikePaquette and @dainperkins should look at ecs_categorization.yml as well.

I've answered a few questions in the comments, I'm challenging a few categorizations & so on. But this is looking great!

Do you think this will be portable to the Filebeat auditd module as well?

webmat · 2020-04-28T20:24:29Z

auditbeat/module/auditd/audit_linux_test.go

+			"event.category": []string{"authentication", "user-login"},
+			"event.type":     []string{"start", "authentication_failure"},


Are these second ones (user-login and auth_failure) marked as deprecated for deletion in 8.0?

webmat · 2020-04-28T20:26:23Z

auditbeat/module/auditd/ecs_categorization.yml

+    # AUDIT_ACCT_LOCK - User's account locked by admin
+    # AUDIT_ACCT_UNLOCK - User's account unlocked by admin


I'm thinking these two should warrant an event.type of "change", wdyt?

webmat · 2020-04-28T20:27:22Z

auditbeat/module/auditd/ecs_categorization.yml

+    default_types:
+      - info


Does event.type: info get added to all of these IAM events? Or is this a fallback that's used only when there's no entry under types: below?

webmat · 2020-04-28T20:29:24Z

auditbeat/module/auditd/ecs_categorization.yml

+    # AUDIT_CRED_DISP - User credential disposed
+    # AUDIT_USER_START - User session start
+    # AUDIT_USER_END - User session end
+    # AUDIT_USER_CHAUTHTOK - User acct password or pin changed // should this be classified EventTypeUserAccount?


should this be classified EventTypeUserAccount?

I would say yes to that.

webmat · 2020-04-28T20:38:24Z

auditbeat/module/auditd/ecs_categorization.yml

+    # AUDIT_USER_START - User session start
+    # AUDIT_USER_END - User session end


Been a long time since we worked on this. But I remember we wanted to treat sessions and authorizations differently than authentications. In other words, there's no category published for it yet.

Am I remembering this right, @dainperkins and @MikePaquette?

webmat · 2020-04-28T20:44:12Z

auditbeat/module/auditd/ecs_categorization.yml

+    # AUDIT_CONFIG_CHANGE - Audit system configuration change
+    # AUDIT_NETFILTER_CFG - Netfilter chain modifications
+    # AUDIT_FEATURE_CHANGE - audit log listing feature changes
+    # AUDIT_REPLACE - Replace auditd if this packet unanswerd


Suggested change

# AUDIT_REPLACE - Replace auditd if this packet unanswerd

# AUDIT_REPLACE - Replace auditd if this packet unanswered

webmat · 2020-04-28T20:44:51Z

auditbeat/module/auditd/ecs_categorization.yml

+    # AUDIT_FEATURE_CHANGE - audit log listing feature changes
+    # AUDIT_REPLACE - Replace auditd if this packet unanswerd
+    categories:
+    #   - config


It's fine not to assign a category for now, until we define a more appropriate one 👍

webmat · 2020-04-28T20:45:19Z

auditbeat/module/auditd/ecs_categorization.yml

+    categories:
+    #     - policy
+
+  EventTypeAnomoly:


Suggested change

EventTypeAnomoly:

EventTypeAnomaly:

Some of the anomalies below sound like they could be associated with existing categories, such as "iam", "file", "process", "authentication" and so on.

Then if we later add an "anomaly" category, these events could also be assigned that category.

yeah, that was my thought too--some of them are policy violations (like exceeding some threshold of failed logins), some are device access/modifications, and some are restricted process spawning, file or iam creation. So I'd be happy with categorizing them there.

Wondering though if it makes sense to add a secondary category in these cases--they're only being signaled because auditd has flagged them as "anomalous" - so they aren't your standard file/creation events, etc.?

ah, just re-read your comment, you were thinking the exact same thing as me :)

Yeah but it still brings a good point.

If this is a second "anomaly" event, will this mean this anomalous IAM action yield two auditd events? One for the regular IAM event, and this second one for the anomaly?

But I'd err on the side of capturing both as IAM, and then we / the user can dedupe if that's the case.

Moreover, if a user turns off "normal" IAM events to reduce noise, this IAM anomaly should be categorized as IAM.

webmat · 2020-04-28T20:49:38Z

auditbeat/module/auditd/ecs_categorization.yml

+      - process
+    types:
+      - end
+  # are these mis-classified?


are these mis-classified?

I think the whole list of associations in this messageTypes structure looks good

webmat · 2020-04-28T20:52:22Z

auditbeat/module/auditd/mk_ecs_categorization.go

+	MessageTypes map[string]messageType `yaml:"messageTypes"`
+}
+
+const fileTemplate = `


Meta-programming in Golang 🤯 😂

andrewstucki · 2020-05-05T18:18:55Z

@webmat thanks for the reviews/ideas, going to close this in favor of adding this stuff to go-libaudit for the next release as I discussed offline with @andrewkroh but I'll take the comments over there, reference back here and ping you when the work is done on that side.

Andrew Stucki added 5 commits April 27, 2020 13:11

Add categorization generator

6bed163

update test

71ceb1d

update generator

0ce5325

Update mappings

5231c24

update yaml

49f7562

andrewstucki requested a review from a team as a code owner April 27, 2020 20:50

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 27, 2020

andrewstucki added Auditbeat enhancement Team:SIEM and removed needs_team Indicates that the issue/PR needs a Team:* label labels Apr 27, 2020

andrewstucki self-assigned this Apr 27, 2020

andrewstucki requested review from andrewkroh, leehinman and webmat April 27, 2020 20:51

normalize whitespace

cb7747c

webmat reviewed Apr 28, 2020

View reviewed changes

andrewstucki closed this May 5, 2020

This was referenced May 15, 2020

[Auditbeat] Update auditbeat ECS mappings #18596

Merged

Cherry-pick #18596 to 7.x: [Auditbeat] Update auditbeat ECS mappings #18598

Merged

		"event.category": []string{"authentication", "user-login"},
		"event.type": []string{"start", "authentication_failure"},

		# AUDIT_ACCT_LOCK - User's account locked by admin
		# AUDIT_ACCT_UNLOCK - User's account unlocked by admin

		# AUDIT_USER_START - User session start
		# AUDIT_USER_END - User session end

	# AUDIT_REPLACE - Replace auditd if this packet unanswerd
	# AUDIT_REPLACE - Replace auditd if this packet unanswered

Conversation

andrewstucki commented Apr 27, 2020

What does this PR do?

Checklist

Related issues

Uh oh!

elasticmachine commented Apr 27, 2020

Uh oh!

andrewkroh commented Apr 27, 2020

Uh oh!

andrewstucki commented Apr 28, 2020

Uh oh!

webmat left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewstucki commented May 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants