Output alert applayer v1

[catenacyber]

Link to redmine ticket:
None, preliminary work for https://redmine.openinfosecfoundation.org/issues/5053 and app-layer plugins
Continuation of #8772

Describe changes:

• Fix setup-app-layer script so that it adds app-layer metadata to alerts
• Adds ftp metadata to alerts
• Adds tftp metadata to alerts

SV_BRANCH=pr/1196

[catenacyber]

Questions about this draft :

• Should we unify logging functions so that they use state ? and configuration flags ?
  Looks like some protocols could get rid of state in their logging function : snmp, rfb, krb5
• What about cases where we want to log multiple objects ? (such as smtp and email)

TODOs : cf output.c

There are more protocols which currently miss this (beyond template, ftp and tftp being fixed here) : pgsql, dcerpc, dhcp, krb5

[suricata-qa]

ERROR:

ERROR: QA failed on ASAN_TLPR1_suri.

Pipeline 13735

[catenacyber]

There are more protocols which currently miss this (beyond template, ftp and tftp being fixed here) : pgsql, dcerpc, dhcp, krb5

I guess tickets should be created for these (including ftp and tftp)

[suricata-qa]

ERROR:

ERROR: QA failed on ASAN_TLPR1_suri.

Pipeline 13737

[catenacyber]

@jasonish what do you think about the Check Rust failure ?

[codecov]

Codecov Report

Merging #8864 (33e61d2) into master (f751c93) will decrease coverage by 0.01%.
The diff coverage is 94.64%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8864      +/-   ##
==========================================
- Coverage   82.28%   82.28%   -0.01%     
==========================================
  Files         969      969              
  Lines      273210   273114      -96     
==========================================
- Hits       224807   224720      -87     
+ Misses      48403    48394       -9     

Flag	Coverage Δ
fuzzcorpus	64.56% <87.50%> (+<0.01%)	⬆️
suricata-verify	60.31% <94.64%> (-0.04%)	⬇️
unittests	62.99% <0.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[suricata-qa]

WARNING:

field	baseline	test	%
	SURI_TLPW1_stats_chk
.tcp.rst	113274	144861	127.89%

Pipeline 13738

[catenacyber]

Also, one next step is to have JsonGenericLogger to replace all JsonBitTorrentDHTLogger (and other), and get rid of output-json-bittorrent-dht.c

Only difficulty there is to know if we have TCP or UDP or both for OutputGenericLogInitSub

[jasonish]

@jasonish what do you think about the Check Rust failure ?

Looks like GitHub changed the format the code is checked out, in such a way we can no longer use git tools like git diff.

[catenacyber]

Looks like GitHub changed the format the code is checked out, in such a way we can no longer use git tools like git diff.

Ok, but I also do not get anything locally running cargo clippy --all-features --fix --allow-no-vcs

And this PR is only removing some lines of rust code...

[jasonish]

Fix: #8866

[jasonish]

• What about cases where we want to log multiple objects ? (such as smtp and email)

By this do you mean that there is not a 1:1 relation between protocol and name and objects logged?

[catenacyber]

By this do you mean that there is not a 1:1 relation between protocol and name and objects logged?

Yes there is.

I mean to ask : should these cases be handled in the generic fashion proposed in this PR ? Or just have some separate logic..

[jasonish]

I mean to ask : should these cases be handled in the generic fashion proposed in this PR ? Or just have some separate logic..

I think some separate logic may make the initial versions of this PR easier to reason about? We're shooting for post 7.0 stuff here right?

[catenacyber]

We're shooting for post 7.0 stuff here right?

Not necessarily : this is code simplification, a fix for the script to setup an app layer, and it is leading to adding missing features (adding ftp metadata to alerts)
Do not you think it can be merged already ? (after some cleanups in this PR)

[catenacyber]

Need to rebase after #8867 to get CI green

[catenacyber]

Replaced by #8872

[jasonish]

Do not you think it can be merged already ? (after some cleanups in this PR)

Perhaps, I wasn't looking at it in that frame of mind as the issue is targetted for 8.0.