confusion between local peer port and external peer port

[bodgit]

I just used Transmission to grab a torrent and I noticed that in Preferences the default port of 51413 was marked as closed. I knew Transmission had support for NAT-PMP and as I wrote the NAT-PMP implementation running on my firewall I'm fairly familiar with the protocol itself so I went poking about to see what might be wrong.

Transmission is successfully setting up a port forwarding rule, but the public port being assigned isn't the same as the private port (this is partly due to my NAT-PMP implementation). I'm assuming Transmission is correctly using the actual assigned public port (the torrent I was grabbing was not "popular" enough to have anyone connect to me) but the Preferences pane is always submitting the private port to the ​http://portcheck.transmissionbt.com/ service rather than use the public port.

The attached crude patch seems to fix things for me, however I don't think it's correct in the case of the Transmission client not being behind a NAT gateway but hopefully it illustrates the problem and someone more clued up with the source can apply a more correct fix.

[bodgit]

Crude patch to fix Preferences pane to use public port instead of private port

[livings124]

Thanks! This looks to apply to all the clients.

[charles]

Actually it looks like there might be some confusion on this issue in dht as well... or maybe I'm just confused on that instead :)

CC'ing jch to ask for a review port.diff.

[charles]

CC'ing jch to ask for a review port.diff.

[jch]

I'm strongly opposed to this patch.

As it currently stands, Transmission expects the NAT punching mechanism to perform an identity mapping between the external and the internal ports. It is up to the user to choose the internal port in such a manner that the NAT will perform an identity mapping.

The generalisation to arbitrary mappings that you propose leads to a lot of complexity. Keeping this ditinction correct will be difficult for our poor, tired brains, and we will doubtless get it wrong at some point.

This kind of effort is just not worth it -- it'll be a lot of effort for us, and it will never work right. Please fix your NAT instead.

--jch

[bodgit]

I can't speak for UPnP but the NAT-PMP spec clearly states that a client isn't guaranteed to get the port it asks for. I have other NAT-PMP clients on multiple machines that all want the same ports forwarded (any Mac OS X host with "Back to My Mac" enabled will try and get UDP ports 4500 and 5353 mapped on the firewall) and have no way of changing the port number so the NAT hole-punching mechanism has to be able to arbitrate between the clients, either no client gets the port they want, or one does and the other clients don't; it doesn't really matter provided the client is aware of this fact. The port randomising feature in Transmission goes some way to avoid collisions but requires all clients use it and doesn't 100% remove the problem that at some point you may have a collision.

The bug as it stands only seemed to affect the port open probe in preferences, the rest of Transmission seemed to be working fine. I left a torrent open and other peers were correctly connecting to the external port on my NAT gateway rather than failing to connect to the internal port so I can only assume most of the code is using the right port. When I close Transmission, it correctly sends a request to delete the mapping for the assigned external port too, so I think you have less to fix than you think.

It goes without saying I'm more than happy to test any changes you might want to make.

Cheers

Matt

[jch]

I can't speak for UPnP but the NAT-PMP spec clearly states that a client isn't guaranteed to get the port it asks for.

Of course -- that's the very definition of NAPT (and all NATs nowadays are NAPTs).

However, any half-sensible implementation of NAPT will use an identity mapping whenever possible. Hence, you should be able to select a port to get an identity mapping. I believe that Transmission should simply warn the user when it wasn't able to obtain an identity mapping.

I think you have less to fix than you think.

It's not about how much there is to fix -- it's about your introducing yet another NAT-related concept, namely the distinction between inner and outer ports, which is yet another possible source of confusion.

However, after thinking it over, I retract my veto -- Charles, it's up to you to decide if you want this patch.

Juliusz

[charles]

I try to always weigh the patch's complexity against its usefulness. And, I agree with jch that this concept has nettling little places where we can get it wrong. For example I'm still not sure my DHT version of this patch is correct.

Weighed against that, this clearly is a case where we could do better, but that doesn't seem to affect very many users.

I guess, on balance, I'd say that this patch is probably not worth it to me. However, bodgit, if this is causing you a proverbial Programmer Itch and you want to test out and debug the DHT/LPD parts of this patch, that would balance the scales a bit. If you decide to do that, please feel encouraged to reopen this ticket.

[bodgit]

Ok, I'll see if I can take a look at this.

Thanks for the comments

Matt

[bodgit]

Right, I have had a bit of time to poke about, firstly the LPD part of Charles' patch shouldn't be necessary as the TTL on the multicast packet is set to 1 so it should never traverse a router and therefore never be subject to NAT.

For the DHT side of things, I found another problem in that Transmission never requests a UDP port mapping via UPnP or NAT-PMP so incoming DHT requests over IPv4 don't currently work anyway. At least for NAT-PMP the spec says that a client requesting both a TCP and UDP mapping to the same internal port must get the same external port so the port logic in the code should still be ok. I had a look at additionally requesting a UDP port mapping and apart from it being conditional on the DHT preference being enabled, it didn't look straightforward without refactoring the code a bit as it seems to assume a single port mapping.

Matt