Skip to content
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

maintain tagging of networks in a list #16

Open
malleVF opened this issue Jun 16, 2018 · 19 comments
Open

maintain tagging of networks in a list #16

malleVF opened this issue Jun 16, 2018 · 19 comments

Comments

@malleVF
Copy link

malleVF commented Jun 16, 2018

Hi,
It means a lot of effort for tagging hundreds of network ranges within the cidr filter. It would be great to do that in the the list configured in network_path. Like the translate filter, it would be great to extend the cidr list with additional information, e.g.:

10.10.2.0 /24: network 2. floor
192.168.3.1/23: sales
10.40.23.0/24: guest wifi

... and so far.
So that I have to maintain only the list, if something changed.

@malleVF malleVF changed the title maintain list of networks with tags in a list maintain tagging of networks in a list Jun 16, 2018
@malleVF
Copy link
Author

malleVF commented Jun 16, 2018

Optional: In case where network range matches the IP, add the matching network range from the list to a new field. With translate filter I could do a lookup on the new field.

@webmat
Copy link

webmat commented Jun 18, 2018

That's a great idea, thanks for suggesting this!

Is this something you'd be willing to tackle, and submit a PR? :-)

Things to take into consideration:

  • Implementation should be backwards compatible. Files without this additional metadata must continue working as is.
  • The field where this metadata ends up should be configurable. I would suggest the attribute to be called target and its default value to be network.name (see also Add network.name elastic/ecs#25)
  • The separator for entries is configurable, not everyone does one CIDR per line. Make sure to take this into account.
  • Optional: Make the separator between CIDR configurable and trim spaces around the metadata.
  • Optional: Also make the network attribute support CIDR => metadata.

The last two optionals can be skipped if you think this is getting too big. We can definitely move forward without them and consider them later, if needed.

@webmat
Copy link

webmat commented Jun 18, 2018

If you're interested in tackling this, I'll make sure to review your PR and help you along. If you can't tackle this, I'll add this to my list of things to do, but it may take a while before I have time to look into it.

@webmat webmat removed their assignment Jun 27, 2018
@welderpb
Copy link

+1 for this feature..

@rwaweber
Copy link

rwaweber commented Aug 20, 2018

Definitely interested in this feature and I've thought about two ways of thinking about implementing it, one likely much simpler than the other:

  1. Since this functionality seems to be very similar to what the translate filter achieves, maybe it makes sense to think about it from there? Though I'm not sure how to work it in without adding its own sort of codec-esque system. It appears that there have been attempts in the past with this pr: Translate ip on cidr keys to values logstash-filter-translate#31.

  2. Another avenue(likely simpler), in light of maintaining backwards compatibility, we add two more configuration options: network_dictionary and network_dictionary_path, so that we can specify the lookups inline and also provide a path to them.

My only reticence for the second avenue is derived from the first suggestion, in that I'm not sure if this would be considered edging up on the responsibilities of other filters.

@GitSweendog
Copy link

If there is some form of compute operation used to match IPs in filters already, shouldn't it just be altered to accept CIDR notation? Such a new version would be backwards compatible with old configurations, although new configurations wouldn't work on old versions.

@homa1978
Copy link

+1 for this feature..

@syepes
Copy link

syepes commented Nov 16, 2018

+1
I would like to see option #2, this is really useful right now its not really manageable to have all that in one filter. Its always simple to have that mapping file in an external YAML, JSON file

@Acmosa
Copy link

Acmosa commented Apr 10, 2019

+1 for this feature.
Currently I'm using an external tool to translate private cidr to regex and translate to add geolocation.
It would really help to be able to add the geolocation fields as return values in a dictionary file containing the normal cidr notation and use it in the cidr filter.

@MikeKemmerer
Copy link

MikeKemmerer commented Sep 4, 2019

+1. I'll start working on the PR in the coming days. My thought process is adding an option to include the matching network n as a user-configurable field.

@kvmuralidhar
Copy link

+1 for this feature.

@dpresbit
Copy link

+11
Very much needed feature. No reason why the matching CIDR shouldn’t be available.
Today I have to list every single IP in the network in a translate dictionary file, then for a value list the CIDR block. Why do this when CIDR lookup filter already does this work for us and could easily return the CIDR on a successful match.

@MikeKemmerer
Copy link

MikeKemmerer commented Nov 20, 2019 via email

@rwaweber
Copy link

One potential bottleneck that might be causing this is the fact we’re doing a dot product between the list of ips in the event and the list of ips indicated in the filter.
https://github.com/logstash-plugins/logstash-filter-cidr/blob/master/lib/logstash/filters/cidr.rb#L169-L175

Not sure how possible it is in Ruby, but would it be possible to turn that list in the filter into a dictionary/map/hash, so that containment operations become faster?

@rkbennett
Copy link

rkbennett commented Feb 3, 2021

I noticed this issue is pretty stale, but if there's still interest in this I made some code changes locally to this I could do a PR for which allows it to accept hashes and adds a few fields.
network_type => "Array||Hash" (type for the network object, defaults to Array)
network_return => true||false (do you want to return the value of a matching subnet key, defaults to false)
target => "anystring" (the destination field for the returned value, defaults to 'result')
You can then point the network_path to a .json file which would be structured something like this:
{
"10.1.0.0/16": "my internal network",
"192.168.1.0/24": "my other internal network"
}
Alternatively, if you're more the do the dictionary in the pipeline type, I modified the network config option to accept arrays or hashes, so you can just call the plugin from within the pipeline like this:
cidr {
address=> [ "10.1.1.1" ]
target => "network"
network_return => true
network => {
"10.1.0.0/16" => "my internal network"
}
}
Obviously, you can nest hashes within the value to be returned and manipulated in the pipeline. This is just what I've done locally for some dynamic geopoint stuff. Let me know if there's interest and I'll do up a PR

@rbansal16
Copy link

I noticed this issue is pretty stale, but if there's still interest in this I made some code changes locally to this I could do a PR for which allows it to accept hashes and adds a few fields.
network_type => "Array||Hash" (type for the network object, defaults to Array)
network_return => true||false (do you want to return the value of a matching subnet key, defaults to false)
target => "anystring" (the destination field for the returned value, defaults to 'result')
You can then point the network_path to a .json file which would be structured something like this:
{
"10.1.0.0/16": "my internal network",
"192.168.1.0/24": "my other internal network"
}
Alternatively, if you're more the do the dictionary in the pipeline type, I modified the network config option to accept arrays or hashes, so you can just call the plugin from within the pipeline like this:
cidr {
address=> [ "10.1.1.1" ]
target => "network"
network_return => true
network => {
"10.1.0.0/16" => "my internal network"
}
}
Obviously, you can nest hashes within the value to be returned and manipulated in the pipeline. This is just what I've done locally for some dynamic geopoint stuff. Let me know if there's interest and I'll do up a PR

Could you please do a PR or share your code with me. This is something I am still attempting to implement.

@rkbennett
Copy link

I just did a commit to the branch had submitted for a PR, so you should be able to view the code from there. It should also be backwards compatible sense it passed all the build tests. Hopefully @magnusbaeck or someone else with permissions can review and accept.

@ipworkx
Copy link

ipworkx commented Feb 26, 2021

Any idea when your PR will be available? I can’t wait.

@rkbennett
Copy link

Pull request is already submitted, if you're needing it quickly you can always try taking my fork and replacing the code in your local logstash instance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests