GEODE-1372 Added support to keep multiple copies of cipher as those are not thread safe. Added unit test for it.
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/3a643084 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/3a643084 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/3a643084 Branch: refs/heads/develop Commit: 3a6430846f9cd8d36b638290a18691001a6e30e5 Parents: c5247cc Author: Hitesh Khamesra <[email protected]> Authored: Fri Jun 3 09:49:05 2016 -0700 Committer: Hitesh Khamesra <[email protected]> Committed: Mon Aug 29 10:39:18 2016 -0700 ---------------------------------------------------------------------- .../membership/gms/messenger/GMSEncrypt.java | 102 +++++++++---- .../LocatorUDPSecurityDUnitTest.java | 5 +- .../gms/messenger/GMSEncryptJUnitTest.java | 148 +++++++++++++++---- 3 files changed, 198 insertions(+), 57 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/3a643084/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncrypt.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncrypt.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncrypt.java index 0bea614..8136c1a 100755 --- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncrypt.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncrypt.java @@ -20,6 +20,7 @@ import java.math.BigInteger; import java.security.*; import java.security.spec.PKCS8EncodedKeySpec; import java.security.spec.X509EncodedKeySpec; +import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -35,8 +36,6 @@ import javax.crypto.spec.SecretKeySpec; import com.gemstone.gemfire.distributed.internal.membership.InternalDistributedMember; import com.gemstone.gemfire.distributed.internal.membership.NetView; import com.gemstone.gemfire.distributed.internal.membership.gms.Services; - - import com.gemstone.gemfire.distributed.internal.DistributionConfig; public class GMSEncrypt implements Cloneable{ @@ -73,8 +72,12 @@ public class GMSEncrypt implements Cloneable{ private NetView view; - private Map<InternalDistributedMember, PeerEncryptor> memberToPeerEncryptor = new ConcurrentHashMap<>(); + private int numberOfPeerEncryptorCopies = 10; + private ConcurrentHashMap<InternalDistributedMember, PeerEncryptor>[] copyOfPeerEncryptors; + private ClusterEncryptor[] clusterEncryptors; + private Map<InternalDistributedMember, byte[]> memberToPeerEncryptor = new ConcurrentHashMap<>(); + private ClusterEncryptor clusterEncryptor; protected void installView(NetView view) { @@ -93,6 +96,7 @@ public class GMSEncrypt implements Cloneable{ protected synchronized void initClusterSecretKey() throws Exception { if(this.clusterEncryptor == null) { this.clusterEncryptor = new ClusterEncryptor(this); + } } @@ -102,19 +106,26 @@ public class GMSEncrypt implements Cloneable{ } protected GMSEncrypt() { - + initEncryptors(); } public GMSEncrypt(Services services) throws Exception { this.services = services; + initEncryptors(); initDHKeys(services.getConfig().getDistributionConfig()); } public GMSEncrypt(Services services, InternalDistributedMember mbr) throws Exception { this.services = services; this.localMember = mbr; + initEncryptors(); initDHKeys(services.getConfig().getDistributionConfig()); } + + void initEncryptors() { + copyOfPeerEncryptors = new ConcurrentHashMap[numberOfPeerEncryptorCopies]; + clusterEncryptors = new ClusterEncryptor[numberOfPeerEncryptorCopies]; + } public byte[] decryptData(byte[] data, InternalDistributedMember member) throws Exception { return getPeerEncryptor(member).decryptBytes(data); @@ -126,7 +137,7 @@ public class GMSEncrypt implements Cloneable{ public byte[] decryptData(byte[] data) throws Exception { - return this.clusterEncryptor.decryptBytes(data); + return getClusterEncryptor().decryptBytes(data); } public byte[] decryptData(byte[] data, byte[] pkBytes) throws Exception { @@ -135,7 +146,7 @@ public class GMSEncrypt implements Cloneable{ } public byte[] encryptData(byte[] data) throws Exception { - return this.clusterEncryptor.encryptBytes(data); + return getClusterEncryptor().encryptBytes(data); } protected byte[] getPublicKeyBytes() { @@ -156,7 +167,8 @@ public class GMSEncrypt implements Cloneable{ protected void setPublicKey(byte[] publickey, InternalDistributedMember mbr) { try { - createPeerEncryptor(mbr, publickey); + //createPeerEncryptor(mbr, publickey); + memberToPeerEncryptor.put(mbr, publickey); }catch(Exception e) { throw new RuntimeException("Unable to create peer encryptor " + mbr, e); } @@ -209,21 +221,56 @@ public class GMSEncrypt implements Cloneable{ } protected PeerEncryptor getPeerEncryptor(InternalDistributedMember member) throws Exception { - PeerEncryptor result = memberToPeerEncryptor.get(member); + Map<InternalDistributedMember, PeerEncryptor> m = getPeerEncryptorMap(); + + PeerEncryptor result = m.get(member); if (result == null) { synchronized (this) { - result = memberToPeerEncryptor.get(member); + result = m.get(member); if (result == null) { - result = createPeerEncryptor(member, (byte[]) view.getPublicKey(member)); + byte[] pk = (byte[])memberToPeerEncryptor.get(member); + result = createPeerEncryptor(member, pk != null ? pk : (byte[]) view.getPublicKey(member)); + m.put(member, result); } } } return result; } + private Map<InternalDistributedMember, PeerEncryptor> getPeerEncryptorMap() { + int h = Math.abs(Thread.currentThread().getName().hashCode() % numberOfPeerEncryptorCopies); + ConcurrentHashMap m = copyOfPeerEncryptors[h]; + + if(m == null) { + synchronized (copyOfPeerEncryptors) { + m = copyOfPeerEncryptors[h]; + if(m == null) { + m = new ConcurrentHashMap<InternalDistributedMember, PeerEncryptor>(); + copyOfPeerEncryptors[h] = m; + } + } + } + return m; + } + + private ClusterEncryptor getClusterEncryptor() { + int h = Math.abs(Thread.currentThread().getName().hashCode() % numberOfPeerEncryptorCopies); + ClusterEncryptor c = clusterEncryptors[h]; + + if(c == null) { + synchronized (copyOfPeerEncryptors) { + c = clusterEncryptors[h]; + if(c == null) { + c = new ClusterEncryptor(getClusterSecretKey()); + clusterEncryptors[h] = c; + } + } + } + return c; + } + private PeerEncryptor createPeerEncryptor(InternalDistributedMember member, byte[] peerKeyBytes) throws Exception { - PeerEncryptor result = new PeerEncryptor(peerKeyBytes); - memberToPeerEncryptor.put(member, result); + PeerEncryptor result = new PeerEncryptor(peerKeyBytes); return result; } @@ -281,10 +328,7 @@ public class GMSEncrypt implements Cloneable{ return blocksize; } - static public byte[] encryptBytes(byte[] data, Cipher encrypt) throws Exception{ - synchronized(GMSEncrypt.class) { - encodingsPerformed++; - } + static public byte[] encryptBytes(byte[] data, Cipher encrypt) throws Exception { return encrypt.doFinal(data); } @@ -292,9 +336,6 @@ public class GMSEncrypt implements Cloneable{ throws Exception{ try { byte[] decryptBytes = decrypt.doFinal(data); - synchronized(GMSEncrypt.class) { - decodingsPerformed++; - } return decryptBytes; }catch(Exception ex) { throw ex; @@ -360,8 +401,8 @@ public class GMSEncrypt implements Cloneable{ } - protected static Cipher getEncryptCipher(String dhSKAlgo, PrivateKey privateKey, PublicKey peerPublicKey) - throws Exception{ + //this needs to synchronize as it uses private key of that member + protected static synchronized Cipher getEncryptCipher(String dhSKAlgo, PrivateKey privateKey, PublicKey peerPublicKey) throws Exception { KeyAgreement ka = KeyAgreement.getInstance("DH"); ka.init(privateKey); ka.doPhase(peerPublicKey, true); @@ -412,10 +453,11 @@ public class GMSEncrypt implements Cloneable{ encrypt.init(Cipher.ENCRYPT_MODE, sks, ivps); } - return encrypt; - } - - protected static Cipher getDecryptCipher(String dhSKAlgo, PrivateKey privateKey, PublicKey publicKey) throws Exception { + return encrypt; + } + + //this needs to synchronize as it uses private key of that member + protected static synchronized Cipher getDecryptCipher(String dhSKAlgo, PrivateKey privateKey, PublicKey publicKey) throws Exception { KeyAgreement ka = KeyAgreement.getInstance("DH"); ka.init(privateKey); ka.doPhase(publicKey, true); @@ -521,9 +563,10 @@ public class GMSEncrypt implements Cloneable{ try { if(encrypt == null) { synchronized (this) { - if(encrypt == null) + if (encrypt == null) { encrypt = GMSEncrypt.getEncryptCipher(dhSKAlgo, secretBytes); - } + } + } } }catch(Exception ex) { throw ex; @@ -541,9 +584,10 @@ public class GMSEncrypt implements Cloneable{ throws Exception{ if(decrypt == null) { synchronized (this) { - if(decrypt == null) + if (decrypt == null) { decrypt = GMSEncrypt.getDecryptCipher(dhSKAlgo, secretBytes); - } + } + } } return decrypt; } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/3a643084/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorUDPSecurityDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorUDPSecurityDUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorUDPSecurityDUnitTest.java index 37f14c3..be88d50 100755 --- a/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorUDPSecurityDUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorUDPSecurityDUnitTest.java @@ -8,13 +8,12 @@ import com.gemstone.gemfire.distributed.internal.DistributionConfig; public class LocatorUDPSecurityDUnitTest extends LocatorDUnitTest{ - public LocatorUDPSecurityDUnitTest(String name) { - super(name); + public LocatorUDPSecurityDUnitTest() { } @Test public void testLoop() throws Exception { - for(int i=0; i < 2; i++) { + for(int i=0; i < 1; i++) { testMultipleLocatorsRestartingAtSameTime(); tearDown(); setUp(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/3a643084/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java index e8f99d9..ace40d6 100755 --- a/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java @@ -18,12 +18,16 @@ import java.math.BigInteger; import java.security.Key; import java.security.KeyPair; import java.security.KeyPairGenerator; + import org.junit.experimental.categories.Category; import java.util.Arrays; import java.util.LinkedList; import java.util.List; import java.util.Properties; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import javax.crypto.KeyAgreement; import javax.crypto.Mac; @@ -112,6 +116,71 @@ public class GMSEncryptJUnitTest { } @Test + public void testOneMemberCanDecryptAnothersMessageMultithreaded() throws Exception{ + initMocks(); + final int runs = 100000; + final GMSEncrypt gmsEncrypt1 = new GMSEncrypt(services, mockMembers[1]); // this will be the sender + final GMSEncrypt gmsEncrypt2 = new GMSEncrypt(services, mockMembers[2]); // this will be the receiver + + // establish the public keys for the sender and receiver + netView.setPublicKey(mockMembers[1], gmsEncrypt1.getPublicKeyBytes()); + netView.setPublicKey(mockMembers[2], gmsEncrypt2.getPublicKeyBytes()); + + gmsEncrypt1.installView(netView, mockMembers[1]); + gmsEncrypt2.installView(netView, mockMembers[2]); + int nthreads = 20; + ExecutorService executorService = Executors.newFixedThreadPool(nthreads); + final CountDownLatch countDownLatch = new CountDownLatch(nthreads); + + for(int j = 0; j < nthreads; j++) + executorService.execute(new Runnable() { + public void run() { + // sender encrypts a message, so use receiver's public key + try { + int count = 0; + for (int i = 0; i < runs; i++) { + // System.out.println("another run " + i + " threadid " + Thread.currentThread().getId()); + String ch = "Hello world"; + byte[] challenge = ch.getBytes(); + byte[] encryptedChallenge = gmsEncrypt1.encryptData(challenge, mockMembers[2]); + + // receiver decrypts the message using the sender's public key + byte[] decryptBytes = gmsEncrypt2.decryptData(encryptedChallenge, mockMembers[1]); + + // now send a response + String response = "Hello yourself!"; + byte[] responseBytes = response.getBytes(); + byte[] encryptedResponse = gmsEncrypt2.encryptData(responseBytes, mockMembers[1]); + + // receiver decodes the response + byte[] decryptedResponse = gmsEncrypt1.decryptData(encryptedResponse, mockMembers[2]); + + Assert.assertFalse(Arrays.equals(challenge, encryptedChallenge)); + + Assert.assertTrue(Arrays.equals(challenge, decryptBytes)); + + Assert.assertFalse(Arrays.equals(responseBytes, encryptedResponse)); + + Assert.assertTrue(Arrays.equals(responseBytes, decryptedResponse)); + count++; + } + Assert.assertEquals(runs, count); + countDownLatch.countDown(); + } catch (Exception e) { + e.printStackTrace(); + + } + + } + }); + + countDownLatch.await(); + executorService.shutdown(); + + + } + + @Test public void testPublicKeyPrivateKeyFromSameMember() throws Exception{ initMocks(); @@ -179,12 +248,13 @@ public class GMSEncryptJUnitTest { } @Test - public void testForClusterSecretKeyFromOtherMember() throws Exception{ + public void testForClusterSecretKeyFromOtherMemberMultipleThreads() throws Exception{ initMocks(); - GMSEncrypt gmsEncrypt1 = new GMSEncrypt(services, mockMembers[1]); // this will be the sender + final GMSEncrypt gmsEncrypt1 = new GMSEncrypt(services, mockMembers[1]); // this will be the sender + Thread.currentThread().sleep(100); gmsEncrypt1.initClusterSecretKey(); - GMSEncrypt gmsEncrypt2 = new GMSEncrypt(services, mockMembers[2]); // this will be the sender + final GMSEncrypt gmsEncrypt2 = new GMSEncrypt(services, mockMembers[2]); // this will be the sender // establish the public keys for the sender and receiver netView.setPublicKey(mockMembers[1], gmsEncrypt1.getPublicKeyBytes()); @@ -197,29 +267,57 @@ public class GMSEncryptJUnitTest { gmsEncrypt2.installView(netView, mockMembers[1]); - // sender encrypts a message, so use receiver's public key - String ch = "Hello world"; - byte[] challenge = ch.getBytes(); - byte[] encryptedChallenge = gmsEncrypt1.encryptData(challenge); - - // receiver decrypts the message using the sender's public key - byte[] decryptBytes = gmsEncrypt2.decryptData(encryptedChallenge); + final int runs = 100000; + int nthreads = 20; + ExecutorService executorService = Executors.newFixedThreadPool(nthreads); + final CountDownLatch countDownLatch = new CountDownLatch(nthreads); - // now send a response - String response = "Hello yourself!"; - byte[] responseBytes = response.getBytes(); - byte[] encryptedResponse = gmsEncrypt2.encryptData(responseBytes); - - // receiver decodes the response - byte[] decryptedResponse = gmsEncrypt1.decryptData(encryptedResponse); - - Assert.assertFalse(Arrays.equals(challenge, encryptedChallenge)); - - Assert.assertTrue(Arrays.equals(challenge, decryptBytes)); - - Assert.assertFalse(Arrays.equals(responseBytes, encryptedResponse)); - - Assert.assertTrue(Arrays.equals(responseBytes, decryptedResponse)); + for (int j = 0; j < nthreads; j++) + executorService.execute(new Runnable() { + public void run() { + // sender encrypts a message, so use receiver's public key + try { + int count = 0; + for (int i = 0; i < runs; i++) { + //System.out.println("run " + i + " threadid " + Thread.currentThread().getId()); + String ch = "Hello world"; + byte[] challenge = ch.getBytes(); + byte[] encryptedChallenge = gmsEncrypt1.encryptData(challenge); + + // receiver decrypts the message using the sender's public key + byte[] decryptBytes = gmsEncrypt2.decryptData(encryptedChallenge); + + // now send a response + String response = "Hello yourself!"; + byte[] responseBytes = response.getBytes(); + byte[] encryptedResponse = gmsEncrypt2.encryptData(responseBytes); + + // receiver decodes the response + byte[] decryptedResponse = gmsEncrypt1.decryptData(encryptedResponse); + + Assert.assertFalse(Arrays.equals(challenge, encryptedChallenge)); + + Assert.assertTrue(Arrays.equals(challenge, decryptBytes)); + + Assert.assertFalse(Arrays.equals(responseBytes, encryptedResponse)); + + Assert.assertTrue(Arrays.equals(responseBytes, decryptedResponse)); + + count++; + } + + Assert.assertEquals(runs, count); + + countDownLatch.countDown(); + } catch (Exception e) { + e.printStackTrace(); + } + + } + }); + + countDownLatch.await(); + executorService.shutdown(); }
