Fall back to BouncyCastle explicitly instead of modifying JCA provider context

Author: emlun

webauthn-server-core/src/main/java/com/yubico/webauthn/AndroidSafetynetAttestationStatementVerifier.java

@@ -107,7 +107,7 @@ private boolean verifySignature(JsonWebSignatureCustom jws) {
 
         Signature signatureVerifier;
         try {
-            signatureVerifier = Signature.getInstance(signatureAlgorithmName);
+            signatureVerifier = Crypto.getSignature(signatureAlgorithmName);
         } catch (NoSuchAlgorithmException e) {
             throw ExceptionUtil.wrapAndLog(log, "Failed to get a Signature instance for " + signatureAlgorithmName, e);
         }

webauthn-server-core/src/main/java/com/yubico/webauthn/Crypto.java

@@ -35,15 +35,21 @@
 import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
+import java.security.KeyFactory;
+import java.security.NoSuchAlgorithmException;
+import java.security.Provider;
 import java.security.PublicKey;
 import java.security.Signature;
 import java.security.cert.X509Certificate;
 import java.security.spec.ECFieldFp;
 import java.security.spec.ECParameterSpec;
 import java.security.spec.EllipticCurve;
 import lombok.experimental.UtilityClass;
+import lombok.extern.slf4j.Slf4j;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
 
 @UtilityClass
+@Slf4j
 final class Crypto
 {
     // Values from https://apps.nsa.gov/iaarchive/library/ia-guidance/ia-solutions-for-classified/algorithm-guidance/mathematical-routines-for-the-nist-prime-elliptic-curves.cfm
@@ -54,6 +60,47 @@ final class Crypto
             new BigInteger("115792089210356248762697446949407573530086143415290314195533631308867097853948", 10),
             new BigInteger("41058363725152142129326129780047268409114441015993725554835256314039467401291", 10));
 
+    /*
+     * TODO: Delete this in the next major version release
+     */
+    private static class BouncyCastleLoader {
+        private static Provider getProvider() {
+            return new BouncyCastleProvider();
+        }
+    }
+
+    /*
+     * TODO: Delete this in the next major version release
+     */
+    public static KeyFactory getKeyFactory(String algorithm) throws NoSuchAlgorithmException {
+        try {
+            return KeyFactory.getInstance(algorithm);
+        } catch (NoSuchAlgorithmException e) {
+            log.debug("Caught {}. Attempting fallback to BouncyCastle...", e.toString());
+            try {
+                return KeyFactory.getInstance(algorithm, BouncyCastleLoader.getProvider());
+            } catch (NoSuchAlgorithmException e2) {
+                throw e;
+            }
+        }
+    }
+
+    /*
+     * TODO: Delete this in the next major version release
+     */
+    public static Signature getSignature(String algorithm) throws NoSuchAlgorithmException {
+        try {
+            return Signature.getInstance(algorithm);
+        } catch (NoSuchAlgorithmException e) {
+            log.debug("Caught {}. Attempting fallback to BouncyCastle...", e.toString());
+            try {
+                return Signature.getInstance(algorithm, BouncyCastleLoader.getProvider());
+            } catch (NoSuchAlgorithmException e2) {
+                throw e;
+            }
+        }
+    }
+
     static boolean isP256(ECParameterSpec params) {
         return P256.equals(params.getCurve());
     }

webauthn-server-core/src/main/java/com/yubico/webauthn/PackedAttestationStatementVerifier.java

@@ -173,7 +173,7 @@ private boolean verifyX5cSignature(AttestationObject attestationObject, ByteArra
                 final String signatureAlgorithmName = WebAuthnCodecs.getJavaAlgorithmName(sigAlg);
                 Signature signatureVerifier;
                 try {
-                    signatureVerifier = Signature.getInstance(signatureAlgorithmName);
+                    signatureVerifier = Crypto.getSignature(signatureAlgorithmName);
                 } catch (NoSuchAlgorithmException e) {
                     throw ExceptionUtil.wrapAndLog(log, "Failed to get a Signature instance for " + signatureAlgorithmName, e);
                 }

webauthn-server-core/src/main/java/com/yubico/webauthn/RelyingParty.java

@@ -50,14 +50,9 @@
 import lombok.NonNull;
 import lombok.Value;
 import lombok.extern.slf4j.Slf4j;
-import org.bouncycastle.jce.provider.BouncyCastleProvider;
-
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.security.KeyFactory;
-import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
-import java.security.Security;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -73,11 +68,6 @@
  * This class has no mutable state. An instance of this class may therefore be thought of as a container for specialized
  * versions (function closures) of these four operations rather than a stateful object.
  * </p>
- *
- * NOTE: Constructing this class may cause side effects in some cases.
- * If the JCA security providers do not provide the <code>RSA</code>, <code>EC</code> and <code>EdDSA</code> algorithms,
- * this class will attempt to create and add a {@link BouncyCastleProvider} to the global JCA context.
- * If the {@link BouncyCastleProvider} class does not exist in the classpath, a warning will be logged.
  */
 @Slf4j
 @Builder(toBuilder = true)
@@ -424,8 +414,6 @@ private RelyingParty(
         this.allowUnrequestedExtensions = allowUnrequestedExtensions;
         this.allowUntrustedAttestation = allowUntrustedAttestation;
         this.validateSignatureCounter = validateSignatureCounter;
-
-        tryLoadBouncyCastleIfNeeded();
     }
 
     private static ByteArray generateChallenge() {
@@ -434,45 +422,6 @@ private static ByteArray generateChallenge() {
         return new ByteArray(bytes);
     }
 
-    /*
-     * TODO: Delete this in the next major version release
-     */
-    private static void tryLoadBouncyCastleIfNeeded() {
-        String[] algs = new String[]{"RSA", "EC", "EdDSA"};
-
-        for (String alg : algs) {
-            try {
-                KeyFactory.getInstance(alg);
-            } catch (NoSuchAlgorithmException e) {
-                log.debug("JCA algorithm provider not available: {}. Attempting to load BouncyCastle...", alg);
-                try {
-                    BouncyCastleLoader.loadBouncyCastleProvider();
-                    log.debug("Successfully loaded BouncyCastle provider.");
-                } catch (NoClassDefFoundError e2) {
-                    log.info("Failed to load BouncyCastle security provider. Some key decoding and/or signature verification may fail.", e2);
-                }
-                break;
-            }
-        }
-
-        for (String alg : algs) {
-            try {
-                KeyFactory.getInstance(alg);
-            } catch (NoSuchAlgorithmException e) {
-                log.warn("JCA algorithm provider not available: {}. Keys and signatures using this algorithm will cause crashes.", alg);
-            }
-        }
-    }
-
-    /*
-     * TODO: Delete this in the next major version release
-     */
-    private static class BouncyCastleLoader {
-        private static void loadBouncyCastleProvider() {
-            Security.addProvider(new BouncyCastleProvider());
-        }
-    }
-
     public PublicKeyCredentialCreationOptions startRegistration(StartRegistrationOptions startRegistrationOptions) {
         PublicKeyCredentialCreationOptionsBuilder builder = PublicKeyCredentialCreationOptions.builder()
             .rp(identity)

webauthn-server-core/src/main/java/com/yubico/webauthn/WebAuthnCodecs.java

@@ -86,7 +86,7 @@ private static PublicKey importCoseRsaPublicKey(CBORObject cose) throws NoSuchAl
             new BigInteger(1, cose.get(CBORObject.FromObject(-1)).GetByteString()),
             new BigInteger(1, cose.get(CBORObject.FromObject(-2)).GetByteString())
         );
-        return KeyFactory.getInstance("RSA").generatePublic(spec);
+        return Crypto.getKeyFactory("RSA").generatePublic(spec);
     }
 
     private static ECPublicKey importCoseP256PublicKey(CBORObject cose) throws CoseException {
@@ -109,7 +109,7 @@ private static PublicKey importCoseEd25519PublicKey(CBORObject cose) throws Inva
             .concat(new ByteArray(new byte[]{ 0x03, (byte) (rawKey.size() + 1), 0}))
             .concat(rawKey);
 
-        KeyFactory kFact = KeyFactory.getInstance("EdDSA");
+        KeyFactory kFact = Crypto.getKeyFactory("EdDSA");
         return kFact.generatePublic(new X509EncodedKeySpec(x509Key.getBytes()));
     }