From d5d07bb9f89aca635c3db00e5cee569be9c25f3c Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Wed, 10 Apr 2019 14:52:17 +0200 Subject: [PATCH 1/4] Base bogus TPM attestation test data off basic instead of self attestation --- .../test/scala/com/yubico/webauthn/RegistrationTestData.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RegistrationTestData.scala b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RegistrationTestData.scala index 6bfc8d435..c414ddabc 100644 --- a/webauthn-server-core/src/test/scala/com/yubico/webauthn/RegistrationTestData.scala +++ b/webauthn-server-core/src/test/scala/com/yubico/webauthn/RegistrationTestData.scala @@ -171,7 +171,7 @@ object RegistrationTestData { ) { override def regenerate() = TestAuthenticator.createSelfAttestedCredential(attestationStatementFormat = "packed", alg = Some(COSEAlgorithmIdentifier.RS256)) } } object Tpm { - val PrivacyCa: RegistrationTestData = Packed.SelfAttestation.editAttestationObject("fmt", "tpm") + val PrivacyCa: RegistrationTestData = Packed.BasicAttestation.editAttestationObject("fmt", "tpm") } } From 582b645dfd0c304b22cb4bbe466d33b76fdf3b0e Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Wed, 10 Apr 2019 14:49:25 +0200 Subject: [PATCH 2/4] Test that TPM attestations don't throw exception if untrusted is okay --- .../RelyingPartyRegistrationSpec.scala | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) 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 f38062130..76fe01fdb 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 @@ -1687,19 +1687,19 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD describe("The default RelyingParty settings") { - it("accept registrations with no attestation.") { - val rp = RelyingParty.builder() - .identity(RelyingPartyIdentity.builder().id("localhost").name("Test party").build()) - .credentialRepository(emptyCredentialRepository) - .build() - - val request = rp.startRegistration(StartRegistrationOptions.builder() - .user(UserIdentity.builder().name("test").displayName("Test Testsson").id(new ByteArray(Array())).build()) - .build() - ).toBuilder() - .challenge(RegistrationTestData.NoneAttestation.Default.clientData.getChallenge) - .build() + val rp = RelyingParty.builder() + .identity(RelyingPartyIdentity.builder().id("localhost").name("Test party").build()) + .credentialRepository(emptyCredentialRepository) + .build() + + val request = rp.startRegistration(StartRegistrationOptions.builder() + .user(UserIdentity.builder().name("test").displayName("Test Testsson").id(new ByteArray(Array())).build()) + .build() + ).toBuilder() + .challenge(RegistrationTestData.NoneAttestation.Default.clientData.getChallenge) + .build() + it("accept registrations with no attestation.") { val result = rp.finishRegistration(FinishRegistrationOptions.builder() .request(request) .response(RegistrationTestData.NoneAttestation.Default.response) @@ -1710,6 +1710,17 @@ class RelyingPartyRegistrationSpec extends FunSpec with Matchers with GeneratorD result.getKeyId.getId should equal (RegistrationTestData.NoneAttestation.Default.response.getId) } + it("accept TPM attestations but reports they're untrusted.") { + val result = rp.finishRegistration(FinishRegistrationOptions.builder() + .request(request) + .response(RegistrationTestData.Tpm.PrivacyCa.response) + .build() + ) + + result.isAttestationTrusted should be (false) + result.getKeyId.getId should equal (RegistrationTestData.Tpm.PrivacyCa.response.getId) + } + } } From badc4630b4a354edfb8a659168d2dd72850ec7d3 Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Tue, 9 Apr 2019 13:41:45 +0200 Subject: [PATCH 3/4] Handle ATTESTATION_CA attestation type the same way as BASIC --- .../main/java/com/yubico/webauthn/FinishRegistrationSteps.java | 3 +++ 1 file changed, 3 insertions(+) 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 ee7cd0a0e..69de4e4f6 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 @@ -487,6 +487,7 @@ public Optional trustResolver() { case SELF_ATTESTATION: return Optional.empty(); + case ATTESTATION_CA: case BASIC: switch (attestation.getFormat()) { case "android-safetynet": @@ -529,6 +530,7 @@ public void validate() { assure(allowUntrustedAttestation, "Self attestation is not allowed."); break; + case ATTESTATION_CA: case BASIC: assure(allowUntrustedAttestation || attestationTrusted(), "Failed to derive trust for attestation key."); break; @@ -553,6 +555,7 @@ public boolean attestationTrusted() { case NONE: return false; + case ATTESTATION_CA: case BASIC: return attestationMetadata().filter(Attestation::isTrusted).isPresent(); default: From fbdaa5179a866be3e9bdfad054134c495e2dd6ae Mon Sep 17 00:00:00 2001 From: Emil Lundberg Date: Wed, 10 Apr 2019 15:00:34 +0200 Subject: [PATCH 4/4] Trust-resolve ATTESTATION_CA TPM attestations using metadataService --- .../main/java/com/yubico/webauthn/FinishRegistrationSteps.java | 1 + 1 file changed, 1 insertion(+) 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 69de4e4f6..5a085f16d 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 @@ -493,6 +493,7 @@ public Optional trustResolver() { case "android-safetynet": case "fido-u2f": case "packed": + case "tpm": return metadataService.map(KnownX509TrustAnchorsTrustResolver::new); default: throw new UnsupportedOperationException(String.format(