slskd / slskd

A modern client-server application for the Soulseek file sharing network.
GNU Affero General Public License v3.0
840 stars 51 forks source link

UPNP/NAT-PMP support #908

Open Sakooooo opened 1 year ago

Sakooooo commented 1 year ago

Nicotine+ and SoulSeekQT have UPNP and NAT-PMP. could it be possible to have that as an option?

jpdillingham commented 1 year ago

It's not high on my list of priorities, but I've added the help wanted tag in case someone else wants to pick it up.

This was the first .NET library to come up when I searched, it looks like it might be fairly easy to integrate. I'd definitely want it to be optional and disabled by default.

Sakooooo commented 1 year ago

alright ill try adding it in sometime maybe if i can

pbogre commented 1 year ago

i would also appreciate this feature since other services like transmission have built-in UPNP support and it makes the port-forwarding side of things a lot easier

jpdillingham commented 1 year ago

I should have clarified at the time, but I'm hesitant to add this because UPnP (and NAT-PMP from what I gather) is discouraged due to security concerns (here's one article: https://www.upguard.com/blog/what-is-upnp)

I'm not strictly opposed to adding the feature but it should definitely be opt-in and the documentation should warn users somehow.

Sakooooo commented 1 year ago

wait upnp is a security flaw!? dammit

mathiascode commented 1 year ago

I should have clarified at the time, but I'm hesitant to add this because UPnP (and NAT-PMP from what I gather) is discouraged due to security concerns (here's one article: https://www.upguard.com/blog/what-is-upnp)

I'm not strictly opposed to adding the feature but it should definitely be opt-in and the documentation should warn users somehow.

NAT-PMP is a simpler protocol strictly focused on forwarding ports, as opposed to UPnP, which has access to more settings, so it should be safer in that aspect. In any case, I've enabled both by default in Nicotine+ in hopes of getting as many open ports on the Soulseek network as possible. If someone has disabled UPnP/NAT-PMP, the operation will silently fail anyway (with a log message).

It might of course be a good idea to encourage users to disable UPnP and manually forward ports instead. Perhaps slskd users would be more inclined to do so anyway.

Sakooooo commented 1 year ago

It might of course be a good idea to encourage users to disable UPnP and manually forward ports instead. Perhaps slskd users would be more inclined to do so anyway.

just a reminder if anyone cant port forward cause they are behind NAT check for another admin dashboard if you are using a wire from router to router for internet, that worked for me

jpdillingham commented 1 year ago

The more I think about it I think @mathiascode is right, the application should be opportunistic and attempt to forward ports automatically; if users have them enabled then that's the behavior they'd want, and if users have it disabled there's no harm.

I think the important part is that the docs don't encourage people to turn these things on without explaining the risks.

I don't think this will work inside of a Docker container, but I'll do some research to see. I can easily detect when the application is running inside of Docker and skip configuration if so, or maybe log a message and tell users what do to do make it work (e.g. supply the host IP, etc)

pbogre commented 1 year ago

i have transmission inside a docker container, and that project using upnp, however i still had to do manual port forwarding through my router dashboard to allow incoming connections.

if there is any way to make upnp function correctly within a docker container i'd be happy to learn about it, both for slskd and other services that i run

Sakooooo commented 1 year ago

i have transmission inside a docker container, and that project using upnp, however i still had to do manual port forwarding through my router dashboard to allow incoming connections.

if there is any way to make upnp function correctly within a docker container i'd be happy to learn about it, both for slskd and other services that i run

i dont think it might be possible had qbittorrent and that didn't show up on the upnp list