kanidm / webauthn-rs

An implementation of webauthn components for Rustlang servers
Mozilla Public License 2.0
485 stars 80 forks source link

Accessor to information about Public Key Credential #297

Closed conr2d closed 1 year ago

conr2d commented 1 year ago

Is your feature request related to a problem? Please describe.

https://www.w3.org/TR/webauthn-2/#iface-authenticatorattestationresponse

According to the specification, there are convenience functions to access the information about Public Key Credential:

interface AuthenticatorAttestationResponse : AuthenticatorResponse {
    [SameObject] readonly attribute ArrayBuffer      attestationObject;
    sequence<DOMString>                              getTransports();
    ArrayBuffer                                      getAuthenticatorData();
    ArrayBuffer?                                     getPublicKey();
    COSEAlgorithmIdentifier                          getPublicKeyAlgorithm();
};

Describe the solution you'd like

// Since AuthenticatorAttestationResponse is an internal type, so Raw type is used.
impl AuthenticatorAttestationResponseRaw {
    pub fn get_transports(&self) -> Vec<AuthenticatorTransport> {}
    pub fn get_authenticator_data(&self) -> Vec<u8> {}
    pub fn get_public_key(&self) -> Option<Vec<u8>> {}
    pub fn get_public_key_algorithm(&self) -> COSEAlgorithm {}
}

Are there any plans to support this feature? If allowed, I can create a PR for it.

If this feature is not in the scope of the supported features by webauthn-rs, a separate package can be considered.

Firstyear commented 1 year ago

We actually include a lot of this data already, but there are two-ish dimensions to it.

First, if you look at https://docs.rs/webauthn-rs-core/0.4.9/webauthn_rs_core/interface/struct.Credential.html then after a successful registration a lot of the metadata you want here is already available in this interface, and can also be accessed via the safe wrapper types in a more limited way via https://docs.rs/webauthn-rs/0.4.8/webauthn_rs/prelude/struct.Passkey.html

In addition we also already provide the needed methods attached to this to allow attestation to be post-registration performed with updated ca-lists. https://docs.rs/webauthn-rs-core/0.4.9/webauthn_rs_core/interface/struct.Credential.html#method.verify_attestation we are still working on the attestation story at the moment due to issues with the fido mds.

So these "accessors" per the specification don't exist in webauthn-rs directly by those method names, but you can access the equivalent data via our interfaces that we provide.

The second part of this is "what are you trying to achieve"? These data are generally useful only with in the webauthn library itself as part of the processing of an attestation or assertion. So is there something you are trying to do?

And finally, i'm going to point out the "getTransports()" call specifically is not to be trusted as the browser in many cases does NOT list the correct list of compatible transports for the device and how it was used. We have actually consciously chosen to not populate the transport field in Credential at the time because the data from getTransports is so unreliable and inconsistent that it should not - and can not - be used correctly. The only "correct" way to get the list of transports is attestation and to access the fido mds and get the list from that. However this comes back to attestation and the issues around that which we are working on to resolve.

Does that help?

conr2d commented 1 year ago

@Firstyear Wow, I really appreciate your detailed explanation.

I am running multiple user services, so I tried making a login hub authenticated by passkey, but each site handles user authentication by oauth2 with that login hub. My plan was using user credential's public key as a unique user identifier in oauth2, but it seems that your mentioning accessor to metadata doesn't support retrieving a public key from credential.

An easier way to solve my problem might be adding a getter for public key from Passkey (and its inner Credential type), but I was curious about missing accessors in specification is intended or not.

Firstyear commented 1 year ago

Is that in the passkey api that those accessors are lacking? I think there is value in exposing some info about the inner credential in Passkey / SecurityKey etc. But yeah, as mentioned the details you want may not have "those accessor names" but those data do exist and are accessible for various reasons.

Using the cred id as a unique id in oauth2 probably actually isn't a problem given what a cred id is and that it SHOULD always be unique between users, but I'd need to see what you are doing in more detail to give more thoughts if you wanted them :)

conr2d commented 1 year ago

webauthn_rs_core::interface::Credential exposes inner public key:

pub struct Credential {
    pub cred_id: CredentialID,
    pub cred: COSEKey, // inner public key
    pub counter: Counter,
    pub transports: Option<Vec<AuthenticatorTransport>>,
    pub user_verified: bool,
    pub backup_eligible: bool,
    pub backup_state: bool,
    pub registration_policy: UserVerificationPolicy,
    pub extensions: RegisteredExtensions,
    pub attestation: ParsedAttestation,
    pub attestation_format: AttestationFormat,
}

But in the case of webauthn_rs::interface::Passkey, its cred field is visible only inside webauthn-rs package:

pub struct Passkey {
    pub(crate) cred: Credential,
}

impl Passkey {
    /// Retrieve a reference to this Pass Key's credential ID.
    pub fn cred_id(&self) -> &CredentialID {}

    /// Retrieve the type of cryptographic algorithm used by this key
    pub fn cred_algorithm(&self) -> &COSEAlgorithm {}

    /// Post authentication, update this credentials properties.
    pub fn update_credential(&mut self, res: &AuthenticationResult) -> Option<bool> {} 
}

Unless I miss something, public key getter might not be provided. ;)

This plan is not set in stone, but I think my login hub can be expanded to global key registry like keyoxide or keybase. And also I am looking to see if user can utilize his public key as digital identity with did-method-key specification.

I would be happy, if passkey can expose the inner public key like:

impl Passkey {
    pub fn cred_public_key(&self) -> &COSEKey {
        &self.cred.cred
    }
}

Even if you have determined that this feature cannot be supported by webauthn-rs, I wholeheartedly respect your decision. Thank you for creating such excellent open source software.

Firstyear commented 1 year ago

My suggestion is that for impl Passkey { } we can extend it with relevant getters to expose the details of the inner credential. I don't want to expose the "Credential" directly since that opens the door to "accidental misuse". I am happy for the "get_public_key()" method to be added :)

The only exception I'll "reject" for now is transports, since we do not store transports as they are almost always misleading and incorrect.

Is that reasonable? :)

conr2d commented 1 year ago

Yes, perfect!

BTW, COSEKey and related types are exported by webauthn_rs_core::proto. What do you think of re-exporting these types via webauthn_rs::prelude?

To utilize returned COSEKey by Passkey::get_public_key(), related types are necessary to access enum field by pattern matching. I think there can be three cases.

Case 1:

// webauthn_rs
pub mod prelude {
    /* re-exporting types here */
}

Case 2:

Create separate sub-module. This will not pollute prelude with too many types when user doesn't need COSEKey and related types, but webauthn-rs will have an additional module.

// webauthn_rs
pub mod cose /* or other name */ {
    /* re-exporting types here */
}

Case 3:

Do not re-export types to webauthn-rs, but user will need to have a direct dependency on webauthn-rs-core.

Additional Consdieration

It needs to determine what types will be re-exported, all related types or only essential types for pattern matching:

pub struct COSEKey {} // optional
pub struct COSEOKPKey {} // optional
pub struct COSEEC2Key {} // optional
pub struct COSERSAKey {} // optional
pub enum COSEAlgorithm {} // required
pub enum COSEKeyType {} // required
pub enum COSEKeyTypeId {} // not used with COSEKey
pub enum ECDSACurve {} // required
pub enum EDDSACurve {} // required, but not supported yet

{
    let key = passkey.get_public_key();
    match key.type_ {
        COSEAlgorithm::ES256 => {
        },
        _ => {},
    }
    /* or */
    match &key.key {
        COSEKeyType::EC_EC2(ref key) => {
            if (key.curve != ECDSACurve::SECP256R1) {
            }
        },
        COSEKeyType::RSA(ref key) => {
        },
        _ => {},

    }
}
Firstyear commented 1 year ago

Yes, perfect!

BTW, COSEKey and related types are exported by webauthn_rs_core::proto. What do you think of re-exporting these types via webauthn_rs::prelude?

To utilize returned COSEKey by Passkey::get_public_key(), related types are necessary to access enum field by pattern matching. I think there can be three cases.

Case 1:

// webauthn_rs
pub mod prelude {
    /* re-exporting types here */
}

I like Case 1 - it's nice and simple, and we just re-export what we need into a singular prelude namespace.

PS: sorry for the delay I was recovering from something.

conr2d commented 1 year ago

@Firstyear No problem. I hope you are recovering well.

Please review the PR I wrote for this issue when you are feeling better. (Not urgent) :)