packages miniupnpd: Update, revision, new network access control and UCI options…

Commits

As this PR is extensive, the descriptions of the individual commits are collapsed here:

1. Update to 2.3.9 to fix issues, refresh building

Update daemon to 2.3.9 to fix removal of nftables rules in upnp_forward and return the correct client port. This also resulted in the excessive opening of new ports
Build from GitHub releases to get a reliable HTTPS server, as the HTTP-only/HTTPS mirror were only available ~85%/77% over 3 months https://github.com/miniupnp/miniupnp/issues/770 https://stats.uptimerobot.com/DwGDxUB914
Build daemon with --disable-pppconn to remove the old/IGDv1-only extra WANPPPConnection SSDP announcements workaround not included in other implementations since >15y
Build daemon with --vendorcfg to allow customisation of the router/friendly name (+5 potential options) displayed in Windows Explorer, 384 bytes extra required on ARMv7 (binary)
Remove old (iptables variant only) patches, as no longer needed
Remove clean_ruleset_interval/threshold UCI config options as not standard/working since OpenWrt 22.03, as nftables not supported

Fix: https://github.com/openwrt/openwrt/issues/18011 Fix: https://github.com/openwrt/luci/issues/7759

7	3	net/miniupnpd/Makefile
1	6	net/miniupnpd/files/miniupnpd.init
0	24	net/miniupnpd/patches/200-remove-default-cflags.patch
0	22	net/miniupnpd/patches/300-macos-compat.patch

2. Patch for UPnP IGDv2 Microsoft/Apple compat

Add workaround to list port maps with the Windows IGDv2-incompatible client by returning an infinite (0) lease duration. To fix listing and editing via GUI (Explorer/Network), if daemon was compiled with IGDv2
Extend detection to older versions of Windows and add Xbox
Detect Apple IGDv2-incompatible clients and apply existing workaround, that only caused problems if PCP/NAT-PMP (prioritised) was disabled

(to merge with prior)

Link: https://github.com/Self-Hosting-Group/miniupnp/tree/upnp-igdv2-compat

75	0	net/miniupnpd/patches/010-upnp-igdv2-compat.patch

3. Patch to fix description filter option

To fix the non-working description regex filter option

(to merge with prior)

Link: https://github.com/miniupnp/miniupnp/pull/853

511	0	net/miniupnpd/patches/020-descr-filter-fix.patch

4. Package revision and new/updated UCI options

The following settings UCI options been added or changed, and the previous options are migrated on updating:

upnpd.config UCI options	Change	Previous name
enabled	Match default (1)
enabled_protocols=upnp-igd	Combined option	enable_upnp=1
enabled_protocols=pcp+nat-pmp	Combined option	enable_natpmp=1
allow_cgnat	Allow allow-filtered (2)	use_stun
stun_host	Allow port inclusion (3)
stun_port	Removed, included in host
allow_third_party_mapping=0	Inverted/extended to PCP	secure_mode=1
log_output	Allow info log level
lease_file	Set by default + IPv6 (4)	upnp_lease_file
upnp_igd_compat=igdv1	Renamed/match default (1)	igdv1=1
download_kbps	In kbit/s and renamed (5)	download
upload_kbps	In kbit/s and renamed (5)	upload
friendly_name	New option, router name
http_port	Renamed, rem. if default	port
notify_interval	Removed if <900s, minimum
internal_iface	Migrated, new section

internal_network UCI options	Change	Previous name
interface	New option
access_preset	New option (6)
accept_ports	New option (6)
reject_ports	New option (7)
custom_acl_before	New option (6)

Notes:

