Skip to content

Commit

Permalink
GEODE-6034 Protobuf clients should not access or modify internal regions
Browse files Browse the repository at this point in the history
The Protobuf API now disallows access to internal cache regions
  • Loading branch information
bschuchardt committed Nov 26, 2018
1 parent 3a3697d commit b8d26b1
Show file tree
Hide file tree
Showing 16 changed files with 199 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.apache.geode.experimental.driver;

import static org.apache.geode.internal.Assert.assertTrue;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.io.IOException;
import java.util.Properties;
Expand All @@ -39,7 +40,7 @@ public class AuthenticationTest {
@Rule
public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();

private static final String TEST_USERNAME = "cluster";
private static final String TEST_USERNAME = "";
private static final String TEST_PASSWORD = TEST_USERNAME;
private Locator locator;
private Cache cache;
Expand Down Expand Up @@ -89,4 +90,16 @@ public void driverCanConnectWhenThereAreServers() throws Exception {
.setPassword(TEST_PASSWORD).create();
assertTrue(driver.isConnected());
}

@Test
public void driverWithBadPasswordIsRejected() throws Exception {
CacheServer server = cache.addCacheServer();
server.setPort(0);
server.start();
DriverFactory factory =
new DriverFactory().addLocator("localhost", locatorPort).setUsername(TEST_USERNAME)
.setPassword("my my my");
assertThatThrownBy(() -> factory.create()).isInstanceOf(IOException.class);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more contributor license
* agreements. See the NOTICE file distributed with this work for additional information regarding
* copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the License. You may obtain a
* copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package org.apache.geode.experimental.driver;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;

import java.io.IOException;
import java.util.Collections;
import java.util.Properties;

import org.assertj.core.api.ThrowableAssert;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;

import org.apache.geode.cache.Cache;
import org.apache.geode.cache.CacheFactory;
import org.apache.geode.cache.RegionAttributes;
import org.apache.geode.cache.RegionShortcut;
import org.apache.geode.cache.server.CacheServer;
import org.apache.geode.distributed.ConfigurationProperties;
import org.apache.geode.distributed.Locator;
import org.apache.geode.internal.cache.GemFireCacheImpl;
import org.apache.geode.internal.cache.InternalRegionArguments;
import org.apache.geode.internal.cache.LocalRegion;
import org.apache.geode.security.SimpleTestSecurityManager;
import org.apache.geode.test.junit.categories.ClientServerTest;

@Category({ClientServerTest.class})
public class AuthorizationTest {
private static final String TEST_USERNAME = "";
private static final String TEST_PASSWORD = TEST_USERNAME;
private Locator locator;
private Cache cache;
private Driver driver;
private int locatorPort;


@Before
public void createServer() throws Exception {
System.setProperty("geode.feature-protobuf-protocol", "true");

// Create a cache
CacheFactory cf = new CacheFactory();
cf.set(ConfigurationProperties.MCAST_PORT, "0");
cf.setSecurityManager(new SimpleTestSecurityManager());
cache = cf.create();

// Start a locator
locator = Locator.startLocatorAndDS(0, null, new Properties());
locatorPort = locator.getPort();

// do not start a cache server
}

@After
public void cleanup() {
locator.stop();
cache.close();
}

@Test
public void performOperationsOnInternalRegion() throws Exception {
// we need to use internal APIs to create an "internal" region
GemFireCacheImpl serverCache = (GemFireCacheImpl) cache;
InternalRegionArguments internalRegionArguments = new InternalRegionArguments();
internalRegionArguments.setIsUsedForPartitionedRegionAdmin(true);
RegionAttributes<String, String> attributes =
serverCache.getRegionAttributes(RegionShortcut.REPLICATE.toString());
LocalRegion serverRegion =
(LocalRegion) serverCache.createVMRegion("internalRegion", attributes,
internalRegionArguments);
assertThat(serverRegion.isInternalRegion()).isTrue();

CacheServer server = cache.addCacheServer();
server.setPort(0);
server.start();
Driver driver =
new DriverFactory().addLocator("localhost", locatorPort).setUsername(TEST_USERNAME)
.setPassword(TEST_PASSWORD).create();
Region region = driver.getRegion("internalRegion");
assertThat(region).isNotNull();
assertFailure(() -> region.clear());
assertFailure(() -> region.get("some key"));
assertFailure(() -> region.getAll(Collections.singleton("some key")));
assertFailure(() -> region.keySet());
assertFailure(() -> region.put("some key", "some value"));
assertFailure(() -> region.putAll(Collections.singletonMap("some key", "some value")));
assertFailure(() -> region.putIfAbsent("some key", "some value"));
assertFailure(() -> region.remove("some key"));
assertFailure(() -> region.size());
}

private void assertFailure(final ThrowableAssert.ThrowingCallable callable) {
assertThatExceptionOfType(IOException.class).isThrownBy(callable)
.withMessageContaining(
"Not authorized");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public Object authenticate(Properties credentials) throws AuthenticationFailedEx

@Override
public boolean authorize(Object principal, ResourcePermission permission) {
return principal == authorizedPrincipal;
return principal.equals(authorizedPrincipal);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public Object authenticate(Properties credentials) throws AuthenticationFailedEx
@Override
public boolean authorize(Object principal, ResourcePermission permission) {
// Only allow data operations and only from the expected principal
if (principal != securityPrincipal
if (!principal.equals(securityPrincipal)
|| permission.getResource() != ResourcePermission.Resource.DATA) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ public class ServerMessageExecutionContext extends MessageExecutionContext {
public ServerMessageExecutionContext(InternalCache cache, ClientStatistics statistics,
SecurityService securityService) {
super(statistics, securityService);
this.cache = cache;
this.cache = cache.getCacheForProcessingClientRequests();
Security security =
securityService.isIntegratedSecurity() ? new NotLoggedInSecurity() : new NoSecurity();
this.secureCache = new SecureCacheImpl(cache, security);
this.secureCache = new SecureCacheImpl(this.cache, security);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import org.junit.experimental.categories.Category;

import org.apache.geode.cache.IncompatibleVersionException;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.InternalCacheForClientAccess;
import org.apache.geode.internal.cache.client.protocol.ClientProtocolProcessor;
import org.apache.geode.internal.cache.tier.CachedRegionHelper;
import org.apache.geode.internal.cache.tier.CommunicationMode;
Expand Down Expand Up @@ -87,7 +87,8 @@ private ProtobufServerConnection getServerConnection(Socket socketMock,
when(socketMock.getRemoteSocketAddress()).thenReturn(inetSocketAddressStub);
when(socketMock.getInetAddress()).thenReturn(inetAddressStub);

InternalCache cache = mock(InternalCache.class);
InternalCacheForClientAccess cache = mock(InternalCacheForClientAccess.class);
when(cache.getCacheForProcessingClientRequests()).thenReturn(cache);
CachedRegionHelper cachedRegionHelper = mock(CachedRegionHelper.class);
when(cachedRegionHelper.getCache()).thenReturn(cache);
return new ProtobufServerConnection(socketMock, cache, cachedRegionHelper,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

import org.apache.geode.cache.IncompatibleVersionException;
import org.apache.geode.internal.Assert;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.InternalCacheForClientAccess;
import org.apache.geode.internal.cache.client.protocol.ClientProtocolProcessor;
import org.apache.geode.internal.cache.tier.CachedRegionHelper;
import org.apache.geode.internal.cache.tier.CommunicationMode;
Expand Down Expand Up @@ -126,7 +126,8 @@ private ProtobufServerConnection getServerConnection(Socket socketMock,
when(socketMock.getRemoteSocketAddress()).thenReturn(inetSocketAddressStub);
when(socketMock.getInetAddress()).thenReturn(inetAddressStub);

InternalCache cache = mock(InternalCache.class);
InternalCacheForClientAccess cache = mock(InternalCacheForClientAccess.class);
when(cache.getCacheForProcessingClientRequests()).thenReturn(cache);
CachedRegionHelper cachedRegionHelper = mock(CachedRegionHelper.class);
when(cachedRegionHelper.getCache()).thenReturn(cache);
return new ProtobufServerConnection(socketMock, cache, cachedRegionHelper,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.apache.geode.internal.protocol.protobuf;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
Expand All @@ -25,7 +26,7 @@
import org.junit.Test;
import org.junit.experimental.categories.Category;

import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.InternalCacheForClientAccess;
import org.apache.geode.internal.protocol.TestExecutionContext;
import org.apache.geode.internal.protocol.protobuf.v1.ProtobufStreamProcessor;
import org.apache.geode.test.junit.categories.ClientServerTest;
Expand All @@ -38,7 +39,8 @@ public void receiveMessage() throws Exception {
OutputStream outputStream = new ByteArrayOutputStream(2);

ProtobufStreamProcessor protobufStreamProcessor = new ProtobufStreamProcessor();
InternalCache mockInternalCache = mock(InternalCache.class);
InternalCacheForClientAccess mockInternalCache = mock(InternalCacheForClientAccess.class);
when(mockInternalCache.getCacheForProcessingClientRequests()).thenReturn(mockInternalCache);
protobufStreamProcessor.receiveMessage(inputStream, outputStream,
TestExecutionContext.getNoAuthCacheExecutionContext(mockInternalCache));
}
Expand Down
Loading

0 comments on commit b8d26b1

Please sign in to comment.