-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove use of BouncyCastle for EC key generation and point (de)compression #129
Changes from 1 commit
6cd9fda
9842559
b1ce45b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,18 @@ | ||
package com.amazonaws.encryptionsdk.internal; | ||
|
||
import java.security.GeneralSecurityException; | ||
import java.security.KeyPair; | ||
import java.security.KeyPairGenerator; | ||
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.math.BigInteger; | ||
import java.security.*; | ||
import java.security.interfaces.ECPublicKey; | ||
import java.security.spec.*; | ||
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 org.apache.commons.lang3.Validate.*; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto on import star. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
import static java.math.BigInteger.ONE; | ||
import static java.math.BigInteger.ZERO; | ||
|
||
/** | ||
* Provides a consistent interface across various trailing signature algorithms. | ||
|
@@ -36,15 +32,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 | ||
|
@@ -66,37 +83,99 @@ public String getRawSignatureAlgorithm() { | |
return hashAndSignAlgorithm; | ||
} | ||
|
||
/** | ||
* Decodes a compressed elliptic curve point as described in SEC-1 v2 section 2.3.4 | ||
* @see <a href="http://www.secg.org/sec1-v2.pdf">http://www.secg.org/sec1-v2.pdf</a> | ||
* @param keyString The serialized and compressed public key | ||
* @return The PublicKey | ||
*/ | ||
@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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and throughout: Please have a space between the closing parenthesis of the cast and the following token. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
final BigInteger a = ecParameterSpec.getCurve().getA(); | ||
final BigInteger b = ecParameterSpec.getCurve().getB(); | ||
|
||
final BigInteger alpha = x.modPow(THREE, p) | ||
SalusaSecondus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.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 | ||
* @see <a href="http://www.secg.org/sec1-v2.pdf">http://www.secg.org/sec1-v2.pdf</a> | ||
* @param key The Elliptic Curve public key to compress and serialize | ||
* @return The serialized and compressed public key | ||
*/ | ||
@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) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,30 @@ 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 | ||
* @return The byte array | ||
*/ | ||
public static byte[] bigIntegerToByteArray(final BigInteger bigInteger, final int length) { | ||
final byte[] result = new byte[length]; | ||
byte[] rawBytes = bigInteger.toByteArray(); | ||
|
||
//Remove sign byte if one is present | ||
if(rawBytes[0] == 0) { | ||
rawBytes = Arrays.copyOfRange(rawBytes, 1, rawBytes.length); | ||
} | ||
|
||
if(length < rawBytes.length) { | ||
throw new IllegalArgumentException("Length must be as least as long as the BigInteger byte array " + | ||
"without the sign byte"); | ||
} | ||
|
||
System.arraycopy(rawBytes, 0, result, length - rawBytes.length, rawBytes.length); | ||
|
||
return result; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's simplify this (and reduce unneeded allocations).
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -174,4 +174,39 @@ 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++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and throughout: space after Here: Space on either side of the equals. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
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; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,19 @@ public void base64something() { | |
assertEquals(encoded, Utils.encodeBase64String(data)); | ||
assertArrayEquals(data, Utils.decodeBase64String(encoded)); | ||
} | ||
|
||
@Test | ||
public void testBigIntegerToByteArray() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please also test exceptional cases. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added tests |
||
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)); | ||
|
||
bytes = new byte[] {0, -47, 126, -42, 34}; | ||
|
||
assertArrayEquals(new byte[]{-47, 126, -42, 34}, | ||
Utils.bigIntegerToByteArray(new BigInteger(bytes), 4)); | ||
|
||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No import stars please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, and updated my IDE