RFC: fido2: add FIDO2/CTAP2 authenticator subsystem#104327
Conversation
heronet
force-pushed
the
subsys/fido2/add-fido2-subsystem
branch
from
f070a4c to
954c225
Compare
heronet
force-pushed
the
subsys/fido2/add-fido2-subsystem
branch
2 times, most recently
from
31fa15e to
920ca5a
Compare
|
Force push to:
|
heronet
force-pushed
the
subsys/fido2/add-fido2-subsystem
branch
3 times, most recently
from
018d152 to
f8362c5
Compare
heronet
changed the title
|
d3zd3z
left a comment
d3zd3z
left a comment
There was a problem hiding this comment.
Aside from just a few questions about specifics, this looks like a really good starting point for a GSoC type of project.
| * A storage backend must provide exactly one definition of this symbol. | ||
| * Multiple definitions will cause a link error. | ||
| */ | ||
| extern const struct fido2_storage_api *fido2_storage_backend; |
There was a problem hiding this comment.
Any reason for the extra indirection? If it is a struct that must be provided, it could be done without the pointer.
There was a problem hiding this comment.
It does not need to be a pointer. Thanks for pointing it out.
| /* PIN storage */ | ||
|
|
||
| /** @brief Store PIN hash */ | ||
| int fido2_storage_pin_set(const uint8_t pin_hash[FIDO2_PIN_HASH_SIZE]); |
There was a problem hiding this comment.
Can you elaborate on what is stored here? Generally, a salt should be included with a hashed pin. As such, it is pretty easy to do a dictionary search to recover the pin, or for things like numeric pins, trivial.
There was a problem hiding this comment.
Authenticator stores LEFT(SHA-256(newPin), 16) internally as CurrentStoredPIN, sets the pinRetries counter to maximum count, and returns CTAP2_OK.
I do think that's a weakness but the spec only mandates storing LEFT(SHA-256(newPin), 16) plain with no salt.
| * @param user_data Opaque context provided during callback registration. | ||
| */ | ||
| typedef void (*fido2_transport_recv_cb_t)(const struct fido2_transport *transport, | ||
| const uint8_t *buf, size_t len, void *user_data); |
There was a problem hiding this comment.
If we're going to define a callback API, we need to be really clear what context that will be called in, does this come from the thread other comments have suggested will be started? Is there any coordination needed to use this from multiple threads?
Also, keep in mind that using callbacks will preclude this from being usable from threads running in Userspace.
There was a problem hiding this comment.
fido2_transport_register_recv_cb is never called from the userspace. It is called as part of the init sequence (fido2_init()). Which then registers the callback for all the available transports. The callback executes in the transport's internal work queue thread. It then hands off to an internal message queue and returns. The actual processing of the data is done by the core in a separate context. No coordination is needed from the caller side.
After giving it some more thought, registering the callback as part of the transport init looked like a better idea.
| * | ||
| * Wipes all stored credentials, PIN state, and resets the authenticator. | ||
| * Requires user presence confirmation (fido2_up_check()), and must be | ||
| * executed within 10 seconds of power-up per CTAP2 specification. |
There was a problem hiding this comment.
Who enforces this 10 second interval? Is that enforced within the fido2 implementation, or is the caller expected to enforce that?
There was a problem hiding this comment.
We enforce it internally. The caller does not need to concern with this. I should improve the comments while working on the implementation.
heronet
force-pushed
the
subsys/fido2/add-fido2-subsystem
branch
2 times, most recently
from
fded2dd to
a3b88e6
Compare
|
Force push to:
|
heronet
force-pushed
the
subsys/fido2/add-fido2-subsystem
branch
3 times, most recently
from
f621dfb to
fca1117
Compare
cfriedt
left a comment
cfriedt
left a comment
There was a problem hiding this comment.
Looks promising - the one thing I would suggest potentially is to make fido2 part of a larger authentication subsystem. There are many ways to authenticate.
An authentication subsystem sounds cool and maybe the better approach in the long run. But I am currently building this fido2 subsystem as a stack. I think it will be better to have a few more authentication systems show up in the tree before we design a generic subsystem API so that we don't over/under-engineer. We could try to build one right now but I think it will need a lot of research on what mechanisms are available and what fits the scope of Zephyr. Thoughts on landing this FIDO2 stack first? |
heronet
force-pushed
the
subsys/fido2/add-fido2-subsystem
branch
from
fca1117 to
83d8e26
Compare
|
LLMs being infinite content generator, we could be fixing PR reviews ad vitam eternam, so good to filter what makes sense checking: not required to fix them all for the Pr to be merged. Picking one particular example, it turns out to be inaccurate: #104327 (comment) despite pretending "high". |
Fixing all of them may even introduce new issues. A few of them were useful though. |
|
For all the mentions of "will be introduced in future PR", the challenge is that we are not 100% sure that the future PR will have this API, maybe the future PR wants a different one for reasons we discover during review of future PR. Also, there's some advise to avoid dead code (#20) which could apply here. Thanks once again! |
The dead code are mostly support infrastructure for Builtin UV, reset, get info etc. I wrote the headers a few months back reading the specs (there are a LOT of if/else in the spec itself) / looking at other implementations. It was needed for my understanding of how I should lay the whole thing out. But at this point I think I have enough idea about it that I can add them back anytime when they need implementation. I also did a ton on edit to the headers during the implementation as well. The next most important thing is clientPin/UV, which most RPs like Thanks for actively following this @josuah! |
|
May you keep all of this pending code preciously on a branch, it will be very useful later IMHO. Maybe even a draft PR just for the sake of making this branch visible... with the WIP future implementation, and a link on the issue? This way this would have visibility. The approach makes sense, thanks for outlining it. |
|
On my side, I do not currently see anything missing other than what was proposed already. And it has the green light from arch meeting for the new subsystem, so LGTM. :) Just a few details left, I will comment more. Just waiting your adjustments and feel free to ping again. |
josuah
left a comment
josuah
left a comment
There was a problem hiding this comment.
I think this is all I could find...
| | :zephyr:code-sample:`fido2` | 0x0012 | | ||
| +----------------------------------------------------+--------+ |
There was a problem hiding this comment.
@jfischer-no @tmon-nordic might be interested to note this.
josuah
left a comment
josuah
left a comment
There was a problem hiding this comment.
Some discussion about API organization this time (sorry I keep noticing random unimportant things as I re-read)
|
Removed unused code |
|
Hi @josuah! Thanks again for your quick feedback and extended review. Only 2 small things remain undecided. I would love to see your feedback when you get the time! |
Introduce public API headers for a FIDO2/CTAP2 authenticator subsystem. The subsystem allows Zephyr devices to act as hardware security keys supporting WebAuthn. Headers define: - Core lifecycle API (init, start, stop, reset, getInfo) - Pluggable transport abstraction (USB HID, BLE, NFC) - Pluggable user verification (PIN, biometric) - Pluggable credential storage (Settings, Secure Storage) - CTAP2 types, status codes, and command definitions Transport and UV backends register via iterable sections. Storage backend is selected via Kconfig choice. Signed-off-by: Siratul Islam <[email protected]>
|
Rebased to fix esp32s3 build error. |
Add the subsys directory structure and Kconfig symbols Signed-off-by: Siratul Islam <[email protected]>
- Implement the authenticatorGetInfo path - Add CBOR encoding for getInfo - Register the subsystem in CMake Signed-off-by: Siratul Islam <[email protected]>
- Add CBOR decoding for makecredential Signed-off-by: Siratul Islam <[email protected]>
- Add PSA crypto for makecredential Signed-off-by: Siratul Islam <[email protected]>
- Add Storage for makecredential Signed-off-by: Siratul Islam <[email protected]>
- Add authentication directory - Move fido2 into authentication Signed-off-by: Siratul Islam <[email protected]>
- Add MakeCredential command handling - Add attestation API - Add self attestation Signed-off-by: Siratul Islam <[email protected]>
- Add runtime state management Signed-off-by: Siratul Islam <[email protected]>
- Add CTAPHID Keepalive Signed-off-by: Siratul Islam <[email protected]>
- handle authenticator selection Signed-off-by: Siratul Islam <[email protected]>
- Add CBOR OP for getAssertion - Implement authenticatorGetNextAssertion(0x08) Signed-off-by: Siratul Islam <[email protected]>
- Add USB sample - Add sample documentation - Tested on 4 boards, included 3 overlays Signed-off-by: Siratul Islam <[email protected]>
- Add top-level authentication group - Add fido2 subsys docs Signed-off-by: Siratul Islam <[email protected]>
- Add top-level Authentication area - Add heronet and ceolin as maintainers Signed-off-by: Siratul Islam <[email protected]>
|
Fixes #104326
This PR introduces the public API headers for a proposed FIDO2/CTAP2 authenticator subsystem. The goal is to allow Zephyr devices to act as hardware security keys supporting WebAuthn.
Headers define:
Transport, UV backends register via iterable sections. Storage backend is a Kconfig choice
Update 23 March 2026: The FIDO2/CTAP2 architecture RFC was presented to the Zephyr Security Working Group on 23 March 2026 and received positive feedback / green light.
Meeting Slides: https://docs.google.com/presentation/d/1vVkEH7RpwIU5rFgGDTBC_mzTVFcqN4us58mL29mjwQU
Progress: