Bug 251822 - net/miniupnpd: firewall type detection broken; upstream pull needed
Summary: net/miniupnpd: firewall type detection broken; upstream pull needed
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks: 232396
  Show dependency treegraph
 
Reported: 2020-12-13 20:51 UTC by Jeremy Cooper
Modified: 2021-07-23 20:50 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (squat)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Cooper 2020-12-13 20:51:11 UTC
SUMMARY

The current in-tree version of miniupnpd does not correctly detect the host firewall type and defaults to building a version which is only compatible with pf no matter the actual host firewall type.


DETAILS

miniupnpd supposedly can be built to use any one of the three supported FreeBSD firewalls: pf, ipfilter and ipfw. The build process attempts to discern which firewall to use by consulting /etc/rc.conf, but it does a poor job of doing so because of a series of precondition checks it performs before attempting to load /etc/rc.conf. These preconditions fail and /etc/rc.conf is never loaded. The build process then falls back to building the pf version.


FIX

In March 2020 I notified the miniupnpd team about this issue, opening a pull request and proposing a fix. The minupnp team accepted the change and incorporated it into their master branch rather quickly and released with miniupnp 2.2.0.

According to the the HEAD of ports SVN the port distinfo file is still pinned at version 2.1.20190210. It was last edited as of ports r496821, which means that it doesn't have the fix for this detection issue.


UPSTREAM ISSUE

The upstream pull request which fixes the firewall detection problem can be found at

https://github.com/miniupnp/miniupnp/issues/431


UPSTREAM VERSION CONTAINING DETECTION FIX

The full fix for this issue was applied in

https://github.com/miniupnp/miniupnp/commit/040fbc40f86a88c14dd4d3f8409e878994839e76

Which then appears to have been incorporated into miniupnp version 2.2.0.
Comment 1 Dmitry Marakasov freebsd_committer 2021-02-19 23:46:28 UTC
This doesn't look like a correct way to detect firewall - this cannot be performed at build time at all, since packages are usually built in different environment than where they are installed. I suggest to submit a patch to make this port flavored, and set firewall type explicitly for the build depending on flavor.
Comment 2 Jeremy Cooper 2021-02-19 23:47:48 UTC
(In reply to Dmitry Marakasov from comment #1)
Oh I agree that build-time firewall detection is the wrong way to go. But that was an even bigger battle I didn't want to join with the upstream developer.
Comment 3 Jeremy Cooper 2021-02-19 23:53:15 UTC
(In reply to Dmitry Marakasov from comment #1)
Until such time as said patch is available, it would still be better to perform an upstream pull.

As much as it is a sin to do build-time firewall detection, that is the method this port has used for a long time. In its current state the build process is making the _worst_ of all decisions.
Comment 4 Dan Mahoney 2021-07-23 20:50:01 UTC
Bumping the port to 2.2.2 is fairly trivial -- the makefile patch needs tweaking so it applies cleanly.

Flavoring the port is also fairly trivial.

That said, I'm looking at the ipfw support.  It's based entirely on the OSX ipfw (which I think used a variant of IPFW1) and would need to be pretty much rewritten to map to the calls in /usr/include/netinet/ip_fw.  Without that, flavoring is pointless and the port should just be hard-coded to only support pf.

I'm going to post a slightly more detailed analysis of what needs to happen over on that ohther bug report.