test/entropy: Update checks to include sid #2588
Conversation
jlucovsky
added
the
requires suricata fix
|
Is there a redmine ticket for this ? |
catenacyber
added
requires suricata pr
tests/entropy/entropy-01/test.yaml
Outdated
| @@ -1,5 +1,5 @@ | |||
| requires: | |||
| min-version: 8 | |||
| version: 8 | |||
There was a problem hiding this comment.
Why this change ? Will this not work in 9 ?
There was a problem hiding this comment.
Good catch -- the test will work for 8.0.0+ so I'll update things.
| alert http any any -> any any (msg:"entropy simple test"; file.data; entropy: value != 4; sid:7;) | ||
| alert http any any -> any any (msg:"entropy simple test"; file.data; entropy: value = 4; sid:8;) | ||
| # The entropy value is 4.150007324019584 | ||
| alert http any any -> any any (msg:"entropy simple test"; file.data; entropy: offset 10, value > 4.14; sid:10;) |
There was a problem hiding this comment.
Not sure I understand the ticket here, but
- a flow may have multiple txs
- a tx may have multiple files (for example I could write a signature to detect a HTTP POST with 2 files : one having a low entropy, and the other file having a high entropy)
So, is it good to combine sid and sticky buffer name ?
There was a problem hiding this comment.
The entropy value is saved as a flow-related variable -- using VarNameStorageRegister -- into a global namespace. The sid is being added to "scope" the name to a rule.
I think you're saying that there needs to be further scoping when entropy is evaluated over multiple transactions?
There was a problem hiding this comment.
Yes, I fear flow+sticky buffer+sid is not unique enough
There was a problem hiding this comment.
Great analysis.
I suppose we could use a combination of file.md5 and the entropy to make it unique?
There was a problem hiding this comment.
But I am not sure I totally get the point : do you just want to have logged with an alert the entropy(s) that matched ?
There was a problem hiding this comment.
Yes -- the idea is to log the calculated entropy value -- not only in the alert but in all places where the flow is logged.
Consider a rule with mult. entropy statements --- we'd like to know the calculated entropy values of each so they can be adjusted if needed
There was a problem hiding this comment.
not only in the alert but in all places where the flow is logged.
It does not make much sense to me to log in a flow event, some value that belongs to a transaction
And even then, each transaction may have multi-buffers (like dns requests) and you would want to log these different values...
I feel this needs more design, instead of reusing what is available in the engine even if that is not fit...
There was a problem hiding this comment.
Considering this affects output, I tend to agree with Jeff. Could we maybe take some time in the next dev meeting for this, or open a thread to discuss and propose?
There was a problem hiding this comment.
I've updated the entropy PR to use <sticky_buffer> where tracks the entropy keyword usage; where 1 is for the first occurrence, 2 for the 2nd, and so on.
There was a problem hiding this comment.
Does it work for a packet having multiple transactions ?
The sid and instance # are part of the log -- e.g., sid:<sid>;buffer:<buffer>;instance:<instance> -- so update the test checks for each sid that alerts and add a rule with multiple entropy usages.
| @@ -1,5 +1,5 @@ | |||
| requires: | |||
| min-version: 8 | |||
| version: 8.0.0 | |||
There was a problem hiding this comment.
Why this change ? Should it not work in 8.0.1 ?
| @@ -0,0 +1,89 @@ | |||
| requires: | |||
| min-version: 8.0.1 | |||
The sid is part of the log -- e.g., sticky-buffer_sid -- so update the test checks for each sid that alerts.
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: