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

Keepkey #1149

Merged
merged 4 commits into from Dec 27, 2015

Conversation

Projects
None yet
3 participants

This pull request is for adding KeepKey wallet to the "Choose Your Wallet" section.

@harding harding added the Wallets label Dec 1, 2015

Contributor

crwatkins commented Dec 9, 2015

I have reviewed KeepKey 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 device was released less than three months ago, I cannot at this time recommend it for listing, but I will recommend it for listing on 23 December 2015.

Note that as a "hardware wallet," only the hardware and firmware components of the device were evaluated. Wallet software that runs externally to the KeepKey device was not evaluated in this review. As an aside, I would like to change this in the future and evaluate combinations of hardware and software as a wallet system, but that's for another day. The KeepKey Chrome extension and MultiBit HD were used during this review, but not evaluated.

I concur with the scoring in the pull request.

KeepKey

Firmware v1.0.3

Review Version 2015120801

The 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 The hardware device used for testing was provided to bitcoin.org by KeepKey at no cost

NOTE Only the hardware/firmware is being evaluated here. The wallet software running external to the device is ignored as out of scope.

Basic requirements:

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

NOTE No independent security audit is available https://www.keepkey.com/keepkey/faq/

PASS Many reviews and discussions are available. For instance:

  1. Hardware and firmware analysis https://medium.com/@AussieHash/keepkey-under-the-hood-3beac31e1064
  2. https://www.reddit.com/r/Bitcoin/comments/3gqnxa/first_review_of_the_keep_key_hardware_wallet/
  3. http://blockzero.org/2015/10/02/keepkey-hardware-wallet-review/
  4. https://www.reddit.com/r/Bitcoin/comments/3na9dy/keepkey_hardware_wallet_review/
  5. Product reviews on amazon.com
  6. https://www.reddit.com/r/keepkey/
  • No indication that users have been harmed considerably by any issue in relation to the wallet

PASS No indication found

  • 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. KeepKey is active and responsive on public forums.

  • No indication that the wallet uses unstable or insecure libraries

PASS No indication. Uses same libraries as Trezor.

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

PASS No indication

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

NOTE Released 23 September 2015. Eligible for listing on 23 December 2015.

  • No concerning bug is found when testing the wallet

PASS No concerning bug was found testing the hardware

  • Website supports HTTPS and 301 redirects HTTP requests

PASS http://www.keepkey.com and http://keepkey.com redirect to https://

PASS https://www.keepkey.com: A+ rating

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

PASS Max-age is one year

  • The identity of CEOs and/or developers is public

PASS https://www.keepkey.com/management-team/

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

N/A Addresses, or paths, are chosen by wallet software

  • 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 Keys are stored in the device. Failed PIN requests result in an exponential backoff of lockout time.

  • 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 Allows manual copying of BIP39 phrase at startup

NOTE KeepKey does not require nor provide an option to confirm the phrase

  • Restoring wallet from backup is working

PASS Restoring the KeepKey device is working. Restoring the wallet (funds) to MultiBit HD using the BIP39 phrase also works.

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

PASS https://github.com/keepkey

NOTE Claims of replication of deterministic builds of shipping firmware: https://medium.com/@AussieHash/keepkey-under-the-hood-3beac31e1064

  • 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:
    • Uses the push model (computer malware cannot sign a transaction without user input)

PASS A physical button press on the device is required for confirmation

  • Protects the seed against unsigned firmware upgrades

PASS FAQ at https://www.keepkey.com/keepkey/faq/ claims keys are erased prior to upgrade and only restored if firmware is signed correctly

NOTE Not tested

  • Supports importing custom seeds

PASS A seed generated by another wallet was imported

  • Provides source code and/or detailed specification for blackbox testing if using a closed-source Secure Element

PASS Provides source code at https://github.com/keepkey

Optional criteria (some could become requirements):

  • Received independent security audit(s)

NOTE No known security audits https://www.keepkey.com/keepkey/faq/

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

N/A

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

N/A KeepKey does not display received transactions

NOTE KeepKey Chrome extension: FAIL: Shows "received from" Bitcoin addresses

  • Uses deterministic ECDSA nonces (RFC 6979)

PASS The same transaction was signed with KeepKey and pybitcointools verifying RFC 6979 with low S signatures

  • Provides a bug reporting policy on the website

PASS https://www.keepkey.com/contact-us/

  • If user has no access over its private keys:

N/A

  • Full reserve audit(s)
  • Insurance(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 Uses standard BIP44 paths

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

PASS

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

N/A

  • On desktop platform:
    • Encrypt the wallet by default

N/A

  • For hardware wallets:
    • Prevents downgrading the firmware

PASS Wallet allows downgrade of firmware, but erases stored seeds. This was not directly tested because no downgrades currently exist.

Contributor

harding commented Dec 9, 2015

@crwatkins Thank you for your review! This looks like a nice hardware wallet, and an excellent addition to the site in a few weeks. I'm going to tag this as merge scheduled, but I'll confirm everything remains ok with you before merging it on/after the 23rd.

@dstanchfield Thank you for providing an evaluation unit, and for working with Craig on the review!

Contributor

crwatkins commented Dec 24, 2015

I recommend KeepKey for listing.

@harding harding merged commit a1fa2f7 into bitcoin-dot-org:master Dec 27, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

harding added a commit that referenced this pull request Dec 27, 2015

@crwatkins crwatkins referenced this pull request Jun 29, 2016

Closed

Add Opendime #1319

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