-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
There is one more event besides "stats" and "alert". It is "drop" for IPS. #13032
Comments
Hi, The bug template exists for a reason. As-is you haven't even said what plugin you are using. Please provide a config without any credentials. Thanks |
It is suricata plugin. "suricata.go" source code line 226 to line 246 referred. |
Moreover, someone may use "reject" instead of "drop" too. |
It looks like there are custom parsers for these alert and stats results. Can you provide an example object to determine how these messages should be parsed? Thanks |
The following is an example suricata rule :
Where "alert" is for IDS. When using Suricata as IPS, "alert" may be "drop" or "reject" depending on the writers' mind. The "drop" means to drop all the related packets without response while "reject" means to drop all the related packets and response to the sender about the action. Moreover, another keyword is "pass" which allows the related packets to pass the IDS/IPS without further action. I think "pass" may be ignored for Telegraf. |
Ah so we can re-use the existing alert parsing to capture and parse possible "drop" and "reject" messages. That should be straightforward to test in a PR |
Replace line 226 to 246 with the following code :
|
@samiux Do you want to put up a PR? If you do the PR will get artifacts attached to it, assuming tests pass, that you could then also confirm work. |
I do not know how to do it. Any procedure? |
I put up PR #13048 which should have some artifacts added as a comment from the telegraf tiger in the next 20-30mins. If you could download one of those artifacts and see if it works, it would be great! |
Sorry, where to download? |
Hmm seems the bot didn't add the links again. I'll follow up with circleci :( here are direct links: Let me know if you are using a different architecture or OS combo and I can get you that link. |
How about arm64 version in debian? |
The changes do not work. I am running Suricata 6.0.10 in IPS mode. The followings are example of the eve.log :
|
Looking at your examples in order 1 - "event_type": "stats": this one should parse fine, what error are you getting? 2 - "event_type": "drop": as you discovered your patch will not work. As the The remaining items you showed do not have a stats, alert, drop, or reject in them: 3 - "event_type": "http" |
The error message is : It is telegraf 1.26.1.
|
The error message of the PR :
|
That is expected, as those are both "event_type:flow" |
The InfluxDB got the data. However, there are no such fields, such as dest_ip, dest_port, src_ip, src_port, proto, in_iface, out_iface and event_type, etc in the InfluxDB. I think Grafana needs them to make a good looking dashboard. |
Are those fields always in a I see them in alerts as well, but looks like we currently do not parse them either. |
Those fields are also in "drop" and "reject" message. However, when making a good looking dashboard, we need those fields to plot the graphs. Such as : Drop and Alert list
Count for protocol list
It is also good to show the signature in the field too. |
I think it is good to combines all event type (such as alert, drop, reject) in one measurement. When we are going to plot a graph, we can just search for the event type and other fields. |
I do not disagree, but at this point you are effectively "breaking" the plugin by changing how it works for existing users if we were to make this change. This "issue" has grown in scope a little bit, so when I get back next week I can think about next steps on this one. |
Would you mind telling me how to get the src_ip, src_port, dest_ip and dest_port? |
To set the timestamp using the message timestamp we would need two new fields one for the fieldname and one for the timestamp format to parse. By default we would want to continue to ignore this field as to not change existing users
Are these always in the root of the message? Looks like someone mentioned these values most-merge in #9322 previously. In terms of collecting these values, we could check if they are in the root and add what we find as fields if we do find them. Thoughts? |
In my opinion, "timestamp" is very important to the network security monitoring as it tells us when was the events triggered. Meanwhile, IP addresses and ports are also important to the security monitoring too. How about to create another measurement (such as suricata_events besides suricata and suricata_alert) to keep all the data (all kinds of event type) from the eve json log for the users who requires all the event types, IP addresses, ports and etc? Meanwhile, elasticsearch, logstash and kibana can capture all the eve json log from Suricata. However, ELK Stack consumes a lot of resources which is not suit for IoT. On the other hand, I find that someone else uses elasticsearch to capture the log and plot the graphs in Grafana, such as https://github.com/TripleConsult/suricata_grafana_dashboard . |
Adds a new v2 message parsing that will parse any event type that is received. fixes: influxdata#13032
Adds a new v2 message parsing that will parse any event type that is received. fixes: influxdata#13032
I have pushed a bigger update to #13048 which introduces a new config option called I need to write some more test cases, but could you give it a try and let me know what you think? |
It seems that it is working properly. I will let the telegraf to run for a while for further checking. Thank you. |
After running it for a while, it is working properly and perfect. Thank you. |
Thank you for trying it out and confirming! I'll wrap up some test cases and put up the PR for review next week. Thanks again! |
Any harm? |
Ah interesting, do you have an example metric that has a version field as a string and another as a float? In general it doesn't do any harm, but it does mean that you are missing that field when it shows up as a string. I think in other plugins we try to store version as a string since it can and usually does have characters or extra periods in it. |
I do not know which data contains the version field that is a float. However, I think that all the fields of the metric data may be string as it can produce log files, such as json. |
By the way, "signature_id", "src_por"t and "dest_port" are also string as they do not do calculation. |
The
In the branch I am specifically setting src_port and dest_port to integers. Is that not what you are seeing? Is there anyting else in the branch that is an issue? or is it generally working? |
"src_port" and "dest_port" are now integer. However, I think all the metric field of Suricata are string only as they do not do any calculation. |
are you asking I flip those back to strings as well? |
If it is making sense, I would like to make them as string. |
Adds a new v2 message parsing that will parse any event type that is received. fixes: influxdata#13032
Relevant telegraf.conf
N/A
Logs from Telegraf
System info
Telegraf 1.26.1
Docker
No response
Steps to reproduce
N/A
Expected behavior
N/A
Actual behavior
N/A
Additional info
I am running Suricata IPS and Telegraf is working not properly. It refused to capture the traffic from unix socket. After checking the source code, I find out that from line 226 to line 246 where the event "drop" is missing. It is because that the event of an IPS is "drop" instead of "alert".
The text was updated successfully, but these errors were encountered: