Address PR review items

Yubico · May 22, 2024 · 76fc590 · 76fc590
1 parent 826bf35
commit 76fc590
Show file tree

Hide file tree

Showing 10 changed files with 28 additions and 32 deletions.
diff --git a/piv/src/main/java/com/yubico/yubikit/piv/PivSession.java b/piv/src/main/java/com/yubico/yubikit/piv/PivSession.java
@@ -884,8 +884,13 @@ public void checkKeySupport(KeyType keyType, PinPolicy pinPolicy, TouchPolicy to
         }
 
         // ROCA
-        if (generate && keyType.params.algorithm == KeyType.Algorithm.RSA) {
-            require(FEATURE_RSA_GENERATION);
+        if (keyType.params.algorithm == KeyType.Algorithm.RSA) {
+            if (generate) {
+                require(FEATURE_RSA_GENERATION);
+            }
+            if (keyType.params.bitLength == 3072 || keyType.params.bitLength == 4096) {
+                require(FEATURE_RSA3072_RSA4096);
+            }
         }
 
         // FIPS
@@ -921,8 +926,7 @@ public void checkKeySupport(KeyType keyType, PinPolicy pinPolicy, TouchPolicy to
      * @throws ApduException        in case of an error response from the YubiKey
      * @throws BadResponseException in case of incorrect YubiKey response
      */
-    public PublicKeyValues generateKeyValues(Slot slot, KeyType keyType, PinPolicy pinPolicy, TouchPolicy touchPolicy) throws IOException, ApduException, BadResponseException
-    {
+    public PublicKeyValues generateKeyValues(Slot slot, KeyType keyType, PinPolicy pinPolicy, TouchPolicy touchPolicy) throws IOException, ApduException, BadResponseException {
         checkKeySupport(keyType, pinPolicy, touchPolicy, true);
 
         Map<Integer, byte[]> tlvs = new LinkedHashMap<>();

diff --git a/piv/src/main/java/com/yubico/yubikit/piv/jca/PivKeyPairGeneratorSpi.java b/piv/src/main/java/com/yubico/yubikit/piv/jca/PivKeyPairGeneratorSpi.java
@@ -71,13 +71,13 @@ public KeyPair generateKeyPair() {
             BlockingQueue<Result<KeyPair, Exception>> queue = new ArrayBlockingQueue<>(1);
             provider.invoke(result -> queue.add(Result.of(() -> {
                 PivSession session = result.getValue();
-                PublicKey publicKey = session.generateKey(spec.slot, spec.keyType, spec.pinPolicy, spec.touchPolicy); //TODO Dont use deprecated function
+                PublicKey publicKey = session.generateKeyValues(spec.slot, spec.keyType, spec.pinPolicy, spec.touchPolicy).toPublicKey();
                 PrivateKey privateKey = PivPrivateKey.from(publicKey, spec.slot, spec.pinPolicy, spec.touchPolicy, spec.pin);
                 return new KeyPair(publicKey, privateKey);
             })));
             return queue.take().getValue();
         } catch (Exception e) {
-            throw new IllegalStateException("An error occurred when generating the key pair");
+            throw new IllegalStateException("An error occurred when generating the key pair", e);
         }
     }
 

diff --git a/testing-android/src/main/java/com/yubico/yubikit/testing/TestActivity.java b/testing-android/src/main/java/com/yubico/yubikit/testing/TestActivity.java
@@ -80,7 +80,7 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
     protected void onResume() {
         super.onResume();
         try {
-            yubiKitManager.startNfcDiscovery(new NfcConfiguration().timeout(50000), this, sessionQueue::add);
+            yubiKitManager.startNfcDiscovery(new NfcConfiguration().timeout(15000), this, sessionQueue::add);
         } catch (NfcNotAvailable e) {
             if (e.isDisabled()) {
                 logger.error("NFC is disabled", e);

diff --git a/testing/src/main/java/com/yubico/yubikit/testing/piv/PivCertificateTests.java b/testing/src/main/java/com/yubico/yubikit/testing/piv/PivCertificateTests.java
@@ -47,8 +47,7 @@ public static void putCompressedCertificate(PivSession piv) throws IOException,
     }
 
     private static void putCertificate(PivSession piv, boolean compressed) throws IOException, ApduException, CertificateException, BadResponseException {
-
-        PivTestUtils.authenticate(piv, DEFAULT_MANAGEMENT_KEY);
+        piv.authenticate(PivTestUtils.getManagementKeyType(piv), DEFAULT_MANAGEMENT_KEY);
 
         for (KeyType keyType : Arrays.asList(KeyType.ECCP256, KeyType.ECCP384, KeyType.RSA1024, KeyType.RSA2048, KeyType.RSA3072, KeyType.RSA4096)) {
 

diff --git a/testing/src/main/java/com/yubico/yubikit/testing/piv/PivDeviceTests.java b/testing/src/main/java/com/yubico/yubikit/testing/piv/PivDeviceTests.java
@@ -45,32 +45,31 @@ public static void testManagementKey(PivSession piv) throws BadResponseException
 
         logger.debug("Authenticate with the wrong key");
         try {
-            PivTestUtils.authenticate(piv, key2);
+            piv.authenticate(PivTestUtils.getManagementKeyType(piv), key2);
             Assert.fail("Authenticated with wrong key");
         } catch (ApduException e) {
             Assert.assertEquals(SW.SECURITY_CONDITION_NOT_SATISFIED, e.getSw());
         }
 
         logger.debug("Change management key");
-        PivTestUtils.authenticate(piv, DEFAULT_MANAGEMENT_KEY);
-        PivTestUtils.setManagementKey(piv, key2, false);
+        piv.authenticate(PivTestUtils.getManagementKeyType(piv), DEFAULT_MANAGEMENT_KEY);
+        piv.setManagementKey(PivTestUtils.getManagementKeyType(piv), key2, false);
 
         logger.debug("Authenticate with the old key");
         try {
-            PivTestUtils.authenticate(piv, DEFAULT_MANAGEMENT_KEY);
+            piv.authenticate(PivTestUtils.getManagementKeyType(piv), DEFAULT_MANAGEMENT_KEY);
             Assert.fail("Authenticated with wrong key");
         } catch (ApduException e) {
             Assert.assertEquals(SW.SECURITY_CONDITION_NOT_SATISFIED, e.getSw());
         }
 
         logger.debug("Change management key");
-        PivTestUtils.authenticate(piv, key2);
-        PivTestUtils.setManagementKey(piv, DEFAULT_MANAGEMENT_KEY, false);
+        piv.setManagementKey(PivTestUtils.getManagementKeyType(piv), DEFAULT_MANAGEMENT_KEY, false);
     }
 
     public static void testPin(PivSession piv) throws ApduException, InvalidPinException, IOException, BadResponseException {
         // Ensure we only try this if the default management key is set.
-        PivTestUtils.authenticate(piv, DEFAULT_MANAGEMENT_KEY);
+        piv.authenticate(PivTestUtils.getManagementKeyType(piv), DEFAULT_MANAGEMENT_KEY);
 
         logger.debug("Verify PIN");
         char[] pin2 = "123123".toCharArray();
@@ -114,7 +113,7 @@ public static void testPin(PivSession piv) throws ApduException, InvalidPinExcep
 
     public static void testPuk(PivSession piv) throws ApduException, InvalidPinException, IOException, BadResponseException {
         // Ensure we only try this if the default management key is set.
-        PivTestUtils.authenticate(piv, DEFAULT_MANAGEMENT_KEY);
+        piv.authenticate(PivTestUtils.getManagementKeyType(piv), DEFAULT_MANAGEMENT_KEY);
 
         // Change PUK
         char[] puk2 = "12341234".toCharArray();

diff --git a/testing/src/main/java/com/yubico/yubikit/testing/piv/PivJcaDecryptTests.java b/testing/src/main/java/com/yubico/yubikit/testing/piv/PivJcaDecryptTests.java
@@ -72,7 +72,8 @@ public static void testDecrypt(PivSession piv, KeyType keyType) throws BadRespon
             throw new IllegalArgumentException("Unsupported");
         }
 
-        PivTestUtils.authenticate(piv, DEFAULT_MANAGEMENT_KEY);
+        piv.authenticate(PivTestUtils.getManagementKeyType(piv), DEFAULT_MANAGEMENT_KEY);
+
         logger.debug("Generate key: {}", keyType);
         KeyPairGenerator kpg = KeyPairGenerator.getInstance("YKPivRSA");
         kpg.initialize(new PivAlgorithmParameterSpec(Slot.KEY_MANAGEMENT, keyType, PinPolicy.DEFAULT, TouchPolicy.DEFAULT, DEFAULT_PIN));

diff --git a/testing/src/main/java/com/yubico/yubikit/testing/piv/PivJcaDeviceTests.java b/testing/src/main/java/com/yubico/yubikit/testing/piv/PivJcaDeviceTests.java
@@ -45,7 +45,7 @@ public class PivJcaDeviceTests {
     @SuppressWarnings("NewApi") // casting to Destroyable is supported from API 26
     public static void testImportKeys(PivSession piv) throws Exception {
         setupJca(piv);
-        PivTestUtils.authenticate(piv, DEFAULT_MANAGEMENT_KEY);
+        piv.authenticate(PivTestUtils.getManagementKeyType(piv), DEFAULT_MANAGEMENT_KEY);
 
         KeyStore keyStore = KeyStore.getInstance("YKPiv");
         keyStore.load(null);
@@ -88,7 +88,7 @@ public static void testImportKeys(PivSession piv) throws Exception {
 
     public static void testGenerateKeys(PivSession piv) throws Exception {
         setupJca(piv);
-        PivTestUtils.authenticate(piv, DEFAULT_MANAGEMENT_KEY);
+        piv.authenticate(PivTestUtils.getManagementKeyType(piv), DEFAULT_MANAGEMENT_KEY);
 
         KeyPairGenerator ecGen = KeyPairGenerator.getInstance("YKPivEC");
         for (KeyType keyType : Arrays.asList(KeyType.ECCP256, KeyType.ECCP384)) {

diff --git a/testing/src/main/java/com/yubico/yubikit/testing/piv/PivJcaSigningTests.java b/testing/src/main/java/com/yubico/yubikit/testing/piv/PivJcaSigningTests.java
@@ -25,6 +25,7 @@
 import com.yubico.yubikit.core.application.BadResponseException;
 import com.yubico.yubikit.core.smartcard.ApduException;
 import com.yubico.yubikit.piv.KeyType;
+import com.yubico.yubikit.piv.ManagementKeyType;
 import com.yubico.yubikit.piv.PinPolicy;
 import com.yubico.yubikit.piv.PivSession;
 import com.yubico.yubikit.piv.Slot;
@@ -70,7 +71,7 @@ public static void testSign(PivSession piv) throws NoSuchAlgorithmException, IOE
     }
 
     public static void testSign(PivSession piv, KeyType keyType) throws NoSuchAlgorithmException, IOException, ApduException, InvalidKeyException, BadResponseException, InvalidAlgorithmParameterException, SignatureException {
-        PivTestUtils.authenticate(piv, DEFAULT_MANAGEMENT_KEY);
+        piv.authenticate(ManagementKeyType.TDES, DEFAULT_MANAGEMENT_KEY);
         logger.debug("Generate key: {}", keyType);
 
         KeyPairGenerator kpg = KeyPairGenerator.getInstance("YKPiv" + keyType.params.algorithm.name());

diff --git a/testing/src/main/java/com/yubico/yubikit/testing/piv/PivMoveKeyTests.java b/testing/src/main/java/com/yubico/yubikit/testing/piv/PivMoveKeyTests.java
@@ -62,7 +62,7 @@ static void moveKey(PivSession piv)
         Slot srcSlot = Slot.RETIRED1;
         Slot dstSlot = Slot.RETIRED2;
 
-        piv.authenticate(ManagementKeyType.AES192, DEFAULT_MANAGEMENT_KEY);
+        piv.authenticate(PivTestUtils.getManagementKeyType(piv), DEFAULT_MANAGEMENT_KEY);
 
         for (KeyType keyType : Arrays.asList(KeyType.ECCP256, KeyType.ECCP384, KeyType.RSA1024, KeyType.RSA2048, KeyType.RSA3072, KeyType.RSA4096)) {
 

diff --git a/testing/src/main/java/com/yubico/yubikit/testing/piv/PivTestUtils.java b/testing/src/main/java/com/yubico/yubikit/testing/piv/PivTestUtils.java
@@ -416,16 +416,8 @@ public static void ecKeyAgreement(PrivateKey privateKey, PublicKey publicKey) th
 
         Assert.assertArrayEquals("Secret mismatch", secret, peerSecret);
     }
-
-    public static void authenticate(PivSession piv, byte[] key) throws BadResponseException, ApduException, IOException {
-        piv.authenticate(getDefaultManagementKeyType(piv), key);
-    }
-
-    public static void setManagementKey(PivSession piv, byte[] key, boolean requireTouch) throws ApduException, IOException {
-        piv.setManagementKey(getDefaultManagementKeyType(piv), key, requireTouch);
-    }
-
-    private static ManagementKeyType getDefaultManagementKeyType(PivSession piv) {
+
+    public static ManagementKeyType getManagementKeyType(PivSession piv) {
         return piv.getVersion().isAtLeast(5, 7, 0)
                 ? ManagementKeyType.AES192
                 : ManagementKeyType.TDES;