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

Audit and Authentication Policy Change Events #20684

Merged
merged 5 commits into from
Jan 25, 2021
Merged

Audit and Authentication Policy Change Events #20684

merged 5 commits into from
Jan 25, 2021

Conversation

janniten
Copy link
Contributor

@janniten janniten commented Aug 19, 2020

What does this PR do?

  • Adds support for Audit and Authentication Policy Change Events
Audit Audit Policy Change Audit Authentication Policy Change
https://docs.microsoft.com/en-us/windows/security/threat-protection/auditing/audit-audit-policy-change https://docs.microsoft.com/en-us/windows/security/threat-protection/auditing/audit-authentication-policy-change
4719(S): System audit policy was changed. 4670(S): Permissions on an object were changed
4817(S): Auditing settings on object were changed. 4706(S): A new trust was created to a domain.
4902(S): The Per-user audit policy table was created. 4707(S): A trust to a domain was removed.
4906(S): The CrashOnAuditFail value has changed. 4716(S): Trusted domain information was modified.
4907(S): Auditing settings on object were changed. 4713(S): Kerberos policy was changed.
4908(S): Special Groups Logon table modified. 4717(S): System security access was granted to an account.
4912(S): Per User Audit Policy was changed. 4718(S): System security access was removed from an account.
4904(S): An attempt was made to register a security event source. 4739(S): Domain Policy was changed.
4905(S): An attempt was made to unregister a security event source.  

Note: Although processing of Event 4715 (The audit policy (SACL) on an object was changed) seems to be identical to 4670, event 4715 was not included due I was not able to generate an example event.

For events where exists information of DACLs or SACLs those ACL are translated from the SDDL (https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-dtyp/f4296d69-1c0f-491f-9587-a960b292d070) to a human-readable from. For example:

image

  • Adds related.ip information in existing events where source.ip is present.

Why is it important?

  • Auditing the changes in policies and event sources is crucial when we want to have a strong security monitoring system. Monitor these kinds of events are also important when we address compliance (SOX, PCI. HIPAA, etc )

  • The related.ip information is useful when we want to pivot data between different sources. For example
    Fortinet Event (37141) indicating a user is connected to a VPN SSL when tunnelip is the asigned address. Tunnelip is also in the related.ip field
    Windows Event 4624 indicating a windows login from a source.ip. If we have source.ip in the related.ip it is easy to match the user connected through VPN with a windows logon

Checklist

  • [x ] My code follows the style guidelines of this project
  • [ x] I have commented my code, particularly in hard-to-understand areas
  • [ x] I have made corresponding changes to the documentation
  • [x ] I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@janniten janniten requested a review from a team as a code owner August 19, 2020 10:33
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

1 similar comment
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Aug 19, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 19, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: leehinman commented: run tests

    • Start Time: 2021-01-25T14:48:17.324+0000
  • Duration: 24 min 23 sec

  • Commit: f820bed

Test stats 🧪

Test Results
Failed 0
Passed 473
Skipped 6
Total 479

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 473
Skipped 6
Total 479

@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem (Team:SIEM)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Aug 19, 2020
@andrewkroh
Copy link
Member

jenkins, test this

@botelastic
Copy link

botelastic bot commented Sep 18, 2020

Hi!
We just realized that we haven't looked into this PR in a while. We're sorry!

We're labeling this issue as Stale to make it hit our filters and make sure we get back to it in as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1.
Thank you for your contribution!

@botelastic botelastic bot added the Stalled label Sep 18, 2020
@janniten
Copy link
Contributor Author

I confirm 👍

@botelastic botelastic bot removed the Stalled label Sep 18, 2020
@botelastic
Copy link

botelastic bot commented Oct 18, 2020

Hi!
We just realized that we haven't looked into this PR in a while. We're sorry!

We're labeling this issue as Stale to make it hit our filters and make sure we get back to it in as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1.
Thank you for your contribution!

@botelastic botelastic bot added the Stalled label Oct 18, 2020
@janniten
Copy link
Contributor Author

I still believe that those kind of events are important.
@andrewkroh what do you think?

Thank you

@botelastic botelastic bot removed the Stalled label Oct 21, 2020
@andrewkroh
Copy link
Member

💯 agree. Sorry it's taking so long to look these changes over.

@janniten
Copy link
Contributor Author

janniten commented Nov 18, 2020

@andrewkroh New ECS 1.7.0 event category configuration used for auditory configuration and changes of this PR and in events from older PRs

https://www.elastic.co/guide/en/ecs/current/ecs-allowed-values-event-category.html#ecs-event-category-configuration

@botelastic
Copy link

botelastic bot commented Dec 18, 2020

Hi!
We just realized that we haven't looked into this PR in a while. We're sorry!

We're labeling this issue as Stale to make it hit our filters and make sure we get back to it in as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1.
Thank you for your contribution!

@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@botelastic botelastic bot removed the Stalled label Dec 21, 2020
@janniten
Copy link
Contributor Author

Events related to audit configuration/changes are relevant when talking about defense evasion and also when you are under strong security regulations.
New ECS 1.7.0 event category configuration used for auditory configuration and changes of this PR and in events from older PRs

@janniten
Copy link
Contributor Author

janniten commented Jan 8, 2021

