webauthn4j / webauthn4j Goto Github PK

Related to #49, I think we need to prepare emun of ALG_KEY like existing AttestationType.java.

Please refer to the following specification:
https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-registry-v2.0-rd-20180702.html#public-key-representation-formats

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

Since ValidationException(s) doesn't have source data, it is difficult for validator caller to handle error properly.

Describe the solution you'd like

Make ValidationException(s) to have source data

As some types are used in both request and response message, it is better to merge into new data package.

WebAuthn4J uses JUnit4 for now, but it is good time to migrate to JUnit5.
It seems there is some migration support around JUnit.

junit-vintage-engine

junit-vintage-engine provides backward compatibility.

IntelliJ Quick Fixes

IntelliJ Quick Fixes helps rewriting test code to modern JUnit5 based code style.
https://blog.jetbrains.com/idea/2017/11/intellij-idea-2017-3-junit-support/

T/O

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

In the verifying assertion flow, you should to verify response.userHandle.

Describe the solution you'd like

2. Identify the user being authenticated and verify that this user is the owner of the public key credential source credentialSource identified by credential.id:

If the user was identified before the authentication ceremony was initiated,

verify that the identified user is the owner of credentialSource. If credential.response.userHandle is present, verify that this value identifies the same user as was previously identified.

If the user was not identified before the authentication ceremony was initiated,

verify that credential.response.userHandle is present, and that the user identified by this value is the owner of credentialSource.

https://www.w3.org/TR/webauthn-1/#verifying-assertion

Describe alternatives you've considered

None.

Additional context

None.

FIDO Metadata Statement (Review Draft 02: at the time of this writing, this is the latest version of Review Draft) says userVerification member is a single USER_VERIFY constant and must be non-zero.

I think we need some verification against this member.

Please refer to the following specification:
https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-metadata-statement-v2.0-rd-20180702.html#dictionary-verificationmethoddescriptor-members

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

Hello,

I want to save the authenticator data only COSEKey, Counter and CredentialId in my DB. Is this a bad idea?

I read your WebAuthnAuthenticationContextValidator.validate() and it seems to be able to authenticate only with COSEKey and Counter.

Describe the solution you'd like

I want an interface that takes COSEKey and Counter as arguments instead of Authenticator.

public WebAuthnAuthenticationContextValidationResponse validate(
    WebAuthnAuthenticationContext authenticationContext, 
    COSEKey coseKey, 
    long counter) throws WebAuthnException {

I think you will need to change the signature counter to the return value of this method, and implementers will need to update it manually.

Describe alternatives you've considered

None

Additional context

None

Related to #47, I think we need to prepare emun of ALG_SIGN like existing AttestationType.java.

Please refer to the following specification:
https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-registry-v2.0-rd-20180702.html#authentication-algorithms

FIDO Metadata Statement (Review Draft 02: at the time of this writing, this is the latest version of Review Draft) says "U2FV1BIN", "FIDOV2", and enumerated strings defined in the FIDO UAF Registry of Predefined Values are supported as assertionScheme member's value.

I think we need some verification against this member.

Please refer to the following specification:
https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-metadata-statement-v2.0-rd-20180702.html#dictionary-metadatastatement-members

Just for record:

JsonTypeInfo has been removed from AttestationStatement from 0.9.8.RELEASE as it generates extra "format" member in the serialized data. It is useful for identifying type while desalinizing, but it was removed for spec conformance.

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "format")

43f9edc#diff-995a91a73371a964e3e97e8a76a1690cL26

Currently, enum types in WebAuthn4J have public static create factory method with @JsonCreator annotation.
Example:

The method is used from two way. One is "from Jackson deserializer", and the other is "from user code".
These usage is to better to be spitted into two methods: create and fromJson.

If fromJson is called with a wrong argument, it should throw InvalidFormatException, which is a Jackson exception. If create is called, it should throw IllegalArgumentException, which is a popular exception when the argument is wrong.

Expected implementation:

• create method should be public, static, and NOT annotated with @JsonCreator method.
• fromJson method should be private, static, and annotated with @JsonCreator method.

AttestationConveyancePreference is a good example of expected implementation.

Framework:
FIDO Conformance Tools v1.1.5(FIDO2 )

Test:
Server-ServerAuthenticatorAttestationResponse-Resp-5 Test server processing "packed" FULL attestation

Request:
F-6 Send ServerAuthenticatorAttestationResponse with FULL "packed" attestation, with attStmt.x5c contains a leaf certificate that is expired, and check that server returns an error

Response:
Error: the string "Promise succeded when expected to fail!" was thrown, throw an Error :)
at h.fail (js/vendor/mocha.min.js:1:43543)

Framework:
FIDO Conformance Tools v1.1.5(FIDO2 )

Test:
Server-ServerAuthenticatorAttestationResponse-Resp-5 Test server processing "packed" FULL attestation

Request:
F-7 Send ServerAuthenticatorAttestationResponse with FULL "packed" attestation, with attStmt.x5c contains a leaf certificate that is not yet started, and check that server returns an error

Response:
Error: the string "Promise succeded when expected to fail!" was thrown, throw an Error :)
at h.fail (js/vendor/mocha.min.js:1:43543)

Is your feature request related to a problem? Please describe.
Provide a Keycloak Authenticator leveraging WebAuthn4j.

Describe the solution you'd like
As a first step to get WebAuthn support into Keycloak it would be great to have an extension created for Keycloak that leverages WebAuthn4j to allow using Security Keys in place of OTP for two factor authentication.

The experimental extension for FIDO U2F can be used as an inspiration.

We have an open issue for this in the Keycloak project here https://issues.jboss.org/browse/KEYCLOAK-9360. If anyone is interested in working on such an extension please let us know.

Is your feature request related to a problem? Please describe.
I tried FIDO MDS using FidoMdsMetadataValidator in the below code. Is my code correct?

var fidoMdsMetadataItemsProvider = new FidoMdsMetadataItemsProvider(new JsonConverter());
fidoMdsMetadataItemsProvider.setFidoMetadataServiceEndpoint("https://mds2.fidoalliance.org/?token=foo");

var fidoMdsMetadataValidator =
        new FidoMdsMetadataValidator(
                new MetadataItemsResolverImpl(
                        fidoMdsMetadataItemsProvider));

var registrationObject = new RegistrationObject(
        null,
        null,
        response.getAttestationObject(),
        null,
        null,
        null,
        null,
        null
);

fidoMdsMetadataValidator.validate(registrationObject);

Describe the solution you'd like
I want to write this code a little shorter like...

var fidoMdsMetadataValidator = new DefaultFidoMdsMetadataValidator(fido_mds_token);
fidoMdsMetadataValidator.validate(response.getAttestationObject());

FIDO Metadata Statement (Review Draft 02: at the time of this writing, this is the latest version of Review Draft) says "uaf", "u2f", and "fido2" are supported as protocolFamily member's value.

I think we need some verification against this member.

Please refer to the following specification:
https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-metadata-statement-v2.0-rd-20180702.html#dictionary-metadatastatement-members

FIDO Metadata Statement (Review Draft 02: at the time of this writing, this is the latest version of Review Draft) says authenticationAlgorithm is a single ALG_ (might be ALG_SIGN) constant defined in the FIDO Registry of Predefined Values and must be non-zero. authenticationAlgorithms has to be a list of such values.

I think we need some verification against this member.

Please refer to the following specification:
https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-metadata-statement-v2.0-rd-20180702.html#dictionary-metadatastatement-members

JsonFileMetadataItemsProvider reads Metadate Statements from JSON files, but the user of this class cannot specify their paths.

Add following methods to Authenticator interface as registration extensions are also to be persisted

Map<String, RegistrationExtensionClientOutput> getClientExtensions();
Map<String, RegistrationExtensionAuthenticatorOutput> getAuthenticatorExtensions();

Describe the bug
Validation based on FIDO MDS must be moved out from MetadataItemsCertPathTrustworthinessValidator
because MetadataItemsCertPathTrustworthinessValidator is called only when attestation has an attestation certificate.
If attestation is none, no MDS validation will be conducted.

Expected behavior

MDS validation should be implemented as custom registration validaor.

Both bug entries concern the experimental webauthn4j-metadata service (0.10.0.RELEASE).

1.Cannot pass the access token to FidoMdsMetadataItemsProvider
As described in the MetadataService of the FIDO Alliance, when retrieving metadata or TOC you have to enhance the URL with your access token. Example (this does not use a valid token): https://mds2.fidoalliance.org/?token=6d6b44d78b09fed0c5559e34c71db291d0d322d4d4de0000.
Unfortunately, I could not find a nice way (except overwriting the HTTPClient in FidoMdsMetadataItemsProvider) to add my access token to the URL of any request to the MDS of FIDO Alliance. Is there any way to do so?

2. Mapping Error
I overwrote the HTTPClient in FidoMdsMetadataItemsProvider in order to add my access token to every request to the MDS of FIDO2 Alliance. But now I have to face a mapping problem in FidoMdsMetadataItemsProvider#fetchMetadataStatement(), line 236:
MetadataStatement metadataStatement = jsonConverter.readValue(metadataStatementStr, MetadataStatement.class); metadataStatementStr should represent the response of the MDS of FIDO Alliance, and the metadataStatementStr cannot be mapped to the Class MetadataStatement.

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

I want to internal class to AuthenticatorTransport.
I can make it like below codes, but I thought it would be better to define it because it's also in the specification.

AuthenticatorTransport.create("internal")

Describe the solution you'd like

/**
 * Indicates the respective authenticator is contacted using a client device-specific transport.
 * These authenticators are not removable from the client device.
 */
public static final AuthenticatorTransport INTERNAL = new AuthenticatorTransport("internal");

Describe alternatives you've considered

None.

Additional context

None.

Is your feature request related to a problem? Please describe.
"webauthn4j-extras" module only contains matadata statement service related codes.
"extras" is a too wide word to describe the module

Describe the solution you'd like
"webauthn4j-metadata" module shoud be spin out from "webauthn4j-extras" before reaching ver. 1.0

Messages like this should be revised:

Hi,

in the readme of webauthn4j, you wrote something like

save(authenticator); // please persist authenticator in your manner
Authenticator authenticator = load(credentialId); // please load authenticator object persisted in the registration process in your manner

I'm fine with that, but I failed to find a way to serialize and deserialize Authenticator instances. I tried several ways, using Jackson, converters, etc, but while serialization works, deserialization does not work :/

All the following examples fails :

val bytes = new CborConverter().writeValueAsBytes(authenticator)
println(bytes)
val auth = new CborConverter().readValue(bytes, classOf[Authenticator])
println(auth)

or

val json = new JsonConverter().writeValueAsString(authenticator)
println(json)
val auth = new JsonConverter().readValue(json, classOf[Authenticator])
println(auth)

or

val module = new com.webauthn4j.converter.jackson.WebAuthnJSONModule(new JsonConverter(), new CborConverter())
val mapper = new ObjectMapper().registerModules(module)
val json = mapper.writeValueAsString(authenticator)
println(json)
val auth =  mapper.readValue(json, classOf[Authenticator])
println(auth)

can you explain what i'm doing wrong ?

Thanks

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

Hello,

First, I thank you for the valuable documentation that can be read in Japanese.

As you may have noticed, I think it's time to update Known Issues, given the impact of this doc.

Describe the solution you'd like

• Safari 13 - Added support for FIDO2-compliant USB security keys with the Web Authentication standard in Safari on macOS
• Chrome 76 - Web Authentication API: Resident Key
• Firefox 66 - Passwordless Web Authentication Support via Windows Hello

Describe alternatives you've considered

None

Additional context

None

Related to #41, I think we need to prepare emun of USER_VERIFY like existing AttestationType.java.

Please refer to the following specification:
https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-registry-v2.0-rd-20180702.html#user-verification-methods

These classes are based on types defined in WebAuthn spec. https://www.w3.org/TR/webauthn/
Javadoc comment should be aligned with it.
It should be like

Fixed in fa77066

This issue blocks #8

gradle-bintray-plugin 1.7.3 doesn't work with gradle 5.x

These classes are based on types defined in WebAuthn spec. https://www.w3.org/TR/webauthn/
Javadoc comment should be aligned with it.
It should be like

WebAuthn4J uses Spring Boot BOM for dependency version management. It's time to update to 2.1.3.RELEASE.

