Skip to content

Add optional HMAC challenge-response for wallet encryption#1967

Draft
kwsantiago wants to merge 16 commits intosparrowwallet:masterfrom
privkeyio:yubikey-hmac-challenge-response
Draft

Add optional HMAC challenge-response for wallet encryption#1967
kwsantiago wants to merge 16 commits intosparrowwallet:masterfrom
privkeyio:yubikey-hmac-challenge-response

Conversation

@kwsantiago
Copy link
Copy Markdown

@kwsantiago kwsantiago commented Mar 11, 2026

Summary

Adds optional HMAC-SHA1 challenge-response as a second factor for wallet encryption, solving #1896.

  • When enabled, the wallet encryption key is derived from SHA256(argon2DerivedKey || HMAC_SHA1(argon2Salt))
  • Supports both password+challenge-response and challenge-response-only (no password) modes
  • Currently supports YubiKey (slot 2) and any compatible device (e.g. OnlyKey) via raw USB control transfers (usb4java/libusb), same protocol as KeePassXC
  • UI is vendor-neutral — "Require challenge-response for unlock", "Touch your security key"
  • New SPRW2 wallet header with flags byte to indicate challenge-response is enabled
  • Old Sparrow versions fail gracefully on SPRW2 wallets

Dependencies

Changes

PersistenceSPRW2 header format with 1-byte flags field (FLAG_CHALLENGE_RESPONSE), DbPersistence reads/writes CR flag, wraps key deriver when provider is set. The flag is needed so Sparrow knows at load time to set up the provider and allow empty passwords.

UI — "Require challenge-response for unlock" checkbox on password set/change dialogs, "Touch your security key" prompt during authentication

Call sites — All wallet unlock/decrypt dialogs detect CR-enabled wallets and set the provider

Test plan

  • Create wallet with password only — works as before (SPRW1 header)
  • Enable challenge-response on wallet — SPRW2 header written, requires both factors
  • Open with correct password + device touch — success
  • Enable challenge-response with no password — works with device-only
  • Open challenge-response-only wallet — allows empty password, requires touch
  • Change password on CR-enabled wallet — verifies with device, re-encrypts
  • Device not connected — shows "No compatible security key found"
  • Touch timeout — detects internal device timeout, cleans up USB interface

@kwsantiago kwsantiago force-pushed the yubikey-hmac-challenge-response branch from 592817f to e7b6fdc Compare March 11, 2026 15:50
@nzb-tuxxx
Copy link
Copy Markdown

I build a custom version sparrow-wallet-yubikey-hmac-challenge-response-git-0.r2286.gdfdca95-1-x86_64.pkg.tar.zst for arch based on this branch for testing, but it crashes at the point where checkbox is set for yubikey in the password dialog:

2026-03-11 19:23:11,975 ERROR Exception in thread "JavaFX Application Thread"
java.lang.reflect.InaccessibleObjectException: Unable to make field final int[] java.math.BigInteger.mag accessible: module java.base does not "opens java.math" to module com.sparrowwallet.drongo

Probably a missing dependency within Gradle-Setup? Could you fix that or did I do something wrong on my side?

Also I noticed that the password set dialog changes unintentionally between "Set yubikey" and "set password" if you first checkbox "Require yubikey for unlock" and then enter a password afterwards.

@kwsantiago
Copy link
Copy Markdown
Author

@nzb-tuxxx Thank you for testing and reporting! The crash was a missing --add-opens in Sparrow's Gradle config, since drongo needs reflective access to BigInteger for zeroing key material. Nothing wrong on your side. The button text bug is fixed too. Both are on the branch if you want to rebuild.

@nzb-tuxxx
Copy link
Copy Markdown

Thanks, I got it working and successfully managed a YubiKey encrypt / decrypt for a testing wallet. What I noticed:

  1. Change password does not work anymore. It only asks for current password but does not accept it (The password was incorrect). IMO there should be 3 ways for changing: No password, YubiKey only, YubiKey + new password. What about new YubiKey: probably unsupported?
  2. If YubiKey is not connected while unlock, error message is com.sparrowwallet.drongo.crypto.KeyCrypterException: Challenge-response key derivation failed - if it is possible to detect that there is no yubikey connected, can we improve the message accordingly ("No YubiKey found, plug-in YubiKey and try again")?
  3. If YubiKey is not pressed within a time frame of 15 seconds, some kind of timeout occurs. Is this intentionally? The YubiKey stops blinking and does not recognize touch afterwards, while the "Touch your YubiKey" Dialog is still displayed. On console I see Caused by: com.sparrowwallet.drongo.crypto.ChallengeResponseException: Failed to claim YubiKey interface: LIBUSB_ERROR_BUSY - maybe this is due to a broken USB Virtualization in VirtualBox. I need to restart Sparrow then in order to try again, all retries without sparrow restart fail

