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

dhcp: Validate dhcp option 60 is being logged #1362

Closed
wants to merge 1 commit into from

Conversation

yatink
Copy link
Contributor

@yatink yatink commented Aug 24, 2023

Feature 4587

..continuation of #1361

This PR adds a test to validate that dhcp option 60 is being logged in dhcp extended logging mode.

Associated suricata PR : OISF/suricata#9397
Redmine ticket: https://redmine.openinfosecfoundation.org/issues/4587

@jasonish jasonish added the requires suricata pr Depends on a PR in Suricata label Aug 24, 2023
min-version: 7

args:
- -k none --set outputs.1.eve-log.types.21.dhcp.extended=yes
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jasonish will it always be 21 ?

Or should rather a minimal suricata.yaml be used here ?

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather a minimal YAML. We do have existing tests digging into arrays like this, but it is fragile. @yatink Could you update this test to provide a minimal YAML file instead of the --set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can read the docs and figure out how to add that. I'll ask here if I have questions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Look out for other Suricata Verify tests having a suricata.yaml in their directory

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok done....I'll squash my changes and open a new PR.

@yatink
Copy link
Contributor Author

yatink commented Aug 30, 2023

Continued in #1367

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
requires suricata pr Depends on a PR in Suricata
Development

Successfully merging this pull request may close these issues.

4 participants