Init UCI default now matches LuCI and initial config file defaults for: enabled=0 and upnp_igd_compat=igdv1
Allow allow-filtered for IPv4 CGNAT use and migrate option from X-Wrt and only use STUN when necessary with a private/CGNAT external IPv4
Remove known incompatible STUN servers and set compatible by default
Configure undocumented daemon option lease_file6=${lease_file}-ipv6 so that active IPv6 port maps are not lost when service restarts, e.g. by deleting an active port map. Use /run path, symlinked and appeared in FHS 3.0 in 2015 and remove option if UCI default is set
Gets converted, config file now defaults to interface link speed instead of 8/4 Mbit/s, which is removed on migration
New options added to select a preset for all devices on the network and decide if the custom ACL should be checked before the preset. Extra ports can also be set that are accepted/rejected. Presets: accept-high-ports/accept-high-ports+web[+dns]/accept-all-ports or 0
Reject ports regardless of other settings. By default reject unsafe: 21 (FTP), 23 (Telnet), DCE/NetBIOS/SMB (135/137-139/445), RDP (3389)

Code refactoring:

Add a function for logging and output to stderr, and extend logging
Revise daemon init/config-gen slightly by declare all UCI options (incl. booleans) according to the same principle and remove upnpd_write_bool
Document and reformat default /etc/config/upnpd UCI config file

Depends on: https://github.com/openwrt/luci/pull/7822

Fix: https://github.com/openwrt/packages/issues/17413

2	0	net/miniupnpd/Makefile
142	64	net/miniupnpd/files/miniupnpd.init
155	0	net/miniupnpd/files/upnpd-migration.uci-defaults
32	24	net/miniupnpd/files/upnpd.config

5. Group/rearrange config-gen and refactoring

Group and rearrange UCI option declaration and config-gen by function/LuCI UI, and comment
Encode required XML entities of text UPnP IGD config options until the daemon does so using the created function xml_encode
Only generate UPnP IGD config if the protocol is enabled

(to merge with prior)

65	60	net/miniupnpd/files/miniupnpd.init

6. Rename UCI section name to `settings` (v2.0)

Inspired/address copilot's PR review for a clearer config by rename UCI section name config (v1.0) -> settings (v2.0), helps on migration and to distinguish the updated config from the previous one easily

(to merge with prior)

2	2	net/miniupnpd/files/firewall3.include
3	3	net/miniupnpd/files/miniupnpd.hotplug
25	25	net/miniupnpd/files/miniupnpd.init
7	0	net/miniupnpd/files/upnpd-migration.uci-defaults
1	1	net/miniupnpd/files/upnpd.config

7. Add second CGNAT UCI option

Alternative option to STUN allow-filtered. As requested by AquanJSW, to test with Tailscale. Also adds the required daemon fix. No public IPv4 address detection; issues with multiple clients, e.g. PCP/NAT-PMP

(proposed for inclusion, to merge with prior)

1	1	net/miniupnpd/files/miniupnpd.init
18	0	net/miniupnpd/patches/030-allow-private-fix.patch

8. Update custom ACL options, migrate section

Note that the custom ACL is now rejected by default, if it is alone used, with no preset or listed accepted ports. Add (ignored) custom ACL template entries on migration
Migrate custom ACL entries to the new section name acl_entry
The following custom ACL UCI options been added or changed, and the previous options are migrated on updating:

acl_entry UCI options	Change	Previous name
action	New/updated values (1)
int_port	Remove colon separator (2)	int_ports
ext_port	Remove colon separator (2)	ext_ports
descr_filter	New option (3)

Allow ignore, and update action option to use the nftables terms (allow/deny -> accept/reject). To avoid adding inverted actions when changing via LuCI, ensure any missing are set, as LuCI and UCI had not matching action defaults. Missing actions are now ignored/logged
Ensure that the hyphen (-) is only used as a port range separator by migration, as the colon (:) is not valid in LuCI
Add missing UCI option to set a regular expression to check for a UPnP IGD IPv4 port map description, and fix the current collision with the comment field which was not noticed due to a daemon bug https://github.com/openwrt/packages/pull/24495 https://github.com/miniupnp/miniupnp/pull/853

Code refactoring:

Add a more universal usable is_port_or_range function instead of upnpd_get_port_range and check if it has a valid range, and removes a shellcheck warning
Rename conf_rule_add function to upnpd_add_custom_acl_entry

(to merge with prior)

29	52	net/miniupnpd/files/miniupnpd.init
82	0	net/miniupnpd/files/upnpd-migration.uci-defaults
11	10	net/miniupnpd/files/upnpd.config

9. Separate service start and config-gen

