diff --git a/.gitmodules b/.gitmodules index ba6ab205f..e69de29bb 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +0,0 @@ -[submodule "lombok"] - path = lombok - url = https://github.com/emlun/lombok.git - branch = builder-javadoc diff --git a/.travis.yml b/.travis.yml index 6b6f7219c..2a461383d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,13 +15,24 @@ jdk: script: - ./gradlew check assembleJavadoc -after_success: - - ./gradlew coveralls +stages: + - test + - mutation-test + - deploy -deploy: - provider: pages - skip-cleanup: true - github-token: $PAGES_DEPLOY_KEY - on: - branch: master - local-dir: 'build/javadoc' +jobs: + include: + - stage: mutation-test + jdk: oraclejdk8 + script: ./gradlew pitest coveralls + + - stage: deploy + jdk: oraclejdk8 + script: ./gradlew assembleJavadoc + deploy: + provider: pages + skip-cleanup: true + github-token: $PAGES_DEPLOY_KEY + on: + branch: master + local-dir: 'build/javadoc' diff --git a/NEWS b/NEWS index 369fd40f7..bce20dc26 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,13 @@ +== Version 1.0.1 == + +Bugfixes: + +* Registration no longer fails for unimplemented attestation statement formats + if `allowUnknownAttestation` is set to `true`. + ** Registration still fails for attestation statement formats not defined in + the WebAuthn Level 1 spec. + + == Version 1.0.0 == * Fixed URL in artifact POM diff --git a/README b/README index 29026462f..b30d6f92d 100644 --- a/README +++ b/README @@ -27,7 +27,7 @@ Maven: com.yubico webauthn-server-core - 1.0.0-RC2 + 1.0.0 compile ---------- @@ -35,7 +35,7 @@ Maven: Gradle: ---------- -compile 'com.yubico:webauthn-server-core:1.0.0-RC2' +compile 'com.yubico:webauthn-server-core:1.0.0' ---------- @@ -60,6 +60,20 @@ and other higher level concepts can make use of this authentication mechanism, but the authentication mechanism alone does not make a security system. +== Known issues + +- In the link:webauthn-server-demo[example app], authentication does not work in + Chrome as of version 70. This is due to a + link:https://bugs.chromium.org/p/chromium/issues/detail?id=847878[bug in + Chrome] which will not be worked around here. To work around this in + application code, you can omit the + link:https://yubico.github.io/java-webauthn-server/webauthn-server-core/com/yubico/webauthn/data/AuthenticatorAssertionResponse.AuthenticatorAssertionResponseBuilder.html#userHandle-java.util.Optional[`userHandle`] + when constructing an + link:https://yubico.github.io/java-webauthn-server/webauthn-server-core/com/yubico/webauthn/data/AuthenticatorAssertionResponse.html[`AuthenticatorAssertionResponse`] + value if the `userHandle` is empty. See + https://github.com/Yubico/java-webauthn-server/issues/12 . + + == Documentation See the @@ -204,14 +218,14 @@ effects, and does not directly interact with any database. This means it is database agnostic and thread safe. The following diagram illustrates an example architecture for an application using the library. -image::https://raw.githubusercontent.com/Yubico/java-webauthn-server/master/docs/img/demo-architecture.svg["Example application architecture",align="center"] +image::https://raw.githubusercontent.com/Yubico/java-webauthn-server/master/docs/img/demo-architecture.svg?sanitize=true["Example application architecture",align="center"] The application manages all state and database access, and communicates with the library via POJO representations of requests and responses. The following diagram illustrates the data flow during a WebAuthn registration or authentication ceremony. -image::https://raw.githubusercontent.com/Yubico/java-webauthn-server/master/docs/img/demo-sequence-diagram.svg["WebAuthn ceremony sequence diagram",align="center"] +image::https://raw.githubusercontent.com/Yubico/java-webauthn-server/master/docs/img/demo-sequence-diagram.svg?sanitize=true["WebAuthn ceremony sequence diagram",align="center"] In this diagram, the *Client* is the user's browser and the application's client-side scripts. The *Server* is the application and its business logic, the diff --git a/build.gradle b/build.gradle index 1e0a7c987..137da416f 100644 --- a/build.gradle +++ b/build.gradle @@ -51,15 +51,8 @@ allprojects { targetCompatibility = 1.8 lombok { - version '1.18.4' - sha256 = '39f3922deb679b1852af519eb227157ef2dd0a21eec3542c8ce1b45f2df39742' - } - configurations.all { - resolutionStrategy { - dependencySubstitution { - substitute module('org.projectlombok:lombok') with module('com.yubico:lombok:1.18.5-custom') - } - } + version '1.18.6' + sha256 = '6373d9ade79efdc028cd48d40a9af9ac6a090dbcfaec55b438ec49556a4e92fb' } tasks.withType(JavaCompile) { @@ -73,7 +66,6 @@ allprojects { repositories { mavenLocal() - maven { url uri("${rootProject.projectDir}/lib") } maven { url "http://repo.maven.apache.org/maven2" } } diff --git a/lib/com/yubico/lombok/1.18.5-custom/README.md b/lib/com/yubico/lombok/1.18.5-custom/README.md deleted file mode 100644 index 6db5d96d1..000000000 --- a/lib/com/yubico/lombok/1.18.5-custom/README.md +++ /dev/null @@ -1,17 +0,0 @@ -Custom Lombok build that also copies javadoc from field definitions to builder -setters. - -Build using the `lombok` submodule. Building Lombok requires JDK 10, therefore -it is not integrated directly into the Gradle build. It is built as such: - -``` -$ git submodule update --init -$ cd lombok -$ ant setupJavaOracle8TestEnvironment -$ rm ../lombok.config -$ ant test -$ ant dist -$ cp dist/lombok-1.18.5.jar ../lib/com/yubico/lombok/1.18.5-custom/lombok-1.18.5-custom.jar -$ cd .. -$ git checkout -- lombok.config -``` diff --git a/lib/com/yubico/lombok/1.18.5-custom/lombok-1.18.5-custom.jar b/lib/com/yubico/lombok/1.18.5-custom/lombok-1.18.5-custom.jar deleted file mode 100644 index 6dbc50dc4..000000000 Binary files a/lib/com/yubico/lombok/1.18.5-custom/lombok-1.18.5-custom.jar and /dev/null differ diff --git a/lombok b/lombok deleted file mode 160000 index 2a2350f08..000000000 --- a/lombok +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 2a2350f0803b7016515f07b7a24df620689ea95a diff --git a/webauthn-server-core/build.gradle b/webauthn-server-core/build.gradle index 6413b8dd3..e17ab63dd 100644 --- a/webauthn-server-core/build.gradle +++ b/webauthn-server-core/build.gradle @@ -35,7 +35,7 @@ jar { manifest { attributes([ 'Specification-Title': 'Web Authentication: An API for accessing Public Key Credentials', - 'Specification-Version': 'Level 1 Candidate Recommendation 2018-03-20', + 'Specification-Version': 'Level 1 Proposed Recommendation 2019-01-17', 'Specification-Vendor': 'World Wide Web Consortium', 'Implementation-Id': 'java-webauthn-server', 'Implementation-Title': 'Yubico Web Authentication server library', diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishRegistrationSteps.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishRegistrationSteps.java index ae780fb0d..ee7cd0a0e 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishRegistrationSteps.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/FinishRegistrationSteps.java @@ -53,7 +53,6 @@ import lombok.extern.slf4j.Slf4j; import static com.yubico.internal.util.ExceptionUtil.assure; -import static com.yubico.webauthn.data.AttestationType.NONE; @Builder @Slf4j @@ -373,24 +372,18 @@ class Step13 implements Step { private final List prevWarnings; @Override - public void validate() { - assure(formatSupported(), "Unsupported attestation statement format: %s", format()); - } + public void validate() {} @Override public Step14 nextStep() { - return new Step14(clientDataJsonHash, attestation, attestationStatementVerifier().get(), allWarnings()); + return new Step14(clientDataJsonHash, attestation, attestationStatementVerifier(), allWarnings()); } public String format() { return attestation.getFormat(); } - public boolean formatSupported() { - return attestationStatementVerifier().isPresent(); - } - - private Optional attestationStatementVerifier() { + public Optional attestationStatementVerifier() { switch (format()) { case "fido-u2f": return Optional.of(new FidoU2fAttestationStatementVerifier()); @@ -410,15 +403,19 @@ private Optional attestationStatementVerifier() { class Step14 implements Step { private final ByteArray clientDataJsonHash; private final AttestationObject attestation; - private final AttestationStatementVerifier attestationStatementVerifier; + private final Optional attestationStatementVerifier; private final List prevWarnings; @Override public void validate() { - assure( - attestationStatementVerifier.verifyAttestationSignature(attestation, clientDataJsonHash), - "Invalid attestation signature." - ); + attestationStatementVerifier.ifPresent(verifier -> { + assure( + verifier.verifyAttestationSignature(attestation, clientDataJsonHash), + "Invalid attestation signature." + ); + }); + + assure(attestationType() != null, "Failed to determine attestation type"); } @Override @@ -428,18 +425,40 @@ public Step15 nextStep() { public AttestationType attestationType() { try { - return attestationStatementVerifier.getAttestationType(attestation); + if (attestationStatementVerifier.isPresent()) { + return attestationStatementVerifier.get().getAttestationType(attestation); + } else { + switch (attestation.getFormat()) { + case "android-key": + // TODO delete this once android-key attestation verification is implemented + return AttestationType.BASIC; + case "tpm": + // TODO delete this once tpm attestation verification is implemented + if (attestation.getAttestationStatement().has("x5c")) { + return AttestationType.ATTESTATION_CA; + } else { + return AttestationType.ECDAA; + } + default: + throw new IllegalArgumentException("Failed to resolve attestation type; unknown attestation statement format: " + attestation.getFormat()); + } + } } catch (IOException | CoseException | CertificateException e) { throw new IllegalArgumentException("Failed to resolve attestation type.", e); } } public Optional> attestationTrustPath() { - if (attestationStatementVerifier instanceof X5cAttestationStatementVerifier) { - try { - return ((X5cAttestationStatementVerifier) attestationStatementVerifier).getAttestationTrustPath(attestation); - } catch (CertificateException e) { - throw new IllegalArgumentException("Failed to resolve attestation trust path.", e); + if (attestationStatementVerifier.isPresent()) { + AttestationStatementVerifier verifier = attestationStatementVerifier.get(); + if (verifier instanceof X5cAttestationStatementVerifier) { + try { + return ((X5cAttestationStatementVerifier) verifier).getAttestationTrustPath(attestation); + } catch (CertificateException e) { + throw new IllegalArgumentException("Failed to resolve attestation trust path.", e); + } + } else { + return Optional.empty(); } } else { return Optional.empty(); @@ -456,10 +475,6 @@ class Step15 implements Step { @Override public void validate() { - assure( - attestationType == AttestationType.SELF_ATTESTATION || attestationType == NONE || trustResolver().isPresent(), - "Failed to obtain attestation trust anchors." - ); } @Override @@ -504,6 +519,11 @@ class Step16 implements Step { @Override public void validate() { + assure( + trustResolver.isPresent() || allowUntrustedAttestation, + "Failed to obtain attestation trust anchors." + ); + switch (attestationType) { case SELF_ATTESTATION: assure(allowUntrustedAttestation, "Self attestation is not allowed."); diff --git a/webauthn-server-core/src/main/java/com/yubico/webauthn/RelyingParty.java b/webauthn-server-core/src/main/java/com/yubico/webauthn/RelyingParty.java index 383d8d220..bbe7dc034 100644 --- a/webauthn-server-core/src/main/java/com/yubico/webauthn/RelyingParty.java +++ b/webauthn-server-core/src/main/java/com/yubico/webauthn/RelyingParty.java @@ -231,6 +231,12 @@ public class RelyingParty { * attestation and none attestation. * *

+ * Regardless of the value of this option, invalid attestation statements of supported formats will always be + * rejected. For example, a "packed" attestation statement with an invalid signature will be rejected even if this + * option is set to true. + *

+ * + *

* The default is true. *

*/ diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala index 134908148..f38062130 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RelyingPartyRegistrationSpec.scala @@ -30,6 +30,7 @@ import java.nio.charset.Charset import java.security.MessageDigest import java.security.KeyPair import java.security.PrivateKey +import java.security.SignatureException import java.security.cert.X509Certificate import java.util.Optional @@ -52,6 +53,7 @@ import com.yubico.webauthn.data.ByteArray import com.yubico.webauthn.data.RegistrationExtensionInputs import com.yubico.webauthn.data.PublicKeyCredentialDescriptor import com.yubico.webauthn.data.UserIdentity +import com.yubico.webauthn.data.AttestationConveyancePreference import com.yubico.webauthn.data.Generators._ import com.yubico.webauthn.test.Util.toStepWithUtilities import javax.security.auth.x500.X500Principal @@ -570,43 +572,63 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD ) } - def checkFailure(format: String): Unit = { - it(s"""Fails if fmt is "${format}".""") { + def checkUnknown(format: String): Unit = { + it(s"""Returns no known attestation statement verifier if fmt is "${format}".""") { val steps = setup(format) val step: FinishRegistrationSteps#Step13 = steps.begin.next.next.next.next.next.next.next.next.next.next.next.next - step.validations shouldBe a [Failure[_]] - step.validations.failed.get shouldBe an [IllegalArgumentException] - step.tryNext shouldBe a [Failure[_]] + step.validations shouldBe a [Success[_]] + step.tryNext shouldBe a [Success[_]] + step.format should equal (format) + step.attestationStatementVerifier.asScala shouldBe empty } } - def checkSuccess(format: String): Unit = { - it(s"""Succeeds if fmt is "${format}".""") { + def checkKnown(format: String): Unit = { + it(s"""Returns a known attestation statement verifier if fmt is "${format}".""") { val steps = setup(format) val step: FinishRegistrationSteps#Step13 = steps.begin.next.next.next.next.next.next.next.next.next.next.next.next step.validations shouldBe a [Success[_]] step.tryNext shouldBe a [Success[_]] step.format should equal (format) - step.formatSupported should be(true) + step.attestationStatementVerifier.asScala should not be empty } } - ignore("Succeeds if fmt is android-key.") { checkSuccess("android-key") } - ignore("Succeeds if fmt is android-safetynet.") { checkSuccess("android-safetynet") } - checkSuccess("fido-u2f") - checkSuccess("none") - checkSuccess("packed") - ignore("Succeeds if fmt is tpm.") { checkSuccess("tpm") } + checkKnown("android-safetynet") + checkKnown("fido-u2f") + checkKnown("none") + checkKnown("packed") - checkFailure("FIDO-U2F") - checkFailure("Fido-U2F") - checkFailure("bleurgh") + checkUnknown("android-key") + checkUnknown("tpm") + + checkUnknown("FIDO-U2F") + checkUnknown("Fido-U2F") + checkUnknown("bleurgh") } describe("14. Verify that attStmt is a correct attestation statement, conveying a valid attestation signature, by using the attestation statement format fmt’s verification procedure given attStmt, authData and the hash of the serialized client data computed in step 7.") { + describe("If allowUntrustedAttestation is set,") { + it("a fido-u2f attestation is still rejected if invalid.") { + val testData = RegistrationTestData.FidoU2f.BasicAttestation.editAttestationObject("attStmt", { attStmtNode: JsonNode => + attStmtNode.asInstanceOf[ObjectNode] + .set("sig", jsonFactory.binaryNode(Array(0, 0, 0, 0))) + }) + val steps = finishRegistration( + testData = testData, + allowUntrustedAttestation = true + ) + val step: FinishRegistrationSteps#Step14 = steps.begin.next.next.next.next.next.next.next.next.next.next.next.next.next + + step.validations shouldBe a [Failure[_]] + step.validations.failed.get.getCause shouldBe a [SignatureException] + step.tryNext shouldBe a [Failure[_]] + } + } + describe("For the fido-u2f statement format,") { it("the default test case is a valid basic attestation.") { val steps = finishRegistration(testData = RegistrationTestData.FidoU2f.BasicAttestation) @@ -632,7 +654,7 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD val step: FinishRegistrationSteps#Step14 = new steps.Step14( new BouncyCastleCrypto().hash(new ByteArray(testData.clientDataJsonBytes.getBytes.updated(20, (testData.clientDataJsonBytes.getBytes()(20) + 1).toByte))), new AttestationObject(testData.attestationObject), - new FidoU2fAttestationStatementVerifier, + Some(new FidoU2fAttestationStatementVerifier).asJava, Nil.asJava ) @@ -651,7 +673,7 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD val step: FinishRegistrationSteps#Step14 = new steps.Step14( new BouncyCastleCrypto().hash(testData.clientDataJsonBytes), new AttestationObject(testData.attestationObject), - new FidoU2fAttestationStatementVerifier, + Some(new FidoU2fAttestationStatementVerifier).asJava, Nil.asJava ) @@ -683,7 +705,7 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD val step: FinishRegistrationSteps#Step14 = new steps.Step14( new BouncyCastleCrypto().hash(testData.clientDataJsonBytes), new AttestationObject(testData.attestationObject), - new FidoU2fAttestationStatementVerifier, + Some(new FidoU2fAttestationStatementVerifier).asJava, Nil.asJava ) @@ -778,7 +800,7 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD val step: FinishRegistrationSteps#Step14 = new steps.Step14( new BouncyCastleCrypto().hash(testData.clientDataJsonBytes), new AttestationObject(testData.attestationObject), - new NoneAttestationStatementVerifier, + Some(new NoneAttestationStatementVerifier).asJava, Nil.asJava ) @@ -809,7 +831,7 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD val steps = finishRegistration(testData = RegistrationTestData.Packed.BasicAttestation) val step: FinishRegistrationSteps#Step14 = steps.begin.next.next.next.next.next.next.next.next.next.next.next.next.next - step.getAttestationStatementVerifier shouldBe a [PackedAttestationStatementVerifier] + step.getAttestationStatementVerifier.get shouldBe a [PackedAttestationStatementVerifier] } describe("the verification procedure is:") { @@ -1188,12 +1210,12 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD it("the attestation statement verifier implementation is AndroidSafetynetAttestationStatementVerifier.") { val steps = finishRegistration( testData = defaultTestData, - allowUntrustedAttestation = true, + allowUntrustedAttestation = false, rp = defaultTestData.rpId ) val step: FinishRegistrationSteps#Step14 = steps.begin.next.next.next.next.next.next.next.next.next.next.next.next.next - step.getAttestationStatementVerifier shouldBe an [AndroidSafetynetAttestationStatementVerifier] + step.getAttestationStatementVerifier.get shouldBe an [AndroidSafetynetAttestationStatementVerifier] } describe("the verification procedure is:") { @@ -1306,6 +1328,16 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD step.validations shouldBe a [Success[_]] step.tryNext shouldBe a [Success[_]] } + + it("Unknown attestation statement formats fail.") { + val steps = finishRegistration(testData = RegistrationTestData.FidoU2f.BasicAttestation.editAttestationObject("fmt", "urgel")) + val step: FinishRegistrationSteps#Step14 = steps.begin.next.next.next.next.next.next.next.next.next.next.next.next.next + + step.validations shouldBe a [Failure[_]] + step.validations.failed.get shouldBe an [IllegalArgumentException] + step.tryNext shouldBe a [Failure[_]] + } + } describe("15. If validation is successful, obtain a list of acceptable trust anchors (attestation root certificates or ECDAA-Issuer public keys) for that attestation type and attestation statement format fmt, from a trusted source or from policy. For example, the FIDO Metadata Service [FIDOMetadataService] provides one way to obtain such information, using the aaguid in the attestedCredentialData in authData.") { @@ -1362,7 +1394,6 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD step.tryNext shouldBe a [Success[_]] } } - } describe("16. Assess the attestation trustworthiness using the outputs of the verification procedure in step 14, as follows:") { @@ -1602,9 +1633,41 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD steps.run.isAttestationTrusted should be (false) } + it("The test case with unknown attestation fails.") { + val testData = RegistrationTestData.FidoU2f.BasicAttestation.editAttestationObject("fmt", "urgel") + val steps = finishRegistration( + testData = testData, + allowUntrustedAttestation = true, + credentialRepository = Some(emptyCredentialRepository) + ) + val result = Try(steps.run) + result.failed.get shouldBe an [IllegalArgumentException] + } + describe("NOTE: However, if permitted by policy, the Relying Party MAY register the credential ID and credential public key but treat the credential as one with self attestation (see §6.4.3 Attestation Types). If doing so, the Relying Party is asserting there is no cryptographic proof that the public key credential has been generated by a particular authenticator model. See [FIDOSecRef] and [UAFProtocol] for a more detailed discussion.") { it("Nothing to test.") {} } + + def testUntrusted(testData: RegistrationTestData): Unit = { + val fmt = new AttestationObject(testData.attestationObject).getFormat + it(s"""A test case with good "${fmt}" attestation but no metadata service succeeds, but reports attestation as not trusted.""") { + val testData = RegistrationTestData.FidoU2f.BasicAttestation + val steps = finishRegistration( + testData = testData, + metadataService = None, + allowUntrustedAttestation = true, + credentialRepository = Some(emptyCredentialRepository) + ) + steps.run.getKeyId.getId should be (testData.response.getId) + steps.run.isAttestationTrusted should be (false) + } + } + + testUntrusted(RegistrationTestData.AndroidKey.BasicAttestation) + testUntrusted(RegistrationTestData.AndroidSafetynet.BasicAttestation) + testUntrusted(RegistrationTestData.FidoU2f.BasicAttestation) + testUntrusted(RegistrationTestData.NoneAttestation.Default) + testUntrusted(RegistrationTestData.Tpm.PrivacyCa) } it("(Deleted) If verification of the attestation statement failed, the Relying Party MUST fail the registration ceremony.") {