search

Yubico / java-webauthn-server

Server-side Web Authentication library for Java https://www.w3.org/TR/webauthn/#rp-operations
Other
465 stars 142 forks source link

Conformity of java-webauthn-server to FIDO2 #94

Open Sahil333 opened 3 years ago

Sahil333 commented 3 years ago

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 commented 3 years ago

+1 for this information.

emlun commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

rochards commented 2 years ago

Hello, me again 😄

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

Thank you.

emlun commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 8 months ago

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

emlun commented 8 months ago

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.