Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 66E93200CFE for ; Fri, 8 Sep 2017 20:14:05 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 659251610AA; Fri, 8 Sep 2017 18:14:05 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id B2BD6160F79 for ; Fri, 8 Sep 2017 20:14:03 +0200 (CEST) Received: (qmail 62123 invoked by uid 500); 8 Sep 2017 18:14:02 -0000 Mailing-List: contact commits-help@geode.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@geode.apache.org Delivered-To: mailing list commits@geode.apache.org Received: (qmail 62114 invoked by uid 99); 8 Sep 2017 18:14:02 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 08 Sep 2017 18:14:02 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 0AF4B81830; Fri, 8 Sep 2017 18:13:59 +0000 (UTC) Date: Fri, 08 Sep 2017 18:13:59 +0000 To: "commits@geode.apache.org" Subject: [geode] branch develop updated: GEODE-3249 Validate internal client/server messages MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <150489443915.23398.988122346044669350@gitbox.apache.org> From: bschuchardt@apache.org Reply-To: "commits@geode.apache.org" X-Git-Host: gitbox.apache.org X-Git-Repo: geode X-Git-Refname: refs/heads/develop X-Git-Reftype: branch X-Git-Oldrev: 97f2921ce4565bfc092dffd97e924c952623c9e2 X-Git-Newrev: abbb359fe59ea3e74462fe48890918108a0edda3 X-Git-Rev: abbb359fe59ea3e74462fe48890918108a0edda3 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated archived-at: Fri, 08 Sep 2017 18:14:05 -0000 This is an automated email from the ASF dual-hosted git repository. bschuchardt pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/geode.git The following commit(s) were added to refs/heads/develop by this push: new abbb359 GEODE-3249 Validate internal client/server messages abbb359 is described below commit abbb359fe59ea3e74462fe48890918108a0edda3 Author: Bruce Schuchardt AuthorDate: Fri Sep 8 09:35:18 2017 -0700 GEODE-3249 Validate internal client/server messages This change leaves the security hole in place but allows you to plug it by setting the system property geode.disallow-internal-messages-without-credentials=true Clients must be upgraded to the release containing this change if you set this system property to true and client/server authentication is enabled. Otherwise client messages to register PDX types or Instantiators will be rejected by the servers. New tests have been added to perform backward-compatibility testing with the old security implementation and the internal message command classes have been modified to perform validation of credentials if the system property is set to true. --- .../cache/tier/sockets/ServerConnection.java | 17 ++++---- .../cache/tier/sockets/command/AddPdxEnum.java | 5 +++ .../cache/tier/sockets/command/AddPdxType.java | 5 +++ .../tier/sockets/command/GetFunctionAttribute.java | 5 +++ .../cache/tier/sockets/command/GetPDXEnumById.java | 5 +++ .../tier/sockets/command/GetPDXIdForEnum.java | 4 ++ .../tier/sockets/command/GetPDXIdForType.java | 5 +++ .../cache/tier/sockets/command/GetPDXTypeById.java | 5 +++ .../cache/tier/sockets/command/GetPdxEnums70.java | 4 ++ .../cache/tier/sockets/command/GetPdxTypes70.java | 4 ++ .../sockets/command/RegisterDataSerializers.java | 6 +++ .../sockets/command/RegisterInstantiators.java | 6 +++ .../security/ClientAuthenticationDUnitTest.java | 24 +++++++++++ .../ClientAuthenticationPart2DUnitTest.java | 24 +++++++++++ .../security/ClientAuthenticationTestCase.java | 45 ++++++++++++++++----- .../security/ClientAuthorizationDUnitTest.java | 47 ++++++++++++++++------ .../security/ClientAuthorizationTestCase.java | 29 ++++++++----- .../apache/geode/security/SecurityTestUtils.java | 4 ++ .../test/dunit/standalone/VersionManager.java | 4 +- .../cache/tier/sockets/command/MonitorCQ.java | 4 ++ ...st.java => ClientAuthorizationCQDUnitTest.java} | 26 +++++++++++- 21 files changed, 234 insertions(+), 44 deletions(-) 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 b243d8e..160e05b 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 @@ -85,16 +85,15 @@ 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"; + public static final String DISALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS_NAME = + "geode.disallow-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 + * When true requires some formerly credential-less messages to carry credentials. See GEODE-3249 + * and ServerConnection.isInternalMessage() */ - private static final boolean ALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS = - Boolean.getBoolean(ALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS_NAME); + public static boolean allowInternalMessagesWithoutCredentials = + !(Boolean.getBoolean(DISALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS_NAME)); private Map commands; @@ -774,7 +773,7 @@ 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(this.requestMsg, ALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS) + && !isInternalMessage(this.requestMsg, allowInternalMessagesWithoutCredentials) && !this.communicationMode.isWAN()) { long uniqueId = getUniqueId(); Subject subject = this.clientUserAuths.getSubject(uniqueId); @@ -1078,7 +1077,7 @@ public abstract class ServerConnection implements Runnable { if (AcceptorImpl.isAuthenticationRequired() && this.handshake.getVersion().compareTo(Version.GFE_65) >= 0 && !this.communicationMode.isWAN() && !this.requestMsg.getAndResetIsMetaRegion() - && !isInternalMessage(this.requestMsg, ALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS)) { + && !isInternalMessage(this.requestMsg, allowInternalMessagesWithoutCredentials)) { setSecurityPart(); return this.securePart; } else { diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxEnum.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxEnum.java index 5a4a07b..194c7c4 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxEnum.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/AddPdxEnum.java @@ -49,6 +49,11 @@ public class AddPdxEnum extends BaseCommand { serverConnection.getName(), clientMessage.getNumberOfParts(), serverConnection.getSocketString()); } + + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + int noOfParts = clientMessage.getNumberOfParts(); EnumInfo enumInfo = (EnumInfo) clientMessage.getPart(0).getObject(); 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 041e12f..9cf3105 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 @@ -50,6 +50,11 @@ public class AddPdxType extends BaseCommand { serverConnection.getName(), clientMessage.getNumberOfParts(), serverConnection.getSocketString()); } + + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + int noOfParts = clientMessage.getNumberOfParts(); PdxType type = (PdxType) clientMessage.getPart(0).getObject(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetFunctionAttribute.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetFunctionAttribute.java index 76cc4a5..bd1bccc 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetFunctionAttribute.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetFunctionAttribute.java @@ -38,6 +38,11 @@ public class GetFunctionAttribute extends BaseCommand { public void cmdExecute(final Message clientMessage, final ServerConnection serverConnection, final SecurityService securityService, long start) throws IOException { serverConnection.setAsTrue(REQUIRES_RESPONSE); + + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + String functionId = clientMessage.getPart(0).getString(); if (functionId == null) { String message = diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXEnumById.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXEnumById.java index 5e59640..8084cb7 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXEnumById.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXEnumById.java @@ -46,6 +46,11 @@ public class GetPDXEnumById extends BaseCommand { serverConnection.getName(), clientMessage.getNumberOfParts(), serverConnection.getSocketString()); } + + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + int enumId = clientMessage.getPart(0).getInt(); EnumInfo result; diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForEnum.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForEnum.java index b0ebaf2..9bd5ba9 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForEnum.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXIdForEnum.java @@ -47,6 +47,10 @@ public class GetPDXIdForEnum extends BaseCommand { serverConnection.getSocketString()); } + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + EnumInfo enumInfo = (EnumInfo) clientMessage.getPart(0).getObject(); int enumId; 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 f2172ef..167fa51 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 @@ -47,6 +47,11 @@ public class GetPDXIdForType extends BaseCommand { serverConnection.getName(), clientMessage.getNumberOfParts(), serverConnection.getSocketString()); } + + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + int noOfParts = clientMessage.getNumberOfParts(); PdxType type = (PdxType) clientMessage.getPart(0).getObject(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXTypeById.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXTypeById.java index e46445b..71b1145 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXTypeById.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPDXTypeById.java @@ -46,6 +46,11 @@ public class GetPDXTypeById extends BaseCommand { serverConnection.getName(), clientMessage.getNumberOfParts(), serverConnection.getSocketString()); } + + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + int pdxId = clientMessage.getPart(0).getInt(); PdxType type; diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPdxEnums70.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPdxEnums70.java index 3fe9750..ced58e3 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPdxEnums70.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPdxEnums70.java @@ -46,6 +46,10 @@ public class GetPdxEnums70 extends BaseCommand { serverConnection.getSocketString()); } + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + Map enums; try { InternalCache cache = serverConnection.getCache(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPdxTypes70.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPdxTypes70.java index e64683f..1cbb717 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPdxTypes70.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetPdxTypes70.java @@ -46,6 +46,10 @@ public class GetPdxTypes70 extends BaseCommand { serverConnection.getSocketString()); } + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + Map types; try { InternalCache cache = serverConnection.getCache(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterDataSerializers.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterDataSerializers.java index eef5195..3460376 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterDataSerializers.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterDataSerializers.java @@ -40,11 +40,17 @@ public class RegisterDataSerializers extends BaseCommand { public void cmdExecute(final Message clientMessage, final ServerConnection serverConnection, final SecurityService securityService, long start) throws IOException, ClassNotFoundException { + serverConnection.setAsTrue(REQUIRES_RESPONSE); if (logger.isDebugEnabled()) { logger.debug("{}: Received register dataserializer request ({} parts) from {}", serverConnection.getName(), clientMessage.getNumberOfParts(), serverConnection.getSocketString()); } + + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + int noOfParts = clientMessage.getNumberOfParts(); // 2 parts per instantiator and one eventId part diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInstantiators.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInstantiators.java index a402cb3..92e740a 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInstantiators.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInstantiators.java @@ -52,11 +52,17 @@ public class RegisterInstantiators extends BaseCommand { public void cmdExecute(final Message clientMessage, final ServerConnection serverConnection, final SecurityService securityService, long start) throws IOException, ClassNotFoundException { + serverConnection.setAsTrue(REQUIRES_RESPONSE); if (logger.isDebugEnabled()) { logger.debug("{}: Received register instantiator request ({} parts) from {}", serverConnection.getName(), clientMessage.getNumberOfParts(), serverConnection.getSocketString()); } + + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + int noOfParts = clientMessage.getNumberOfParts(); // Assert parts Assert.assertTrue((noOfParts - 1) % 3 == 0); diff --git a/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationDUnitTest.java b/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationDUnitTest.java index ca7b2b6..7a8e322 100644 --- a/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationDUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/security/ClientAuthenticationDUnitTest.java @@ -14,13 +14,20 @@ */ package org.apache.geode.security; +import java.util.Collection; +import java.util.List; + +import org.apache.geode.test.dunit.standalone.VersionManager; import org.apache.geode.test.junit.categories.DistributedTest; import org.apache.geode.test.junit.categories.FlakyTest; import org.apache.geode.test.junit.categories.SecurityTest; +import org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory; import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; /** * Test for authentication from client to server. This tests for both valid and invalid @@ -30,7 +37,24 @@ import org.junit.experimental.categories.Category; * @since GemFire 5.5 */ @Category({DistributedTest.class, SecurityTest.class}) +@RunWith(Parameterized.class) +@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class) public class ClientAuthenticationDUnitTest extends ClientAuthenticationTestCase { + @Parameterized.Parameters + public static Collection data() { + List result = VersionManager.getInstance().getVersions(); + if (result.size() < 1) { + throw new RuntimeException("No older versions of Geode were found to test against"); + } else { + System.out.println("running against these versions: " + result); + } + return result; + } + + public ClientAuthenticationDUnitTest(String version) { + super(); + clientVersion = version; + } @Test public void testValidCredentials() throws Exception { 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 f8ebe05..db2ba95 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 @@ -16,23 +16,47 @@ package org.apache.geode.security; import static org.mockito.Mockito.*; +import java.util.Collection; +import java.util.List; + import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; 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.dunit.standalone.VersionManager; import org.apache.geode.test.junit.categories.DistributedTest; import org.apache.geode.test.junit.categories.SecurityTest; +import org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory; /** * this class contains test methods that used to be in its superclass but that test started taking * too long and caused dunit runs to hang */ @Category({DistributedTest.class, SecurityTest.class}) +@RunWith(Parameterized.class) +@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class) public class ClientAuthenticationPart2DUnitTest extends ClientAuthenticationTestCase { + @Parameterized.Parameters + public static Collection data() { + List result = VersionManager.getInstance().getVersions(); + if (result.size() < 1) { + throw new RuntimeException("No older versions of Geode were found to test against"); + } else { + System.out.println("running against these versions: " + result); + } + return result; + } + + public ClientAuthenticationPart2DUnitTest(String version) { + super(); + clientVersion = version; + } @Test public void testNoCredentialsForMultipleUsers() throws Exception { 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 0ecd72f..417bb50 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 @@ -31,20 +31,27 @@ import java.util.Properties; import javax.net.ssl.SSLException; import javax.net.ssl.SSLHandshakeException; +import org.junit.Assert; +import org.junit.Rule; + 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.ServerOperationException; 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.internal.cache.tier.sockets.ServerConnection; import org.apache.geode.security.generator.CredentialGenerator; import org.apache.geode.security.generator.DummyCredentialGenerator; import org.apache.geode.test.dunit.Host; import org.apache.geode.test.dunit.VM; import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase; +import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties; +import org.apache.geode.test.dunit.standalone.VersionManager; public abstract class ClientAuthenticationTestCase extends JUnit4DistributedTestCase { @@ -92,15 +99,22 @@ public abstract class ClientAuthenticationTestCase extends JUnit4DistributedTest } } - + public String clientVersion = VersionManager.CURRENT_VERSION; @Override public final void postSetUp() throws Exception { final Host host = Host.getHost(0); server1 = host.getVM(0); server2 = host.getVM(1); - client1 = host.getVM(2); - client2 = host.getVM(3); + server1.invoke(() -> ServerConnection.allowInternalMessagesWithoutCredentials = false); + server2.invoke(() -> ServerConnection.allowInternalMessagesWithoutCredentials = false); + if (VersionManager.isCurrentVersion(clientVersion)) { + client1 = host.getVM(2); + client2 = host.getVM(3); + } else { + client1 = host.getVM(clientVersion, 2); + client2 = host.getVM(clientVersion, 3); + } addIgnoredException("Connection refused: connect"); @@ -110,6 +124,14 @@ public abstract class ClientAuthenticationTestCase extends JUnit4DistributedTest client2.invoke(() -> registerExpectedExceptions(clientIgnoredExceptions)); } + @Override + public void postTearDown() throws Exception { + super.postTearDown(); + server1.invoke(() -> ServerConnection.allowInternalMessagesWithoutCredentials = true); + server2.invoke(() -> ServerConnection.allowInternalMessagesWithoutCredentials = true); + } + + protected void doTestValidCredentials(final boolean multiUser) throws Exception { CredentialGenerator gen = new DummyCredentialGenerator(); Properties extraProps = gen.getSystemProperties(); @@ -246,21 +268,25 @@ public abstract class ClientAuthenticationTestCase extends JUnit4DistributedTest client2.invoke(() -> doPuts(2, OTHER_EXCEPTION)); } else { - client2.invoke( + client1.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", () -> { + client1.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 (ServerOperationException e) { + Assert.assertEquals(AuthenticationRequiredException.class, e.getCause().getClass()); } catch (UnsupportedOperationException e) { - // "UnsupportedOperationException: Use Pool APIs for doing operations when - // multiuser-secure-mode-enabled is set to true." + // expected } }); + client2.invoke( + () -> createCacheClient(null, null, null, port1, port2, 0, multiUser, AUTHREQ_EXCEPTION)); + // Try to register a DataSerializer with the server client2.invoke("register a data serializer", () -> { EventID eventId = InternalDataSerializer.generateEventId(); @@ -269,9 +295,10 @@ public abstract class ClientAuthenticationTestCase extends JUnit4DistributedTest RegisterDataSerializersOp.execute((ExecutablePool) pool, new DataSerializer[] {new MyDataSerializer()}, eventId); throw new Error("operation should have been rejected"); + } catch (ServerOperationException e) { + Assert.assertEquals(AuthenticationRequiredException.class, e.getCause().getClass()); } catch (UnsupportedOperationException e) { - // "UnsupportedOperationException: Use Pool APIs for doing operations when - // multiuser-secure-mode-enabled is set to true." + // expected } }); } diff --git a/geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationDUnitTest.java b/geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationDUnitTest.java index 09aedbe..e42f889 100644 --- a/geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationDUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/security/ClientAuthorizationDUnitTest.java @@ -21,6 +21,7 @@ import static org.apache.geode.test.dunit.IgnoredException.*; import static org.apache.geode.test.dunit.LogWriterUtils.*; import java.util.ArrayList; +import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.Properties; @@ -28,16 +29,22 @@ import java.util.Properties; import org.apache.geode.internal.AvailablePortHelper; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.apache.geode.cache.operations.OperationContext.OperationCode; +import org.apache.geode.internal.cache.tier.sockets.ServerConnection; import org.apache.geode.security.generator.AuthzCredentialGenerator; import org.apache.geode.security.generator.CredentialGenerator; import org.apache.geode.security.generator.DummyCredentialGenerator; import org.apache.geode.security.generator.XmlAuthzCredentialGenerator; import org.apache.geode.security.templates.UserPasswordAuthInit; +import org.apache.geode.test.dunit.Host; import org.apache.geode.test.dunit.VM; +import org.apache.geode.test.dunit.standalone.VersionManager; import org.apache.geode.test.junit.categories.DistributedTest; import org.apache.geode.test.junit.categories.SecurityTest; +import org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory; /** * Tests for authorization from client to server. This tests for authorization of all operations @@ -47,7 +54,25 @@ import org.apache.geode.test.junit.categories.SecurityTest; * @since GemFire 5.5 */ @Category({DistributedTest.class, SecurityTest.class}) +@RunWith(Parameterized.class) +@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class) public class ClientAuthorizationDUnitTest extends ClientAuthorizationTestCase { + @Parameterized.Parameters + public static Collection data() { + List result = VersionManager.getInstance().getVersions(); + if (result.size() < 1) { + throw new RuntimeException("No older versions of Geode were found to test against"); + } else { + System.out.println("running against these versions: " + result); + } + return result; + } + + public ClientAuthorizationDUnitTest(String version) { + super(); + clientVersion = version; + } + @Override public final void preTearDownClientAuthorizationTestBase() throws Exception { @@ -112,6 +137,13 @@ public class ClientAuthorizationDUnitTest extends ClientAuthorizationTestCase { String authInit = cGen.getAuthInit(); String accessor = gen.getAuthorizationCallback(); + // this test registers a PDX type and needs the servers to accept it without + // credentials in old clients + if (clientVersion.compareTo("130") < 0 && !VersionManager.isCurrentVersion(clientVersion)) { + server1.invoke(() -> ServerConnection.allowInternalMessagesWithoutCredentials = true); + server2.invoke(() -> ServerConnection.allowInternalMessagesWithoutCredentials = true); + } + getLogWriter().info("testPutAllWithSecurity: Using authinit: " + authInit); getLogWriter().info("testPutAllWithSecurity: Using authenticator: " + authenticator); getLogWriter().info("testPutAllWithSecurity: Using accessor: " + accessor); @@ -518,19 +550,7 @@ public class ClientAuthorizationDUnitTest extends ClientAuthorizationTestCase { new OperationWithAction(OperationCode.DESTROY, 3, OpFlags.USE_NEWVAL | OpFlags.CHECK_NOTAUTHZ, 4), new OperationWithAction(OperationCode.DESTROY), - new OperationWithAction(OperationCode.GET, 2, OpFlags.USE_OLDCONN | OpFlags.CHECK_FAIL, 4), // bruce: - // added - // check_nokey - // because - // we - // now - // bring - // tombstones - // to - // the - // client - // in - // 8.0 + new OperationWithAction(OperationCode.GET, 2, OpFlags.USE_OLDCONN | OpFlags.CHECK_FAIL, 4), // Repopulate the region new OperationWithAction(OperationCode.PUT, 1, OpFlags.USE_NEWVAL, 4), @@ -582,6 +602,7 @@ public class ClientAuthorizationDUnitTest extends ClientAuthorizationTestCase { OpFlags.USE_GET_ENTRY_IN_TX | OpFlags.CHECK_FAIL, 4), OperationWithAction.OPBLOCK_END}; + } private Properties getUserPassword(final String userName) { 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 a4fd365..a41a720 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 @@ -55,14 +55,17 @@ import org.apache.geode.internal.AvailablePort.*; import org.apache.geode.internal.AvailablePortHelper; import org.apache.geode.internal.cache.AbstractRegionEntry; import org.apache.geode.internal.cache.LocalRegion; +import org.apache.geode.internal.cache.tier.sockets.ServerConnection; import org.apache.geode.security.generator.AuthzCredentialGenerator; import org.apache.geode.security.generator.AuthzCredentialGenerator.ClassCode; import org.apache.geode.security.generator.CredentialGenerator; import org.apache.geode.security.generator.DummyCredentialGenerator; import org.apache.geode.security.generator.XmlAuthzCredentialGenerator; +import org.apache.geode.test.dunit.Host; import org.apache.geode.test.dunit.VM; import org.apache.geode.test.dunit.WaitCriterion; import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase; +import org.apache.geode.test.dunit.standalone.VersionManager; /** * Base class for tests for authorization from client to server. It contains utility functions for @@ -79,6 +82,8 @@ public abstract class ClientAuthorizationTestCase extends JUnit4DistributedTestC protected static VM client1 = null; protected static VM client2 = null; + public String clientVersion = VersionManager.CURRENT_VERSION; + protected static final String regionName = REGION_NAME; // TODO: remove protected static final String SUBREGION_NAME = "AuthSubregion"; @@ -103,10 +108,18 @@ public abstract class ClientAuthorizationTestCase extends JUnit4DistributedTestC } private void setUpClientAuthorizationTestBase() throws Exception { - server1 = getHost(0).getVM(0); - server2 = getHost(0).getVM(1); - client1 = getHost(0).getVM(2); - client2 = getHost(0).getVM(3); + Host host = getHost(0); + server1 = host.getVM(0); + server2 = host.getVM(1); + server1.invoke(() -> ServerConnection.allowInternalMessagesWithoutCredentials = false); + server2.invoke(() -> ServerConnection.allowInternalMessagesWithoutCredentials = false); + if (VersionManager.isCurrentVersion(clientVersion)) { + client1 = host.getVM(2); + client2 = host.getVM(3); + } else { + client1 = host.getVM(clientVersion, 2); + client2 = host.getVM(clientVersion, 3); + } setUpIgnoredExceptions(); } @@ -155,10 +168,10 @@ public abstract class ClientAuthorizationTestCase extends JUnit4DistributedTestC public final void postTearDown() throws Exception {} private final void tearDownClientAuthorizationTestBase() throws Exception { - // close the clients first + server1.invoke(() -> ServerConnection.allowInternalMessagesWithoutCredentials = true); + server2.invoke(() -> ServerConnection.allowInternalMessagesWithoutCredentials = true); client1.invoke(() -> closeCache()); client2.invoke(() -> closeCache()); - // then close the servers server1.invoke(() -> closeCache()); server2.invoke(() -> closeCache()); } @@ -293,10 +306,6 @@ public abstract class ClientAuthorizationTestCase extends JUnit4DistributedTestC boolean exceptionOccurred = false; boolean breakLoop = false; - if (op.isGet() || op.isContainsKey() || op.isKeySet() || op.isQuery() || op.isExecuteCQ()) { - Thread.sleep(PAUSE); - } - for (int indexIndex = 0; indexIndex < numOps; ++indexIndex) { if (breakLoop) { break; 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 e69f36d..828210c 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 @@ -991,6 +991,8 @@ public class SecurityTestUtils { fail("Expected a NotAuthorizedException while executing sizeOnServer"); } assertEquals(size, sizeOnServer); + } catch (NoSuchMethodError ex) { + // expected with backward-compatibility tests } catch (Exception ex) { fail("Got unexpected exception when executing sizeOnServer", ex); } @@ -1004,6 +1006,8 @@ public class SecurityTestUtils { fail("Expected a NotAuthorizedException while executing isEmptyOnServer"); } assertEquals(isEmpty, isEmptyOnServer); + } catch (NoSuchMethodError ex) { + // expected with old version clients } catch (Exception ex) { fail("Got unexpected exception when executing isEmptyOnServer", ex); } 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 9f4c357..6b934a3 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 @@ -49,8 +49,8 @@ public class VersionManager { final String installLocations = "geodeOldVersionInstalls.txt"; instance.findVersions(fileName); instance.findInstalls(installLocations); - System.out - .println("VersionManager has loaded the following classpaths:\n" + instance.classPaths); + // System.out + // .println("VersionManager has loaded the following classpaths:\n" + instance.classPaths); } public static VersionManager getInstance() { diff --git a/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/MonitorCQ.java b/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/MonitorCQ.java index f5e31df..e8df2e2 100644 --- a/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/MonitorCQ.java +++ b/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/MonitorCQ.java @@ -45,6 +45,10 @@ public class MonitorCQ extends BaseCQCommand { serverConnection.setAsTrue(REQUIRES_RESPONSE); serverConnection.setAsTrue(REQUIRES_CHUNKED_RESPONSE); + if (!ServerConnection.allowInternalMessagesWithoutCredentials) { + serverConnection.getAuthzRequest(); + } + int op = clientMessage.getPart(0).getInt(); if (op < 1) { diff --git a/geode-cq/src/test/java/org/apache/geode/security/ClientAuthorizationTwoDUnitTest.java b/geode-cq/src/test/java/org/apache/geode/security/ClientAuthorizationCQDUnitTest.java similarity index 90% rename from geode-cq/src/test/java/org/apache/geode/security/ClientAuthorizationTwoDUnitTest.java rename to geode-cq/src/test/java/org/apache/geode/security/ClientAuthorizationCQDUnitTest.java index f5f686c..83358ea 100644 --- a/geode-cq/src/test/java/org/apache/geode/security/ClientAuthorizationTwoDUnitTest.java +++ b/geode-cq/src/test/java/org/apache/geode/security/ClientAuthorizationCQDUnitTest.java @@ -17,12 +17,19 @@ package org.apache.geode.security; import static org.apache.geode.security.SecurityTestUtils.*; import static org.apache.geode.test.dunit.IgnoredException.*; +import java.util.Collection; +import java.util.List; + import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.apache.geode.cache.operations.OperationContext.OperationCode; +import org.apache.geode.test.dunit.standalone.VersionManager; import org.apache.geode.test.junit.categories.DistributedTest; import org.apache.geode.test.junit.categories.SecurityTest; +import org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory; /** * Tests for authorization from client to server. This tests for authorization of all operations @@ -35,7 +42,24 @@ import org.apache.geode.test.junit.categories.SecurityTest; * @since GemFire 5.5 */ @Category({DistributedTest.class, SecurityTest.class}) -public class ClientAuthorizationTwoDUnitTest extends ClientAuthorizationTestCase { +@RunWith(Parameterized.class) +@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class) +public class ClientAuthorizationCQDUnitTest extends ClientAuthorizationTestCase { + @Parameterized.Parameters + public static Collection data() { + List result = VersionManager.getInstance().getVersions(); + if (result.size() < 1) { + throw new RuntimeException("No older versions of Geode were found to test against"); + } else { + System.out.println("running against these versions: " + result); + } + return result; + } + + public ClientAuthorizationCQDUnitTest(String version) { + super(); + clientVersion = version; + } @Override public final void postSetUpClientAuthorizationTestBase() throws Exception { -- To stop receiving notification emails like this one, please contact ['"commits@geode.apache.org" '].