Merge changes from Nitrokey/fido-authenticator #41
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some services do not accept arbitrary long key handle (aka Credential ID), which makes the FIDO operations failing. This patch removes some fields from credential data serialization while making credential ID, and with this it reduces key handle size by around 30% (from ~320 to ~220 using test site [1]). Tested on Gitlab, and this patch makes it working correctly (both registering and signing, as opposed to 500 error code returned otherwise). Presumably the hidden limit is 255 bytes, which would be compatible with CTAP1. Resident Keys stay the same, with full metadata stored on the device. [1] webauthn.bin.coffee
Remove some fields from credential data serialization while making credential ID. Reduces key handle size by around 30% (from ~320 to ~220). Tested on Gitlab, and this patch makes it working correctly (both registering and signing, as opposed to 500 error code returned otherwise). Presumably the hidden limit is 255 bytes, which would be compatible with CTAP1. Related: trussed-dev#8
This patch adds a configuration option to skip the additional user presence check for the first Get Assertion or Authenticate request within a certain duration after boot. In this case, the device insertion is interpreted as a user presence indicator.
Instead of panicking, we now return a RequestTooLarge error if the encrypted and serialized credential ID is longer than 255 bytes. Fixes: trussed-dev#15
Skip user presence check directly after boot
Return error if credential ID is too long
Trussed itself already ignored this associated data (trussed-dev/trussed#108), and the unwrapping was already performed with no associated data. Not removing it would lead to breakage once (trussed-dev/trussed#108) is merged. Adding the AD to the unwrapping step would break compatibility with currently registerd credentials. Security: This is not an issue because the credentials stored locally are encrypted with the proper app id as associated data which is checked when the credential is decrypted.
Remove associated data in wrapping of keys
This patch bumps the Trussed dependency to include the update to littlefs2 0.4.0.
Bump trussed
Previously, we estimated that we can handle 100 resident keys when returning the number of remaining resident keys in the credential management command. This patch introduces a config option to set a maximum count of resident keys that is used to report the number of remaining resident keys and that is enforced when trying to create a new resident key.
Make maximum resident credential count configurable
If we overwrite an existing resident credential, the corresponding RP directory can be empty when we call CredentialManagement::get_creds_metadata to count the existing credentials. This causes an error in the current implementation. This patch changes CredentialManagement::get_creds_metadata to gracefully handle empty RP directories. Fixes Nitrokey/nitrokey-3-firmware#254
If we try to register a new resident credential, we delete existing credentials with the same RP and user ID. If we then cannot store the new credential because the credential limit is reached or there is a filesystem write error, we may be left with an empty RP dir. This patch adds a check to delete empty RP dirs in this case.
Fix overwriting resident keys
Adapt to interrupt mechanism
This patch sets the makeCredUvNotRqd CTAP option to true to indicate that we support makeCredential operations without user verification. See also: https://fidoalliance.org/specs/fido-v2.1-rd-20201208/fido-client-to-authenticator-protocol-v2.1-rd-20201208.html#getinfo-makecreduvnotrqd Fixes: trussed-dev#26
Set makeCredUvNotRqd in CTAP options
The getAssertion command does not use the rk option so we return an InvalidOption error if it is set. Fixes: #23
Previously, a PinAuthBlocked error was already returned after two wrong PIN entries. The reason for this as that decrement_retries also checks if the allowed retries are exceeded. This as unnecessary because pin_blocked is always checked before decrement_retries is called. This patch removes the check in decrement_retries. Fixes: #27
As required by the Webauthn spec, we now ignore public key credential parameters with a type other than "public-key". Fixes: #20
…hash` from `rp_id_hash/credential_id_hash` The goal is to make credential storage more efficient, by making use of littlefs's ability to inline file contents into the directory metadata when the file is small.
This patch moves the key generation and signing logic into the SigningAlgorithm enum, removing some duplicated code from the ctap2 and ctap2::credential_management modules.
This fixes a new clippy lint.
This changes the error code if uv = true to InvalidOption even if a PIN is set. Previously, we returned PinRequired if a PIN is set. The new implementation follows § 6.1.2 Step 5 of the specification more closely. https://fidoalliance.org/specs/fido-v2.1-rd-20201208/fido-client-to-authenticator-protocol-v2.1-rd-20201208.html#sctn-makeCred-authnr-alg
Currently, we always require the PIN to be used for make_credential operations if it is set. This patch implements the makeCredUvNotRqd option that allows non-discoverable credentials to be created without using the PIN according to § 6.1.2 Step 6 of the specification, see: https://fidoalliance.org/specs/fido-v2.1-rd-20201208/fido-client-to-authenticator-protocol-v2.1-rd-20201208.html#sctn-makeCred-authnr-alg https://fidoalliance.org/specs/fido-v2.1-rd-20201208/fido-client-to-authenticator-protocol-v2.1-rd-20201208.html#getinfo-makecreduvnotrqd Fixes: #34
This fixes compatibility with CTAP 2.1. Fixes: #118
Instead, the nitrokey-3-firmware usbip runner should be used.
This pulls in all changes from the Nitrokey/fido-authenticator repository, improving compliance with the CTAP spec, adding support for CTAP 2.1 and implementing new features like the largeBlob extension.
daringer
approved these changes
Sep 1, 2025
This was referenced Sep 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pulls in all changes from the Nitrokey/fido-authenticator repository, improving compliance with the CTAP spec, adding support for CTAP 2.1 and implementing new features like the largeBlob extension.
Due to the large number of changes in Nitrokey/fido-authenticator, it is not feasible to port them individually to upstream. As trussed-dev/fido-authenticator does not have any significant changes, I think it would be best to just merge in the changes from the fork even if this creates merge commits, similar to what we did with piv-authenticator. See the changelog for more information on the included changes.