output: Add direction for eve JSON logs

[StianHB]

• I have read the contributing guide lines at https://redmine.openinfosecfoundation.org/projects/suricata/wiki/Contributing
• I have signed the Open Information Security Foundation contribution agreement at https://suricata-ids.org/about/contribution-agreement/

Link to redmine ticket: #2644

Describe changes:

• Created a new field in EVE output declaring the direction of the flow ( TO_SERVER/TO_CLIENT )
  This makes it easier to understand what direction the event actually happend in, so you don't have to wonder if a request is to or from a service depending on ports.

Example:
"flow_direction": "to_client",
"pcap_cnt": 23,
"event_type": "dns",
"src_ip": "10.0.0.10",
"src_port": 53,
"dest_ip": "10.0.0.201",
"dest_port": 52697,

I saw from #3438 that you tried to do this for Alerts, but i think it should be part of every event.
I also disabled this for IPS mode as it might behave weird in IPS mode.

[StianHB]

After having mulled over this a bit I am inclined to change the "flow_direction" into a better name, just not sure what it should be ? event_direction maybe?

[victorjulien]

I agree the field could be very useful. My primary concern with it is that in the various Suricata use cases (IDS, IPS) and corner cases (flow timeout, early shutdown, etc) it may not always lead to consistent values. So I think for this feature to be accepted we need test cases covering each of those scenarios in https://github.com/OISF/suricata-verify

[fojac]

Hi Victor,
Sorry crossposting a little.
Got the work done by Stian patched on both 3.2 and master branch in production, as well as on brand 4.x new installs over the last day.
With libhtp installed and a ./configure --enable-hiredis --prefix=/usr/ --sysconfdir=/etc/ --localstatedir=/var/.
The field is empty only in cas of app_proto failed (flow timeout) or when tcp_state is closed, which seems OK to me.
I tested the master branch with ../suricata-verify-master/run.py > results.log, here is the result:

===> alert-testmyids: OK
===> alert-testmyids-not-established: OK
===> dnp3: OK
===> dnp3-dnp3_data-alert: OK
===> dnp3-dnp3_func-alert: OK
===> dns-eve: OK
===> dns-eve-v2-udp-dig-a-www-suricata-ids-org: OK
===> dns-json-log: OK
===> dns-lua-rules: SKIPPED: requires feature HAVE_LUA
===> dns-single-request: OK
===> dns-tcp-multirequest-buffer-1: OK
===> dns-tcp-ts-gap: OK
===> dns-tcp-www-google-com: OK
===> dns-udp-dig-a-www-suricata-ids-org: OK
===> dns-udp-dns-log-unanswered: OK
===> dns-udp-double-request-response: OK
===> dns-udp-eve-log-aaaa-only: OK
===> dns-udp-eve-log-answer-only: OK
===> dns-udp-eve-log-mx-only: OK
===> dns-udp-eve-log-query-only: OK
===> dns-udp-eve-log-txt: OK
===> dns-udp-nxdomain-soa: OK
===> dns-udp-unsolicited-response: OK
===> dns-udp-z-flag-fp: OK
===> eve-alert-metadata-defaults: OK
===> eve-alert-metadata-enable-rule: OK
===> eve-alert-metadata-off: OK
===> eve-metadata: OK
===> filestore-v2.1-forced: SKIPPED: requires feature HAVE_NSS
===> filestore-v2.2-forced-with-open-files: SKIPPED: requires feature HAVE_NSS
===> filestore-v2.3-fserror: SKIPPED: requires feature HAVE_NSS
===> filestore-v2.4-forced-with-meta: SKIPPED: requires feature HAVE_NSS
===> filestore-v2.5-both-enabled: SKIPPED: requires feature HAVE_NSS
===> http-xff-eve-forward-extra-data: OK
===> http-xff-eve-forward-overwrite: OK
===> http-xff-eve-reverse-extra-data: OK
===> http-xff-eve-reverse-overwrite: OK
===> http-xff-unified2: SKIPPED: requires script returned false
===> linktype-228: OK
===> lua-output-dns: SKIPPED: requires feature HAVE_LUA
===> lua-output-http: SKIPPED: requires feature HAVE_LUA
===> lua-output-smtp: SKIPPED: requires feature HAVE_LUA
===> output-eve-fileinfo: OK
===> output-pcap-log: OK
===> output-tcp-data: OK
===> proto-mismatch-http-ssh: OK
===> show-help: OK
===> smtp: OK
===> test-config-empty-rule-file: OK
===> tls: OK
===> tls-fingerprint-alert: OK
===> tls-json-output-ids: OK
===> tls-json-output-ips: OK

PASSED: 43
FAILED: 0
SKIPPED: 10

So this look fine to me, the feature gives great enhancement for my SIEM, especially for graphs and live maps, as well as for operators quick understanding of flows.
Please let me know if you need more testing.

[victorjulien]

What I was hoping for on the suricata-verify side, is a PR that adds tests for this feature and/or updates existing tests. That the existing tests have to pass is the default, but we need more tests to have confidence in this feature.

[victorjulien]

@glongo has offered to create a more comprehensive test set to validate all variations of tcp, udp, midstream, async, ids, ips.

[victorjulien]

@glongo you can take OISF/suricata-verify#25 as a starting point.

[victorjulien]

@glongo have you been able to take a look at the tests?

[victorjulien]

Please rebase and reopen with a comprehensive SV test PR as well.

[j0nny55555]

@victorjulien doing some looking around and so far, this is the most recent thing about direction detection in Suricata, did this get committed, what is required to enable its output?