Walletwasabi: Hardware Wallet Integration

Created on 10 Apr 2019  路  39Comments  路  Source: zkSNACKs/WalletWasabi

Is your feature request related to a problem? Please describe.

No

Describe the solution you'd like

Work with you to add Trezor integration with Trezor.Net

Describe alternatives you've considered

Months and months of grueling hard work to create another C# Trezor library.

featurenhancement priority questioresearch

Most helpful comment

Good news @nopara73! @MelbourneDeveloper (author of Trezor.Net library) wants to help with integrating the library into the Wasabi Wallet.

All 39 comments

Good news @nopara73! @MelbourneDeveloper (author of Trezor.Net library) wants to help with integrating the library into the Wasabi Wallet.

Hope we integrate Ledger as well :)

@2pac1

https://github.com/MelbourneDeveloper/Ledger.Net

It's based on the same hardware integration as Trezor.Net

https://github.com/MelbourneDeveloper/Device.Net

Who can I talk to about integrating all the wallets?

If we could get both trezor and ledger, that would be great.

@2pac1 point me to a document about what you trying to achieve and I'll start working on something.

Also, I do https://github.com/MelbourneDeveloper/KeepKey.Net

@MelbourneDeveloper integration we hardware wallets is in the roadmap but there are a few issues that we still don't know how to address see: New Wallet Development Direction.

Basically the main issue we found is that it is not possible to integrate HWs with wasabi coinjoin feature because the HW should be able to sign a transaction without human intervention, what HWs don't do. This was discussed here and alternatives here. Personally I think the easier alternative could be to disable the Coinjoin tab when a user opens a HW.

Documentation

We don't have enough documentation about the internal design of Wasabi wallet but we are happy to give you all the info that you need by chat, mail or whatever mean is better for you. Anyway, if you want to do a PoC this is what you could do:

  • abstract the KeyManager class: the KeyManager class is the one that provides us the key pairs. What you could do is override the method that derivates and returns pubkeys with an implementation that fetches pubkeys from the a Trezor device.

@nopara73 It would be a good idea to invite @MelbourneDeveloper to slack, don't you think?

Update
Changing he KeyManager implementation is not enough I know, a next step would be to delegate the transaction signing but that could be done in a second step.

@lontivero can do. This makes sense. Please do invite me to Slack.

I can't say when I'll be able to get started but I will.

Hi @MelbourneDeveloper, I just got to this issue now. I didn't know about your libraries. This is awesome!

One thing that complicates things is that we don't use external libraries other than NBitcoin, so I wonder what's the right way to go about it.

First of all, I just created an invite link with 30 day expiry to slack: https://join.slack.com/t/tumblebit/shared_invite/enQtNjAwMjkyNzkyNTc3LTRmMzUyOGJmNjM3N2JkYWIxYWZlYjQzODU1ODYzYTQ1YjM1MDcwNDVhOTU2ODQyNmYyZmRhOTUxYTJlZDJjYjY

Now, current hardware wallet integration with disabling the CoinJoins tab is possible and I'm pretty sure is unavoidable eventually, so we could just start the work on it right now.

As I see you have Trezor.Net, Ledger.Net and KeepKey.Net, but you also have Hardwarewallets.Net, which says "The aim is to achieve uniform access to all the major wallets and all supported coins on all the major platforms."

I think we'll have enough income to finance a proper non-hacky hardware wallet integration. So there're many possibilities we could go forward, and I think the following would be the best:

  1. Let's talk about a deal for a 3 month period.
  2. Let's wait until 25th of this month when we'll see our income and do some calculations and predictions on the future and we'll be able to discuss the deal within the company with more complete information.
  3. Add the most future proof hardware wallet integration primitives to NBitcoin (@NicolasDorier, would that be fine by you?) I guess you have a lot of code duplication like transaction classes, transaction id classes and stuff like that which would make me not want to add your libraries as they are.
  4. Figure out the UX/user workflow together (UI plans.)
  5. Build the UI and the business logic in parallel.

Now, we must start with only one hardware wallet. I'd let you decide which one, I guess you'd do Trezor. This begs the question: Can you do bech32-only wallets on Trezor? Because those are the only address types Wasabi is able to monitor in a private way.

Can you do bech32-only wallets on Trezor?

The firmware supports these. So this should not be a problem

Implementing Trezor support for non-coinjoin part of the wallet would be the first step.

