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

filestore v2: print sid in json output (Optimization #2530) #3600

Closed
wants to merge 1 commit into from

Conversation

klingerko
Copy link
Contributor

Make sure these boxes are signed before submitting your Pull Request -- thank you.

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/2530
Describe changes:

  • print SID (signature id) of a triggering rule in json fileinfo file
  • default sid = 0
  • if more than one sid triggers for the same file only the first sid will be printed

PRScript output (if applicable):

@klingerko klingerko requested a review from a team as a code owner December 25, 2018 16:59
Copy link
Contributor

@regit regit left a comment

Choose a reason for hiding this comment

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

The case of multiple signatures triggering filestore is not taken into account.
Also please put the reference to the redmine issue in the body of the commit message and not in the subject.

@@ -264,6 +264,7 @@ static int DetectFilestoreMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx,
* matches. */
if (file != NULL) {
file_id = file->file_store_id;
file->sid = s->id;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is happening if we have multiple signatures triggering the filestore ?

@@ -149,6 +149,9 @@ json_t *JsonBuildFileInfoRecord(const Packet *p, const File *ff,
json_object_set_new(fjs, "filename", SCJsonString(s));
if (s != NULL)
SCFree(s);

json_object_set_new(fjs, "sid", json_integer(ff->sid));
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 like the idea of having a 0 here if the signature is not defined. It is better not to set the variable at all.

@klingerko
Copy link
Contributor Author

klingerko commented Jan 7, 2019

Thank you Eric for your feedback, I made a new pull request in #3608

@klingerko klingerko deleted the filestore_v2_sid_in_json branch January 7, 2019 10:29
victorjulien added a commit to victorjulien/suricata that referenced this pull request Apr 28, 2020
victorjulien added a commit to victorjulien/suricata that referenced this pull request Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants