Tapsigner

[scgbckbone]

• add Tapsigner to HWI
• add Tapsigner simulator
• testing
• adjust README.md in tests

[prusnak]

Please rebase on top of current master to remove the merge commit.

[scgbckbone]

Please rebase on top of current master to remove the merge commit.

will do squash and rebase after this is reviewed and ready to go - I assume this will need some more commits (build is failing + implementation of review comments)

[scgbckbone]

Few notes here. We use password field for CVC, which is something like pin code but you have to provide it for every auth command. Therefore it is not suitable for us to use PIN (with sendpin/promptpin) as PIN does not unlock the card for some amount of time as with Coldcard or Trezor, instead we would need to use sendpin before every command which is not very practical to do.

• added togglepassphrase command which is interactive way how to change CVC. (keep in mind passphrase is CVC for Tapsigner and there is no way how to add bip39 passphrase to the card so I think it is ok)
• added restore command which will not restore the card as it is not possible, but is the way how to decrypt backup file with card a dump xprv to console (unsafe but I have a warning there)
• I have tested this with core v23.0 with all supported script types, you can check here and here (signer is tapsigner simulator)
• with core you need to specify signer with CVC(password) on command line like this -signer='./hwi.py -p 123456' which definitely has some security implications for the user (but you still need to tap with the physical card).
• I have squashed and rebased on master as @prusnak proposed

Any review is greatly appreciated.

[achow101]

Please rebase now that CI is fixed.

[scgbckbone]

rebased on current master

[scgbckbone]

coinkite/coinkite-tap-proto#24 merged

[scgbckbone]

updated poetry.lock with poetry update && poetry lock after addition of coinkite-tap-protocol --> without this update installing via poetry would produce solver error

[achow101]

Also, rebase for CI to be not as dead.

[scgbckbone]

@achow101 rebased and fixed - will squash those commits after - I think it is easier for you to review changes unsquashed

[scgbckbone]

coldcard-multisig.patch was outdated - fixed in 38be6be

[scgbckbone]

52aa685f95b01a9480c0fb07acb7e433dc98ef2a Moved Tapsigner and its dependencies to extras. This aims to partly alleviate your concerns with regards to sdist and wheel distributions. In sdist and wheel, only references to dependencies are stored. With normal pip install ... it won't install Tapsigner and its dependencies - so user has to specify .[tapsigner] or -E tpsigner to actually install/use it.

In this revision, however I still kept Tapsigner in binary distributions as I was not sure how "bit concerned" you are. I can remove Tapsigner from build_{bin,wine}.sh meaning that it would only ship by wheel and sdist not binaries. Or make separate binaries with tapsigner support? not sure what is your opinion on this @achow101 ?

[scgbckbone]

52aa685f95b01a9480c0fb07acb7e433dc98ef2a Moved Tapsigner and its dependencies to extras. This aims to partly alleviate your concerns with regards to sdist and wheel distributions. In sdist and wheel, only references to dependencies are stored. With normal pip install ... it won't install Tapsigner and its dependencies - so user has to specify .[tapsigner] or -E tpsigner to actually install/use it.

In this revision, however I still kept Tapsigner in binary distributions as I was not sure how "bit concerned" you are. I can remove Tapsigner from build_{bin,wine}.sh meaning that it would only ship by wheel and sdist not binaries. Or make separate binaries with tapsigner support? not sure what is your opinion on this @achow101 ?

@prusnak @Sjors any opinions on this ?

[Sjors]

I don't understand Python dependencies that well. But if a lot a extra stuff is needed for NFC, can we (initially) have the user install those seperately and not fail if they're missing?

[scgbckbone]

I don't understand Python dependencies that well. But if a lot a extra stuff is needed for NFC, can we (initially) have the user install those separately and not fail if they're missing?

This is already done with regards to python dependencies (tapsigner dependencies are part of extras). Namely coinkite-tap-protocol and pyscard. Not installed by default.

My question is whether you would accept additional dependencies (swig libpcsclite-dev) in binaries. Currently these two additional dependencies are needed for tapsigner to work. Other option is to not provide tapsigner support in binaries at all. User would need to install swig and libpcsclite-dev on their own and can only use sdist and wheel distributions with tapsigner explicitly specified as extra during installation (see above)

[Sjors]

I hadn't thought about binaries. At some point I think hardware wallet companies should make their own binaries that are compatible with the HWI interface. However, that's not very practical right now, as e.g. Bitcoin Core won't let you configure a different -externalsigner per wallet.

User would need to install swig and libpcsclite-dev on their own

This might be the best option for now. Using NFC in a desktop environment, which afaik is where HWI is mostly used, is still a bit niche (though I plan to try it).

We could also ship separate binaries that include NFC support, if those are not too painful to generate automatically.

Another option might be to include the dependenceis, but leave the functionality disabled until the user calls hwi --enable-nfc, but that's not really easier for the end user than the above options.

[scgbckbone]

Fair points. I will remove NFC dependencies from binaries and therefore tapsigner support will be purely optional and only available via sdist and wheel. Need to make tests work somehow. Thanks for your input @Sjors

[scgbckbone]

@Sjors I use HID Omnikey 5022 CL (if you want to play with coinkite NFC cards on desktop)

[Sjors]

I ordered the ACR122U because a guy on Reddit said so.

[scgbckbone]

his reditt post is for libnfc which isn't what is used to talk to the reader (sclite) ... so doesn't apply

Seems like the one you choose coinkite lists as "not recommended" here https://github.com/coinkite/coinkite-tap-proto/blob/master/README.md#requirements

Check coinkite store https://store.coinkite.com/store/category/accessories correct readers are available and there is also 25% discount today

[Sjors]

Oh man, why are there multiple NFC libraries for Linux?

[prusnak]

Check coinkite store

"IMPORTANT: Incompatible with Mk4 NFC features! Works only with NFC cards."

Wow, so Coldcard and Tapsigner use incompatible NFC standards?

[scgbckbone]

Tapsigner is now completely optional and not included in binary distributions. Tests are handled to not fail if extras are not installed.

[achow101]

I've spent a while thinking on this and I've come to the conclusion that NFC cards don't really fit into HWI's expected workflow. With USB devices,there is a persistent connection between the device and the computer by virtue of having to plug it in. However with NFC, the connection is really ephemeral with the user expecting to be prompted to tap the card to perform an action. Using a NFC card with HWI would require having to but the card on a reader as if it were plugged in, but this is not how NFC is generally used. HWI is often used in a way where the type of device being used is unknown at the time of command invocation. Users either call enumerate to figure that out before issuing the command, or they use the fingerprint and let HWI do it automatically. Both of these require that the device is connected prior to the command being invoked. With NFC cards, this would require the caller to prompt the user to place their card on the reader and to not move it for some time. I don't think users expect to need to do that, nor for callers to know the type of device in order to make such a prompt.

Additionally, I question whether NFC cards would be used with HWI. HWI is a software targeted at desktop and laptop computers. Unless the user purchases a NFC reader, desktops and laptops are unlikely to be able to even interface with the card. ISTM the vast majority of users will only be able to use NFC from their phones, but HWI isn't expected to work on those devices. So I'm not sure that including Tapsigner support is worth the additional maintenance burden it will incur.

tl;dr I don't think Tapsigner is a good fit for HWI.

[nvk]

Check coinkite store

"IMPORTANT: Incompatible with Mk4 NFC features! Works only with NFC cards."

Wow, so Coldcard and Tapsigner use incompatible NFC standards?

Yes, was unavoidable. The hardware capabilities and the software stack are too far apart.