From 56a5b8df8737afccb37672632b03308665bd3717 Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Fri, 2 Feb 2018 17:52:18 +0000
Subject: [PATCH] Use Curve25519 for key agreement.
---
.../bramble/api/crypto/CryptoConstants.java | 2 +-
bramble-core/build.gradle | 2 +-
.../bramble/crypto/CryptoComponentImpl.java | 54 +++++++------------
.../bramble/crypto/Curve25519KeyParser.java | 26 +++++++++
.../bramble/crypto/Curve25519PrivateKey.java | 18 +++++++
.../bramble/crypto/Curve25519PublicKey.java | 18 +++++++
.../crypto/EllipticCurveConstants.java | 32 -----------
.../EllipticCurveMultiplicationTest.java | 23 ++++----
.../crypto/EllipticCurvePerformanceTest.java | 2 -
.../bramble/crypto/KeyAgreementTest.java | 29 +++++++---
.../crypto/KeyEncodingAndParsingTest.java | 2 +-
11 files changed, 120 insertions(+), 88 deletions(-)
create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519KeyParser.java
create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519PrivateKey.java
create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519PublicKey.java
delete mode 100644 bramble-core/src/main/java/org/briarproject/bramble/crypto/EllipticCurveConstants.java
diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoConstants.java b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoConstants.java
index d221c85c5f..3b12c5cf75 100644
--- a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoConstants.java
+++ b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoConstants.java
@@ -5,7 +5,7 @@ public interface CryptoConstants {
/**
* The maximum length of an agreement public key in bytes.
*/
- int MAX_AGREEMENT_PUBLIC_KEY_BYTES = 65;
+ int MAX_AGREEMENT_PUBLIC_KEY_BYTES = 32;
/**
* The maximum length of a signature public key in bytes.
diff --git a/bramble-core/build.gradle b/bramble-core/build.gradle
index 84f2acc874..a6189c910f 100644
--- a/bramble-core/build.gradle
+++ b/bramble-core/build.gradle
@@ -12,6 +12,7 @@ dependencies {
implementation 'com.h2database:h2:1.4.192' // The last version that supports Java 1.6
implementation 'org.bitlet:weupnp:0.1.4'
implementation 'net.i2p.crypto:eddsa:0.2.0'
+ implementation 'org.whispersystems:curve25519-java:0.4.1'
apt 'com.google.dagger:dagger-compiler:2.0.2'
@@ -23,7 +24,6 @@ dependencies {
testImplementation "org.jmock:jmock-legacy:2.8.2"
testImplementation "org.hamcrest:hamcrest-library:1.3"
testImplementation "org.hamcrest:hamcrest-core:1.3"
- testImplementation "org.whispersystems:curve25519-java:0.4.1"
testApt 'com.google.dagger:dagger-compiler:2.0.2'
}
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java
index 4a0a3e4a6f..4161db31ab 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java
@@ -14,15 +14,11 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
import org.briarproject.bramble.api.system.SecureRandomProvider;
import org.briarproject.bramble.util.ByteUtils;
import org.briarproject.bramble.util.StringUtils;
-import org.spongycastle.crypto.AsymmetricCipherKeyPair;
import org.spongycastle.crypto.CryptoException;
import org.spongycastle.crypto.Digest;
-import org.spongycastle.crypto.agreement.ECDHCBasicAgreement;
import org.spongycastle.crypto.digests.Blake2bDigest;
-import org.spongycastle.crypto.generators.ECKeyPairGenerator;
-import org.spongycastle.crypto.params.ECKeyGenerationParameters;
-import org.spongycastle.crypto.params.ECPrivateKeyParameters;
-import org.spongycastle.crypto.params.ECPublicKeyParameters;
+import org.whispersystems.curve25519.Curve25519;
+import org.whispersystems.curve25519.Curve25519KeyPair;
import java.security.GeneralSecurityException;
import java.security.NoSuchAlgorithmException;
@@ -35,7 +31,6 @@ import javax.annotation.Nullable;
import javax.inject.Inject;
import static java.util.logging.Level.INFO;
-import static org.briarproject.bramble.crypto.EllipticCurveConstants.PARAMETERS;
import static org.briarproject.bramble.util.ByteUtils.INT_32_BYTES;
@NotNullByDefault
@@ -44,7 +39,6 @@ class CryptoComponentImpl implements CryptoComponent {
private static final Logger LOG =
Logger.getLogger(CryptoComponentImpl.class.getName());
- private static final int AGREEMENT_KEY_PAIR_BITS = 256;
private static final int SIGNATURE_KEY_PAIR_BITS = 256;
private static final int STORAGE_IV_BYTES = 24; // 196 bits
private static final int PBKDF_SALT_BYTES = 32; // 256 bits
@@ -52,7 +46,7 @@ class CryptoComponentImpl implements CryptoComponent {
private final SecureRandom secureRandom;
private final PasswordBasedKdf passwordBasedKdf;
- private final ECKeyPairGenerator agreementKeyPairGenerator;
+ private final Curve25519 curve25519;
private final KeyPairGenerator signatureKeyPairGenerator;
private final KeyParser agreementKeyParser, signatureKeyParser;
private final MessageEncrypter messageEncrypter;
@@ -80,15 +74,11 @@ class CryptoComponentImpl implements CryptoComponent {
}
secureRandom = new SecureRandom();
this.passwordBasedKdf = passwordBasedKdf;
- ECKeyGenerationParameters params = new ECKeyGenerationParameters(
- PARAMETERS, secureRandom);
- agreementKeyPairGenerator = new ECKeyPairGenerator();
- agreementKeyPairGenerator.init(params);
+ curve25519 = Curve25519.getInstance("java");
signatureKeyPairGenerator = new KeyPairGenerator();
signatureKeyPairGenerator.initialize(SIGNATURE_KEY_PAIR_BITS,
secureRandom);
- agreementKeyParser = new Sec1KeyParser(PARAMETERS,
- AGREEMENT_KEY_PAIR_BITS);
+ agreementKeyParser = new Curve25519KeyParser();
signatureKeyParser = new EdKeyParser();
messageEncrypter = new MessageEncrypter(secureRandom);
}
@@ -133,16 +123,17 @@ class CryptoComponentImpl implements CryptoComponent {
// Package access for testing
byte[] performRawKeyAgreement(PrivateKey priv, PublicKey pub)
throws GeneralSecurityException {
- if (!(priv instanceof Sec1PrivateKey))
+ if (!(priv instanceof Curve25519PrivateKey))
throw new IllegalArgumentException();
- if (!(pub instanceof Sec1PublicKey))
+ if (!(pub instanceof Curve25519PublicKey))
throw new IllegalArgumentException();
- ECPrivateKeyParameters ecPriv = ((Sec1PrivateKey) priv).getKey();
- ECPublicKeyParameters ecPub = ((Sec1PublicKey) pub).getKey();
long now = System.currentTimeMillis();
- ECDHCBasicAgreement agreement = new ECDHCBasicAgreement();
- agreement.init(ecPriv);
- byte[] secret = agreement.calculateAgreement(ecPub).toByteArray();
+ byte[] secret = curve25519.calculateAgreement(pub.getEncoded(),
+ priv.getEncoded());
+ // If the shared secret is all zeroes, the public key is invalid
+ byte allZero = 0;
+ for (byte b : secret) allZero |= b;
+ if (allZero == 0) throw new GeneralSecurityException();
long duration = System.currentTimeMillis() - now;
if (LOG.isLoggable(INFO))
LOG.info("Deriving shared secret took " + duration + " ms");
@@ -151,18 +142,10 @@ class CryptoComponentImpl implements CryptoComponent {
@Override
public KeyPair generateAgreementKeyPair() {
- AsymmetricCipherKeyPair keyPair =
- agreementKeyPairGenerator.generateKeyPair();
- // Return a wrapper that uses the SEC 1 encoding
- ECPublicKeyParameters ecPublicKey =
- (ECPublicKeyParameters) keyPair.getPublic();
- PublicKey publicKey = new Sec1PublicKey(ecPublicKey
- );
- ECPrivateKeyParameters ecPrivateKey =
- (ECPrivateKeyParameters) keyPair.getPrivate();
- PrivateKey privateKey = new Sec1PrivateKey(ecPrivateKey,
- AGREEMENT_KEY_PAIR_BITS);
- return new KeyPair(publicKey, privateKey);
+ Curve25519KeyPair keyPair = curve25519.generateKeyPair();
+ PublicKey pub = new Curve25519PublicKey(keyPair.getPublicKey());
+ PrivateKey priv = new Curve25519PrivateKey(keyPair.getPrivateKey());
+ return new KeyPair(pub, priv);
}
@Override
@@ -172,7 +155,8 @@ class CryptoComponentImpl implements CryptoComponent {
@Override
public KeyPair generateSignatureKeyPair() {
- java.security.KeyPair keyPair = signatureKeyPairGenerator.generateKeyPair();
+ java.security.KeyPair keyPair =
+ signatureKeyPairGenerator.generateKeyPair();
EdDSAPublicKey edPublicKey = (EdDSAPublicKey) keyPair.getPublic();
PublicKey publicKey = new EdPublicKey(edPublicKey.getAbyte());
EdDSAPrivateKey edPrivateKey = (EdDSAPrivateKey) keyPair.getPrivate();
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519KeyParser.java b/bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519KeyParser.java
new file mode 100644
index 0000000000..d7aeac26a2
--- /dev/null
+++ b/bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519KeyParser.java
@@ -0,0 +1,26 @@
+package org.briarproject.bramble.crypto;
+
+import org.briarproject.bramble.api.crypto.KeyParser;
+import org.briarproject.bramble.api.crypto.PrivateKey;
+import org.briarproject.bramble.api.crypto.PublicKey;
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+
+import java.security.GeneralSecurityException;
+
+@NotNullByDefault
+class Curve25519KeyParser implements KeyParser {
+
+ @Override
+ public PublicKey parsePublicKey(byte[] encodedKey)
+ throws GeneralSecurityException {
+ if (encodedKey.length != 32) throw new GeneralSecurityException();
+ return new Curve25519PublicKey(encodedKey);
+ }
+
+ @Override
+ public PrivateKey parsePrivateKey(byte[] encodedKey)
+ throws GeneralSecurityException {
+ if (encodedKey.length != 32) throw new GeneralSecurityException();
+ return new Curve25519PrivateKey(encodedKey);
+ }
+}
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519PrivateKey.java b/bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519PrivateKey.java
new file mode 100644
index 0000000000..b7f1a0cc51
--- /dev/null
+++ b/bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519PrivateKey.java
@@ -0,0 +1,18 @@
+package org.briarproject.bramble.crypto;
+
+import org.briarproject.bramble.api.Bytes;
+import org.briarproject.bramble.api.crypto.PrivateKey;
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+
+@NotNullByDefault
+class Curve25519PrivateKey extends Bytes implements PrivateKey {
+
+ Curve25519PrivateKey(byte[] bytes) {
+ super(bytes);
+ }
+
+ @Override
+ public byte[] getEncoded() {
+ return getBytes();
+ }
+}
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519PublicKey.java b/bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519PublicKey.java
new file mode 100644
index 0000000000..94d703d01e
--- /dev/null
+++ b/bramble-core/src/main/java/org/briarproject/bramble/crypto/Curve25519PublicKey.java
@@ -0,0 +1,18 @@
+package org.briarproject.bramble.crypto;
+
+import org.briarproject.bramble.api.Bytes;
+import org.briarproject.bramble.api.crypto.PublicKey;
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+
+@NotNullByDefault
+class Curve25519PublicKey extends Bytes implements PublicKey {
+
+ Curve25519PublicKey(byte[] bytes) {
+ super(bytes);
+ }
+
+ @Override
+ public byte[] getEncoded() {
+ return getBytes();
+ }
+}
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/crypto/EllipticCurveConstants.java b/bramble-core/src/main/java/org/briarproject/bramble/crypto/EllipticCurveConstants.java
deleted file mode 100644
index e4b4b04e88..0000000000
--- a/bramble-core/src/main/java/org/briarproject/bramble/crypto/EllipticCurveConstants.java
+++ /dev/null
@@ -1,32 +0,0 @@
-package org.briarproject.bramble.crypto;
-
-import org.spongycastle.asn1.teletrust.TeleTrusTNamedCurves;
-import org.spongycastle.asn1.x9.X9ECParameters;
-import org.spongycastle.crypto.params.ECDomainParameters;
-import org.spongycastle.math.ec.ECCurve;
-import org.spongycastle.math.ec.ECMultiplier;
-import org.spongycastle.math.ec.ECPoint;
-import org.spongycastle.math.ec.MontgomeryLadderMultiplier;
-
-import java.math.BigInteger;
-
-/**
- * Parameters for curve brainpoolp256r1 - see RFC 5639.
- */
-class EllipticCurveConstants {
-
- static final ECDomainParameters PARAMETERS;
-
- static {
- // Start with the default implementation of the curve
- X9ECParameters x9 = TeleTrusTNamedCurves.getByName("brainpoolp256r1");
- // Use a constant-time multiplier
- ECMultiplier monty = new MontgomeryLadderMultiplier();
- ECCurve curve = x9.getCurve().configure().setMultiplier(monty).create();
- BigInteger gX = x9.getG().getAffineXCoord().toBigInteger();
- BigInteger gY = x9.getG().getAffineYCoord().toBigInteger();
- ECPoint g = curve.createPoint(gX, gY);
- // Convert to ECDomainParameters using the new multiplier
- PARAMETERS = new ECDomainParameters(curve, g, x9.getN(), x9.getH());
- }
-}
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/crypto/EllipticCurveMultiplicationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/crypto/EllipticCurveMultiplicationTest.java
index 9be3dc0ddc..b4e6390478 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/crypto/EllipticCurveMultiplicationTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/crypto/EllipticCurveMultiplicationTest.java
@@ -13,11 +13,11 @@ import org.spongycastle.crypto.params.ECPrivateKeyParameters;
import org.spongycastle.crypto.params.ECPublicKeyParameters;
import org.spongycastle.math.ec.ECCurve;
import org.spongycastle.math.ec.ECPoint;
+import org.spongycastle.math.ec.MontgomeryLadderMultiplier;
import java.math.BigInteger;
import java.security.SecureRandom;
-import static org.briarproject.bramble.crypto.EllipticCurveConstants.PARAMETERS;
import static org.junit.Assert.assertEquals;
public class EllipticCurveMultiplicationTest extends BrambleTestCase {
@@ -31,15 +31,11 @@ public class EllipticCurveMultiplicationTest extends BrambleTestCase {
ECPoint defaultG = defaultX9Parameters.getG();
BigInteger defaultN = defaultX9Parameters.getN();
BigInteger defaultH = defaultX9Parameters.getH();
- // Check that the default parameters are equal to our parameters
- assertEquals(PARAMETERS.getCurve(), defaultCurve);
- assertEquals(PARAMETERS.getG(), defaultG);
- assertEquals(PARAMETERS.getN(), defaultN);
- assertEquals(PARAMETERS.getH(), defaultH);
- // ECDomainParameters doesn't have an equals() method, but it's just a
- // container for the parameters
ECDomainParameters defaultParameters = new ECDomainParameters(
defaultCurve, defaultG, defaultN, defaultH);
+ // Instantiate an implementation using the Montgomery ladder multiplier
+ ECDomainParameters montgomeryParameters =
+ constantTime(defaultParameters);
// Generate two key pairs with each set of parameters, using the same
// deterministic PRNG for both sets of parameters
byte[] seed = new byte[32];
@@ -47,7 +43,7 @@ public class EllipticCurveMultiplicationTest extends BrambleTestCase {
// Montgomery ladder multiplier
SecureRandom random = new PseudoSecureRandom(seed);
ECKeyGenerationParameters montgomeryGeneratorParams =
- new ECKeyGenerationParameters(PARAMETERS, random);
+ new ECKeyGenerationParameters(montgomeryParameters, random);
ECKeyPairGenerator montgomeryGenerator = new ECKeyPairGenerator();
montgomeryGenerator.init(montgomeryGeneratorParams);
AsymmetricCipherKeyPair montgomeryKeyPair1 =
@@ -107,4 +103,13 @@ public class EllipticCurveMultiplicationTest extends BrambleTestCase {
assertEquals(sharedSecretMontgomeryMontgomery,
sharedSecretDefaultDefault);
}
+
+ private static ECDomainParameters constantTime(ECDomainParameters in) {
+ ECCurve curve = in.getCurve().configure().setMultiplier(
+ new MontgomeryLadderMultiplier()).create();
+ BigInteger x = in.getG().getAffineXCoord().toBigInteger();
+ BigInteger y = in.getG().getAffineYCoord().toBigInteger();
+ ECPoint g = curve.createPoint(x, y);
+ return new ECDomainParameters(curve, g, in.getN(), in.getH());
+ }
}
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/crypto/EllipticCurvePerformanceTest.java b/bramble-core/src/test/java/org/briarproject/bramble/crypto/EllipticCurvePerformanceTest.java
index 0909bdc4e1..abf2798b1c 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/crypto/EllipticCurvePerformanceTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/crypto/EllipticCurvePerformanceTest.java
@@ -38,7 +38,6 @@ import java.util.Collections;
import java.util.List;
import static net.i2p.crypto.eddsa.EdDSAEngine.SIGNATURE_ALGORITHM;
-import static org.briarproject.bramble.crypto.EllipticCurveConstants.PARAMETERS;
// Not a JUnit test
public class EllipticCurvePerformanceTest {
@@ -65,7 +64,6 @@ public class EllipticCurvePerformanceTest {
runTest(name + " default", params);
runTest(name + " constant", constantTime(params));
}
- runTest("ours", PARAMETERS);
runCurve25519Test();
runEd25519Test();
}
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyAgreementTest.java b/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyAgreementTest.java
index d035bbe4b3..a086046c36 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyAgreementTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyAgreementTest.java
@@ -2,11 +2,13 @@ package org.briarproject.bramble.crypto;
import org.briarproject.bramble.api.crypto.CryptoComponent;
import org.briarproject.bramble.api.crypto.KeyPair;
+import org.briarproject.bramble.api.crypto.PublicKey;
import org.briarproject.bramble.api.crypto.SecretKey;
import org.briarproject.bramble.test.BrambleTestCase;
import org.briarproject.bramble.test.TestSecureRandomProvider;
import org.junit.Test;
+import java.security.GeneralSecurityException;
import java.util.Random;
import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.SHARED_SECRET_LABEL;
@@ -15,20 +17,33 @@ import static org.junit.Assert.assertArrayEquals;
public class KeyAgreementTest extends BrambleTestCase {
- @Test
- public void testDeriveSharedSecret() throws Exception {
- CryptoComponent crypto =
- new CryptoComponentImpl(new TestSecureRandomProvider(), null);
- KeyPair aPair = crypto.generateAgreementKeyPair();
- KeyPair bPair = crypto.generateAgreementKeyPair();
+ private final CryptoComponent crypto =
+ new CryptoComponentImpl(new TestSecureRandomProvider(), null);
+ private final byte[][] inputs;
+
+ public KeyAgreementTest() {
Random random = new Random();
- byte[][] inputs = new byte[random.nextInt(10) + 1][];
+ inputs = new byte[random.nextInt(10) + 1][];
for (int i = 0; i < inputs.length; i++)
inputs[i] = getRandomBytes(random.nextInt(256));
+ }
+
+ @Test
+ public void testDerivesSharedSecret() throws Exception {
+ KeyPair aPair = crypto.generateAgreementKeyPair();
+ KeyPair bPair = crypto.generateAgreementKeyPair();
SecretKey aShared = crypto.deriveSharedSecret(SHARED_SECRET_LABEL,
bPair.getPublic(), aPair, inputs);
SecretKey bShared = crypto.deriveSharedSecret(SHARED_SECRET_LABEL,
aPair.getPublic(), bPair, inputs);
assertArrayEquals(aShared.getBytes(), bShared.getBytes());
}
+
+ @Test(expected = GeneralSecurityException.class)
+ public void testRejectsInvalidPublicKey() throws Exception {
+ KeyPair keyPair = crypto.generateAgreementKeyPair();
+ PublicKey invalid = new Curve25519PublicKey(new byte[32]);
+ crypto.deriveSharedSecret(SHARED_SECRET_LABEL, invalid, keyPair,
+ inputs);
+ }
}
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyEncodingAndParsingTest.java b/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyEncodingAndParsingTest.java
index 1f2cad4dc7..30d62ef8cd 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyEncodingAndParsingTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyEncodingAndParsingTest.java
@@ -26,7 +26,7 @@ public class KeyEncodingAndParsingTest extends BrambleTestCase {
public void testAgreementPublicKeyLength() throws Exception {
// Generate 10 agreement key pairs
for (int i = 0; i < 10; i++) {
- KeyPair keyPair = crypto.generateSignatureKeyPair();
+ KeyPair keyPair = crypto.generateAgreementKeyPair();
// Check the length of the public key
byte[] publicKey = keyPair.getPublic().getEncoded();
assertTrue(publicKey.length <= MAX_AGREEMENT_PUBLIC_KEY_BYTES);
--
GitLab