FIDO Metadata Statement (Review Draft 02: at the time of this writing, this is the latest version of Review Draft) says publicKeyAlgAndEncoding is a single ALG_KEY constant defined in the FIDO Registry of Predefined Values and must be non-zero. publicKeyAlgAndEncodings has to be a list of such values.

I think we need some verification against this member.

Please refer to the following specification:
https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-metadata-statement-v2.0-rd-20180702.html#dictionary-metadatastatement-members

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

Hello,

First I want to say thank you for the great library.

I have a question about PublicKeyCredentialCreationOptions. I want to use PublicKeyCredentialCreationOptions directory to return Options to the client as JSON.

But I read your sharplab/spring-security-webauthn and seemed to create a new class like a AttestationOptions without using PublicKeyCredentialCreationOptions.

Should I not use PublicKeyCredentialCreationOptions too? (Same for PublicKeyCredentialRequestOptions)

Describe the solution you'd like

If I can use it, I thought it would be better to create a builder like;

    public PublicKeyCredentialCreationOptions creationOptions(Account account) {
        var credentials = credentialRepository.findCredentials(account.email);
        return PublicKeyCredentialCreationOptions.builder()
                .setRp(PublicKeyCredentialRpEntity.builder()
                        .setId("localhost")
                        .setName("Relying Party")
                        .setIcon("")
                        .build())
                .setUser(PublicKeyCredentialUserEntity.builder()
                        .setId(account.id.getBytes())
                        .setName(account.email)
                        .setDisplayName(account.name)
                        .setIcon("")
                        .build())
                .setChallenge(new DefaultChallenge())
                .setPubKeyCredParams(List.of(
                        PublicKeyCredentialParameters.builder()
                                .setType(PublicKeyCredentialType.PUBLIC_KEY)
                                .setAlg(COSEAlgorithmIdentifier.ES256)
                                .build(),
                        PublicKeyCredentialParameters.builder()
                                .setType(PublicKeyCredentialType.PUBLIC_KEY)
                                .setAlg(COSEAlgorithmIdentifier.RS256)
                                .build()
                ))
                .setTimeout(12000L)
                .setExcludeCredentials(credentials.stream()
                        .map(credential -> PublicKeyCredentialDescriptor.builder()
                                .setType(PublicKeyCredentialType.PUBLIC_KEY)
                                .setId(credential.credentialId)
                                .setTransports(Set.of(
                                        AuthenticatorTransport.BLE,
                                        AuthenticatorTransport.NFC,
                                        AuthenticatorTransport.USB,
                                        AuthenticatorTransport.create("internal")))
                                .build())
                        .collect(Collectors.toList())
                )
                .setAuthenticatorSelection(AuthenticatorSelectionCriteria.builder()
                        .setAuthenticatorAttachment(AuthenticatorAttachment.PLATFORM)
                        .setUserVerification(UserVerificationRequirement.REQUIRED)
                        .setRequireResidentKey(true)
                        .build())
                .setAttestation(AttestationConveyancePreference.DIRECT)
                .setExtensions(new AuthenticationExtensionsClientInputs<>())
                .build();
    }

Describe alternatives you've considered

Yubico/java-webauthn-server has a builder with a inner class in the core library.
PublicKeyCredentialCreationOptionsBuilder

Additional context

None

