Remove use of BouncyCastle for EC key generation and point (de)compression (#129)

WesleyRosenblum · web-flow · commit 4ebbbcc0b9f5 · 2019-10-16T11:17:21.000-07:00
*Issue #, if available:* #41 *Description of changes:* Removes explicit use of BouncyCastle from the `ECDSASignatureAlgorithm` implementation of `TrailingSignatureAlgorithm`. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. # Check any applicable: - [ ] Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 ### Maintenance
 * Add support for standard test vectors via `testVectorZip` system property.
 * No longer require use of BouncyCastle with RSA `JceMasterKey`s
+* No longer use BouncyCastle for Elliptic Curve key generation and point compression/decompression
 
 ## 1.6.0 -- 2019-05-31
 
diff --git a/src/main/java/com/amazonaws/encryptionsdk/internal/TrailingSignatureAlgorithm.java b/src/main/java/com/amazonaws/encryptionsdk/internal/TrailingSignatureAlgorithm.java
@@ -1,22 +1,31 @@
 package com.amazonaws.encryptionsdk.internal;
 
+import java.math.BigInteger;
+import java.security.AlgorithmParameters;
 import java.security.GeneralSecurityException;
+import java.security.KeyFactory;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
+import java.security.NoSuchAlgorithmException;
 import java.security.PublicKey;
-
-import org.bouncycastle.crypto.params.ECDomainParameters;
-import org.bouncycastle.crypto.params.ECPublicKeyParameters;
-import org.bouncycastle.jcajce.provider.asymmetric.ec.BCECPublicKey;
-import org.bouncycastle.jce.ECNamedCurveTable;
-import org.bouncycastle.jce.interfaces.ECPublicKey;
-import org.bouncycastle.jce.provider.BouncyCastleProvider;
-import org.bouncycastle.jce.spec.ECNamedCurveParameterSpec;
-import org.bouncycastle.math.ec.ECPoint;
+import java.security.interfaces.ECPublicKey;
+import java.security.spec.ECFieldFp;
+import java.security.spec.ECGenParameterSpec;
+import java.security.spec.ECParameterSpec;
+import java.security.spec.ECPoint;
+import java.security.spec.ECPublicKeySpec;
+import java.security.spec.InvalidKeySpecException;
+import java.security.spec.InvalidParameterSpecException;
+import java.util.Arrays;
 
 import com.amazonaws.encryptionsdk.CryptoAlgorithm;
 
-import static com.amazonaws.encryptionsdk.internal.BouncyCastleConfiguration.INTERNAL_BOUNCY_CASTLE_PROVIDER;
+import static com.amazonaws.encryptionsdk.internal.Utils.bigIntegerToByteArray;
+import static com.amazonaws.encryptionsdk.internal.Utils.encodeBase64String;
+import static java.math.BigInteger.ONE;
+import static java.math.BigInteger.ZERO;
+import static org.apache.commons.lang3.Validate.isInstanceOf;
+import static org.apache.commons.lang3.Validate.notNull;
 
 /**
  * Provides a consistent interface across various trailing signature algorithms.
@@ -36,15 +45,36 @@ private TrailingSignatureAlgorithm() {
     public abstract String serializePublicKey(PublicKey key);
     public abstract KeyPair generateKey() throws GeneralSecurityException;
 
+    /* Standards for Efficient Cryptography over a prime field */
+    private static final String SEC_PRIME_FIELD_PREFIX = "secp";
+
     private static final class ECDSASignatureAlgorithm extends TrailingSignatureAlgorithm {
-        private final ECNamedCurveParameterSpec ecSpec;
+        private final ECGenParameterSpec ecSpec;
+        private final ECParameterSpec ecParameterSpec;
         private final String messageDigestAlgorithm;
         private final String hashAndSignAlgorithm;
+        private static final String ELLIPTIC_CURVE_ALGORITHM = "EC";
+        /* Constants used by SEC-1 v2 point compression and decompression algorithms */
+        private static final BigInteger TWO = BigInteger.valueOf(2);
+        private static final BigInteger THREE = BigInteger.valueOf(3);
+        private static final BigInteger FOUR = BigInteger.valueOf(4);
+
+        private ECDSASignatureAlgorithm(ECGenParameterSpec ecSpec, String messageDigestAlgorithm) {
+            if (!ecSpec.getName().startsWith(SEC_PRIME_FIELD_PREFIX)) {
+                throw new IllegalStateException("Non-prime curves are not supported at this time");
+            }
 
-        private ECDSASignatureAlgorithm(ECNamedCurveParameterSpec ecSpec, String messageDigestAlgorithm) {
             this.ecSpec = ecSpec;
             this.messageDigestAlgorithm = messageDigestAlgorithm;
             this.hashAndSignAlgorithm = messageDigestAlgorithm + "withECDSA";
+
+            try {
+                final AlgorithmParameters parameters = AlgorithmParameters.getInstance(ELLIPTIC_CURVE_ALGORITHM);
+                parameters.init(ecSpec);
+                this.ecParameterSpec = parameters.getParameterSpec(ECParameterSpec.class);
+            } catch (NoSuchAlgorithmException | InvalidParameterSpecException e) {
+                throw new IllegalStateException("Invalid algorithm", e);
+            }
         }
 
         @Override
@@ -62,41 +92,107 @@ public String getRawSignatureAlgorithm() {
             return "NONEwithECDSA";
         }
 
-        @Override public String getHashAndSignAlgorithm() {
+        @Override
+        public String getHashAndSignAlgorithm() {
             return hashAndSignAlgorithm;
         }
 
+        /**
+         * Decodes a compressed elliptic curve point as described in SEC-1 v2 section 2.3.4
+         *
+         * @param keyString The serialized and compressed public key
+         * @return The PublicKey
+         * @see <a href="http://www.secg.org/sec1-v2.pdf">http://www.secg.org/sec1-v2.pdf</a>
+         */
         @Override
         public PublicKey deserializePublicKey(String keyString) {
-            final ECPoint q = ecSpec.getCurve().decodePoint(Utils.decodeBase64String(keyString));
-
-            ECPublicKeyParameters keyParams = new ECPublicKeyParameters(
-                    q,
-                    new ECDomainParameters(ecSpec.getCurve(), ecSpec.getG(), ecSpec.getN(), ecSpec.getH())
-            );
-
-            return new BCECPublicKey("EC", keyParams, ecSpec, BouncyCastleProvider.CONFIGURATION);
+            notNull(keyString, "keyString is required");
+
+            final byte[] decodedKey = Utils.decodeBase64String(keyString);
+            final BigInteger x = new BigInteger(1, Arrays.copyOfRange(decodedKey, 1, decodedKey.length));
+
+            final byte compressedY = decodedKey[0];
+            final BigInteger yOrder;
+
+            if (compressedY == TWO.byteValue()) {
+                yOrder = ZERO;
+            } else if (compressedY == THREE.byteValue()) {
+                yOrder = ONE;
+            } else {
+                throw new IllegalArgumentException("Compressed y value was invalid");
+            }
+
+            final BigInteger p = ((ECFieldFp) ecParameterSpec.getCurve().getField()).getP();
+            final BigInteger a = ecParameterSpec.getCurve().getA();
+            final BigInteger b = ecParameterSpec.getCurve().getB();
+
+            //alpha must be equal to y^2, this is validated below
+            final BigInteger alpha = x.modPow(THREE, p)
+                    .add(a.multiply(x).mod(p))
+                    .add(b)
+                    .mod(p);
+
+            final BigInteger beta;
+            if (p.mod(FOUR).equals(THREE)) {
+                beta = alpha.modPow(p.add(ONE).divide(FOUR), p);
+            } else {
+                throw new IllegalArgumentException("Curve not supported at this time");
+            }
+
+            final BigInteger y = beta.mod(TWO).equals(yOrder) ? beta : p.subtract(beta);
+
+            //Validate that Y is a root of Y^2 to prevent invalid point attacks
+            if (!alpha.equals(y.modPow(TWO, p))) {
+                throw new IllegalArgumentException("Y was invalid");
+            }
+
+            try {
+                return KeyFactory.getInstance(ELLIPTIC_CURVE_ALGORITHM).generatePublic(
+                        new ECPublicKeySpec(new ECPoint(x, y), ecParameterSpec));
+            } catch (InvalidKeySpecException | NoSuchAlgorithmException e) {
+                throw new IllegalStateException("Invalid algorithm", e);
+            }
         }
 
+        /**
+         * Encodes a compressed elliptic curve point as described in SEC-1 v2 section 2.3.3
+         *
+         * @param key The Elliptic Curve public key to compress and serialize
+         * @return The serialized and compressed public key
+         * @see <a href="http://www.secg.org/sec1-v2.pdf">http://www.secg.org/sec1-v2.pdf</a>
+         */
         @Override
         public String serializePublicKey(PublicKey key) {
-            return Utils.encodeBase64String(((ECPublicKey)key).getQ().getEncoded(true));
+            notNull(key, "key is required");
+            isInstanceOf(ECPublicKey.class, key, "key must be an instance of ECPublicKey");
+
+            final BigInteger x = ((ECPublicKey) key).getW().getAffineX();
+            final BigInteger y = ((ECPublicKey) key).getW().getAffineY();
+            final BigInteger compressedY = y.mod(TWO).equals(ZERO) ? TWO : THREE;
+
+            final byte[] xBytes = bigIntegerToByteArray(x,
+                    ecParameterSpec.getCurve().getField().getFieldSize() / Byte.SIZE);
+
+            final byte[] compressedKey = new byte[xBytes.length + 1];
+            System.arraycopy(xBytes, 0, compressedKey, 1, xBytes.length);
+            compressedKey[0] = compressedY.byteValue();
+
+            return encodeBase64String(compressedKey);
         }
 
         @Override
         public KeyPair generateKey() throws GeneralSecurityException {
-            // We use BouncyCastle for this so that we can easily serialize the compressed point.
-            KeyPairGenerator keyGen = KeyPairGenerator.getInstance("EC", INTERNAL_BOUNCY_CASTLE_PROVIDER);
+            KeyPairGenerator keyGen = KeyPairGenerator.getInstance(ELLIPTIC_CURVE_ALGORITHM);
             keyGen.initialize(ecSpec, Utils.getSecureRandom());
 
             return keyGen.generateKeyPair();
         }
     }
 
     private static final ECDSASignatureAlgorithm SHA256_ECDSA_P256
-            = new ECDSASignatureAlgorithm(ECNamedCurveTable.getParameterSpec("secp256r1"), "SHA256");
+            = new ECDSASignatureAlgorithm(new ECGenParameterSpec(SEC_PRIME_FIELD_PREFIX + "256r1"), "SHA256");
     private static final ECDSASignatureAlgorithm SHA384_ECDSA_P384
-            = new ECDSASignatureAlgorithm(ECNamedCurveTable.getParameterSpec("secp384r1"), "SHA384");
+            = new ECDSASignatureAlgorithm(new ECGenParameterSpec(SEC_PRIME_FIELD_PREFIX + "384r1"), "SHA384");
 
     public static TrailingSignatureAlgorithm forCryptoAlgorithm(CryptoAlgorithm algorithm) {
         switch (algorithm) {
diff --git a/src/main/java/com/amazonaws/encryptionsdk/internal/Utils.java b/src/main/java/com/amazonaws/encryptionsdk/internal/Utils.java
@@ -14,6 +14,7 @@
 package com.amazonaws.encryptionsdk.internal;
 
 import java.io.Serializable;
+import java.math.BigInteger;
 import java.nio.Buffer;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
@@ -279,4 +280,34 @@ public static byte[] decodeBase64String(final String encoded) {
     public static String encodeBase64String(final byte[] data) {
         return Base64.toBase64String(data);
     }
+
+    /**
+     * Removes the leading zero sign byte from the byte array representation of a BigInteger (if present)
+     * and left pads with zeroes to produce a byte array of the given length.
+     * @param bigInteger The BigInteger to convert to a byte array
+     * @param length The length of the byte array, must be at least
+     *              as long as the BigInteger byte array without the sign byte
+     * @return The byte array
+     */
+    public static byte[] bigIntegerToByteArray(final BigInteger bigInteger, final int length) {
+        byte[] rawBytes = bigInteger.toByteArray();
+        // If rawBytes is already the correct length, return it.
+        if (rawBytes.length == length) {
+            return rawBytes;
+        }
+
+        // If we're exactly one byte too large, but we have a leading zero byte, remove it and return.
+        if(rawBytes.length == length + 1 && rawBytes[0] == 0) {
+            return Arrays.copyOfRange(rawBytes, 1, rawBytes.length);
+        }
+
+        if (rawBytes.length > length) {
+            throw new IllegalArgumentException("Length must be at least as long as the BigInteger byte array " +
+                    "without the sign byte");
+        }
+
+        final byte[] paddedResult = new byte[length];
+        System.arraycopy(rawBytes, 0, paddedResult, length - rawBytes.length, rawBytes.length);
+        return paddedResult;
+    }
 }
diff --git a/src/test/java/com/amazonaws/encryptionsdk/TestUtils.java b/src/test/java/com/amazonaws/encryptionsdk/TestUtils.java
@@ -174,4 +174,41 @@ public static int[] getFrameSizesToTest(final CryptoAlgorithm cryptoAlg) {
       };
       return frameSizeToTest;
   }
+
+    /**
+     * Converts an array of unsigned bytes (represented as int values between 0 and 255 inclusive)
+     * to an array of Java primitive type byte, which are by definition signed.
+     *
+     * @param unsignedBytes An array on unsigned bytes
+     * @return An array of signed bytes
+     */
+    public static byte[] unsignedBytesToSignedBytes(final int[] unsignedBytes) {
+        byte[] signedBytes = new byte[unsignedBytes.length];
+
+        for (int i = 0; i < unsignedBytes.length; i++) {
+            if (unsignedBytes[i] > 255) {
+                throw new IllegalArgumentException("Encountered unsigned byte value > 255");
+            }
+            signedBytes[i] = (byte) (unsignedBytes[i] & 0xff);
+        }
+
+        return signedBytes;
+    }
+
+    /**
+     * Converts an array of Java primitive type bytes (which are by definition signed) to
+     * an array of unsigned bytes (represented as int values between 0 and 255 inclusive).
+     *
+     * @param signedBytes An array of signed bytes
+     * @return An array of unsigned bytes
+     */
+    public static int[] signedBytesToUnsignedBytes(final byte[] signedBytes) {
+        int[] unsignedBytes = new int[signedBytes.length];
+
+        for (int i = 0; i < signedBytes.length; i++) {
+            unsignedBytes[i] = ((int) signedBytes[i]) & 0xff;
+        }
+
+        return unsignedBytes;
+    }
 }
diff --git a/src/test/java/com/amazonaws/encryptionsdk/UtilsTest.java b/src/test/java/com/amazonaws/encryptionsdk/UtilsTest.java
@@ -5,6 +5,7 @@
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 
@@ -96,5 +97,29 @@ public void base64something() {
         assertEquals(encoded, Utils.encodeBase64String(data));
         assertArrayEquals(data, Utils.decodeBase64String(encoded));
     }
+
+    @Test
+    public void testBigIntegerToByteArray() {
+        byte[] bytes = new byte[] {23, 47, 126, -42, 34};
+
+        assertArrayEquals(new byte[]{0, 0, 0, 23, 47, 126, -42, 34},
+                Utils.bigIntegerToByteArray(new BigInteger(bytes), 8));
+        assertArrayEquals(new byte[]{23, 47, 126, -42, 34},
+                Utils.bigIntegerToByteArray(new BigInteger(bytes), 5));
+
+        bytes = new byte[] {0, -47, 126, -42, 34};
+
+        assertArrayEquals(new byte[]{-47, 126, -42, 34},
+                Utils.bigIntegerToByteArray(new BigInteger(bytes), 4));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testBigIntegerToByteArray_InvalidLength() {
+        byte[] bytes = new byte[] {0, -47, 126, -42, 34};
+
+        assertArrayEquals(bytes,
+                Utils.bigIntegerToByteArray(new BigInteger(bytes), 3));
+    }
+
 }
 
diff --git a/src/test/java/com/amazonaws/encryptionsdk/internal/TrailingSignatureAlgorithmTest.java b/src/test/java/com/amazonaws/encryptionsdk/internal/TrailingSignatureAlgorithmTest.java
