Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set all builds to have UPNP disabled by default #461

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Set all builds to have UPNP disabled by default #461

wants to merge 1 commit into from

Conversation

IngCr3at1on
Copy link
Contributor

this at least builds in gitian (1 hour) and partly addresses #264

@jpass022
Copy link

Why would you want upnp disabled by default? I think this will just cause problems for normal users...

@IngCr3at1on
Copy link
Contributor Author

#264

You'll find that even though they've updated the UPNP version for bitcoin they have disabled it be default on bitcoin as well to avoid possible issues like the above mentioned in the future.

The vulnerability really only effects nodes on a public LAN and as I understand it it does not allow access to the wallet it's more a stability issue but having a potentially vulnerable or vulnerable function enabled by default isn't always the best option (even if it does make things easier for some people).

@dbroyhill
Copy link

Does disabling UPNP require users to perform additional steps, such as adding exceptions to their firewalls and opening ports on their routers?

@IngCr3at1on
Copy link
Contributor Author

Possibly in some cases, otherwise they may not need to as that will only effect incomming connections. A wallet can function with nothing but outgoing connections (I've done that with a few daemons at different points).

If a user finds they have issues with the connections with UPNP disabled they can either open ports or enable UPNP in their conf or when starting the wallet. This doesn't remove UPNP it just changes it from being Opt-Out (which is potentially unsafe) to being Opt-In (requiring at least some minor education and responsibility on the users part).

@CeForce
Copy link

CeForce commented Feb 18, 2016

We've had scenarios in support where we have had to help or direct users to open their firewall ports even when UPNP enabled. If it's still going to only impact the occasional or a small % of users it makes sense to set the default to UPNP disabled.

I'd rather see something a little more inherently secure be the default and it require a user action to change that status.

@IngCr3at1on
Copy link
Contributor Author

This has been rebased over master do to merge conflicts (5 minutes)

@MitchellCash
Copy link
Contributor

utACK to making it disabled by default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants