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 2017/08/15 23:11:00 UTC
[49/50] [abbrv] geode git commit: GEODE-3249: internal messages
should have credentials
GEODE-3249: internal messages should have credentials
added a system property to allow old clients to not send credentials and
a unit test of this change
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/e0fbb3cc
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/e0fbb3cc
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/e0fbb3cc
Branch: refs/heads/feature/GEODE-3249
Commit: e0fbb3cce45528362c1df67bf191ddc4846a9a7f
Parents: b9f952f
Author: Bruce Schuchardt <bs...@pivotal.io>
Authored: Wed Aug 9 15:40:23 2017 -0700
Committer: Bruce Schuchardt <bs...@pivotal.io>
Committed: Tue Aug 15 15:34:00 2017 -0700
----------------------------------------------------------------------
.../cache/tier/sockets/ServerConnection.java | 92 ++++++++++++--------
.../ClientAuthenticationPart2DUnitTest.java | 26 ++++++
2 files changed, 83 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/e0fbb3cc/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 91712e0..d25722b 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);
@@ -1068,7 +1080,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,37 +1094,47 @@ public abstract class ServerConnection implements Runnable {
return null;
}
- private boolean isInternalMessage() {
- return this.requestMsg.messageType == MessageType.PING
- || this.requestMsg.messageType == MessageType.USER_CREDENTIAL_MESSAGE
- || this.requestMsg.messageType == MessageType.REQUEST_EVENT_VALUE
- || this.requestMsg.messageType == MessageType.MAKE_PRIMARY
- || this.requestMsg.messageType == MessageType.REMOVE_USER_AUTH
- || this.requestMsg.messageType == MessageType.CLIENT_READY
- || this.requestMsg.messageType == MessageType.SIZE
- || this.requestMsg.messageType == MessageType.TX_FAILOVER
- || this.requestMsg.messageType == MessageType.TX_SYNCHRONIZATION
- || this.requestMsg.messageType == MessageType.COMMIT
- || this.requestMsg.messageType == MessageType.ROLLBACK
- || this.requestMsg.messageType == MessageType.CLOSE_CONNECTION
- || this.requestMsg.messageType == MessageType.INVALID
- || this.requestMsg.messageType == MessageType.PERIODIC_ACK
- || this.requestMsg.messageType == MessageType.GET_CLIENT_PARTITION_ATTRIBUTES;
-
- // || this.requestMsg.messageType == MessageType.GETCQSTATS_MSG_TYPE
- // || this.requestMsg.messageType == MessageType.GET_CLIENT_PR_METADATA
- // || this.requestMsg.messageType == MessageType.MONITORCQ_MSG_TYPE
- // || this.requestMsg.messageType == MessageType.REGISTER_DATASERIALIZERS
- // || this.requestMsg.messageType == MessageType.REGISTER_INSTANTIATORS
- // || 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.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
+ 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
+ || 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_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.GET_CLIENT_PR_METADATA
+ || 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() {
@@ -1719,8 +1742,7 @@ public abstract class ServerConnection implements Runnable {
(HandShake) this.handshake, this.connectionId);
} else {
throw new AuthenticationRequiredException(
- LocalizedStrings.HandShake_NO_SECURITY_CREDENTIALS_ARE_PROVIDED.toLocalizedString()
- + "; for message " + this.requestMsg);
+ LocalizedStrings.HandShake_NO_SECURITY_CREDENTIALS_ARE_PROVIDED.toLocalizedString());
}
return uniqueId;
}
http://git-wip-us.apache.org/repos/asf/geode/blob/e0fbb3cc/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 b9f6223..f1d4f23 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,12 +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);
}