Merge pull request #7694 from corda/shams-bc-eddsa

ENT-11661: Replaced SunEC Ed25519 implementation with Bouncy Castle

core/src/main/kotlin/net/corda/core/crypto/Crypto.kt

@@ -10,7 +10,6 @@ import net.corda.core.crypto.internal.bouncyCastlePQCProvider
 import net.corda.core.crypto.internal.cordaBouncyCastleProvider
 import net.corda.core.crypto.internal.cordaSecurityProvider
 import net.corda.core.crypto.internal.providerMap
-import net.corda.core.crypto.internal.sunEcProvider
 import net.corda.core.internal.utilities.PrivateInterner
 import net.corda.core.serialization.serialize
 import net.corda.core.utilities.ByteSequence
@@ -38,6 +37,7 @@ import org.bouncycastle.jcajce.provider.asymmetric.edec.BCEdDSAPrivateKey
 import org.bouncycastle.jcajce.provider.asymmetric.edec.BCEdDSAPublicKey
 import org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPrivateKey
 import org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPublicKey
+import org.bouncycastle.jcajce.spec.EdDSAParameterSpec
 import org.bouncycastle.jce.ECNamedCurveTable
 import org.bouncycastle.jce.provider.BouncyCastleProvider
 import org.bouncycastle.jce.spec.ECNamedCurveParameterSpec
@@ -64,7 +64,6 @@ import java.security.SignatureException
 import java.security.interfaces.EdECPrivateKey
 import java.security.interfaces.EdECPublicKey
 import java.security.spec.InvalidKeySpecException
-import java.security.spec.NamedParameterSpec
 import java.security.spec.PKCS8EncodedKeySpec
 import java.security.spec.X509EncodedKeySpec
 import javax.crypto.Mac
@@ -144,10 +143,10 @@ object Crypto {
             "EDDSA_ED25519_SHA512",
             AlgorithmIdentifier(EdECObjectIdentifiers.id_Ed25519, null),
             emptyList(), // Both keys and the signature scheme use the same OID.
-            sunEcProvider.name,
+            cordaBouncyCastleProvider.name,
             "Ed25519",
             "Ed25519",
-            NamedParameterSpec.ED25519,
+            EdDSAParameterSpec(EdDSAParameterSpec.Ed25519),
             256,
             "EdDSA signature scheme using the ed25519 twisted Edwards curve."
     )
@@ -946,8 +945,10 @@ object Crypto {
         }
         return when (publicKey) {
             is BCECPublicKey -> publicKey.parameters == signatureScheme.algSpec && !publicKey.q.isInfinity && publicKey.q.isValid
+            // It's not clear if the isOnCurve25519 check is necessary since we use BC for Ed25519, and it seems the BCEdDSAPublicKey c'tor
+            // does a validation check.
             is EdECPublicKey -> signatureScheme == EDDSA_ED25519_SHA512 && publicKey.params.name.equals("Ed25519", ignoreCase = true) && publicKey.point.isOnCurve25519
-            else -> throw IllegalArgumentException("Unsupported key type: ${publicKey::class}")
+            else -> throw IllegalArgumentException("Unsupported key type: ${publicKey.javaClass.name}")
         }
     }
 
@@ -970,7 +971,7 @@ object Crypto {
             is BCECPublicKey, is EdECPublicKey -> publicKeyOnCurve(signatureScheme, key)
             is BCRSAPublicKey -> key.modulus.bitLength() >= 2048 // Although the recommended RSA key size is 3072, we accept any key >= 2048bits.
             is BCSphincs256PublicKey -> true
-            else -> throw IllegalArgumentException("Unsupported key type: ${key::class}")
+            else -> throw IllegalArgumentException("Unsupported key type: ${key.javaClass.name}")
         }
     }
 