Remove config_foreach upnpd "upnpd" and replace it with regular function call, as init was not designed for a multi-instance setup, as the same tmpconf will be used/overwritten, and non-anonymous section
Move code to make the custom vs. config file generation decision earlier, and only perform external interface detection with the second one, and rename function upnpd to upnpd_generate_config
Replace unnecessary if cases with elif in init/hotplug
Exit with 1 on errors to get an inactive service status
Do not restart daemon in hotplug when using a custom config file, as then this file will not be regenerated on restarts
Use procd_add_reload_trigger "firewall" instead of listening /etc/config/firewall

(to merge with prior)

13	21	net/miniupnpd/files/miniupnpd.hotplug
62	70	net/miniupnpd/files/miniupnpd.init

10. Rearrange init and format `firewall3.include`

Arrange start_service and main init functions first
Format firewall3.include using shfmt

(to merge with prior)

22	22	net/miniupnpd/files/firewall3.include
78	78	net/miniupnpd/files/miniupnpd.init

(The italic commits are intended to be merged with the prior ones after review.)

Screenshots

The new network-wide access control functionality… can best be described using the LuCI screenshots:

Enabled Networks / Access Control (new)

Edit network access control settings (new)

Advanced Settings tab with new CGNAT functionality

UPnP IGD Adjustments tab (new)

LuCI notification if the related package is not updated (new)

Full LuCI screenshot

Depends on LuCI PR: https://github.com/openwrt/luci/pull/7822 The first commit here has no dependencies and is intended for early cherry-picking. Tested on: OpenWrt 24.10.4 and current snapshot

The Port Control Protocol (PCP) is the successor to NAT-PMP, shares similar protocol concepts and packet formats, but supports IPv6 port mapping and options/extensions. For more information, see: Port Mapping Protocols Overview and Comparison 2025: About UPnP IGD & PCP/NAT-PMP https://github.com/Self-Hosting-Group/wiki/wiki/Port-Mapping-Protocols-Overview

Sep 18 '24 07:09 Self-Hosting-Group

@Self-Hosting-Group: Nice PR!

cc: @systemcrash

Nov 08 '24 11:11 Neustradamus

A downgrade included in a patchset won't get accepted, since a downgrade may subtly reintroducing bugs for existing users, if we assume that point releases fix bugs only. Better to wait for a new release, and bump to that version.

Migrations are probably a more serious matter: those must be carried basically 'forever'. The best way is simply to avoid those. One might introduce a new setting, and deprecate the old one, and change the UI over to use the new one. Still a bit of a bumpy road.

I think personally this is minor in the grand scheme of things (rather unimportant settings), but other reviewers may take a much firmer stance on it since you are, after all, changing setting names.

Jan 06 '25 15:01 systemcrash

What do you think about backporting the commit?

Acceptable. It just breaks compile at the next release bump when it no longer applies. Minor, I guess.

Jan 06 '25 17:01 systemcrash

Every single test-build failed: Dirty patches detected, please refresh and review the diff

Jan 13 '25 14:01 systemcrash

This solve my problem https://github.com/openwrt/openwrt/issues/18011 While I search this problem I found https://community.wd.com/t/why-my-cloud-home-make-so-much-upnp-forwarding-up-to-115/473182 with this problem And I know several people from 4pda.to that downgraded from 24.10 to 23.05.5 because of this problem

Mar 03 '25 15:03 abubaca4

OK, 2.3.8 is finally here. That's what's required.

ping @BKPepe @neheb

Apr 04 '25 16:04 systemcrash

Several commits tend to not follow guidelines, I suggest to check what you can find here:

https://openwrt.org/submitting-patches

Subjects are too long, some of them have really short commit description or even none at all.

Apr 04 '25 16:04 BKPepe

Just motivate briefly each commit; its reasoning. Separate commits are acceptable, and here possibly necessary in case any one step turns out to be problematic.

Apr 05 '25 13:04 systemcrash

@systemcrash I just deleted obsolete comments here, e.g. no more downgrade, and thanks for the knowledge transfer... So maybe it would be good to delete them before we continue here, for a cleaner history. OK?

