Significantly enhance hardware key robustness by droidmonkey · Pull Request #4584 · keepassxreboot/keepassxc

droidmonkey · 2020-04-11T17:09:56Z

Type of change

✅ Bug fix (non-breaking change which fixes an issue)
✅ Refactor (significant modification to existing code)

Description and Context

Significantly improve user experience when using hardware keys on databases in both GUI and CLI modes. Prevent locking up the YubiKey USB interface for prolonged periods of time. Allows for other apps to use the key concurrently with KeePassXC.
Improve messages displayed to user when finding keys and when user interaction is required. Output specific error messages when handling hardware keys during database read/write.
Only poll for keys when previously used or upon user request. Prevent continuously polling keys when accessing the UI such as switching tabs and minimize/maximize.
Add support for using multiple hardware keys simultaneously. Keys are identified by their serial number which prevents using the wrong key during open and save operations.
Fixes Unlocking database with Onlykey is a hit and miss #4400
Fixes Yubikey is not detected if already plugged #4065
Fixes Add ability to use more than one connected Yubikey #1050
Fixes Yubikey remains "open" after use preventing other applications to use it #1215
Fixes Yubikey is listed twice in combobox #3087
Fixes Global Auto-type when minimized to tray in Windows + Yubikey: no prompt to touch the key (appears stalled) #1088
Fixes Prompt to insert yubikey if missing on autoreload #1869

Screenshots

Coming soon

Testing strategy

Updated YubiKey test and CLI test.

Checklist:

✅ I have read the CONTRIBUTING document. [REQUIRED]
✅ My code follows the code style of this project. [REQUIRED]
✅ All new and existing tests passed. [REQUIRED]
✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]
✅ My change requires a change to the documentation, and I have updated it accordingly.
✅ I have added tests to cover my changes.

droidmonkey · 2020-04-12T17:13:10Z

This is ready for review!

src/cli/Command.cpp

src/gui/DatabaseOpenWidget.cpp

src/keys/drivers/YubiKey.cpp

tests/TestYkChallengeResponseKey.cpp

droidmonkey · 2020-04-23T20:12:05Z

I made the requested changes. I don't want to include the PasswordKey change in this PR, I think we need to do that on a more global refactor. Also, tests now pass using a NEO 😉

phoerious · 2020-04-27T20:05:11Z

Still failing for me:

FAIL!  : TestYubiKeyChallengeResponse::testDetectDevices() 'displayName.contains("Challenge Response - Slot")' returned
FALSE. ()
D:\Documents\KeePassXC\keepassxc\tests\TestYkChallengeResponseKey.cpp(52) : failure location
SKIP   : TestYubiKeyChallengeResponse::testKeyChallenge() No YubiKey contains a slot in passive mode.
D:\Documents\KeePassXC\keepassxc\tests\TestYkChallengeResponseKey.cpp(79) : failure location****

It's also still showing both slots as configured.

droidmonkey · 2020-05-11T01:14:24Z

Tests fixed and rebased

* Significantly improve user experience when using hardware keys on databases in both GUI and CLI modes. Prevent locking up the YubiKey USB interface for prolonged periods of time. Allows for other apps to use the key concurrently with KeePassXC. * Improve messages displayed to user when finding keys and when user interaction is required. Output specific error messages when handling hardware keys during database read/write. * Only poll for keys when previously used or upon user request. Prevent continuously polling keys when accessing the UI such as switching tabs and minimize/maximize. * Add support for using multiple hardware keys simultaneously. Keys are identified by their serial number which prevents using the wrong key during open and save operations. * Fixes #4400 * Fixes #4065 * Fixes #1050 * Fixes #1215 * Fixes #3087 * Fixes #1088 * Fixes #1869

onlykey · 2020-06-09T13:07:43Z

Has anyone had any issues with Yubikey since this change? Asking because we have a user saying that OnlyKey works fine with this but now they are having issue with Yubikey on Linux - trustcrypto/OnlyKey-Firmware#98 (comment)

droidmonkey · 2020-06-09T18:25:20Z

I tested extensively with both OK and YK keys. I have a YK Neo and YK 4 and both work flawlessly. Perhaps this is a Linux problem? If someone else can replicate I will be concerned.

