This is an automated email from the ASF dual-hosted git repository. boglesby 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 a96edf2 GEODE-5404: Created static default whitelist and modified to call length a96edf2 is described below commit a96edf2e6fc988bd96ce78e6011d080494ec18df Author: Barry Oglesby <bogle...@users.noreply.github.com> AuthorDate: Wed Jul 11 09:51:58 2018 -0700 GEODE-5404: Created static default whitelist and modified to call length --- .../RestrictedMethodInvocationAuthorizer.java | 10 ++- .../cache/tier/sockets/BaseCommandQuery.java | 6 +- .../RestrictedMethodInvocationAuthorizerTest.java | 14 +++ .../QuerySecurityDistinctQueryDistributedTest.java | 99 ++++++++++++++++++++++ .../apache/geode/security/query/data/PdxTrade.java | 61 +++++++++++++ 5 files changed, 185 insertions(+), 5 deletions(-) diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/RestrictedMethodInvocationAuthorizer.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/RestrictedMethodInvocationAuthorizer.java index 254c817..bd28886 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/RestrictedMethodInvocationAuthorizer.java +++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/RestrictedMethodInvocationAuthorizer.java @@ -31,6 +31,8 @@ public class RestrictedMethodInvocationAuthorizer implements MethodInvocationAut public static final String UNAUTHORIZED_STRING = "Unauthorized access to method: "; + protected static final HashMap<String, Set> DEFAULT_WHITELIST = createWhiteList(); + private SecurityService securityService; // List of methods that can be invoked by @@ -39,10 +41,10 @@ public class RestrictedMethodInvocationAuthorizer implements MethodInvocationAut public RestrictedMethodInvocationAuthorizer(SecurityService securityService) { this.securityService = securityService; - whiteListedMethodsToClass = createWhiteList(); + whiteListedMethodsToClass = DEFAULT_WHITELIST; } - private HashMap<String, Set> createWhiteList() { + private static HashMap<String, Set> createWhiteList() { HashMap<String, Set> whiteListMap = new HashMap(); Set<Class> objectCallers = new HashSet(); objectCallers.add(Object.class); @@ -122,6 +124,10 @@ public class RestrictedMethodInvocationAuthorizer implements MethodInvocationAut return whiteListMap; } + protected HashMap<String, Set> getWhiteList() { + return whiteListedMethodsToClass; + } + boolean isWhitelisted(Method method) { String methodName = method.getName(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java index a162e75..7dcffb5 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java @@ -364,7 +364,7 @@ public abstract class BaseCommandQuery extends BaseCommand { } Object[] results = new Object[MAXIMUM_CHUNK_SIZE]; for (int i = 0; i < MAXIMUM_CHUNK_SIZE; i++) { - if ((resultIndex) == selectResults.size()) { + if ((resultIndex) == objs.length) { incompleteArray = true; break; } @@ -425,7 +425,7 @@ public abstract class BaseCommandQuery extends BaseCommand { } if (sendResults) { - writeQueryResponseChunk(results, collectionType, (resultIndex == selectResults.size()), + writeQueryResponseChunk(results, collectionType, (resultIndex == objs.length), servConn); if (logger.isDebugEnabled()) { @@ -435,7 +435,7 @@ public abstract class BaseCommandQuery extends BaseCommand { } // If we have reached the last element of SelectResults then we should // break out of loop here only. - if (resultIndex == selectResults.size()) { + if (resultIndex == objs.length) { break; } } diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/internal/RestrictedMethodInvocationAuthorizerTest.java b/geode-core/src/test/java/org/apache/geode/cache/query/internal/RestrictedMethodInvocationAuthorizerTest.java index 0f5e589..e72cebe 100644 --- a/geode-core/src/test/java/org/apache/geode/cache/query/internal/RestrictedMethodInvocationAuthorizerTest.java +++ b/geode-core/src/test/java/org/apache/geode/cache/query/internal/RestrictedMethodInvocationAuthorizerTest.java @@ -15,6 +15,7 @@ package org.apache.geode.cache.query.internal; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -483,6 +484,19 @@ public class RestrictedMethodInvocationAuthorizerTest { testNumberMethods(AtomicLong.class); } + @Test + public void verifyAuthorizersUseDefaultWhiteList() { + RestrictedMethodInvocationAuthorizer authorizer1 = + new RestrictedMethodInvocationAuthorizer(null); + RestrictedMethodInvocationAuthorizer authorizer2 = + new RestrictedMethodInvocationAuthorizer(null); + assertThat(authorizer1.getWhiteList()).isSameAs(authorizer2.getWhiteList()); + assertThat(authorizer1.getWhiteList()) + .isSameAs(RestrictedMethodInvocationAuthorizer.DEFAULT_WHITELIST); + assertThat(authorizer2.getWhiteList()) + .isSameAs(RestrictedMethodInvocationAuthorizer.DEFAULT_WHITELIST); + } + private void testNumberMethods(Class<?> clazz) throws NoSuchMethodException { Method byteValue = clazz.getMethod("byteValue"); Method doubleValue = clazz.getMethod("doubleValue"); diff --git a/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityDistinctQueryDistributedTest.java b/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityDistinctQueryDistributedTest.java new file mode 100644 index 0000000..643ee3a --- /dev/null +++ b/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityDistinctQueryDistributedTest.java @@ -0,0 +1,99 @@ +/* + * 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.security.query; + +import static org.assertj.core.api.Assertions.assertThat; + +import junitparams.JUnitParamsRunner; +import junitparams.Parameters; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; + +import org.apache.geode.cache.RegionShortcut; +import org.apache.geode.cache.client.ServerOperationException; +import org.apache.geode.cache.query.QueryService; +import org.apache.geode.cache.query.SelectResults; +import org.apache.geode.cache.query.internal.RestrictedMethodInvocationAuthorizer; +import org.apache.geode.security.NotAuthorizedException; +import org.apache.geode.security.query.data.PdxTrade; +import org.apache.geode.test.dunit.VM; +import org.apache.geode.test.junit.categories.DistributedTest; +import org.apache.geode.test.junit.categories.SecurityTest; + +/** + * This test verifies client/server distinct order-by queries with integrated security + */ +@Category({DistributedTest.class, SecurityTest.class}) +@RunWith(JUnitParamsRunner.class) +public class QuerySecurityDistinctQueryDistributedTest extends QuerySecurityBase { + + private static final int NUM_ENTRIES = 1000; + + public RegionShortcut getRegionType() { + return RegionShortcut.PARTITION; + } + + @Test + @Parameters({"99", "100", "499", "500", "999", "1000", "1500"}) + public void verifyDistinctOrderByQueryWithLimits(int limit) { + // Do puts from the client + putIntoRegion(superUserClient, NUM_ENTRIES); + + // Execute query from the client and validate the size of the results + superUserClient.invoke(() -> { + String query = "<trace> select distinct * from /" + regionName + + " where cusip = 'PVTL' order by id asc limit " + limit; + QueryService queryService = getClientCache().getQueryService(); + Object results = queryService.newQuery(query).execute(); + assertThat(results).isInstanceOf(SelectResults.class); + assertThat(((SelectResults) results).size()).isEqualTo(Math.min(limit, NUM_ENTRIES)); + }); + } + + @Test + public void verifyDistinctOrderByQueryOnMethodFails() { + // Do puts from the client + putIntoRegion(superUserClient, 1); + + // Execute query from the client and validate the size of the results + superUserClient.invoke(() -> { + String methodName = "getCusip"; + String query = "<trace> select distinct * from /" + regionName + + " where " + methodName + " = 'PVTL' order by id asc"; + QueryService queryService = getClientCache().getQueryService(); + try { + queryService.newQuery(query).execute(); + } catch (Exception e) { + assertThat(e).isInstanceOf(ServerOperationException.class); + assertThat(e.getCause()).isInstanceOf(NotAuthorizedException.class); + assertThat(e.getMessage()).contains( + RestrictedMethodInvocationAuthorizer.UNAUTHORIZED_STRING + methodName); + } + }); + } + + private void putIntoRegion(VM clientVM, int numEntries) { + keys = new Object[numEntries]; + values = new Object[numEntries]; + for (int i = 0; i < numEntries; i++) { + String key = String.valueOf(i); + Object value = new PdxTrade(key, "PVTL", 100, 30); + keys[i] = key; + values[i] = value; + } + putIntoRegion(clientVM, keys, values, regionName); + } +} diff --git a/geode-core/src/test/java/org/apache/geode/security/query/data/PdxTrade.java b/geode-core/src/test/java/org/apache/geode/security/query/data/PdxTrade.java new file mode 100644 index 0000000..3bf49fc --- /dev/null +++ b/geode-core/src/test/java/org/apache/geode/security/query/data/PdxTrade.java @@ -0,0 +1,61 @@ +/* + * 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.security.query.data; + +import java.io.Serializable; + +import org.apache.geode.pdx.PdxReader; +import org.apache.geode.pdx.PdxSerializable; +import org.apache.geode.pdx.PdxWriter; + +public class PdxTrade implements PdxSerializable, Serializable { + + private String id; + + private String cusip; + + private int shares; + + private int price; + + public PdxTrade() {} + + public String getCusip() { + return this.cusip; + } + + public PdxTrade(String id, String cusip, int shares, int price) { + this.id = id; + this.cusip = cusip; + this.shares = shares; + this.price = price; + } + + @Override + public void toData(PdxWriter writer) { + writer.writeString("id", id); + writer.writeString("cusip", cusip); + writer.writeInt("shares", shares); + writer.writeInt("price", price); + } + + @Override + public void fromData(PdxReader reader) { + id = reader.readString("id"); + cusip = reader.readString("cusip"); + shares = reader.readInt("shares"); + price = reader.readInt("price"); + } +}