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

Add SeedSigner as HWW #4179

Merged
merged 7 commits into from
Jul 3, 2024
Merged

Conversation

omerskywalker
Copy link

This PR adds SeedSigner as a HWW to Bitcoin.org Wallets section

@crwatkins
Copy link
Contributor

Thanks for the submission! Should this DIY project be in the hardware wallet category? In the past I have discussed the possibility of creating a separate category for DIY signing devices such as the SeedSigner, but that is a longer term option. There is also a current submission to add AirGap Vault which is a DIY signing solution using mobile phone hardware which we decided to submit in the regular mobile wallet category. However, I believe SeedSigner is different in that it

  1. Uses a mostly standard set of hardware components
  2. Can be purchased as an assembled device

I believe the review should move forward in the hardware wallet category, but I welcome any other input from the community.

@kdmukai
Copy link

kdmukai commented Apr 22, 2024

The description, features, and checkbox matrix all look correct to me (though I defer to @SeedSigner for thoughts on the exact description text).

I enthusiastically support adding SeedSigner to the hardware wallet list on the site.

ACK.

@kdmukai
Copy link

kdmukai commented Apr 22, 2024

Minor grammar nit: I see no reason to hyphenate "via a QR-exchange process".

Sentence could also just be simplified to: "...via QR codes."

@crwatkins
Copy link
Contributor

While we are suggesting description edits: Bitcoin should be capitalized in the description. On this site we (try to) follow the rule that Bitcoin protocol references are capitalized and the currency is not.

@SeedSigner
Copy link

I apologize for not checking in sooner, I have been coordinating with @omerskywalker on this PR. I appreciate your thoughtful consideration and agree that it makes sense to include it with hardware wallets. Also have no issue with the capitalization of B/bitcoin as suggested -- thank you again.

@newtonick
Copy link

ACK

@omerskywalker
Copy link
Author

I've edited the description text with grammatical fixes for the capitalization errors and adjusted the wording for QR codes.

Thank you guys for the suggestions.

@crwatkins
Copy link
Contributor

All of the currently listed hardware wallets are seed storage devices with an offline signing engine. SeedSigner is an offline signing engine without the seed storage. One of the current criteria for hardware wallet listing is

  • Protects the seed against unsigned firmware upgrades

I believe that this criterion was originally added to help protect against seed exfiltration by malware (originally by USB) after a firmware update. This could also apply to malware leaking keys in signature nonces or perhaps via any PSBT transport. SeedSigner does not verify firmware signatures autonomously and in the strictest sense does not meet this requirement. Likewise, neither would other DIY/BYOD projects such as Specter-DIY or AirGap Vault.

I have proposed a PR to create a new environment category for wallets in which the hardware platform is user supplied and to indicate that the hardware protection may not be as strong as other solutions. The new category is actually quite consistent with the purpose and the application of the existing categories.

As a side note, when thinking about physical protection, I'm reminded that many of the devices that are currently listed have shown vulnerabilities to various levels of attacks with physical access to the device. It is important to note that there are currently no criteria dealing directly with physical attacks to the device.

If PR #4194 is merged, I will propose updating the environment score in this PR to reflect the relaxed interpretation of the above requirement.

@crwatkins
Copy link
Contributor

I have reviewed the SeedSigner device based on the current wallet requirements criteria and my evaluation is below. The summary is that the device passes on security and overall design, excluding the criteria directly related to the storage and the protection of the seed which is not provided in this BYOD (bring-your-own-device) device. I have proposed a new environment category for DIY/BYOD "hardware wallets" which could apply to devices that may not provide secure seed storage. In addition, there is an administrative issue that needs to be addressed on the seedsigner.com site for listing. I will be glad to recommend SeedSigner for listing after the following items have been completed:

  1. The new environment category checkpassenvironmentbyod is implemented on bitcoin.org
  2. The environment score in this listing is updated to checkpassenvironmentbyod
  3. HSTS has been added to the seedsigner.com site (see below)

There are no current criteria relating directly to gathering entropy or seed generation, so the following opinions are simply observations made during the review.

I personally have a concern over using the seed generation from image feature. I believe that this feature requires too much trust in assumptions about the user-chosen camera device as well as the user's operation of that device with no further instructions other than "click image" and "accept". Of the three entropy inputs to this feature, two of them, system clock and serial number, are definitely insufficient, requiring the image input to stand on its own for every seed generated. I am uncomfortable with that assumption. The exact hardware employed is unknown and the failure modes (and even the operational modes) cannot be characterized, and the knowledge of the user is unknown. There are no sanity checks in the code of the data received from the camera other than relying on the potentially novice user to believe it is acceptable. I'm all for combining multiple sources of entropy, however the Raspberry Pi hardware random number generator would be an astronomically better source than the serial number. I have expressed these concerns to the project. I cannot personally recommend using this feature.

