Fix spotbugs warnings

android/src/main/java/com/yubico/yubikit/android/internal/Base64Codec.java → android/src/main/java/com/yubico/yubikit/android/internal/Base64CodecImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2023 Yubico.
+ * 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.
@@ -18,7 +18,7 @@
 
 import android.util.Base64;
 
-public class Base64Codec implements com.yubico.yubikit.core.internal.codec.Base64Codec {
+public class Base64CodecImpl implements com.yubico.yubikit.core.internal.codec.Base64Codec {
 
     @Override
     public String toUrlSafeString(byte[] data) {

android/src/main/resources/META-INF/services/com.yubico.yubikit.core.internal.codec.Base64Codec

@@ -1 +1 @@
-com.yubico.yubikit.android.internal.Base64Codec
+com.yubico.yubikit.android.internal.Base64CodecImpl

buildSrc/src/main/groovy/project-convention-spotbugs.gradle

@@ -22,6 +22,7 @@ spotbugs {
 
     effort = Effort.MORE
     reportLevel = Confidence.valueOf('DEFAULT')
+    excludeFilter = file("../spotbugs/excludeFilter.xml")
 }
 
 tasks.matching {

core/src/main/java/com/yubico/yubikit/core/fido/FidoProtocol.java

@@ -18,6 +18,7 @@
 import com.yubico.yubikit.core.internal.Logger;
 import com.yubico.yubikit.core.Version;
 import com.yubico.yubikit.core.application.CommandState;
+import com.yubico.yubikit.core.util.RandomUtils;
 import com.yubico.yubikit.core.util.StringUtils;
 
 import org.slf4j.LoggerFactory;
@@ -59,8 +60,7 @@ public FidoProtocol(FidoConnection connection) throws IOException {
         this.connection = connection;
 
         // init
-        byte[] nonce = new byte[8];
-        new SecureRandom().nextBytes(nonce);
+        byte[] nonce = RandomUtils.getRandomBytes(8);
 
         channelId = 0xffffffff;
         ByteBuffer buffer = ByteBuffer.wrap(sendAndReceive(CTAPHID_INIT, nonce, null));

core/src/main/java/com/yubico/yubikit/core/util/RandomUtils.java

@@ -22,6 +22,7 @@
  * Utility class to generate random data.
  */
 public class RandomUtils {
+    private static final SecureRandom secureRandom = new SecureRandom();
     /**
      * Returns a byte array containing random values.
      */
@@ -32,7 +33,7 @@ public static byte[] getRandomBytes(int length) {
             SecureRandom.getInstanceStrong().nextBytes(bytes);
         } catch (NoSuchMethodError | NoSuchAlgorithmException e) {
             // Fallback for older Android versions
-            new SecureRandom().nextBytes(bytes);
+            secureRandom.nextBytes(bytes);
         }
         return bytes;
     }

fido/src/main/java/com/yubico/yubikit/fido/ctap/PinUvAuthProtocolV1.java

@@ -43,6 +43,8 @@
 import javax.crypto.spec.IvParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 /**
  * Implements PIN/UV Auth Protocol 1
  *
@@ -113,6 +115,10 @@ public byte[] kdf(byte[] z) {
         }
     }
 
+    @SuppressFBWarnings(value = {"CIPHER_INTEGRITY", "STATIC_IV"},
+            justification = "No padding is performed as the size of demPlaintext is required " +
+                    "to be a multiple of the AES block length. The specification for " +
+                    "PIN/UV Auth Protocol One expects all null IV")
     @Override
     public byte[] encrypt(byte[] key, byte[] demPlaintext) {
         try {
@@ -126,6 +132,10 @@ public byte[] encrypt(byte[] key, byte[] demPlaintext) {
         }
     }
 
+
+    @SuppressFBWarnings(value = "CIPHER_INTEGRITY",
+            justification = "No padding is performed as the size of demPlaintext is required " +
+                    "to be a multiple of the AES block length.")
     @Override
     public byte[] decrypt(byte[] key, byte[] demCiphertext) {
         try {

fido/src/main/java/com/yubico/yubikit/fido/ctap/PinUvAuthProtocolV2.java

@@ -33,6 +33,8 @@
 import javax.crypto.spec.IvParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 /**
  * Implements PIN/UV Auth Protocol 2
  *
@@ -141,6 +143,10 @@ public byte[] authenticate(byte[] key, byte[] message) {
         return mac.doFinal(message);
     }
 
+    @SuppressFBWarnings(value = {"CIPHER_INTEGRITY", "STATIC_IV"},
+            justification = "No padding is performed as the size of demPlaintext is required " +
+                    "to be a multiple of the AES block length. The IV is randomly generated " +
+                    "for every encrypt operation")
     private Cipher getCipher(int mode, byte[] secret, byte[] iv) {
         try {
             Cipher cipher = Cipher.getInstance("AES/CBC/NoPadding");

fido/src/main/java/com/yubico/yubikit/fido/webauthn/AuthenticatorSelectionCriteria.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.
@@ -79,7 +79,7 @@ public static AuthenticatorSelectionCriteria fromMap(
         String residentKeyRequirement = (String) map.get(RESIDENT_KEY);
         if (residentKeyRequirement == null) {
             // Backwards compatibility with WebAuthn level 1
-            if(map.get(REQUIRE_RESIDENT_KEY) == Boolean.TRUE) {
+            if(Boolean.TRUE.equals(map.get(REQUIRE_RESIDENT_KEY))) {
                 residentKeyRequirement = ResidentKeyRequirement.REQUIRED;
             } else {
                 residentKeyRequirement = ResidentKeyRequirement.DISCOURAGED;

piv/src/test/java/com/yubico/yubikit/piv/KeyTypeTest.java

@@ -28,6 +28,8 @@
 import java.security.spec.ECGenParameterSpec;
 
 public class KeyTypeTest {
+    private static final SecureRandom secureRandom = new SecureRandom();
+
     private static KeyPair secp256r1;
     private static KeyPair secp384r1;
     private static KeyPair secp521r1;
@@ -38,12 +40,13 @@ public class KeyTypeTest {
 
     @BeforeClass
     public static void setupKeys() throws NoSuchAlgorithmException, InvalidAlgorithmParameterException {
+
         KeyPairGenerator kpg = KeyPairGenerator.getInstance(KeyType.Algorithm.EC.name());
-        kpg.initialize(new ECGenParameterSpec("secp256r1"), new SecureRandom());
+        kpg.initialize(new ECGenParameterSpec("secp256r1"), secureRandom);
         secp256r1 = kpg.generateKeyPair();
-        kpg.initialize(new ECGenParameterSpec("secp384r1"), new SecureRandom());
+        kpg.initialize(new ECGenParameterSpec("secp384r1"), secureRandom);
         secp384r1 = kpg.generateKeyPair();
-        kpg.initialize(new ECGenParameterSpec("secp521r1"), new SecureRandom());
+        kpg.initialize(new ECGenParameterSpec("secp521r1"), secureRandom);
         secp521r1 = kpg.generateKeyPair();
 
         kpg = KeyPairGenerator.getInstance(KeyType.Algorithm.RSA.name());

spotbugs/excludeFilter.xml

@@ -0,0 +1,20 @@
+<!--
+  ~ 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.
+  -->
+<FindBugsFilter>
+    <Match>
+        <Bug code="EI, EI2, CT" />
+    </Match>
+</FindBugsFilter>

testing/src/main/java/com/yubico/yubikit/testing/fido/Ctap2ConfigTests.java

@@ -43,7 +43,7 @@ static Config getConfig(Ctap2Session session, PinUvAuthProtocol pinUvAuthProtoco
     public static void testReadWriteEnterpriseAttestation(Ctap2Session session, Object... args) throws Throwable {
         Config config = getConfig(session, Ctap2ClientPinTests.getPinUvAuthProtocol(args));
         config.enableEnterpriseAttestation();
-        assertSame(TRUE, session.getInfo().getOptions().get("ep"));
+        assertEquals(TRUE, session.getInfo().getOptions().get("ep"));
     }
 
     public static void testToggleAlwaysUv(Ctap2Session session, Object... args) throws Throwable {
@@ -68,6 +68,6 @@ public static void testSetMinPinLength(Ctap2Session session, Object... args) thr
     }
 
     static boolean getAlwaysUv(Ctap2Session session) throws IOException, CommandException {
-        return session.getInfo().getOptions().get("alwaysUv") == TRUE;
+        return TRUE.equals(session.getInfo().getOptions().get("alwaysUv"));
     }
 }

testing/src/main/java/com/yubico/yubikit/testing/fido/EnterpriseAttestationTests.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.
@@ -52,7 +52,7 @@ public class EnterpriseAttestationTests {
     static void enableEp(Ctap2Session session, PinUvAuthProtocol pinUvAuthProtocol)
             throws CommandException, IOException {
         // enable ep if not enabled
-        if (session.getCachedInfo().getOptions().get("ep") == FALSE) {
+        if (FALSE.equals(session.getCachedInfo().getOptions().get("ep"))) {
 
             ClientPin clientPin = new ClientPin(session, pinUvAuthProtocol);
             byte[] pinToken = clientPin.getPinToken(TestData.PIN, ClientPin.PIN_PERMISSION_ACFG, null);

testing/src/main/java/com/yubico/yubikit/testing/piv/PivTestUtils.java

@@ -53,13 +53,16 @@
 import java.security.spec.PKCS8EncodedKeySpec;
 import java.security.spec.X509EncodedKeySpec;
 import java.util.Date;
+import java.util.Objects;
 
 import javax.crypto.Cipher;
 import javax.crypto.KeyAgreement;
 
 @SuppressWarnings("SpellCheckingInspection")
 public class PivTestUtils {
 
+    private static final SecureRandom secureRandom = new SecureRandom();
+
     private static final Logger logger = LoggerFactory.getLogger(PivTestUtils.class);
 
     private enum StaticKey {
@@ -271,7 +274,9 @@ public static KeyPair generateKey(KeyType keyType) {
             case ECCP384:
                 return generateEcKey("secp384r1");
             case ED25519:
-                return generateEd25519Key();
+                return generateCv25519Key("ED25519");
+            case X25519:
+                return generateCv25519Key("X25519");
             case RSA1024:
             case RSA2048:
             case RSA3072:
@@ -284,16 +289,19 @@ public static KeyPair generateKey(KeyType keyType) {
     private static KeyPair generateEcKey(String curve) {
         try {
             KeyPairGenerator kpg = KeyPairGenerator.getInstance(KeyType.Algorithm.EC.name());
-            kpg.initialize(new ECGenParameterSpec(curve), new SecureRandom());
+            kpg.initialize(new ECGenParameterSpec(curve), secureRandom);
             return kpg.generateKeyPair();
         } catch (NoSuchAlgorithmException | InvalidAlgorithmParameterException e) {
             throw new IllegalStateException(e);
         }
     }
 
-    private static KeyPair generateEd25519Key() {
+    private static KeyPair generateCv25519Key(String keyType) {
+        if (!Objects.equals(keyType, "ED25519") && !Objects.equals(keyType, "X25519")) {
+            throw new IllegalArgumentException("Invalid key keyType");
+        }
         try {
-            KeyPairGenerator kpg = KeyPairGenerator.getInstance("ED25519");
+            KeyPairGenerator kpg = KeyPairGenerator.getInstance(keyType);
             return kpg.generateKeyPair();
         } catch (NoSuchAlgorithmException e) {
             throw new IllegalStateException(e);
@@ -434,14 +442,14 @@ public static void cv25519Tests() throws Exception {
 
     public static void ecSignAndVerify(PrivateKey privateKey, PublicKey publicKey) throws Exception {
         for (String algorithm : EC_SIGNATURE_ALGORITHMS) {
-            logger.debug("Test {}", algorithm);
+            logger.debug("Sign and verify test for {}", algorithm);
             verify(publicKey, Signature.getInstance(algorithm), sign(privateKey, Signature.getInstance(algorithm)));
         }
     }
 
     public static void ed25519SignAndVerify(PrivateKey privateKey, PublicKey publicKey) throws Exception {
         String algorithm = "ED25519";
-        logger.debug("Test {}", algorithm);
+        logger.debug("Sign and verify test for Ed25519");
         verify(publicKey, Signature.getInstance(algorithm), sign(privateKey, Signature.getInstance(algorithm)));
     }