Skip to content

detect/analyzer: add more details for the tcp window keyword-v1#12021

Closed
Nancyenos wants to merge 1 commit intoOISF:masterfrom
Nancyenos:detect/analyzer-add-more-details-for-the-tcp-window-keyword-v1
Closed

detect/analyzer: add more details for the tcp window keyword-v1#12021
Nancyenos wants to merge 1 commit intoOISF:masterfrom
Nancyenos:detect/analyzer-add-more-details-for-the-tcp-window-keyword-v1

Conversation

@Nancyenos
Copy link
Contributor

@Nancyenos Nancyenos commented Oct 23, 2024

Ticket: 6352

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

Contribution style:

Our Contribution agreements:

Link to ticket: https://redmine.openinfosecfoundation.org/issues/6352

Describe changes:
-detect/analyzer: add more details for the tcp window keyword
-I need help with this, am I in the right direction?
-Also am thinking of below tests and would like to know if i need to add more, i tested for exact and negated size

alert tcp any any -> any any (msg:"TCP window size"; window:30336; sid:1;)
alert tcp any any -> any any (msg:"TCP window size"; window:!1024; sid:2;)

=

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.

This looks like the right approach to me, too :)

One minor change needed (a typo), please see inline comment.

The rules you've shared for the test candidates look good to me, maybe we could have one of them use tcp.window, instead of window, just to showcase that both work.

const DetectWindowData *wd = (const DetectWindowData *)smd->ctx;
jb_open_object(js, "window");
jb_set_uint(js, "size", wd->size);
jb_set_bool(js, "negate", wd->negated);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
jb_set_bool(js, "negate", wd->negated);
jb_set_bool(js, "negated", wd->negated);

@jufajardini jufajardini added the outreachy Contributions made by Outreachy applicants label Oct 23, 2024
@github-actions
Copy link

NOTE: This PR may contain new authors.

@codecov
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 83.25%. Comparing base (30806ce) to head (8f2ccdc).
Report is 9 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12021      +/-   ##
==========================================
+ Coverage   83.22%   83.25%   +0.03%     
==========================================
  Files         910      910              
  Lines      258136   258143       +7     
==========================================
+ Hits       214831   214924      +93     
+ Misses      43305    43219      -86     
Flag Coverage Δ
fuzzcorpus 61.49% <0.00%> (+0.07%) ⬆️
livemode 19.38% <0.00%> (-0.01%) ⬇️
pcap 44.45% <0.00%> (-0.01%) ⬇️
suricata-verify 62.75% <0.00%> (+<0.01%) ⬆️
unittests 59.28% <0.00%> (-0.01%) ⬇️

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

@Nancyenos
Copy link
Contributor Author

Thankyou so much for the help @jufajardini

@jufajardini jufajardini added the needs rebase Needs rebase to main label Oct 24, 2024
@jufajardini
Copy link
Contributor

Work continued with #12024

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

Labels

needs rebase Needs rebase to main outreachy Contributions made by Outreachy applicants

Development

Successfully merging this pull request may close these issues.

2 participants