@kwsantiago
Copy link
Copy Markdown
Author

kwsantiago commented Mar 11, 2026

@nzb-tuxxx Thank you for the thorough testing! All three are fixed:

  1. Change password: the verify step was accidentally disabling the YubiKey provider before key derivation, so it always mismatched. Fixed.
  2. Error messages: the specific YubiKey errors (e.g. "No YubiKey found") were getting swallowed by a generic wrapper. Now they pass through to the dialog.
  3. Timeout/LIBUSB_ERROR_BUSY: the pending YubiKey operation wasn't being cancelled on timeout, leaving the USB interface locked. Fixed, though VirtualBox USB passthrough may still add its own quirks.

Please let me know if you still see these or other issues.

@nzb-tuxxx
Copy link
Copy Markdown

nzb-tuxxx commented Mar 12, 2026

Thanks, 1 and 2 looking good. But 3 is still unchanged. Need to restart sparrow to be able to use yubikey again after 15 seconds timeout. I'll test this on real hardware, just to be sure.


Edit: Also happens on real hardware

@craigraw
Copy link
Copy Markdown
Collaborator

Thanks for this. I think this would be a good addition to Sparrow in principle, but there are significant code changes which need careful review - I haven't attempted this yet.

There is however a problem which needs addressing. Currently, there is a UI change to add a checkbox with the text Require YubiKey for unlock. This ties the Sparrow UI to a specific vendor, which I dislike on principle. But further, how are other vendors then accommodated? For example, both Trezor (FIDO2) and Ledger (U2F) support challenge/response authentication, and are probably going to end up being used more than the Yubikey. Until this is addressed I think the PRs should be marked as draft.

A further question: what value is FLAG_CHALLENGE_RESPONSE adding?

@nzb-tuxxx
Copy link
Copy Markdown

This ties the Sparrow UI to a specific vendor, which I dislike on principle

Agree, besides the mentioned hardware wallets, there is also OnlyKey which supports the same protocol: https://docs.onlykey.io/usersguide.html#using-onlykey-with-a-software-password-manager - Maybe just name it 2FA (Challenge-Response) with a question-mark that links to sparrow wallet faq for an entry that should be created in the future, before merging, that explains the whole process - maybe https://sparrowwallet.com/docs/faq.html#how-do-i-use-2fa-for-wallet-encryption)?

@kwsantiago kwsantiago marked this pull request as draft March 12, 2026 12:09
@kwsantiago kwsantiago changed the title Add optional YubiKey HMAC challenge-response for wallet encryption Add optional HMAC challenge-response for wallet encryption Mar 12, 2026
@kwsantiago
Copy link
Copy Markdown
Author

@craigraw Agreed and addressed, all user-facing strings now say "challenge-response" / "security key" instead of "YubiKey". FLAG_CHALLENGE_RESPONSE is needed in the header so Sparrow knows at load time to set up the provider and allow empty passwords, otherwise key derivation silently produces the wrong key. Marked as draft until further notice.

@nzb-tuxxx on issue 3, this should be fixed now. The poll loop wasn't detecting the YubiKey's internal ~15s timeout, so it held the USB interface locked. Now detects it immediately and cleans up. Should be retryable without restart. Can you try it again?

Agreed regarding OnlyKey, they use the same HMAC-SHA1 protocol, and the ChallengeResponseProvider interface makes it straightforward to add support for other devices.

@nzb-tuxxx
Copy link
Copy Markdown

With 5accef3 every touch responses with Security key timed out.. . Not sure if i memorized my test password wrong or if this build is broken

@kwsantiago
Copy link
Copy Markdown
Author

@nzb-tuxxx Bug in the timeout detection. After a successful touch, RESP_TIMEOUT_WAIT_FLAG clears at the same time RESP_PENDING_FLAG sets, but the code was checking for timeout before checking for a response. Fixed in de070b1, should work now.

@nzb-tuxxx
Copy link
Copy Markdown

Timeout is working now as expected, thanks for your effort @kwsantiago

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants