diff --git a/briar-core/src/net/sf/briar/crypto/Sec1KeyParser.java b/briar-core/src/net/sf/briar/crypto/Sec1KeyParser.java index 522a5543aa6da00191b6270c56716cc9346897e9..729e109154ff5d985da4bb1fea72613b58884717 100644 --- a/briar-core/src/net/sf/briar/crypto/Sec1KeyParser.java +++ b/briar-core/src/net/sf/briar/crypto/Sec1KeyParser.java @@ -28,7 +28,7 @@ class Sec1KeyParser implements KeyParser { this.params = params; this.modulus = modulus; this.keyBits = keyBits; - bytesPerInt = (int) Math.ceil(keyBits / 8.0); + bytesPerInt = keyBits + 7 / 8; publicKeyBytes = 1 + 2 * bytesPerInt; privateKeyBytes = bytesPerInt; } @@ -47,7 +47,7 @@ class Sec1KeyParser implements KeyParser { if(x.compareTo(modulus) >= 0) throw new GeneralSecurityException(); // The y co-ordinate must be >= 0 and < q byte[] yBytes = new byte[bytesPerInt]; - System.arraycopy(encodedKey, bytesPerInt + 1, yBytes, 0, bytesPerInt); + System.arraycopy(encodedKey, 1 + bytesPerInt, yBytes, 0, bytesPerInt); BigInteger y = new BigInteger(1, yBytes); // Positive signum if(y.compareTo(modulus) >= 0) throw new GeneralSecurityException(); // Verify that y^2 == x^3 + ax + b (mod p) diff --git a/briar-core/src/net/sf/briar/crypto/Sec1PrivateKey.java b/briar-core/src/net/sf/briar/crypto/Sec1PrivateKey.java index fc346e839e3dcef0dd7a5e696d50c03c8a4af5f1..e6a0fc0f6e512be4367e0cccb82f6267a8c37066 100644 --- a/briar-core/src/net/sf/briar/crypto/Sec1PrivateKey.java +++ b/briar-core/src/net/sf/briar/crypto/Sec1PrivateKey.java @@ -1,7 +1,5 @@ package net.sf.briar.crypto; -import java.math.BigInteger; - import net.sf.briar.api.crypto.PrivateKey; import org.spongycastle.crypto.params.ECPrivateKeyParameters; @@ -9,20 +7,17 @@ import org.spongycastle.crypto.params.ECPrivateKeyParameters; class Sec1PrivateKey implements PrivateKey { private final ECPrivateKeyParameters key; - private final int privateKeyBytes; + private final int bytesPerInt; Sec1PrivateKey(ECPrivateKeyParameters key, int keyBits) { this.key = key; - privateKeyBytes = (int) Math.ceil(keyBits / 8.0); + bytesPerInt = keyBits + 7 / 8; } public byte[] getEncoded() { - byte[] encodedKey = new byte[privateKeyBytes]; - BigInteger d = key.getD(); - // Copy up to privateKeyBytes bytes into exactly privateKeyBytes bytes - byte[] dBytes = d.toByteArray(); - for(int i = 0; i < dBytes.length && i < privateKeyBytes; i++) - encodedKey[privateKeyBytes - 1 - i] = dBytes[dBytes.length - 1 - i]; + byte[] encodedKey = new byte[bytesPerInt]; + byte[] d = key.getD().toByteArray(); + Sec1Utils.convertToFixedLength(d, encodedKey, bytesPerInt, 0); return encodedKey; } diff --git a/briar-core/src/net/sf/briar/crypto/Sec1PublicKey.java b/briar-core/src/net/sf/briar/crypto/Sec1PublicKey.java index 2cb9a213c8038c54b0ce373ce4b1fcf6636fa023..a51ded2693e25630768e0dcf50b86926fa51eabd 100644 --- a/briar-core/src/net/sf/briar/crypto/Sec1PublicKey.java +++ b/briar-core/src/net/sf/briar/crypto/Sec1PublicKey.java @@ -1,7 +1,5 @@ package net.sf.briar.crypto; -import java.math.BigInteger; - import net.sf.briar.api.crypto.PublicKey; import org.spongycastle.crypto.params.ECPublicKeyParameters; @@ -18,22 +16,18 @@ class Sec1PublicKey implements PublicKey { Sec1PublicKey(ECPublicKeyParameters key, int keyBits) { this.key = key; - bytesPerInt = (int) Math.ceil(keyBits / 8.0); + bytesPerInt = keyBits + 7 / 8; publicKeyBytes = 1 + 2 * bytesPerInt; } public byte[] getEncoded() { byte[] encodedKey = new byte[publicKeyBytes]; encodedKey[0] = 4; - BigInteger x = key.getQ().getX().toBigInteger(); - BigInteger y = key.getQ().getY().toBigInteger(); - // Copy up to bytesPerInt bytes into exactly bytesPerInt bytes - byte[] xBytes = x.toByteArray(); - for(int i = 0; i < xBytes.length && i < bytesPerInt; i++) - encodedKey[bytesPerInt - i] = xBytes[xBytes.length - 1 - i]; - byte[] yBytes = y.toByteArray(); - for(int i = 0; i < yBytes.length && i < bytesPerInt; i++) - encodedKey[2 * bytesPerInt - i] = yBytes[yBytes.length - 1 - i]; + byte[] x = key.getQ().getX().toBigInteger().toByteArray(); + Sec1Utils.convertToFixedLength(x, encodedKey, bytesPerInt, 1); + byte[] y = key.getQ().getY().toBigInteger().toByteArray(); + Sec1Utils.convertToFixedLength(y, encodedKey, bytesPerInt, + 1 + bytesPerInt); return encodedKey; } diff --git a/briar-core/src/net/sf/briar/crypto/Sec1Utils.java b/briar-core/src/net/sf/briar/crypto/Sec1Utils.java new file mode 100644 index 0000000000000000000000000000000000000000..debb6cb8657e9db735fe3eef136aed07331fb966 --- /dev/null +++ b/briar-core/src/net/sf/briar/crypto/Sec1Utils.java @@ -0,0 +1,15 @@ +package net.sf.briar.crypto; + +class Sec1Utils { + + static void convertToFixedLength(byte[] src, byte[] dest, int destLen, + int destOff) { + if(src.length < destLen) { + destOff += destLen - src.length; + System.arraycopy(src, 0, dest, destOff, src.length); + } else { + int srcOff = src.length - destLen; + System.arraycopy(src, srcOff, dest, destOff, destLen); + } + } +} diff --git a/briar-tests/src/net/sf/briar/crypto/KeyEncodingAndParsingTest.java b/briar-tests/src/net/sf/briar/crypto/KeyEncodingAndParsingTest.java index d46e9ee7bc248f9ceb3f13527aa06dff600fad42..d71dd3491c5899902feca25b46ae3fd278178152 100644 --- a/briar-tests/src/net/sf/briar/crypto/KeyEncodingAndParsingTest.java +++ b/briar-tests/src/net/sf/briar/crypto/KeyEncodingAndParsingTest.java @@ -1,6 +1,10 @@ package net.sf.briar.crypto; import static org.junit.Assert.assertArrayEquals; + +import java.security.GeneralSecurityException; +import java.util.Random; + import net.sf.briar.BriarTestCase; import net.sf.briar.api.crypto.KeyPair; import net.sf.briar.api.crypto.KeyParser; @@ -47,6 +51,35 @@ public class KeyEncodingAndParsingTest extends BriarTestCase { assertArrayEquals(secret, secret1); } + @Test + public void testAgreementKeyParserByFuzzing() throws Exception { + KeyParser parser = crypto.getAgreementKeyParser(); + // Generate a key pair to get the proper public key length + KeyPair p = crypto.generateAgreementKeyPair(); + int pubLength = p.getPublic().getEncoded().length; + int privLength = p.getPrivate().getEncoded().length; + // Parse some random byte arrays - expect GeneralSecurityException + Random random = new Random(); + byte[] pubFuzz = new byte[pubLength]; + byte[] privFuzz = new byte[privLength]; + for(int i = 0; i < 1000; i++) { + random.nextBytes(pubFuzz); + try { + parser.parsePublicKey(pubFuzz); + } catch(GeneralSecurityException expected) { + } catch(Exception e) { + fail(); + } + random.nextBytes(privFuzz); + try { + parser.parsePrivateKey(privFuzz); + } catch(GeneralSecurityException expected) { + } catch(Exception e) { + fail(); + } + } + } + @Test public void testSignaturePublicKeyEncodingAndParsing() throws Exception { KeyParser parser = crypto.getSignatureKeyParser(); @@ -80,4 +113,33 @@ public class KeyEncodingAndParsingTest extends BriarTestCase { byte[] secret1 = crypto.deriveSharedSecret(bPriv, aPair.getPublic()); assertArrayEquals(secret, secret1); } + + @Test + public void testSignatureKeyParserByFuzzing() throws Exception { + KeyParser parser = crypto.getSignatureKeyParser(); + // Generate a key pair to get the proper public key length + KeyPair p = crypto.generateSignatureKeyPair(); + int pubLength = p.getPublic().getEncoded().length; + int privLength = p.getPrivate().getEncoded().length; + // Parse some random byte arrays - expect GeneralSecurityException + Random random = new Random(); + byte[] pubFuzz = new byte[pubLength]; + byte[] privFuzz = new byte[privLength]; + for(int i = 0; i < 1000; i++) { + random.nextBytes(pubFuzz); + try { + parser.parsePublicKey(pubFuzz); + } catch(GeneralSecurityException expected) { + } catch(Exception e) { + fail(); + } + random.nextBytes(privFuzz); + try { + parser.parsePrivateKey(privFuzz); + } catch(GeneralSecurityException expected) { + } catch(Exception e) { + fail(); + } + } + } }