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.

Sign up for GitHub

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

Jump to bottom

Issue #217 WebAuthn Authenticator Registration fail when Yubikey5+Chrome+ResidentKey #218

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Issue #217 WebAuthn Authenticator Registration fail when Yubikey5+Chrome+ResidentKey #218

wants to merge 5 commits into from

Conversation

DTonoki
Copy link
Contributor

@DTonoki DTonoki commented Jun 26, 2020

Analysis

#217
WebAuthn Authenticator Registration fail when Yubikey5+Chrome+ResidentKey.
Yubikey(v5.2.4) + Google Chrome return "credProtect" extension from CTAP2.1draft .
WebAuthn Authentication module's validation proccess can't handle unexpected "credProtect" extension.

Solution

add credProtect to expectedExtensionIds.

Compatibility

jackson-2.10.4 and webauthn4j-0.12.0 are needed for this fix.

Testing

Config WebAuthn(Registor) module with Residentkey = true.
Registration Residentkey with Chrome and Yubikey5 on Linux or MacOS platoform.

Regression testing

Windows 10 with Edge(Chrome) registration/authentication with windows hello
Windows 10 with Edge(Chrome) residentkey registration/authentication windows hello
Windows 10 with Edge(Chrome) registration/authentication with yubikey5
Windows 10 with Edge(Chrome) residentkey registration/authentication yubikey5
Linux Chrome registration/authentication with yubikey5
Linux Chrome registration/authentication residentkey with yubikey5

@DTonoki
Update WebAuthn4J to 0.12.0 release
33e1723
@DTonoki
Fix about reviewer's comments.
4bc31b8 
- delete import ObjectMapper line
- instance variable definitions in one place
- parsing and validation are placed one place between try-catch block
- update copylight year
@DTonoki
fix typo
c8a89eb
@DTonoki
Fix copyright years.
7ddb9a0
@DTonoki
add credProtect to expectedExtensionIds
4abae82
@tsujiguchitky tsujiguchitky changed the title Issues/217 web authn registration fail Issue #217 WebAuthn Authenticator Registration fail when Yubikey5+Chrome+ResidentKey Jul 6, 2020
@tsujiguchitky
Copy link
Contributor

Applying this fix without updating jackson results in the following debug log.

WebAuthnRegister:07/06/2020 02:24:11:380 PM JST: Thread[ajp-bio-8009-exec-1,5,main]: TransactionId[399ea06d-67c0-4981-ab17-1c9626efccbc-85]
ERROR: WebAuthnValidator.validateCreateResponse : Error validating response. User handle is 32bf23d8-e834-4429-b27c-751c557d9f19
java.io.UncheckedIOException: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of `java.io.Serializable` (no Creators, like default construct, exist): abstract types either need to be mapped to concrete types, have custom deserializer, or contain additional type information
 at [Source: (ByteArrayInputStream); line: -1, column: 13] (through reference chain: java.util.LinkedHashMap["credProtect"])
        at com.webauthn4j.converter.util.CborConverter.readValue(CborConverter.java:86)
        at com.webauthn4j.converter.AuthenticatorDataConverter.convertToExtensions(AuthenticatorDataConverter.java:145)
        at com.webauthn4j.converter.AuthenticatorDataConverter.convert(AuthenticatorDataConverter.java:122)
        at com.webauthn4j.converter.jackson.deserializer.AuthenticatorDataDeserializer.deserialize(AuthenticatorDataDeserializer.java:51)
        at com.webauthn4j.converter.jackson.deserializer.AuthenticatorDataDeserializer.deserialize(AuthenticatorDataDeserializer.java:33)
        at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:530)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeWithErrorWrapping(BeanDeserializer.java:528)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeUsingPropertyBasedWithExternalTypeId(BeanDeserializer.java:945)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeWithExternalTypeId(BeanDeserializer.java:853)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:324)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:159)
        at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4013)
        at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3091)
        at com.webauthn4j.converter.util.CborConverter.readValue(CborConverter.java:52)
        at com.webauthn4j.converter.AttestationObjectConverter.convert(AttestationObjectConverter.java:70)
        at com.webauthn4j.WebAuthnRegistrationManager.parse(WebAuthnRegistrationManager.java:251)
        at com.webauthn4j.WebAuthnManager.parse(WebAuthnManager.java:222)
        at jp.co.osstech.openam.authentication.modules.webauthn.WebAuthn4JValidatorImpl.validateCreateResponse(WebAuthn4JValidatorImpl.java:100)
        at jp.co.osstech.openam.authentication.modules.webauthn.WebAuthnRegister.storeAuthenticator(WebAuthnRegister.java:245)
        at jp.co.osstech.openam.authentication.modules.webauthn.WebAuthnRegister.process(WebAuthnRegister.java:131)
        at com.sun.identity.authentication.spi.AMLoginModule.wrapProcess(AMLoginModule.java:1056)
        at com.sun.identity.authentication.spi.AMLoginModule.login(AMLoginModule.java:1224)
        ...
