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

[Filebeat][New Module] Adding support for Oracle Audit logs #21991

Merged
merged 8 commits into from
Nov 11, 2020

Conversation

P1llus
Copy link
Member

@P1llus P1llus commented Oct 19, 2020

What does this PR do?

This PR is the initial release of a Oracle module, with a audit log fileset.

Why is it important?

Adding support for more OOTB integrations

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • 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.

@P1llus P1llus added Filebeat Filebeat Team:SIEM needs_reviewer PR needs to be assigned a reviewer labels Oct 19, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem (Team:SIEM)

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

elasticmachine commented Oct 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: [Pull request #21991 updated]

  • Start Time: 2020-10-29T19:26:18.939+0000

  • Duration: 51 min 37 sec

Test stats 🧪

Test Results
Failed 0
Passed 4493
Skipped 561
Total 5054

@andresrc andresrc added the Team:Services (Deprecated) Label for the former Integrations-Services team label Oct 20, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-services (Team:Services)

@andresrc
Copy link
Contributor

ping @sorantis @sayden

Copy link
Contributor

@leehinman leehinman left a comment

Choose a reason for hiding this comment

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

Looking good.

Do you think we can add ECS categorization fields?

…d fixing some null value fields not being removed
@P1llus
Copy link
Member Author

P1llus commented Oct 28, 2020

@leehinman
For timezones I removed the default filebeat ones (removing add_locale processor), and I added a grok parser to grab the correct timezone since the date processor does not add timezone for some reason. If I don't set a timezone then event.timezone will stay empty, but it will be translated to UTC correctly.

For ECS Categorization I added event.type, event.cateogry, event.outcome and event.action. At this moment they really need to be static as there is no good way to differentiate the intent (is it a change, access, deletion etc).

The rest of the comments should also have been resolved with minor tweaks of the pipeline :)

@P1llus P1llus requested a review from leehinman October 28, 2020 21:21
@elasticmachine
Copy link
Collaborator

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 4493
Skipped 561
Total 5054

Copy link
Contributor

@leehinman leehinman left a comment

Choose a reason for hiding this comment

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

Changes LGTM

might want to set event.kind = event

Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

I'd change all database_audit mentions to audit_logs for example or simply audit. Maybe it's just me but the word database there feels redundant Maybe I'm missing that there are more type of audit logs and we are specifying which ones are those? Like the database_audit but you also have the server_audit and the user_audit? I don't know

@P1llus
Copy link
Member Author

P1llus commented Oct 29, 2020

I'd change all database_audit mentions to audit_logs for example or simply audit. Maybe it's just me but the word database there feels redundant Maybe I'm missing that there are more type of audit logs and we are specifying which ones are those? Like the database_audit but you also have the server_audit and the user_audit? I don't know

Much appreciated feedback, thanks @sayden ! 👍

I think I agree with most of the field name changes, I was unsure if we wanted to keep the original names or not, because once we start renaming fields we have to ensure that any new fields that comes up at a later date does not conflict with the current ones.

On the topic of database_audit, we unfortunately have to keep some sort of reference to database here, as there is sooo many Oracle products, and a large amount of their portfolio has audit functionality, so we need to differentiate somewhat between products here I would say?

@sayden
Copy link
Contributor

sayden commented Oct 29, 2020

The convention on Beats is to prettify everything with some exceptions in some specific fields (none that comes to my memory right now). It's not that I agree a lot, I think I'd prefer to maintain original names but this "prettifycation" is what you'll find in most Beats modules, so it's good for Beats users to maintain that familiarity between modules.

About the database_audit topic: sure! if you feel it's makes things more clear with the rest of oracle products that's great and thank you very much for the clarification and explanation :)

…syslog for now, as it won't be possible to use at this moment
@P1llus P1llus requested a review from sayden October 29, 2020 21:40
@P1llus
Copy link
Member Author

P1llus commented Oct 29, 2020

Should hopefully be good now @sayden . Always open for more changes if needed 👍

@sayden
Copy link
Contributor

sayden commented Nov 5, 2020

Good work with the module!

@leehinman leehinman merged commit 8af7145 into elastic:master Nov 11, 2020
@P1llus P1llus deleted the filebeat_oracle_audit branch November 11, 2020 20:56
leehinman pushed a commit to leehinman/beats that referenced this pull request Nov 16, 2020
…21991)

Oracle module, with a audit log fileset

(cherry picked from commit 8af7145)
leehinman added a commit that referenced this pull request Nov 19, 2020
…22556)

Oracle module, with a audit log fileset

(cherry picked from commit 8af7145)

Co-authored-by: Marius Iversen <[email protected]>
@elasticmachine
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Filebeat Filebeat needs_reviewer PR needs to be assigned a reviewer Team:Services (Deprecated) Label for the former Integrations-Services team v7.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants