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

Added Coinomi wallet #780

Merged
merged 5 commits into from
May 22, 2015
Merged

Added Coinomi wallet #780

merged 5 commits into from
May 22, 2015

Conversation

xeniacx
Copy link
Contributor

@xeniacx xeniacx commented Feb 27, 2015

No description provided.

@harding harding self-assigned this Apr 24, 2015
…e no deterministic build system is involved in the build process whatsoever
Downgraded Coinomi's transparency level to checkpasstransparencyopensource since no deterministic build system is involved in the build process whatsoever
@crwatkins
Copy link
Contributor

I have reviewed Coinomi based on the current wallet requirements criteria and my evaluation is below. The summary is that the wallet passes on security and overall design. However, because the website that links to executable code is not yet secure (no TLS) and the website does not reference the project principals as I noted in the review, I cannot at this time recommend it for listing. Coinomi is working on these issues and I will be glad to recommend Coinomi for listing once the website is updated.

I concur with the current scoring in the pull request.

Coinomi

Version v1.5.13

Review version 2015042501

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 Coinomi is a multi-coin wallet. Only the Bitcoin wallet functionality was tested/reviewed.

Basic requirements:

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

PASS No concerning issues

NOTE Fairly small amount of feedback

https://github.com/Coinomi/coinomi-android/issues
https://bitcointalk.org/index.php?topic=713649.0;all
https://groups.google.com/forum/#!forum/coinomi

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

PASS No indication (see above sources)

  • 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. Few issues above were addressed quickly.

  • No indication that the wallet uses unstable or unsecure libraries

PASS Uses bitcoinj

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

PASS No indication. Some unit testing. Well functioning code.

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

PASS Released 4-Nov-2014 https://bitcointalk.org/index.php?topic=713649.msg9431499#msg9431499

  • No concerning bug is found when testing the wallet

PASS No concerning bugs were found

  • Website supports HTTPS and 301 redirects HTTP requests

FAIL No TLS support for site referencing download coinomi.com

FAIL No TLS

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

FAIL No TLS

  • The identity of CEOs and/or developers is public

FAIL Not listed on coinomi.com. Developers only listed on github.

  • 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 When setting up a new wallet, passwords must be 8 characters long and must not be in a common 10,000 password list

NOTE Setting a password is optional

NOTE When restoring an existing wallet, password complexity is not checked

  • If user has no access over its private keys:

N/A

  • Provides 2FA authentication feature
  • Reminds the user to enable 2FA by email or in the main UI of the wallet
  • User session is not persistent, or requires authentication for spending
  • Provides account recovery feature
  • If user has exclusive access over its private keys:
    • Allows backup of the wallet

PASS BIP39 phrase and QR code in Settings

  • Restoring wallet from backup is working

PASS Restoring wallet from BIP39 phrase works

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

PASS https://github.com/Coinomi/coinomi-android

  • If user has no access to some of the private keys in a multi-signature wallet:

N/A

  • Provides 2FA authentication feature
  • Reminds the user to enable 2FA by email or in the main UI of the wallet
  • User session is not persistent, or requires authentication for spending
  • Gives control to the user over moving their funds out of the multi-signature wallet
  • 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)

NOTE No formal audit, but some peer review http://www.reddit.com/r/blackcoin/comments/2r5k2k/blackcoin_added_to_coinomi_multicoin_android/

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

PASS Uses new change addresses

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

PASS Uses a new receiving address for each transaction (can be disabled in settings)

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

PASS Does not show “received from” addresses

  • Uses deterministic ECDSA nonces (RFC 6979)

PASS Uses RFC 6979. Wallet signatures were duplicated with custom code.

  • Provides a bug reporting policy on the website

PASS Provides a contact form on coinomi.com

  • 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 Supports BIP32 with multi-coin BIP44 path. Used custom code to verify generated addresses.

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

PASS During setup provides a step to write down recovery phrase and allows user to test it

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

PASS Uses Scrypt

  • On desktop platform:

N/A

  • Encrypt the wallet by default
  • For hardware wallets:

N/A

  • Prevents downgrading the firmware

@erasmospunk
Copy link
Contributor

@crwatkins Thanks for the review and feedback.

We are working on a new website that uses HTTPS and will update this once we are ready.

@xeniacx
Copy link
Contributor Author

xeniacx commented May 14, 2015

@crwatkins thanks for your review, I am happy to confirm that the failing issues:

  • Website supports HTTPS and 301 redirects HTTP requests

  • SSL certificate passes Qualys SSL Labs SSL test

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

  • The identity of CEOs and/or developers is public

    have now been resolved. As a reminder, Coinomi's website is: https://coinomi.com/

@crwatkins
Copy link
Contributor

I have re-reviewed Coinomi based on the current wallet requirements criteria and the updates to my evaluation are below. I gladly recommend Coinomi for listing.

Coinomi

Version v1.5.13

Review version 2015051401

  • Website supports HTTPS and 301 redirects HTTP requests

PASS HTTP redirects to HTTPS on coinomi.com and www.coinomi.com

PASS Grade A+ (CloudFlare)

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

PASS 180 days

  • The identity of CEOs and/or developers is public

PASS Front and center on coinomi.com

@harding
Copy link
Contributor

harding commented May 15, 2015

64d1cdc looks good to me (preview below). Thanks everyone!

In the absence of critical feedback, this pull will be merged around 12:00 UTC Monday.

2015-05-15-083008_1280x800_scrot

@xeniacx
Copy link
Contributor Author

xeniacx commented May 15, 2015

The preview looks great! @harding @crwatkins thanks so much both for your help!

@harding harding merged commit 64d1cdc into bitcoin-dot-org:master May 22, 2015
harding added a commit that referenced this pull request May 22, 2015
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.

4 participants