From fe5ed9c378aa608ba256e40ca6bf02f457352d30 Mon Sep 17 00:00:00 2001 From: Adam Velebil Date: Tue, 16 Jan 2024 09:09:13 +0100 Subject: [PATCH] MC/GA UserVerificationRequirement.DISCOURAGED --- .../fido/client/BasicWebAuthnClient.java | 13 +- .../fido/UvDiscouragedInstrumentedTests.java | 55 +++++++ .../fido/BasicWebAuthnClientTests.java | 134 +++++++++++++++++- 3 files changed, 196 insertions(+), 6 deletions(-) create mode 100644 testing-android/src/androidTest/java/com/yubico/yubikit/testing/fido/UvDiscouragedInstrumentedTests.java diff --git a/fido/src/main/java/com/yubico/yubikit/fido/client/BasicWebAuthnClient.java b/fido/src/main/java/com/yubico/yubikit/fido/client/BasicWebAuthnClient.java index d776a2b7..2f2475b5 100644 --- a/fido/src/main/java/com/yubico/yubikit/fido/client/BasicWebAuthnClient.java +++ b/fido/src/main/java/com/yubico/yubikit/fido/client/BasicWebAuthnClient.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2020-2023 Yubico. + * Copyright (C) 2020-2024 Yubico. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ import com.yubico.yubikit.core.application.CommandException; import com.yubico.yubikit.core.application.CommandState; import com.yubico.yubikit.core.fido.CtapException; +import com.yubico.yubikit.core.internal.Logger; import com.yubico.yubikit.fido.ctap.ClientPin; import com.yubico.yubikit.fido.ctap.CredentialManagement; import com.yubico.yubikit.fido.ctap.Ctap2Session; @@ -187,6 +188,7 @@ public PublicKeyCredential makeCredential( if (e.getCtapError() == CtapException.ERR_PIN_INVALID) { throw new PinInvalidClientError(e, clientPin.getPinRetries().getCount()); } + Logger.debug(logger, "makeCredential CTAP error: {}", String.format("0x%02x", e.getCtapError())); throw ClientError.wrapCtapException(e); } } @@ -399,7 +401,7 @@ protected Ctap2Session.CredentialData ctapMakeCredential( } byte[] pinUvAuthParam = null; - int pinUvAuthProtocol = 0; + @Nullable Integer pinUvAuthProtocol = null; Map ctapOptions = new HashMap<>(); AuthenticatorSelectionCriteria authenticatorSelection = @@ -430,7 +432,9 @@ protected Ctap2Session.CredentialData ctapMakeCredential( pinToken = clientPin.getUvToken(ClientPin.PIN_PERMISSION_MC, rpId, null); pinUvAuthParam = clientPin.getPinUvAuth().authenticate(pinToken, clientDataHash); pinUvAuthProtocol = clientPin.getPinUvAuth().getVersion(); - } else if (pinConfigured && !ctapOptions.containsKey(OPTION_USER_VERIFICATION)) { + } else if (pinConfigured && Boolean.TRUE.equals(ctapOptions.get(OPTION_RESIDENT_KEY))) { + // the authenticator supports pin and a discoverable credential creation has been + // requested, but no PIN was provided throw new PinRequiredClientError(); } @@ -520,7 +524,7 @@ protected List ctapGetAssertions( } byte[] pinUvAuthParam = null; - int pinUvAuthProtocol = 0; + @Nullable Integer pinUvAuthProtocol = null; byte[] pinToken = null; try { if (pin != null) { @@ -551,6 +555,7 @@ protected List ctapGetAssertions( if (e.getCtapError() == CtapException.ERR_PIN_INVALID) { throw new PinInvalidClientError(e, clientPin.getPinRetries().getCount()); } + Logger.debug(logger, "getAssertion CTAP error: {}", String.format("0x%02x", e.getCtapError())); throw ClientError.wrapCtapException(e); } finally { if (pinToken != null) { diff --git a/testing-android/src/androidTest/java/com/yubico/yubikit/testing/fido/UvDiscouragedInstrumentedTests.java b/testing-android/src/androidTest/java/com/yubico/yubikit/testing/fido/UvDiscouragedInstrumentedTests.java new file mode 100644 index 00000000..bf74790a --- /dev/null +++ b/testing-android/src/androidTest/java/com/yubico/yubikit/testing/fido/UvDiscouragedInstrumentedTests.java @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2024 Yubico. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.yubico.yubikit.testing.fido; + +import androidx.test.filters.LargeTest; + +import com.yubico.yubikit.fido.client.ClientError; +import com.yubico.yubikit.fido.ctap.Ctap2Session; +import com.yubico.yubikit.testing.framework.FidoInstrumentedTests; + +import org.junit.Test; + +@LargeTest +public class UvDiscouragedInstrumentedTests extends FidoInstrumentedTests { + + static boolean hasPin(Ctap2Session session) { + final Ctap2Session.InfoData info = session.getCachedInfo(); + return Boolean.TRUE.equals(info.getOptions().get("clientPin")); + } + + @Test + public void testMakeCredentialGetAssertion() throws Throwable { + withCtap2Session( + "This device has a PIN set", + (device, session) -> !hasPin(session), + BasicWebAuthnClientTests::testUvDiscouragedMakeCredentialGetAssertion); + } + + + /** + * Run this test only on devices with PIN set + * this is expected to fail with 0x36 + */ + @Test(expected = ClientError.class) + public void testMakeCredentialGetAssertionOnProtected() throws Throwable { + withCtap2Session( + "This device has no PIN set", + (device, session) -> hasPin(session), + BasicWebAuthnClientTests::testUvDiscouragedMakeCredentialGetAssertion); + } +} diff --git a/testing/src/main/java/com/yubico/yubikit/testing/fido/BasicWebAuthnClientTests.java b/testing/src/main/java/com/yubico/yubikit/testing/fido/BasicWebAuthnClientTests.java index 4c259f30..c71f29ab 100644 --- a/testing/src/main/java/com/yubico/yubikit/testing/fido/BasicWebAuthnClientTests.java +++ b/testing/src/main/java/com/yubico/yubikit/testing/fido/BasicWebAuthnClientTests.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2020-2023 Yubico. + * Copyright (C) 2020-2024 Yubico. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,6 +23,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -46,6 +47,7 @@ import com.yubico.yubikit.fido.webauthn.PublicKeyCredentialType; import com.yubico.yubikit.fido.webauthn.PublicKeyCredentialUserEntity; import com.yubico.yubikit.fido.webauthn.ResidentKeyRequirement; +import com.yubico.yubikit.fido.webauthn.UserVerificationRequirement; import java.io.IOException; import java.nio.ByteBuffer; @@ -149,6 +151,119 @@ public static void testMakeCredentialGetAssertion( deleteCredentials(webauthn, deleteCredIds); } + public static void testUvDiscouragedMakeCredentialGetAssertion( + Ctap2Session session, + Object... unusedArgs) throws Throwable { + + BasicWebAuthnClient webauthn = new BasicWebAuthnClient(session); + + // Test non rk credential + PublicKeyCredentialCreationOptions creationOptionsNonRk = getCreateOptions( + new PublicKeyCredentialUserEntity( + "user", + "user".getBytes(StandardCharsets.UTF_8), + "User" + ), + false, + Collections.singletonList(TestData.PUB_KEY_CRED_PARAMS_ES256), + null, + UserVerificationRequirement.DISCOURAGED + ); + PublicKeyCredential credNonRk = webauthn.makeCredential( + TestData.CLIENT_DATA_JSON_CREATE, + creationOptionsNonRk, + Objects.requireNonNull(creationOptionsNonRk.getRp().getId()), + null, + null, + null + ); + + AuthenticatorAttestationResponse responseNonRk = (AuthenticatorAttestationResponse) credNonRk.getResponse(); + assertNotNull("Failed to make non resident key credential", responseNonRk); + assertNotNull("Credential missing attestation object", responseNonRk.getAttestationObject()); + assertNotNull("Credential missing client data JSON", responseNonRk.getClientDataJson()); + + // Get assertions + PublicKeyCredentialRequestOptions requestOptionsNonRk = new PublicKeyCredentialRequestOptions( + TestData.CHALLENGE, + (long) 90000, + TestData.RP_ID, + Collections.singletonList(new PublicKeyCredentialDescriptor(credNonRk.getType(), credNonRk.getRawId())), + UserVerificationRequirement.DISCOURAGED, + null + ); + + try { + PublicKeyCredential credential = webauthn.getAssertion( + TestData.CLIENT_DATA_JSON_GET, + requestOptionsNonRk, + TestData.RP_ID, + null, + null + ); + AuthenticatorAssertionResponse response = (AuthenticatorAssertionResponse) credential.getResponse(); + assertNotNull("Assertion response missing authenticator data", response.getAuthenticatorData()); + assertNotNull("Assertion response missing signature", response.getSignature()); + // User identifiable information (name, DisplayName, icon) MUST NOT be returned if user verification is not done by the authenticator. + assertNull("Assertion response contains user handle", response.getUserHandle()); + } catch (MultipleAssertionsAvailable multipleAssertionsAvailable) { + fail("Got MultipleAssertionsAvailable even though there should only be one credential"); + } + + + // test rk credential + PublicKeyCredentialCreationOptions creationOptionsRk = getCreateOptions( + new PublicKeyCredentialUserEntity( + "rkuser", + "rkuser".getBytes(StandardCharsets.UTF_8), + "RkUser" + ), + true, + Collections.singletonList(TestData.PUB_KEY_CRED_PARAMS_ES256), + null, + UserVerificationRequirement.DISCOURAGED + ); + PublicKeyCredential credRk = webauthn.makeCredential( + TestData.CLIENT_DATA_JSON_CREATE, + creationOptionsRk, + Objects.requireNonNull(creationOptionsRk.getRp().getId()), + null, + null, + null + ); + + AuthenticatorAttestationResponse responseRk = (AuthenticatorAttestationResponse) credRk.getResponse(); + assertNotNull("Failed to make non resident key credential", responseRk); + assertNotNull("Credential missing attestation object", responseRk.getAttestationObject()); + assertNotNull("Credential missing client data JSON", responseRk.getClientDataJson()); + + // Get assertions + PublicKeyCredentialRequestOptions requestOptionsRk = new PublicKeyCredentialRequestOptions( + TestData.CHALLENGE, + (long) 90000, + TestData.RP_ID, + Collections.singletonList(new PublicKeyCredentialDescriptor(credRk.getType(), credRk.getRawId())), + UserVerificationRequirement.DISCOURAGED, + null + ); + + try { + PublicKeyCredential credential = webauthn.getAssertion( + TestData.CLIENT_DATA_JSON_GET, + requestOptionsRk, + TestData.RP_ID, + null, + null + ); + AuthenticatorAssertionResponse response = (AuthenticatorAssertionResponse) credential.getResponse(); + assertNotNull("Assertion response missing authenticator data", response.getAuthenticatorData()); + assertNotNull("Assertion response missing signature", response.getSignature()); + assertNotNull("Assertion response missing user handle", response.getUserHandle()); + } catch (MultipleAssertionsAvailable multipleAssertionsAvailable) { + fail("Got MultipleAssertionsAvailable even though there should only be one credential"); + } + } + public static void testGetAssertionMultipleUsersRk(Ctap2Session session, Object... args) throws Throwable { Ctap2ClientPinTests.ensureDefaultPinSet(session, Ctap2ClientPinTests.getPinUvAuthProtocol(args)); @@ -526,6 +641,21 @@ private static PublicKeyCredentialCreationOptions getCreateOptions( boolean rk, List credParams, @Nullable List excludeCredentials + ) { + return getCreateOptions( + user, + rk, + credParams, + excludeCredentials, + null); + } + + private static PublicKeyCredentialCreationOptions getCreateOptions( + @Nullable PublicKeyCredentialUserEntity user, + boolean rk, + List credParams, + @Nullable List excludeCredentials, + @Nullable String userVerification ) { if (user == null) { user = TestData.USER; @@ -536,7 +666,7 @@ private static PublicKeyCredentialCreationOptions getCreateOptions( rk ? ResidentKeyRequirement.REQUIRED : ResidentKeyRequirement.DISCOURAGED, - null + userVerification ); return new PublicKeyCredentialCreationOptions( rp,