Skip to content

Remaining fixes to allow BouncyCastle to be swapped out with other implementations. #131

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 21, 2019
Merged

Remaining fixes to allow BouncyCastle to be swapped out with other implementations. #131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
af0e969
Remove use of BouncyCastle for HMAC key derivation
WesleyRosenblum Oct 17, 2019
3fa1cf8
Fixing copyright dates
WesleyRosenblum Oct 18, 2019
d36235f
*Issue #, if available:* #41
WesleyRosenblum Oct 18, 2019
9d86ce2
Merge branch 'master' of https://github.com/aws/aws-encryption-sdk-ja…
WesleyRosenblum Oct 18, 2019
4c2c0a5
Adding comment about sun classes usage and using a constant for empty…
WesleyRosenblum Oct 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 4 additions & 14 deletions src/main/java/com/amazonaws/encryptionsdk/CryptoAlgorithm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;

import com.amazonaws.encryptionsdk.internal.BouncyCastleConfiguration;
import com.amazonaws.encryptionsdk.internal.HmacKeyDerivationFunction;

import com.amazonaws.encryptionsdk.internal.Constants;
Expand Down Expand Up @@ -99,27 +98,19 @@ public enum CryptoAlgorithm {
private final int dataKeyLen_;
private final boolean safeToCache_;

/**
* This block is used to ensure static blocks of BouncyCastleConfiguration are evaluated as a dependency of
* the CryptoAlgorithm class
*/
static {
BouncyCastleConfiguration.init();
}

/*
* Create a mapping between the CiphertextType object and its byte value representation. Make
* this is a static method so the map is created when the object is created. This enables fast
* lookups of the CryptoAlgorithm given its short value representation.
*/
private static final Map<Short, CryptoAlgorithm> ID_MAPPING = new HashMap<Short, CryptoAlgorithm>();
private static final Map<Short, CryptoAlgorithm> ID_MAPPING = new HashMap<>();
static {
for (final CryptoAlgorithm s : EnumSet.allOf(CryptoAlgorithm.class)) {
ID_MAPPING.put(s.value_, s);
}
}

private CryptoAlgorithm(
CryptoAlgorithm(
final int blockSizeBits, final int nonceLenBytes, final int tagLenBytes,
final long maxContentLen, final String keyAlgo, final int keyLenBytes, final int value,
final String dataKeyAlgo, final int dataKeyLen, boolean safeToCache
Expand All @@ -130,7 +121,7 @@ private CryptoAlgorithm(

}

private CryptoAlgorithm(
CryptoAlgorithm(
final int blockSizeBits, final int nonceLenBytes, final int tagLenBytes,
final long maxContentLen, final String keyAlgo, final int keyLenBytes, final int value,
final String dataKeyAlgo, final int dataKeyLen,
Expand Down Expand Up @@ -164,8 +155,7 @@ private CryptoAlgorithm(
* @return the CryptoAlgorithm object that matches the given value, null if no match is found.
*/
public static CryptoAlgorithm deserialize(final short value) {
final CryptoAlgorithm result = ID_MAPPING.get(value);
return result;
return ID_MAPPING.get(value);
}

/**
Expand Down
37 changes: 0 additions & 37 deletions src/main/java/com/amazonaws/encryptionsdk/internal/BouncyCastleConfiguration.java
View file
Open in desktop

This file was deleted.

8 changes: 4 additions & 4 deletions src/main/java/com/amazonaws/encryptionsdk/internal/TrailingSignatureAlgorithm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ private static final class ECDSASignatureAlgorithm extends TrailingSignatureAlgo
private static final BigInteger THREE = BigInteger.valueOf(3);
private static final BigInteger FOUR = BigInteger.valueOf(4);

private ECDSASignatureAlgorithm(ECGenParameterSpec ecSpec, String messageDigestAlgorithm) {
private ECDSASignatureAlgorithm(ECGenParameterSpec ecSpec, String messageDigestAlgorithm, String hashAndSignAlgorithm) {
if (!ecSpec.getName().startsWith(SEC_PRIME_FIELD_PREFIX)) {
throw new IllegalStateException("Non-prime curves are not supported at this time");
}

this.ecSpec = ecSpec;
this.messageDigestAlgorithm = messageDigestAlgorithm;
this.hashAndSignAlgorithm = messageDigestAlgorithm + "withECDSA";
this.hashAndSignAlgorithm = hashAndSignAlgorithm;

try {
final AlgorithmParameters parameters = AlgorithmParameters.getInstance(ELLIPTIC_CURVE_ALGORITHM);
Expand Down Expand Up @@ -190,9 +190,9 @@ public KeyPair generateKey() throws GeneralSecurityException {
}

private static final ECDSASignatureAlgorithm SHA256_ECDSA_P256
= new ECDSASignatureAlgorithm(new ECGenParameterSpec(SEC_PRIME_FIELD_PREFIX + "256r1"), "SHA256");
= new ECDSASignatureAlgorithm(new ECGenParameterSpec(SEC_PRIME_FIELD_PREFIX + "256r1"), "SHA-256", "SHA256withECDSA");
private static final ECDSASignatureAlgorithm SHA384_ECDSA_P384
= new ECDSASignatureAlgorithm(new ECGenParameterSpec(SEC_PRIME_FIELD_PREFIX + "384r1"), "SHA384");
= new ECDSASignatureAlgorithm(new ECGenParameterSpec(SEC_PRIME_FIELD_PREFIX + "384r1"), "SHA-384", "SHA384withECDSA");

public static TrailingSignatureAlgorithm forCryptoAlgorithm(CryptoAlgorithm algorithm) {
switch (algorithm) {
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/amazonaws/encryptionsdk/internal/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.WeakHashMap;
import java.util.concurrent.atomic.AtomicLong;

import org.apache.commons.lang3.ArrayUtils;
import org.bouncycastle.util.encoders.Base64;

/**
Expand Down Expand Up @@ -268,7 +269,7 @@ public static ByteBuffer limit(final ByteBuffer buff, final int newLimit) {
* @return decoded data as a byte array
*/
public static byte[] decodeBase64String(final String encoded) {
return Base64.decode(encoded);
return encoded.isEmpty() ? ArrayUtils.EMPTY_BYTE_ARRAY : Base64.decode(encoded);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/com/amazonaws/encryptionsdk/internal/CipherHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void tamperCiphertext() {
final byte[] keyBytes = RandomBytesGenerator.generate(cryptoAlgorithm.getKeyLength());
final byte[] nonce = RandomBytesGenerator.generate(cryptoAlgorithm.getNonceLen());

final SecretKey key = new SecretKeySpec(keyBytes, cryptoAlgorithm.name());
final SecretKey key = new SecretKeySpec(keyBytes, cryptoAlgorithm.getKeyAlgo());
CipherHandler cipherHandler = createCipherHandler(key, cryptoAlgorithm, Cipher.ENCRYPT_MODE);
final byte[] encryptedBytes = cipherHandler.cipherData(nonce, contentAad_, content, 0, content.length);

Expand All @@ -72,7 +72,7 @@ private byte[] encryptDecrypt(final byte[] content, final CryptoAlgorithm crypto
final byte[] keyBytes = RandomBytesGenerator.generate(cryptoAlgorithm.getKeyLength());
final byte[] nonce = RandomBytesGenerator.generate(cryptoAlgorithm.getNonceLen());

final SecretKey key = new SecretKeySpec(keyBytes, cryptoAlgorithm.name());
final SecretKey key = new SecretKeySpec(keyBytes, cryptoAlgorithm.getKeyAlgo());
CipherHandler cipherHandler = createCipherHandler(key, cryptoAlgorithm, Cipher.ENCRYPT_MODE);
final byte[] encryptedBytes = cipherHandler.cipherData( nonce, contentAad_, content, 0, content.length);

Expand Down
88 changes: 45 additions & 43 deletions src/test/java/com/amazonaws/encryptionsdk/jce/KeyStoreProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,14 @@
import java.security.KeyStore.PasswordProtection;
import java.security.KeyStoreException;
import java.security.SecureRandom;
import java.security.Security;
import java.security.cert.Certificate;
import java.security.cert.X509Certificate;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Date;

import javax.crypto.spec.SecretKeySpec;
import javax.security.auth.x500.X500Principal;

import org.bouncycastle.asn1.x509.X509Name;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.bouncycastle.x509.X509V3CertificateGenerator;
import org.junit.Before;
import org.junit.Test;

Expand All @@ -47,7 +44,18 @@
import com.amazonaws.encryptionsdk.exception.CannotUnwrapDataKeyException;
import com.amazonaws.encryptionsdk.multi.MultipleProviderFactory;

@SuppressWarnings("deprecation")
/* These internal sun classes are included solely for test purposes as
this test cannot use BouncyCastle cert generation, as there are incompatibilities
between how standard BC and FIPS BC perform cert generation. */
import sun.security.x509.AlgorithmId;
import sun.security.x509.CertificateAlgorithmId;
import sun.security.x509.CertificateSerialNumber;
import sun.security.x509.CertificateValidity;
import sun.security.x509.CertificateX509Key;
import sun.security.x509.X500Name;
import sun.security.x509.X509CertImpl;
import sun.security.x509.X509CertInfo;
Comment on lines +50 to +57
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to do this without importing sun. packages. This may be test code but it's a pretty severe anti-pattern.

Copy link
Contributor Author

@WesleyRosenblum WesleyRosenblum Oct 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Standard BC and FIPS BC have different APIs for certificate generation, and the two packages are not compatible with each other so we can't just add a new test dependency. I added a comment about the usage of the sun packages.

This method passes the compatibility tests using either standard BC or FIPS BC


public class KeyStoreProviderTest {
private static final SecureRandom RND = new SecureRandom();
private static final KeyPairGenerator KG;
Expand All @@ -72,7 +80,7 @@ public void setup() throws Exception {
}

@Test
public void singleKeyPkcs1() throws GeneralSecurityException {
public void singleKeyPkcs1() throws Exception {
addEntry("key1");
final KeyStoreProvider mkp = new KeyStoreProvider(ks, PP, "KeyStore", "RSA/ECB/PKCS1Padding", "key1");
final JceMasterKey mk1 = mkp.getMasterKey("key1");
Expand All @@ -87,7 +95,7 @@ public void singleKeyPkcs1() throws GeneralSecurityException {
}

@Test
public void singleKeyOaepSha1() throws GeneralSecurityException {
public void singleKeyOaepSha1() throws Exception {
addEntry("key1");
final KeyStoreProvider mkp = new KeyStoreProvider(ks, PP, "KeyStore", "RSA/ECB/OAEPWithSHA-1AndMGF1Padding",
"key1");
Expand All @@ -103,7 +111,7 @@ public void singleKeyOaepSha1() throws GeneralSecurityException {
}

@Test
public void singleKeyOaepSha256() throws GeneralSecurityException {
public void singleKeyOaepSha256() throws Exception {
addEntry("key1");
final KeyStoreProvider mkp = new KeyStoreProvider(ks, PP, "KeyStore", "RSA/ECB/OAEPWithSHA-256AndMGF1Padding",
"key1");
Expand All @@ -119,7 +127,7 @@ public void singleKeyOaepSha256() throws GeneralSecurityException {
}

@Test
public void multipleKeys() throws GeneralSecurityException {
public void multipleKeys() throws Exception {
addEntry("key1");
addEntry("key2");
final KeyStoreProvider mkp = new KeyStoreProvider(ks, PP, "KeyStore", "RSA/ECB/OAEPWithSHA-256AndMGF1Padding",
Expand All @@ -146,7 +154,7 @@ public void multipleKeys() throws GeneralSecurityException {
}

@Test(expected = CannotUnwrapDataKeyException.class)
public void encryptOnly() throws GeneralSecurityException {
public void encryptOnly() throws Exception {
addPublicEntry("key1");
final KeyStoreProvider mkp = new KeyStoreProvider(ks, PP, "KeyStore", "RSA/ECB/OAEPWithSHA-256AndMGF1Padding",
"key1");
Expand All @@ -157,7 +165,7 @@ public void encryptOnly() throws GeneralSecurityException {
}

@Test
public void escrowAndSymmetric() throws GeneralSecurityException {
public void escrowAndSymmetric() throws Exception {
addPublicEntry("key1");
addEntry("key2");
final KeyStoreProvider mkp = new KeyStoreProvider(ks, PP, "KeyStore", "RSA/ECB/OAEPWithSHA-256AndMGF1Padding",
Expand Down Expand Up @@ -185,7 +193,7 @@ public void escrowAndSymmetric() throws GeneralSecurityException {
}

@Test
public void escrowAndSymmetricSecondProvider() throws GeneralSecurityException {
public void escrowAndSymmetricSecondProvider() throws GeneralSecurityException, IOException {
addPublicEntry("key1");
addEntry("key2");
final KeyStoreProvider mkp = new KeyStoreProvider(ks, PP, "KeyStore", "RSA/ECB/OAEPWithSHA-256AndMGF1Padding",
Expand Down Expand Up @@ -263,40 +271,34 @@ public void keystoreAndRawProvider() throws GeneralSecurityException, IOExceptio
assertArrayEquals(PLAINTEXT, crypto.decryptData(ksp, ct.getResult()).getResult());
}

private void addEntry(final String alias) throws GeneralSecurityException {
private void addEntry(final String alias) throws GeneralSecurityException, IOException {
final KeyPair pair = KG.generateKeyPair();
// build a certificate generator
final X509V3CertificateGenerator certGen = new X509V3CertificateGenerator();
final X500Principal dnName = new X500Principal("cn=" + alias);

certGen.setSerialNumber(new BigInteger(256, RND));
certGen.setSubjectDN(new X509Name("dc=" + alias));
certGen.setIssuerDN(dnName); // use the same
certGen.setNotBefore(new Date(System.currentTimeMillis() - 24 * 60 * 60 * 1000));
certGen.setNotAfter(new Date(System.currentTimeMillis() + 2 * 365 * 24 * 60 * 60 * 1000));
certGen.setPublicKey(pair.getPublic());
certGen.setSignatureAlgorithm("SHA256WithRSA");
final X509Certificate cert = certGen.generate(pair.getPrivate(), "BC");

ks.setEntry(alias, new KeyStore.PrivateKeyEntry(pair.getPrivate(), new X509Certificate[] { cert }), PP);
ks.setEntry(alias, new KeyStore.PrivateKeyEntry(pair.getPrivate(),
new X509Certificate[] { generateCertificate(pair, alias) }), PP);
}

private void addPublicEntry(final String alias) throws GeneralSecurityException {
private void addPublicEntry(final String alias) throws GeneralSecurityException, IOException {
final KeyPair pair = KG.generateKeyPair();
// build a certificate generator
final X509V3CertificateGenerator certGen = new X509V3CertificateGenerator();
final X500Principal dnName = new X500Principal("cn=" + alias);

certGen.setSerialNumber(new BigInteger(256, RND));
certGen.setSubjectDN(new X509Name("dc=" + alias));
certGen.setIssuerDN(dnName); // use the same
certGen.setNotBefore(new Date(System.currentTimeMillis() - 24 * 60 * 60 * 1000));
certGen.setNotAfter(new Date(System.currentTimeMillis() + 2 * 365 * 24 * 60 * 60 * 1000));
certGen.setPublicKey(pair.getPublic());
certGen.setSignatureAlgorithm("SHA256WithRSA");
final X509Certificate cert = certGen.generate(pair.getPrivate(), "BC");

ks.setEntry(alias, new KeyStore.TrustedCertificateEntry(cert), null);
ks.setEntry(alias, new KeyStore.TrustedCertificateEntry(generateCertificate(pair, alias)), null);
}

private X509Certificate generateCertificate(final KeyPair pair, final String alias) throws GeneralSecurityException, IOException {
final X509CertInfo info = new X509CertInfo();
final X500Name name = new X500Name("dc=" + alias);
info.set(X509CertInfo.SERIAL_NUMBER, new CertificateSerialNumber(new BigInteger(256, RND)));
info.set(X509CertInfo.SUBJECT, name);
info.set(X509CertInfo.ISSUER, name);
info.set(X509CertInfo.VALIDITY,
new CertificateValidity(Date.from(Instant.now().minus(1, ChronoUnit.DAYS)),
Date.from(Instant.now().plus(730, ChronoUnit.DAYS))));
info.set(X509CertInfo.KEY, new CertificateX509Key(pair.getPublic()));
info.set(X509CertInfo.ALGORITHM_ID,
new CertificateAlgorithmId(new AlgorithmId(AlgorithmId.sha256WithRSAEncryption_oid)));

final X509CertImpl cert = new X509CertImpl(info);
cert.sign(pair.getPrivate(), AlgorithmId.sha256WithRSAEncryption_oid.toString());

return cert;
}

private void copyPublicPart(final KeyStore src, final KeyStore dst, final String alias) throws KeyStoreException {
Expand Down