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 BitGo chrome app to desktop section of choose your wallet #827

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

bpdavenport commented Apr 17, 2015

No description provided.

Contributor

bpdavenport commented Apr 17, 2015

Chrome app runs identical code to bitgo.com -- just packaged.

Contributor

saivann commented Apr 18, 2015

@bpdavenport I see that the source code is missing (see GreenAddress entry on the same page) and cannot be found on your GitHub account, would it be possible to publish that code and keep it up-to-date under version control system and clear release tags or commits?

A small issue with the pull request: The screenshot needs to be cropped to the same dimension as other screenshots. All screenshots on bitcoin.org are quite small.

Contributor

luke-jr commented Apr 18, 2015

I think Chrome Apps should be under the Web section...

Contributor

harding commented Apr 19, 2015

@luke-jr given the current labeling of "Web", I think that's a reasonable position. However, our policy has made "Web" into "web wallets and quarantine area for bitcoin banks". Allowing non-banks like BitGo to break out of the quarantine by getting listed in the other sections (when appropriate) seems like a good policy to me.

Contributor

bpdavenport commented Apr 20, 2015

@saivann We are working on open-sourcing the client - we need to split out the client code from other server-side code. We do have non-minified JS at https://www.bitgo.com/bitgo.js but understand that people would like to see structure, commit history, etc. Do you consider having the source published to be a blocker for getting listed here? Thanks

Contributor

saivann commented Apr 20, 2015

@bpdavenport It appears like a blocking issue to me indeed as all other desktop apps are open-source and that is one of the reasons why apps usually require less trust. It would be great to have the complete BitGo app source code becoming public and versioned :) .

@harding harding added the Wallets label Apr 20, 2015

Contributor

bpdavenport commented Apr 21, 2015

@saivann Source is up at https://github.com/BitGo/bitgo-chrome, and updated the pull request with the link.

Contributor

saivann commented Apr 23, 2015

@bpdavenport That's great, thanks! Can you fix the size of the screenshot?

Contributor

bpdavenport commented Apr 23, 2015

@saivann Should be good to go

@harding harding self-assigned this Apr 23, 2015

Contributor

saivann commented Apr 23, 2015

Thanks! Untested LGTM.

Contributor

crwatkins commented Apr 24, 2015

I have reviewed the BitGo wallet based on the current wallet requirements criteria and my evaluation is below. The summary is that I can recommend this wallet for listing. BitGo has informed me that they plan to switch to using deterministic nonces for signatures in the near future and I urge them to do that.

BitGo

Version 1.1.0

Source Commit 65fda5f781

Review Version 2015042301

The wallet list is based on the personal evaluation of the maintainer(s) and regular contributors of this site, according to the criterias detailed below.

These requirements are meant to be updated and strengthened over time. Innovative wallets are exciting and encouraged, so if your wallet has a good reason for not following some of the rules below, please submit it anyway and we'll consider updating the rules.

NOTE This is a review of the Chrome App version of BitGo, but unless otherwise mentioned the notes apply to the web version also.

Basic requirements:

  • Sufficient users and/or developers feedback can be found without concerning issues, or independent security audit(s) is available

PASS Interaction on Bitcointalk, Reddit, and bounties on https://www.crowdcurity.com/bitgo

  • No indication that users have been harmed considerably by any issue in relation to the wallet

PASS No indications found using standard Google fu

  • No indication that security issues have been concealed, ignored, or not addressed correctly in order to prevent new or similar issues from happening in the future

PASS No indication.

  • No indication that the wallet uses unstable or unsecure libraries

PASS Uses BitcoinJS

  • No indication that changes to the code are not properly tested

PASS No indication (one way or the other)

  • Wallet was publicly announced and released since at least 3 months

PASS Wallet has been available for over a year. This is the packaging of the same code in a Chrome App.

  • No concerning bug is found when testing the wallet

PASS No concerning bugs were found

  • Website supports HTTPS and 301 redirects HTTP requests

PASS on bitgo.com and www.bitgo.com

PASS A+ Grade

  • Website serving executable code or requiring authentication uses HSTS with a max-age of at least 180 days

PASS max-age is 365 days

  • The identity of CEOs and/or developers is public

