Skip to content

Use FIDO2 touch functions to wait for touch#37203

Merged
codingllama merged 5 commits intomasterfrom
codingllama/fido2-get-touch
Jan 31, 2024
Merged

Use FIDO2 touch functions to wait for touch#37203
codingllama merged 5 commits intomasterfrom
codingllama/fido2-get-touch

Conversation

@codingllama
Copy link
Copy Markdown
Contributor

@codingllama codingllama commented Jan 24, 2024

Use libfido2 touch polling functions instead of fake assertions when trying to determine if a device was touched.

The touch functions make it simpler to decide if the device was actually touched and likely cause less unnecessary work for the authenticator.

#36640

@codingllama codingllama added backport/branch/v13 no-changelog Indicates that a PR does not require a changelog entry labels Jan 24, 2024
@github-actions github-actions Bot requested review from greedy52 and tigrato January 24, 2024 20:56
@codingllama
Copy link
Copy Markdown
Contributor Author

if touched, _ := waitForTouch(dev); touched {
err = nil // OK, selected successfully
} else {
err = &nonInteractiveError{err: err}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't spot any problems during testing, but arguably this line should be a part of #37181.

Comment thread lib/auth/webauthncli/fido2.go
Comment thread lib/auth/webauthncli/fido2.go
Comment thread lib/auth/webauthncli/fido2.go
@codingllama
Copy link
Copy Markdown
Contributor Author

PTAL?

Comment thread go.mod Outdated
Comment thread lib/auth/webauthncli/fido2.go Outdated
Comment thread lib/auth/webauthncli/fido2.go Outdated
Comment thread lib/auth/webauthncli/fido2.go Outdated
Comment thread lib/auth/webauthncli/fido2.go Outdated
@codingllama
Copy link
Copy Markdown
Contributor Author

PTAL?

@codingllama codingllama force-pushed the codingllama/fido2-st-device branch from d670c01 to 1042150 Compare January 30, 2024 16:15
@codingllama codingllama force-pushed the codingllama/fido2-get-touch branch from e8ebf19 to 5a2d73c Compare January 30, 2024 16:18
@codingllama
Copy link
Copy Markdown
Contributor Author

PTAL @rosstimothy ?

Base automatically changed from codingllama/fido2-st-device to master January 30, 2024 17:20
@codingllama codingllama force-pushed the codingllama/fido2-get-touch branch from 5a2d73c to 9942bdd Compare January 30, 2024 17:48
@codingllama
Copy link
Copy Markdown
Contributor Author

Rebased onto master, diffs should be clear again.

@codingllama
Copy link
Copy Markdown
Contributor Author

Friendly ping @rosstimothy @greedy52? I would like to get all FIDO2 changes merged close together, so hopefully they all land in a single release.

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from greedy52 January 31, 2024 17:16
@codingllama codingllama added this pull request to the merge queue Jan 31, 2024
Merged via the queue into master with commit 5f644b6 Jan 31, 2024
@codingllama codingllama deleted the codingllama/fido2-get-touch branch January 31, 2024 18:26
@public-teleport-github-review-bot
Copy link
Copy Markdown

@codingllama See the table below for backport results.

Branch Result
branch/v13 Create PR
branch/v14 Create PR
branch/v15 Create PR

codingllama added a commit that referenced this pull request Jan 31, 2024
* Use FIDO2 touch functions to wait for touch

* nit: Initialize nonInteractiveError uniformly

* Change fido2TouchMaxWait to 200ms

* Log dev.TouchBegin() errors

* Apply various error-handling fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants