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.

Sign up for GitHub

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

Jump to bottom

[Filebeat][Fortinet Module] Small bugfixes for time formats and IP address arrays #19316

Merged
merged 3 commits into from
Jun 24, 2020
Merged

[Filebeat][Fortinet Module] Small bugfixes for time formats and IP address arrays #19316

merged 3 commits into from
Jun 24, 2020

Conversation

P1llus
Copy link
Member

@P1llus P1llus commented Jun 22, 2020 •
edited
Loading

What does this PR do?

This PR resolves reported issues around the eventtime field being in both seconds and milliseconds and that the field ipaddr can also be an array.

Why is it important?

Resolves niche usecases that would otherwise throw an error

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.

Author's Checklist

Added new logs and reran nosetests to ensure it was fixed.

Related issues

@P1llus
added split for when dns is array, only fix eventtime if the time is …
c3406c7 
…not in seconds, added new timezone format option. New testlogs has been added
@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 Jun 22, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 22, 2020 •
edited
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #19316 updated]

  • Start Time: 2020-06-24T15:21:08.863+0000

  • Duration: 44 min 9 sec

Test stats 🧪

Test Results
Failed 0
Passed 2350
Skipped 382
Total 2732

leehinman
leehinman requested changes Jun 23, 2020
View reviewed changes
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.

could you add a changelog entry?

code changes look good.

@P1llus
Copy link
Member Author

P1llus commented Jun 23, 2020

Done @leehinman

@P1llus P1llus requested a review from leehinman June 23, 2020 13:16
@rosswakelin
Copy link

rosswakelin commented Jun 24, 2020 •
edited
Loading

This update does not fix all the date format issues. Using the following event:

<189>date=2020-06-24 time=01:16:08 devname="LAB-AH-BCN-FG01" devid="FG5H1E5818905592" logid="0000000013" type="traffic" subtype="forward" level="notice" vd="OPERATIONAL" eventtime=1592961368 srcip=10.38.160.201 srcport=60899 srcintf="ATM-GAMMA" srcintfrole="lan" dstip=10.38.34.3 dstport=161 dstintf="CORE-OPS" dstintfrole="lan" sessionid=1539390213 proto=17 action="deny" policyid=0 policytype="policy" service="SNMP" dstcountry="Reserved" srccountry="Reserved" trandisp="noop" duration=0 sentbyte=0 rcvdbyte=0 sentpkt=0 appcat="unscanned" crscore=30 craction=131072 crlevel="high"

results in this parsing error
Invalid ID for ZoneOffset, invalid format:

Fortigate version 6.0.5
This issue has existed since the release of this module, but this update does not fix it.
Note - the event has no TZ data - the firewall is set to use UTC.

@P1llus
Copy link
Member Author

P1llus commented Jun 24, 2020

Il take a look @rosswakelin . This specific case was not known to me, as we do handle non-tz events in the pipeline, though seems not in this case. I will add the fix soon and include it + add another log event similar to yours (though anonymized) to our unit tests.

@P1llus
Copy link
Member Author

P1llus commented Jun 24, 2020

@rosswakelin Much appreciated for the comment, it has now been resolved with the last commit and tests updated to catch anything similar later.

Since we do not know anything about the timezone we will have to default to something, currently that is UTC

@leehinman leehinman merged commit 39961ad into elastic:master Jun 24, 2020
@leehinman leehinman mentioned this pull request Jun 24, 2020
Merged
6 tasks
@leehinman leehinman mentioned this pull request Jun 24, 2020
Merged
6 tasks
leehinman pushed a commit to leehinman/beats that referenced this pull request Jun 25, 2020
@P1llus @leehinman
[Filebeat][Fortinet Module] Small bugfixes for time formats and IP ad…
dd37fc2 
…dress arrays (elastic#19316)

* added split for when dns is array
* only fix eventtime if the time is not in seconds
* added new timezone format option
* new testlogs has been added
* updated changelog
* fixing format set for events with non timezones

(cherry picked from commit 39961ad)
leehinman pushed a commit to leehinman/beats that referenced this pull request Jun 25, 2020
@P1llus @leehinman
[Filebeat][Fortinet Module] Small bugfixes for time formats and IP ad…
a091e6c 
…dress arrays (elastic#19316)

* added split for when dns is array
* only fix eventtime if the time is not in seconds
* added new timezone format option
* new testlogs has been added
* updated changelog
* fixing format set for events with non timezones

(cherry picked from commit 39961ad)
leehinman added a commit that referenced this pull request Jun 25, 2020
@leehinman @P1llus
[Filebeat][Fortinet Module] Small bugfixes for time formats and IP ad…
a4b9e89 
…dress arrays (#19316) (#19373)

* added split for when dns is array
* only fix eventtime if the time is not in seconds
* added new timezone format option
* new testlogs has been added
* updated changelog
* fixing format set for events with non timezones

(cherry picked from commit 39961ad)

Co-authored-by: Marius Iversen <pillus@chasenet.org>
leehinman added a commit that referenced this pull request Jun 25, 2020
@leehinman @P1llus
[Filebeat][Fortinet Module] Small bugfixes for time formats and IP ad…
9ec191e 
…dress arrays (#19316) (#19374)

* added split for when dns is array
* only fix eventtime if the time is not in seconds
* added new timezone format option
* new testlogs has been added
* updated changelog
* fixing format set for events with non timezones

(cherry picked from commit 39961ad)

Co-authored-by: Marius Iversen <pillus@chasenet.org>
@fredtj
Copy link

fredtj commented Jun 29, 2020

finding time errors in event.start,

i added UNIX alongside UNIX_MS to formats in pipeline.yml, this should now cover seconds and milliseconds?

@P1llus P1llus deleted the filebeat_fortigate_fix_19154_and_19010 branch June 30, 2020 12:15
@P1llus
Copy link
Member Author

P1llus commented Jun 30, 2020

Please create a new issue if you find anything @fredtj else it might be possible for any of us to miss it. I have added the update here: #19512

melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
@P1llus @melchiormoulin
[Filebeat][Fortinet Module] Small bugfixes for time formats and IP ad…
37a22cb 
…dress arrays (elastic#19316)

* added split for when dns is array
* only fix eventtime if the time is not in seconds
* added new timezone format option
* new testlogs has been added
* updated changelog
* fixing format set for events with non timezones
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leehinman leehinman leehinman approved these changes

Assignees
No one assigned
Labels
bug Filebeat Filebeat review v7.8.1 v7.9.0
Projects
None yet
Milestone
No milestone
5 participants
@P1llus @elasticmachine @rosswakelin @fredtj @leehinman