UPnP-IGD support for NAT traversal

[ichorid]

UPnP provides a way for an application/device to request a port forwarding through the network's NAT from a UPnP-enabled router. This is another technique to enable better NAT traversal. UPnP-IGD uses broadcast HTTP requests over UDP (and is already enabled in Libtorrent). We can copy the logic for announcing the port from there.

[qstokkink]

In order to make sure the source code doesn't turn into full-on spaghetti, this should be postponed until after #615.

[qstokkink]

These look promising:

• https://pypi.org/project/UPnPy/
• [deprecated] https://pypi.org/project/netdisco/
• https://pypi.org/project/uPnPClient/

[qstokkink]

UPnPy only required one patch, here's my test for service discovery:

import logging
import socket

import upnpy
from upnpy.ssdp.SSDPRequest import SSDPDevice, SSDPRequest

class PatchedRequest(SSDPRequest):

    def _send_request(self, message):
        self.socket.sendto(message.encode(), (self.SSDP_MCAST_ADDR, self.SSDP_PORT))
        devices = []
        try:
            while True:
                response, addr = self.socket.recvfrom(65507)
                try:
                    device = SSDPDevice(addr, response.decode())
                    devices.append(device)
                except:
                    logging.warning("Failed to connect to UPnP device at %s", addr)
        except socket.timeout:
            pass

        return devices

upnp = upnpy.UPnP()
upnp.ssdp = PatchedRequest()

devices = upnp.discover()

print(devices)

device = upnp.get_igd()

print(device.get_services())

for service in device.get_services():
    print("=" * 2, service, "=" * 8)
    for action in service.get_actions():
        print(action, "INPUTS:", action.get_input_arguments(), "OUTPUTS:", action.get_output_arguments())

[qstokkink]

Here is the same script with uPnPclient, which holds my preference:

import upnpclient

devices = upnpclient.discover()

print(devices)

for device in devices:
    print(device.friendly_name, device.services)

    for service in device.services:
        print("=" * 2, service, "=" * 8)
        for action in service.actions:
            print(action, "INPUTS:", action.argsdef_in, "OUTPUTS:", action.argsdef_out)

[ichorid]

What's the upsides of upnpclient vs upnpy? Brevity? Which one is better supported?

[qstokkink]

upnpclient works out-of-the-box and has more stars on GitHub, but was last updated 10 months ago. upnpy has 1/10th of the stars, but was last updated a month ago.

Judging from the repository activity, neither has active support.

[ichorid]

Is is possible to create a simple test for the stuff? Like, starting a upnp server and verify that the thing really works as intended?

[qstokkink]

Isolated and local: no.

However, pretty much any commercial router supports UPnP. So, usually it will probably work. Downside being that no single router supports it the same way and there are all sorts of vendor-specific differences. Actually, most routers don't even fully support UPnP because it is full of critical security flaws. After reading about and playing with UPnP, I wonder if we really want it. Also, it's a pain to work with.

[ichorid]

As long as we do not require UPnP to be enabled on the router, it is fine to exploit the opportunity for better NAT traversal. Of course, we should be able to fallback to non-UPnP solution in case the router's UPnP implementation is faulty.

Does it worth it? We'll see when we enable it.

[qstokkink]

It is not the opportunity I oppose, it is the fact that we may be opening up unsuspecting users to some terrible security leaks.

Ultimately, I think we can do more with a simple UDP broadcast over the LAN.

[ichorid]

By using UPnP correctly we're not opening anyone to a security leak.

[qstokkink]

Let me quote the 5th link I posted (feel free to read them all in-depth though):

You might assume that you’re secure as long as no malware is running on any local devices – but you’re probably wrong.

UPnP is utterly flawed and a huge security risk to the point of pretty much every security expert in the field advising to disable it on your local router.

I don't want to give users any reason to enable it again.

[ichorid]

I don't want to give users any reason to enable it again.

That's so responsible of you! Maybe we should ask Arvid to remove UPnP support from Libtorrent for the same reason, BTW? 😉

[qstokkink]

Alright, it seems we have enough information. I'll close this issue and round up this information here.

Reasons to not support UPnP:

• There is overwhelming evidence that UPnP hardware is half-implemented and full of security holes.
• UPnP APIs are inconsistent and therefore fragile.
• UPnP support is non-trivial and we will have to maintain this code.
• UPnP support requires an additional dependency (or a few hundred lines of code).
• UPnP does not offer anything we cannot get from UDP broadcasts.
• We are responsible.

If any of these reasons can be shown to be factually incorrect, we can reopen this issue and have a discussion.

Should we ever want to enable UPnP anyway:

• https://pypi.org/project/uPnPClient/ seems like a viable option.