In the meanwhile, we'll introduce the CoinJoinSignTx message which can be used later to implement CoinJoin in Wasabi via Trezor.

Let me address some of your issues, and ideas one by one.

it is not possible to integrate HWs with wasabi coinjoin feature because the HW should be able to sign a transaction without human intervention, what HWs don't do.

That sounds about right. This feature would need to be turned off when using a hardwarewallet.

We don't have enough documentation about the internal design of Wasabi wallet but we are happy to give you all the info that you need by chat [...]
abstract the KeyManager class: the KeyManager class is the one that provides us the key pairs. What you could do is override the method that derivates and returns pubkeys with an implementation that fetches pubkeys from the a Trezor device.

This makes sense.

One thing that complicates things is that we don't use external libraries other than NBitcoin, so I wonder what's the right way to go about it.

So, you want to keep dependencies low? This is a dependency diagram of my libraries. The important thing to understand is that Device.Net holds the interfaces while Usb.Net, Hid.Net, and Device.Net.LibUsb are the transports for the wallets. Anything can be swapped in there. My hope is that whenLedger Nano X is released, there will be a readily available library that can be wrapped for Ledger Nano X support. These device libraries are independent components by themselves and a critical for stability and functionality, but other libraries can be wrapped and plugged in with DI.

My libraries have an MIT license, so there's nothing stopping you from wholesale dumping the code in to your codebase. However, that would make upgrades etc. more time consuming and error prone because of the copying and pasting aspect. I update these libraries quite frequently.
d

image

Let's talk about a deal for a 3 month period.

I wouldn't worry about funding or a deal at the moment. I'm keen to work on this project. I'm happy to make a start and demo a few approaches to you guys and then go from there. I'd need to work with someone in your team that understands what I've built and can help to get the idea off the ground.

