net/upnp: add missing firewall anchors#3165
net/upnp: add missing firewall anchors#3165fichtner merged 3 commits intoopnsense:masterfrom Tawmu:upnp-fwrules
Conversation
|
@Tawmu thanks, do you have a quick reference to when they started to use these anchors (changelog possibly)? |
|
@fichtner my primary motivation for this is to get multiple game consoles behind an opnsense router working properly when trying to join lobbies together - there was quite a bit of work done between miniupnpd and another firewall distro to get this working. Sometime in 2020/21 miniupnpd fixed some NAT rule generation (I think around version 2.2.0, the changelog is a little lacking (https://github.com/miniupnp/miniupnp/blob/master/miniupnpd/Changelog.txt#L90)) - part of the reason for adding these extra anchors in is to ensure the rules are inserted at the proper place (i.e. before other NAT rules generated by opnsense). I'll be submitting an update to ports for a miniupnpd package update soon to improve this behavior in opnsense. Forgive me if it's not very clear or I've done something wrong - I don't usually dip my feet into BSD based stuff so pf and opnsense dev is relatively new to me. |
|
|
||
| $fw->registerAnchor('miniupnpd', 'rdr'); | ||
| $fw->registerAnchor('miniupnpd', 'fw'); | ||
| $fw->registerAnchor('miniupnpd', 'nat', '', 'head'); |
There was a problem hiding this comment.
'head' might be much to the surprise of the user, but then I don't expect upnp active for anyone but the game console use case in a home setting as you describe. but maybe we should warn users more about the potential security issues. not sure where or how
There was a problem hiding this comment.
Totally agree - I guess the other situation is that (automatic/manual) NAT rules could potentially break UPNP without any user notification if it wasn't at the top.
Perhaps some kind of warning on the opnsense UPNP plugin page? I imagine that would be sufficient since a user needs to go and enable the UPNP service manually after install anyway.
There was a problem hiding this comment.
let's merge this one and work on the other one to include some sort of sensible warning
|
thanks for the details. from the ports PR I see that before we do this the update must be carried out, right? that could be a slow grind in the FreeBSD bugzilla, but we shall see :) |
|
Yeah AFAIK miniupnpd needs to be updated first but I guess it wouldn't hurt to merge this in anticipation (pending above comment about warning the user) |
|
Merged, thanks! |
|
Is there any way for me to add this code change manually in a current build? |
Later releases of miniupnpd require additional firewall anchors to function properly. This commit adds those.