Repository: geode Updated Branches: refs/heads/feature/GEODE-3249 2eca73ed8 -> 6ee5771dd
GEODE-3249: internal messages should have credentials Require all client-side message classes to invoke processSecureBytes(). Allow client PR metadata fetch from a background thread with no credentials. Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/6ee5771d Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/6ee5771d Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/6ee5771d Branch: refs/heads/feature/GEODE-3249 Commit: 6ee5771dd1156b3d754662259e30e635702721b8 Parents: 2eca73e Author: Bruce Schuchardt <[email protected]> Authored: Tue Aug 15 12:02:10 2017 -0700 Committer: Bruce Schuchardt <[email protected]> Committed: Tue Aug 15 12:02:10 2017 -0700 ---------------------------------------------------------------------- .../geode/cache/client/internal/AbstractOp.java | 39 +++++++++----------- .../client/internal/CloseConnectionOp.java | 3 -- .../geode/cache/client/internal/CommitOp.java | 3 -- .../client/internal/GetClientPRMetaDataOp.java | 11 ++++++ .../GetClientPartitionAttributesOp.java | 3 -- .../cache/client/internal/GetEventValueOp.java | 3 -- .../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 -- .../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 -- .../cache/tier/sockets/ServerConnection.java | 6 +-- .../client/internal/GatewaySenderBatchOp.java | 3 -- 17 files changed, 33 insertions(+), 64 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/6ee5771d/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 5f44058..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,28 +140,10 @@ public abstract class AbstractOp implements Op { } /** - * 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; - } - - /** - * 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) + * 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 AbstractOp#sendMessage(Connection) - * @see AbstractOp#needsUserId() * @see ServerConnection#updateAndGetSecurityPart() */ protected void processSecureBytes(Connection cnx, Message message) throws Exception { @@ -188,6 +170,21 @@ public abstract class AbstractOp implements Op { } /** + * 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. * http://git-wip-us.apache.org/repos/asf/geode/blob/6ee5771d/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/6ee5771d/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/6ee5771d/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 ec843c3..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 @@ -67,6 +67,17 @@ public class GetClientPRMetaDataOp { getMessage().addStringPart(regionFullPath); } + @Override + protected boolean needsUserId() { + return false; + } + + @Override + protected void sendMessage(Connection cnx) throws Exception { + getMessage().clearMessageHasSecurePartFlag(); + getMessage().send(false); + } + @SuppressWarnings("unchecked") @Override protected Object processResponse(Message msg) throws Exception { http://git-wip-us.apache.org/repos/asf/geode/blob/6ee5771d/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/6ee5771d/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/6ee5771d/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/6ee5771d/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/6ee5771d/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/6ee5771d/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/6ee5771d/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/6ee5771d/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/6ee5771d/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/6ee5771d/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/6ee5771d/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/6ee5771d/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 51e398c..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 @@ -1056,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); @@ -1095,9 +1096,6 @@ public abstract class ServerConnection implements Runnable { } public boolean isInternalMessage(Message message, boolean allowOldInternalMessages) { - if (message.isSecureMode()) { - return false; - } int messageType = message.getMessageType(); boolean isInternalMessage = messageType == MessageType.PING || messageType == MessageType.USER_CREDENTIAL_MESSAGE @@ -1107,6 +1105,7 @@ public abstract class ServerConnection implements Runnable { || 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 @@ -1114,7 +1113,6 @@ public abstract class ServerConnection implements Runnable { // to be performed. if (!isInternalMessage && allowOldInternalMessages) { isInternalMessage = messageType == MessageType.GETCQSTATS_MSG_TYPE - || messageType == MessageType.GET_CLIENT_PR_METADATA || messageType == MessageType.MONITORCQ_MSG_TYPE || messageType == MessageType.REGISTER_DATASERIALIZERS || messageType == MessageType.REGISTER_INSTANTIATORS http://git-wip-us.apache.org/repos/asf/geode/blob/6ee5771d/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; }
