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

decode/ethertype: Event on unknown ethertype #11632

Closed
wants to merge 2 commits into from

Conversation

jlucovsky
Copy link
Contributor

Continuation of #11557

Issue: 7129

Create a decode/engine event if unknown ethertypes are observed.

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

Describe changes:

  • Add an event created when unknown ethertypes are observed
  • Update schema with event counter
  • Add rule for event.

Updates

  • Rebase

Provide values to any of the below to override the defaults.

  • To use an LibHTP, Suricata-Verify or Suricata-Update pull request,
    link to the pull request in the respective _BRANCH variable.
  • Leave unused overrides blank or remove.

SV_REPO=
SV_BRANCH=OISF/suricata-verify#1954
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

Issue: 7129

Create a decode/engine event if unknown ethertypes are observed.
Copy link

codecov bot commented Aug 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.50%. Comparing base (304271e) to head (359ec44).
Report is 111 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11632      +/-   ##
==========================================
- Coverage   82.61%   82.50%   -0.12%     
==========================================
  Files         919      919              
  Lines      248997   249004       +7     
==========================================
- Hits       205717   205432     -285     
- Misses      43280    43572     +292     
Flag Coverage Δ
fuzzcorpus 60.88% <100.00%> (+<0.01%) ⬆️
livemode 18.67% <100.00%> (+0.01%) ⬆️
pcap 43.39% <100.00%> (-0.75%) ⬇️
suricata-verify 61.65% <100.00%> (-0.25%) ⬇️
unittests 59.01% <75.00%> (+<0.01%) ⬆️

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

@jasonish
Copy link
Member

So out of the box behavior would be to log the first unknown ether_type, then after 60 seconds, relog for that same one or perhaps another? So in the case where there are multiple unknown ethertype being seen, it could be a while, or even never before they all get logged?

@jasonish
Copy link
Member

So out of the box behavior would be to log the first unknown ether_type, then after 60 seconds, relog for that same one or perhaps another? So in the case where there are multiple unknown ethertype being seen, it could be a while, or even never before they all get logged?

Oh, I see Victor already commented on this: #11455 (comment)

I wonder how we can capture this information for users.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 22126

@jlucovsky
Copy link
Contributor Author

#11455 (comment)

What information would we want to collect?

The invalid ethertype values are probably due to misconfiguration or an unsupported encapsulation type.

@jasonish
Copy link
Member

#11455 (comment)

What information would we want to collect?

The invalid ethertype values are probably due to misconfiguration or an unsupported encapsulation type.

For me its more about a comment/documentation on how not all unknown types may generate an event, so you might not be able to enumerate them. Since the thresholding is done on the rule, not the ethernet type.

@jufajardini
Copy link
Contributor

#11455 (comment)

What information would we want to collect?
The invalid ethertype values are probably due to misconfiguration or an unsupported encapsulation type.

For me its more about a comment/documentation on how not all unknown types may generate an event, so you might not be able to enumerate them. Since the thresholding is done on the rule, not the ethernet type.

If I understand the situation correctly, we have stats for each time an unknown ethertype is seen, but our event-based Suricata rule, due to thresholding, might not catch all (possibly) different cases, is that right?

I have two documentation suggestions:

  • document the decode.ethertype.unknown Stats output
  • document the Suricata unknown ethertype event rule, to explain how it works and its interpretation.

@jlucovsky
Copy link
Contributor Author

#11455 (comment)

What information would we want to collect?
The invalid ethertype values are probably due to misconfiguration or an unsupported encapsulation type.

For me its more about a comment/documentation on how not all unknown types may generate an event, so you might not be able to enumerate them. Since the thresholding is done on the rule, not the ethernet type.

If I understand the situation correctly, we have stats for each time an unknown ethertype is seen, but our event-based Suricata rule, due to thresholding, might not catch all (possibly) different cases, is that right?

We will not be able to provide information as to what the unknown ethertype values were.

I have two documentation suggestions:

  • document the decode.ethertype.unknown Stats output
  • document the Suricata unknown ethertype event rule, to explain how it works and its interpretation.

@catenacyber
Copy link
Contributor

#11455 (comment)

What information would we want to collect?

The invalid ethertype values are probably due to misconfiguration or an unsupported encapsulation type.

The information requested was the list of all unsupported encapsulation types seen on the wire...

@@ -1322,6 +1326,9 @@ void DecodeUnregisterCounters(void);
#define PKT_FIRST_ALERTS BIT_U32(29)
#define PKT_FIRST_TAG BIT_U32(30)

/** Unknown/unsupported */
#define PKT_IS_UNKNOWN BIT_U32(31)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe precise unknown ether type
As we can have another "unknown" ip type

@catenacyber
Copy link
Contributor

So, for me this PR is good in itself, but not complete :
Would we want events to have the ability to have some metadata associated to it ?
How does Zeek do it ?

@jufajardini
Copy link
Contributor

#11455 (comment)

What information would we want to collect?
The invalid ethertype values are probably due to misconfiguration or an unsupported encapsulation type.

The information requested was the list of all unsupported encapsulation types seen on the wire...

Like, have these in our docs? If yes, I approve of this suggestion!

@catenacyber
Copy link
Contributor

The information requested was the list of all unsupported encapsulation types seen on the wire...

Like, have these in our docs? If yes, I approve of this suggestion!

No, not in our docs, the feature request was like "I have this network traffic, I want Suricata to tell me which ethernet types in this network traffic are unsupported by Suricata"

@jufajardini
Copy link
Contributor

The information requested was the list of all unsupported encapsulation types seen on the wire...

Like, have these in our docs? If yes, I approve of this suggestion!

No, not in our docs, the feature request was like "I have this network traffic, I want Suricata to tell me which ethernet types in this network traffic are unsupported by Suricata"

Oh, so when Suri sees something unsupported, it would log out what type that is?

@catenacyber
Copy link
Contributor

No, not in our docs, the feature request was like "I have this network traffic, I want Suricata to tell me which ethernet types in this network traffic are unsupported by Suricata"

Oh, so when Suri sees something unsupported, it would log out what type that is?

It is a way to solve this

@jlucovsky
Copy link
Contributor Author

No, not in our docs, the feature request was like "I have this network traffic, I want Suricata to tell me which ethernet types in this network traffic are unsupported by Suricata"

Oh, so when Suri sees something unsupported, it would log out what type that is?

It is a way to solve this

Except we can't log output at packet rates. That's the issue with packet decode events -- how to provide enough information without creating a log storm.

Thoughts or ideas?

That's where the thresholds are valuable (rate limiting) but there is information loss.

@jufajardini
Copy link
Contributor

No, not in our docs, the feature request was like "I have this network traffic, I want Suricata to tell me which ethernet types in this network traffic are unsupported by Suricata"

Oh, so when Suri sees something unsupported, it would log out what type that is?

It is a way to solve this

Except we can't log output at packet rates. That's the issue with packet decode events -- how to provide enough information without creating a log storm.

Thoughts or ideas?

Maybe... not for real usage, but for investigation purposes, could we save stats on these unknown ether types and save this as some sort of data set? Like... if we see an unknown ethertype, we save that to a table, and if that shows up again, we increase that stats counter. Or, idk, it's just already in the data set. And as with stats, we regularly update the file. But not something to be enabled all the time. 🤔 (brainstorm mode off)

That's where the thresholds are valuable (rate limiting) but there is information loss.

@catenacyber
Copy link
Contributor

Except we can't log output at packet rates. That's the issue with packet decode events

The people asking for the feature can log at packet rate an anomaly event for every packet that has an unknown ethernet type (which means not every packet at all)...

This PR already provides the event to have a rule for unknown ethernet type (and it is good), so people can already do the log storm if they want.
We just need some additional info.
It could be done with the eve-log.ethernet suricata.yaml option which now only logs the Mac addresses but could also log the ethernet type for instance.

@jlucovsky
Copy link
Contributor Author

Continued in #11850

@jlucovsky jlucovsky closed this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants