Commit 3a9e668c authored by Julien Muchembled's avatar Julien Muchembled

UPnP: randomize external port

Some routers are so broken that UPnP NAT don't report ConflictInMappingEntry
when redirecting the same port several times.

Here is for example what we had with a Numericable Box (France):

0 (1024, 'TCP', ('192.168.0.29', 1194), 're6stnet openvpn server (1194/tcp)', '1', '', 0)
1 (1024, 'TCP', ('192.168.0.16', 1194), 're6stnet openvpn server (1194/tcp)', '1', '', 0)
2 (1024, 'TCP', ('192.168.0.33', 1194), 're6stnet openvpn server (1194/tcp)', '1', '', 0)
3 (1024, 'TCP', ('192.168.0.20', 1194), 're6stnet openvpn server (1194/tcp)', '1', '', 0)
('192.168.0.29', 1194, 're6stnet openvpn server (1194/tcp)', True, 0)

Obviously, this can't work.

It seems that this router also accepts a limited number of NAT rules, far less
than we'd like, so even if there's still a probability of conflict with this
commit, it will be good enough for our use.
parent e3c42494
from functools import wraps from functools import wraps
import logging, socket, time import logging, random, socket, time
import miniupnpc import miniupnpc
...@@ -8,10 +8,18 @@ class UPnPException(Exception): ...@@ -8,10 +8,18 @@ class UPnPException(Exception):
class Forwarder(object): class Forwarder(object):
"""
External port is chosen randomly between 32768 & 49151 included.
"""
next_refresh = 0 next_refresh = 0
_next_retry = -1 _next_retry = -1
_next_port = 1024 _next_port = random.randrange(0, 8192)
@classmethod
def _getExternalPort(cls):
port = cls._next_port = (cls._next_port + 1) % 8192
return 32768 + port
def __init__(self, description): def __init__(self, description):
self._description = description self._description = description
...@@ -73,6 +81,7 @@ class Forwarder(object): ...@@ -73,6 +81,7 @@ class Forwarder(object):
logging.debug('Refreshing port forwarding') logging.debug('Refreshing port forwarding')
ip = self.externalipaddress() ip = self.externalipaddress()
lanaddr = self._u.lanaddr lanaddr = self._u.lanaddr
retry = 8191
for r in self._rules: for r in self._rules:
local, proto, port = r local, proto, port = r
if port and not force: if port and not force:
...@@ -81,11 +90,11 @@ class Forwarder(object): ...@@ -81,11 +90,11 @@ class Forwarder(object):
args = proto.upper(), lanaddr, local, desc, '' args = proto.upper(), lanaddr, local, desc, ''
while True: while True:
if port is None: if port is None:
port = self._next_port if not retry:
if port > 65535:
raise UPnPException('No free port to redirect %s' raise UPnPException('No free port to redirect %s'
% desc) % desc)
self._next_port = port + 1 retry -= 1
port = self._getExternalPort()
try: try:
self.addportmapping(port, *args) self.addportmapping(port, *args)
break break
...@@ -99,10 +108,6 @@ class Forwarder(object): ...@@ -99,10 +108,6 @@ class Forwarder(object):
return ip return ip
def clear(self): def clear(self):
try:
del self._next_port
except AttributeError:
return
for r in self._rules: for r in self._rules:
port = r[2] port = r[2]
if port: if port:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment