feat!: Rewrite FIDO2 device interactions to be non-concurrent#37181
feat!: Rewrite FIDO2 device interactions to be non-concurrent#37181codingllama merged 18 commits intomasterfrom
Conversation
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
|
May be easier to review commit-by-commit. The big rewrite is at 9a1dcc8, all tests are green starting from d3cc5e4. Apart from the re-write most commits are intended to be small and somewhat self-contained. |
|
Manually tested MFA and passwordless logins/registrations, as well as many sequential interactions. The occasional "tx error" pops up, but I don't know that I can prevent that. Overall seems much more solid compared to the older version; hopefully the code is a bit more streamlined too. |
|
Friendly ping @rosstimothy @GavinFrazar @hugoShaka? It would be nice if I could manage to land this one for 15.0. |
|
PTAL? Comments addressed. |
|
Thanks everyone. @rosstimothy, are you happy for this to land? |
This reverts commit 2f87127.
codingllama
force-pushed
the
codingllama/fido2-st-device
branch
from
d670c01 to
1042150
Compare
|
Rebased onto current master, updated go-libfido2 to use the newly-created v1.5.3-teleport.1 tag. PTAL @rosstimothy. |
|
@codingllama See the table below for backport results.
|
* Bump go-libfido2 to 399e6dce025f0fae1e47088e4912e113512240cb bump * Add the IsFIDO2 and Close functions to FIDODevice * Rewrite FIDO2 device interactions as single-threaded * test: Wait for device goroutines on tests * test: Drop "metered" tests * test: Adjust tests according to behavioral changes * nit: Invert u2f flag to fido2 * Reintroduce Info retries * nit: Update old comments * Delete unused code * test: Fix minor lint issues * Set the libfido2 device timeout * Appease linter * Revert "test: Wait for device goroutines on tests" This reverts commit 2f87127. * nit: Function declaration indent * Document that openedDevices.devices doesn't change length after assigned * Log the retry interval * Bump go-libfido2 to v1.5.3-teleport.1
#37539) * Bump go-libfido2 to 399e6dce025f0fae1e47088e4912e113512240cb bump * Add the IsFIDO2 and Close functions to FIDODevice * Rewrite FIDO2 device interactions as single-threaded * test: Wait for device goroutines on tests * test: Drop "metered" tests * test: Adjust tests according to behavioral changes * nit: Invert u2f flag to fido2 * Reintroduce Info retries * nit: Update old comments * Delete unused code * test: Fix minor lint issues * Set the libfido2 device timeout * Appease linter * Revert "test: Wait for device goroutines on tests" This reverts commit 2f87127. * nit: Function declaration indent * Document that openedDevices.devices doesn't change length after assigned * Log the retry interval * Bump go-libfido2 to v1.5.3-teleport.1
4 participants
This PR does two important changes:
The go-libfido2 patch (1) avoids opening and closing the device for each interaction, making interactions faster and more responsive. It comes with the added responsibility of managing the device lifecycle, which is much simpler to do non-concurrently (2). Put together the outcome should be more responsive and reliable FIDO2 interactions.
The new logic is equivalent in behavior to the previous code, with the exception of one breaking change: for simplicity we don't continuously poll for new devices, they must be plugged before tsh runs. (I expect this is already the norm for most users.)
#36640
Changelog: tsh FIDO2 backend re-written for improved responsiveness and reliability.