Added - Custom Light and Dark themes [#4110, #4769, #4791, #4796, #4892, #4915] - Compact mode to use classic Group and Entry line height [#4910] - View menu to quickly switch themes, compact mode, and toggle UI elements [#4910] - Search for groups and scope search to matched groups [#4705] - Save Database Backup feature [#4550] - Sort entries by "natural order" and move lines up/down [#4357] - Option to launch KeePassXC on system startup/login [#4675] - Caps Lock warning on password input fields [#3646] - Add "Size" column to entry view [#4588] - Browser-like tab experience using Ctrl+[Num] (Alt+[Num] on Linux) [#4063, #4305] - Password Generator: Define additional characters to choose from [#3876] - Reports: Database password health check (offline) [#3993] - Reports: HIBP online service to check for breached passwords [#4438] - Auto-Type: DateTime placeholders [#4409] - Browser: Show group name in results sent to browser extension [#4111] - Browser: Ability to define a custom browser location (macOS and Linux only) [#4148] - Browser: Ability to change root group UUID and inline edit connection ID [#4315, #4591] - CLI: `db-info` command [#4231] - CLI: Use wl-clipboard if xclip is not available (Linux) [#4323] - CLI: Incorporate xclip into snap builds [#4697] - SSH Agent: Key file path env substitution, SSH_AUTH_SOCK override, and connection test [#3769, #3801, #4545] - SSH Agent: Context menu actions to add/remove keys [#4290] Changed - Complete replacement of default database icons [#4699] - Complete replacement of application icons [#4066, #4161, #4203, #4411] - Complete rewrite of documentation and manpages using Asciidoctor [#4937] - Complete refactor of config files; separate between local and roaming [#4665] - Complete refactor of browser integration and proxy code [#4680] - Complete refactor of hardware key integration (YubiKey and OnlyKey) [#4584, #4843] - Significantly improve performance when saving and opening databases [#4309, #4833] - Remove read-only detection for database files [#4508] - Overhaul of password fields and password generator [#4367] - Replace instances of "Master Key" with "Database Credentials" [#4929] - Change settings checkboxes to positive phrasing for consistency [#4715] - Improve UX of using entry actions (focus fix) [#3893] - Set expiration time to Now when enabling entry expiration [#4406] - Always show "New Entry" in context menu [#4617] - Issue warning before adding large attachments [#4651] - Improve importing OPVault [#4630] - Improve AutoOpen capability [#3901, #4752] - Check for updates every 7 days even while still running [#4752] - Improve Windows installer UI/UX [#4675] - Improve config file handling of portable distribution [#4131, #4752] - macOS: Hide dock icon when application is hidden to tray [#4782] - Browser: Use unlock dialog to improve UX of opening a locked database [#3698] - Browser: Improve database and entry settings experience [#4392, #4591] - Browser: Improve confirm access dialog [#2143, #4660] - KeeShare: Improve monitoring file changes of shares [#4720] - CLI: Rename `create` command to `db-create` [#4231] - CLI: Cleanup `db-create` options (`--set-key-file` and `--set-password`) [#4313] - CLI: Use stderr for help text and password prompts [#4086, #4623] - FdoSecrets: Display existing secret service process [#4128] Fixed - Fix changing focus around the main window using tab key [#4641] - Fix search field clearing while still using the application [#4368] - Improve search help widget displaying on macOS and Linux [#4236] - Return keyboard focus after editing an entry [#4287] - Reset database path after failed "Save As" [#4526] - Use SHA256 Digest for Windows code signing [#4129] - Improve handling of ccache when building [#4104, #4335] - macOS: Properly re-hide application window after browser integration and Auto-Type usage [#4909] - Auto-Type: Fix crash when performing on new entry [#4132] - Browser: Send legacy HTTP settings to recycle bin [#4589] - Browser: Fix merging browser keys [#4685] - CLI: Fix encoding when exporting database [#3921] - SSH Agent: Improve reliability and underlying code [#3833, #4256, #4549, #4595] - FdoSecrets: Fix crash when editing settings before service is enabled [#4332]

droidmonkey added high priority 🚨 feature: Hardware Keys pr: refactoring Pull request refactors code labels Apr 11, 2020

droidmonkey added this to the v2.6.0 milestone Apr 11, 2020

droidmonkey requested a review from phoerious April 11, 2020 17:09

droidmonkey added the PR: Needs Work label Apr 11, 2020

droidmonkey force-pushed the fix/robust-hardware-key branch from 4e10b73 to 901cc18 Compare April 12, 2020 17:12

droidmonkey removed the PR: Needs Work label Apr 12, 2020

droidmonkey force-pushed the fix/robust-hardware-key branch 4 times, most recently from a430c30 to 048386c Compare April 14, 2020 11:48

phoerious requested changes Apr 14, 2020

View reviewed changes

onlykey mentioned this pull request Apr 14, 2020

HMAC Challenge Response Leads to Unresponsive OnlyKey trustcrypto/OnlyKey-Firmware#98

Closed

droidmonkey force-pushed the fix/robust-hardware-key branch from 048386c to 6ea7b9a Compare April 23, 2020 20:11

droidmonkey force-pushed the fix/robust-hardware-key branch 2 times, most recently from 0d63edd to 2eb94da Compare April 23, 2020 20:22

droidmonkey requested a review from phoerious April 27, 2020 18:54

droidmonkey force-pushed the fix/robust-hardware-key branch from 2eb94da to 6d5318d Compare May 11, 2020 01:14

droidmonkey force-pushed the fix/robust-hardware-key branch from 6d5318d to 64367d2 Compare May 11, 2020 01:31

droidmonkey force-pushed the fix/robust-hardware-key branch from 64367d2 to 27b4d06 Compare May 14, 2020 21:50

phoerious approved these changes May 14, 2020

View reviewed changes

droidmonkey merged commit 5142981 into develop May 15, 2020

droidmonkey deleted the fix/robust-hardware-key branch May 15, 2020 00:20

bugzy mentioned this pull request Jul 22, 2020

Add support for hmac-secret FIDO2 extension #3560

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Significantly enhance hardware key robustness#4584

Significantly enhance hardware key robustness#4584
droidmonkey merged 1 commit intodevelopfrom
fix/robust-hardware-key

droidmonkey commented Apr 11, 2020 •

edited

Loading

Uh oh!

droidmonkey commented Apr 12, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

droidmonkey commented Apr 23, 2020

Uh oh!

phoerious commented Apr 27, 2020

Uh oh!

droidmonkey commented May 11, 2020

Uh oh!

onlykey commented Jun 9, 2020

Uh oh!

droidmonkey commented Jun 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

droidmonkey commented Apr 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Description and Context

Screenshots

Testing strategy

Checklist:

Uh oh!

droidmonkey commented Apr 12, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

droidmonkey commented Apr 23, 2020

Uh oh!

phoerious commented Apr 27, 2020

Uh oh!

droidmonkey commented May 11, 2020

Uh oh!

onlykey commented Jun 9, 2020

Uh oh!

droidmonkey commented Jun 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

droidmonkey commented Apr 11, 2020 •

edited

Loading