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

output/alert: rewrite code for app-layer properties #9252

Closed

Conversation

catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/3827
preliminary work for https://redmine.openinfosecfoundation.org/issues/5053 and app-layer plugins
preliminary work for https://redmine.openinfosecfoundation.org/issues/5977 as well
Part of #8961

Describe changes:

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

After that, there is still from #8961

  • addition of protocols missing alert metadata (like krb5) + behavioral change for dns alert metadata
  • reusing these SimpleTxLogFunc from a JsonGenericLogger to remove many C files

Rebase of #9182

Especially fix setup-app-layer script to not forget this part

Ticket: OISF#3827
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 15329

@victorjulien victorjulien added this to the 8.0 milestone Jul 24, 2023
Comment on lines +211 to +216
typedef bool (*SimpleTxLogFunc)(void *, struct JsonBuilder *);

typedef struct AppLayerLogger {
AppProto proto;
SimpleTxLogFunc LogTx;
} AppLayerLogger;
Copy link
Member

Choose a reason for hiding this comment

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

From a code organization perspective, I'd like to see output.[ch] just deal with the core/raw logging registration, which JSON/EVE should not be a part of. So this looks a bit more like a JsonAppLayerLogger, and SimpleTxLogFunc looks more like a SimpleJsonTxLogFunc, or maybe simply JsonTxLogFunc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok for renaming.

SimpleJsonTxLogFunc does not cover all the cases (when the state is needed, to when some option flags are passed)

@catenacyber
Copy link
Contributor Author

Continued in #9499

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

Successfully merging this pull request may close these issues.

4 participants