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 Wasabi Wallet #2788

Merged
merged 61 commits into from
Feb 4, 2019
Merged

Add Wasabi Wallet #2788

merged 61 commits into from
Feb 4, 2019

Conversation

nopara73
Copy link
Contributor

@nopara73 nopara73 commented Dec 15, 2018

This is a working in progress pull request to add Wasabi Wallet to Bitcoin.org website.

Wasabi Wallet is an open-source, cross-platform, non-custodial, privacy focused Bitcoin wallet for Desktop. Its main features are built-in Tor, CoinJoin and coin control.

ToDo

See comment: #2788 (comment)

Concerns

  • While the development of Wasabi Wallet started in April of 2016 under the name of HiddenWallet, Wasabi Wallet has only been released in August 1, 2018, which raises the question if the product should mature before making it to the site.

Help With Translation

Consider helping the translation work: WalletWasabi/WalletWasabi#963

Feedback

If you have any feedback as to what should be improved on the wallet in order to make it to the site I am all ears of.

Screenshot

image

References

@wbnns
Copy link
Contributor

wbnns commented Dec 15, 2018

@nopara73 Thanks for working on this.

@crwatkins
Copy link
Contributor

@nopara73 Thanks for the submission! I think this may be the first WIP community supported wallet listing PR. One note on the English wallet description: Most of the other wallet descriptions begin with a full sentence usually including the verb "is", while your description begins with a phrase describing the wallet. I'm not sure that the inconsistency would currently be obvious, or even noticeable, but if we use the descriptions in other places in the future, it would good to maintain the consistency.

@nopara73
Copy link
Contributor Author

nopara73 commented Dec 17, 2018

@crwatkins Fair enough. I looked at the website and thought why duplicate the wallet name, but you're right it's more future proof, for example if you want to put the descriptions under logos.
4cf8336 resolves it.

image

@lontivero
Copy link
Contributor

dotnet abstracts the details and provides us an uniform interface, the System.Security.Cryptography.RandomNumberGenerator. NBitcoin allows us to add additional entropy to re returned number, i think that's because @NicolasDorier didn't trust on a closed source library, after all for Windows at least it relies on the proprietary windows library.

@harding
Copy link
Contributor

harding commented Jan 14, 2019

@nopara73 I wonder if I'm not being clear enough about the simple form of the attack, so here's an extended description.

Alice uses Wasabi. Mallory is Alice's ISP, providing home Internet service. Alice creates a new wallet, generates a new address, and uses an exchange to send some bitcoins to that address. The transaction containing that payment is mined into block 123.

Alice's Wasabi wallet receives the filter for block 123, sees that the block might contain a transaction interesting to the wallet, and downloads the block. Since the request is over the public (unencrypted) Internet, Mallory logs block 123 as being interesting to Alice.

Later Alice participates in a coinjoin that gets mined in block 456. Again, Alice downloads block 456 and Mallory logs that block.

Even later Alice spends some of her money in a transaction that gets mined in block 789. Again, Alice downloads that block and Mallory logs it.

Now Mallory wants to attempt to build a profile of Alice. Mallory uses the block chain to look for connections between the transactions in blocks 123, 456, and 789---and notices that there's path from an output received in 123 to inputs and outputs in 456 to inputs and outputs in 789. This is highly suggestive that Alice was a party to all those transactions, despite there being a coinjoin involved.

This privacy leak doesn't require access to a single node, nor any fancy equipment. It only requires being an ISP, a tcpdump equivalent, and some block chain analysis. If you want, you can test out the connection monitoring part on your laptop right now using Wireshark (which has some nice Bitcoin dissectors built in).

@nopara73
Copy link
Contributor Author

nopara73 commented Jan 14, 2019

@harding You forgot the false positives, but anyway, the attack requires the specific ISP of the user and any Blockchain analysis company to collude in a probably unlawful way. If a user's threat model includes protection against such a high-end conspiracy, then chances are the user already uses Whonix/Tails or other default traffic torifiers.

@harding
Copy link
Contributor

harding commented Jan 14, 2019

@nopara73 in my country, government agencies routinely work with ISPs to spy on their users (and, of course, I'm not proud of that). I think it's the same for a lot of countries.

The point here is that downloading specific blocks has a very real potential for leaking information that's not a risk in clients that download all blocks, and so it's appropriate that Bitcoin.org score these two operating modes differently. For that reason, I agree with @crwatkins that checkpassprivacydisclosurefullnode: "Avoids disclosing information" is not the right score for Wasabi.

I am happy, of course, to support a new score that better reflects the advantages of targeted-block downloads used by BIP157/158 clients (and Wasabi's own current version of that).

@nopara73
Copy link
Contributor Author

nopara73 commented Jan 14, 2019

@harding By that logic Bitcoin Core discloses even more specific sensitive information to your ISP by simply broadcasting transactions over the clearnet, yet I don't think you have any doubt in your mind that Core should get checkpassprivacydisclosurefullnode: "Avoids disclosing information".

@harding
Copy link
Contributor

harding commented Jan 15, 2019

@nopara73

Bitcoin Core discloses even more specific sensitive information to your ISP by simply broadcasting transactions over the clearnet

My node relayed 6 GB of data in the last 24 hours using both clearnet and Tor (which Bitcoin Core will use automatically if it's available on the computer---it'll even automatically setup an ephemeral hidden service if you've enabled Tor cookie authentication). Buried in those over 300,000 unique transactions may or may not have been transactions originating from one or more of the wallets on my intranet, including my personal hot and watching-only wallets. How does an attacker separate my potential specific transactions from all the others I merely relayed?

I don't think you have any doubt in your mind [...]

I feel like you're starting to take this discussion personally. That wasn't my intent. I'm a huge fan of what you're doing with Wasabi and I really would like to see it listed here and on other sites, so I'll back off and try to stay out of further discussion on this issue. Thank you and @lontivero for all the work you've done to help improve Bitcoin privacy.

@nopara73
Copy link
Contributor Author

How does an attacker separate my potential specific transactions from all the others I merely relayed?

Relayed transactions must first arrive to you. If it never arrives, only leaves your node, that's the one.

I feel like you're starting to take this discussion personally.

I agree, my apologies, I will try to keep the conversation on point.

@crwatkins
Copy link
Contributor

@lontivero @NicolasDorier

dotnet abstracts the details and provides us an uniform interface, the System.Security.Cryptography.RandomNumberGenerator.

Please forgive my dotnet ignorance. Does dotnet provide the entropy modules for Linux and Mac? I'm trying to understand the source of entropy on all supported platforms.

@lontivero
Copy link
Contributor

lontivero commented Jan 15, 2019

The dotnet framework provides an uniform cross-platform interface as I said that abstracts the internal details, however we can follow the code and see the three different implementations:

Digging a bit more we can find the code that calls the native functions:

// Enable calling OpenSSL functions through shims to enable support for
// different versioned so files naming and different configuration options
// on various Linux distributions.


Update:

  • Note 1: FWIW the CC in the function's name CCRandomGenerateBytes comes from CommonCrypto so, it is not a PRNG but CSPRNG.

  • Note 2: I used links to source code just to make clear that we based Wasabi on open source projects that can be audited by anyone. We can clone the repo and compile everything as we do we any other project. Sorry for the rant.

@crwatkins
Copy link
Contributor

@lontivero Thanks for your notes on your random number sources. I quickly reviewed them to assist in verifying our criteria

No indication that the wallet uses unstable or insecure libraries

but did not perform any type of security audit on their generation.

@crwatkins
Copy link
Contributor

crwatkins commented Jan 22, 2019

I have reviewed the Wasabi wallet 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 wallet was released less than three months ago, I cannot recommend it for listing at this time. If an outstanding bug found during the review is fixed, I can recommend it for listing on 31 January 2019.

Note that Wasabi wallet only receives to bech32 addresses. We don't currently have any criteria that specifically relate to address format, however I've always assumed an unwritten generic "interoperability with other wallets" criteria. Since Wasabi supports sending to legacy addresses, I'm less concerned that it only allows receiving to bech32 addresses.

Also note that Wasabi has very interesting privacy features such as coin selection and CoinJoin that were demonstrated but not evaluated during this review.

Per previous comments I believe that the scoring in 1113c4c should be updated to

privacy: "checkpassprivacybasic"
privacydisclosure: "checkfailprivacydisclosurespv"

I would support creating finer grained privacy scoring in the future, but I would be happy to list this wallet sooner with this score unless the developers would prefer to wait.

In addition, I think there was some confusion with the transparency score being

transparency: "checkgoodtransparencydeterministic"

I believe it should be changed to

transparency: "checkpasstransparencyopensource"

since Wasabi does not currently support deterministic builds. With those changes to 1113c4c, I concur with the scoring.

Wasabi Wallet

Version 1.1.0

Review Version 2019012201

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 There is a fairly active user community https://www.reddit.com/r/WasabiWallet/ and developer community https://github.com/zkSNACKs/WalletWasabi/ . User support is done on both forums. User discussion at https://www.reddit.com/r/Bitcoin/comments/9sziy5/privacy_wasabi_wallet_10_is_finally_released/ .

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

PASS No indication

  • 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 No indication. Uses NBitcoin. It is claimed that NBitcoin obtains entropy from the standard OS specific entropy sources.

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

PASS Little indication; tests at https://github.com/zkSNACKs/WalletWasabi/blob/master/WalletWasabi.Tests/

NOTE Development is very fast paced and probably could use more review before release. Recently a password length limit was instituted resulting in some users not being able to access funds in a hard fork.

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

NOTE Wasabi 1.0 was released 31 October 2018. Eligible for listing on 31 January 2019.

  • No concerning bug is found when testing the wallet

PASS No concerning bug was found while testing. A number of small bugs were found which were addressed quickly such as WalletWasabi/WalletWasabi#1091

  • Website supports HTTPS and 301 redirects HTTP requests

PASS http://wasabiwallet.io redirects to https

PASS https://wasabiwallet.io rating A+

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

PASS https://wasabiwallet.io max-age is 365 days

  • The identity of CEOs and/or developers is public

PASS https://zksnacks.com/#contact

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

PASS Receive addresses immediately disappear from the UI once a transaction is received

  • Uses deterministic ECDSA nonces (RFC 6979)

PASS Claims to use low R deterministic nonce (not tested)

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

PASS

  • User has access to private keys
  • 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

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

PASS Provides BIP39 phrase when wallet is created

  • Restoring wallet from backup is working

PASS The wallet was restored using the BIP39 phrase on Wasabi as well as Electrum.

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

PASS https://github.com/zkSNACKs/WalletWasabi

  • 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 criteria (some could become requirements):

  • Received independent security audit(s)

NOTE No audit

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

PASS Does not show "received from"

  • Provides a bug reporting policy on the website

PASS The support link on https://wasabiwallet.io links to reddit.com; app Report Bug menu item points to http://github.com/zkSNACKs/WalletWasabi/issues

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

PASS https:://wasabiwallet.io has been submitted to the preload list

  • If user has exclusive access over its private keys:
    • Supports HD wallets (BIP32)

PASS Uses BIP84 path

  • 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

PASS Uses BIP38 password protection

  • On desktop platform:
    • Encrypt the wallet by default

PASS Uses BIP38 password protection for storing the master seed

NOTE Does not encrypt address labels

  • For hardware wallets:
    • Prevents downgrading the firmware

N/A

Edit: Added sentence about privacy features not being evaluated to intro

@nopara73
Copy link
Contributor Author

nopara73 commented Jan 22, 2019

  1. Rebased.
  2. Changed transparency to checkpasstransparencyopensource

@nopara73
Copy link
Contributor Author

nopara73 commented Jan 22, 2019

Per previous comments I believe that the scoring in 1113c4c should be updated to
privacy: "checkpassprivacybasic"

@crwatkins It was not discussed previously, your linked comment was about privacydisclosure. Which I would like to reflect on also, but first I'd like to confirm with you if this was just a typo by you or you have something in mind.

FTR:

  • checkgoodprivacyimproved: "Improved privacy"
  • checkpassprivacybasic: "Basic privacy"

@crwatkins
Copy link
Contributor

@nopara73 Thanks for asking; I should have provided more background. The privacy score is an "umbrella score" that is derived by the algorithm specified here based on privacyaddressreuse, privacydisclosure, and privacynetwork.

@nopara73
Copy link
Contributor Author

Since the discussion on network level privacy ended by me pointing out that Bitcoin Core without Tor broadcasts all its transactions on the clearnet, thus ISP can figure out accurately which tx was broadcasted by the local node, regardless if we are talking about a listening node or not, I believe downgrading Wasabi's privacy scoring to be equivalent with Bread Wallet's would be misleading and a disservice to the community.

To progress the conversation forward on network level privacy, let me write a comprehensive analysis and comparison between Wasabi, Core and Bread. My intuition is that it would sufficiently highlight that Wasabi and Core is way more similar to each other than Wasabi and Bread regarding network level privacy.

Finally I'd like to note that from the next release Wasabi will autodetect any running listening full node and fetch the block from it through local P2P connection. If there is none, then it will try to fetch the block from the disk, from the blocks folder, and only if Wasabi doesn't find those sources will it move ahead and use its P2P connections for block fetching. WalletWasabi/WalletWasabi#1113

However for the analysis, I'd like to ask for sine time, since I have some deadlines coming up.

@crwatkins
Copy link
Contributor

@nopara73

To progress the conversation forward on network level privacy, let me write a comprehensive analysis and comparison between Wasabi, Core and Bread. My intuition is that it would sufficiently highlight that Wasabi and Core is way more similar to each other than Wasabi and Bread regarding network level privacy.

I think that would be very interesting read.

Perhaps some background on the scoring may help you in your position. The rest of this comment really just refers to bitcoin.org and may even be slightly off topic for this PR.

The scoring (perhaps not the best name for it; the scores are the values that are assigned to categories) criteria has been established with community consensus. I've been interpreting it for a few years in line with what I believe that the consensus intended and I have some organizational recollection of decisions and the reasoning behind those decisions. Scores are generally chosen for wallets on a case by case basis of the score that the wallet best meets or exceeds. Seldom are wallets directly compared or ranked. The scores are intended to describe an individual wallet. If new scores are needed to describe a new type of wallet, a new score would be created, but historically that has seldom (or never?) happened. I'm all for it, but only if it is a significant new score with multiple wallets qualifying, or anticipated to qualify, in the near future (one wallet does not make a score). Similarly any wallet with a score that it does not meet or exceed should be changed.

@crwatkins
Copy link
Contributor

As detailed in my review above I believe Wasabi Wallet is eligible for listing as the bug cited has been fixed and the three month release time period is now satisfied.

I still believe that the scoring should be changed to

privacy: "checkpassprivacybasic"
privacydisclosure: "checkfailprivacydisclosurespv"

but otherwise I concur with scoring in 7eff4e0.

As I've mentioned, I would support changes to the privacy scoring criteria in the future which would likely result in a different score being applied here.

I would recommend a merge with the above change.

@Cobra-Bitcoin
Copy link
Contributor

This is scheduled to be merged on Monday the 4th of February.

@Cobra-Bitcoin Cobra-Bitcoin merged commit 292f256 into bitcoin-dot-org:master Feb 4, 2019
@nopara73 nopara73 deleted the wasabi branch February 16, 2019 20:45
@nopara73
Copy link
Contributor Author

nopara73 commented Apr 3, 2019

As I promised I have done a comparision between Core's and Wasabi's network level privacy.

Also we have addressed the ISP adversary issue raised in this PR: Wasabi noew communicates over Tor on the P2P layer, too, in a way that it only connects to onion nodes, so the end-to-end encryption defeats the ISP adversary raised here. WalletWasabi/WalletWasabi#1265

Note that, for a handful of releases now, if a full node was running in the background Wasabi was (and still is) automatically acquiring the blocks from it instead of peers, so this issue was partly addressed previously, too.

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.

None yet