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

[attestation] Missing `jackson-dataformat-cbor` Maven dependency #272

Closed iaik-jheher closed 1 year ago

iaik-jheher commented 1 year ago

FidoMetadataDownloader appears to have an undocumented dependency on com.fasterxml.jackson.dataformat.cbor.CBORFactory, found in the jackson-dataformat-cbor artifact.

If one only adds the webauthn-server-core and webauthn-server-attestation dependency entries to pom.xml, uses of FidoMetadataDownloader will throw NoClassDefFoundError when downloading a certblob:

Exception in thread "main" java.lang.NoClassDefFoundError: com/fasterxml/jackson/dataformat/cbor/CBORFactory
        at com.yubico.fido.metadata.FidoMetadataDownloader.verifyBlob(FidoMetadataDownloader.java:1030)
        at com.yubico.fido.metadata.FidoMetadataDownloader.parseAndVerifyBlob(FidoMetadataDownloader.java:1018)
        at com.yubico.fido.metadata.FidoMetadataDownloader.refreshBlobInternal(FidoMetadataDownloader.java:807)
        at com.yubico.fido.metadata.FidoMetadataDownloader.loadCachedBlob(FidoMetadataDownloader.java:712)
        at toy.Test.main(Test.java:29)
Caused by: java.lang.ClassNotFoundException: com.fasterxml.jackson.dataformat.cbor.CBORFactory
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
        ... 5 more

Adding an explicit dependency on jackson-dataformat-cbor serves as a workaround:

<dependency>
  <groupId>com.fasterxml.jackson.dataformat</groupId>
  <artifactId>jackson-dataformat-cbor</artifactId>
  <version>[2.15,2.16)</version>
  <scope>runtime</scope>
</dependency>

I am using openjdk 17.0.3 2022-04-19 and webauthn-server-attestation-2.4.1-RC1.

emlun commented 1 year ago

That's strange, because that is a transitive dependency from the internal yubico-util module, but that does declare a runtime dependency on jackson-dataformat-cbor. Your dependency resolution tool should be able to resolve this. How are you building the project where you encounter this issue?

I tried to reproduce the issue by adding an integration test to the test-dependent-projects/java-dep-webauthn-server-attestation subproject, which declares a dependency on only webauthn-server-attestation. This subproject is meant to simulate a downstream dependent project, but I do not encounter the issue. I've pushed the change on branch issue-272-cbor-dependency; you can run the test by running ./gradlew :test-dependent-projects:java-dep-webauthn-server-attestation:check. You can also dump a dependency report by running ./gradlew :test-dependent-projects:java-dep-webauthn-server-attestation:dependencies, where you should see jackson-dataformat-cbor among the transitive dependencies.

iaik-jheher commented 1 year ago

I am building using Maven.

Upon review, I am noticing the following output in debug logs:

[WARNING] The POM for com.yubico:yubico-util:jar:2.4.1-RC1 is invalid, transitive dependencies (if any) will not be available: 3 problems were encountered while building the effective model for com.yubico:yubico-util:2.4.1-RC1
[ERROR] 'dependencies.dependency.version' for com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:jar is missing. @
[ERROR] 'dependencies.dependency.version' for com.fasterxml.jackson.datatype:jackson-datatype-jdk8:jar is missing. @
[ERROR] 'dependencies.dependency.version' for com.fasterxml.jackson.datatype:jackson-datatype-jsr310:jar is missing. @

This sounds like it would be related to this issue. (Apologies for missing the output when initially filing the issue.)

emlun commented 1 year ago

Oh, interesting. Does this issue also appear with version 2.4.0? (You might encounter different issues instead, though).

iaik-jheher commented 1 year ago

No, the issue does not appear to occur in 2.4.0.

emlun commented 1 year ago

I see. This is likely related to this change, then. I'll investigate if we're importing the Jackson BOM incorrectly.

emlun commented 1 year ago

Pre-release 2.4.1-RC2 is now available on Maven Central. Could you test to see if the issue persists with that version?

iaik-jheher commented 1 year ago

The issue still occurs on 2.4.1-RC2.

[WARNING] The POM for com.yubico:webauthn-server-core:jar:2.4.1-RC2 is invalid, transitive dependencies (if any) will not be available: 2 problems were encountered while building the effective model for com.yubico:webauthn-server-core:2.4.1-RC2
[ERROR] 'dependencyManagement.dependencies.dependency.version' for com.fasterxml.jackson:jackson-bom:pom is missing. @
[ERROR] 'dependencies.dependency.version' for com.fasterxml.jackson.core:jackson-databind:jar is missing. @

[WARNING] The POM for com.yubico:webauthn-server-attestation:jar:2.4.1-RC2 is invalid, transitive dependencies (if any) will not be available: 2 problems were encountered while building the effective model for com.yubico:webauthn-server-attestation:2.4.1-RC2
[ERROR] 'dependencyManagement.dependencies.dependency.version' for com.fasterxml.jackson:jackson-bom:pom is missing. @
[ERROR] 'dependencies.dependency.version' for com.fasterxml.jackson.core:jackson-databind:jar is missing. @
emlun commented 1 year ago

Ok, how about 2.4.1-RC3? Thank you for your patience!

iaik-jheher commented 1 year ago

Still fails, though with a new error:

org.eclipse.aether.collection.DependencyCollectionException: Failed to collect dependencies at com.yubico:webauthn-server-core:jar:2.4.1-RC3
Caused by: org.eclipse.aether.resolution.ArtifactDescriptorException: Failed to read artifact descriptor for com.yubico:webauthn-server-core:jar:2.4.1-RC3
Caused by: org.apache.maven.model.resolution.UnresolvableModelException: Could not find artifact com.fasterxml.jackson:jackson-bom:pom:[2.13.2.1,3) in maven-central (https://repo.maven.apache.org/maven2/)
emlun commented 1 year ago

Ok, in 2.4.1-RC4 I've restored the related settings to essentially what they were in 2.4.0. Please take one more look!

iaik-jheher commented 1 year ago

2.4.1-RC4 parses correctly, and includes the necessary dependencies. Thanks for the fix!

emlun commented 1 year ago

Great, thanks for confirming! I'll go ahead and promote that to the proper 2.4.1 release later today.

emlun commented 1 year ago

Release 2.4.1 is available now on Maven Central. Thanks again!