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

smb: New keyword smb.filename v4 #7337

Closed
wants to merge 2 commits into from

Conversation

zer1t0
Copy link
Contributor

@zer1t0 zer1t0 commented Apr 28, 2022

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

Describe changes:

  • Add new sticky buffer smb.filename to match the filenames that are being accessed by SMB through the create file request
  • Add documentation for the keyword

Rule example: alert smb any any -> any any (msg: "SMB file a.txt";smb.filename; content:"a.txt";sid:1;)

suricata-verify-pr: 802

@codecov
Copy link

codecov bot commented Apr 28, 2022

Codecov Report

Merging #7337 (c70c43b) into master (2ebb525) will decrease coverage by 1.90%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #7337      +/-   ##
==========================================
- Coverage   77.68%   75.78%   -1.91%     
==========================================
  Files         628      657      +29     
  Lines      185657   190093    +4436     
==========================================
- Hits       144232   144064     -168     
- Misses      41425    46029    +4604     
Flag Coverage Δ
fuzzcorpus 60.26% <36.00%> (+2.21%) ⬆️
suricata-verify 51.62% <88.88%> (-2.84%) ⬇️
unittests 61.01% <36.00%> (-2.03%) ⬇️

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

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.

CI : ✅
Code : looks good to me
Commits segmentation : ok that way with the 2 commits squashed together
Commit messages : ok
Git ID set : looks fine for me
CLA : I do not have access, but looks like some commits were already merged
Doc update : thanks for the update
Redmine ticket : ok
Rustfmt : was not enforced
Tests : Suricata-verify tests look fine
Dependencies added: none

@jufajardini jufajardini added the needs rebase Needs rebase to master label Nov 28, 2022
@jufajardini
Copy link
Contributor

Added the needs rebase label due to the conflicts, sorry if I'm wrong...

@victorjulien
Copy link
Member

Closing due to inactivity. If you're interested in picking this back up, please open a new PR addressing the comments. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase Needs rebase to master
4 participants