gonids icon indicating copy to clipboard operation
gonids copied to clipboard
verified publisher icon google

PR for additional validations

Open 0xtf opened this issue 5 years ago • 3 comments

Hi @duanehoward,

I recently reached out via e-mail in regards to a PR I'd like to know if you'd find useful. Basically some additional validations, such as:

  • action keyword
  • protocol
  • network information (IP and port)

We have the changes at https://github.com/pdr9rc/gonids in case you'd like to have a look. Happy to make any changes to fit gonids requirements.

Also, if you prefer to talk via e-mail feel free to close/ignore this issue.

0xtf avatar Dec 16 '20 02:12 0xtf

Hi, I'm having trouble finding the PR you're referencing to review (I don't see one here in the google/gonids repo or in your own?) Can you link me directly to a PR, issue or commit that has your proposed changes? additional validation is always welcome!

duanehoward avatar Dec 16 '20 05:12 duanehoward

Ah, I think it's these commits[0][1] you're looking at? These look interesting, and validation logic is always welcome (that's one of the core values this lib can provide IMO). I do notice at minimum a few "protocol" keywords that are missing since suricata allows tcp-stream and tcp-pkt in addition to tcp probably we'd need some comprehensive documentation to make sure we capture everything allowed. We're currently trying to support version 5.x and haven't update to 6.x yet, but probably the 6.x protocols would be a good start. I think we'll also need to validate ports to ensure things like 0 are never allowed by the engine with some special meaning, etc.

I'm pretty busy at the moment, but I may have time to review PRs during the holiday break.

[0] https://github.com/pdr9rc/gonids/commit/5a66964705179fe4e37d1f08ec541510b4b2aa92#diff-83eb8e32639d01cf443d6d8bde24c1c8be78766090d8c5f8586c36250cfedca6 [1] https://github.com/pdr9rc/gonids/commit/22fabaed4a5463d9b0208c3cccb9746865e3147f

duanehoward avatar Dec 16 '20 05:12 duanehoward

That is correct, yes. I didn't do a PR yet.

Those are valid points in regards to the "protocol". We'll make the changes in regards to the port number (not allowing 0) and I'll look into adding the missing protocols.

I do not see, however, mention to tcp-stream or tcp in the documentation. Would you have a definitive list that you'd like to follow?

0xtf avatar Dec 21 '20 00:12 0xtf