-
Notifications
You must be signed in to change notification settings - Fork 50
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
Missing protocols and transformations #156
Comments
@satta Can you provide a quick pointer in the source to where these other protocols are defined (probably an issue should be opened with OISF to update the docs to make them complete as well). Additionally can you give me an example of where |
I don't think they are listed in the documentation itself. The last time I needed this information, I just looked them up in the source code. App-layer protocols:
Lower layer protocols are mentioned in
|
Here's an example for the
which does not parse giving |
You can test the parser with https://gist.github.com/satta/a19397fd91ce182df7570efbd2a14b92 and the ET OPEN ruleset (https://rules.emergingthreats.net/open/suricata-5.0/emerging-all.rules.tar.gz). For example, here are the issues still seen with the current master:
|
I can do that and open an issue in their Redmine later. |
I think with the current revisions all of the protocol parsing issues should be resolved. Feel free to pull the newest revision and let me know. I still haven't addressed the |
note: network parsing issues were addressed in #165 |
Thanks! Looks good.
Nice, I tried something complicated like
and it looks like it was parsed correctly now. However, the parser still seems to disallow spaces between the brackets, which according to the Suricata documentation is OK to do. I also noticed that apparently invalid source/dest patterns like
(note the disbalanced brackets) is currently successfully parsed to
I think this should be an error.
I think since this is not a syntax issue I would be inclined to allow the value
I see. The |
IIRC we removed support for spaces in the past, possibly because it was introducing some odd complexity in the lexer, and we noted that basically all rulesets and examples don't use spaces. (the lexer can't look for a #167 should resolve the unbalanced The handling of nested groups appears to have introduced another bug in handling negated network groups where the negation is the first part of the pattern something like: This issue is strangely complex to find a solution for, and frankly the more I look at this, I wonder if it would be simpler to move the network definitions into better defined structs and lex them differently. That, however is not something I can handle in the short term, so I'll track that as a distinct issue. TODO(duane): |
Yes, I suspected that. But I think that's more of a minor issue since we do get an error message if spaces are encountered instead of silently parsing the range incorrectly.
Great, thanks!
Yes, it is more complex than one might think at first glance. When I wrote a rule parser in Python the last time, I used https://github.com/lark-parser/lark to work out a comprehensive grammar, which I found easier to tweak and optimize than having to work with actual parsing code. I didn't research if there is something similar for Go though. Maybe something like https://github.com/alecthomas/participle 🤔 Anyway, thanks so far for all the work you put in, it has really improved things! |
From issue #154 @satta notes:
The text was updated successfully, but these errors were encountered: