Skip to content

Conversation

@janvi-elastic
Copy link
Contributor

What does this PR do?

  • Updated data collection logic for the log data stream.
  • Updated the ingest pipeline for the log data stream.
  • Mapped fields according to the ECS schema and added Fields metadata in the appropriate yml files.
  • Added dashboards and visualizations.
  • Updated test for pipeline for the log data stream.
  • Updated system test cases for the log data stream.

Integration release checklist

This checklist is intended for integrations maintainers to ensure consistency
when creating or updating a Package, Module or Dataset for an Integration.

All changes

  • Change follows the contributing guidelines
  • Supported versions of the monitoring target is documented
  • Supported operating systems are documented (if applicable)
  • Integration or System tests exist
  • Documentation exists
  • Fields follow ECS and naming conventions
  • At least a manual test with ES / Kibana / Agent has been performed.
  • Required Kibana version set to: ^8.3.0

New Package

  • Screenshot of the "Add Integration" page on Fleet added

Dashboards changes

  • Dashboards exists
  • Screenshots added or updated
  • Datastream filters added to visualizations

Log dataset changes

  • Pipeline tests exist (if applicable)
  • Generated output for at least 1 log file exists
  • Sample event (sample_event.json) exists

How to test this PR locally

  • Clone integrations repo.
  • Install elastic package locally.
  • Start elastic stack using elastic-package.
  • Move to integrations/packages/fortinet_fortimail directory.
  • Run the following command to run tests.

elastic-package test

Related issues

Screenshots

browser integration
Integration overview page
pipeline test
system test
static test
asset test

@elasticmachine
Copy link

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

@elasticmachine
Copy link

elasticmachine commented Mar 3, 2023

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2023-03-16T13:30:46.051+0000

  • Duration: 16 min 34 sec

Test stats 🧪

Test Results
Failed 0
Passed 17
Skipped 0
Total 17

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Mar 3, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 100.0% (7/7) 💚
Classes 100.0% (7/7) 💚
Methods 97.727% (43/44)
Lines 92.254% (917/994)
Conditionals 100.0% (0/0) 💚
- append:
field: error.message
value: '{{{_ingest.on_failure_message}}}'
- grok:
Copy link
Contributor

@efd6 efd6 Mar 6, 2023

Choose a reason for hiding this comment

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

This grok can be replaced with a dissect/grok/convert

 - dissect: field: _tmp.event.original if: ctx._tmp?.event?.original != null && ctx._tmp.event.original != '' pattern: '<%{fortinet_fortimail.log.priority_number}>date=%{fortinet_fortimail.log.date},time=%{fortinet_fortimail.log.time},device_id=%{fortinet_fortimail.log.device_id},log_id=%{fortinet_fortimail.log.id},type=%{fortinet_fortimail.log.type},%{_tmp.message_suffix}' on_failure: - append: field: error.message value: '{{{_ingest.on_failure_message}}}' - grok: field: _tmp.message_suffix if: ctx._tmp?.message_suffix != null && ctx._tmp.message_suffix != '' patterns: - '^%{PREFIX},%{GREEDYDATA:_tmp.message},subject=\"%{DATA:fortinet_fortimail.log.subject}\",%{GREEDYDATA:_tmp.extra_message},msg=\"%{GREEDYDATA:fortinet_fortimail.log.message}\"$' - '^%{PREFIX},%{GREEDYDATA:_tmp.message},subject=\"%{DATA:fortinet_fortimail.log.subject}\",msg=\"%{GREEDYDATA:fortinet_fortimail.log.message}\"$' - '^%{PREFIX},%{GREEDYDATA:_tmp.message},subject=\"%{DATA:fortinet_fortimail.log.subject}\",%{GREEDYDATA:_tmp.extra_message}$' - '^%{PREFIX},%{GREEDYDATA:_tmp.message},msg=\"%{GREEDYDATA:fortinet_fortimail.log.message}\"$' - '^%{PREFIX},msg=\"%{GREEDYDATA:fortinet_fortimail.log.message}\"$' - '^pri=%{DATA:fortinet_fortimail.log.priority},%{GREEDYDATA:_tmp.message}$' pattern_definitions: PREFIX: '(?:subtype=%{DATA:fortinet_fortimail.log.sub_type},)?pri=%{DATA:fortinet_fortimail.log.priority}' on_failure: - append: field: error.message value: '{{{_ingest.on_failure_message}}}' - convert: field: fortinet_fortimail.log.priority_number type: long ignore_missing: true ignore_failure: true 

