Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

output/reference: Include reference information in alert (if configured) #11668

Closed
wants to merge 4 commits into from

Conversation

jlucovsky
Copy link
Contributor

@jlucovsky jlucovsky commented Aug 28, 2024

Continuation of #11656

When configured, include the reference value in the alert. The configuration value is in the alert section: types.alert.reference. The default value is off/no. Set to yes to include the expanded reference from the rule in the alert record.

Link to redmine ticket: 4974

Describe changes:

  • Add reference value to suricata.yaml.in (default no/off)
  • Set flag in output logger if the config setting is on
  • Format the reference as a sequence, e.g., references: [ "ref-1" [, "ref-2" [, ...]]]

Updates:

  • Removed unneeded parameters in output path
  • Unneeded BUG_ON checks when using reference key/values.
  • Rebase

Provide values to any of the below to override the defaults.

SV_BRANCH=OISF/suricata-verify#2022

jlucovsky and others added 4 commits August 24, 2024 09:16
Issue: 4974

Optionally include rule references with the alert. Since there can be
multiple reference keywords, they are collected into an array.
Issue: 4974

1. Use https instead of http everywhere
2. Organize and annotate references by
    - Referenced by ET/Open and ET/Pro
        - URL resolves and works as intended (to provide supplemental
          information regarding a reference value, e.g., bug id, cve
          value)
        - URL no longer resolves
        - URL resolves but doesn't work as intended (to provide
          supplemental information)
    - Not referenced by ET/Open nor ET/Pro
        - URL resolves and works as intended (to provide supplemental
          information regarding a reference value, e.g., bug id, cve
          value)
        - URL no longer resolves
        - URL resolves but doesn't work as intended (to provide
          supplemental information)
Issue: 4974

Remove unused parameters in output path for
- AlertJsonMetadata
- AlertJsonHeader
@jlucovsky jlucovsky changed the title 4974/19 output/reference: Include reference information in alert (if configured) Aug 28, 2024
Copy link
Contributor

@catenacyber catenacyber left a comment

Choose a reason for hiding this comment

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

Approved as #11656 (review)

git diff pr/11656 pr/11668 finds no diff, it is just the SV test that got updated, right @jlucovsky ?

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 22278

Copy link
Contributor

@jufajardini jufajardini left a comment

Choose a reason for hiding this comment

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

Looks like all past comments had been addressed, left one question to confirm one point, only.

Comment on lines +190 to +194
if (kv->reference_len >= kv->key_len && strncmp(kv->reference, kv->key, kv->key_len) == 0)
snprintf(kv_store, size_needed, "%s", kv->reference);
else
snprintf(kv_store, size_needed, "%s%s", kv->key, kv->reference);
jb_append_string(jb, kv_store);
Copy link
Contributor

Choose a reason for hiding this comment

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

While trying to confirm whether one of Jason's comments had been addressed, I noticed that we leave the sizes check for when the values are parsed. Is that being done here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's being done here is to account for many cases where the reference information includes the key. Usually, a reference is key,value --> url,www.google.com/some-link. Url is https:// so this reference is expanded to https://www.google.com/some-link

There are references that are incorrectly listed in a rules file such as url,https://www.google.com/some-link. The logic here detects that and avoids adding the url value if it's already present.

@jlucovsky
Copy link
Contributor Author

Approved as #11656 (review)

git diff pr/11656 pr/11668 finds no diff, it is just the SV test that got updated, right @jlucovsky ?

Yes, that and a rebase.

@jlucovsky
Copy link
Contributor Author

Continued in #11758

@jlucovsky jlucovsky closed this Sep 11, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants