geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bschucha...@apache.org
Subject geode git commit: GEODE-3249: Validate internal client/server messages
Date Tue, 15 Aug 2017 23:18:05 GMT
Repository: geode
Updated Branches:
  refs/heads/feature/GEODE-3249b [created] f8e7ddd5e


GEODE-3249: Validate internal client/server messages

This was merely a matter of changing the server to require the credentials
and changing the client to send credentials.  I removed the general overriding
of AbstractOp.processSecureBytes() because it made no sense.  If the server
sends a secure byte "part" in a message the client is obligated to process
it or the next message it sends will cause a security violation.

I've added a server-side property that folks can set to allow old clients
to continue to work.  This must be used to roll the servers forward to the
new version that contains this change.  Clients must then be rolled
forward & the servers can then be rolled once again without the property set.

The system property is
  geode.allow-internal-messages-without-credentials=true


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/f8e7ddd5
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/f8e7ddd5
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/f8e7ddd5

Branch: refs/heads/feature/GEODE-3249b
Commit: f8e7ddd5e4696907ce60a14f581ef1ca83e65232
Parents: c5dd26b
Author: Bruce Schuchardt <bschuchardt@pivotal.io>
Authored: Tue Aug 15 16:15:07 2017 -0700
Committer: Bruce Schuchardt <bschuchardt@pivotal.io>
Committed: Tue Aug 15 16:15:07 2017 -0700

----------------------------------------------------------------------
 .../geode/cache/client/internal/AbstractOp.java |  93 ++++++++--------
 .../cache/client/internal/AddPDXEnumOp.java     |  14 ---
 .../cache/client/internal/AddPDXTypeOp.java     |  14 ---
 .../client/internal/CloseConnectionOp.java      |   3 -
 .../geode/cache/client/internal/CommitOp.java   |   3 -
 .../client/internal/GetClientPRMetaDataOp.java  |   3 -
 .../GetClientPartitionAttributesOp.java         |   3 -
 .../cache/client/internal/GetEventValueOp.java  |   3 -
 .../client/internal/GetFunctionAttributeOp.java |  13 ---
 .../cache/client/internal/GetPDXEnumByIdOp.java |  14 ---
 .../cache/client/internal/GetPDXEnumsOp.java    |  13 ---
 .../client/internal/GetPDXIdForEnumOp.java      |  13 ---
 .../client/internal/GetPDXIdForTypeOp.java      |  14 ---
 .../cache/client/internal/GetPDXTypeByIdOp.java |  13 ---
 .../cache/client/internal/GetPDXTypesOp.java    |  13 ---
 .../cache/client/internal/MakePrimaryOp.java    |   3 -
 .../geode/cache/client/internal/PingOp.java     |   1 +
 .../cache/client/internal/PrimaryAckOp.java     |   3 -
 .../geode/cache/client/internal/PutOp.java      |   4 +-
 .../cache/client/internal/ReadyForEventsOp.java |   3 -
 .../internal/RegisterDataSerializersOp.java     |  13 ---
 .../internal/RegisterInstantiatorsOp.java       |  13 ---
 .../geode/cache/client/internal/RollbackOp.java |   3 -
 .../geode/cache/client/internal/SizeOp.java     |   3 -
 .../cache/client/internal/TXFailoverOp.java     |   3 -
 .../client/internal/TXSynchronizationOp.java    |   3 -
 .../internal/cache/tier/sockets/Message.java    |   1 +
 .../cache/tier/sockets/ServerConnection.java    |  77 ++++++++------
 .../cache/tier/sockets/command/AddPdxType.java  |   1 +
 .../tier/sockets/command/GetPDXIdForType.java   |   1 +
 .../ClientAuthenticationPart2DUnitTest.java     |  32 ++++++
 .../security/ClientAuthenticationTestCase.java  | 106 +++++++++++++++++++
 .../security/ClientAuthorizationTestCase.java   |   3 +-
 .../geode/security/SecurityTestUtils.java       |   3 +-
 .../test/dunit/standalone/VersionManager.java   |   2 -
 .../client/internal/GatewaySenderBatchOp.java   |   3 -
 36 files changed, 239 insertions(+), 271 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java