Maybe a new PR instead of this would be better?

Apr 29 '25 13:04 Headcrabed

Any comments?

Better. You've just given a description of the changes, not motivated the reason that the migrations must be there. Any other reviewer will look at the migration section and think 'do we really have to carry that forever?'. And in short, the answer is yes, unless you document this in the wiki, that a user must upgrade through certain versions to be sure a migration works, so that migration code can be dropped later on.

Jun 06 '25 12:06 systemcrash

@Self-Hosting-Group: Oh yes, good catch, a new version was here:

https://github.com/miniupnp/miniupnp/releases

Jun 11 '25 10:06 Neustradamus

LuCI: How can the stun_host UI element only be required if it is enabled by the dependency (use_stun)? I was able to make the field unconditionally required by a LuCI function validate, but I could not make it required only if the field is also enabled (using isActive). Any tips?

This question is off-topic here, and should be posed in the luci repo (but you should consult the luci online documentation).

Jun 17 '25 11:06 systemcrash

Fine by me.

Jul 16 '25 14:07 systemcrash

Any comments or an expected date of acceptance?

Jul 23 '25 12:07 Self-Hosting-Group

Sure, if it is ready. It doesnt look like it since every day you are doing some changes and force pushing it. It makes it harder for review..

Jul 23 '25 13:07 BKPepe

I'm fine with the changes. Just stop making them unless you're asked to do so.

Jul 25 '25 23:07 systemcrash

I will remove myself as a reviewer here - stop making changes.

@copilot

Oct 20 '25 11:10 systemcrash

I will remove myself as a reviewer here - stop making changes.

OK. Thanks for the response. Perhaps you could review the LuCI PR. There are hardly any changes to be expected there.

Oct 20 '25 11:10 Self-Hosting-Group

OK. Thanks for the response. Perhaps you could review the LuCI PR. There are hardly any changes to be expected there.

Get this merged first.

Oct 20 '25 13:10 systemcrash

I've looked right now at only one commit, which should be quick and the easiest one to get merged: https://github.com/openwrt/packages/pull/24988/commits/3e4c15d3bb7a5adcb65de011be4d61159c463c5a

I haven't reviewed the other commits yet, but to me, the overall description seems a bit too extensive. The use of redirect.github.com is interesting, and I understand the reasoning behind it — every force push would otherwise spam the related issue, which makes sense. However, the strikethrough text doesn’t work properly in the GitHub web UI, so it can be removed.

Also, I disagree with using a mirror that is only available over HTTP. That change should be done in a separate commit. If the project cannot ensure the reliability of its main website, even with the use of a CDN if necessary, that’s rather concerning.

I would focus only on the changes that are clear, simple, and low-risk — such as updating to the latest version, removing obsolete parts, or disabling unnecessary features.

I haven’t checked the remaining commits yet, because there are constant changes — new commits, updates to existing ones — and I’m hesitant to merge something that might immediately require follow-up fixes. It doesn’t feel ready to be merged yet.

Additionally, I’m concerned that the Signed-off-by line does not include a real name, which might be an issue for compliance.

Oct 20 '25 17:10 BKPepe

@rbqvq: What do you think about this PR?

Nov 05 '25 13:11 Neustradamus

Hello @BKPepe. Now, no new pushes to this repository, package works well! Finally, some code was moved and now I think is the right time to review the following commits. ;-) The mirror issues are addressed by using the official GitHub releases, see below. And thanks for your comments.

https://github.com/openwrt/packages/blob/5ca92d916df664d6318fbaf488a0ef03d70b4962/net/miniupnpd/Makefile#L14

The commits were recreated in a better order, and are easier to review
Thanks to optimisations, the additional code for the new features in init is only ~37 lines, not counting the migration code
I have deleted the comment with the questions. Because everything, including the separate service start and config-gen, and cancel earlier if no migration is necessary, has now been implemented or has been resolved in the meantime

In my eyes, everything fits, and I don't see anything else that needs to be done. The optimisations took a while, but it's now ready for review.

I've looked right now at only one commit, which should be quick and the easiest one to get merged: aa7e58c

Yes, this commit was intended for cherry-picking, as were the two LuCI commits, as preparation to display a message to update in the event of an unsuitable config file being found, and get prepared for the section change. These commits have no dependencies.

Daemon PR:

update daemon to 2.3.9 to fix issues and refresh building
patch for UPnP IGD compatibility with Microsoft/Apple clients
patch to fix description filter option
new/revised UCI options and improve daemon init/config-gen
group/rearrange config-gen and refactoring
new/revised custom ACL UCI options and migrate section
rename UCI section name to settings (v2.0)
refactoring by separate service start and config-gen
rearrange init functions

LuCI PR:

improve existing UI slightly
add UPnP IGD Adjustments tab, rearrange as many options
ui redesign and adapt to new/revised package UCI options
new/revised custom ACL UCI options and migrate section
rename UCI section name to settings (v2.0)

(The commits in bold were for cherry-picking, and those in italics were intended to be merged with the previous commit.)

This PR has now implemented all the functions, from my point of view. The following LuCI screenshots are provided to help you understand the new/extended network-wide access control functionality during the review process. Thanks to the universal implementation of the acl_accept_ports/acl_reject_ports options, this functionality provides a more powerful and useful alternative to existing custom ACL entries in many user cases.

The new network-wide access control functionality can best be described using the LuCI screenshots:

Enabled Networks / Access Control

Edit network access control settings

Advanced Settings tab with new CGNAT functionality

New UPnP IGD Adjustments tab

LuCI notification if the related package is not updated

Full LuCI screenshot

Nov 10 '25 13:11 Self-Hosting-Group

Hello @systemcrash.

Could you please advise on the wording for the additional tab in LuCI? I have currently named it UPnP IGD Adjustments. Now I would like to to add a header with the same name in OPNsense. I used a different word to Settings because these options are rarely changed and are usually just for adjusting reported values, such as the friendly name. The following options are included: download_kbps, upload_kbps, friendly_name, model_number, serial_number, presentation_url, uuid, http_port, notify_interval. (italic options are now hidden in LuCI, and upnp_igd_compat is exceptionally in Service Setup) I am OK with the current wording, but perhaps you have a suggestion for a better and shorter alternative?
The same for this UI option: UPnP IGD compatibility mode (shorter/better wording?) Report a specific device to workaround IGDv2-incompatible clients (or) Act/emulate as a specific/different device to workaround/support/handle/bypass/assist/mitigate IGDv2-incompatible clients (alternative text welcome) Options:

IGDv1 (IPv4 only)
IGDv2 (with workarounds)

Custom Access Control List: Good wording and capitalisation? Also for for Service Setup, Advanced Settings and Enabled Networks / Access Control? Is there a guideline for the capitalisation in headlines and tabs?

This would help me a lot to continue working on OPNsense and to complete the PR there next week. I also think that some standardisation between new introduced options in the router OSs makes sense.

Nov 10 '25 13:11 Self-Hosting-Group

Hello @Rondom @kilroy98 @AquanJSW @Neustradamus

You all responded positively to my PR description. I think it will increase the reviewer's confidence if not only I have tested the new, revised daemon and LuCI packages in practice, but also if you find the new functionality useful for your user cases.

If you do not have an OpenWrt build environment for creating the new packages, I can compile them for easy installation if you wish. To do this, I need the router's Target Platform (from Status/Overview).

Nov 10 '25 15:11 Self-Hosting-Group

Just compiled and tested on my machine, it seems to be working fine. But no plan to add ext_allow_private_ipv4 related option?

Nov 10 '25 16:11 aquanjsw

Great! ;-) Thanks for testing.

Just compiled and tested on my machine, it seems to be working fine. But no plan to add ext_allow_private_ipv4 related option?

https://github.com/openwrt/packages/blob/5ca92d916df664d6318fbaf488a0ef03d70b4962/net/miniupnpd/files/miniupnpd.init#L123-L124

Setting the existing UCI option external_ip to, e.g., 203.1.2.3 (public IPv4) is a better solution in terms of client compatibility than using ext_allow_private_ipv4=yes.

