ethereum / go-ethereum

Go implementation of the Ethereum protocol
https://geth.ethereum.org
GNU Lesser General Public License v3.0
47.26k stars 20.01k forks source link

UPNP port mapping fails on routers that only support permanent lease #16732

Open abdes opened 6 years ago

abdes commented 6 years ago

Hi there,

please note that this is an issue tracker reserved for bug reports and feature requests.

For general questions please use the gitter channel or the Ethereum stack exchange at https://ethereum.stackexchange.com.

System information

Geth version: geth version Geth Version: 1.8.8-unstable Git Commit: 784aa83942e3dbc9bab0385475dbf3755a9892ac Architecture: amd64 Protocol Versions: [63 62] Network Id: 1 Go Version: go1.10.2 Operating System: linux GOPATH= GOROOT=/usr/lib/go

OS & Version: Windows/Linux/OSX Linux

Commit hash : (if develop) Git Commit: 784aa83942e3dbc9bab0385475dbf3755a9892ac

Expected behaviour

At startup geth attempts to add a port mapping using UPNP. Some routers only support permanent leases (i.e. lifetime = 0). In such scenario, it is expected that geth understands the returned HTTP response and the error in the SOAP message and re-attempts the port mapping with a permanent lease.

Actual behaviour

If the router only supports permanent leases (i.e. lifetime = 0), geth fails to handle the HTTP error response and just abandons the port mapping.

Steps to reproduce the behaviour

Start get behind a NAT using NetGear router R8000 or R6300 or any other router that only supports permanent lease for UPNP port mapping.

Changing p2p/nat.go mapTimeout to 0 results in the UPNP request succeeding.

Backtrace

POST /Public_UPNP_C3 HTTP/1.1
Host: 192.168.1.1:5000
User-Agent: Go-http-client/1.1
Content-Length: 619
CONTENT-TYPE: text/xml; charset="utf-8"
SOAPACTION: "urn:schemas-upnp-org:service:WANIPConnection:1#AddPortMapping"
Accept-Encoding: gzip

<?xml version="1.0" encoding="UTF-8"?>
<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/" s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/"><s:Body><u:AddPortMapping xmlns:u="urn:schemas-upnp-org:service:WANIPConnection:1"><NewRemoteHost></NewRemoteHost><NewExternalPort>30303</NewExternalPort><NewProtocol>UDP</NewProtocol><NewInternalPort>30303</NewInternalPort><NewInternalClient>192.168.1.49</NewInternalClient><NewEnabled>1</NewEnabled><NewPortMappingDescription>ethereum discovery</NewPortMappingDescription><NewLeaseDuration>1200</NewLeaseDuration></u:AddPortMapping></s:Body></s:Envelope>

HTTP/1.1 500 Internal Server Error
EXT:
CONTENT-TYPE:text/xml
SERVER:Linux/2.6.12 UPnP/1.0 NETGEAR-UPNP/1.0
CONTENT-LENGTH:438

<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/" s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/">
<s:Body>
<s:Fault>
<faultcode>s:Client</faultcode>
<faultstring>UPnPError</faultstring>
<detail>
<UPnPError xmlns="urn:schemas-upnp-org:control-1-0">
<errorCode>725</errorCode>
<errorDescription>OnlyPermanentLeasesSupported</errorDescription></UPnPError>
</detail>
</s:Fault>
</s:Body>
</s:Envelope>
stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

abdes commented 5 years ago

This is still a bug

anjmao commented 4 years ago

In Mysterium we solved this issue by retrying with 0 lifetime https://github.com/mysteriumnetwork/node/blob/master/nat/mapping/port_mapping.go#L113

fjl commented 4 years ago

Probably a good idea for us too.

fjl commented 4 years ago

There is also #2380 which could be tackled at the same time.