You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2018/01/31 22:48:30 UTC

[geode] branch feature/GEODE-4439 updated: GEODE-4439 Refactor HandShake.java

This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch feature/GEODE-4439
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/feature/GEODE-4439 by this push:
     new fc5a8be  GEODE-4439 Refactor HandShake.java
fc5a8be is described below

commit fc5a8be7afc68dc5b1ba65d505b6dff8185dbbbe
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Wed Jan 31 14:47:42 2018 -0800

    GEODE-4439 Refactor HandShake.java
    
    fixing test failures
---
 .../client/internal/ClientSideHandshakeImpl.java   |  6 +--
 .../internal/cache/tier/sockets/AcceptorImpl.java  |  4 +-
 .../internal/cache/tier/sockets/EncryptorImpl.java | 61 +++++++++-------------
 .../internal/cache/tier/sockets/Handshake.java     | 12 +++--
 4 files changed, 36 insertions(+), 47 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientSideHandshakeImpl.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientSideHandshakeImpl.java
index e617da1..57cd90e 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientSideHandshakeImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientSideHandshakeImpl.java
@@ -443,11 +443,7 @@ public class ClientSideHandshakeImpl extends Handshake implements ClientSideHand
     return super.writeCredential(dos, dis, authInit, isNotification, member, heapdos);
   }
 
-  /**
-   * Handshake implements the Diffie-Hellman encryption algorithms
-   *
-   * @return
-   */
+  @Override
   public Encryptor getEncryptor() {
     return encryptor;
   }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
index 08d237a..5aff86c 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
@@ -1817,7 +1817,7 @@ public class AcceptorImpl implements Acceptor, Runnable, CommBufferPool {
     releaseCommBuffer(Message.setTLCommBuffer(null));
   }
 
