/bitcoin

net: Disable upnp by default #6795

Merged
merged 1 commit into from Oct 10, 2015
+3 −3

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
P2P
Projects
None yet
Milestone
No milestone
4 participants
@laanwj @btcdrak @luke-jr @TheBlueMatt
@laanwj
Owner

laanwj commented Oct 9, 2015

Common sentiment is that the miniupnpc codebase likely contains further vulnerabilities (context: #6789).

I'd prefer to get rid of the dependency completely, but a compromise for now is to at least disable it by default, to prevent UPnP vulnerabilities being a structural danger to the network.

Also get rid of the confusing --[enable|disable]-upnp-defaultautoconf and define magic.

Edit: needs backport to 0.11 and 0.10

laanwj added the P2P label Oct 9, 2015

laanwj referenced this pull request Oct 9, 2015

Closed

Default UPNP to off. #6794

@laanwj
Owner

laanwj commented Oct 9, 2015

Right, fixed
@btcdrak
Member

btcdrak commented Oct 9, 2015

utACK

laanwj referenced this pull request Oct 9, 2015

Closed

Defaults UPNP to off when discovery is disabled. #6792

@luke-jr
Member

luke-jr commented Oct 9, 2015

NACK removal of configure option; please just change the options gitian uses so people don't need to hand-patch :(
@TheBlueMatt
Contributor

TheBlueMatt commented Oct 9, 2015

Concept ACK. I'm fine with removing the configure option, not sure we really need it. Its a very different world from when UPnP defaults were set, and I really dont think there is much need for either the GUI or bitcoind to default to UPnP on anymore.
@laanwj laanwj net: Disable upnp by default 
Common sentiment is that the miniupnpc codebase likely contains further
vulnerabilities.

I'd prefer to get rid of the dependency completely, but a compromise for
now is to at least disable it by default.
21d27eb

@laanwj laanwj merged commit 21d27eb into bitcoin:master Oct 10, 2015
1 check was pending

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@laanwj laanwj added a commit that referenced this pull request Oct 10, 2015

@laanwj laanwj Merge pull request #6795 
21d27eb net: Disable upnp by default (Wladimir J. van der Laan)
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 74810B012346C9A6 Learn about signing commits
b94ae81

@laanwj laanwj added a commit that referenced this pull request Oct 10, 2015

@laanwj laanwj net: Disable upnp by default 
Common sentiment is that the miniupnpc codebase likely contains further
vulnerabilities.

I'd prefer to get rid of the dependency completely, but a compromise for
now is to at least disable it by default.

Rebased-From: 21d27eb
Github-Pull: #6795
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 74810B012346C9A6 Learn about signing commits
f2778e0

@laanwj laanwj added a commit that referenced this pull request Oct 10, 2015

@laanwj laanwj net: Disable upnp by default 
Common sentiment is that the miniupnpc codebase likely contains further
vulnerabilities.

I'd prefer to get rid of the dependency completely, but a compromise for
now is to at least disable it by default.

Github-Pull: #6795
Rebased-From: 21d27eb
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 74810B012346C9A6 Learn about signing commits
4dbcec0

laanwj referenced this pull request in bitcoin-dot-org/bitcoin.org Oct 12, 2015

Merged

add warning about UPnP vulnerability #1086

2 of 3 tasks complete

@laanwj laanwj added a commit that referenced this pull request Oct 15, 2015

@laanwj laanwj Disable upnp by default 
Github-Pull: #6795
Rebased-From: 21d27eb
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 74810B012346C9A6 Learn about signing commits
460ccfb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment