From 0259702df2b976bbb14db13513f6a392c1214f68 Mon Sep 17 00:00:00 2001 From: catbref Date: Tue, 21 May 2019 17:06:01 +0100 Subject: [PATCH] Fix generating X25519 shared secret. X25519 shared secrets now match those generated by libsodium. New tests show that shared secrets are the same using either set of private+public key combinations. Changed proxy private key generation from using simple SHA256 of shared secret to using SHA256(shared secret + both public keys). Added a temporary "BouncyCastle25519" shim class to provide missing key conversion from Ed25519 to X25519. --- .../org/qora/account/PrivateKeyAccount.java | 28 ++-- .../qora/api/resource/AddressesResource.java | 5 +- .../org/qora/crypto/BouncyCastle25519.java | 97 +++++++++++++ src/test/java/org/qora/test/CryptoTests.java | 136 +++++++++++++++--- .../java/org/qora/test/common/Common.java | 4 +- src/test/resources/proxy-key-example.html | 13 +- 6 files changed, 243 insertions(+), 40 deletions(-) create mode 100644 src/main/java/org/qora/crypto/BouncyCastle25519.java diff --git a/src/main/java/org/qora/account/PrivateKeyAccount.java b/src/main/java/org/qora/account/PrivateKeyAccount.java index f374b13b..d9f84cf4 100644 --- a/src/main/java/org/qora/account/PrivateKeyAccount.java +++ b/src/main/java/org/qora/account/PrivateKeyAccount.java @@ -5,16 +5,18 @@ import org.bouncycastle.crypto.params.Ed25519PublicKeyParameters; import org.bouncycastle.crypto.params.X25519PrivateKeyParameters; import org.bouncycastle.crypto.params.X25519PublicKeyParameters; import org.bouncycastle.math.ec.rfc8032.Ed25519; - +import org.qora.crypto.BouncyCastle25519; +import org.qora.crypto.Crypto; import org.qora.repository.Repository; -// TODO change "seed" to "privateKey" to keep things consistent +import com.google.common.primitives.Bytes; + public class PrivateKeyAccount extends PublicKeyAccount { private static final int SIGNATURE_LENGTH = 64; private static final int SHARED_SECRET_LENGTH = 32; - private final byte[] seed; + private final byte[] privateKey; private final Ed25519PrivateKeyParameters edPrivateKeyParams; /** @@ -36,12 +38,12 @@ public class PrivateKeyAccount extends PublicKeyAccount { private PrivateKeyAccount(Repository repository, Ed25519PrivateKeyParameters edPrivateKeyParams, Ed25519PublicKeyParameters edPublicKeyParams) { super(repository, edPublicKeyParams); - this.seed = edPrivateKeyParams.getEncoded(); + this.privateKey = edPrivateKeyParams.getEncoded(); this.edPrivateKeyParams = edPrivateKeyParams; } - public byte[] getSeed() { - return this.seed; + public byte[] getPrivateKey() { + return this.privateKey; } public byte[] sign(byte[] message) { @@ -53,8 +55,11 @@ public class PrivateKeyAccount extends PublicKeyAccount { } public byte[] getSharedSecret(byte[] publicKey) { - X25519PrivateKeyParameters xPrivateKeyParams = new X25519PrivateKeyParameters(this.seed, 0); - X25519PublicKeyParameters xPublicKeyParams = new X25519PublicKeyParameters(publicKey, 0); + byte[] x25519PrivateKey = BouncyCastle25519.toX25519PrivateKey(this.privateKey); + X25519PrivateKeyParameters xPrivateKeyParams = new X25519PrivateKeyParameters(x25519PrivateKey, 0); + + byte[] x25519PublicKey = BouncyCastle25519.toX25519PublicKey(publicKey); + X25519PublicKeyParameters xPublicKeyParams = new X25519PublicKeyParameters(x25519PublicKey, 0); byte[] sharedSecret = new byte[SHARED_SECRET_LENGTH]; xPrivateKeyParams.generateSecret(xPublicKeyParams, sharedSecret, 0); @@ -62,4 +67,11 @@ public class PrivateKeyAccount extends PublicKeyAccount { return sharedSecret; } + public byte[] getProxyPrivateKey(byte[] publicKey) { + byte[] sharedSecret = this.getSharedSecret(publicKey); + + byte[] proxyHashData = Bytes.concat(sharedSecret, this.getPublicKey(), publicKey); + return Crypto.digest(proxyHashData); + } + } diff --git a/src/main/java/org/qora/api/resource/AddressesResource.java b/src/main/java/org/qora/api/resource/AddressesResource.java index c6ba816a..63dd278d 100644 --- a/src/main/java/org/qora/api/resource/AddressesResource.java +++ b/src/main/java/org/qora/api/resource/AddressesResource.java @@ -338,11 +338,10 @@ public class AddressesResource { if (generatorKey.length != Transformer.PRIVATE_KEY_LENGTH || recipientKey.length != Transformer.PRIVATE_KEY_LENGTH) throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.INVALID_PRIVATE_KEY); PrivateKeyAccount generator = new PrivateKeyAccount(null, generatorKey); - byte[] sharedSecret = generator.getSharedSecret(recipientKey); - byte[] proxySeed = Crypto.digest(sharedSecret); + byte[] proxyPrivateKey = generator.getProxyPrivateKey(recipientKey); - return Base58.encode(proxySeed); + return Base58.encode(proxyPrivateKey); } catch (NumberFormatException e) { throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.INVALID_PRIVATE_KEY, e); } diff --git a/src/main/java/org/qora/crypto/BouncyCastle25519.java b/src/main/java/org/qora/crypto/BouncyCastle25519.java new file mode 100644 index 00000000..c5e8b824 --- /dev/null +++ b/src/main/java/org/qora/crypto/BouncyCastle25519.java @@ -0,0 +1,97 @@ +package org.qora.crypto; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.Arrays; + +import org.bouncycastle.crypto.Digest; +import org.bouncycastle.math.ec.rfc7748.X25519; +import org.bouncycastle.math.ec.rfc7748.X25519Field; +import org.bouncycastle.math.ec.rfc8032.Ed25519; + +/** Additions to BouncyCastle providing Ed25519 to X25519 key conversion. */ +public class BouncyCastle25519 { + + private static final Class pointExtClass; + private static final Constructor pointExtCtor; + private static final Method decodePointVarMethod; + private static final Field yField; + + static { + try { + Class ed25519Class = Ed25519.class; + pointExtClass = Arrays.stream(ed25519Class.getDeclaredClasses()).filter(clazz -> clazz.getSimpleName().equals("PointExt")).findFirst().get(); + if (pointExtClass == null) + throw new ClassNotFoundException("Can't locate PointExt inner class inside Ed25519"); + + decodePointVarMethod = ed25519Class.getDeclaredMethod("decodePointVar", byte[].class, int.class, boolean.class, pointExtClass); + decodePointVarMethod.setAccessible(true); + + pointExtCtor = pointExtClass.getDeclaredConstructors()[0]; + pointExtCtor.setAccessible(true); + + yField = pointExtClass.getDeclaredField("y"); + yField.setAccessible(true); + } catch (NoSuchMethodException | SecurityException | IllegalArgumentException | NoSuchFieldException | ClassNotFoundException e) { + throw new RuntimeException("Can't initialize BouncyCastle25519 shim", e); + } + } + + private static int[] obtainYFromPublicKey(byte[] ed25519PublicKey) { + try { + Object pA = pointExtCtor.newInstance(); + + Boolean result = (Boolean) decodePointVarMethod.invoke(null, ed25519PublicKey, 0, true, pA); + if (result == null || !result) + return null; + + return (int[]) yField.get(pA); + } catch (SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) { + throw new RuntimeException("Can't reflect into BouncyCastle", e); + } + } + + public static byte[] toX25519PublicKey(byte[] ed25519PublicKey) { + int[] one = new int[X25519Field.SIZE]; + X25519Field.one(one); + + int[] y = obtainYFromPublicKey(ed25519PublicKey); + + int[] oneMinusY = new int[X25519Field.SIZE]; + X25519Field.sub(one, y, oneMinusY); + + int[] onePlusY = new int[X25519Field.SIZE]; + X25519Field.add(one, y, onePlusY); + + int[] oneMinusYInverted = new int[X25519Field.SIZE]; + X25519Field.inv(oneMinusY, oneMinusYInverted); + + int[] u = new int[X25519Field.SIZE]; + X25519Field.mul(onePlusY, oneMinusYInverted, u); + + byte[] x25519PublicKey = new byte[X25519.SCALAR_SIZE]; + X25519Field.encode(u, x25519PublicKey, 0); + + return x25519PublicKey; + } + + public static byte[] toX25519PrivateKey(byte[] ed25519PrivateKey) { + Digest d = Ed25519.createPrehash(); + byte[] h = new byte[d.getDigestSize()]; + + d.update(ed25519PrivateKey, 0, ed25519PrivateKey.length); + d.doFinal(h, 0); + + byte[] s = new byte[X25519.SCALAR_SIZE]; + + System.arraycopy(h, 0, s, 0, X25519.SCALAR_SIZE); + s[0] &= 0xF8; + s[X25519.SCALAR_SIZE - 1] &= 0x7F; + s[X25519.SCALAR_SIZE - 1] |= 0x40; + + return s; + } + +} diff --git a/src/test/java/org/qora/test/CryptoTests.java b/src/test/java/org/qora/test/CryptoTests.java index 110bd9ac..083eb434 100644 --- a/src/test/java/org/qora/test/CryptoTests.java +++ b/src/test/java/org/qora/test/CryptoTests.java @@ -3,13 +3,13 @@ package org.qora.test; import org.junit.Test; import org.qora.account.PrivateKeyAccount; import org.qora.block.BlockChain; +import org.qora.crypto.BouncyCastle25519; import org.qora.crypto.Crypto; import org.qora.test.common.Common; import static org.junit.Assert.*; -import java.security.NoSuchAlgorithmException; -import java.security.NoSuchProviderException; +import java.security.SecureRandom; import org.bitcoinj.core.Base58; import org.bouncycastle.crypto.agreement.X25519Agreement; @@ -65,7 +65,7 @@ public class CryptoTests extends Common { } @Test - public void testBCseed() throws NoSuchAlgorithmException, NoSuchProviderException { + public void testBCseed() { final String privateKey58 = "A9MNsATgQgruBUjxy2rjWY36Yf19uRioKZbiLFT2P7c6"; final String publicKey58 = "2tiMr5LTpaWCgbRvkPK8TFd7k63DyHJMMFFsz9uBf1ZP"; @@ -74,48 +74,140 @@ public class CryptoTests extends Common { String expected58 = publicKey58; String actual58 = Base58.encode(account.getPublicKey()); - assertEquals("qora-core generated public key incorrect", expected58, actual58); + assertEquals("qora-core derived public key incorrect", expected58, actual58); Ed25519PrivateKeyParameters privateKeyParams = new Ed25519PrivateKeyParameters(privateKey, 0); Ed25519PublicKeyParameters publicKeyParams = privateKeyParams.generatePublicKey(); actual58 = Base58.encode(publicKeyParams.getEncoded()); - assertEquals("BouncyCastle generated public key incorrect", expected58, actual58); + assertEquals("BouncyCastle derived public key incorrect", expected58, actual58); + + final byte[] publicKey = Base58.decode(publicKey58); + publicKeyParams = new Ed25519PublicKeyParameters(publicKey, 0); + + actual58 = Base58.encode(publicKeyParams.getEncoded()); + assertEquals("BouncyCastle decoded public key incorrect", expected58, actual58); } - @Test - public void testBCSharedSecret() throws NoSuchAlgorithmException, NoSuchProviderException { - final byte[] ourPrivateKey = Base58.decode("A9MNsATgQgruBUjxy2rjWY36Yf19uRioKZbiLFT2P7c6"); - final byte[] theirPublicKey = Base58.decode("2sbcMmVKke5inS4yrbeoG6Cyw2mZCptQNjyWgnY4YHaF"); - final String expectedProxyPrivateKey = "EZhKy6wEh1ncQsvx6x3yV2sqjjsoU1bTTqrMcFLjLmp4"; + private static byte[] calcBCSharedSecret(byte[] ed25519PrivateKey, byte[] ed25519PublicKey) { + byte[] x25519PrivateKey = BouncyCastle25519.toX25519PrivateKey(ed25519PrivateKey); + X25519PrivateKeyParameters privateKeyParams = new X25519PrivateKeyParameters(x25519PrivateKey, 0); - X25519PrivateKeyParameters ourPrivateKeyParams = new X25519PrivateKeyParameters(ourPrivateKey, 0); - X25519PublicKeyParameters theirPublicKeyParams = new X25519PublicKeyParameters(theirPublicKey, 0); + byte[] x25519PublicKey = BouncyCastle25519.toX25519PublicKey(ed25519PublicKey); + X25519PublicKeyParameters publicKeyParams = new X25519PublicKeyParameters(x25519PublicKey, 0); byte[] sharedSecret = new byte[32]; X25519Agreement keyAgree = new X25519Agreement(); - keyAgree.init(ourPrivateKeyParams); - keyAgree.calculateAgreement(theirPublicKeyParams, sharedSecret, 0); + keyAgree.init(privateKeyParams); + keyAgree.calculateAgreement(publicKeyParams, sharedSecret, 0); - String proxyPrivateKey = Base58.encode(Crypto.digest(sharedSecret)); - - assertEquals("proxy private key incorrect", expectedProxyPrivateKey, proxyPrivateKey); + return sharedSecret; } @Test - public void testSharedSecret() throws NoSuchAlgorithmException, NoSuchProviderException { + public void testBCSharedSecret() { final byte[] ourPrivateKey = Base58.decode("A9MNsATgQgruBUjxy2rjWY36Yf19uRioKZbiLFT2P7c6"); - final byte[] theirPublicKey = Base58.decode("2sbcMmVKke5inS4yrbeoG6Cyw2mZCptQNjyWgnY4YHaF"); - final String expectedProxyPrivateKey = "EZhKy6wEh1ncQsvx6x3yV2sqjjsoU1bTTqrMcFLjLmp4"; + final byte[] theirPublicKey = Base58.decode("C6wuddsBV3HzRrXUtezE7P5MoRXp5m3mEDokRDGZB6ry"); + + final String expectedOurX25519PrivateKey = "HBPAUyWkrHt41s1a7yd6m7d1VswzLs4p9ob6AsqUQSCh"; + final String expectedTheirX25519PublicKey = "ANjnZLRSzW9B1aVamiYGKP3XtBooU9tGGDjUiibUfzp2"; + final String expectedSharedSecret = "DTMZYG96x8XZuGzDvHFByVLsXedimqtjiXHhXPVe58Ap"; + + byte[] ourX25519PrivateKey = BouncyCastle25519.toX25519PrivateKey(ourPrivateKey); + assertEquals("X25519 private key incorrect", expectedOurX25519PrivateKey, Base58.encode(ourX25519PrivateKey)); + + byte[] theirX25519PublicKey = BouncyCastle25519.toX25519PublicKey(theirPublicKey); + assertEquals("X25519 public key incorrect", expectedTheirX25519PublicKey, Base58.encode(theirX25519PublicKey)); + + byte[] sharedSecret = calcBCSharedSecret(ourPrivateKey, theirPublicKey); + + assertEquals("shared secret incorrect", expectedSharedSecret, Base58.encode(sharedSecret)); + } + + @Test + public void testSharedSecret() { + final byte[] ourPrivateKey = Base58.decode("A9MNsATgQgruBUjxy2rjWY36Yf19uRioKZbiLFT2P7c6"); + final byte[] theirPublicKey = Base58.decode("C6wuddsBV3HzRrXUtezE7P5MoRXp5m3mEDokRDGZB6ry"); + final String expectedSharedSecret = "DTMZYG96x8XZuGzDvHFByVLsXedimqtjiXHhXPVe58Ap"; PrivateKeyAccount generator = new PrivateKeyAccount(null, ourPrivateKey); byte[] sharedSecret = generator.getSharedSecret(theirPublicKey); - String proxyPrivateKey = Base58.encode(Crypto.digest(sharedSecret)); + assertEquals("shared secret incorrect", expectedSharedSecret, Base58.encode(sharedSecret)); + } - assertEquals("proxy private key incorrect", expectedProxyPrivateKey, proxyPrivateKey); + @Test + public void testSharedSecretMatchesBC() { + final byte[] ourPrivateKey = Base58.decode("A9MNsATgQgruBUjxy2rjWY36Yf19uRioKZbiLFT2P7c6"); + final byte[] theirPublicKey = Base58.decode("C6wuddsBV3HzRrXUtezE7P5MoRXp5m3mEDokRDGZB6ry"); + final String expectedSharedSecret = "DTMZYG96x8XZuGzDvHFByVLsXedimqtjiXHhXPVe58Ap"; + + PrivateKeyAccount generator = new PrivateKeyAccount(null, ourPrivateKey); + + byte[] ourSharedSecret = generator.getSharedSecret(theirPublicKey); + + assertEquals("shared secret incorrect", expectedSharedSecret, Base58.encode(ourSharedSecret)); + + byte[] bcSharedSecret = calcBCSharedSecret(ourPrivateKey, theirPublicKey); + + assertEquals("shared secrets do not match", Base58.encode(ourSharedSecret), Base58.encode(bcSharedSecret)); + } + + @Test + public void testRandomBCSharedSecret2() { + // Check shared secret is the same generated from either set of private/public keys + SecureRandom random = new SecureRandom(); + + X25519PrivateKeyParameters ourPrivateKeyParams = new X25519PrivateKeyParameters(random); + X25519PrivateKeyParameters theirPrivateKeyParams = new X25519PrivateKeyParameters(random); + + X25519PublicKeyParameters ourPublicKeyParams = ourPrivateKeyParams.generatePublicKey(); + X25519PublicKeyParameters theirPublicKeyParams = theirPrivateKeyParams.generatePublicKey(); + + byte[] ourSharedSecret = new byte[32]; + + X25519Agreement keyAgree = new X25519Agreement(); + keyAgree.init(ourPrivateKeyParams); + keyAgree.calculateAgreement(theirPublicKeyParams, ourSharedSecret, 0); + + byte[] theirSharedSecret = new byte[32]; + + keyAgree = new X25519Agreement(); + keyAgree.init(theirPrivateKeyParams); + keyAgree.calculateAgreement(ourPublicKeyParams, theirSharedSecret, 0); + + assertEquals("shared secrets do not match", Base58.encode(ourSharedSecret), Base58.encode(theirSharedSecret)); + } + + @Test + public void testBCSharedSecret2() { + // Check shared secret is the same generated from either set of private/public keys + final byte[] ourPrivateKey = Base58.decode("A9MNsATgQgruBUjxy2rjWY36Yf19uRioKZbiLFT2P7c6"); + final byte[] ourPublicKey = Base58.decode("2tiMr5LTpaWCgbRvkPK8TFd7k63DyHJMMFFsz9uBf1ZP"); + + final byte[] theirPrivateKey = Base58.decode("AdTd9SUEYSdTW8mgK3Gu72K97bCHGdUwi2VvLNjUohot"); + final byte[] theirPublicKey = Base58.decode("C6wuddsBV3HzRrXUtezE7P5MoRXp5m3mEDokRDGZB6ry"); + + byte[] ourSharedSecret = calcBCSharedSecret(ourPrivateKey, theirPublicKey); + + byte[] theirSharedSecret = calcBCSharedSecret(theirPrivateKey, ourPublicKey); + + assertEquals("shared secrets do not match", Base58.encode(ourSharedSecret), Base58.encode(theirSharedSecret)); + } + + @Test + public void testProxyKeys() { + final byte[] ourPrivateKey = Base58.decode("A9MNsATgQgruBUjxy2rjWY36Yf19uRioKZbiLFT2P7c6"); + final byte[] theirPublicKey = Base58.decode("C6wuddsBV3HzRrXUtezE7P5MoRXp5m3mEDokRDGZB6ry"); + + final String expectedProxyPrivateKey = "CwBXkJRRaGzWRvdE9vewVUbcYNSVrcTpunNWm8zidArZ"; + + PrivateKeyAccount mintingAccount = new PrivateKeyAccount(null, ourPrivateKey); + byte[] proxyPrivateKey = mintingAccount.getProxyPrivateKey(theirPublicKey); + + assertEquals(expectedProxyPrivateKey, Base58.encode(proxyPrivateKey)); } } diff --git a/src/test/java/org/qora/test/common/Common.java b/src/test/java/org/qora/test/common/Common.java index 86825a56..56a435de 100644 --- a/src/test/java/org/qora/test/common/Common.java +++ b/src/test/java/org/qora/test/common/Common.java @@ -78,11 +78,11 @@ public class Common { } public static TestAccount getTestAccount(Repository repository, String name) { - return new TestAccount(repository, name, testAccountsByName.get(name).getSeed()); + return new TestAccount(repository, name, testAccountsByName.get(name).getPrivateKey()); } public static List getTestAccounts(Repository repository) { - return testAccountsByName.values().stream().map(account -> new TestAccount(repository, account.accountName, account.getSeed())).collect(Collectors.toList()); + return testAccountsByName.values().stream().map(account -> new TestAccount(repository, account.accountName, account.getPrivateKey())).collect(Collectors.toList()); } public static void useSettings(String settingsFilename) throws DataException { diff --git a/src/test/resources/proxy-key-example.html b/src/test/resources/proxy-key-example.html index a994a485..900167a5 100644 --- a/src/test/resources/proxy-key-example.html +++ b/src/test/resources/proxy-key-example.html @@ -6,20 +6,23 @@