Unfortunately it cannot be further reduced, otherwise greediness beats the preference ordering you have here.

@janvi-elastic janvi-elastic requested a review from efd6 March 7, 2023 13:22

The Fortinet FortiMail integration collects one type of data stream: log.

**Log** helps users to keep a record of many different email activities and traffic including system-related events, such as system restarts and HA activity, virus detections, spam filtering results, POP3, SMTP, IMAP, and webmail events. See more details [About FortiMail logging](https://docs.fortinet.com/document/fortimail/7.2.2/administration-guide/435158/about-fortimail-logging)
Copy link
Contributor

Choose a reason for hiding this comment

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

Either

Suggested change
**Log** helps users to keep a record of many different email activities and traffic including system-related events, such as system restarts and HA activity, virus detections, spam filtering results, POP3, SMTP, IMAP, and webmail events. See more details [About FortiMail logging](https://docs.fortinet.com/document/fortimail/7.2.2/administration-guide/435158/about-fortimail-logging)
**Log** helps users to keep a record of email activity and traffic including system-related events, such as system restarts and HA activity, virus detections, spam filtering results, POP3, SMTP, IMAP, and webmail events. See more details [About FortiMail logging](https://docs.fortinet.com/document/fortimail/7.2.2/administration-guide/435158/about-fortimail-logging)

or

Suggested change
**Log** helps users to keep a record of many different email activities and traffic including system-related events, such as system restarts and HA activity, virus detections, spam filtering results, POP3, SMTP, IMAP, and webmail events. See more details [About FortiMail logging](https://docs.fortinet.com/document/fortimail/7.2.2/administration-guide/435158/about-fortimail-logging)
**Log** helps users to keep a record of many different email activities and email traffic including system-related events, such as system restarts and HA activity, virus detections, spam filtering results, POP3, SMTP, IMAP, and webmail events. See more details [About FortiMail logging](https://docs.fortinet.com/document/fortimail/7.2.2/administration-guide/435158/about-fortimail-logging)

(Doesn't sound right to combine nouns with number and without in a noun phrase conjunction)

- kv:
field: _tmp.extra_message
target_field: fortinet_fortimail.log.log_details
field_split: \s*,\s*
Copy link
Contributor

Choose a reason for hiding this comment

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

This will split within values if there is a comma in the value.

Comment on lines 27 to 30
- gsub:
field: _tmp.event.original
pattern: ',,'
replacement: ','
Copy link
Contributor

Choose a reason for hiding this comment

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

This will mutate values if there is a ,, within a (quoted) value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @efd6 ,
We are analyzing the pipeline and optimizing it with a better solution using the KV processor, soon will update you on this.

Comment on lines 95 to 98
- gsub:
field: _tmp.extra_message
pattern: '"'
replacement: ''
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is safe to do. You can remove quotes after the KV processor.

Comment on lines 11 to 14
- '^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$'
- "^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg},%{SPACE}sent from:%{SPACE}\\'%{DATA:fortinet_fortimail.log.sent_from}\\',%{SPACE}subject:%{SPACE}\\'%{GREEDYDATA:_tmp.subject}\\'%{SPACE}%{GREEDYDATA:_tmp.msg2}$"
- "^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg},%{SPACE}sent from:%{SPACE}\\'%{DATA:fortinet_fortimail.log.sent_from}\\',%{SPACE}subject:%{SPACE}\\'%{GREEDYDATA:_tmp.subject}\\'$"
- '^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg}$'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- '^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$'
- "^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg},%{SPACE}sent from:%{SPACE}\\'%{DATA:fortinet_fortimail.log.sent_from}\\',%{SPACE}subject:%{SPACE}\\'%{GREEDYDATA:_tmp.subject}\\'%{SPACE}%{GREEDYDATA:_tmp.msg2}$"
- "^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg},%{SPACE}sent from:%{SPACE}\\'%{DATA:fortinet_fortimail.log.sent_from}\\',%{SPACE}subject:%{SPACE}\\'%{GREEDYDATA:_tmp.subject}\\'$"
- '^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg}$'
- '^%{DATA}[Uu]ser %{NOTSPACE:_tmp.user} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$'
- "^%{DATA}[Uu]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg},%{SPACE}sent from:%{SPACE}\\'%{DATA:fortinet_fortimail.log.sent_from}\\',%{SPACE}subject:%{SPACE}\\'%{GREEDYDATA:_tmp.subject}\\'(?:%{SPACE}%{GREEDYDATA:_tmp.msg2})?$"
- '^%{DATA}[Uu]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg}$'

(untested)
Could also consider using (?i) in place of [Uu] (not [U|u] which matches U, u and |), but this depends on the remainder of the expectations.

Comment on lines 47 to 49
- "^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$"
- "^%{DATA}[L|l]ogin for \\'%{NOTSPACE:_tmp.user}\\' %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$"
- "^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg}$"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- "^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$"
- "^%{DATA}[L|l]ogin for \\'%{NOTSPACE:_tmp.user}\\' %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$"
- "^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg}$"
- "^%{PREFIX} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$"
- "^%{DATA}[Uu]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg}$"
pattern_definitions:
PREFIX: "(?:%{DATA}[Uu]ser %{NOTSPACE:_tmp.user}|%{DATA}[Ll]ogin for \\'%{NOTSPACE:_tmp.user}\\')"

(untested)
Note also possibility of (?i).

Comment on lines 86 to 89
- '^%{DATA}[I|i]nterface %{NUMBER:fortinet_fortimail.log.port:long}%{DATA} [U|u]ser %{NOTSPACE:_tmp.user} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$'
- '^%{DATA}[I|i]nterface %{NUMBER:fortinet_fortimail.log.port:long}%{DATA} [U|u]ser %{NOTSPACE:_tmp.user}%{GREEDYDATA:_tmp.msg}$'
- '^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$'
- '^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg}$'
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar reductions and corrections.

Suggested change
- '^%{DATA}[I|i]nterface %{NUMBER:fortinet_fortimail.log.port:long}%{DATA} [U|u]ser %{NOTSPACE:_tmp.user} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$'
- '^%{DATA}[I|i]nterface %{NUMBER:fortinet_fortimail.log.port:long}%{DATA} [U|u]ser %{NOTSPACE:_tmp.user}%{GREEDYDATA:_tmp.msg}$'
- '^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$'
- '^%{DATA}[U|u]ser %{NOTSPACE:_tmp.user} %{GREEDYDATA:_tmp.msg}$'
- '^%{PREFIX} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$'
- '^%{PREFIX}%{GREEDYDATA:_tmp.msg}$'
- '^%{DATA}%{USER} %{DATA}%{IP:fortinet_fortimail.log.ip}%{GREEDYDATA:_tmp.msg}$'
- '^%{DATA}%{USER} %{GREEDYDATA:_tmp.msg}$'
pattern_definitions:
PREFIX: '%{DATA}[Ii]nterface %{NUMBER:fortinet_fortimail.log.port:long}%{DATA} %{USER}'
USER: '[Uu]ser %{NOTSPACE:_tmp.user}'

(untested)

- '^%{WORD}%{SPACE}\(%{SPACE}%{IP:fortinet_fortimail.log.ui_ip}%{SPACE}\)$'
- '^%{DATA}%{IP:fortinet_fortimail.log.ui_ip}%{GREEDYDATA:_tmp.msg}$'
pattern_definitions:
PROTOCOL: SSH|telnet|ssh|http|HTTP
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
PROTOCOL: SSH|telnet|ssh|http|HTTP
PROTOCOL: '(?i)(?:telnet|ssh|http)'
@janvi-elastic janvi-elastic requested a review from efd6 March 15, 2023 13:17
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

Thanks. Very minor test additions.

@@ -0,0 +1 @@
<190>date=2023-01-30,time=16:09:16.825,device_id=FEVM02TM23000064,log_id=0300003065,type=spam,subtype=default,pri=information,session_id="",client_name="",client_ip="",dst_ip="",from="",to="",subject="",msg="mailfilterd: Starting"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a test with non-empty values as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, will add test with non-empty values.

@@ -0,0 +1,2 @@
<190>date=2023-01-30,time=16:09:15.246,device_id=FEVM02TM23000064,log_id=0400003064,type=encrypt,pri=information,session_id="",msg="Starting cryptod"
<142>date=2023-01-30,time=16:09:15.246,device_id=FEVM02TM23000064,log_id=0400003064,type=encrypt,pri=information,session_id="",msg="User user1@1.ca read secure message, id:'q79EiV8S007017-q79EiV8T0070170001474', sent from: 'user2@2.ca', subject: 'ppt file'"
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a test with a non-empty session_id.

<191>date=2023-02-06,time=18:06:10.119,device_id=FEVM02TM23000064,log_id=0002002810,type=event,subtype=imap,pri=debug, user=hello,ui=mail,action=none,status=success,session_id='',msg="mailbox Inbox[/var/spool/home/internalsmtp.example.local/timesheet~internalsmtp.example.local/mail/Inbox] is updated"
<191>date=2023-02-06,time=18:28:49.954,device_id=FEVM02TM23000064,log_id=0002003307,type=event,subtype=imap,pri=debug, user=mail,ui=mail,action=none,status=success,session_id='',msg="mailbox dead.letter[/var/spool/dead/dead.letter] is updated"
<182>date=2023-02-01,time=14:42:35.521,device_id=FEVM02TM23000064,log_id=0000004943,type=event,subtype=webmail,pri=information, user=webmail,ui=webmail,action=unknown,status=failure,msg="WebMail: Login for 'timesheet@internalsmtp.example.local' from 10.212.128.53 failed"
<190>date=2023-01-30,time=16:06:24.345,device_id=FEVM02TM23000064,log_id=0003002428,type=event,subtype=smtp,pri=information, user=mail,ui=mail,action=NONE,status=N/A,session_id='',msg="starting daemon: persistent-queueing@00:05:00"
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a non-empty session_id.

@janvi-elastic janvi-elastic requested a review from efd6 March 16, 2023 13:31
- append:
field: error.message
value: '{{{_ingest.on_failure_message}}}'
value: 'Processor {{{_ingest.on_failure_processor_type}}} with tag {{{_ingest.on_failure_processor_tag}}} in pipeline {{{_ingest.pipeline}}} failed with message: {{{_ingest.on_failure_message}}}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this to include the terser fail-{{{_ingest.on_failure_processor_tag}}} value as well? (I don't have any strong opinion on this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @efd6 ,
Instead we have added the tag name with mentioned keyword "Failed" to error.message , So we don't think we should repeat it again. Sill if you feel so let us know will update the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can add it later if needed. The reason I was thinking it would be useful is that it becomes an exact match search for finding how many times any particular processor has failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, we will add in this if needed. Now onwards following same for all the up coming integration.

@P1llus P1llus merged commit 9dc1fdd into elastic:main Mar 21, 2023
@elasticmachine
Copy link

Package fortinet_fortimail - 2.0.0 containing this change is available at https://epr.elastic.co/search?package=fortinet_fortimail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

5 participants