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

doc: add multi buffer matching documentation v4 #9180

Closed

Conversation

jmtaylor90
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/6032

Describe changes:

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

To use a pull request use a branch name like pr/N where N is the
pull request number.

Alternatively, SV_BRANCH may also be a link to an
OISF/suricata-verify pull-request.

SV_REPO=
SV_BRANCH=
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

Signed-off-by: jason taylor <jtfas90@gmail.com>
Signed-off-by: jason taylor <jtfas90@gmail.com>
@catenacyber catenacyber added the typo/doc update No code change : only doc or typo fixes label Jul 7, 2023
@@ -18,6 +18,8 @@ Example::

filename:"secret";

``file.name`` supports multiple buffer matching, see :doc:`multi-buffer-matching`.
Copy link
Contributor

Choose a reason for hiding this comment

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

How did you list all these ? (Did you run a git grep ?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How did you list all these ? (Did you run a git grep ?)

I looked at the commit history for the patch set, though now I realize I would have missed any later commits. I went back and looked again and did miss something. So I will have an additional commit to this PR for review.

I do have a question about:
src/detect-quic-cyu-hash.c: DetectBufferTypeSupportsMultiInstance(BUFFER_NAME);
src/detect-quic-cyu-string.c: DetectBufferTypeSupportsMultiInstance(BUFFER_NAME);

Should () contain the respective quic buffer names instead of BUFFER_NAME?

Copy link
Contributor

Choose a reason for hiding this comment

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

There must be a macro #define BUFFER_NAME a few lines up...

Signed-off-by: jason taylor <jtfas90@gmail.com>
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.

Explanation looks way more complete now, thanks for that! :)

Noticed one thing that needs fixing, and one nit aspect.


.. container:: example-rule

`alert http2 any any -> any any (msg:"HTTP2 Multiple Header Buffer Example"; flow:established,to_server; http.request_header; content:"method|3a 20|GET"; http.request_header; content:"authority|3a 20|example.com"; classtype:misc-activity; sid:1; rev:1;)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: rule is missing closing `

Comment on lines +54 to +55
**Note:** This is new behavior, in versions of Suricata prior to
version 7 multiple statements of the same sticky buffer did not
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
**Note:** This is new behavior, in versions of Suricata prior to
version 7 multiple statements of the same sticky buffer did not
**Note:** This is new behavior. In versions of Suricata prior to
version 7, multiple statements of the same sticky buffer did not

@codecov
Copy link

codecov bot commented Jul 8, 2023

Codecov Report

Merging #9180 (b3c70bf) into master (4ccc9ae) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9180      +/-   ##
==========================================
- Coverage   82.34%   82.34%   -0.01%     
==========================================
  Files         968      968              
  Lines      273546   273546              
==========================================
- Hits       225258   225246      -12     
- Misses      48288    48300      +12     
Flag Coverage Δ
fuzzcorpus 64.60% <ø> (ø)
suricata-verify 60.71% <ø> (-0.01%) ⬇️
unittests 62.90% <ø> (-0.01%) ⬇️

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

@jmtaylor90
Copy link
Contributor Author

continued in #9199

@jmtaylor90 jmtaylor90 closed this Jul 10, 2023
@jmtaylor90 jmtaylor90 deleted the doc-multi-buffer-matching-v4 branch July 13, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typo/doc update No code change : only doc or typo fixes
3 participants