-
Notifications
You must be signed in to change notification settings - Fork 5
WIP: Use libwebauthn for JSON response serialization #117
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
base: libwebauthn-json
Are you sure you want to change the base?
Conversation
7c84e5d to
41945cd
Compare
This commit migrates from custom JSON response serialization to libwebauthn's WebAuthnIDLResponse::to_inner_model() for both create credential (MakeCredential) and get credential (GetAssertion) responses. Changes: - Use libwebauthn's to_inner_model() to serialize responses, then modify the result to add transport and authenticator_attachment information that is known at the credential service level - Remove create_credential_request_try_into_ctap2's client_data_json return value (now extracted from the request by libwebauthn) - Remove get_credential_request_try_into_ctap2's client_data_json return value - Update gateway.rs to clone the request for response serialization - Remove unused modules: cbor.rs, cose.rs, serde/mod.rs - Simplify webauthn.rs to just re-exports from libwebauthn This removes ~800 lines of custom serialization code including: - CreatePublicKeyCredentialResponse and GetPublicKeyCredentialResponse - AttestationStatement enum and create_attestation_object function - All the extension output types (CredentialPropertiesOutput, etc.) - Custom CBOR writer for attestation object serialization - COSE key type helpers The response serialization now uses libwebauthn's implementation which: - Handles attestation object CBOR encoding - Handles all extension output serialization - Handles base64url encoding of binary fields - Produces WebAuthn Level 3 compliant JSON responses
41945cd to
82aefc6
Compare
msirringhaus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor questions
| use tracing::debug; | ||
|
|
||
| use crate::cose::CoseKeyAlgorithmIdentifier; | ||
| //! WebAuthn types re-exported from libwebauthn. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this file at all now? Is it used anywhere else except in dbus/model.rs?
| .authenticator_data | ||
| .to_response_bytes() | ||
| .map_err(|err| format!("Failed to parse authenticator data: {err}"))?; | ||
| .to_inner_model(request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this function get a different name? to_inner_model() isn't really intuitive from a user-perspective. Same for MakeCredential.
This PR migrates from custom JSON response serialization to libwebauthn's
WebAuthnIDLResponse::to_inner_model()for both create credential (MakeCredential) and get credential (GetAssertion) responses.Stacked on: #116
Changes
to_inner_model()to serialize responsestransportsandauthenticator_attachment(see libwebauthn#159)client_data_jsonfrom return value (now extracted by libwebauthn)to_inner_model())Behavioral Changes
New fields in response (WebAuthn Level 3 enhancements):
response.authenticatorData- authenticator data separately encodedresponse.publicKey- public key in COSE formatresponse.publicKeyAlgorithm- COSE algorithm identifiertype- credential type field ("public-key")TODOs from Deleted Code
The old code had one TODO that should be implemented in libwebauthn:
Created linux-credentials/libwebauthn#161.
Related Issues