luci-app-upnp: Revision, new access control and UCI options…

[Self-Hosting-Group]

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

1. Improve existing UI

• Workaround daemon bug to fix listing of active port maps with iptables >= 1.8.8, and relax nftables parsing for the new release
• Change Active Service Port Maps to Active Port Maps, use the same wording for the table headings and ACL as on the overview status page
• Change description field header to Added via / description, always include the protocol and clearer/less redundant protocol labels
• Disable IPv6 mapping (ipv6_disable): UI option added, UCI exists
• Don't clear dependent options if UPnP IGD/STUN temporarily disabled
• Set notify_interval minimum to 900 s (default), as recommended by UDA 1.1 (2x=1800 in the standard), because daemon/OpenWrt wrongly suggested 30x less in the past, and to reduce multicast traffic and power consumption in wireless networks, clearer help
• Report system instead of service uptime, UUID and service lease file: UI options removed (disabled, to keep translations) as rarely used
• Slightly improve some text strings
• Show hint if no suitable configuration found (missing related update), link to package manager, and show form as read-only to prevent changes
• Configure service lease file by default in rpcd ucode for full UI functionality without option set
• Refactoring by moving ubus connection as jow-/ucode@a58fe47 was merged
• Adapt to JavaScript ES6 and remove some line breaks and format files
• A commit has been prepared that manually adjusts the translations, without losing them

Fixes: openwrt/openwrt#12843

2. Add UPnP IGD Adjustments tab

And rearrange as many options

(to merge with prior)

3. Revision and adapt to new UCI options

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

• Only display Active Port Maps if the service is enabled and Access Control List if it is used
• Enable protocols (enable_protocols): Combined UI option added
• Allow CGNAT/STUN (allow_cgnat): Accept new values for IPv4 CGNAT use and update help with newer wording of RFC 5780
• STUN server (stun_host): Allow port inclusion
• STUN port: Removed, as now accepted in STUN server
• Override external IPv4 (external_ip): UI option added for CGNAT use
• Allow third-party mapping (allow_third_party_mapping): Inverted from secure mode and optionally extended to PCP
• Log level (log_output): Allow info log level
• UPnP IGD compatibility (upnp_igd_compat): Reworded/extensible
• Download/upload speed (download_kbps/upload_kbps): In kbit/s and datatype set, now, interface link speed by default
• Router/friendly name (friendly_name): UI option added to set name displayed in Windows Explorer, model/serial number removed
• Enable Networks / Access Control (internal_network): Section added to select the enabled networks and their access control. By:
  • Internal network (interface): UI option added to select the local/internal (LAN) network interface to enable the service for
  • Access defaults (access_defaults): UI option added to select access defaults for ports that all devices on the network can map
  • Accept extra ports (accept_ports): UI option added to accept these ports or port ranges on the network as well
  • Reject ports (reject_ports): UI option added to reject ports on the network; overrides other settings
  • Check ACL (check_acl): UI option added to check the ACL entries first; these extend/override the defaults
• Slightly improve introduction text

More details on changed options can be found in the dependent package PR

Fixes: #8313
Fixes: #8677
Depends on: openwrt/packages#28765

4. Rename UCI section to settings v2.0

Rename UCI section config (v1.0) -> settings (v2.0), helps on migration and to distinguish the updated config from the previous one

(to merge with prior)

5. Update ACL options, migrate section

• Disableable and cloneable ACL entries, always translated Action
• Improve UI with direct editability, clearer help wording, and rename to Access Control List
• The ACL is now rejected last if not accepted by access defaults. Add (disabled) ACL template entries on migration
• Migrate ACL entries to the new section name acl_entry
• The following 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)

1. Allow disabled, 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
2. Ensure that the hyphen (-) is only used as a port range separator by migration, as the colon (:) is not valid in LuCI
3. 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 miniupnpd: Update to 2.3.7 and enable regex filter packages#24495 miniupnpd: Rewrite permission line parser miniupnp/miniupnp#853

(to merge with prior)

6. List active IPv6 port maps

Supports nftables and ip6tables; IPv6 port maps not yet deletable, as not parsing extra IPv6 lease file

(Note: Daemon's IPv6 lease file differs from the IPv4 format in order and separator. With IPv4, the remote IP field is missing and not retained during daemon restarts. IMHO, it would be better to have a complete/combined IPv4/IPv6 lease file upstream, with unique mapping IDs for reliable deletion in the 1st field, with a standardised order and more fields, and then use it without rewriting the parsing.)

(to merge with prior)

Fixes: #2985
Fixes: #5197
Fixes: #8567

7. Service-wide access control settings

This implements variant B

upnpd.settings UCI options	Change	Previous name
access_defaults	New option (1)
accept_ports	New option (1)
reject_ports	New option (2)
check_acl	New option (1)

Notes:

1. New options added for default ports that all devices can map, client-specific permissions using the access control list (ACL) can extend/override the defaults. Access defaults: accept-high-ports accept-web+high-ports/accept-web-ports/accept-all-ports
2. Reject ports; overrides other settings. By default reject unsafe: FTP (21), Telnet (23), DCE/NetBIOS/SMB (135/137-139/445), RDP (3389)

(to merge with prior)

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

Screenshots

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

New Access Control tab

Advanced Settings with new CGNAT functionality

UPnP IGD Adjustments tab (new)

LuCI notification if the related package is not updated

Full LuCI screenshot

Depends on packages PR: openwrt/packages#28765
Tested on: OpenWrt 24.10.8/25.12.5 with iptables/nftables
Maintainer: @jow-

miniupnpd: Core functionality issues
https://github.com/Self-Hosting-Group/miniupnpd-issues

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 2026+: About UPnP IGD & PCP/NAT-PMP
https://github.com/Self-Hosting-Group/wiki/wiki/Port-Mapping-Protocols-Overview

[openwrt-ai]

Commit checks

• c29a7b9ea6 "luci-app-upnp: update ACL options, migrate section" — the body's last two bullets describe shell-init refactors (is_port_or_range/upnpd_get_port_range, renaming conf_rule_add to upnpd_add_acl_entry, removing a shellcheck warning), but the only file touched is htdocs/.../upnp/upnp.js. Those changes belong to the dependent openwrt/packages PR; either drop them from this commit's body or split the message to clarify what landed here.
• Several commits (de0657afcb, 75a1884052, c29a7b9ea6, 338e2fdc73, 30933b757c) carry (to merge with prior) markers — please squash before merging so the history doesn't ship those notes.

(General note, not blocking inline: this PR's UCI schema is co-dependent on openwrt/packages#28765 — without that, the migration to settings/acl_entry/internal_network does not happen and the LuCI page renders the "No suitable configuration was found" banner on existing deployments. Worth surfacing this dependency in the PR title/body too, not just the commit message.)

---

Generated by Claude Code

[openwrt-ai]

Re-reviewed after the force-push that reworked the series (the previous review's commits are no longer in history, so this pass treats the current diff as new). One new issue found inline; the earlier notes about the (to merge with prior) squash markers and the openwrt/packages#28765 co-dependency still apply and are not repeated here.

---

Generated by Claude Code

[github-actions]

Warning

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an ❌ are failing checks.

Commit 10aaf18

• 🔶 Author name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'
• 🔶 Commit(ter) name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'

Commit 54c9c4c

• 🔶 Author name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'
• 🔶 Commit(ter) name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'

Commit 8d854bb

• 🔶 Author name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'
• 🔶 Commit(ter) name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'

Commit f6ff2c8

• 🔶 Author name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'
• 🔶 Commit(ter) name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'

Commit cfe6b2e

• 🔶 Author name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'
• 🔶 Commit(ter) name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'

Commit 61fc4f0

• 🔶 Author name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'
• 🔶 Commit(ter) name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'

Commit 4316994

• 🔶 Author name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'
• 🔶 Commit(ter) name must be either a real name 'firstname lastname' or a nickname/alias/handle
  Actual: Self-Hosting-Group seems to be a nickname or an alias
  Expected: a real name 'firstname lastname'
• 🔶 Commit message lines should be <= 80 characters long
  Actual: line 26 is 84 characters long
  $\textsf{[UDA 1.1]: https://upnp.org/specs/arch/UPnP-arch-DeviceArchitecture-v1.1.pdf\#pag\color{red}{e=30}}$

For more details, see the full job log.

_{Something broken? Consider providing feedback.}

[openwrt-ai]

The branch was rebased since the last review (the previously reviewed commit 98ea0ba is no longer in history), so this pass treats the current 7-commit series as new.

Commit checks

• cfe6b2e "luci-app-upnp: revision and adapt to new UCI options" — the message describes an "Enable Networks / Access Control (internal_network)" section with a sub-option "Internal network (interface)", but the code adds no internal_network section and no interface option. The network selector is implemented as option internal_iface in the settings section, and the access_defaults/accept_ports/reject_ports/check_acl options the bullet lists under that section are placed in settings (the Access Control tab) instead. The later commit 10aaf18 "service-wide access control settings" notes it "implements variant B", which looks like it superseded the per-network design this body still describes. Is the cfe6b2e body just stale relative to the variant-B implementation? If so, please reword it to match what actually lands (internal_iface in settings, service-wide access-control options) so the history isn't misleading.

Not re-flagged (already disclosed in the PR description): the (to merge with prior) markers on 5 of 7 commits (the body states the italic commits are to be squashed after review), and the co-dependency on openwrt/packages#28765 for the config→settings/acl_entry UCI migration. I confirmed against the current openwrt/packages master that internal_iface and lease_file are real backend options, so the renamed schema is backend-grounded once that PR merges.

---

Generated by Claude Code

[openwrt-ai]

The 7-commit series was rebased since my last review (10aaf18 is no longer in history), so this pass re-examines the current head 5550b4a. One new inline issue below.

Still applicable from earlier passes, not re-flagged here as they are already disclosed in the PR description: the (to merge with prior) squash markers on the italic commits, and the co-dependency on openwrt/packages#28765 for the settings / acl_entry / int_addr UCI schema (I re-confirmed against openwrt/packages master that int_addr and internal_iface are real miniupnpd backend options). The unresolved null-dereference when toggling check_acl from off (existing inline thread) also still stands in the current code.

---

Generated by Claude Code