Caused by: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of `java.io.Serializable` (no Creators, like default construct, exist): abstract types either need to be mapped to concrete types, have custom deserializer, or contain additional type information
 at [Source: (ByteArrayInputStream); line: -1, column: 13] (through reference chain: java.util.LinkedHashMap["credProtect"])
        at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
        at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1452)
        at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1028)
        at com.fasterxml.jackson.databind.deser.AbstractDeserializer.deserialize(AbstractDeserializer.java:265)
        at com.fasterxml.jackson.databind.DeserializationContext.readValue(DeserializationContext.java:760)
        at com.fasterxml.jackson.databind.DeserializationContext.readValue(DeserializationContext.java:747)
        at com.webauthn4j.converter.jackson.deserializer.UnknownExtensionAuthenticatorOutputDeserializer.deserialize(UnknownExtensionAuthenticatorOutputDeserializer.java:35)
        at com.webauthn4j.converter.jackson.deserializer.UnknownExtensionAuthenticatorOutputDeserializer.deserialize(UnknownExtensionAuthenticatorOutputDeserializer.java:27)
        at com.fasterxml.jackson.databind.DeserializationContext.readValue(DeserializationContext.java:760)
        at com.fasterxml.jackson.databind.DeserializationContext.readValue(DeserializationContext.java:747)
        at com.webauthn4j.converter.jackson.deserializer.ExtensionAuthenticatorOutputDeserializer.deserialize(ExtensionAuthenticatorOutputDeserializer.java:68)
        at com.webauthn4j.converter.jackson.deserializer.ExtensionAuthenticatorOutputDeserializer.deserialize(ExtensionAuthenticatorOutputDeserializer.java:38)
        at com.fasterxml.jackson.databind.deser.std.MapDeserializer._readAndBindStringKeyMap(MapDeserializer.java:527)
        at com.fasterxml.jackson.databind.deser.std.MapDeserializer.deserialize(MapDeserializer.java:364)
        at com.fasterxml.jackson.databind.deser.std.MapDeserializer.deserialize(MapDeserializer.java:29)
        at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1283)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:326)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:159)
        at com.fasterxml.jackson.databind.DeserializationContext.readValue(DeserializationContext.java:760)
        at com.webauthn4j.converter.jackson.deserializer.AuthenticationExtensionsAuthenticatorOutputsEnvelopeDeserializer.deserialize(AuthenticationExtensionsAuthenticatorOutputsEnvelopeDeserializer.java:44)
        at com.webauthn4j.converter.jackson.deserializer.AuthenticationExtensionsAuthenticatorOutputsEnvelopeDeserializer.deserialize(AuthenticationExtensionsAuthenticatorOutputsEnvelopeDeserializer.java:32)
        at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4013)
        at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3077)
        at com.webauthn4j.converter.util.CborConverter.readValue(CborConverter.java:82)
        ... 106 more

@tsujiguchitky
Copy link
Contributor

ResourceAttributeUtilTest.shouldCatchFailureIfDeserialisationFails() fails.

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=96m; support was removed in 8.0
Running TestSuite
Tests run: 552, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 8.005 sec <<< FAILURE! - in TestSuite
shouldCatchFailureIfDeserialisationFails(com.sun.identity.entitlement.xacml3.ResourceAttributeUtilTest)  Time elapsed: 0.011 sec  <<< FAILURE!
org.mockito.exceptions.base.MockitoException: 
Checked exception is invalid for this method!
Invalid: java.io.IOException
	at com.sun.identity.entitlement.xacml3.ResourceAttributeUtilTest.shouldCatchFailureIfDeserialisationFails(ResourceAttributeUtilTest.java:57)


Results :

Failed tests: 
  ResourceAttributeUtilTest.shouldCatchFailureIfDeserialisationFails:57 Mockito ...

Tests run: 552, Failures: 1, Errors: 0, Skipped: 0

@tsujiguchitky
Copy link
Contributor

ResourceAttributeUtilTest.shouldCatchFailureIfDeserialisationFails() fails.

This cause is similar to openam-jp/forgerock-commons#18 (comment).

@tsujiguchitky
Copy link
Contributor

tsujiguchitky commented Jul 6, 2020 •
edited
Loading

This fix requires:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DTonoki @tsujiguchitky