Pinging @elastic/security-external-integrations , @andrewkroh
Do you consider that this PR is still relevant? If not I can close it
Thank you!

@leehinman
Copy link
Contributor

@janniten do you mind if I rebase off master and fix some conflicts. There are a bunch in eventActionTypes, to support multiple values for event.category and event.type those are now arrays, so conflicts like:

"4697": [["iam", "configuration"], ["admin", "change"],"service-installed"],
vs
"4697": ["iam","admin","service-installed"]

@janniten
Copy link
Contributor Author

@janniten do you mind if I rebase off master and fix some conflicts. There are a bunch in eventActionTypes, to support multiple values for event.category and event.type those are now arrays, so conflicts like:

"4697": [["iam", "configuration"], ["admin", "change"],"service-installed"],
vs
"4697": ["iam","admin","service-installed"]

Hi @leehinman. No problem! Go ahead!
Thank you

@janniten
Copy link
Contributor Author

janniten commented Jan 19, 2021

@janniten do you mind if I rebase off master and fix some conflicts. There are a bunch in eventActionTypes, to support multiple values for event.category and event.type those are now arrays, so conflicts like:

"4697": [["iam", "configuration"], ["admin", "change"],"service-installed"],
vs
"4697": ["iam","admin","service-installed"]

Hi @leehinman. No problem! Go ahead!
Thank you

I use in array values un order to suppport múltiples categoriew/types, but It is added later in code the when process the event.
For example for event 4697

    var event4697 = new processor.Chain()
        .Add(copySubjectUser)
        .Add(copySubjectUserLogonId)
        .Add(renameCommonAuthFields)
        .Add(addServiceFields)
        .Add(addEventFields)
        .Add(function(evt) {
            evt.AppendTo("event.type", "admin");
        })
        .Build();

But it is much better to define It in the way you propose :)
Please let me know if I can help
Thank you!

@leehinman
Copy link
Contributor

@janniten I couldn't push to your repo so I uploaded my changes here https://github.com/leehinman/beats/tree/janniten_aa_policy_change

Can you take a quick look and see if that looks right? The intent was just address the eventActionTypes and to add golden files.

@janniten
Copy link
Contributor Author

@janniten I couldn't push to your repo so I uploaded my changes here https://github.com/leehinman/beats/tree/janniten_aa_policy_change

Can you take a quick look and see if that looks right? The intent was just address the eventActionTypes and to add golden files.

@leehinman , LGTM
I've noticed that there are some events defined in the eventActionTypes but not being processed (events 4616,4657 for example). Once finishished this PR I can add those ones.
Thank you!

@leehinman
Copy link
Contributor

run tests

@andrewkroh
Copy link
Member

The new event IDs from this PR will need added to the list in https://www.elastic.co/guide/en/beats/winlogbeat/master/winlogbeat-module-security.html#winlogbeat-module-security.

@janniten janniten deleted the aa_policy_change branch January 26, 2021 08:18
@janniten
Copy link
Contributor Author

The new event IDs from this PR will need added to the list in https://www.elastic.co/guide/en/beats/winlogbeat/master/winlogbeat-module-security.html#winlogbeat-module-security.

@andrewkroh , @leehinman Doc updated in #23674

v1v added a commit to v1v/beats that referenced this pull request Jan 26, 2021
…pack-when-oss-changes

* upstream/master:
  [DOCS] Add setup content to Kubernetes and Cloud Foundry docs (elastic#23580)
  [CI] Mandatory windows support for all the versions (elastic#23615)
  Add check when retrieving the worker process id using performance counters  (elastic#23647)
  Remove 4912 evtx from testing (elastic#23669)
  Add missing SSL settings (elastic#23632)
  Update X-Pack Packetbeat config (elastic#23666)
  Use hostname check from verify.go to handle patterns in TLS certs (elastic#23661)
  Fix: Dissect Cisco ASA 302013 message usernames (elastic#21196)
  Add FAQ entry for MADV settings in older versions (elastic#23429)
  Sync fixes from Integration Package Testing (elastic#23424)
  [Filebeat] Add Cisco ASA message '302023' parsing (elastic#23092)
  [Elastic Log Driver] Change hosts config flag (elastic#23628)
  Audit and Authentication Policy Change Events (elastic#20684)
leehinman pushed a commit to leehinman/beats that referenced this pull request Jan 27, 2021
* [Winlogbeat] Audit and Authentication Policy Change Events

Co-authored-by: Lee E. Hinman <[email protected]>
(cherry picked from commit dd7a1b3)
andrewkroh pushed a commit that referenced this pull request Feb 1, 2021
…nts (#23659)

* Audit and Authentication Policy Change Events (#20684)

* [Winlogbeat] Audit and Authentication Policy Change Events

Co-authored-by: Lee E. Hinman <[email protected]>
(cherry picked from commit dd7a1b3)

* Remove 4912 evtx from testing (#23669)

- causing failures on Win 7,8, 2008R2 & 2012R2

(cherry picked from commit d4e193d)

* Add Winlogbeat Security Module Doc (#23674)

* Add Winlogbeat Security Module Doc

* Update source file used to generate security module docs

(cherry picked from commit ee485bd)

Co-authored-by: Anabella Cristaldi <[email protected]>
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.

4 participants