Skip to content
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

Conformity of java-webauthn-server to FIDO2 #94

Open
Sahil333 opened this issue Dec 24, 2020 · 13 comments
Open

Conformity of java-webauthn-server to FIDO2 #94

Sahil333 opened this issue Dec 24, 2020 · 13 comments

Comments

@Sahil333
Copy link

Hi,

I am comparing this lib to other libraries specifically webauthn4j which says they are conformant to all the tests set by fido specification. Also, it says it supports all the attestation format.

Can someone please confirm if this lib is fido conformant and supports all the attestation formats?

Thanks

@RamitPahwa
Copy link

+1 for this information.

@emlun
Copy link
Member

emlun commented Jan 18, 2021

Hi,

We have not yet run the FIDO tests against this library, and unfortunately haven't implemented all the attestation formats yet - specifically, the android-key, apple and tpm attestation statement formats are not yet implemented. All these are things we would like to do, but we don't have a concrete timeline right now.

@rochards
Copy link

rochards commented Aug 20, 2021

Hello,

According to W3 android-safetynet-attestation and Android verify-attestation-response, it seems like this library performs all the verification procedures for Android SafetyNet Attestion in class AndroidSafetynetAttestationStatementVerifier. So, why do I still need to implement the MetadataService interface? And why was said above that this library doesn't implement this type of attestation?

Thank you.

@emlun
Copy link
Member

emlun commented Aug 20, 2021

Ah, sorry, that should have been android-key, not android-safetynet. I'll update the previous comment with that.

A MetadataService implementation is not necessary for verifying that the attestation signature is valid (that is built in, as you've pointed out), but it is necessary if you want to verify that the signature was made by a trusted entity. The library does not ship with the attestation root certificates needed for that additional step.

Or in other words: the library by implements steps 1-19 and 22-24 of §7.1. Registering a New Credential, but by default skips steps 20 and 21. A MetadataService implementation is needed for steps 20 and 21.

@emlun
Copy link
Member

emlun commented Aug 20, 2021

Oh, and the apple attestation statement format is also supported now.

@rochards
Copy link

Hello, me again 😄

Is there any chance this library will implement MetadaService for devices other than Yubico in the year 2022?

Thank you.

@emlun
Copy link
Member

emlun commented Dec 6, 2021

Hi! Work is currently underway to implement built-in support for the FIDO Metadata Service 3, which will make up the bulk of release 2.0. We're hoping to finish and release that in January 2022.

@ashensw
Copy link

ashensw commented Jan 26, 2022

Hi @emlun ,

Will the new version support all the attestation formats and conform to all mandatory test cases of FIDO2 Test Tool provided by FIDO Alliance as well. Also, can we expect the release this month?

@emlun
Copy link
Member

emlun commented Jan 28, 2022

Hi @ashensw,

Sorry, version 2.0 will not yet support all attestation formats and will not be finished this month. Conformance tests will also come later. Sorry for the slow progress on this.

@ionelMihai
Copy link

Hi @emlun

I have a question related to the userHandle from the authentication part.
Spec https://www.w3.org/TR/webauthn-2/#iface-authenticatorassertionresponse is saying that userHandle is nullable. Still FinishAssertionSteps.Step6 is requiring the userHandle to be present on either request or response.
But for an authentication where options.allowCredentials is empty and if the authenticator does not set the userHandle on the response, that check form FinishAssertionSteps.Step6 would fail.

My question is, does that validation meet the spec requirements?

We have a case where a user is using Mac Studio (Ventura 13.3.1 ), and for the authentication where options.allowCredentials is empty his request is failing because it looks like Mac Studio is not setting the userHandle on the response either. So the validation in FinishAssertionSteps.Step6 is failing.
Shouldn't the library search for the RegisteredCredential only by credentialId?

@emlun
Copy link
Member

emlun commented May 17, 2023

@ionelMihai Hm, that's actually a very good question.

The intent behind AuthenticatorAssertionResponse.userHandle being nullable is that non-discoverable credentials typically do not store any credential data on the authenticator, so they would not be able to store a user handle. But they would also only be usable with a non-empty allowCredentials, so the RP most likely has identified the user already (for example by a username) and can therefore know which user is authenticating anyway.

The purpose of having both the credential ID and the user handle is so that RPs aren't forced to use the credential ID as a primary lookup key, since the RP has no control of the choice of the credential ID. See this discussion from when user handles were introduced.

So it's certainly against the spirit of the spec to not return a user handle when allowCredentials is empty, because the above purpose of the user handle is undermined if the authenticator doesn't provide it when the RP needs it - and the authenticator cannot know which RPs need it and which don't. By extension, the CredentialRepository interface cannot know beforehand which lookup implementations will need the user handle and which ones won't, so it must be able to always provide the user handle, which in turn means it must always be able to get it either from AuthenticatorAssertionResponse or from StartAssertionOptions (possibly combined with CredentialRepository.getUserHandleForUsername).

But looking through the spec, I don't think we actually require the authenticator to return a user handle when allowCredentials is empty, it looks like that might only be an implicitly understood convention. So I'm not sure we can really fault Mac Studio for its implementation either.

I'd say this is a question for a WebAuthn spec issue. Either we amend the spec to explicitly require a non-null userHandle when allowCredentials is empty, in which case Mac Studio is non-compliant with that new spec; or we decide this is not a requirement, in which case java-webauthn-server is non-compliant and needs a change.

I'll open the issue in w3c/webauthn, then once that is resolved we'll decide if anything needs to change in java-webauthn-server. Thanks for raising the question!

@svartkanin
Copy link

Hey,
I would like to follow up on the status of full support for all attestation formats and fido conformance tests?

@emlun
Copy link
Member

emlun commented Jan 17, 2024

The android-key attestation statement format is not yet supported. We have not yet ran the FIDO conformance tests. Sorry, we don't currently have a plan for when we'll do either of those.

We would welcome contributions to implement android-key attestation, just like android-safetynet was accelerated by community contribution in PR #5. But we're currently not aware of any real authenticators that emit android-key attestations yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

7 participants