Skip to content

Fix signature counter to improve spec compliance#66

Open
robin-nitrokey wants to merge 7 commits into
mainfrom
signature-counter
Open

Fix signature counter to improve spec compliance#66
robin-nitrokey wants to merge 7 commits into
mainfrom
signature-counter

Conversation

@robin-nitrokey
Copy link
Copy Markdown
Member

The current signature counter implementation has three major problems: it starts at zero, it always increments the counter by 1 and it does not handle overflows. But Requirement 2.3.2 of the FIDO Security Requirements v1.5 requires that zero is only used to indicate missing support for signature counters or an error state, that a global signature counter is incremented by a random non-zero value to increase privacy and that the counter does not overflow.

This PR changes the signature counter implementation to accommodate these requirements. The increment is set to 1 + 0..255. As the counter is a u32, this still leaves us more than 16 million operations in the worst case until the counter overflows.

This ensures that all fields are reset and makes it possible to use a
custom reset value that is different than the Default implementation.
Returning the signature counter 0 would indicate that we don’t support
signature counters or that a counter error occured.
As described in Requirement 2.3.2 of the Security Requirements v1.5, a
signature counter value of zero indicates an error. If the authenticator
returns zero once, it may not return a non-zero value in subsequent
calls, so we have to stay in the error state if an overflow occurs.
As defined in Requirement 2.3.2 of the Security Requirements v1.5, we
have to use a random (positive) increment for a global signature
counter. This patch uses a random u8 + 1. As the signature counter is a
u32, this still gives us more than 16 million operations until the
counter can potentially overflow.
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.

2 participants