Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
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
Added greenbits #1065
Conversation
harding
added
the
Wallets
label
Sep 21, 2015
|
I have reviewed GreenBits based on the current wallet requirements criteria and my evaluation is below. The summary is that I found nothing wrong with the security or the overall design. In fact, it is impressive that GreenBits supports many of the innovative features that GreenAddress is well known for. However, because of the bugs and rough edges noted in the review, in addition to the lack of HSTS on the web site, I cannot at this time recommend it for listing. I would be happy to re-review GreenBits and recommend it for listing as soon as the following are addressed:
GreenBitsVersion 1.53Review version 2015092501The wallet list is based on the personal evaluation of the maintainer(s) and regular contributors of this site, according to the criteria 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 GreenBits is the new Android native version of the existing GreenAddress wallet. However it is not yet fully featured. For example, some settings are not yet available, such as enabling time locked transaction emails for 2of2 recovery or creating new accounts. Another important operation, “redepositing” time locked transactions which have expired, requires using the older GreenAddress wallet instead. Because of this, undestading the "full security story" with this wallet is more difficult than with the GreenAddress wallet alone. NOTE No BIP70 review/testing was done beyond the simplest testing of a request NOTE No social media (Reddit, etc) addressing was reviewed/tested NOTE HW.1 was used with relative success for HD seed storage and login. NOTE NFC seed storage was tested. When logging in with NFC, sometimes the phrase is displayed on the Android screen in plaintext which is not acceptable. NOTE Trezor could not be tested. GreenBits cannot create a new wallet with Trezor, so the wallet needs to be created with GreenAddress. This did not work and was reported to info@greenaddress.it in April, but no reply was ever received. Others on Reddit and Bitcointalk have the same issue with Trezor: http://www.reddit.com/r/Bitcoin/comments/2uv8iv/greenbits_the_all_new_snappy_android_bitcoin/cocviy2 https://bitcointalk.org/index.php?topic=521988.msg10398459#msg10398459 This was reported again earlier this week to info@greenaddress.it, but no suggestions regarding Trezor have been received. NOTE When switching major tabs between “Send” and “Receive” and “All” the account selected may change. If the account is set to “Account 1” on Receive, it should be on “Account 1” when you switch to Send, but it is not always. This is fairly confusing. This was reported in April. Basic requirements:
PASS Google Play store lists 1000-5000 downloads and there is some discussion on Bitcointalk https://play.google.com/store/apps/details?id=com.greenaddress.greenbits_android_wallet https://bitcointalk.org/index.php?topic=521988.0
PASS No evidence of considerable harm has been found, however: CONCERN GreenAddress servers were down about a month ago and users were certainly inconvenienced https://www.reddit.com/r/Bitcoin/comments/3fvhh4/greenaddress_has_been_down_the_past_few_days_how/ The biggest concern is that there was no evidence that users were able to successfully use recovery tools while the servers were down. See below for more detailed concerns on recovery tools.
PASS Responsive and willing to engage on Reddit and Bitcointalk.
PASS Uses bitcoinj
PASS No direct indication that changes are not properly test, but In April some odd fees (some high, some unusual) were noticed and reported on transactions with no change. During this review, it was found that some transactions with no change failed and a blank error message was displayed. Since sending the entire balance of a wallet is not unusual, a bug doing so might suggest a lack of adequate testing.
PASS 5-Feb-2015
FAIL Concerning bug has been noted that GreenBits was not able to send an amount that returned no change. It would fail with a blank error message. This has been reported and is being fixed.
PASS For greenaddress.it
PASS Rating: A
FAIL greenaddress.it does not support HSTS. Previous test on 12 Apr 2015 passed with HSTS max-age of 356 days NOTE Executables are downloaded via Google Play and F-Droid
PASS https://greenaddress.it/en/about/
PASS Uses a different change address for each transaction. It gets change addresses from the same path as received addresses (m/1/i)
NOTE The client's HD seed is stored locally encrypted by a key which is stored online (server) and accessed via a PIN
PASS PIN system provides aggressive lockout (3 failures, keys are erased) enforced by server. Original HD phrase is then required for recovery.
N/A
NOTE User has some access to private keys (multi-sig wallet)
PASS Wallet can be backed up using BIP39 phrase NOTE To enable the 2of2 account timelock recovery feature which would allow the user to recover from a server failure, the user has to enable this feature from the GreenAddress wallet. It cannot yet seem be done from GreenBits even though the GreenBits UI suggests that enabling E-mail 2FA will enable it.
PASS Wallet can be logged in (restored fully) with wallet BIP39 phrase NOTE If GreenAddress server is not available, 2of2 multi-sig accounts can be recovered by using an offline utility using the wallet’s BIP39 phrase and time locked transactions email previously from server (after time lock expires). The main account BIP32 path is m/1/i. NOTE Using the 2of2 recovery utility Gentle from online source https://greenaddress.github.io/gentle/ fails because Gentle can’t access mixed (http vs https) content at http://btc.blockr.io/api/v1/block/info/last . Firefox blocks mixed content and blockr is going to redirect to https anyway, so these references should be changed to https. This was reported in April. NOTE Using Gentle on Firefox from local file system caused Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://btc.blockr.io/api/v1/block/info/last. This can be fixed by moving the resource to the same domain or enabling CORS. This was reported in April. This means that Gentle failed to restore funds because it did not know that the locktime had expired. It would not attempt to submit the transactions. I extracted the raw transaction and submitted them by hand using the core client. That worked. PASS Timelock transaction emails can be used to recover funds FAIL Gentle would not be able to be used as a tool to recover those 2of2 funds for most users NOTE This issue was reported in April in Review version 2015041201 and has still not been addressed. NOTE If GreenAddress server is not available, 2of3 multi-sig accounts can be recovered using the wallet’s BIP39 phrase plus the “subaccount recovery mnemonic passphrase” (BIP39) and the “subaccount GreenAddress xpub” (xpub key). NOTE GreenAddress provides a 2of3 recovery procedure here http://blog.greenaddress.it/2of3recovery/ However it involves a fairly complex source install (because of dependencies) of Electrum with a GreenAddress plugin patch. During a recent GreenAddress server outage, there was no evidence found that end users were successful with this approach: https://www.reddit.com/r/Bitcoin/comments/3fvhh4/greenaddress_has_been_down_the_past_few_days_how/ NOTE However I was able to generate all three addresses for a transaction by hand (BIP32 path m/k’/1’/i where k is the sub account, and the P2SH address order 1) server, 2) wallet (hot), 3) recovery(cold) (not sorted), so this should be possible to recover.
PASS https://github.com/greenaddress/GreenBits
NOTE This is a multi-signature wallet with 2of2 and 2of3 account options
PASS Provides a rich set of 2FA options: E-mail, Google Authenticator, SMS, and Phone call
PASS Very obvious constant reminder in orange bar at bottom of screen (can be disabled in settings)
PASS 2FA is required for spending by default
PASS Can send funds to any address or use recovery tools.
N/A
Optional criteria (some could become requirements):
NOTE No known independent audits performed
PASS
PASS
PASS Software and hardware signatures are deterministic. This was tested by recreating a 2of2 script by hand and signing it using custom code based on pybtctools. The result was the same signature as created by the wallet, which indicates that it is RFC 6979 compliant.
PASS Provides contact email address and phone number on support page https://greenaddress.it/en/support/ NOTE There have been some issues with response to email during testing and by others http://www.reddit.com/r/Bitcoin/comments/30apm1/stay_away_from_greenaddressit/
N/A
NOTE This is a multi-sig wallet. User has some control over private keys.
PASS BIP32 with BIP39 seed, uses m/1/i path for 2of2 sig
PASS Users are provided with BIP39 mnemonics in addition to QR code or saving to NFC token
PASS Wallet storage uses strong keying material stored on server retrieved with PIN
N/A
N/A
|
|
Thank you for all the helpful feedback. We'll tackle these blocking issues asap. |
We're not totally sure what that buys the user security-wise. NFC can already be read from far away if not shielded properly. |
For a targeted attack, I definitely agree this doesn't buy you much. The contrived scenario that I'm thinking of is me in a coffee shop with my RFID on my key ring on the table next to my mobile. My key ring gets a little too close to my mobile and all of a sudden it is displaying my seed phrase for all to see. Sometimes it appears for only a few seconds but sometimes it displays forever (until I click on something). My original concern was the "forever" display, but displaying the seed anytime provides no mitigating value. NFC support is can of worms, but that's a entire topic of its own. |
|
@crwatkins Can you give me steps to reproduce this:
I can't reproduce. |
Errors will appear in web console (and Current block on web page will be blank). Are you perhaps using an edited version? Perhaps a version with greenaddress/gentle#10 already applied? That would likely fix it. |
|
Hmm, that's strange. If that PR fixes it, great, but I nor Lawrence can seem to reproduce. |
|
I don't have a CORS issue after applying greenaddress/gentle#10 |
saivann
added
the
Need more info
label
Oct 16, 2015
|
@crwatkins We'd like to ask for your consideration again.
Done
WIP. We are refactoring the backend to support this. Many wallets don't support this, but we are committed to supporting it ASAP.
We have written and published a series of tutorials which will walk them through it. https://blog.greenaddress.it/how-to-recover-your-funds-using-gentle-tool-2-of-2-setup/ We have also done bugfixing on the gentle tool.
https://blog.greenaddress.it/how-to-intitialize-your-ledger-wallet-with-greenaddress-services/ These tutorials cover the nooks and crannies of implementation quirks between the platforms.
Done and released in 1.54. greenaddress/GreenBits@eecb6ac The issues where it didn't log in automatically all the time should be fixed as well.
Still in our TODO pile. Agreed in principle. I believe our app is ready for listing. I'll have to update this PR but I wanted your recommendation first. |
|
I've tried to initialize my Trezor per your instructions. Twice I got all the way through setting up 2FA and getting into a new wallet following the instructions. However when I log out, I can't log back in again with Trezor and the Chrome app (or GreenBits). The first time I try it says "An error has occurred which has forced us to log you out". The next time I try it says "Account not found, please create a new one with a hardware wallet." I'll contact you via support to debug. |
|
@instagibbs thank you for your updates. I concur with the current scoring in the pull request.
|
|
Great! I'll rebase and see if we can finish this up ASAP |
|
thank you @crwatkins We really appreciated all the time and effort you spent thoroughly reviewing the app. From #725 and in general I would also like to thank @saivann and @harding for all their work And obviously thank you @instagibbs for getting GreenBits ready for bitcoin.org |
|
Looks great! Thank you to everyone for iterating on this until it was ready. In the absence of critical feedback, this will be merged Monday. |
instagibbs commentedSep 21, 2015
This change adds GreenBits to the mobile wallet list for Android.
GreenBits is a native Android Bitcoin Wallet app based on BitcoinJ's latest stable release.
The wallet is BIP32, supports BIP70 and other payment protocol related BIPS, supports a variety of hardware wallets (Ledger, Trezor, KeepKey and anything that uses the same interface), uses P2SH multisignature in 2of2 and 2of3 forms with a multisig oracle (GreenAddress) while validating all data via SPV and generally aiming for zero/minimal trust including minimal permissions involved with the app on the stores.
The app has been in production for a number of months being released in early February of 2015, and the [open] source code is available on GitHub and the app is also available on F-Droid.
The oracle service it uses has been in operation since 2013-04-16 and on bitcoin.org with HTML5 hybrid apps since April 2014.
Let us know if you have any questions or suggestions.
Updates: We have released a new version of the application(1.53) in production with many bug fixes including the ones listed in this thread, and enhancements, including additional security and privacy features such as SPV connection to a trusted Tor onion node. A number of updates have also been completed to enhance the supporting tools such as Gentle.
We would like to “re-submit” this for consideration as we have removed the issues that were blocking this app from being included. It is now also boasting some next-level features such as Tor connectivity to trusted node, which we feel sets the bar for future security of thin wallets.