Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-incrementing signature counters could be due to race condition #2176

Merged
merged 2 commits into from
Oct 23, 2024

Conversation

sbweeden
Copy link
Contributor

@sbweeden sbweeden commented Oct 3, 2024

Closes #2172

Includes observation that an RP processing order race condition may be a cause of non-incrementing signature counters during validation.


Preview | Diff

Copy link
Contributor

@zacknewman zacknewman left a comment

Choose a reason for hiding this comment

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

LGTM.

index.bs Outdated Show resolved Hide resolved
@timcappalli timcappalli changed the title Address #2172 Non-incrementing signature counters could be due to race condition Oct 7, 2024
Copy link
Contributor

@MasterKale MasterKale left a comment

Choose a reason for hiding this comment

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

WebAuthn is a human-centric API with ceremony times long enough to prevent most race conditions related to counters not incrementing between responses. What types of RP use cases is this updated verbiage intended to address? I can only think of convoluted scenarios like "a single passkey used by multiple users across multiple kiosks not syncing counter state" that this might unblock. But otherwise why would we weaken the counter check like this?

@zacknewman
Copy link
Contributor

zacknewman commented Oct 9, 2024

What types of RP use cases is this updated verbiage intended to address? I can only think of convoluted scenarios like "a single passkey used by multiple users across multiple kiosks not syncing counter state" that this might unblock.

See the linked issue.

But otherwise why would we weaken the counter check like this?

Not sure I agree that this "weakens the counter check". No changes are made to the recommendation that an RP SHOULD fail the ceremony. This provides an example of a situation that RPs are welcome to ignore no differently than the possibility of a faulty authenticator which I don't believe "weakens" the counter check either.

An eager user can log in multiple devices using the same credential before the first ceremony is actually complete. The path data sent from one client takes may be encumbered with issues that data sent from another does not encounter allowing a "later" response to actually be received or at least processed before the "earlier" response.

accepting line breaks as elum suggested

Co-authored-by: Emil Lundberg <emil@emlun.se>
@nadalin nadalin added this to the L3-WD-02 milestone Oct 23, 2024
Copy link
Contributor

@MasterKale MasterKale left a comment

Choose a reason for hiding this comment

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

We had a good talk about this at today's WG call. I've come around and think this is fine guidance to include in the spec (since it doesn't change anything about actual counter-related response verification logic)

@nsatragno
Copy link
Member

What if the RP stores a snapshot of the counter of all authenticators for a user at the time a challenge is vended? That way you'd be able to compare the counter against the value at the time you requested an assertion.

@sbweeden sbweeden merged commit 3154b78 into w3c:main Oct 23, 2024
2 checks passed
github-actions bot added a commit that referenced this pull request Oct 23, 2024
SHA: 3154b78
Reason: push, by sbweeden

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@sbweeden
Copy link
Contributor Author

What if the RP stores a snapshot of the counter of all authenticators for a user at the time a challenge is vended? That way you'd be able to compare the counter against the value at the time you requested an assertion.

That's a clever remediation, but the focus of this PR is simply to describe some possible causes for counter mismatch rather than suggest how an RP should deal with it.

@sbweeden sbweeden deleted the sweeden_2172 branch October 23, 2024 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should race condition be added as a reason for a signature counter not increasing?
7 participants