New plugin: net-mgmt/arp-ndp-logging

[mr-manuel]

Hello,

I wrote a new plugin that has similar features as arpwatch. I would like to share it with the community so that everyone can use it.

This is my first plugin and I still not fully understand how everything interacts with the OPNsense UI/API. Could you help me with a few questions?

1. How to troubleshoot API errors? When the GUI tries to call https://192.168.133.1/api/opnarplog/service/stop then it returns {"response":"Error (127)"}. Before I renamed my plugin everything worked. Now I'm clueless where the error could be. Same for https://192.168.133.1/api/opnarplog/service/status where I get {"status":"unknown","widget":{"caption_stop":"stop service","caption_start":"start service","caption_restart":"restart service"}}. SOLVED: I made a call to /usr/local/bin/bash which does not exist on a plain installation. It was replaced with a service call.

2. How does the logging work in OPNsense? Currently I'm writing to a logfile under /var/log/opnarplog.log and rotate it with my script. The logformat is not recognized, but somehow the logfile is recognized on the logpage.

To test the plugin :

1. Copy the content of the repository folder net-mgmt/opnarplog/src to the firewall folder /usr/local
2. Set the permission for two files:

   chmod 755 /usr/local/bin/openarplog.py
   chmod 755 /usr/local/etc/rc.d/opnarplog
   

3. Restart configd

   service configd restart
   

[mr-manuel]

@Monviech @fichtner the PR is now ready for merge. Let me know, if I should change anything else.

[mr-manuel]

Is there any ETA to get this merged?

[mr-manuel]

@Monviech @fichtner would be nice to receive any feedback, if this is going to be merged or not. Else I will start my own repository, but would be easiert to access for users, if it's merged in the main repo.

[Monviech]

I did not have time yet to give this another look. Might still take a while.

[mr-manuel]

Thanks for the feedback!

[Monviech]

Could you give the files a license that allows redistributing the code?

[mr-manuel]

@Monviech is AGPL fine or which one do you prefer?

[mr-manuel]

Is it enough, if I just add my name to https://github.com/opnsense/plugins/blob/master/LICENSE ?

[Monviech]

That file will be auto generated when licenses are updated so you dont have to add anything there.

Just choose the same license as all the other plugins have, the BSD Clause 2 Simplified license.

[fichtner]

My main concern here is the burden of reviewing opnarplog.py -- it can be done but there won't be any peer review involved when not going through the FreeBSD ports tree for example.

I'm also not fond of the name since we have used "OPN" prefix for most of our business plugins. There's also most ARP/NDP consolidated in Interfaces: Neighbors and Interfaces: Diagnostics: ARP Table and Interfaces: Diagnostics: NDP Table so it may be worth consolidating everything into a joined sub menu and plugging this one into it as well. Naming and feature scope needs to be clear and easy to understand for users.

[fichtner]

And yes this should be BSD 2 Clause to enter at least for the plugin management files that OPNsense needs. The Python script could be any license you think is good, but that again would be nicer to bank a foreign license via a package in FreeBSD ports (which I think will not happen due to the very narrow and specific use case).

Working with this script in the future, in this plugin repository with a foreign license will likely hamper maintenance directly done by us and or others meaning when problems arise the plugin will be quickly removed from a release and subsequently removed from the plugins tree.

I don't want to be unhelpful, but this really matters for maintenance reasons later on.

[mr-manuel]

Thanks for the feedback. I want to keep things as simple as possible for everyone. I added the BSD 2-Clause license.

Should I leave the name as it is right now and you change it afterwards, or should I already remove it?

[mr-manuel]

What about placing it under Interfaces -> Diagnostics and naming it ARP/NDP Logging?

[fichtner]

Ok, you need to put the license in every file you assert copyright for otherwise these will be "commercial" and no permission to be used was given. We also need to at least settle on a directory name. The rest we can work on later.

What about placing it under Interfaces -> Diagnostics and naming it ARP/NDP Table Logging?

I prefer merging the options under a single sub menu, but I'll ask internally first if that's ok.

[mr-manuel]

In my opinion it's better to put it under services, since you setup a service that constantly monitors something. If you do diagnostics, then you do it only on the need to do so.

[fichtner]

In my opinion it's better to put it under services, since you setup a service that constantly monitors something. If you do diagnostics, then you do it only on the need to do so.

Yes, after a brief internal discussion that is also what we think.

[mr-manuel]

Sounds good. Then I rename it from OPNarplog to ARP/NDP Logging and we should be good?

[mr-manuel]

I did not yet test the plugin after the renaming. Can do it tomorrow.

[mr-manuel]

I tested now for nearly a month since the renaming, made small changes and updated everything. Would now be ready to merge. @fichtner

Else the plugin is running fine since a few months now.

[mr-manuel]

@Monviech maybe you can take a look, so that we can merge this before the PR gets one year old?

[Monviech]

@mr-manuel Sorry its not in my scope to decide this. @fichtner has to approve of this.

From what I understand the main reason for this to have a low priority so far is that this does not follow any upstream freebsd package for its core functionality.

[mr-manuel]

Thanks for your feedback, then let's hope @fichtner can approve this soon 😃

[mr-manuel]

@Monviech could you assign this PR to @fichtner? Maybe this helps him to keep that visible.

[AdSchellevis]

We likely shouldn’t merge this, using the ndp command for host discovery will be quite slow and inefficient as we’ve also seen while looking at captive portal on ipv6.

As a small experiment, I’m currently looking into a host watcher developed in rust using libpcap, which is a similar approach as arpwatch is using (and memory safe in rust).

not sure when we will release anything, but highly likely it will be available in the community version as this might also be needed for ipv6 captive portal to function properly as @swhite2 mentioned to me earlier.

Offering a solution now which we need to advise against using later is likely not a good idea.

Sorry to bring this up so late, last year this wasn’t a topic yet, if the host discovery didn’t cross our radar a couple of weeks ago, over time we might have merged something, although the review part remained an issue (often it costs us more time to read and comment on than building a similar feature ourselves)

[mr-manuel]

@AdSchellevis thanks for your reply. I just wanted to make it users easy to use this functionallity. Should the code not match the requirements OPNsense have, then I would close the PR and add it to my own repository.

Please just let me know, if you need further time to review that or if the decision was made to not merge this.