As existing ContextValidator(s) does data parse and validation in the same method call ( ContextValidator#validate ), caller cannot access the parsed data when an error occurred in the validation phase.
To resolve this issue, introduce WebAuthnManager class which splits data parse phase and data validation phase into two dedicated method call.

Sample code:

RegistrationRequest registrationRequest = new RegistrationRequest(
attestationObject, // Caution: order of the arguments is changed from `WebAuthnRegistrationContext`
clientDataJSON, 
clientExtensionJSON, 
transports);
RegistrationParameters registrationParameters = new RegistrationParameters(serverProperty, userVerificationRequired, userPresenceRequired, expectedExtensionIds);
RegistrationData registrationData;
try{
    // data parse phase
    registrationData = webAuthnManager.parse(registrationRequest);
}
catch (DataConversionException e){
    // If you would like to handle WebAuthn data structure parse error, please catch DataConversionException
    throw e;
}
try{
    // data validation phase
    webAuthnManager.validate(registrationData, registrationParameters);
}
catch (ValidationException e){
    // If you would like to handle WebAuthn data validation error, please catch ValidationException
    throw e;
}

If you would like to parse and validate in one call, use WebAuthnManager#validate method.

RegistrationRequest registrationRequest = new RegistrationRequest(
attestationObject, // Caution: order of the arguments is changed from `WebAuthnRegistrationContext`
clientDataJSON, 
clientExtensionJSON, 
transports);
RegistrationParameters registrationParameters = new RegistrationParameters(serverProperty, userVerificationRequired, userPresenceRequired, expectedExtensionIds);
RegistrationData registrationData;
try{
    // data parse and validation in one method call
    webAuthnManager.validate(registrationRequest, registrationParameters);
}
catch (DataConversionException e){
    // If you would like to handle WebAuthn data structure parse error, please catch DataConversionException
    throw e;
}
catch (ValidationException e){
    // If you would like to handle WebAuthn data validation error, please catch ValidationException
    throw e;
}

WebAuthnManager deprecates existing WebAuthnRegistrationContextValidator and WebAuthnAuthenticationContextValidator. These classes will be removed by 1.0 GA.

I think we need to prepare emun of KEY_PROTECTION like existing AttestationType.java.

Please refer to the following specification:
https://fidoalliance.org/specs/fido-v2.0-rd-20180702/fido-registry-v2.0-rd-20180702.html#key-protection-types

I've looked into FIDO Metadata Statement (Draft 27: at the time of this writing, this is the latest version of 2.0 Implementation Draft) and found that MetadataStatement.java did not contain required "description" member.

Please refer to the following link.
https://fidoalliance.org/specs/fido-v2.0-id-20180227/fido-metadata-statement-v2.0-id-20180227.html#widl-MetadataStatement-description

'com.webauthn4j.metadata.data' package still have some mutable list classes, which inherits ArrayList like VerificationMethodANDCombinations.
These class should inherit AbstractList directly like AttestationCertificatePath.

Some classes under under com.webauthn4j.converter package requires Registry class injection, but it should be removed for making it simple.

Objective

1.0.0 release is a next big target. Since WebAuthn4J has been adopted by projects like Keycloak, declare 1.0.0 version and stop breaking change.

Milestones

To make it happen, WebAuthn4J need to be stable and mature. Following items need to be resolved before 1.0.0 release.

• Review design through use-cases
  • Review keycloak usage and feedback to WebAuthn4J
    • Split WebAuthnManager #289
      • WebAuthnManager is intended that its instance is shared between registration phase and authentication phase to keep configuration consistency easily.
        But in some use-cases, sharing its instance between two phases are not straight-forward and restricts application design.
  • Implement webauthn4-spring-security to check WebAuthn4J works in various use-cases
    • WebAuthn as second-factor
    • WebAuthn as First-factor and id-less login
    • Associate multiple authenticators to a user
    • Avoid duplicated registration of same authenticator
• Revisit WebAuthn extension processing design
  since extensions were implemented to conform W3C WebAuthn specification, but not tested in real scenario because most of them are not implemented by browsers.
  • Clarify inconsistency between W3C WebAuthn spec. and FIDO CTAP2 spec regarding WebAuthn extension identifier rule : w3c/webauthn#1430
    • This is not WebAuthn4J's issue but W3C WebAuthn spec. issue, but it may impact WebAuthn4J extension API signature.
• Deprecate features removed from W3C WebAuthn specification Level2.
  • Some features defined in WebAuthn Level1 are removed from Level2. To make WebAuthn4J implementation simple, remove them before 1.0.0 release. #295
• Remove features removed from W3C WebAuthn specification Level2.
  • Some features defined in WebAuthn Level1 are removed from Level2. To make WebAuthn4J implementation simple, remove them before 1.0.0 release. #285
• Support Apple Anonymous Attestation

Hi,

We've been evaluating the project's source code a bit an saw that

• (for example) AttestationStatementValidator does not include an exception signature in the "validate" method.
• The thrown examples are RuntimeExceptions (at least the ones extending from WebAuthnException).

Would it no be useful make a distinction between checked and unchecked exceptions? A failure in the validation of the signature should be catchable by the implementor of the library I would think?

Regards,
K

It seems there is no need to restrict to "SunEC" provider.

If there is a similar issue, it should be fixed too.