Fix MC/GA UserVerificationRequirement.DISCOURAGED

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.
@@ -105,6 +105,20 @@ boolean supportsEpForRpId(@Nullable String rpId) {
         }
     }
 
+    private static class AuthParams {
+        @Nullable
+        private final byte[] pinUvAuthParam;
+        @Nullable
+        private final Integer pinUvAuthProtocol;
+
+        AuthParams(
+                @Nullable byte[] pinUvAuthParam,
+                @Nullable Integer pinUvAuthProtocol) {
+            this.pinUvAuthParam = pinUvAuthParam;
+            this.pinUvAuthProtocol = pinUvAuthProtocol;
+        }
+    }
+
     public BasicWebAuthnClient(Ctap2Session session) throws IOException, CommandException {
         this.ctap = session;
         Ctap2Session.InfoData info = ctap.getInfo();
@@ -380,101 +394,86 @@ protected Ctap2Session.CredentialData ctapMakeCredential(
 
         final SerializationType serializationType = SerializationType.CBOR;
 
-        byte[] pinToken = null;
-        try {
-            if (options.getExtensions() != null) {
-                throw new ClientError(
-                        ClientError.Code.CONFIGURATION_UNSUPPORTED,
-                        "Extensions not supported");
-            }
+        if (options.getExtensions() != null) {
+            throw new ClientError(
+                    ClientError.Code.CONFIGURATION_UNSUPPORTED,
+                    "Extensions not supported");
+        }
 
-            Map<String, ?> rp = options.getRp().toMap(serializationType);
-            String rpId = options.getRp().getId();
-            if (rpId == null) {
-                ((Map<String, Object>) rp).put("id", effectiveDomain);
-            } else if (!(effectiveDomain.equals(rpId) || effectiveDomain.endsWith("." + rpId))) {
-                throw new ClientError(
-                        ClientError.Code.BAD_REQUEST,
-                        "RP ID is not valid for effective domain");
-            }
+        Map<String, ?> rp = options.getRp().toMap(serializationType);
+        String rpId = options.getRp().getId();
+        if (rpId == null) {
+            ((Map<String, Object>) rp).put("id", effectiveDomain);
+        } else if (!(effectiveDomain.equals(rpId) || effectiveDomain.endsWith("." + rpId))) {
+            throw new ClientError(
+                    ClientError.Code.BAD_REQUEST,
+                    "RP ID is not valid for effective domain");
+        }
 
-            byte[] pinUvAuthParam = null;
-            int pinUvAuthProtocol = 0;
-
-            Map<String, Boolean> ctapOptions = new HashMap<>();
-            AuthenticatorSelectionCriteria authenticatorSelection =
-                    options.getAuthenticatorSelection();
-            if (authenticatorSelection != null) {
-                String residentKeyRequirement = authenticatorSelection.getResidentKey();
-                if (ResidentKeyRequirement.REQUIRED.equals(residentKeyRequirement) ||
-                        (ResidentKeyRequirement.PREFERRED.equals(residentKeyRequirement) &&
-                                (pinSupported || uvSupported)
-                        )
-                ) {
-                    ctapOptions.put(OPTION_RESIDENT_KEY, true);
-                }
-                if (getCtapUv(authenticatorSelection.getUserVerification(), pin != null)) {
-                    ctapOptions.put(OPTION_USER_VERIFICATION, true);
-                }
-            } else {
-                if (getCtapUv(UserVerificationRequirement.PREFERRED, pin != null)) {
-                    ctapOptions.put(OPTION_USER_VERIFICATION, true);
-                }
+        Map<String, Boolean> ctapOptions = new HashMap<>();
+        AuthenticatorSelectionCriteria authenticatorSelection =
+                options.getAuthenticatorSelection();
+        if (authenticatorSelection != null) {
+            String residentKeyRequirement = authenticatorSelection.getResidentKey();
+            if (ResidentKeyRequirement.REQUIRED.equals(residentKeyRequirement) ||
+                    (ResidentKeyRequirement.PREFERRED.equals(residentKeyRequirement) &&
+                            (pinSupported || uvSupported)
+                    )
+            ) {
+                ctapOptions.put(OPTION_RESIDENT_KEY, true);
             }
-
-            if (pin != null) {
-                pinToken = clientPin.getPinToken(pin, ClientPin.PIN_PERMISSION_MC, rpId);
-                pinUvAuthParam = clientPin.getPinUvAuth().authenticate(pinToken, clientDataHash);
-                pinUvAuthProtocol = clientPin.getPinUvAuth().getVersion();
-            } else if (pinConfigured && ctapOptions.containsKey(OPTION_USER_VERIFICATION)) {
-                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)) {
-                throw new PinRequiredClientError();
+            if (getCtapUv(authenticatorSelection.getUserVerification(), pin != null)) {
+                ctapOptions.put(OPTION_USER_VERIFICATION, true);
+            }
+        } else {
+            if (getCtapUv(UserVerificationRequirement.PREFERRED, pin != null)) {
+                ctapOptions.put(OPTION_USER_VERIFICATION, true);
             }
+        }
 
-            final List<PublicKeyCredentialDescriptor> excludeCredentials =
-                    removeUnsupportedCredentials(
-                            options.getExcludeCredentials()
-                    );
+        final AuthParams authParams = getAuthParams(
+                clientDataHash,
+                ctapOptions.containsKey(OPTION_USER_VERIFICATION),
+                pin,
+                ClientPin.PIN_PERMISSION_MC,
+                rpId);
 
-            final Map<String, ?> user = options.getUser().toMap(serializationType);
+        final List<PublicKeyCredentialDescriptor> excludeCredentials =
+                removeUnsupportedCredentials(
+                        options.getExcludeCredentials()
+                );
 
-            List<Map<String, ?>> pubKeyCredParams = new ArrayList<>();
-            for (PublicKeyCredentialParameters param : options.getPubKeyCredParams()) {
-                if (isPublicKeyCredentialTypeSupported(param.getType())) {
-                    pubKeyCredParams.add(param.toMap(serializationType));
-                }
-            }
+        final Map<String, ?> user = options.getUser().toMap(serializationType);
 
-            @Nullable Integer validatedEnterpriseAttestation = null;
-            if (isEnterpriseAttestationSupported() &&
-                    AttestationConveyancePreference.ENTERPRISE.equals(options.getAttestation()) &&
-                    userAgentConfiguration.supportsEpForRpId(rpId) &&
-                    enterpriseAttestation != null &&
-                    (enterpriseAttestation == 1 || enterpriseAttestation == 2)) {
-                validatedEnterpriseAttestation = enterpriseAttestation;
+        List<Map<String, ?>> pubKeyCredParams = new ArrayList<>();
+        for (PublicKeyCredentialParameters param : options.getPubKeyCredParams()) {
+            if (isPublicKeyCredentialTypeSupported(param.getType())) {
+                pubKeyCredParams.add(param.toMap(serializationType));
             }
+        }
 
-            return ctap.makeCredential(
-                    clientDataHash,
-                    rp,
-                    user,
-                    pubKeyCredParams,
-                    getCredentialList(excludeCredentials),
-                    null,
-                    ctapOptions.isEmpty() ? null : ctapOptions,
-                    pinUvAuthParam,
-                    pinUvAuthProtocol,
-                    validatedEnterpriseAttestation,
-                    state
-            );
-        } finally {
-            if (pinToken != null) {
-                Arrays.fill(pinToken, (byte) 0);
-            }
+        @Nullable Integer validatedEnterpriseAttestation = null;
+        if (isEnterpriseAttestationSupported() &&
+                AttestationConveyancePreference.ENTERPRISE.equals(options.getAttestation()) &&
+                userAgentConfiguration.supportsEpForRpId(rpId) &&
+                enterpriseAttestation != null &&
+                (enterpriseAttestation == 1 || enterpriseAttestation == 2)) {
+            validatedEnterpriseAttestation = enterpriseAttestation;
         }
+
+        return ctap.makeCredential(
+                clientDataHash,
+                rp,
+                user,
+                pubKeyCredParams,
+                getCredentialList(excludeCredentials),
+                null,
+                ctapOptions.isEmpty() ? null : ctapOptions,
+                authParams.pinUvAuthParam,
+                authParams.pinUvAuthProtocol,
+                validatedEnterpriseAttestation,
+                state
+        );
     }
 
     /**
@@ -519,20 +518,14 @@ protected List<Ctap2Session.AssertionData> ctapGetAssertions(
             throw new ClientError(ClientError.Code.CONFIGURATION_UNSUPPORTED, "Extensions not supported");
         }
 
-        byte[] pinUvAuthParam = null;
-        int pinUvAuthProtocol = 0;
-        byte[] pinToken = null;
-        try {
-            if (pin != null) {
-                pinToken = clientPin.getPinToken(pin, ClientPin.PIN_PERMISSION_GA, rpId);
-                pinUvAuthParam = clientPin.getPinUvAuth().authenticate(pinToken, clientDataHash);
-                pinUvAuthProtocol = clientPin.getPinUvAuth().getVersion();
-            } else if (pinConfigured && ctapOptions.containsKey(OPTION_USER_VERIFICATION)) {
-                pinToken = clientPin.getUvToken(ClientPin.PIN_PERMISSION_GA, rpId, null);
-                pinUvAuthParam = clientPin.getPinUvAuth().authenticate(pinToken, clientDataHash);
-                pinUvAuthProtocol = clientPin.getPinUvAuth().getVersion();
-            }
+        final AuthParams authParams = getAuthParams(
+                clientDataHash,
+                ctapOptions.containsKey(OPTION_USER_VERIFICATION),
+                pin,
+                ClientPin.PIN_PERMISSION_GA,
+                rpId);
 
+        try {
             final List<PublicKeyCredentialDescriptor> allowCredentials = removeUnsupportedCredentials(
                     options.getAllowCredentials()
             );
@@ -543,19 +536,15 @@ protected List<Ctap2Session.AssertionData> ctapGetAssertions(
                     getCredentialList(allowCredentials),
                     null,
                     ctapOptions.isEmpty() ? null : ctapOptions,
-                    pinUvAuthParam,
-                    pinUvAuthProtocol,
+                    authParams.pinUvAuthParam,
+                    authParams.pinUvAuthProtocol,
                     state
             );
         } catch (CtapException e) {
             if (e.getCtapError() == CtapException.ERR_PIN_INVALID) {
                 throw new PinInvalidClientError(e, clientPin.getPinRetries().getCount());
             }
             throw ClientError.wrapCtapException(e);
-        } finally {
-            if (pinToken != null) {
-                Arrays.fill(pinToken, (byte) 0);
-            }
         }
     }
 
@@ -603,6 +592,47 @@ private boolean getCtapUv(String userVerification, boolean pinProvided) throws C
         }
     }
 
+    private AuthParams getAuthParams(
+            byte[] clientDataHash,
+            boolean shouldUv,
+            @Nullable char[] pin,
+            @Nullable Integer permissions,
+            @Nullable String rpId
+    ) throws ClientError, IOException, CommandException {
+        @Nullable byte[] authToken = null;
+        @Nullable byte[] authParam = null;
+        @Nullable Integer authProtocolVersion = null;
+
+        try {
+            if (pin != null) {
+                authToken = clientPin.getPinToken(pin, permissions, rpId);
+                authParam = clientPin.getPinUvAuth().authenticate(authToken, clientDataHash);
+                authProtocolVersion = clientPin.getPinUvAuth().getVersion();
+            } else if (pinConfigured) {
+                if (shouldUv && uvConfigured) {
+                    if (ClientPin.isTokenSupported(ctap.getCachedInfo())) {
+                        authToken = clientPin.getUvToken(permissions, rpId, null);
+                        authParam = clientPin.getPinUvAuth().authenticate(authToken, clientDataHash);
+                        authProtocolVersion = clientPin.getPinUvAuth().getVersion();
+                    }
+                    // no authToken is created means that internal UV is used
+                } else {
+                    // the authenticator supports pin but no PIN was provided
+                    throw new PinRequiredClientError();
+                }
+            }
+            return new AuthParams(
+                    authParam,
+                    authProtocolVersion
+            );
+
+        } finally {
+            if (authToken != null) {
+                Arrays.fill(authToken, (byte) 0);
+            }
+        }
+    }
+
     /**
      * Calculates the preferred pinUvAuth protocol for authenticator provided list.
      * Returns PinUvAuthDummyProtocol if the authenticator does not support any of the SDK

testing-android/src/androidTest/java/com/yubico/yubikit/testing/fido/BasicWebAuthnClientInstrumentedTests.java

@@ -20,6 +20,7 @@
 
 import androidx.test.filters.LargeTest;
 
+import com.yubico.yubikit.fido.ctap.ClientPin;
 import com.yubico.yubikit.fido.ctap.PinUvAuthProtocol;
 import com.yubico.yubikit.fido.ctap.PinUvAuthProtocolV1;
 import com.yubico.yubikit.fido.ctap.PinUvAuthProtocolV2;
@@ -54,7 +55,18 @@ public void testMakeCredentialGetAssertion() throws Throwable {
             withCtap2Session(
                     (device, session) -> supportsPinUvAuthProtocol(session, pinUvAuthProtocol),
                     BasicWebAuthnClientTests::testMakeCredentialGetAssertion,
-                    pinUvAuthProtocol);
+                    pinUvAuthProtocol,
+                    TestData.PIN);
+        }
+
+        @Test
+        public void testMakeCredentialGetAssertionTokenUvOnly() throws Throwable {
+            withCtap2Session(
+                    (device, session) -> supportsPinUvAuthProtocol(session, pinUvAuthProtocol)
+                            && ClientPin.isTokenSupported(session.getCachedInfo()),
+                    BasicWebAuthnClientTests::testMakeCredentialGetAssertion,
+                    pinUvAuthProtocol,
+                    null);
         }
 
         @Test

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.PinRequiredClientError;
+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
+     * Expected to fail with PinRequiredClientError
+     */
+    @Test(expected = PinRequiredClientError.class)
+    public void testMakeCredentialGetAssertionOnProtected() throws Throwable {
+        withCtap2Session(
+                "This device has no PIN set",
+                (device, session) -> hasPin(session),
+                BasicWebAuthnClientTests::testUvDiscouragedMakeCredentialGetAssertion);
+    }
+}