index c4035f9..a706a29 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AbstractOp.java
@@ -140,6 +140,51 @@ public abstract class AbstractOp implements Op {
   }
 
   /**
+   * Process the security information in a response from the server.  If the server
+   * sends a security "part" we must process it so all subclasses should allow this
+   * method to be invoked.
+   *
+   * @see ServerConnection#updateAndGetSecurityPart()
+   */
+  protected void processSecureBytes(Connection cnx, Message message) throws Exception {
+    if (cnx.getServer().getRequiresCredentials()) {
+      if (!message.isSecureMode()) {
+        // This can be seen during shutdown
+        if (logger.isDebugEnabled()) {
+          logger.trace(LogMarker.BRIDGE_SERVER,
+              "Response message from {} for {} has no secure part.", cnx, this);
+        }
+        return;
+      }
+      byte[] partBytes = message.getSecureBytes();
+      if (partBytes == null) {
+        if (logger.isDebugEnabled()) {
+          logger.debug("Response message for {} has no bytes in secure part.", this);
+        }
+        return;
+      }
+      byte[] bytes = ((ConnectionImpl) cnx).getHandShake().decryptBytes(partBytes);
+      DataInputStream dis = new DataInputStream(new ByteArrayInputStream(bytes));
+      cnx.setConnectionID(dis.readLong());
+    }
+  }
+
+  /**
+   * New implementations of AbstractOp should override this method to return false if the
+   * implementation should be excluded from client authentication. e.g. PingOp#needsUserId()
+   * <P/>
+   * Also, such an operation's <code>MessageType</code> must be added in the 'if' condition in
+   * {@link ServerConnection#updateAndGetSecurityPart()}
+   *
+   * @return boolean
+   * @see AbstractOp#sendMessage(Connection)
+   * @see ServerConnection#updateAndGetSecurityPart()
+   */
+  protected boolean needsUserId() {
+    return true;
+  }
+
+  /**
    * Attempts to read a response to this operation by reading it from the given connection, and
    * returning it.
    * 
@@ -174,38 +219,6 @@ public abstract class AbstractOp implements Op {
   }
 
   /**
-   * New implementations of AbstractOp should override this method if the implementation should be
-   * excluded from client authentication. e.g. PingOp#processSecureBytes(Connection cnx, Message
-   * message)
-   * 
-   * @see AbstractOp#sendMessage(Connection)
-   * @see AbstractOp#needsUserId()
-   * @see ServerConnection#updateAndGetSecurityPart()
-   */
-  protected void processSecureBytes(Connection cnx, Message message) throws Exception {
-    if (cnx.getServer().getRequiresCredentials()) {
-      if (!message.isSecureMode()) {
-        // This can be seen during shutdown
-        if (logger.isDebugEnabled()) {
-          logger.trace(LogMarker.BRIDGE_SERVER,
-              "Response message from {} for {} has no secure part.", cnx, this);
-        }
-        return;
-      }
-      byte[] partBytes = message.getSecureBytes();
-      if (partBytes == null) {
-        if (logger.isDebugEnabled()) {
-          logger.debug("Response message for {} has no bytes in secure part.", this);
-        }
-        return;
-      }
-      byte[] bytes = ((ConnectionImpl) cnx).getHandShake().decryptBytes(partBytes);
-      DataInputStream dis = new DataInputStream(new ByteArrayInputStream(bytes));
-      cnx.setConnectionID(dis.readLong());
-    }
-  }
-
-  /**
    * By default just create a normal one part msg. Subclasses can override this.
    */
   protected Message createResponseMessage() {
@@ -405,22 +418,6 @@ public abstract class AbstractOp implements Op {
   protected abstract void endAttempt(ConnectionStats stats, long start);
 
   /**
-   * New implementations of AbstractOp should override this method to return false if the
-   * implementation should be excluded from client authentication. e.g. PingOp#needsUserId()
-   * <P/>
-   * Also, such an operation's <code>MessageType</code> must be added in the 'if' condition in
-   * {@link ServerConnection#updateAndGetSecurityPart()}
-   * 
-   * @return boolean
-   * @see AbstractOp#sendMessage(Connection)
-   * @see AbstractOp#processSecureBytes(Connection, Message)
-   * @see ServerConnection#updateAndGetSecurityPart()
-   */
-  protected boolean needsUserId() {
-    return true;
-  }
-
-  /**
    * Subclasses for AbstractOp should override this method to return false in this message should
    * not participate in any existing transaction
    * 

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXEnumOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXEnumOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXEnumOp.java
index ca7790a..857d1d3 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXEnumOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXEnumOp.java
@@ -75,25 +75,11 @@ public class AddPDXEnumOp {
       stats.endAddPdxType(start, hasTimedOut(), hasFailed());
     }
 
-    @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
     // Don't send the transaction id for this message type.
     @Override
     protected boolean participateInTransaction() {
       return false;
     }
 
-    // override since this is not a message subject to security
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXTypeOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXTypeOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXTypeOp.java
index 88c8551..4eb137d 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXTypeOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AddPDXTypeOp.java
@@ -75,25 +75,11 @@ public class AddPDXTypeOp {
       stats.endAddPdxType(start, hasTimedOut(), hasFailed());
     }
 
-    @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
     // Don't send the transaction id for this message type.
     @Override
     protected boolean participateInTransaction() {
       return false;
     }
 
-    // override since this is not a message subject to security
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/CloseConnectionOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/CloseConnectionOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/CloseConnectionOp.java
index ffcdc39..ea8a8b5 100755
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/CloseConnectionOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/CloseConnectionOp.java
@@ -54,9 +54,6 @@ public class CloseConnectionOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/CommitOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/CommitOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/CommitOp.java
index edffb2b..f44d62d 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/CommitOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/CommitOp.java
@@ -72,9 +72,6 @@ public class CommitOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetClientPRMetaDataOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetClientPRMetaDataOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetClientPRMetaDataOp.java
index 2ba3e3a..cc25416 100755
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetClientPRMetaDataOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetClientPRMetaDataOp.java
@@ -68,9 +68,6 @@ public class GetClientPRMetaDataOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetClientPartitionAttributesOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetClientPartitionAttributesOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetClientPartitionAttributesOp.java
index 49567dd..ba7463e 100755
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetClientPartitionAttributesOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetClientPartitionAttributesOp.java
@@ -73,9 +73,6 @@ public class GetClientPartitionAttributesOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetEventValueOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetEventValueOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetEventValueOp.java
index 3fb5fcf..8804e05 100755
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetEventValueOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetEventValueOp.java
@@ -59,9 +59,6 @@ public class GetEventValueOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetFunctionAttributeOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetFunctionAttributeOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetFunctionAttributeOp.java
index c7edbfe..dea49a2 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetFunctionAttributeOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetFunctionAttributeOp.java
@@ -63,18 +63,5 @@ public class GetFunctionAttributeOp {
       stats.endGet(start, hasTimedOut(), hasFailed());
     }
 
-    @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXEnumByIdOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXEnumByIdOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXEnumByIdOp.java
index 7bbf740..dc94fe5 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXEnumByIdOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXEnumByIdOp.java
@@ -72,24 +72,10 @@ public class GetPDXEnumByIdOp {
       stats.endGetPDXTypeById(start, hasTimedOut(), hasFailed());
     }
 
-    @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
     // Don't send the transaction id for this message type.
     @Override
     protected boolean participateInTransaction() {
       return false;
     }
-
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXEnumsOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXEnumsOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXEnumsOp.java
index be4c092..3158eb3 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXEnumsOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXEnumsOp.java
@@ -84,22 +84,9 @@ public class GetPDXEnumsOp {
     protected void endAttempt(ConnectionStats stats, long start) {}
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
-    @Override
     protected boolean participateInTransaction() {
       return false;
     }
 
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXIdForEnumOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXIdForEnumOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXIdForEnumOp.java
index d87371c..9ad85f0 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXIdForEnumOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXIdForEnumOp.java
@@ -94,24 +94,11 @@ public class GetPDXIdForEnumOp {
       stats.endGetPDXTypeById(start, hasTimedOut(), hasFailed());
     }
 
-    @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
     // Don't send the transaction id for this message type.
     @Override
     protected boolean participateInTransaction() {
       return false;
     }
 
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXIdForTypeOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXIdForTypeOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXIdForTypeOp.java
index 27f600e..cc0cd65 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXIdForTypeOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXIdForTypeOp.java
@@ -93,24 +93,10 @@ public class GetPDXIdForTypeOp {
       stats.endGetPDXTypeById(start, hasTimedOut(), hasFailed());
     }
 
-    @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
     // Don't send the transaction id for this message type.
     @Override
     protected boolean participateInTransaction() {
       return false;
     }
-
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXTypeByIdOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXTypeByIdOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXTypeByIdOp.java
index bee50b5..826d4cd 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXTypeByIdOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXTypeByIdOp.java
@@ -72,24 +72,11 @@ public class GetPDXTypeByIdOp {
       stats.endGetPDXTypeById(start, hasTimedOut(), hasFailed());
     }
 
-    @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
     // Don't send the transaction id for this message type.
     @Override
     protected boolean participateInTransaction() {
       return false;
     }
 
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXTypesOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXTypesOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXTypesOp.java
index 5256924..9186680 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXTypesOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/GetPDXTypesOp.java
@@ -84,22 +84,9 @@ public class GetPDXTypesOp {
     protected void endAttempt(ConnectionStats stats, long start) {}
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
-    @Override
     protected boolean participateInTransaction() {
       return false;
     }
 
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/MakePrimaryOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/MakePrimaryOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/MakePrimaryOp.java
index e1d3d50..0332507 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/MakePrimaryOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/MakePrimaryOp.java
@@ -49,9 +49,6 @@ public class MakePrimaryOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/PingOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PingOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PingOp.java
index 2e52542..fb07b39 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PingOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PingOp.java
@@ -53,6 +53,7 @@ public class PingOp {
 
     @Override
     protected void processSecureBytes(Connection cnx, Message message) throws Exception {
+      super.processSecureBytes(cnx, message);
       Message.MESSAGE_TYPE.set(null);
     }
 

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/PrimaryAckOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PrimaryAckOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PrimaryAckOp.java
index e380e99..d7d32a7 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PrimaryAckOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PrimaryAckOp.java
@@ -56,9 +56,6 @@ public class PrimaryAckOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutOp.java
index 447ed38..1390c2d 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutOp.java
@@ -409,9 +409,7 @@ public class PutOp {
 
     @Override
     protected void processSecureBytes(Connection cnx, Message message) throws Exception {
-      if (!this.isMetaRegionPutOp) {
-        super.processSecureBytes(cnx, message);
-      }
+      super.processSecureBytes(cnx, message);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/ReadyForEventsOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ReadyForEventsOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ReadyForEventsOp.java
index f6d0ccb..12e15b4 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ReadyForEventsOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ReadyForEventsOp.java
@@ -48,9 +48,6 @@ public class ReadyForEventsOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/RegisterDataSerializersOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/RegisterDataSerializersOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/RegisterDataSerializersOp.java
index 5b25961..b40a840 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/RegisterDataSerializersOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/RegisterDataSerializersOp.java
@@ -117,18 +117,5 @@ public class RegisterDataSerializersOp {
       stats.endRegisterDataSerializers(start, hasTimedOut(), hasFailed());
     }
 
-    @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/RegisterInstantiatorsOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/RegisterInstantiatorsOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/RegisterInstantiatorsOp.java
index 114bebe..40ce619 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/RegisterInstantiatorsOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/RegisterInstantiatorsOp.java
@@ -150,18 +150,5 @@ public class RegisterInstantiatorsOp {
       stats.endRegisterInstantiators(start, hasTimedOut(), hasFailed());
     }
 
-    @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
-    protected boolean needsUserId() {
-      return false;
-    }
-
-    @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      getMessage().clearMessageHasSecurePartFlag();
-      getMessage().send(false);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/RollbackOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/RollbackOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/RollbackOp.java
index 4704f3a..97cb2e6 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/RollbackOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/RollbackOp.java
@@ -80,9 +80,6 @@ public class RollbackOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/SizeOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/SizeOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/SizeOp.java
index ac8c95e..fb3ffec 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/SizeOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/SizeOp.java
@@ -75,9 +75,6 @@ public class SizeOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXFailoverOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXFailoverOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXFailoverOp.java
index 17fc701..0995981 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXFailoverOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXFailoverOp.java
@@ -74,9 +74,6 @@ public class TXFailoverOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXSynchronizationOp.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXSynchronizationOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXSynchronizationOp.java
index 0c4086c..a02d463 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXSynchronizationOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/TXSynchronizationOp.java
@@ -147,9 +147,6 @@ public class TXSynchronizationOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Message.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Message.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Message.java
index 1f9ef91..b7835a3 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Message.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Message.java
@@ -1029,6 +1029,7 @@ public class Message {
     sb.append("type=").append(MessageType.getString(this.messageType));
     sb.append("; payloadLength=").append(this.payloadLength);
     sb.append("; numberOfParts=").append(this.numberOfParts);
+    sb.append("; hasSecurePart=").append(isSecureMode());
     sb.append("; transactionId=").append(this.transactionId);
     sb.append("; currentPart=").append(this.currentPart);
     sb.append("; messageModified=").append(this.messageModified);

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
index 870d0ff..be6080e 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
@@ -84,6 +84,17 @@ public abstract class ServerConnection implements Runnable {
    */
   private static final int TIMEOUT_BUFFER_FOR_CONNECTION_CLEANUP_MS = 5000;
 
+  public static final String ALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS_NAME =
+      "geode.allow-internal-messages-without-credentials";
+
+  /**
+   * This property allows folks to perform a rolling upgrade from pre-1.2.1 to a post-1.2.1 cluster.
+   * Normally internal messages that can affect server state require credentials but pre-1.2.1 this
+   * wasn't the case. See GEODE-3249
+   */
+  private static final boolean ALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS =
+      Boolean.getBoolean(ALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS_NAME);
+
   private Map commands;
 
   private final SecurityService securityService;
@@ -764,7 +775,8 @@ public abstract class ServerConnection implements Runnable {
 
         // if a subject exists for this uniqueId, binds the subject to this thread so that we can do
         // authorization later
-        if (AcceptorImpl.isIntegratedSecurity() && !isInternalMessage()
+        if (AcceptorImpl.isIntegratedSecurity()
+            && !isInternalMessage(this.requestMsg, ALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS)
             && this.communicationMode != Acceptor.GATEWAY_TO_GATEWAY) {
           long uniqueId = getUniqueId();
           Subject subject = this.clientUserAuths.getSubject(uniqueId);
@@ -1044,6 +1056,7 @@ public abstract class ServerConnection implements Runnable {
   private void setSecurityPart() {
     try {
       this.connectionId = randomConnectionIdGen.nextLong();
+      logger.info("ServerConnection setting connectionId to {} for message {}", connectionId, requestMsg);
       this.securePart = new Part();
       byte[] id = encryptId(this.connectionId, this);
       this.securePart.setPartState(id, false);
@@ -1068,7 +1081,8 @@ public abstract class ServerConnection implements Runnable {
     if (AcceptorImpl.isAuthenticationRequired()
         && this.handshake.getVersion().compareTo(Version.GFE_65) >= 0
         && (this.communicationMode != Acceptor.GATEWAY_TO_GATEWAY)
-        && (!this.requestMsg.getAndResetIsMetaRegion()) && (!isInternalMessage())) {
+        && (!this.requestMsg.getAndResetIsMetaRegion())
+        && (!isInternalMessage(this.requestMsg, ALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS))) {
       setSecurityPart();
       return this.securePart;
     } else {
@@ -1081,34 +1095,37 @@ public abstract class ServerConnection implements Runnable {
     return null;
   }
 
-  private boolean isInternalMessage() {
-    return (this.requestMsg.messageType == MessageType.CLIENT_READY
-        || this.requestMsg.messageType == MessageType.CLOSE_CONNECTION
-        || this.requestMsg.messageType == MessageType.GETCQSTATS_MSG_TYPE
-        || this.requestMsg.messageType == MessageType.GET_CLIENT_PARTITION_ATTRIBUTES
-        || this.requestMsg.messageType == MessageType.GET_CLIENT_PR_METADATA
-        || this.requestMsg.messageType == MessageType.INVALID
-        || this.requestMsg.messageType == MessageType.MAKE_PRIMARY
-        || this.requestMsg.messageType == MessageType.MONITORCQ_MSG_TYPE
-        || this.requestMsg.messageType == MessageType.PERIODIC_ACK
-        || this.requestMsg.messageType == MessageType.PING
-        || this.requestMsg.messageType == MessageType.REGISTER_DATASERIALIZERS
-        || this.requestMsg.messageType == MessageType.REGISTER_INSTANTIATORS
-        || this.requestMsg.messageType == MessageType.REQUEST_EVENT_VALUE
-        || this.requestMsg.messageType == MessageType.ADD_PDX_TYPE
-        || this.requestMsg.messageType == MessageType.GET_PDX_ID_FOR_TYPE
-        || this.requestMsg.messageType == MessageType.GET_PDX_TYPE_BY_ID
-        || this.requestMsg.messageType == MessageType.SIZE
-        || this.requestMsg.messageType == MessageType.TX_FAILOVER
-        || this.requestMsg.messageType == MessageType.TX_SYNCHRONIZATION
-        || this.requestMsg.messageType == MessageType.GET_FUNCTION_ATTRIBUTES
-        || this.requestMsg.messageType == MessageType.ADD_PDX_ENUM
-        || this.requestMsg.messageType == MessageType.GET_PDX_ID_FOR_ENUM
-        || this.requestMsg.messageType == MessageType.GET_PDX_ENUM_BY_ID
-        || this.requestMsg.messageType == MessageType.GET_PDX_TYPES
-        || this.requestMsg.messageType == MessageType.GET_PDX_ENUMS
-        || this.requestMsg.messageType == MessageType.COMMIT
-        || this.requestMsg.messageType == MessageType.ROLLBACK);
+  public boolean isInternalMessage(Message message, boolean allowOldInternalMessages) {
+    int messageType = message.getMessageType();
+    boolean isInternalMessage = messageType == MessageType.PING
+        || messageType == MessageType.USER_CREDENTIAL_MESSAGE
+        || messageType == MessageType.REQUEST_EVENT_VALUE || messageType == MessageType.MAKE_PRIMARY
+        || messageType == MessageType.REMOVE_USER_AUTH || messageType == MessageType.CLIENT_READY
+        || messageType == MessageType.SIZE || messageType == MessageType.TX_FAILOVER
+        || messageType == MessageType.TX_SYNCHRONIZATION || messageType == MessageType.COMMIT
+        || messageType == MessageType.ROLLBACK || messageType == MessageType.CLOSE_CONNECTION
+        || messageType == MessageType.INVALID || messageType == MessageType.PERIODIC_ACK
+        || messageType == MessageType.GET_CLIENT_PR_METADATA
+        || messageType == MessageType.GET_CLIENT_PARTITION_ATTRIBUTES;
+
+    // we allow older clients to not send credentials for a handful of messages
+    // if and only if a system property is set. This allows a rolling upgrade
+    // to be performed.
+    if (!isInternalMessage && allowOldInternalMessages) {
+      isInternalMessage = messageType == MessageType.GETCQSTATS_MSG_TYPE
+          || messageType == MessageType.MONITORCQ_MSG_TYPE
+          || messageType == MessageType.REGISTER_DATASERIALIZERS
+          || messageType == MessageType.REGISTER_INSTANTIATORS
+          || messageType == MessageType.ADD_PDX_TYPE
+          || messageType == MessageType.GET_PDX_ID_FOR_TYPE
+          || messageType == MessageType.GET_PDX_TYPE_BY_ID
+          || messageType == MessageType.GET_FUNCTION_ATTRIBUTES
+          || messageType == MessageType.ADD_PDX_ENUM
+          || messageType == MessageType.GET_PDX_ID_FOR_ENUM
+          || messageType == MessageType.GET_PDX_ENUM_BY_ID
+          || messageType == MessageType.GET_PDX_TYPES || messageType == MessageType.GET_PDX_ENUMS;
+    }
+    return isInternalMessage;
   }
 
   public void run() {

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxType.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxType.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxType.java
index cb4b261..041e12f 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxType.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxType.java
@@ -24,6 +24,7 @@ import org.apache.geode.internal.cache.tier.sockets.BaseCommand;
 import org.apache.geode.internal.cache.tier.sockets.Message;
 import org.apache.geode.internal.cache.tier.sockets.ServerConnection;
 import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.security.AuthorizeRequest;
 import org.apache.geode.internal.security.SecurityService;
 import org.apache.geode.pdx.internal.PdxType;
 import org.apache.geode.pdx.internal.TypeRegistry;

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForType.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForType.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForType.java
index caa0661..f2172ef 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForType.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForType.java
@@ -22,6 +22,7 @@ import org.apache.geode.internal.cache.tier.MessageType;
 import org.apache.geode.internal.cache.tier.sockets.BaseCommand;
 import org.apache.geode.internal.cache.tier.sockets.Message;
 import org.apache.geode.internal.cache.tier.sockets.ServerConnection;
+import org.apache.geode.internal.security.AuthorizeRequest;
 import org.apache.geode.internal.security.SecurityService;
 import org.apache.geode.pdx.internal.PdxType;
 import org.apache.geode.pdx.internal.TypeRegistry;

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationPart2DUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationPart2DUnitTest.java b/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationPart2DUnitTest.java
index 3cf2efc..5a78535 100644
--- a/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationPart2DUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationPart2DUnitTest.java
@@ -14,10 +14,16 @@
  */
 package org.apache.geode.security;
 
+import static org.mockito.Mockito.*;
+
+import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
+import org.apache.geode.internal.cache.tier.MessageType;
+import org.apache.geode.internal.cache.tier.sockets.Message;
+import org.apache.geode.internal.cache.tier.sockets.ServerConnection;
 import org.apache.geode.test.junit.categories.DistributedTest;
 import org.apache.geode.test.junit.categories.SecurityTest;
 
@@ -33,6 +39,32 @@ public class ClientAuthenticationPart2DUnitTest extends ClientAuthenticationTest
     doTestNoCredentials(true);
   }
 
+  // GEODE-3249
+  @Test
+  public void testNoCredentialsForMultipleUsersCantRegisterMetadata() throws Exception {
+    doTestNoCredentialsCantRegisterMetadata(true);
+  }
+
+  @Test
+  public void testServerConnectionAcceptsOldInternalMessagesIfAllowed() throws Exception {
+
+    ServerConnection serverConnection = mock(ServerConnection.class);
+    when(serverConnection.isInternalMessage(any(Message.class), any(Boolean.class)))
+        .thenCallRealMethod();
+
+    int[] oldInternalMessages = new int[] {MessageType.ADD_PDX_TYPE, MessageType.ADD_PDX_ENUM,
+        MessageType.REGISTER_INSTANTIATORS, MessageType.REGISTER_DATASERIALIZERS};
+
+    for (int i = 0; i < oldInternalMessages.length; i++) {
+      Message message = mock(Message.class);
+      when(message.getMessageType()).thenReturn(oldInternalMessages[i]);
+
+      serverConnection.setRequestMsg(message);
+      Assert.assertFalse(serverConnection.isInternalMessage(message, false));
+      Assert.assertTrue(serverConnection.isInternalMessage(message, true));
+    }
+  }
+
   @Test
   public void testInvalidCredentialsForMultipleUsers() throws Exception {
     doTestInvalidCredentials(true);

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationTestCase.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationTestCase.java b/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationTestCase.java
index 1293aff..0ecd72f 100644
--- a/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationTestCase.java
+++ b/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationTestCase.java
@@ -24,11 +24,22 @@ import static org.apache.geode.test.dunit.IgnoredException.*;
 import static org.apache.geode.test.dunit.LogWriterUtils.*;
 import static org.apache.geode.test.dunit.Wait.*;
 
+import java.io.DataInput;
+import java.io.DataOutput;
 import java.io.IOException;
 import java.util.Properties;
 import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLHandshakeException;
 
+import org.apache.geode.DataSerializer;
+import org.apache.geode.cache.client.Pool;
+import org.apache.geode.cache.client.PoolManager;
+import org.apache.geode.cache.client.internal.ExecutablePool;
+import org.apache.geode.cache.client.internal.RegisterDataSerializersOp;
+import org.apache.geode.internal.HeapDataOutputStream;
+import org.apache.geode.internal.InternalDataSerializer;
+import org.apache.geode.internal.Version;
+import org.apache.geode.internal.cache.EventID;
 import org.apache.geode.security.generator.CredentialGenerator;
 import org.apache.geode.security.generator.DummyCredentialGenerator;
 import org.apache.geode.test.dunit.Host;
@@ -52,6 +63,37 @@ public abstract class ClientAuthenticationTestCase extends JUnit4DistributedTest
       {AuthenticationRequiredException.class.getName(),
           AuthenticationFailedException.class.getName(), SSLHandshakeException.class.getName()};
 
+
+  public static enum Color {
+    red, orange, yellow, green, blue, indigo, violet
+  }
+
+
+  public static class MyDataSerializer extends DataSerializer {
+    public MyDataSerializer() {}
+
+    @Override
+    public Class<?>[] getSupportedClasses() {
+      return new Class[] {Color.class};
+    }
+
+    public int getId() {
+      return 1073741824;
+    }
+
+    @Override
+    public boolean toData(Object object, DataOutput output) {
+      return true;
+    }
+
+    @Override
+    public Object fromData(DataInput in) throws IOException, ClassNotFoundException {
+      return Color.red;
+    }
+  }
+
+
+
   @Override
   public final void postSetUp() throws Exception {
     final Host host = Host.getHost(0);
@@ -172,6 +214,70 @@ public abstract class ClientAuthenticationTestCase extends JUnit4DistributedTest
     }
   }
 
+  protected void doTestNoCredentialsCantRegisterMetadata(final boolean multiUser) throws Exception {
+    CredentialGenerator gen = new DummyCredentialGenerator();
+    Properties extraProps = gen.getSystemProperties();
+    Properties javaProps = gen.getJavaProperties();
+    String authenticator = gen.getAuthenticator();
+    String authInit = gen.getAuthInit();
+
+    // Start the servers
+    int locPort1 = getLocatorPort();
+    int locPort2 = getLocatorPort();
+    String locString = getAndClearLocatorString();
+
+    int port1 = createServer1(extraProps, javaProps, authenticator, locPort1, locString);
+    int port2 = server2
+        .invoke(() -> createCacheServer(locPort2, locString, authenticator, extraProps, javaProps));
+
+    // Start first client with valid credentials
+    Properties credentials1 = gen.getValidCredentials(1);
+    Properties javaProps1 = gen.getJavaProperties();
+
+    createClient1NoException(multiUser, authInit, port1, port2, credentials1, javaProps1);
+
+    // Trying to create the region on client2
+    if (gen.classCode().equals(CredentialGenerator.ClassCode.SSL)) {
+      // For SSL the exception may not come since the server can close socket
+      // before handshake message is sent from client. However exception
+      // should come in any region operations.
+      client2.invoke(
+          () -> createCacheClient(null, null, null, port1, port2, 0, multiUser, NO_EXCEPTION));
+      client2.invoke(() -> doPuts(2, OTHER_EXCEPTION));
+
+    } else {
+      client2.invoke(
+          () -> createCacheClient(null, null, null, port1, port2, 0, multiUser, AUTHREQ_EXCEPTION));
+
+      // Try to register a PDX type with the server
+      client2.invoke("register a PDX type", () -> {
+        HeapDataOutputStream outputStream = new HeapDataOutputStream(100, Version.CURRENT);
+        try {
+          DataSerializer.writeObject(new Employee(106l, "David", "Copperfield"), outputStream);
+          throw new Error("operation should have been rejected");
+        } catch (UnsupportedOperationException e) {
+          // "UnsupportedOperationException: Use Pool APIs for doing operations when
+          // multiuser-secure-mode-enabled is set to true."
+        }
+      });
+
+      // Try to register a DataSerializer with the server
+      client2.invoke("register a data serializer", () -> {
+        EventID eventId = InternalDataSerializer.generateEventId();
+        Pool pool = PoolManager.getAll().values().iterator().next();
+        try {
+          RegisterDataSerializersOp.execute((ExecutablePool) pool,
+              new DataSerializer[] {new MyDataSerializer()}, eventId);
+          throw new Error("operation should have been rejected");
+        } catch (UnsupportedOperationException e) {
+          // "UnsupportedOperationException: Use Pool APIs for doing operations when
+          // multiuser-secure-mode-enabled is set to true."
+        }
+      });
+    }
+
+  }
+
   protected void doTestInvalidCredentials(final boolean multiUser) throws Exception {
     CredentialGenerator gen = new DummyCredentialGenerator();
     Properties extraProps = gen.getSystemProperties();

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationTestCase.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationTestCase.java b/geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationTestCase.java
index 9d3f721..a4fd365 100644
--- a/geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationTestCase.java
+++ b/geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationTestCase.java
@@ -288,7 +288,8 @@ public abstract class ClientAuthorizationTestCase extends JUnit4DistributedTestC
 
     final int numOps = indices.length;
     System.out.println("Got doOp for op: " + op.toString() + ", numOps: " + numOps + ", indices: "
-        + indicesToString(indices) + ", expect: " + expectedResult);
+        + indicesToString(indices) + ", expect: " + expectedResult + " flags: "
+        + OpFlags.description(flags));
     boolean exceptionOccurred = false;
     boolean breakLoop = false;
 

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/test/java/org/apache/geode/security/SecurityTestUtils.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/security/SecurityTestUtils.java b/geode-core/src/test/java/org/apache/geode/security/SecurityTestUtils.java
index b1c0907..e69f36d 100644
--- a/geode-core/src/test/java/org/apache/geode/security/SecurityTestUtils.java
+++ b/geode-core/src/test/java/org/apache/geode/security/SecurityTestUtils.java
@@ -1825,7 +1825,7 @@ public class SecurityTestUtils {
 
   // ------------------------------- inner classes ----------------------------
 
-  private static class Employee implements PdxSerializable {
+  public static class Employee implements PdxSerializable {
 
     private Long Id;
     private String fname;
@@ -1854,4 +1854,5 @@ public class SecurityTestUtils {
       out.writeString("lname", lname);
     }
   }
+
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java b/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java
index 739b690..8eefa01 100755
--- a/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java
+++ b/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java
@@ -45,8 +45,6 @@ public class VersionManager {
     instance = new VersionManager();
     final String fileName = "geodeOldVersionClasspaths.txt";
     instance.findVersions(fileName);
-    System.out
-        .println("VersionManager has loaded the following classpaths:\n" + instance.classPaths);
   }
 
   public static VersionManager getInstance() {

http://git-wip-us.apache.org/repos/asf/geode/blob/f8e7ddd5/geode-wan/src/main/java/org/apache/geode/cache/client/internal/GatewaySenderBatchOp.java
----------------------------------------------------------------------
diff --git a/geode-wan/src/main/java/org/apache/geode/cache/client/internal/GatewaySenderBatchOp.java b/geode-wan/src/main/java/org/apache/geode/cache/client/internal/GatewaySenderBatchOp.java
index b8616a9..d7c721d 100755
--- a/geode-wan/src/main/java/org/apache/geode/cache/client/internal/GatewaySenderBatchOp.java
+++ b/geode-wan/src/main/java/org/apache/geode/cache/client/internal/GatewaySenderBatchOp.java
@@ -224,9 +224,6 @@ public class GatewaySenderBatchOp {
     }
 
     @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {}
-
-    @Override
     protected boolean needsUserId() {
       return false;
     }


Mime
View raw message