-  private class ClientQueueInitializerTask implements Runnable {
+  private static class ClientQueueInitializerTask implements Runnable {
     private final Socket socket;
     private final boolean isPrimaryServerToClient;
     private final AcceptorImpl acceptor;
@@ -1838,7 +1838,7 @@ public class AcceptorImpl implements Acceptor, Runnable, CommBufferPool {
             acceptor.getAcceptorId(), acceptor.isNotifyBySubscription());
       } catch (IOException ex) {
         closeSocket(socket);
-        if (isRunning()) {
+        if (acceptor.isRunning()) {
           if (!acceptor.loggedAcceptError) {
             acceptor.loggedAcceptError = true;
             if (ex instanceof SocketTimeoutException) {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/EncryptorImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/EncryptorImpl.java
index cea4b0b..a7aa072 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/EncryptorImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/EncryptorImpl.java
@@ -74,7 +74,7 @@ import org.apache.geode.internal.logging.InternalLogWriter;
 import org.apache.geode.security.AuthenticationFailedException;
 import org.apache.geode.security.GemFireSecurityException;
 
-public class EncryptorImpl implements Encryptor{
+public class EncryptorImpl implements Encryptor {
   // Parameters for the Diffie-Hellman key exchange
   private static final BigInteger dhP =
       new BigInteger("13528702063991073999718992897071702177131142188276542919088770094024269"
@@ -93,33 +93,33 @@ public class EncryptorImpl implements Encryptor{
   private static final int dhL = 1023;
 
   private Cipher _encrypt;
-  private Cipher _decrypt = null;
+  private Cipher _decrypt;
 
-  private PublicKey clientPublicKey = null;
+  private PublicKey clientPublicKey;
 
-  private String clientSKAlgo = null;
+  private String clientSKAlgo;
 
-  private static PrivateKey dhPrivateKey = null;
+  private static PrivateKey dhPrivateKey;
 
-  private static PublicKey dhPublicKey = null;
+  private static PublicKey dhPublicKey;
 
-  private static String dhSKAlgo = null;
+  private static String dhSKAlgo;
 
   // Members for server authentication using digital signature
 
-  private static String certificateFilePath = null;
+  private static String certificateFilePath;
 
-  private static HashMap certificateMap = null;
+  private static HashMap certificateMap;
 
-  private static String privateKeyAlias = null;
+  private static String privateKeyAlias;
 
-  private static String privateKeySubject = null;
+  private static String privateKeySubject;
 
-  private static PrivateKey privateKeyEncrypt = null;
+  private static PrivateKey privateKeyEncrypt;
 
-  private static String privateKeySignAlgo = null;
+  private static String privateKeySignAlgo;
 
-  private static SecureRandom random = null;
+  private static SecureRandom random;
 
   private byte appSecureMode = (byte) 0;
 
@@ -141,7 +141,7 @@ public class EncryptorImpl implements Encryptor{
   }
 
   public static byte[] decryptBytes(byte[] data, Cipher decrypt) throws Exception {
-      return decrypt.doFinal(data);
+    return decrypt.doFinal(data);
   }
 
   protected Cipher getDecryptCipher(String dhSKAlgo, PublicKey publicKey) throws Exception {
@@ -347,20 +347,18 @@ public class EncryptorImpl implements Encryptor{
   }
 
   boolean isEnabled() {
-    return dhSKAlgo == null || dhSKAlgo.length() == 0;
+    return dhSKAlgo != null && dhSKAlgo.length() > 0;
   }
 
   byte writeEncryptedCredential(DataOutputStream dos, DataInputStream dis, boolean isNotification,
-                                        DistributedMember member, HeapDataOutputStream heapdos)
-      throws IOException {
+      DistributedMember member, HeapDataOutputStream heapdos) throws IOException {
     byte acceptanceCode;
     try {
       logWriter.fine("HandShake: using Diffie-Hellman key exchange with algo " + dhSKAlgo);
       boolean requireAuthentication =
           (certificateFilePath != null && certificateFilePath.length() > 0);
       if (requireAuthentication) {
-        logWriter
-            .fine("HandShake: server authentication using digital " + "signature required");
+        logWriter.fine("HandShake: server authentication using digital " + "signature required");
       }
       // Credentials with encryption indicator
       heapdos.writeByte(CREDENTIALS_DHENCRYPT);
@@ -407,8 +405,7 @@ public class EncryptorImpl implements Encryptor{
             throw new AuthenticationFailedException(
                 "Mismatch in client " + "challenge bytes. Malicious server?");
           }
-          logWriter
-              .fine("HandShake: Successfully verified the " + "digital signature from server");
+          logWriter.fine("HandShake: Successfully verified the " + "digital signature from server");
         }
 
         // Read server challenge bytes
@@ -442,17 +439,15 @@ public class EncryptorImpl implements Encryptor{
   }
 
   byte writeEncryptedCredentials(DataOutputStream dos, DataInputStream dis,
-                                 Properties p_credentials,
-                                 boolean isNotification, DistributedMember member,
-                                 HeapDataOutputStream heapdos) throws IOException {
+      Properties p_credentials, boolean isNotification, DistributedMember member,
+      HeapDataOutputStream heapdos) throws IOException {
     byte acceptanceCode;
     try {
       logWriter.fine("HandShake: using Diffie-Hellman key exchange with algo " + dhSKAlgo);
       boolean requireAuthentication =
           (certificateFilePath != null && certificateFilePath.length() > 0);
       if (requireAuthentication) {
-        logWriter
-            .fine("HandShake: server authentication using digital " + "signature required");
+        logWriter.fine("HandShake: server authentication using digital signature required");
       }
       // Credentials with encryption indicator
       heapdos.writeByte(CREDENTIALS_DHENCRYPT);
@@ -498,8 +493,7 @@ public class EncryptorImpl implements Encryptor{
             throw new AuthenticationFailedException(
                 "Mismatch in client " + "challenge bytes. Malicious server?");
           }
-          logWriter
-              .fine("HandShake: Successfully verified the " + "digital signature from server");
+          logWriter.fine("HandShake: Successfully verified the " + "digital signature from server");
         }
 
         byte[] challenge = DataSerializer.readByteArray(dis);
@@ -535,9 +529,8 @@ public class EncryptorImpl implements Encryptor{
     return acceptanceCode;
   }
 
-  void readEncryptedCredentials(DataInputStream dis, DataOutputStream dos,
-                                DistributedSystem system, boolean requireAuthentication)
-      throws Exception {
+  void readEncryptedCredentials(DataInputStream dis, DataOutputStream dos, DistributedSystem system,
+      boolean requireAuthentication) throws Exception {
     this.appSecureMode = CREDENTIALS_DHENCRYPT;
     boolean sendAuthentication = dis.readBoolean();
     InternalLogWriter securityLogWriter = (InternalLogWriter) system.getSecurityLogWriter();
@@ -619,9 +612,7 @@ public class EncryptorImpl implements Encryptor{
   }
 
   static Properties getDecryptedCredentials(DataInputStream dis, DataOutputStream dos,
-                                            DistributedSystem system,
-                                            boolean requireAuthentication,
-                                            Properties credentials)
+      DistributedSystem system, boolean requireAuthentication, Properties credentials)
       throws IOException, NoSuchAlgorithmException, InvalidKeySpecException, InvalidKeyException,
       SignatureException, NoSuchPaddingException, InvalidAlgorithmParameterException,
       IllegalBlockSizeException, BadPaddingException, ClassNotFoundException {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Handshake.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Handshake.java
index cf9a876..71599d7 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Handshake.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Handshake.java
@@ -255,7 +255,8 @@ public abstract class Handshake {
       return;
     }
 
-    byte acceptanceCode = encryptor.writeEncryptedCredentials(dos, dis, p_credentials, isNotification, member, heapdos);
+    byte acceptanceCode = encryptor.writeEncryptedCredentials(dos, dis, p_credentials,
+        isNotification, member, heapdos);
     if (acceptanceCode != REPLY_OK && acceptanceCode != REPLY_AUTH_NOT_REQUIRED) {
       // Ignore the useless data
       dis.readByte();
@@ -281,8 +282,8 @@ public abstract class Handshake {
   }
 
   // This assumes that authentication is the last piece of info in handshake
-  Properties readCredential(DataInputStream dis, DataOutputStream dos,
-      DistributedSystem system) throws GemFireSecurityException, IOException {
+  Properties readCredential(DataInputStream dis, DataOutputStream dos, DistributedSystem system)
+      throws GemFireSecurityException, IOException {
 
     Properties credentials = null;
     boolean requireAuthentication = securityService.isClientSecurityRequired();
@@ -290,7 +291,7 @@ public abstract class Handshake {
       byte secureMode = dis.readByte();
       throwIfMissingRequiredCredentials(requireAuthentication, secureMode != CREDENTIALS_NONE);
       if (secureMode == CREDENTIALS_NORMAL) {
-encryptor.setAppSecureMode(CREDENTIALS_NORMAL);
+        encryptor.setAppSecureMode(CREDENTIALS_NORMAL);
       } else if (secureMode == CREDENTIALS_DHENCRYPT) {
         encryptor.readEncryptedCredentials(dis, dos, system, requireAuthentication);
       }
@@ -446,7 +447,8 @@ encryptor.setAppSecureMode(CREDENTIALS_NORMAL);
           DataSerializer.readProperties(dis); // ignore the credentials
         }
       } else if (secureMode == CREDENTIALS_DHENCRYPT) {
-        credentials = EncryptorImpl.getDecryptedCredentials(dis, dos, system, requireAuthentication, credentials);
+        credentials = EncryptorImpl.getDecryptedCredentials(dis, dos, system, requireAuthentication,
+            credentials);
       } else if (secureMode == SECURITY_MULTIUSER_NOTIFICATIONCHANNEL) {
         // hitesh there will be no credential CCP will get credential(Principal) using
         // ServerConnection..

-- 
To stop receiving notification emails like this one, please contact
bschuchardt@apache.org.