The ext_perform_stun=allow-filtered daemon option, which was added shortly afterwards (my contribution, unfortunately not published under my name because the maintainer (@miniupnp) blocked me), is IMHO better, as it returns the correct IPv4.

Even though ext_allow_private_ipv4=yes option had to be corrected after its introduction and unexpectedly changed the logging (new issue), I am no longer sure whether it should have been introduced at all, with all this trouble and regression. I know of others that were recently introduced, but the maintainer does not want me to submit them to get resolved.

And the reason why allow-filtered with STUN is necessary at all under OpenWrt with an unrestricted endpoint-independent IPv4 CGNAT is that the CGNAT test in the daemon has always been badly implemented. This requires that all UDP ports (random test ports) on the external IPv4 are open, which is not the case in any modern route OS for security reasons, instead of what would make much more sense, namely adding regular port maps to itself (internal IP) for the test, which would then work directly everywhere. This idea was also well received by a FreeBSD developer.

Screenshot of Advanced Settings tab with new CGNAT functionality

Nov 10 '25 18:11 Self-Hosting-Group

(PS: I also heard about a recent bug bounty where a user liked a bug fix, but he gave up on his plan after contacting the maintainer, who maybe have lost interest in the project.)

Nov 10 '25 18:11 Self-Hosting-Group

@Self-Hosting-Group: Good job since more one year about it. I hope to see the merging one day! Maybe you can update the title, it is not only for 24.10...

Nov 10 '25 18:11 Neustradamus

Great! ;-) Thanks for testing.

Just compiled and tested on my machine, it seems to be working fine. But no plan to add ext_allow_private_ipv4 related option?

https://github.com/openwrt/packages/blob/5ca92d916df664d6318fbaf488a0ef03d70b4962/net/miniupnpd/files/miniupnpd.init#L123-L124

Setting the existing UCI option external_ip to, e.g., 203.1.2.3 (public IPv4) is a better solution in terms of client compatibility than using ext_allow_private_ipv4=yes.

The ext_perform_stun=allow-filtered daemon option, which was added shortly afterwards (my contribution, unfortunately not published under my name because the maintainer (@miniupnp) blocked me), is IMHO better, as it returns the correct IPv4.

Even though ext_allow_private_ipv4=yes option had to be corrected after its introduction and unexpectedly changed the logging (new issue), I am no longer sure whether it should have been introduced at all, with all this trouble and regression. I know of others that were recently introduced, but the maintainer does not want me to submit them to get resolved.

And the reason why allow-filtered with STUN is necessary at all under OpenWrt with an unrestricted endpoint-independent IPv4 CGNAT is that the CGNAT test in the daemon has always been badly implemented. This requires that all UDP ports (random test ports) on the external IPv4 are open, which is not the case in any modern route OS for security reasons, instead of what would make much more sense, namely adding regular port maps to itself (internal IP) for the test, which would then work directly everywhere. This idea was also well received by a FreeBSD developer.

Screenshot of Advanced Settings tab with new CGNAT functionality

Thanks for your explanation, but there actually exist some scenarios that only ext_allow_private_ipv4 can resolve, at least in my test.

Hope we can add this in the future when it becomes stable.

Nov 11 '25 04:11 aquanjsw

Thanks for your explanation, but there actually exist some scenarios that only ext_allow_private_ipv4 can resolve, at least in my test.

Could you please explain this in more detail and describe a scenario in which the STUN function cannot work with allow-filtered?

Hope we can add this in the future when it becomes stable.

How? The fact that some PCP/NAT-PMP clients are not compatible when a private IPv4 address is returned will not change. They require a public IPv4 address to function, with these protocols, it is required that the external IP address (reported by the router) must be sent along with the renewal. Therefore, I see the only option as sending a public IPv4 address so that port mapping works with all clients. And I don't want to suggest new, additional options that create compatibility issues.

Removing the comment of this code would work immediately and would be a more compatible alternative to the daemon option if someone does not want to use STUN:

# [ "$allow_cgnat" = "allow-private-ext-ipv4" ] && external_ip=203.1.2.3

If we have a use case, I will gladly enable the code.

Nov 11 '25 07:11 Self-Hosting-Group