future proof hardware wallet integration primitives to NBitcoin (@NicolasDorier

I've worked with Nicolas a little on this: https://github.com/LedgerHQ/ledger-dotnet-api. It uses an early version of Hid.Net. The plan was to get that library working on other platforms, but I never had a chance to revisit that. The plan is to revisit that. However, you will see that there has been a strong effort to standardize access across all wallets in Hardwarewallets.Net. So, I'd want to stick to using implementations of the interfaces in there so that your app is not depending too much on concretes classes in libraries. Nicolas was interested in Hardwarewallets.Net implementation and it may not be too hard so that Ledger.Net could be swappable with my libraries.

I will take a stab at Trezor first. As @prusnak pointed out bech32 should be pretty straight forward. I will ask for help in Slack. I will fork the repo and start playing around. It would be good if someone could give me a hand on my branch as I ask questions. I will try to do as little as possible as a POC. I will avoid changing UI, and just get it to a point where it does something with the wallet.

@nopara73, @2pac1, @lontivero

UX

Bear with me, because I never used a hardware wallet before. So I went through from a UX point of view and tried to identify the tasks what a HW should do.

In the wallet manager we should add a new tab: Hardware Wallet.

image

Here the software should automatically detect any hardware wallet plugged in through USB and display to the user.

Here we must tell the user somehow that we can only do bech32 keypaths.

I suppose when the user "Connect," the user has to accept the connection on the hardware wallet and Wasabi should get the extpubkey and based on the extpubkey we'd know if we'd have to create a new wallet or just load an existing one.

Then Wasabi should load a watch-only wallet that has hardware wallet.

Wasabi loads a wallet that has coinjoin disabled and the only thing that Wasabi should do differently is the Send. In the Send, after the tx is built, the hex should be sent to the hardware wallet for signing and the user should OK the signing. Finally Wasabi can propagate it.

Are my assumptions correct here?

Replies

We don't have enough documentation about the internal design of Wasabi wallet but we are happy to give you all the info that you need by chat [...]

No, sorry, I have to know and understand each and every line that's being added because of the hardware wallet integration.

Usb.Net, Hid.Net, and Device.Net.LibUsb are the transports for the wallets.

What? .NET Core is not able to work with USB without external libraries?! What the hell? I have to trust more developers I don't know to not execute malicious code in my security critical wallet software in order to make my wallet "more secure" by integrating hardware wallets?

I guess there's no good solution here. Every other wallet takes this security risk (I can imagine even more) and the lack of hardware wallet support is a systemic privacy risk for Wasabi Wallet, so it's a must. At this point it doesn't introduce much more security risk if we use your NuGet libraries or not. At least we kind of know who you are and you seemed to be trustworthy for now. So we can progress forward with integration as a "normal software company" would do.

I was trying to figure out what's the best course of action and I think @achow101's library is the most secure solution we can do, because Bitcoin Core is going to use this and is already using this to an extent. This is poised to be the reference hardware wallet library: https://github.com/bitcoin-core/HWI

So I can sleep calmly at night and be sure that a bunch of core developer have looked at the source code and some even checks the dependencies so those won't execute malicious code on my users' computer.

A C# wrapper around the command line options of the released binary seems like the best way going forward. @MelbourneDeveloper any thoughts on this approach?

@nopara73 , I've been working with hardwarewallets for over a year. I've poured countless hours in to designing, testing, and refining the libraries for all the different platforms. I can understand your apprehension with working with them, but everything I've done is open source and it is fully auditable by anyone. If you are worried about built packages, there can be some kind of mechanism for ensuring that there is a CRC check on the NuGet packages before they are added in to your code base. I.e. a guarantee that the code I give you is the same code that is compiled in to the NuGet packages.

I guess there's no good solution here.

I recommend you go over my code with a fine tooth comb, and make sure there is nothing you don't like about what I've done.

What? .NET Core is not able to work with USB without external libraries?! What the hell? I have to trust more developers I don't know to not execute malicious code in my security critical wallet software in order to make my wallet "more secure" by integrating hardware wallets?

As far as I know, out of the box, .NET Core does not come with a USB API. As far as I know, .NET and .NET Core both rely on Windows APIs to communicate with USB and Hid devices. Most of my grueling hard work has been around getting USB transports to work in the same way across all platforms.

Here is the code for that (all of which I wrote):

Windows USB Calls:
https://github.com/MelbourneDeveloper/Device.Net/blob/master/src/Usb.Net/Windows/WinUsbApiCalls.cs

Windows Hid Calls:
https://github.com/MelbourneDeveloper/Device.Net/blob/master/src/Hid.Net/Windows/HidAPICalls.cs

UWP does have an API, and this is the wrapper for Device.Net
https://github.com/MelbourneDeveloper/Device.Net/blob/master/src/Usb.Net.UWP/UWPUsbDevice.cs

This is the Android API
https://github.com/MelbourneDeveloper/Device.Net/blob/master/src/Usb.Net.Android/AndroidUsbDevice.cs

A C# wrapper around the command line options of the released binary seems like the best way going forward.

If that's the path you want to go down, I cannot stop you, but I'd have to say that this would be the worst possible solution from a security point of view, and a UX point of view.

  • This library has a huge set of dependencies. Python is its own platform with a huge codebase. It's open source, but you want to audit the entire Python codebase?

  • If the computer is installed with malware, the command line tool could have something malicious swapped in, so suddenly you're depending on an external dependency that you cannot trust.

  • The user needs to install python by them self. Even I struggle to get Python installed. Nothing deters users from using apps like installing 3rd party software. What if you tell them to install python, and then some other website gives them a tutorial which directs them to an infected Python installer?

  • Calling external apps simply shifts the problem of trusting code off to a different place where you have no control over it.

at least we kind of know who you are and you seemed to be trustworthy for now.

I'm not asking anyone to trust me. My code is right there for you to audit. You can disassembly the IL of my libraries with DNSpy. It's not like there's some magical way I can hide malicious code in an assembly. The code is not obfuscated at the IL level.

@nopara73

Here the software should automatically detect any hardware wallet plugged in through USB and display to the user.

Here we must tell the user somehow that we can only do bech32 keypaths.

I suppose when the user "Connect," the user has to accept the connection on the hardware wallet and Wasabi should get the extpubkey and based on the extpubkey we'd know if we'd have to create a new wallet or just load an existing one.

Then Wasabi should load a watch-only wallet that has hardware wallet.

This all sounds fairly straight forward and doable.

  • If the computer is installed with malware, the command line tool could have something malicious swapped in, so suddenly you're depending on an external dependency that you cannot trust.

A simple hardcoded hash can be used to check that the binary being executed is the one that is expected.

  • The user needs to install python by them self. Even I struggle to get Python installed. Nothing deters users from using apps like installing 3rd party software. What if you tell them to install python, and then some other website gives them a tutorial which directs them to an infected Python installer?

The HWI binaries are completely self contained. It uses pyinstaller which packages together python and all of the binaries into a self contained binary, so there is no need for users to install python or any packages.

  • This library has a huge set of dependencies. Python is its own platform with a huge codebase. It's open source, but you want to audit the entire Python codebase?

Not really. The only dependencies are the vendor device libraries and some of their dependencies. Those vendor device libraries are actually stripped down versions of those published on their github's which significantly reduces the number of dependencies required.


HWI entirely uses BIP 174 PSBTs for passing all of the information needed for signing, so Wasabi would need to support BIP 174 in order to use it.

Also, there are a few usability caveats. Not all devices support all of the same things so some things (such as multisig) may not be available for all devices. Also, HWI currently completely disables change detection which means that devices will show change outputs which may be confusing to users.

@nopara73, @achow101, @prusnak, @2pac1

You guys work out the path forward you want to take. Personally, I wouldn't touch an app that relies on python with a ten foot pole, but that's your choice. If/when you want hardwarewallet integration in C#, let me know.

@MelbourneDeveloper First of all I'd like to apologize for my previous tantrum, I couldn't imagine the hardware wallet rabbit hole is this deep and I have been working very very hard to remove as many dependencies as possible, to reduce the complexity and to increase the maintainability of the software. I've been constantly kicking @danwalmsley's ass to try to remove more UI dependencies and his stupid submodules xD, and I've been vigorously refusing all of @lontivero's Python script PR attempts and trying hard to change @molnard's "This library will fix our issue mentality" and I felt like my hard work is going to waste because of the HW deps.

I will play around with your and @achow101's libraries in the coming days and come back here with our decision.

I wouldn't touch an app that relies on python with a ten foot pole

lol, I've been a Python dev before and I'd agree under any other circumstances, but considering that HWI will become the way Core handles hardware wallets I assume the project has high standards. Anyway we'll see, I start digging and playing with the alternatives right now and get back to this thread in a couple of days.

I can confirm this and he is still doing that. :-D

@nopara73 If you don't want to use Trezor.NET library, there is another option - Trezor provides Trezor bridge, which is a simple HTTP server daemon running on localhost - it provides a bridge which eats HTTP requests and communicates with the device via USB., turning the USB responses into HTTP responses.

I don't think PSBT can be used for CoinJoin right now unless there is the proof-of-not-ownership (like we discussed in the hotel lobby) added into the spec.

@nopara73 thanks! I completely understand the desire to remove complexity and the fear of having to rely on other people's code. Whenever I open up a new project, the first thing I do is check to see if there are any NuGet packages or assemblies that can be removed.

Believe me, I didn't intend on having so many assemblies. The only reason they are like that is so that you can deploy the bare minimum with your app. You don't want to deploy UWP Hid code on Android for example so the code is physically divided.

I'm not criticising python. Python is a useful tool. But it's not fit for for purpose when you're building a .NET app. It's a gigantic dependency that is not simple to install or get working. I tried and failed when I first go the Trezor.

@prusnak using the bridge is always an option. But even then, you'd still probably want to consider using Trezor.Net with a custom transport that talks to the bridge. This is why I've built the libraries in layers - so that the transport can be directly swapped in. At the same time - there's no need for the bridge. The whole point of my libraries is to provide seamless integration without relying on any external components. Hardwarewallets.Net puts a layer over all hardwarewallets so that there are common interfaces for simple tasks like getting addresses. This is a huge undertaking, and still a work in progress. It would not be trivial to do replicate it.

Anyway, the point is, I'm happy to help if you decide that C# is the way to go. I don't know anything about Bitcoin core using python , but that sounds like a bad move to me. It means that Bitcoin would no longer be language independent. That's a big step in the wrong direction in my opinion, but I also don't know the full background and there may be a good reason for it.

@nopara73 lastly, it's worth pointing out the journey I had with building Hardfolio. I originally intended on building a simple portfolio app but I realised that hardwarewallet integration is the future. I didn't want to use WebUsb and I tried to get the Trezor Python stuff working but I quickly realised that there was nothing in the space for seamless c# integration. That's when I started building.

The other thing I was shocked by was the lack of cross platform Hid and USB APIS in C#. That's why I ended taking 6 months plus detour from Hardfolio to work on the stability of the transport libraries.

I've been down the same path you're coming to realise now. My app Hardfolio has suffered because I naively thought that integration would be straight forward. I wish that someone had already built this infrastructure when I started. Hardfolio would be finished by now.

@nopara73 are you intending on putting wasabi in the app stores? My libraries have gone through Google Play and Microsoft Store. This was another reason that python was not an option for Hardfolio.

I don't know anything about Bitcoin core using python , but that sounds like a bad move to me. It means that Bitcoin would no longer be language independent. That's a big step in the wrong direction in my opinion, but I also don't know the full background and there may be a good reason for it.

Bitcoin Core itself isn't using Python. The HWI project is in Python simply because vendors have provided libraries in Python (which means that risks are significantly reduced as protocols don't need to be reimplemented). Bitcoin Core will be executing HWI as a separate process; it will not be containing python itself. Please don't make comments like this when you have no knowledge of the subject.

@achow101 exactly my point. Bitcoin Core would be executing HWI as a seperate process which means that it would be dependent on something other than its original language. It sounds like a bad idea to me but there might be a valid reason for it.

@achow101 that is not a statement about HWI or python at all. I don't know anything about that project. I'm just saying that if Bitcoin Core requires dependencies on external systems, the architecture should be loosely coupled so that other pieces can be swapped in. That's system design 101.

'm just saying that if Bitcoin Core requires dependencies on external systems, the architecture should be loosely coupled so that other pieces can be swapped in. That's system design 101.

And it will be. But that is off topic for this discussion.


Obviously you are here to advocate your solution, as am I to advocate for mine. But if you are going to list disadvantages of other solutions, then you should do some research into what those solutions are actually doing and how they work. So far, all you have said about HWI and Bitcoin Core are categorically false (and that is primarily why I am responding - to stop the spread of misinformation). I suggest that you take some time to do your own research and experimentation as @nopara73 is doing and actually see for yourself how HWI works and think about how the integration of either project into Wasabi would work.

@achow101 calm down. I'm not here to advocate anything. I'm here offering help with my libraries if that's the path people want to go down. I am not disparaging HWI in any way. I don't know anything about it.

The only thing I'm saying is that in general, it is usually a bad idea to drop out to external processes. That said, there are sometimes good reasons for it. I can't comment on this because I don't know what the reasons are for dropping out to an external process.

@nopara73 I apologize for turning this in to a debate. I didn't mean to do that. The choice is yours and I'll stay out of this until you need more information.

@nopara73 fair call. If you want to revisit this down the track, feel free to contact me.

Ping @MelbourneDeveloper I think it is worth considering we create a HW repo in .NET as HWI which rely on PSBT. Now NBitcoin support it perfectly.

@nicolasdorier I'm happy to help with any projects.

PSBT is @achow101's BIP right?

Do you mean .NET integration with the HWI Python library to leverage PSBT?

I mean a replacement of HWI coded as a .NET library, with exactly same design and API.

I've been working pretty hard to put layer over all the hardwarewallets @NicolasDorier . What is special about HWI's design and API that needs to be replaced?

@MelbourneDeveloper I have not take too much time reviewing your library. Maybe the response is that I should deprecate my ledger wallet API and advise people to use yours.

The only thing needed should be Sign(PSBT), which is quite easy.
I was under the impression HWI support more hardware.

What is the project with the abstraction layer you did over all hardware? I thought you only did Trezor.

@NicolasDorier , I also do https://github.com/MelbourneDeveloper/KeepKey.Net and https://github.com/MelbourneDeveloper/Ledger.Net . In the end, the reason I created Ledger.Net is that I needed the library to work inside my app and I could never get Ledger .NET API to work properly on Android (mostly because of the reasons I am talking about in the other thread). If you are interested in merging the Ledger libraries, I am definitely interested in talking about how to do that.

The abstraction layer is here: https://github.com/MelbourneDeveloper/Hardwarewallets.Net . It's still a work in progress. It is mostly focused on getting addresses across multiple different coin types. Signing transactions is very difficult because it's different from wallet to wallet. I still hope to keep working on it when I have time.

Yes, I believe HWI supports more wallets, but I honestly don't know anything about that library, and as far as I know it can only be access via the command line because it is a python library. You would need to talk to Andrew Chow about that.

Things would be much easier if .NET projects used libraries by @MelbourneDeveloper.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

MaxHillebrand picture MaxHillebrand  路  3Comments

kenny47 picture kenny47  路  3Comments

2pac1 picture 2pac1  路  3Comments

yahiheb picture yahiheb  路  3Comments

UkolovaOlga picture UkolovaOlga  路  3Comments