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

Add missing Suricata schema types and fields #1176

Merged
merged 6 commits into from Nov 17, 2020

Conversation

satta
Copy link
Contributor

@satta satta commented Nov 17, 2020

📔 Description

This PR introduces the currently missing Suricata schema types nfs, tftp, snmp and ikev2. It also adds correct and consistent types for the common fields vlan and in_iface.

📝 Checklist

  • All user-facing changes have changelog entries. (n/a)
  • The changes are reflected on docs.tenzir.com/vast, if necessary. (no dedicated Suricata schema documentation there yet)
  • The PR description contains instructions for the reviewer, if necessary. (see below)

🎯 Review Instructions

Please double-check whether the changes correspond to current best practices and whether the schema loads for the relevant VAST versions.

@mavam
Copy link
Member

mavam commented Nov 17, 2020

Thanks for contributing these definitions!

Since you changed the layout, the integration test baseline needs to be updated. (This is why CI fails.) If you invoke make integration in the build directory, you'll see the diff to the baseline. If this looks reasonable, you can invoke ./integration_BUILD_TYPE.sh -u where -u stands for "update baseline."

Thereafter, CI will be green.

@satta
Copy link
Contributor Author

satta commented Nov 17, 2020

Thanks for the explanation, will do.

It's OK to introduce optional fields where the output might be 'null', right? I did this because, for example, vlan and pcap_cnt was defined only in some types, while in fact they are common to all types, just only present when the traffic has VLAN tags and/or was replayed from pcaps.

@mavam
Copy link
Member

mavam commented Nov 17, 2020

It's OK to introduce optional fields where the output might be 'null', right?

Yes, that works. In fact, that's currently how we can have non-breaking updates. The new type can technically be seen as a strict superset of the old type.

Schema additions have affected the output of some integration test
runs. This commit adjusts the expected output to match the new
output format after the schema change.
Copy link
Member

@mavam mavam left a comment

Choose a reason for hiding this comment

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

Nice work! 🙏

@mavam mavam merged commit 5827b06 into tenzir:master Nov 17, 2020
@satta satta deleted the schema-missing-types-suricata branch November 17, 2020 19:13
mavam added a commit that referenced this pull request Nov 17, 2020
@satta satta mentioned this pull request Nov 17, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants