Skip to content

net/upnp: Complete service improvements 2/2 #5256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fichtner merged 3 commits into from
Mar 4, 2026
Merged

net/upnp: Complete service improvements 2/2 #5256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcca2b7
net/upnp: Complete service improvements 2/2 #5256
Self-Hosting-Group Mar 2, 2026
1de9ce3
Apply suggestion from @fichtner
fichtner Mar 4, 2026
8f152fe
Apply suggestion from @fichtner
fichtner Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions net/upnp/pkg-descr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,22 @@ WWW: https://miniupnp.tuxfamily.org/
Plugin Changelog
================

1.9

* Separate service log file and log level UI option
* More specific allow third-party mapping UI option
* Impove help/wording and update missed changelog
* Add daemon patch to improve logging

1.8

* New UI options: disable IPv6 mapping, allow third-party mapping, UPnP IGD compatibility, router/friendly name; remove option: report system uptime (bug)
* List IPv6 maps and keep active maps when reconfiguring/restarting service, clearer added via / description field
* New UI sections, rewording plugin, set allow-filtered with STUN to workaround CGNAT test limitation, clean up daemon config
* Update daemon to 2.3.9, add build options (e.g. IGDv2 support), add daemon patch to improve UPnP IGDv2 compatibility

(1.8/1.9 contributed by Self-Hosting-Group)

1.7

* Add option to allow arbitrary number of UPnP/NAT-PMP rules (contributed by Kreeblah)
Expand Down
23 changes: 13 additions & 10 deletions net/upnp/src/etc/inc/plugins.inc.d/miniupnpd.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,12 @@ function miniupnpd_firewall($fw)
return;
}

/* required for IPv4: */
$fw->registerAnchor('miniupnpd', 'rdr');
/* required for IPv6: */
$fw->registerAnchor('miniupnpd', 'fw');
/* required for IPv4 NAT hairpinning: */
$fw->registerAnchor('miniupnpd', 'nat', 0, 'head');
$fw->registerAnchor('miniupnpd', 'binat');
}

function miniupnpd_services()
Expand Down Expand Up @@ -88,14 +90,11 @@ function miniupnpd_start()
$cmd_args = ['/var/etc/miniupnpd.conf', '/var/run/miniupnpd.pid'];

switch ($log_level) {
case 'debug':
$cmd_frmt[] = '-v';
/* FALLTHROUGH */
case 'info':
$cmd_frmt[] = '-v';
/* FALLTHROUGH */
default:
break;
case 'debug':
$cmd_frmt[] = '-vv';
Copy link
Copy Markdown
Member

@fichtner fichtner Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need evidence -v -v works differently than -vv

Copy link
Copy Markdown
Contributor Author

@Self-Hosting-Group Self-Hosting-Group Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit message explains why that is needed for the special behaving daemon: Fix debug logging not treating -v -v (follow-up to 7658677)

Copy link
Copy Markdown
Member

@fichtner fichtner Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The miniupnpd code suggests it doesn't care if it's -vv or -v -v which is why I raised the question after you said it's wrong. I just want a dependable answer.

https://github.com/miniupnp/miniupnp/blob/f33ae48402835b5b93029b1861a32630f9f17d1b/miniupnpd/miniupnpd.c#L1609-L1615

Copy link
Copy Markdown
Contributor Author

@Self-Hosting-Group Self-Hosting-Group Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/miniupnp/miniupnp/blob/f33ae48402835b5b93029b1861a32630f9f17d1b/miniupnpd/miniupnpd.c#L2233

Copy link
Copy Markdown
Member

@fichtner fichtner Mar 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both -v -v and -vv trigger verbosity_level++; twice if I read this correctly. I just want to know what mistake I made so I can avoid it in the future.

Copy link
Copy Markdown
Contributor Author

@Self-Hosting-Group Self-Hosting-Group Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a mistake. When I first tested it, it didn't seem to work, but I just tested it again and now it works. Sorry!

}

mwexecfb($cmd_frmt, $cmd_args);
Expand All @@ -105,8 +104,8 @@ function miniupnpd_stop()
{
killbypid('/var/run/miniupnpd.pid');

mwexecf('/sbin/pfctl -a miniupnpd -Fr');
mwexecf('/sbin/pfctl -a miniupnpd -Fn');
mwexecf('/sbin/pfctl -a miniupnpd -F rules');
mwexecf('/sbin/pfctl -a miniupnpd -F nat');
}

function miniupnpd_configure()
Expand Down Expand Up @@ -221,11 +220,15 @@ function miniupnpd_configure_do($verbose = false)
$config_text .= "bitrate_up={$upload}\n";
}

if (!empty($upnp_config['allow_third_party_mapping'])) {
if (in_array($upnp_config['allow_third_party_mapping'] ?? '', ['1', 'upnp-igd'])) {
$config_text .= "secure_mode=no\n";
$config_text .= "pcp_allow_thirdparty=yes\n";
} else {
$config_text .= "secure_mode=yes\n";
}

if (in_array($upnp_config['allow_third_party_mapping'] ?? '', ['1', 'pcp'])) {
Comment thread
fichtner marked this conversation as resolved.
$config_text .= "pcp_allow_thirdparty=yes\n";
} else {
$config_text .= "pcp_allow_thirdparty=no\n";
}

Expand Down
4 changes: 2 additions & 2 deletions net/upnp/src/opnsense/mvc/app/models/OPNsense/UPnP/ACL/ACL.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
<pattern>status_upnp.php*</pattern>
</patterns>
</page-status-upnpstatus>
<page-services-upnp-log>
<page-diagnostics-logs-upnp>
<name>Services: UPnP IGD &amp; PCP: Log File</name>
<patterns>
<pattern>ui/diagnostics/log/core/miniupnpd/*</pattern>
<pattern>api/diagnostics/log/core/miniupnpd/*</pattern>
</patterns>
</page-services-upnp-log>
</page-diagnostics-logs-upnp>
</acl>
15 changes: 10 additions & 5 deletions net/upnp/src/www/services_upnp.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,15 @@ function miniupnpd_validate_port($port)
// save form data
$upnp = [];
// boolean types
foreach (['enable', 'enable_upnp', 'enable_natpmp', 'logpackets', 'sysuptime', 'permdefault', 'allow_third_party_mapping', 'ipv6_disable'] as $fieldname) {
foreach (['enable', 'enable_upnp', 'enable_natpmp', 'logpackets', 'sysuptime', 'permdefault', 'ipv6_disable'] as $fieldname) {
$upnp[$fieldname] = !empty($pconfig[$fieldname]);
}
// numeric types
if (!empty($pconfig['num_permuser'])) {
$upnp['num_permuser'] = $pconfig['num_permuser'];
}
// text field types
foreach (['download', 'ext_iface', 'friendly_name', 'log_level', 'overridesubnet', 'overridewanip', 'stun_host', 'stun_port', 'upload', 'upnp_igd_compat'] as $fieldname) {
foreach (['allow_third_party_mapping', 'download', 'ext_iface', 'friendly_name', 'log_level', 'overridesubnet', 'overridewanip', 'stun_host', 'stun_port', 'upload', 'upnp_igd_compat'] as $fieldname) {
$upnp[$fieldname] = $pconfig[$fieldname];
}
foreach (miniupnpd_permuser_list() as $fieldname) {
Expand Down Expand Up @@ -234,7 +234,7 @@ function miniupnpd_validate_port($port)
</thead>
<tbody>
<tr>
<td><a id="help_for_enable" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Enable service");?></td>
<td><a id="help_for_enable" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Enabled");?></td>
<td>
<input name="enable" type="checkbox" value="yes" <?=!empty($pconfig['enable']) ? "checked=\"checked\"" : ""; ?> />
<div class="hidden" data-for="help_for_enable">
Expand Down Expand Up @@ -350,9 +350,14 @@ function miniupnpd_validate_port($port)
<tr>
<td><a id="help_for_allow_third_party_mapping" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Allow third-party mapping");?></td>
<td>
<input name="allow_third_party_mapping" type="checkbox" value="yes" <?=!empty($pconfig['allow_third_party_mapping']) ? "checked=\"checked\"" : ""; ?> />
<select name="allow_third_party_mapping">
<option value="0" <?= ($pconfig['allow_third_party_mapping'] ?? '') == '0' ? 'selected="selected"' : '' ?> ><?= gettext('Disabled (recommended)') ?></option>
<option value="1" <?= ($pconfig['allow_third_party_mapping'] ?? '') == '1' ? 'selected="selected"' : '' ?> ><?= gettext('Enabled') ?></option>
<option value="upnp-igd" <?= ($pconfig['allow_third_party_mapping'] ?? '') == 'upnp-igd' ? 'selected="selected"' : '' ?> ><?= gettext('Enabled (UPnP IGD only)') ?></option>
<option value="pcp" <?= ($pconfig['allow_third_party_mapping'] ?? '') == 'pcp' ? 'selected="selected"' : '' ?> ><?= gettext('Enabled (PCP only)') ?></option>
</select>
<div class="hidden" data-for="help_for_allow_third_party_mapping">
<?=gettext("Allow adding port maps for non-requesting IP addresses.");?>
<?=gettext("Allow adding port maps for non-requesting IP addresses; use with care.");?>
</div>
</td>
</tr>
Expand Down