From 33ba145149c87ae1f19ee37b09c77513588c8efb Mon Sep 17 00:00:00 2001 From: Viktor Kolomeyko Date: Thu, 5 Oct 2017 17:42:16 +0100 Subject: [PATCH] CORDA-540: Add verification to ensure that private keys can only be serialized with specific contexts (#1800) --- .../serialization/AllButBlacklisted.kt | 3 ++ .../nodeapi/internal/serialization/Kryo.kt | 18 ++++---- .../serialization/UseCaseAwareness.kt | 12 ++++++ .../internal/serialization/KryoTests.kt | 2 +- .../PrivateKeySerializationTest.kt | 42 +++++++++++++++++++ 5 files changed, 67 insertions(+), 10 deletions(-) create mode 100644 node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/UseCaseAwareness.kt create mode 100644 node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/PrivateKeySerializationTest.kt diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/AllButBlacklisted.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/AllButBlacklisted.kt index eadd5394144..4bc36e9a8b2 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/AllButBlacklisted.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/AllButBlacklisted.kt @@ -30,6 +30,9 @@ import kotlin.collections.LinkedHashSet * we can often end up pulling in a lot of objects that do not make sense to put in a checkpoint. * Thus, by blacklisting classes/interfaces we don't expect to be serialised, we can better handle/monitor the aforementioned behaviour. * Inheritance works for blacklisted items, but one can specifically exclude classes from blacklisting as well. + * Note: Custom serializer registration trumps white/black lists. So if a given type has a custom serializer and has its name + * in the blacklist - it will still be serialized as specified by custom serializer. + * For more details, see [net.corda.nodeapi.internal.serialization.CordaClassResolver.getRegistration] */ object AllButBlacklisted : ClassWhitelist { diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/Kryo.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/Kryo.kt index 2905922460b..7b998d5cd73 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/Kryo.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/Kryo.kt @@ -10,30 +10,23 @@ import com.esotericsoftware.kryo.util.MapReferenceResolver import net.corda.core.concurrent.CordaFuture import net.corda.core.contracts.PrivacySalt import net.corda.core.contracts.StateRef -import net.corda.core.crypto.CompositeKey import net.corda.core.crypto.Crypto import net.corda.core.crypto.TransactionSignature import net.corda.core.identity.Party import net.corda.core.internal.uncheckedCast import net.corda.core.serialization.SerializationContext +import net.corda.core.serialization.SerializationContext.UseCase.* import net.corda.core.serialization.SerializeAsTokenContext import net.corda.core.serialization.SerializedBytes import net.corda.core.toFuture import net.corda.core.toObservable import net.corda.core.transactions.* -import net.i2p.crypto.eddsa.EdDSAPrivateKey -import net.i2p.crypto.eddsa.EdDSAPublicKey -import net.i2p.crypto.eddsa.spec.EdDSANamedCurveSpec -import net.i2p.crypto.eddsa.spec.EdDSAPrivateKeySpec -import net.i2p.crypto.eddsa.spec.EdDSAPublicKeySpec import org.bouncycastle.asn1.ASN1InputStream import org.bouncycastle.asn1.x500.X500Name import org.bouncycastle.cert.X509CertificateHolder import org.slf4j.Logger import org.slf4j.LoggerFactory import rx.Observable -import sun.security.ec.ECPublicKeyImpl -import sun.security.util.DerValue import java.io.ByteArrayInputStream import java.io.InputStream import java.lang.reflect.InvocationTargetException @@ -285,9 +278,16 @@ object SignedTransactionSerializer : Serializer() { } } +sealed class UseCaseSerializer(private val allowedUseCases: EnumSet) : Serializer() { + protected fun checkUseCase() { + checkUseCase(allowedUseCases) + } +} + @ThreadSafe -object PrivateKeySerializer : Serializer() { +object PrivateKeySerializer : UseCaseSerializer(EnumSet.of(Storage, Checkpoint)) { override fun write(kryo: Kryo, output: Output, obj: PrivateKey) { + checkUseCase() output.writeBytesWithLength(obj.encoded) } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/UseCaseAwareness.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/UseCaseAwareness.kt new file mode 100644 index 00000000000..fdf57d28be4 --- /dev/null +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/UseCaseAwareness.kt @@ -0,0 +1,12 @@ +package net.corda.nodeapi.internal.serialization + +import net.corda.core.serialization.SerializationContext +import net.corda.core.serialization.SerializationFactory +import java.util.* + +internal fun checkUseCase(allowedUseCases: EnumSet) { + val currentContext: SerializationContext = SerializationFactory.currentFactory?.currentContext ?: throw IllegalStateException("Current context is not set") + if(!allowedUseCases.contains(currentContext.useCase)) { + throw IllegalStateException("UseCase '${currentContext.useCase}' is not within '$allowedUseCases'") + } +} \ No newline at end of file diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/KryoTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/KryoTests.kt index 1855a4c8c7c..bf9c157c864 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/KryoTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/KryoTests.kt @@ -43,7 +43,7 @@ class KryoTests : TestDependencyInjectionBase() { AllWhitelist, emptyMap(), true, - SerializationContext.UseCase.P2P) + SerializationContext.UseCase.Storage) } @Test diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/PrivateKeySerializationTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/PrivateKeySerializationTest.kt new file mode 100644 index 00000000000..bf9cd8f2e52 --- /dev/null +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/PrivateKeySerializationTest.kt @@ -0,0 +1,42 @@ +package net.corda.nodeapi.internal.serialization + +import net.corda.core.crypto.Crypto +import net.corda.core.serialization.SerializationContext.UseCase.* +import net.corda.core.serialization.SerializationDefaults +import net.corda.core.serialization.serialize +import net.corda.testing.TestDependencyInjectionBase +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import java.security.PrivateKey +import kotlin.test.assertTrue + +@RunWith(Parameterized::class) +class PrivateKeySerializationTest(private val privateKey: PrivateKey, private val testName: String) : TestDependencyInjectionBase() { + + companion object { + @JvmStatic + @Parameterized.Parameters(name = "{1}") + fun data(): Collection> { + val privateKeys: List = Crypto.supportedSignatureSchemes().filterNot { Crypto.COMPOSITE_KEY === it } + .map { Crypto.generateKeyPair(it).private } + + return privateKeys.map { arrayOf(it, PrivateKeySerializationTest::class.java.simpleName + "-" + it.javaClass.simpleName) } + } + } + + @Test + fun `passed with expected UseCases`() { + assertTrue { privateKey.serialize(context = SerializationDefaults.STORAGE_CONTEXT).bytes.isNotEmpty() } + assertTrue { privateKey.serialize(context = SerializationDefaults.CHECKPOINT_CONTEXT).bytes.isNotEmpty() } + } + + @Test + fun `failed with wrong UseCase`() { + assertThatThrownBy { privateKey.serialize(context = SerializationDefaults.P2P_CONTEXT) } + .isInstanceOf(IllegalStateException::class.java) + .hasMessageContaining("UseCase '${P2P}' is not within") + + } +} \ No newline at end of file