Likewise when using the dice feature, the user should be able to perform more than 50 (99 for 256 bits) rolls as this exact limit assumes nearly unbiased dice and nearly perfect user operation. All physical dice are biased. The question is not "if," but "how much?". It's unlikely that most dice are sufficiently biased to provide an exploitable vulnerability, but the point is we do not know. Extra rolls, while very simple and not very time consuming could mitigate mistakes made by novice users without exact instructions on how to use the feature. For example an uninformed user may throw multiple dice at a time and either intentionally or unintentionally enter them in a specific order that does not preserve the full entropy.

As an aside, I'm a big fan of combining multiple independent sources of entropy in a way that is externally verifiable. In line with the original focus of SeedSinger, I've always thought that the device should be able to accept and combine multiple sources of entropy. My suggestion would be to accept seeds via words or QR codes like SeedSigner already does and perform a SeedXOR on the input to create a new seed. Input seeds could easily come from multiple sources such as other hardware wallets, mobile apps, desktop apps, or physical methods such as dice, coins, and cards. The entire operation can be externally verified with paper and pencil or a small portion of the seed can be independently spot checked. I would like to be clear that I'm not advocating for or against using SeedXOR for it's original seed backup purpose; I'm advocating for a novel approach to combining entropy.

Enough time on my soapbox. Let's get back to the review.

Note that as a signing project, only the SeedSinger signing device was evaluated. Wallet software that runs externally to the SeedSigner device was not evaluated in this review. Sparrow Wallet, Specter Desktop, and BlueWallet were used during this review, but were not evaluated.

With the exception of the environment score mentioned above, I concur with the scoring in eb383bc


SeedSigner

Firmware v0.7.0

Review Version 2024041301

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.

Basic requirements:

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

    PASS Significant discussions can be found on bitcointalk.org, Twitter/X, and Reddit with many videos on YouTube

    NOTE There has recently been a fair amount of discussion on social media regarding physical attacks (such as evil maid attacks) on hardware devices with some concluding that these attacks would be easier on a SeedSigner than on hardware wallets with special-purpose hardware. In general, I agree with this. Users should keep this in mind when choosing a device depending on their own environmental situation and their specific attack concerns. bitcoin.org currently has no criteria directly relating to attacks which involve physical access to the wallet.

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

    PASS No indication given a review of the above mentioned 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 indiction

  • No indication that the wallet uses unstable or unsecure libraries

    PASS No indication. Uses embit library.

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

    PASS No indication. Tests at https://github.com/SeedSigner/seedsigner/tree/dev/tests

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

    PASS First tag was 16 December 2020

  • No concerning bug is found when testing the wallet

    PASS No concerning bugs were found

    NOTE While taproot is present in the menus and there is significant taproot support, scanning taproot addresses for verification is not yet implemented

  • Provides a bug reporting method on the website and/or in the app

    PASS https://seedsigner.com/get-in-touch/

  • Website supports HTTPS and 301 redirects HTTP requests

    PASS http://seedsigner.com redirects to https://seedsigner.com

  • SSL certificate passes Qualys SSL Labs SSL
    test

    PASS https://seedsigner.com A rating

  • Website serving or linking to executable code or requiring authentication uses HSTS

    • Existing listings: With a max-age of at least 180 days
    • New listings: With a max-age of at least 1 year, and preload and includeSubDomains directives to qualify for browser preload list inclusion
      e.g. Strict-Transport-Security: max-age=31536000; includeSubDomains; preload

    FAIL No HSTS header on https://seedsigner.com

  • The identity of CEOs and/or developers is public

    PASS Lead developer Keith Mukai is active in interviews and conferences

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

    N/A Addresses and paths are chosen by wallet software

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

    N/A Addresses and paths are chosen by wallet software

  • Uses deterministic ECDSA nonces (RFC 6979)

    PASS Uses deterministic nonces with low R grinding from embit library that match signatures from Sparrow.

  • User has access to private keys for all major components of the wallet

    PASS User has access to the seed words and QR code from the UI menu

  • 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.

    N/A Keys are not stored online

  • If user has exclusive access over its private keys:

    • Allows backup of the wallet

    PASS The wallet can be backed up to words or QR code on offline media

    • Restoring wallet from backup is working

    PASS Restoring the wallet from SeedQR is working

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

    PASS https://github.com/SeedSigner/seedsigner

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

    • 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

    N/A

  • For hardware wallets:

    • Uses the push model (computer malware cannot sign a transaction without user
      input)

    PASS Device requires user confirmation to sign a transaction

    • Protects the seed against unsigned firmware upgrades

    N/A The BYOD environment proposed for this listing does not necessarily include secure seed storage

    • Supports importing custom seeds

    PASS

    • Provides source code created for all open components and provides detailed specification for blackbox testing of
      any closed-source secure elements

      PASS

Optional criteria (some could become requirements):

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

    N/A

  • Website serving executable code or requiring authentication is included in the
    HSTS preload list

    FAIL HSTS is not enabled

  • If user has exclusive access over its private keys:

    • Supports HD wallets (BIP32)

    PASS

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

    PASS Has tools for manual transcription of QR codes and BIP39 phrases

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

    N/A The BYOD environment proposed for this listing does not necessarily include secure seed storage

    • On desktop platform:
      • Encrypt the wallet by default

    N/A

  • For hardware wallets:

    • Prevents downgrading the firmware

    N/A The BYOD environment proposed for this listing does not necessarily include secure seed storage

