Skip to content

Conversation

@Self-Hosting-Group
Copy link
Contributor

@Self-Hosting-Group Self-Hosting-Group commented Jun 17, 2025

Commits

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

1. Improve existing UI slightly
  • 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

Pre-update to comming PR: #7822

2. Add `UPnP IGD Adjustments` tab

And rearrange as many options

(to merge with prior)

3. Revision and adapt to updated package 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): Allow new option for IPv4 CGNAT use (allow filtered), and updated 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 output level (log_output): Allow info log level, and reworded
  • 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 preset (access_preset): UI option added to select an access control preset for ports that all devices on this network can map
    • Accept extra ports (accept_ports): UI option added to accept these ports or port ranges on this network as well
    • Reject ports (reject_ports): UI option added to reject ports on this network; override other settings
    • Ignore ACL (ignore_acl): UI option added to not check ACL entries before a preset; can extend/override a preset
  • Slightly improve introduction text

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

Depends on: openwrt/packages#24988

4. 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)

5. 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 STUN public IPv4 detection; various issues, e.g. with PCP/NAT-PMP clients

(proposed for inclusion, to merge with prior)

6. Update ACL options, migrate section
  • Ignorable and cloneable ACL entries, always translated Action
  • Improve UI with direct editability, clearer help wording, and rename to Access Control List
  • Note that the ACL is now rejected by default, with no preset and accepted extra ports. Add (ignored) 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 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
  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
  • Refactoring by adding 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_acl_entry

(to merge with prior)

(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:

Enable Networks / Access Control (new) luci-network-access-control
Edit Network Access Control Settings (new) luci-network-access-control-edit
Advanced Settings tab with new CGNAT functionality luci-advanced
UPnP IGD Adjustments tab (new) luci-igd
LuCI notification if the related package is not updated (new) luci-notification
Full LuCI screenshot luci-screenshot

Depends on packages PR: openwrt/packages#24988
The first two commits here have no dependencies and are intended for early cherry-picking
Tested on: OpenWrt 24.10.5 and 25.12.0-rc3
Maintainer: @jow-

Wanted: Newer Microsoft Xbox (One/Series) console users with OpenWrt to provide UPnP IGD logs as specified in openwrt/packages#24988 (comment) (updated package not necessary).

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

@Self-Hosting-Group
Copy link
Contributor Author

Placeholder comment

@systemcrash
Copy link
Contributor

Hi - please don't open PRs here until you've finished with the PR in the packages repo first.

@hnyman hnyman added the depends on PR in other repo PR depends on PR in sister repo e.g. openwrt/packags label Jun 17, 2025
@hnyman
Copy link
Contributor

hnyman commented Jun 17, 2025

Reference to openwrt/packages#24988

@Neustradamus
Copy link

@Self-Hosting-Group: Thanks for this PR!

@openwrt openwrt deleted a comment from Self-Hosting-Group Jun 19, 2025
@Self-Hosting-Group Self-Hosting-Group force-pushed the upnp-revision branch 2 times, most recently from 5c69300 to 70f10e8 Compare June 30, 2025 06:01
@Self-Hosting-Group Self-Hosting-Group marked this pull request as ready for review June 30, 2025 06:02
@openwrt openwrt locked and limited conversation to collaborators Jun 30, 2025
@Self-Hosting-Group Self-Hosting-Group force-pushed the upnp-revision branch 9 times, most recently from f7ee946 to 93eac3f Compare July 7, 2025 07:27
@Self-Hosting-Group Self-Hosting-Group force-pushed the upnp-revision branch 10 times, most recently from a7e70d4 to 37b06b0 Compare July 9, 2025 16:12
@github-actions

This comment has been minimized.

@Self-Hosting-Group Self-Hosting-Group force-pushed the upnp-revision branch 2 times, most recently from ec0b876 to 645aadf Compare January 5, 2026 17:01
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@Self-Hosting-Group
Copy link
Contributor Author

Unfortunately I don't have access to this repo ;( @ldir-EDB0 and @systemcrash may interested in this

Thanks for checking. systemcrash would also be good for his wording, JS and LuCI expertise for the first two, and the translation adaptation commit.

@Self-Hosting-Group Self-Hosting-Group force-pushed the upnp-revision branch 6 times, most recently from 645aadf to 890f2f4 Compare January 9, 2026 17:18
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@Neustradamus
Copy link

About all the warnings like here, I have already informed the problem with the author but no changes yet about:

🔶 Author name (Self-Hosting-Group) seems to be a nickname or an alias
🔶 Committer name (Self-Hosting-Group) seems to be a nickname or an alias

The problem is same for this PR:

@Neustradamus
Copy link

Attention: To add more informations, warnings specified in my previous comment are here, it is not from me (I have previously cited only the latest one):

🔶 Author name (Self-Hosting-Group) seems to be a nickname or an alias
🔶 Committer name (Self-Hosting-Group) seems to be a nickname or an alias

Recall:

  • @Self-Hosting-Group has been banned by miniupnp team.
  • I have already informed the PR author about the needed changes (nothing must be merged before changes).

The OpenWrt "Submission Guidelines" is here and clear:

It is specified:

  • all commits must contain Signed-off-by: My Name <my@email.address> where you write your real name and real email address, in accordance with Section 11 of the Linux Kernel patches guide: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=HEAD#n416
  • GitHub web interface or GUI application for git: you must append the Signed-off-by: line manually in the commit description
  • git command-line interface:
    git commit --signoff
  • the Author field must match the Signed-off-by: line
  • GitHub web interface: you must specify your real name in the Name field and the Primary email address to match the Signed-off-by: line
  • git command-line interface:
    git config --global user.name "my name"
    git config --global user.email "my@email.address"

@github-actions

This comment has been minimized.

@Self-Hosting-Group Self-Hosting-Group force-pushed the upnp-revision branch 2 times, most recently from 1cd7cb9 to f11066b Compare January 21, 2026 16:14
@github-actions

This comment has been minimized.

@github-actions
Copy link

Warning

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an ❌ are failing checks.

Commit f11066b

  • 🔶 Author name (Self-Hosting-Group) seems to be a nickname or an alias
  • 🔶 Committer name (Self-Hosting-Group) seems to be a nickname or an alias

Commit 2faa58b

  • 🔶 Author name (Self-Hosting-Group) seems to be a nickname or an alias
  • 🔶 Committer name (Self-Hosting-Group) seems to be a nickname or an alias

Commit 0d99e08

  • 🔶 Author name (Self-Hosting-Group) seems to be a nickname or an alias
  • 🔶 Committer name (Self-Hosting-Group) seems to be a nickname or an alias
  • 🔶 Commit subject length: recommended max 50, required max 60 characters

Commit eba65fb

  • 🔶 Author name (Self-Hosting-Group) seems to be a nickname or an alias
  • 🔶 Committer name (Self-Hosting-Group) seems to be a nickname or an alias
  • 🔶 Commit subject length: recommended max 50, required max 60 characters

Commit 9d98f9d

  • 🔶 Author name (Self-Hosting-Group) seems to be a nickname or an alias
  • 🔶 Committer name (Self-Hosting-Group) seems to be a nickname or an alias

Commit 146f4da

For more details, see the full job log.

Something broken? Consider providing feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

depends on PR in other repo PR depends on PR in sister repo e.g. openwrt/packags

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants