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

Already on GitHub? Sign in to your account

add warning about UPnP vulnerability #1086

Merged
merged 6 commits into from Oct 12, 2015

Conversation

Projects
None yet
3 participants
Contributor

laanwj commented Oct 12, 2015

Add warning about UPnP vulnerability.

Context:

TODO:

  • actually release 0.11.1 (or at the least rc binaries)
  • move the screenshot image to a sane place
  • maybe add instructinos about manually forwarding ports (not necessarily here)

laanwj and others added some commits Oct 12, 2015

Contributor

saivann commented Oct 12, 2015

@laanwj Just pushed a commit to fix the path of the image (feel free to do whatever with that commit)

Edit: LGTM up to 5b4ca42

Contributor

harding commented Oct 12, 2015

Reviewing now. @laanwj how fast do you want to get this out?

saivann and others added some commits Oct 12, 2015

Contributor

harding commented Oct 12, 2015

Commit 3fb01e8 links to our doc about setting up manual port forwarding. Here's the preview:

image

Contributor

harding commented Oct 12, 2015

Should we mention that 0.11.1/0.10.3 haven't been released yet but that we'll add the links to them as soon as they're available?

Contributor

saivann commented Oct 12, 2015

@harding Thanks! I see that you've enabled the alert banner site-wide. Is the vulnerability serious enough for it? Or perhaps would it be more appropriate to use the orange banner : bannerclass: warning ?

Contributor

harding commented Oct 12, 2015

@saivann I don't know how serious it was. I've been too focused on other things lately to realize that this was happening.

@laanwj and other devs: the open questions are:

  • should we place a red banner at the top of each page notifying every visitor about this, should we use a less-aggressive orange banner, or should we use no banner at all? (People with RSS subscriptions to the Alerts page will still be notified no matter what the next time they poll the site after this is published)
  • should we temporarily disable the links on the Download page that point to 0.11.0?
  • Should we release this alert now, or should we wait for 0.11.1 to be released?
Contributor

laanwj commented Oct 12, 2015

should we place a red banner at the top of each page notifying every visitor about this, should we use a less-aggressive orange banner, or should we use no banner at all.

I'd put up a red banner, when it goes up.

should we temporarily disable the links on the Download page that point to 0.11.0?

Not sure. It's ok to use 0.11.0 as long as people follow the steps in the alert.

0.11.1rc2 is waiting for code signing. After that is done I'll upload them.
Then people can test that...

Should we release this alert now, or should we wait for 0.11.1 to be released?

I'd like to wait at least until rc binaries have been uploaded.

Contributor

harding commented Oct 12, 2015

@laanwj thanks for your reply; the red banner is included now (see the preview image above), and it'll be displayed over the download page, so that should provide an effective warning to new downloaders.

Do you want the alert to link to the 0.11.1RC2 directory? AFAIK, we've never recommended anyone but testers use an RC and I feel a bit ambivalent doing about that.

Contributor

laanwj commented Oct 12, 2015

Do you want the alert to link to the 0.11.1RC2 directory? AFAIK, we've never recommended anyone but testers use an RC and I feel a bit ambivalent doing about that.

I feel the same way, but I don't really know anymore now.

We can update the text that 0.11.1 and 0.10.3 aren't released yet and people should upgrade to them when released, but this will make the call to action much weaker.

Contributor

harding commented Oct 12, 2015

@laanwj I guess it's ok if we tell them about the small additional risk. What do you think about this:

diff --git a/_alerts/2015-10-12-upnp-vulnerability.md b/_alerts/2015-10-12-upnp-vulnerability.md
index 5475c4a..26f8d72 100644
--- a/_alerts/2015-10-12-upnp-vulnerability.md
+++ b/_alerts/2015-10-12-upnp-vulnerability.md
@@ -19,7 +19,10 @@ Either
 - add the line `upnp=0` to your `bitcoin.conf` file
 - add `-upnp=0` to the command line options

-Alternatively, upgrade to a version of Bitcoin Core at least 0.10.3 or 0.11.1.
+Alternatively, upgrade to a version of Bitcoin Core at least 0.10.3 or 0.11.1
+when they become available---we'll update this document with links when
+that happens.  (If you want to take a small amount of risk, you can
+download the pre-release versions: [0.11.1RC](FIXME).)
 These versions upgrade the library to a non-vulnerable version, as well as
 disable UPnP by default to prevent this problem in the future.
Contributor

harding commented Oct 12, 2015

I'll be merging momentarily per an IRC conversation with @laanwj.

@harding harding merged commit 38c1079 into master Oct 12, 2015

0 of 2 checks passed

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

harding added a commit that referenced this pull request Oct 12, 2015

@harding harding deleted the upnp-vulnerability branch Dec 23, 2015

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