PASS http://bitgoinc.com/#About

  • If private keys or encryption keys are stored online:
    • Refuses weak passwords (short passwords and/or common passwords) used to secure access to any funds, or provides an aggressive account lock-out feature in response to failed login attempts along with a strict account recovery process.

PASS Refuses weak passwords. Uses a very aggressive password meter (zxcvbn).

  • If user has no access over its private keys:
    • Provides 2FA authentication feature

PASS Uses Authy

  • Reminds the user to enable 2FA by email or in the main UI of the wallet

PASS Forces 2FA during signup

  • User session is not persistent, or requires authentication for spending

PASS Requires password authentication plus 2FA for spending

  • Provides account recovery feature

PASS Provides recovery utility in https://github.com/BitGo/BitGoJS
Instructions at https://www.youtube.com/watch?v=lfhERxtzu2M
Uses keys on PDF created at wallet creation

  • If user has exclusive access over its private keys:
    • Allows backup of the wallet

PASS Backup of keys is done to a PDF at wallet creation time

  • Restoring wallet from backup is working

PASS Restoring funds from wallet using utility at https://github.com/BitGo/BitGoJS worked.

PASS Using custom code using a BIP32 path of m/0/0/c/i and key ordering of user key, backup key, bitgo (server) key, sweeping funds from the wallet using the keys on the PDF was possible.

  • Source code is public and kept up to date under version control system

PASS https://github.com/BitGo/bitgo-chrome

  • If user has no access to some of the private keys in a multi-signature wallet:
    • Provides 2FA authentication feature

PASS Uses Authy

  • Reminds the user to enable 2FA by email or in the main UI of the wallet

PASS 2FA is required

  • User session is not persistent, or requires authentication for spending

PASS Requires password authentication and 2FA for cosigning

  • Gives control to the user over moving their funds out of the multi-signature wallet

PASS User can spend funds with backup key using recovery tool without involvement of server

  • For hardware wallets:

N/A

  • Uses the push model (computer malware cannot sign a transaction without user input)
  • Protects the seed against unsigned firmware upgrades
  • Supports importing custom seeds
  • Provides source code and/or detailed specification for blackbox testing if using a closed-source Secure Element

Optional criterias (some could become requirements):

  • Received independent security audit(s)

PASS Full external security audit publicly claimed https://bitcointalk.org/index.php?topic=396790.0

  • Avoid address reuse by using a new change address for each transaction

PASS Uses a new change address each time. Used custom code to scan the blockchain and found each change address never used more than once

  • Avoid address reuse by displaying a new receiving address for each transaction in the wallet UI

PASS The wallet generates new receiving addresses as others are used

  • Does not show "received from" Bitcoin addresses in the UI

PASS Does not show received from addresses

  • Uses deterministic ECDSA nonces (RFC 6979)

FAIL Uses random nonces. BitGo plans to switch to deterministic nonces soon.

  • Provides a bug reporting policy on the website

PASS http://bitgoinc.com/help/

  • If user has no access over its private keys:

N/A

  • Full reserve audit(s)
  • Insurrance(s) against failures on their side
  • Reminds the user to enable 2FA in the main UI of the wallet
    • If user has exclusive access over its private keys:
  • Supports HD wallets (BIP32)

PASS BIP32 path m/0/0/c/i with multisig

  • Provides users with step to print or write their wallet seed on setup

PASS Provides a PDF document with QR codes

  • Uses a strong KDF and key stretching for wallet storage and backups

PASS PBKDF2 with 10000 iterations for PDF backup and server based backup

  • On desktop platform:
    • Encrypt the wallet by default

NOTE No desktop storage is used

  • For hardware wallets:

N/A

  • Prevents downgrading the firmware
Contributor

harding commented Apr 24, 2015

@crwatkins great review, thanks!

@bpdavenport I compiled a preview of the listing (see below) and I think it would be better if we could cut just a little bit of text from the description so the scroll bars aren't necessary. (This isn't a requirement; it just looks better.)

Otherwise, in the absence of critical feedback, I'll merge this pull around 12:00 UTC Monday. (I'll be squashing down to one commit at that time.)

2015-04-24-074840_1280x800_scrot

Contributor

bpdavenport commented Apr 25, 2015

@harding Text no longer causes scrolling for me in Chrome. YMMV with different browsers/fonts

Contributor

harding commented Apr 25, 2015

@bpdavenport looks great! Thanks!

Contributor

luke-jr commented Apr 26, 2015

@harding Chrome Apps have the same security level as any other webwallet, and worse than bitcoin banks... Unlike bitcoin banks that are straightforward about their nature (which can implement best practices such as offline wallet storage), browser apps expose the user's funds to being online pretty much all the time.

Contributor

harding commented Apr 26, 2015

@luke-jr If key storage and transaction signing is done locally, and an app requires user permission to be upgraded, then an app has a different security level than a web wallet that can have its code changed at any time.

I also don't think that an always-online wallet is significantly less secure than a sometimes-online wallet given that malware can wait in the background for a wallet to become unlocked and online. In that way, Chome apps are similar to every other desktop app.

I continue to think that allowing browser apps outside of the Web Wallet quarantine is good policy for the current layout. Hopefully, a hypothetical future layout will better be able to convey the comparative risks between wallets without having to segregate web wallets into a special-case category.

Contributor

luke-jr commented Apr 26, 2015

@harding Do Chrome Apps ever require user permission to upgrade? I thought they were strictly auto-upgrade.

I wasn't contrasting always-online to sometimes-online, but to the always-offline cold wallet used by bitcoin banks.

Contributor

harding commented Apr 26, 2015

@luke-jr hrrm; I don't regularly use Chrome/Chromium and I thought updates required manual intervention by default like in Firefox/Iceweasel---but I was wrong. I can't even find a setting to disable auto-updating.

I'm going to think about this for a bit before I reply further. Thanks for correcting me!

I'm also going to push back the planned merge date on this until at least Wednesday so everyone has a chance to comment on this complication.

Contributor

crwatkins commented Apr 26, 2015

I believe our "Web" section is a selection category used by a visitor to narrow their search for a wallet based on the platform they wish to use. If they are using a shared computer, or maybe their work computer, or a Windows Phone, they may want a “Web” wallet. A Chrome app does not fall into this category because you have to install it and change the state of the computer.

However, I definitely agree with @luke-jr that Chrome apps have the same security level as web wallets. This we reflect in our transparency score as a remote app:

checkfailtransparencyremote: "Remote app”

This wallet is loaded from a remote location. This means that whenever you use your wallet, you need to trust the developers not to steal or lose your bitcoins in an incident on their site. Using a browser extension or mobile app, if available, can reduce that risk.

which I would also apply to any native app that updated automatically. For those reasons, I believe that BitGo is currently scored correctly.

Contributor

bpdavenport commented Apr 27, 2015

A few comments:

I scored the "Remote App" the same as the web app for the reasons above. Chrome has made it quite difficult to disable auto-updates (it appears to potentially be possible on Windows machines at least, not sure).

There is a key difference from a web-app however. With a web-app, the code is unsigned, and is downloaded from the server afresh every time, presenting a larger attack surface. With the Chrome app, an update is a signed piece of code from BitGo which can be verified against our repository. Moreover with the chrome app, BitGo would have no mechanism to serve different code to 1 user than another, if demanded to do so by a three-letter agency. (We have not received any such demands or requests to date).

Secondly, if you follow the instructions in the source repository, you can install from source directly, and have no auto-update behavior whatsoever.

Contributor

harding commented Apr 27, 2015

@crwatkins great point about the "Remote app" score! Somehow I'd forgotten about that.

@bpdavenport thank you for your comments!

@luke-jr I think the conspicuous yellow-colored "Remote app" score text clearly (but succinctly) notifies the reader of the risks you're worried about, so I feel it's acceptable to put Chrome apps in the Desktop category. Please let us know if you still have a reason for disagreeing.

Otherwise, in the absence of additional critical feedback, I'll merge this pull around 14:00 UTC Wednesday.

@harding harding closed this in 1de5c8f Apr 30, 2015

@crwatkins crwatkins referenced this pull request Jun 25, 2015

Closed

Add Copay wallet #888

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