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

detect: Recognize ERSPAN Type I packets #4627

Closed
wants to merge 3 commits into from

Conversation

jlucovsky
Copy link
Contributor

@jlucovsky jlucovsky commented Mar 3, 2020

[Backport of #4475]

This PR adds support for ERSPAN Type I packets to 5.0.x

This document and wireshark were used as a reference for this work.

Link to redmine ticket:3481

Describe changes:

Suricata-verify PR #195

(cherry picked from commit 427ec4e)
(cherry picked from commit aec4e9a)
For the backport, ERSPAN TypeI decode is

1. Disabled by default
2. Configurable: `decoder.erspan_typeI.enabled`
@jlucovsky jlucovsky requested a review from a team as a code owner March 3, 2020 13:55
{
int enabled = 0;
if (ConfGetBool("decoder.erspan_typeI.enabled", &enabled) == 1) {
g_erspan_typeI_enabled = enabled == 1;
Copy link
Member

Choose a reason for hiding this comment

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

true instead of 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In 5.0.x, ConfGetBool returns an int

src/decode-erspan.c Show resolved Hide resolved
@@ -1338,6 +1338,9 @@ decoder:
vxlan:
enabled: true
ports: $VXLAN_PORTS # syntax: '8472, 4789'
# ERSPAN Type I
erspan_typeI:
Copy link
Member

Choose a reason for hiding this comment

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

wonder if we should make this something like:

erspan:
  typeI:
    enabled: false

Also, wondering if we need to make sure this carries forward to newer releases. If the config has this explicitly disabled here, it should be respected in 6.0 as well right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. The intent of making it configurable is to provide control over (what might be) the additional packet load introduced by the back port.

Only Teredo and VXLAN decoding is configurable in 6.0.

For 6.0, I'd recommend that we detect the configuration setting and either

  1. Exit with an error message ("Deprecated setting" or similar)
  2. Issue a warning and continue execution ("Warning: ERSPAN Type I is no longer configurable")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue 3515 created to track.

Copy link
Member

Choose a reason for hiding this comment

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

  1. Issue a warning and continue execution ("Warning: ERSPAN Type I is no longer configurable")

I think I'd prefer this approach.

@jlucovsky
Copy link
Contributor Author

Continued in #4635

@jlucovsky jlucovsky closed this Mar 5, 2020
@jlucovsky jlucovsky deleted the 3481/1 branch May 6, 2020 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants