net/upnp: Complete service improvements

[Self-Hosting-Group]

No description provided.

[Self-Hosting-Group]

See 26.1 coming soon. Can we continue here?

[Self-Hosting-Group]

If I just copy the syslog filter template, it will not be applied; a /etc/rc.d/syslogd restart is necessary.

[fichtner]

syslog is always restarted post-install for plugins:

https://github.com/opnsense/core/blob/5da971f2c678fa6d01fb2db4b8a190fd41b8c70a/src/etc/rc.configure_plugins#L46-L49

See 26.1 coming soon. Can we continue here?

Sure, the PR is still in draft mode... you need to signal when you're ready first.

[fichtner]

yes please, I did forget these

[fichtner]

what about these? and can you bring it out of draft mode please?

[Self-Hosting-Group]

Sorry for the delay. I've just pushed the changes. Since the OPNsense 26.1 release is tomorrow and this PR is ready, it would be great if you could give me a quick comment on the following points. I'll then prepare the missing changelog and remove the draft flag from the PR to be merged.

Sure, the PR is still in draft mode... you need to signal when you're ready first.

I did this so that the PR wouldn't be merged unexpectedly too early. Thank you for the review.

syslog is always restarted post-install for plugins:

https://github.com/opnsense/core/blob/5da971f2c678fa6d01fb2db4b8a190fd41b8c70a/src/etc/rc.configure_plugins#L46-L49

1. OK. My question was incomplete. I noticed that copying the snippet and restarting the syslogd logs daemon errors, but not after restarting the router. Does this work with a normal plugin installation? I didn't compile the package this time.

2. Daemon errors are now logged, but messages with higher syslog severity (e.g. info with miniupnpd -v) seem to be filtered out, even if enabled in the UI. Is there anything that can be done?

[fichtner]

OK. My question was incomplete. I noticed that copying the snippet and restarting the syslogd logs daemon errors, but not after restarting the router. Does this work with a normal plugin installation? I didn't compile the package this time.

Yep should be good as is.

Daemon errors are now logged, but messages with higher syslog severity (e.g. info with miniupnpd -v) seem to be filtered out, even if enabled in the UI. Is there anything that can be done?

Where are you logging to? System log? It may be better to add a separate log file and page for miniupnpd alone. The system log is capped to notice (so won't see info or debug) for historic reasons.

[Self-Hosting-Group]

It may be better to add a separate log file and page for miniupnpd alone.

Separate log file implemented. Please review.

[Self-Hosting-Group]

@fichtner @fraenki Critical issue detected in OPNsense 26.1 and registerAnchor function used in plugin.

pfctl_get_rules_info: Invalid argument is logged. registerAnchor function used in miniupnpd.inc does not seem to be working.

/sbin/pfctl -P -a miniupnpd -s nat
pfctl: DIOCGETRULES: Invalid argument

See: https://forum.opnsense.org/index.php?msg=258237

[fichtner]

Thanks for the hint. I found it https://forum.opnsense.org/index.php?topic=50520.msg258554#msg258554

[Self-Hosting-Group]

A 26.1_5 release?

[fichtner]

This should have been in today’s hotfix, but there likely won’t be another. We’re targeting a 26.1.1 next week for a lot of other reasons. Not sure how early or late in the week that will be.

[Self-Hosting-Group]

Let us publish this PR in OPNsense 26.1.2 (or 26.1.1_1) after a release that fixes the creation of anchors in the core.

[Self-Hosting-Group]

PS: The last update changed the headings Source IP/port to Remote IP/port, and a ports patch to improve daemon logging will be available next week.

[Self-Hosting-Group]

I would like to better understand the pf daemon backend. Since I know that the daemon does not create binat rules, I first removed this registration, then others, and then realised that only one registration is sufficient to create the anchor, and that IPv4/IPv6 port mapping works. Therefore, why does the rule type (rdr/nat...) need to be registered at all?

plugins/net/upnp/src/etc/inc/plugins.inc.d/miniupnpd.inc

Lines 42 to 45 in 85f1bb9

	$fw->registerAnchor('miniupnpd', 'rdr');
	$fw->registerAnchor('miniupnpd', 'fw');
	$fw->registerAnchor('miniupnpd', 'nat', 0, 'head');
	$fw->registerAnchor('miniupnpd', 'binat');

And what do you think of this wording addition in the UI help?
Allow adding port maps for non-requesting IP addresses; normally disabled for security

[Self-Hosting-Group]

Hello @fichtner. I am updating the PR. A short comment on the above would be very helpful.

[Self-Hosting-Group]

Below are three comments on OPNsense 26.1 that are unrelated to this PR:

1. When 26.1 is newly installed and the setup wizard is not launched, IPv6 works on the LAN as in previous versions, but when I complete the wizard, the Dnsmasq IPv6 range is deleted. Is this intentional? How about if only the IPv4 range is deleted by the wizard? PS: I have seen that there are plans to extend the wizard for IPv6 in 26.7, which is of course the optimal solution. See: https://github.com/opnsense/core/blob/master/src/opnsense/mvc/app/models/OPNsense/Core/InitialSetup.php#L383-L385

2. Clicking on the logo bypasses the wizard, but this is not permanent

3. The ACL name of Dnsmasq was probably written with a slash because XML entities were not decoded at the time, but could now be adapted to the menu name. See: https://github.com/opnsense/core/blob/master/src/opnsense/mvc/app/models/OPNsense/Dnsmasq/ACL/ACL.xml

[Self-Hosting-Group]

@fichtner @AdSchellevis If the last comment would lead to an adjustment of the setup wizard, it might be good to have this in the planned updated 26.1 images.