@@ -998,13 +999,12 @@ object Crypto {
      */
     @JvmStatic
     fun toSupportedPublicKey(key: PublicKey): PublicKey {
-        return when (key) {
+        return when {
-            is BCECPublicKey -> internPublicKey(key)
+            key is BCEdDSAPublicKey && key is EdECPublicKey -> internPublicKey(key)  // The BC implementation is not public
-            is BCRSAPublicKey -> internPublicKey(key)
+            key is BCECPublicKey -> internPublicKey(key)
-            is BCSphincs256PublicKey -> internPublicKey(key)
+            key is BCRSAPublicKey -> internPublicKey(key)
-            is EdECPublicKey -> internPublicKey(key)
+            key is BCSphincs256PublicKey -> internPublicKey(key)
-            is CompositeKey -> internPublicKey(key)
+            key is CompositeKey -> internPublicKey(key)
-            is BCEdDSAPublicKey -> internPublicKey(key)
             else -> decodePublicKey(key.encoded)
         }
     }
@@ -1019,12 +1019,11 @@ object Crypto {
      */
     @JvmStatic
     fun toSupportedPrivateKey(key: PrivateKey): PrivateKey {
-        return when (key) {
+        return when {
-            is BCECPrivateKey -> key
+            key is BCEdDSAPrivateKey && key is EdECPrivateKey -> key  // The BC implementation is not public
-            is BCRSAPrivateKey -> key
+            key is BCECPrivateKey -> key
-            is BCSphincs256PrivateKey -> key
+            key is BCRSAPrivateKey -> key
-            is EdECPrivateKey -> key
+            key is BCSphincs256PrivateKey -> key
-            is BCEdDSAPrivateKey -> key
             else -> decodePrivateKey(key.encoded)
         }
     }

core/src/main/kotlin/net/corda/core/crypto/internal/ProviderMap.kt

@@ -36,6 +36,6 @@ val bouncyCastlePQCProvider = BouncyCastlePQCProvider().apply {
 // i.e. if someone removes a Provider and then he/she adds a new one with the same name.
 // The val is immutable to avoid any harmful state changes.
 internal val providerMap: Map<String, Provider> = unmodifiableMap(
-    listOf(sunEcProvider, cordaBouncyCastleProvider, cordaSecurityProvider, bouncyCastlePQCProvider)
+    listOf(cordaBouncyCastleProvider, cordaSecurityProvider, bouncyCastlePQCProvider)
         .associateByTo(LinkedHashMap(), Provider::getName)
 )

core/src/main/kotlin/net/corda/core/crypto/internal/Secp256k1SupportProvider.kt

@@ -2,9 +2,11 @@
 
 package net.corda.core.crypto.internal
 
+import net.corda.core.crypto.Crypto
+import org.bouncycastle.jcajce.provider.asymmetric.util.EC5Util
+import org.bouncycastle.jce.ECNamedCurveTable
 import org.bouncycastle.jce.provider.BouncyCastleProvider
-import java.math.BigInteger
+import org.bouncycastle.jce.spec.ECNamedCurveSpec
-import java.math.BigInteger.ZERO
 import java.security.AlgorithmParameters
 import java.security.KeyPair
 import java.security.KeyPairGeneratorSpi
@@ -17,15 +19,15 @@ import java.security.SignatureSpi
 import java.security.interfaces.ECPrivateKey
 import java.security.interfaces.ECPublicKey
 import java.security.spec.AlgorithmParameterSpec
-import java.security.spec.ECFieldFp
 import java.security.spec.ECParameterSpec
-import java.security.spec.ECPoint
-import java.security.spec.EllipticCurve
 import java.security.spec.NamedParameterSpec
 
 /**
  * Augment the SunEC provider with secp256k1 curve support by delegating to [BouncyCastleProvider] when secp256k1 keys or params are
  * requested. Otherwise delegates to SunEC.
+ *
+ * Note, this class only exists to cater for the scenerio where [Signature.getInstance] is called directly without a provider (which happens
+ * to be the JCE recommendation) and thus the `SunEC` provider is selected. Bouncy Castle is already automatically used via [Crypto].
  */
 class Secp256k1SupportProvider : Provider("Secp256k1Support", "1.0", "Augmenting SunEC with support for the secp256k1 curve via BC") {
     init {
@@ -164,25 +166,12 @@ class Secp256k1SupportProvider : Provider("Secp256k1Support", "1.0", "Augmenting
     }
 }
 
-/**
+private val bcSecp256k1Spec = ECNamedCurveTable.getParameterSpec("secp256k1")
- * Parameters for the secp256k1 curve
- */
-private object Secp256k1 {
-    val n = BigInteger("fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141", 16)
-    val g = ECPoint(
-            BigInteger("79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", 16),
-            BigInteger("483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8", 16)
-    )
-    val curve = EllipticCurve(
-            ECFieldFp(BigInteger("fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f", 16)),
-            ZERO,
-            7.toBigInteger()
-    )
-}
 
 val AlgorithmParameterSpec?.isSecp256k1: Boolean
     get() = when (this) {
-        is ECParameterSpec -> cofactor == 1 && order == Secp256k1.n && curve == Secp256k1.curve && generator == Secp256k1.g
         is NamedParameterSpec -> name.equals("secp256k1", ignoreCase = true)
+        is ECNamedCurveSpec -> name.equals("secp256k1", ignoreCase = true)
+        is ECParameterSpec -> EC5Util.convertSpec(this) == bcSecp256k1Spec
         else -> false
     }

core/src/test/kotlin/net/corda/core/crypto/CryptoUtilsTest.kt

@@ -9,7 +9,6 @@ import net.corda.core.crypto.Crypto.SPHINCS256_SHA256
 import net.corda.core.crypto.internal.PlatformSecureRandomService
 import net.corda.core.utilities.OpaqueBytes
 import org.apache.commons.lang3.ArrayUtils.EMPTY_BYTE_ARRAY
-import org.assertj.core.api.Assertions.assertThat
 import org.assertj.core.api.Assertions.assertThatIllegalArgumentException
 import org.assertj.core.api.Assertions.assertThatThrownBy
 import org.bouncycastle.asn1.pkcs.PrivateKeyInfo
@@ -19,7 +18,6 @@ import org.bouncycastle.jcajce.provider.asymmetric.ec.BCECPublicKey
 import org.bouncycastle.jce.ECNamedCurveTable
 import org.bouncycastle.jce.interfaces.ECKey
 import org.bouncycastle.jce.spec.ECNamedCurveParameterSpec
-import org.bouncycastle.math.ec.rfc8032.Ed25519
 import org.bouncycastle.pqc.jcajce.provider.sphincs.BCSphincs256PrivateKey
 import org.bouncycastle.pqc.jcajce.provider.sphincs.BCSphincs256PublicKey
 import org.junit.Assert.assertNotEquals
@@ -492,9 +490,9 @@ class CryptoUtilsTest {
         val keyPairEd = Crypto.generateKeyPair(EDDSA_ED25519_SHA512)
         val (privEd, pubEd) = keyPairEd
 
-        assertEquals(privEd.algorithm, "EdDSA")
+        assertEquals(privEd.algorithm, "Ed25519")
         assertEquals((privEd as EdECPrivateKey).params.name, NamedParameterSpec.ED25519.name)
-        assertEquals(pubEd.algorithm, "EdDSA")
+        assertEquals(pubEd.algorithm, "Ed25519")
         assertEquals((pubEd as EdECPublicKey).params.name, NamedParameterSpec.ED25519.name)
     }
 
@@ -514,11 +512,11 @@ class CryptoUtilsTest {
         val encodedPubEd = pubEd.encoded
 
         val decodedPrivEd = Crypto.decodePrivateKey(encodedPrivEd)
-        assertEquals(decodedPrivEd.algorithm, "EdDSA")
+        assertEquals(decodedPrivEd.algorithm, "Ed25519")
         assertEquals(decodedPrivEd, privEd)
 
         val decodedPubEd = Crypto.decodePublicKey(encodedPubEd)
-        assertEquals(decodedPubEd.algorithm, "EdDSA")
+        assertEquals(decodedPubEd.algorithm, "Ed25519")
         assertEquals(decodedPubEd, pubEd)
     }
 
@@ -661,13 +659,6 @@ class CryptoUtilsTest {
             // Use R1 curve for check.
             assertFalse(Crypto.publicKeyOnCurve(ECDSA_SECP256R1_SHA256, pubEdDSA))
         }
-        val invalidKey = run {
-            val bytes = ByteArray(Ed25519.PUBLIC_KEY_SIZE).also { it[0] = 2 }
-            val encoded = SubjectPublicKeyInfo(EDDSA_ED25519_SHA512.signatureOID, bytes).encoded
-            Crypto.decodePublicKey(encoded)
-        }
-        assertThat(invalidKey).isInstanceOf(EdECPublicKey::class.java)
-        assertThat(Crypto.publicKeyOnCurve(EDDSA_ED25519_SHA512, invalidKey)).isFalse()
     }
 
     @Test(timeout = 300_000)

core/src/test/kotlin/net/corda/core/crypto/EdDSATests.kt

@@ -7,6 +7,7 @@ import net.corda.core.utilities.toHex
 import org.assertj.core.api.Assertions.assertThat
 import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo
 import org.junit.Test
+import java.security.KeyFactory
 import java.security.PrivateKey
 import java.security.spec.EdECPrivateKeySpec
 import java.security.spec.NamedParameterSpec
@@ -149,19 +150,17 @@ class EdDSATests {
                         "3dca179c138ac17ad9bef1177331a704"
         )
 
-        val keyFactory = EDDSA_ED25519_SHA512.keyFactory
-
         val testVectors = listOf(testVector1, testVector2, testVector3, testVector1024, testVectorSHAabc)
         testVectors.forEach { testVector ->
             val messageBytes = testVector.messageToSignHex.hexToByteArray()
             val signatureBytes = testVector.signatureOutputHex.hexToByteArray()
             // Check the private key produces the expected signature
-            val privateKey = keyFactory.generatePrivate(EdECPrivateKeySpec(NamedParameterSpec.ED25519, testVector.privateKeyHex.hexToByteArray()))
+            val privateKey = KeyFactory.getInstance("Ed25519", "SunEC").generatePrivate(EdECPrivateKeySpec(NamedParameterSpec.ED25519, testVector.privateKeyHex.hexToByteArray()))
             assertThat(doSign(privateKey, messageBytes)).isEqualTo(signatureBytes)
             // Check the public key verifies the signature
             val result = withSignature(EDDSA_ED25519_SHA512) { signature ->
                 val publicKeyInfo = SubjectPublicKeyInfo(EDDSA_ED25519_SHA512.signatureOID, testVector.publicKeyHex.hexToByteArray())
-                val publicKey = keyFactory.generatePublic(X509EncodedKeySpec(publicKeyInfo.encoded))
+                val publicKey = EDDSA_ED25519_SHA512.keyFactory.generatePublic(X509EncodedKeySpec(publicKeyInfo.encoded))
                 signature.initVerify(publicKey)
                 signature.update(messageBytes)
                 signature.verify(signatureBytes)
@@ -182,7 +181,7 @@ class EdDSATests {
                         "5a5ca2df6668346291c2043d4eb3e90d"
         )
 
-        val privateKey = keyFactory.generatePrivate(EdECPrivateKeySpec(NamedParameterSpec.ED25519, testVectorEd25519ctx.privateKeyHex.hexToByteArray()))
+        val privateKey = KeyFactory.getInstance("Ed25519", "SunEC").generatePrivate(EdECPrivateKeySpec(NamedParameterSpec.ED25519, testVectorEd25519ctx.privateKeyHex.hexToByteArray()))
         assertThat(doSign(privateKey, testVectorEd25519ctx.messageToSignHex.hexToByteArray()).toHex().lowercase()).isNotEqualTo(testVectorEd25519ctx.signatureOutputHex)
     }
 

node/src/integration-test/kotlin/net/corda/node/CustomSerializationSchemeDriverTest.kt

@@ -41,6 +41,7 @@ import net.corda.core.transactions.WireTransaction
 import net.corda.core.utilities.ByteSequence
 import net.corda.core.utilities.getOrThrow
 import net.corda.core.utilities.unwrap
+import net.corda.nodeapi.internal.serialization.kryo.PublicKeySerializer
 import net.corda.serialization.internal.CordaSerializationMagic
 import net.corda.serialization.internal.SerializationFactoryImpl
 import net.corda.testing.core.ALICE_NAME
@@ -51,6 +52,7 @@ import net.corda.testing.driver.DriverParameters
 import net.corda.testing.driver.NodeParameters
 import net.corda.testing.driver.driver
 import net.corda.testing.node.internal.enclosedCordapp
+import org.bouncycastle.jcajce.provider.asymmetric.edec.BCEdDSAPublicKey
 import org.junit.Test
 import org.objenesis.instantiator.ObjectInstantiator
 import org.objenesis.strategy.InstantiatorStrategy
@@ -307,6 +309,7 @@ class CustomSerializationSchemeDriverTest {
             kryo.classLoader = classLoader
             @Suppress("ReplaceJavaStaticMethodWithKotlinAnalog")
             kryo.register(Arrays.asList("").javaClass, ArraysAsListSerializer())
+            kryo.addDefaultSerializer(BCEdDSAPublicKey::class.java, PublicKeySerializer)
         }
 
         //Stolen from DefaultKryoCustomizer.kt