search

vectordotdev / vrl

Vector Remap Language
Mozilla Public License 2.0
115 stars 52 forks source link

Community ID is incorrect for ICMP #891

Open DylanRJohnston opened 2 weeks ago

DylanRJohnston commented 2 weeks ago

I've let the upstream library know of the problem, but I thought you guys might also be interested in the problem. https://github.com/traceflight/rs-community-id/issues/3

jszwedko commented 2 weeks ago

Thanks! We'll track the upstream issue.

DylanRJohnston commented 2 weeks ago

I've got a pull request up to fix the issue https://github.com/traceflight/rs-community-id/pull/4

DylanRJohnston commented 2 weeks ago

I also think the documentation needs to be updated on the community_id function to point out that source_port and destination_port for ICMP are actually and ICMP type and ICMP code respectively. This was very confusing when I was first investigating the issue, especially as Elastic Search separates those arguments. Perhaps it would be wise to do the same as semantic overloading of named arguments is very confusing. Although if you wanted to introduce a more accurate error message this would be a breaking change.

DylanRJohnston commented 2 weeks ago

Found further problems with the handling of ICMP with IPv6 addresses https://github.com/traceflight/rs-community-id/issues/5

jszwedko commented 2 weeks ago

Nice, thanks @DylanRJohnston, good finds.

The overloading of port to be ICMP type and code is a bit confusing. I see that, for example, the Go library has separate functions per level 4 protocol and also ICMP. We could do something similar. Notably the C implementation seems to also overload like we are. I'll open a PR to update the docs for now at least.

jszwedko commented 2 weeks ago

Docs update: https://github.com/vectordotdev/vector/pull/20677

traceflight commented 2 weeks ago

Thanks @DylanRJohnston. I have released a new version of rs-community-id. Feel free to mention me if any issue found~