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.

Sign up for GitHub

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

Jump to bottom

tests: add tests for smb.version keyword #1366

Closed
wants to merge 1 commit into from
Closed

tests: add tests for smb.version keyword #1366

wants to merge 1 commit into from

Conversation

jmtaylor90
Copy link
Contributor

Ticket

If your pull request is related to a Suricata ticket, please provide
the full URL to the ticket here so this pull request can monitor
changes to the ticket status:

Redmine ticket:
https://redmine.openinfosecfoundation.org/issues/5075

@catenacyber
Copy link
Collaborator

Thanks @jmtaylor90 are you also picking up OISF/suricata#7336 back ?

@catenacyber catenacyber added the requires suricata pr Depends on a PR in Suricata label Aug 30, 2023
@catenacyber
Copy link
Collaborator

Requires OISF/suricata#9415 apparently

@jmtaylor90
Copy link
Contributor Author

Requires OISF/suricata#9415 apparently

It does, what is the way to cross reference the PRs?

@catenacyber
Copy link
Collaborator

It does, what is the way to cross reference the PRs?

I copy the link of the S-V PR in the suricata one (and GitHub makes it appear in the S-V PR)

If you are interested, we use labels on S-V PRs, basically it is :

  • needs suricata fix (exhibits a bug for which there is no fix)
  • needs suricata PR (CI should be red now, but becomes green when PR gets merged)
  • tests pass (CI should be green)

jufajardini
jufajardini reviewed Aug 30, 2023
View reviewed changes
tests/smb-version-keyword/README.md
Comment on lines +1 to +4
TEST
====

Test smb.version keyword
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add an indication of where the pcap used here comes from? :)

Copy link
Contributor Author

@jmtaylor90 jmtaylor90 Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In this case I obtained it from the original submitters branch, it appears to be a capture from a local network they had access to. I am not sure how to denote that. How would you like that documented?

https://github.com/zer1t0/suricata-verify/tree/test/smb_version

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair enough for me

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the late reply. A note with something along those lines - even if just - 'sample traffic from a local network' works, IMHO. :)

@jmtaylor90
Copy link
Contributor Author

continued in #1380

@jmtaylor90 jmtaylor90 closed this Sep 6, 2023
@jmtaylor90 jmtaylor90 deleted the smb-version-keyword-tests-v1 branch January 29, 2024 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@catenacyber catenacyber catenacyber left review comments

@jufajardini jufajardini jufajardini left review comments

Assignees
No one assigned
Labels
requires suricata pr Depends on a PR in Suricata
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jmtaylor90 @catenacyber @jufajardini