@SeedSigner
Copy link

I appreciate the time and thought that was put into this evaluation. We will activate HSTS on seedsigner.com and I will advise when this is has been completed.

On the issue of a separate category for DIY/BYOD, though I worry that a separate category tends to sideline these kinds of devices (which in my opinion are viable alternatives that are more in line with the core ethos of bitcoin) in favor of devices that are exclusively manufactured and distributed by centralized, for-profit companies, the DIY/BYOD descriptor for this other category is factually accurate and makes sense.

I would however express concern about the (exclusive) use of the term "wallet". In my view, the core characteristic of a "wallet" is that its purpose is to store something of value -- in the conventional sense, a "wallet" stores physical currency, credit cards, personal credentials, etc. One of the primary strengths of SeedSigner is that it embraces an amnesiac/stateless approach whereby the device itself intentionally doesn't store anything of critical value (i.e. PKs), making the term "wallet" not appropriate.

Early on in the project's life, I opted to focus on the term "signer" because of our separation of the function of key storage from key signing. A significant component of the larger SeedSigner approach is eschewing the idea of secure digital key storage (a never-ending cat-and-mouse game, the details of which are opaque and beyond the technical comprehension of most people, and still requires analog backup of key material), thus encouraging users to focus their attention on other approaches to securing private keys, namely use of a BIP39 passphrase, or better yet, multi-signature wallets (SeedSigner brings the marginal cost of creating and using multiple keys effectively to zero).

I don't know if it makes sense to acknowledge the wallet / signer distinction within your categorization scheme, but wanted to point this out because I think it is worth noting. Thank you again for the time and effort you have put into this evaluation.

@crwatkins
Copy link
Contributor

Thanks for looking into the HSTS.

Since we have some confusing issues with terminology on the site with terms such as categories, features, and scores, I thought I would try to clarify how this new “environment score” will show up on the site. We have an unfortunate situation in that we have concepts that are named differently in “the code” to how they are presented to users. I’ll try to ignore both sets of naming and clarify in English what will be shown with this new classification.

First of all, there is a wizard and a filter which allows selecting from two mobile operating systems, three desktop operating systems, or hardware. That’s not changing. The PR is to list SeedSigner in the hardware list. When a hardware list is selected, SeedSigner will show up in a table with some columns. One of those columns is labelled Environment. All currently listed wallets have a dark green icon in that column. SeedSigner will have a light green icon. The dark green icon is labeled (at the bottom of the table) Good. The light green icon is labeled Acceptable. When existing hardware wallets are selected the following is displayed (along with the other information):

Very secure environment

This wallet is loaded from a secure specialized environment provided by the device. This provides very strong protection against computer vulnerabilities and malware since no software can be installed on this environment.


When SeedSigner is selected the following is displayed instead:

User sourced environment

This wallet is loaded on a device sourced by the user. This can provide reasonable protection against malware as long as you source your hardware correctly and secure it sufficiently.


That’s the only difference.

As for the hardware wallet terminology, as I’ve frequently mentioned before:

We've struggled with terminology in the past that most "hardware wallets" are not really wallets, but rather a combination key store and signing engine. (Perhaps the notable exception is the Case hardware wallet which has been discontinued.) In the past we've chosen to continue using the "hardware wallet" nomenclature to be consistent with developers and the market.

Back in 2019, during the last major revamp of the bitcoin.org wallet pages, it was discussed whether nomenclature should be changed to to de-emphasize the wallet terminology across the board. I seem to remember that it was decided that doing so at that time would actually be too confusing since all of the vendors and projects listed used the wallet terminology themselves. Perhaps it is time to create an issue to resume those discussions.

@crwatkins
Copy link
Contributor

@omerskywalker #4194 has been merged. You can now update the environment score if you wish without failing the CI.

@crwatkins
Copy link
Contributor

  1. Add environment score for DIY hardware wallets #4194 has been merged.

  2. @SeedSigner thanks for updating the HSTS on the site. While not required, you might want to consider submitting to hstspreload.org.

  3. @omerskywalker once the environment score in this listing is updated to checkpassenvironmentbyod I will be glad to recommend SeedSigner for listing.

@crwatkins
Copy link
Contributor

@omerskywalker Thanks for the update. The build failed because I did not update the test schema. It's my problem, not yours. I will fix that soon.

@omerskywalker
Copy link
Author

Thanks for the quick reply @crwatkins - I will resubmit once the test schema has been updated to allow for the newly created value.

@crwatkins
Copy link
Contributor

Once #4237 is merged, this PR should be able to be merged.

I recommend SeedSigner for listing.

@omerskywalker
Copy link
Author

All checks have passed for this PR - @crwatkins could we proceed with the merge? Thank you for your support on this.

@crwatkins
Copy link
Contributor

@omerskywalker Thanks! After a brief period to allow for comments on the review, @Cobra-Bitcoin will likely merge.

@Cobra-Bitcoin Cobra-Bitcoin merged commit ec76da8 into bitcoin-dot-org:master Jul 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants