From 0db32da20fd02b4d7cd49e28860e15625b8579cc Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang <92374539+JinwooHwang@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:35:24 -0400 Subject: [PATCH 1/9] [GEODE-10619] Remediation of CVE-2026-59296: Micrometer 1.15.12 to 1.16.7 (#8044) --- boms/geode-all-bom/src/test/resources/expected-pom.xml | 2 +- .../geode/gradle/plugins/DependencyConstraints.groovy | 2 +- .../src/integrationTest/resources/assembly_content.txt | 7 ++++--- .../src/integrationTest/resources/expected_jars.txt | 1 + .../resources/gfsh_dependency_classpath.txt | 7 ++++--- .../apache/geode/metrics/internal/NoopMeterRegistry.java | 5 +---- .../src/integrationTest/resources/dependency_classpath.txt | 7 ++++--- 7 files changed, 16 insertions(+), 15 deletions(-) diff --git a/boms/geode-all-bom/src/test/resources/expected-pom.xml b/boms/geode-all-bom/src/test/resources/expected-pom.xml index ca41c043f528..d40e19ff4966 100644 --- a/boms/geode-all-bom/src/test/resources/expected-pom.xml +++ b/boms/geode-all-bom/src/test/resources/expected-pom.xml @@ -195,7 +195,7 @@ io.micrometer micrometer-core - 1.15.12 + 1.16.7 io.swagger.core.v3 diff --git a/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy b/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy index 29e0d699c211..72ae49070199 100644 --- a/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy +++ b/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy @@ -48,7 +48,7 @@ class DependencyConstraints { deps.put("jgroups.version", "3.6.20.Final") deps.put("log4j.version", "2.25.5") deps.put("log4j-slf4j2-impl.version", "2.23.1") - deps.put("micrometer.version", "1.15.12") + deps.put("micrometer.version", "1.16.7") deps.put("shiro.version", "3.0.0") // GEODE-10583: Pin Bouncy Castle (transitive via shiro-crypto-hash) to a fixed version deps.put("bouncycastle.version", "1.85") diff --git a/geode-assembly/src/integrationTest/resources/assembly_content.txt b/geode-assembly/src/integrationTest/resources/assembly_content.txt index 862f8c53a3af..eb63cebdf6a6 100644 --- a/geode-assembly/src/integrationTest/resources/assembly_content.txt +++ b/geode-assembly/src/integrationTest/resources/assembly_content.txt @@ -1013,6 +1013,7 @@ lib/jna-5.11.0.jar lib/jna-platform-5.11.0.jar lib/joda-time-2.12.7.jar lib/jopt-simple-5.0.4.jar +lib/jspecify-1.0.1.jar lib/jul-to-slf4j-2.0.17.jar lib/log4j-api-2.25.5.jar lib/log4j-core-2.25.5.jar @@ -1024,9 +1025,9 @@ lib/lucene-analysis-phonetic-9.12.3.jar lib/lucene-core-9.12.3.jar lib/lucene-queries-9.12.3.jar lib/lucene-queryparser-9.12.3.jar -lib/micrometer-commons-1.15.12.jar -lib/micrometer-core-1.15.12.jar -lib/micrometer-observation-1.15.12.jar +lib/micrometer-commons-1.16.7.jar +lib/micrometer-core-1.16.7.jar +lib/micrometer-observation-1.16.7.jar lib/mx4j-3.0.2.jar lib/mx4j-remote-3.0.2.jar lib/mx4j-tools-3.0.1.jar diff --git a/geode-assembly/src/integrationTest/resources/expected_jars.txt b/geode-assembly/src/integrationTest/resources/expected_jars.txt index facbde0d7f6b..37c04b61a0b8 100644 --- a/geode-assembly/src/integrationTest/resources/expected_jars.txt +++ b/geode-assembly/src/integrationTest/resources/expected_jars.txt @@ -79,6 +79,7 @@ joda-time jopt-simple json-path json-smart +jspecify jul-to-slf4j lang-tag log4j-api diff --git a/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt b/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt index 68a5913622f0..40e811d4495c 100644 --- a/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt +++ b/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt @@ -76,7 +76,7 @@ commons-digester-2.1.jar commons-io-2.19.0.jar commons-logging-1.3.5.jar classgraph-4.8.147.jar -micrometer-core-1.15.12.jar +micrometer-core-1.16.7.jar HdrHistogram-2.2.2.jar fastutil-8.5.8.jar jakarta.resource-api-2.1.0.jar @@ -124,9 +124,10 @@ jline-reader-3.26.3.jar jline-style-3.26.3.jar jline-terminal-3.26.3.jar jline-native-3.26.3.jar -micrometer-observation-1.15.12.jar +micrometer-observation-1.16.7.jar spring-jcl-6.1.21.jar -micrometer-commons-1.15.12.jar +micrometer-commons-1.16.7.jar +jspecify-1.0.1.jar LatencyUtils-2.0.3.jar snakeyaml-2.5.jar reactor-core-3.6.10.jar diff --git a/geode-core/src/main/java/org/apache/geode/metrics/internal/NoopMeterRegistry.java b/geode-core/src/main/java/org/apache/geode/metrics/internal/NoopMeterRegistry.java index bd70d1d99240..b93f069a6bc4 100644 --- a/geode-core/src/main/java/org/apache/geode/metrics/internal/NoopMeterRegistry.java +++ b/geode-core/src/main/java/org/apache/geode/metrics/internal/NoopMeterRegistry.java @@ -39,13 +39,10 @@ import io.micrometer.core.instrument.noop.NoopLongTaskTimer; import io.micrometer.core.instrument.noop.NoopMeter; import io.micrometer.core.instrument.noop.NoopTimer; -import io.micrometer.core.lang.NonNullApi; -import io.micrometer.core.lang.Nullable; import org.apache.geode.annotations.Immutable; import org.apache.geode.annotations.VisibleForTesting; -@NonNullApi public class NoopMeterRegistry extends MeterRegistry { @Immutable @@ -71,7 +68,7 @@ private NoopMeterRegistry(Clock clock) { } @Override - protected Gauge newGauge(Meter.Id id, @Nullable T obj, ToDoubleFunction valueFunction) { + protected Gauge newGauge(Meter.Id id, T obj, ToDoubleFunction valueFunction) { return new NoopGauge(id); } diff --git a/geode-server-all/src/integrationTest/resources/dependency_classpath.txt b/geode-server-all/src/integrationTest/resources/dependency_classpath.txt index 8ba910109e4d..c1d33d9a29f9 100644 --- a/geode-server-all/src/integrationTest/resources/dependency_classpath.txt +++ b/geode-server-all/src/integrationTest/resources/dependency_classpath.txt @@ -45,7 +45,7 @@ spring-shell-standard-commands-3.3.3.jar spring-shell-standard-3.3.3.jar spring-shell-core-3.3.3.jar commons-io-2.19.0.jar -micrometer-core-1.15.12.jar +micrometer-core-1.16.7.jar jakarta.resource-api-2.1.0.jar jetty-ee10-annotations-12.0.37.jar spring-boot-starter-validation-3.3.13.jar @@ -124,8 +124,9 @@ jline-reader-3.26.3.jar jline-style-3.26.3.jar jline-terminal-3.26.3.jar jline-native-3.26.3.jar -micrometer-observation-1.15.12.jar -micrometer-commons-1.15.12.jar +micrometer-observation-1.16.7.jar +micrometer-commons-1.16.7.jar +jspecify-1.0.1.jar LatencyUtils-2.0.3.jar snakeyaml-2.5.jar spring-jcl-6.1.21.jar From 7d317c24ffc11bcb05230e21815486e643d8d9b0 Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang <92374539+JinwooHwang@users.noreply.github.com> Date: Thu, 3 Sep 2026 20:25:16 -0400 Subject: [PATCH 2/9] [GEODE-10621] Remediation of CVE-2026-19032: Jackson 2.21.5 to 2.21.6 (#8048) --- boms/geode-all-bom/src/test/resources/expected-pom.xml | 8 ++++---- .../geode/gradle/plugins/DependencyConstraints.groovy | 4 ++-- .../src/integrationTest/resources/assembly_content.txt | 10 +++++----- .../resources/gfsh_dependency_classpath.txt | 10 +++++----- .../integrationTest/resources/dependency_classpath.txt | 10 +++++----- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/boms/geode-all-bom/src/test/resources/expected-pom.xml b/boms/geode-all-bom/src/test/resources/expected-pom.xml index d40e19ff4966..84d481c32760 100644 --- a/boms/geode-all-bom/src/test/resources/expected-pom.xml +++ b/boms/geode-all-bom/src/test/resources/expected-pom.xml @@ -475,22 +475,22 @@ com.fasterxml.jackson.core jackson-core - 2.21.5 + 2.21.6 com.fasterxml.jackson.core jackson-databind - 2.21.5 + 2.21.6 com.fasterxml.jackson.datatype jackson-datatype-joda - 2.21.5 + 2.21.6 com.fasterxml.jackson.datatype jackson-datatype-jsr310 - 2.21.5 + 2.21.6 com.jayway.jsonpath diff --git a/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy b/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy index 72ae49070199..1cb70b31d45d 100644 --- a/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy +++ b/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy @@ -55,9 +55,9 @@ class DependencyConstraints { deps.put("slf4j-api.version", "2.0.18") deps.put("jakarta.transaction-api.version", "2.0.1") deps.put("jboss-modules.version", "1.11.0.Final") - deps.put("jackson.version", "2.21.5") + deps.put("jackson.version", "2.21.6") deps.put("jackson.annotations.version", "2.21") - deps.put("jackson.databind.version", "2.21.5") + deps.put("jackson.databind.version", "2.21.6") // Spring Framework 6.x Migration deps.put("springshell.version", "3.3.3") deps.put("springframework.version", "6.1.21") diff --git a/geode-assembly/src/integrationTest/resources/assembly_content.txt b/geode-assembly/src/integrationTest/resources/assembly_content.txt index eb63cebdf6a6..52491b4aaa4e 100644 --- a/geode-assembly/src/integrationTest/resources/assembly_content.txt +++ b/geode-assembly/src/integrationTest/resources/assembly_content.txt @@ -965,11 +965,11 @@ lib/httpcore5-5.4.3.jar lib/httpcore5-h2-5.4.3.jar lib/istack-commons-runtime-4.1.1.jar lib/jackson-annotations-2.21.jar -lib/jackson-core-2.21.5.jar -lib/jackson-databind-2.21.5.jar -lib/jackson-dataformat-yaml-2.21.5.jar -lib/jackson-datatype-joda-2.21.5.jar -lib/jackson-datatype-jsr310-2.21.5.jar +lib/jackson-core-2.21.6.jar +lib/jackson-databind-2.21.6.jar +lib/jackson-dataformat-yaml-2.21.6.jar +lib/jackson-datatype-joda-2.21.6.jar +lib/jackson-datatype-jsr310-2.21.6.jar lib/jakarta.activation-api-2.1.3.jar lib/jakarta.annotation-api-2.1.1.jar lib/jakarta.el-api-5.0.0.jar diff --git a/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt b/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt index 40e811d4495c..f4770a5f02dc 100644 --- a/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt +++ b/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt @@ -21,11 +21,11 @@ spring-shell-starter-3.3.3.jar spring-web-6.1.21.jar commons-lang3-3.18.0.jar rmiio-2.1.2.jar -jackson-datatype-jsr310-2.21.5.jar -jackson-datatype-joda-2.21.5.jar -jackson-core-2.21.5.jar -jackson-dataformat-yaml-2.21.5.jar -jackson-databind-2.21.5.jar +jackson-datatype-jsr310-2.21.6.jar +jackson-datatype-joda-2.21.6.jar +jackson-core-2.21.6.jar +jackson-dataformat-yaml-2.21.6.jar +jackson-databind-2.21.6.jar swagger-annotations-2.2.22.jar jaxb-runtime-4.0.2.jar jaxb-core-4.0.2.jar diff --git a/geode-server-all/src/integrationTest/resources/dependency_classpath.txt b/geode-server-all/src/integrationTest/resources/dependency_classpath.txt index c1d33d9a29f9..cf760a03c5ea 100644 --- a/geode-server-all/src/integrationTest/resources/dependency_classpath.txt +++ b/geode-server-all/src/integrationTest/resources/dependency_classpath.txt @@ -19,11 +19,11 @@ geode-unsafe-0.0.0.jar geode-deployment-legacy-0.0.0.jar snappy-0.5.jar swagger-annotations-2.2.22.jar -jackson-datatype-jsr310-2.21.5.jar -jackson-dataformat-yaml-2.21.5.jar -jackson-core-2.21.5.jar -jackson-datatype-joda-2.21.5.jar -jackson-databind-2.21.5.jar +jackson-datatype-jsr310-2.21.6.jar +jackson-dataformat-yaml-2.21.6.jar +jackson-core-2.21.6.jar +jackson-datatype-joda-2.21.6.jar +jackson-databind-2.21.6.jar httpclient5-5.4.4.jar httpcore5-h2-5.4.3.jar httpcore5-5.4.3.jar From 3c26a210e5ab58e8d39403fca156b4c2ca46ba5f Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang <92374539+JinwooHwang@users.noreply.github.com> Date: Thu, 3 Sep 2026 20:56:47 -0400 Subject: [PATCH 3/9] [GEODE-10622] Remediation of CVE-2026-47857: Reactor Core 3.6.10 to 3.8.7 (#8049) --- .../apache/geode/gradle/plugins/DependencyConstraints.groovy | 5 +++++ .../src/integrationTest/resources/assembly_content.txt | 2 +- .../integrationTest/resources/gfsh_dependency_classpath.txt | 2 +- .../src/integrationTest/resources/dependency_classpath.txt | 2 +- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy b/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy index 1cb70b31d45d..a21a6e558fdb 100644 --- a/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy +++ b/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy @@ -67,6 +67,9 @@ class DependencyConstraints { deps.put("springldap.version", "3.2.7") deps.put("springdoc.version", "2.6.0") + // Pin Reactor Core (transitive via spring-shell-core) to a fixed version + deps.put("reactor-core.version", "3.8.7") + // These version numbers are used in testing various versions of tomcat and are consumed explicitly // in will be called explicitly in the relevant extensions module, and respective configurations // in geode-assembly.gradle. Moreover, dependencyManagement does not seem to play nicely when @@ -149,6 +152,8 @@ class DependencyConstraints { api(group: 'io.github.resilience4j', name: 'resilience4j-retry', version: '1.7.1') api(group: 'io.lettuce', name: 'lettuce-core', version: '6.1.8.RELEASE') api(group: 'io.micrometer', name: 'micrometer-core', version: get('micrometer.version')) + // Pin Reactor Core (pulled in via spring-shell-core) to 3.8.7 + api(group: 'io.projectreactor', name: 'reactor-core', version: get('reactor-core.version')) api(group: 'io.swagger.core.v3', name: 'swagger-annotations', version: '2.2.22') api(group: 'org.hdrhistogram', name: 'HdrHistogram', version: '2.2.2') api(group: 'it.unimi.dsi', name: 'fastutil', version: get('fastutil.version')) diff --git a/geode-assembly/src/integrationTest/resources/assembly_content.txt b/geode-assembly/src/integrationTest/resources/assembly_content.txt index 52491b4aaa4e..2f88d6a0f2c8 100644 --- a/geode-assembly/src/integrationTest/resources/assembly_content.txt +++ b/geode-assembly/src/integrationTest/resources/assembly_content.txt @@ -1033,7 +1033,7 @@ lib/mx4j-remote-3.0.2.jar lib/mx4j-tools-3.0.1.jar lib/ra.jar lib/reactive-streams-1.0.4.jar -lib/reactor-core-3.6.10.jar +lib/reactor-core-3.8.7.jar lib/rmiio-2.1.2.jar lib/shiro-cache-3.0.0.jar lib/shiro-config-core-3.0.0.jar diff --git a/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt b/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt index f4770a5f02dc..8a26d0186bf1 100644 --- a/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt +++ b/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt @@ -130,7 +130,7 @@ micrometer-commons-1.16.7.jar jspecify-1.0.1.jar LatencyUtils-2.0.3.jar snakeyaml-2.5.jar -reactor-core-3.6.10.jar +reactor-core-3.8.7.jar ST4-4.3.3.jar txw2-4.0.2.jar asm-commons-9.10.1.jar diff --git a/geode-server-all/src/integrationTest/resources/dependency_classpath.txt b/geode-server-all/src/integrationTest/resources/dependency_classpath.txt index cf760a03c5ea..d0c9eccab8aa 100644 --- a/geode-server-all/src/integrationTest/resources/dependency_classpath.txt +++ b/geode-server-all/src/integrationTest/resources/dependency_classpath.txt @@ -134,7 +134,7 @@ asm-commons-9.10.1.jar asm-tree-9.10.1.jar asm-9.10.1.jar txw2-4.0.2.jar -reactor-core-3.6.10.jar +reactor-core-3.8.7.jar ST4-4.3.3.jar jakarta.enterprise.lang-model-4.0.1.jar reactive-streams-1.0.4.jar From 3c135beebd4a5b3414aac68e9ba04144edd9c7cf Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang <92374539+JinwooHwang@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:35:45 -0400 Subject: [PATCH 4/9] [GEODE-10618] Make named-query endpoint permissions consistent with region data endpoints (#8043) --- .../web/RestSecurityIntegrationTest.java | 4 + .../web/RestSecurityPostProcessorTest.java | 2 +- ...ueryAccessControllerAuthorizationTest.java | 192 ++++++++++++++++++ .../controllers/QueryAccessController.java | 4 +- 4 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/QueryAccessControllerAuthorizationTest.java diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java index 2a9bc83f35e1..7a0a97fe7242 100644 --- a/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java +++ b/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java @@ -107,6 +107,8 @@ public void testPostQuery() { assertResponse(restClient.doPost("/queries?id=0&q=", "user", "user", "")) .hasStatusCode(403); assertResponse(restClient.doPost("/queries?id=0&q=", "dataRead", "dataRead", "")) + .hasStatusCode(403); + assertResponse(restClient.doPost("/queries?id=0&q=", "dataWrite", "dataWrite", "")) .hasStatusCode(500); } @@ -127,6 +129,8 @@ public void testPutQuery() { assertResponse(restClient.doPut("/queries/id", "user", "user", "{\"id\" : \"foo\"}")) .hasStatusCode(403); assertResponse(restClient.doPut("/queries/id", "dataRead", "dataRead", "{\"id\" : \"foo\"}")) + .hasStatusCode(403); + assertResponse(restClient.doPut("/queries/id", "dataWrite", "dataWrite", "{\"id\" : \"foo\"}")) .hasStatusCode(404); } diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java index a30855ac3631..cd5a3fd73004 100644 --- a/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java +++ b/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java @@ -158,7 +158,7 @@ public void namedQuery() throws Exception { // Install the named query assertResponse( restClient.doPost("/queries?id=selectCustomer&q=" + URLEncoder.encode(namedQuery, "UTF-8"), - "dataReader", "1234567", "")) + "dataUser", "1234567", "")) .hasStatusCode(201); // Verify the query has been installed diff --git a/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/QueryAccessControllerAuthorizationTest.java b/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/QueryAccessControllerAuthorizationTest.java new file mode 100644 index 000000000000..72e0db3fbb90 --- /dev/null +++ b/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/QueryAccessControllerAuthorizationTest.java @@ -0,0 +1,192 @@ +/* + * 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.rest.internal.web.controllers; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic; +import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.web.GenericXmlWebContextLoader; +import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.test.context.web.WebMergedContextConfiguration; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.RequestPostProcessor; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.web.context.WebApplicationContext; +import org.springframework.web.context.support.GenericWebApplicationContext; + +import org.apache.geode.cache.Region; +import org.apache.geode.cache.RegionShortcut; +import org.apache.geode.cache.internal.HttpService; +import org.apache.geode.examples.SimpleSecurityManager; +import org.apache.geode.management.internal.RestAgent; +import org.apache.geode.test.junit.rules.ServerStarterRule; + +/** + * Verifies the permissions the named-query endpoints of {@link QueryAccessController} require. + * + *

+ * The endpoints that read query state require {@code DATA:READ}; the endpoints that create, update + * or remove a stored named query all require {@code DATA:WRITE}, matching the permission required + * for the equivalent operations on ordinary region data. + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration(locations = {"classpath*:WEB-INF/geode-servlet.xml"}, + loader = SecuredTestContextLoader.class) +@WebAppConfiguration +public class QueryAccessControllerAuthorizationTest { + + private static final String QUERY_STORE = "__ParameterizedQueries__"; + private static final String REGION_NAME = "customers"; + + private static final String READ_USER = "dataRead"; + private static final String WRITE_USER = "dataWrite"; + + private static final String OQL = "SELECT * FROM " + Region.SEPARATOR + REGION_NAME; + private static final String OTHER_OQL = + "SELECT c.name FROM " + Region.SEPARATOR + REGION_NAME + " c"; + + private static final RequestPostProcessor JSON = new JsonRequestPostProcessor(); + + @ClassRule + public static ServerStarterRule rule = new ServerStarterRule() + .withProperty("log-level", "warn") + .withSecurityManager(SimpleSecurityManager.class) + .withRegion(RegionShortcut.REPLICATE, REGION_NAME); + + @Autowired + private WebApplicationContext webApplicationContext; + + private MockMvc mockMvc; + + @BeforeClass + public static void createQueryStore() { + RestAgent.createParameterizedQueryRegion(); + } + + @Before + public void setUp() { + mockMvc = MockMvcBuilders.webAppContextSetup(webApplicationContext) + .apply(springSecurity()) + .build(); + queryStore().clear(); + } + + private static Region queryStore() { + return rule.getCache().getInternalRegionByPath(Region.SEPARATOR + QUERY_STORE); + } + + @Test + public void createIsRefusedForAUserWithoutWritePermission() throws Exception { + mockMvc.perform(post("/v1/queries?id=q1&q=" + OQL) + .with(httpBasic(READ_USER, READ_USER)) + .with(JSON)) + .andExpect(status().isForbidden()); + + assertThat(queryStore()).doesNotContainKey("q1"); + } + + @Test + public void updateIsRefusedForAUserWithoutWritePermission() throws Exception { + queryStore().put("q1", OQL); + + mockMvc.perform(put("/v1/queries/q1?q=" + OTHER_OQL) + .with(httpBasic(READ_USER, READ_USER)) + .with(JSON)) + .andExpect(status().isForbidden()); + + assertThat(queryStore().get("q1")).isEqualTo(OQL); + } + + @Test + public void deleteIsRefusedForAUserWithoutWritePermission() throws Exception { + queryStore().put("q1", OQL); + + mockMvc.perform(delete("/v1/queries/q1") + .with(httpBasic(READ_USER, READ_USER)) + .with(JSON)) + .andExpect(status().isForbidden()); + + assertThat(queryStore()).containsKey("q1"); + } + + @Test + public void createAndUpdateAreAllowedForAUserWithWritePermission() throws Exception { + mockMvc.perform(post("/v1/queries?id=q1&q=" + OQL) + .with(httpBasic(WRITE_USER, WRITE_USER)) + .with(JSON)) + .andExpect(status().isCreated()); + + assertThat(queryStore().get("q1")).isEqualTo(OQL); + + mockMvc.perform(put("/v1/queries/q1?q=" + OTHER_OQL) + .with(httpBasic(WRITE_USER, WRITE_USER)) + .with(JSON)) + .andExpect(status().isOk()); + + assertThat(queryStore().get("q1")).isEqualTo(OTHER_OQL); + } + + @Test + public void listIsAllowedForAUserWithReadPermission() throws Exception { + queryStore().put("q1", OQL); + + mockMvc.perform(get("/v1/queries") + .with(httpBasic(READ_USER, READ_USER)) + .with(JSON)) + .andExpect(status().isOk()); + } + + private static class JsonRequestPostProcessor implements RequestPostProcessor { + + @SuppressWarnings("deprecation") + private static final MediaType APPLICATION_JSON_UTF8 = MediaType.APPLICATION_JSON_UTF8; + + @Override + public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) { + request.addHeader(HttpHeaders.ACCEPT, APPLICATION_JSON_UTF8); + request.addHeader(HttpHeaders.CONTENT_TYPE, APPLICATION_JSON_UTF8); + return request; + } + } +} + + +class SecuredTestContextLoader extends GenericXmlWebContextLoader { + @Override + protected void loadBeanDefinitions(GenericWebApplicationContext context, + WebMergedContextConfiguration webMergedConfig) { + super.loadBeanDefinitions(context, webMergedConfig); + context.getServletContext().setAttribute( + HttpService.SECURITY_SERVICE_SERVLET_CONTEXT_PARAM, + QueryAccessControllerAuthorizationTest.rule.getCache().getSecurityService()); + } +} diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java index 5540ba455de7..42071306eb5f 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java @@ -129,7 +129,7 @@ public ResponseEntity list() { @ApiResponse(responseCode = "403", description = "Insufficient privileges for operation."), @ApiResponse(responseCode = "409", description = "QueryId already assigned to other query."), @ApiResponse(responseCode = "500", description = "GemFire throws an error or exception.")}) - @PreAuthorize("@securityService.authorizeBoolean('DATA', 'READ')") + @PreAuthorize("@securityService.authorizeBoolean('DATA', 'WRITE')") public ResponseEntity create(@RequestParam("id") final String queryId, @RequestParam(value = "q", required = false) String oqlInUrl, @RequestBody(required = false) final String oqlInBody) { @@ -311,7 +311,7 @@ public ResponseEntity runNamedQuery(@PathVariable("query") String queryI @ApiResponse(responseCode = "403", description = "Insufficient privileges for operation."), @ApiResponse(responseCode = "404", description = "queryId does not exist."), @ApiResponse(responseCode = "500", description = "GemFire throws an error or exception.")}) - @PreAuthorize("@securityService.authorizeBoolean('DATA', 'READ')") + @PreAuthorize("@securityService.authorizeBoolean('DATA', 'WRITE')") public ResponseEntity update(@PathVariable("query") final String queryId, @RequestParam(value = "q", required = false) String oqlInUrl, @RequestBody(required = false) final String oqlInBody) { From 634944d8f9cf4eb5d2861c1849de8f0f6978359a Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang <92374539+JinwooHwang@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:36:30 -0400 Subject: [PATCH 5/9] GEODE-10613: Add configurable export directories for export data (#8042) --- .../implementing_authorization.html.md.erb | 2 +- .../gfsh/command-pages/export.html.md.erb | 16 ++ .../commands/ExportDataIntegrationTest.java | 7 + .../commands/ImportDataIntegrationTest.java | 7 + .../cli/commands/ExportDataCommand.java | 26 +++ .../cli/functions/ExportDataFunction.java | 60 ++++++- .../cli/commands/ExportDataCommandTest.java | 32 ++++ .../ExportDataDirectoryConfigTest.java | 169 ++++++++++++++++++ .../internal/security/TestCommand.java | 2 +- 9 files changed, 317 insertions(+), 4 deletions(-) create mode 100644 geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/functions/ExportDataDirectoryConfigTest.java diff --git a/geode-docs/security/implementing_authorization.html.md.erb b/geode-docs/security/implementing_authorization.html.md.erb index 37dcb917394c..d30712d29370 100644 --- a/geode-docs/security/implementing_authorization.html.md.erb +++ b/geode-docs/security/implementing_authorization.html.md.erb @@ -154,7 +154,7 @@ This table classifies the permissions assigned for `gfsh` operations. | execute function | Defaults to DATA:WRITE. Override `Function.getRequiredPermissions` to change the permission. | | export cluster-configuration | CLUSTER:READ | | export config | CLUSTER:READ | -| export data | CLUSTER:READ | +| export data | DATA:READ:RegionName and CLUSTER:WRITE | | export logs | CLUSTER:READ | | export offline-disk-store | CLUSTER:READ | | export stack-traces | CLUSTER:READ | diff --git a/geode-docs/tools_modules/gfsh/command-pages/export.html.md.erb b/geode-docs/tools_modules/gfsh/command-pages/export.html.md.erb index 0fc1a76be7ad..4c360119ef7f 100644 --- a/geode-docs/tools_modules/gfsh/command-pages/export.html.md.erb +++ b/geode-docs/tools_modules/gfsh/command-pages/export.html.md.erb @@ -165,6 +165,22 @@ In this scenario, partitioned region data is exported simultaneously on all host | ‑‑dir | Directory to which the exported data is to be written. Required if ‑‑parallel is true. Cannot be specified at the same time as ‑‑file.| | ‑‑parallel | Export local data on each node to a directory on that machine. Available for partitioned regions only. | +**Export locations:** + +The snapshot is written by the member named in `--member`, on that member's host. A member writes +exports into its own working directory (and sub-directories of it). To export somewhere else, such +as a mounted backup location, set the `gemfire.export.data.dirs` system property on the member to +the additional directories, separated by the platform's path separator: + +``` pre +-Dgemfire.export.data.dirs=/mnt/backup/geode:/var/exports/geode +``` + +A path containing a `..` segment is not accepted, and a path that resolves outside the configured +directories is rejected by the member. + +**Required permission:** `DATA:READ` on the exported region, plus `CLUSTER:WRITE`. + **Example Commands:** ``` pre diff --git a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java index 80082f15f167..86e800d56058 100644 --- a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java +++ b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java @@ -17,6 +17,7 @@ package org.apache.geode.management.internal.cli.commands; import static org.apache.geode.cache.Region.SEPARATOR; +import static org.apache.geode.management.internal.cli.functions.ExportDataFunction.EXPORT_DATA_DIRS_PROPERTY; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertFalse; @@ -31,6 +32,7 @@ import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; import org.junit.rules.TemporaryFolder; import org.apache.geode.DataSerializable; @@ -58,6 +60,9 @@ public class ExportDataIntegrationTest { @Rule public TemporaryFolder tempDir = new TemporaryFolder(); + @Rule + public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + private Region region; private Path snapshotFile; private Path snapshotDir; @@ -87,6 +92,8 @@ public void setup() throws Exception { region = server.getCache().getRegion(TEST_REGION_NAME); loadRegion("value"); Path basePath = tempDir.getRoot().toPath(); + // configure the test's temporary folder as an export destination + System.setProperty(EXPORT_DATA_DIRS_PROPERTY, basePath.toString()); snapshotFile = basePath.resolve(SNAPSHOT_FILE); snapshotDir = basePath.resolve(SNAPSHOT_DIR); } diff --git a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ImportDataIntegrationTest.java b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ImportDataIntegrationTest.java index 63fb1461bf4c..2317ce847c91 100644 --- a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ImportDataIntegrationTest.java +++ b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ImportDataIntegrationTest.java @@ -17,6 +17,7 @@ package org.apache.geode.management.internal.cli.commands; import static org.apache.geode.cache.Region.SEPARATOR; +import static org.apache.geode.management.internal.cli.functions.ExportDataFunction.EXPORT_DATA_DIRS_PROPERTY; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; @@ -30,6 +31,7 @@ import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; import org.junit.rules.TemporaryFolder; import org.apache.geode.cache.Region; @@ -55,6 +57,9 @@ public class ImportDataIntegrationTest { @Rule public TemporaryFolder tempDir = new TemporaryFolder(); + @Rule + public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + private Region region; private Path snapshotFile; private Path snapshotDir; @@ -65,6 +70,8 @@ public void setup() throws Exception { region = server.getCache().getRegion(TEST_REGION_NAME); loadRegion("value"); Path basePath = tempDir.getRoot().toPath(); + // configure the test's temporary folder as an export destination + System.setProperty(EXPORT_DATA_DIRS_PROPERTY, basePath.toString()); snapshotFile = basePath.resolve(SNAPSHOT_FILE); snapshotDir = basePath.resolve(SNAPSHOT_DIR); } diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java index 9892ceef5f3d..385d2c3e5242 100644 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java @@ -16,6 +16,8 @@ package org.apache.geode.management.internal.cli.commands; import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; import java.util.Optional; @@ -33,6 +35,7 @@ import org.apache.geode.management.internal.cli.result.model.ResultModel; import org.apache.geode.management.internal.functions.CliFunctionResult; import org.apache.geode.management.internal.i18n.CliStrings; +import org.apache.geode.security.ResourcePermission; import org.apache.geode.security.ResourcePermission.Operation; import org.apache.geode.security.ResourcePermission.Resource; @@ -54,6 +57,7 @@ public ResultModel exportData( help = CliStrings.EXPORT_DATA__PARALLEL_HELP) boolean parallel) { authorize(Resource.DATA, Operation.READ, regionName); + authorize(Resource.CLUSTER, Operation.WRITE, ResourcePermission.ALL); final DistributedMember targetMember = getMember(memberNameOrId); Optional validationResult = validatePath(filePath, dirPath, parallel); @@ -100,6 +104,28 @@ private Optional validatePath(String filePath, String dirPath, bool return Optional.of(ResultModel.createError(CliStrings.format( CliStrings.INVALID_FILE_EXTENSION, CliStrings.GEODE_DATA_FILE_EXTENSION))); } + + if (filePath != null && containsParentDirectorySegment(filePath)) { + return Optional.of(invalidPathError(CliStrings.EXPORT_DATA__FILE, filePath)); + } + if (dirPath != null && containsParentDirectorySegment(dirPath)) { + return Optional.of(invalidPathError(CliStrings.EXPORT_DATA__DIR, dirPath)); + } + return Optional.empty(); } + + private static boolean containsParentDirectorySegment(String path) { + for (Path element : Paths.get(path)) { + if ("..".equals(element.toString())) { + return true; + } + } + return false; + } + + private static ResultModel invalidPathError(String option, String path) { + return ResultModel.createError(String.format( + "Option \"%s\" must not contain a \"..\" path segment: %s", option, path)); + } } diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/functions/ExportDataFunction.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/functions/ExportDataFunction.java index 0c83d40a8ae6..2f0a18174721 100644 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/functions/ExportDataFunction.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/functions/ExportDataFunction.java @@ -15,6 +15,9 @@ package org.apache.geode.management.internal.cli.functions; import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import org.apache.geode.cache.Cache; import org.apache.geode.cache.Region; @@ -27,12 +30,15 @@ import org.apache.geode.management.cli.CliFunction; import org.apache.geode.management.internal.functions.CliFunctionResult; import org.apache.geode.management.internal.i18n.CliStrings; +import org.apache.geode.util.internal.GeodeGlossary; /*** * Function which carries out the export of a region to a file on a member. Uses the * RegionSnapshotService to export the data * - * + *

+ * Export destinations are resolved to their canonical form and must be within the export + * directories configured for this member. */ public class ExportDataFunction extends CliFunction { private static final long serialVersionUID = 1L; @@ -40,6 +46,18 @@ public class ExportDataFunction extends CliFunction { private static final String ID = "org.apache.geode.management.internal.cli.functions.ExportDataFunction"; + /** + * System property naming additional directories this member writes {@code export data} snapshots + * into. Several directories may be listed, separated by {@link File#pathSeparator}. Exports into + * sub-directories of a configured directory are included. + * + *

+ * The member's working directory is always configured, since that is where a relative export + * path resolves to, so when this property is not set it is the only export destination. + */ + public static final String EXPORT_DATA_DIRS_PROPERTY = + GeodeGlossary.GEMFIRE_PREFIX + "export.data.dirs"; + @Override public String getId() { return ID; @@ -62,7 +80,7 @@ public CliFunctionResult executeFunction(FunctionContext context) thro String hostName = cache.getDistributedSystem().getDistributedMember().getHost(); if (region != null) { RegionSnapshotService snapshotService = region.getSnapshotService(); - final File exportFile = new File(fileName); + final File exportFile = resolveExportFile(fileName); if (parallel) { SnapshotOptions options = new SnapshotOptionsImpl<>().setParallelMode(true); snapshotService.save(exportFile, SnapshotFormat.GEODE, options); @@ -81,4 +99,42 @@ public CliFunctionResult executeFunction(FunctionContext context) thro return result; } + + /** + * Resolves the requested export path against the export directories configured for this member. + * + * @param fileName the path requested by the caller, which may be relative or absolute + * @return the canonical file to export to + * @throws IllegalArgumentException if the path is not within a configured export directory + */ + static File resolveExportFile(String fileName) throws IOException { + File exportFile = new File(fileName).getCanonicalFile(); + List exportDirs = configuredExportDirs(); + + for (File exportDir : exportDirs) { + if (exportFile.toPath().startsWith(exportDir.toPath())) { + return exportFile; + } + } + + throw new IllegalArgumentException(String.format( + "Cannot export to %s: the path is not within the export directories configured for this member (%s). Use the %s system property to configure additional directories.", + exportFile, exportDirs, EXPORT_DATA_DIRS_PROPERTY)); + } + + private static List configuredExportDirs() throws IOException { + List exportDirs = new ArrayList<>(); + exportDirs.add(new File(System.getProperty("user.dir")).getCanonicalFile()); + + String configuredDirs = System.getProperty(EXPORT_DATA_DIRS_PROPERTY); + if (configuredDirs != null) { + for (String configuredDir : configuredDirs.split(File.pathSeparator)) { + if (!configuredDir.trim().isEmpty()) { + exportDirs.add(new File(configuredDir.trim()).getCanonicalFile()); + } + } + } + + return exportDirs; + } } diff --git a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandTest.java b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandTest.java index dd9a1f1e1370..61e8b655499e 100644 --- a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandTest.java +++ b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandTest.java @@ -15,10 +15,18 @@ package org.apache.geode.management.internal.cli.commands; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; + import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; +import org.apache.geode.distributed.DistributedMember; import org.apache.geode.test.junit.rules.GfshParserRule; @@ -34,6 +42,14 @@ public void setUp() { command = new ExportDataCommand(); } + /** A command whose option values are checked without contacting a member. */ + private ExportDataCommand commandWithMember() { + ExportDataCommand withMember = spy(ExportDataCommand.class); + doNothing().when(withMember).authorize(any(), any(), anyString()); + doReturn(mock(DistributedMember.class)).when(withMember).getMember(anyString()); + return withMember; + } + @Test public void missingMember() throws Exception { // Command parses successfully but fails during execution because cache is null @@ -41,4 +57,20 @@ public void missingMember() throws Exception { .statusIsError() .containsOutput("cache"); } + + @Test + public void fileOptionWithParentDirectorySegmentIsRejected() { + gfsh.executeAndAssertThat(commandWithMember(), + "export data --member=server1 --region=regionA --file=exports/../regionA.gfd") + .statusIsError() + .containsOutput("must not contain a \"..\" path segment"); + } + + @Test + public void dirOptionWithParentDirectorySegmentIsRejected() { + gfsh.executeAndAssertThat(commandWithMember(), + "export data --member=server1 --region=regionA --dir=exports/../elsewhere") + .statusIsError() + .containsOutput("must not contain a \"..\" path segment"); + } } diff --git a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/functions/ExportDataDirectoryConfigTest.java b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/functions/ExportDataDirectoryConfigTest.java new file mode 100644 index 000000000000..29e790a4f452 --- /dev/null +++ b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/functions/ExportDataDirectoryConfigTest.java @@ -0,0 +1,169 @@ +/* + * 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.management.internal.cli.functions; + +import static org.apache.geode.management.internal.cli.functions.ExportDataFunction.EXPORT_DATA_DIRS_PROPERTY; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.After; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +/** + * Tests the export directories configured for a member, and which destinations resolve within + * them. + */ +public class ExportDataDirectoryConfigTest { + + private static final String SNAPSHOT = "testRegion.gfd"; + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private String originalProperty; + private Path configuredDir; + private Path otherDir; + + @Before + public void before() throws Exception { + originalProperty = System.getProperty(EXPORT_DATA_DIRS_PROPERTY); + configuredDir = temporaryFolder.newFolder("exports").toPath().toRealPath(); + otherDir = temporaryFolder.newFolder("elsewhere").toPath().toRealPath(); + configure(configuredDir); + } + + @After + public void after() { + if (originalProperty == null) { + System.clearProperty(EXPORT_DATA_DIRS_PROPERTY); + } else { + System.setProperty(EXPORT_DATA_DIRS_PROPERTY, originalProperty); + } + } + + private void configure(Path... dirs) { + StringBuilder value = new StringBuilder(); + for (Path dir : dirs) { + if (value.length() > 0) { + value.append(File.pathSeparator); + } + value.append(dir); + } + System.setProperty(EXPORT_DATA_DIRS_PROPERTY, value.toString()); + } + + @Test + public void exportIntoConfiguredDirectorySucceeds() throws Exception { + Path destination = configuredDir.resolve(SNAPSHOT); + + File resolved = ExportDataFunction.resolveExportFile(destination.toString()); + + assertThat(resolved.toPath()).isEqualTo(destination); + } + + @Test + public void exportIntoSubdirectoryOfConfiguredDirectorySucceeds() throws Exception { + Path destination = configuredDir.resolve("daily").resolve(SNAPSHOT); + + File resolved = ExportDataFunction.resolveExportFile(destination.toString()); + + assertThat(resolved.toPath()).isEqualTo(destination); + } + + @Test + public void exportOutsideConfiguredDirectoriesIsRejected() { + Path destination = otherDir.resolve(SNAPSHOT); + + assertThatThrownBy(() -> ExportDataFunction.resolveExportFile(destination.toString())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("not within the export directories configured") + .hasMessageContaining(EXPORT_DATA_DIRS_PROPERTY); + } + + @Test + public void directoryWithMatchingNamePrefixIsNotIncluded() throws Exception { + Path sibling = temporaryFolder.newFolder("exports-archive").toPath().toRealPath(); + Path destination = sibling.resolve(SNAPSHOT); + + assertThatThrownBy(() -> ExportDataFunction.resolveExportFile(destination.toString())) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void severalDirectoriesCanBeConfigured() throws Exception { + configure(configuredDir, otherDir); + + assertThat(ExportDataFunction.resolveExportFile(configuredDir.resolve(SNAPSHOT).toString())) + .isNotNull(); + assertThat(ExportDataFunction.resolveExportFile(otherDir.resolve(SNAPSHOT).toString())) + .isNotNull(); + } + + @Test + public void emptyEntriesInThePropertyAreIgnored() throws Exception { + System.setProperty(EXPORT_DATA_DIRS_PROPERTY, + File.pathSeparator + configuredDir + File.pathSeparator + File.pathSeparator); + + File resolved = + ExportDataFunction.resolveExportFile(configuredDir.resolve(SNAPSHOT).toString()); + + assertThat(resolved.toPath()).isEqualTo(configuredDir.resolve(SNAPSHOT)); + } + + @Test + public void workingDirectoryIsUsedWhenThePropertyIsNotSet() throws Exception { + System.clearProperty(EXPORT_DATA_DIRS_PROPERTY); + Path workingDir = new File(System.getProperty("user.dir")).getCanonicalFile().toPath(); + + File resolved = ExportDataFunction.resolveExportFile(workingDir.resolve(SNAPSHOT).toString()); + + assertThat(resolved.toPath()).isEqualTo(workingDir.resolve(SNAPSHOT)); + assertThatThrownBy(() -> ExportDataFunction.resolveExportFile(otherDir.resolve(SNAPSHOT) + .toString())).isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void relativePathResolvesInsideTheWorkingDirectory() throws Exception { + System.clearProperty(EXPORT_DATA_DIRS_PROPERTY); + Path workingDir = new File(System.getProperty("user.dir")).getCanonicalFile().toPath(); + + File resolved = ExportDataFunction.resolveExportFile(SNAPSHOT); + + assertThat(resolved.toPath()).isEqualTo(workingDir.resolve(SNAPSHOT)); + } + + @Test + public void linkedDirectoryResolvesToItsTarget() throws Exception { + Path link = configuredDir.resolve("archive"); + try { + Files.createSymbolicLink(link, otherDir); + } catch (IOException | UnsupportedOperationException e) { + Assume.assumeNoException("filesystem does not support links", e); + } + + assertThatThrownBy(() -> ExportDataFunction.resolveExportFile(link.resolve(SNAPSHOT) + .toString())).isInstanceOf(IllegalArgumentException.class); + } +} diff --git a/geode-junit/src/main/java/org/apache/geode/management/internal/security/TestCommand.java b/geode-junit/src/main/java/org/apache/geode/management/internal/security/TestCommand.java index ebbd8c950c43..0199985546af 100644 --- a/geode-junit/src/main/java/org/apache/geode/management/internal/security/TestCommand.java +++ b/geode-junit/src/main/java/org/apache/geode/management/internal/security/TestCommand.java @@ -135,7 +135,7 @@ private static void init() { // Data Commands createTestCommand("rebalance --include-region=RegionA", ResourcePermissions.DATA_MANAGE); createTestCommand("export data --region=RegionA --file=export.txt --member=exportMember", - regionARead); + regionARead, ResourcePermissions.CLUSTER_WRITE); createTestCommand("import data --region=RegionA --file=import.txt --member=importMember", regionAWrite); createTestCommand("put --key=key1 --value=value1 --region=RegionA", regionAWrite); From 5327575cca71adc2386a755773fd8d789f44d271 Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang <92374539+JinwooHwang@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:37:23 -0400 Subject: [PATCH 6/9] GEODE-10612: Encode region path in Pulse region-detail error messages (#8041) When a region-detail request names a path that does not resolve, the region services return "Region [] is not available" in the errorOnRegion field, which the Pulse UI displays. Paths containing characters such as '<' or '&' did not display correctly. --- ...gionDetailErrorMessageIntegrationTest.java | 155 ++++++++++++++++++ .../service/ClusterSelectedRegionService.java | 5 +- .../ClusterSelectedRegionsMemberService.java | 5 +- .../RegionErrorMessageEncodingTest.java | 132 +++++++++++++++ 4 files changed, 295 insertions(+), 2 deletions(-) create mode 100644 geode-pulse/src/integrationTest/java/org/apache/geode/tools/pulse/controllers/RegionDetailErrorMessageIntegrationTest.java create mode 100644 geode-pulse/src/test/java/org/apache/geode/tools/pulse/internal/service/RegionErrorMessageEncodingTest.java diff --git a/geode-pulse/src/integrationTest/java/org/apache/geode/tools/pulse/controllers/RegionDetailErrorMessageIntegrationTest.java b/geode-pulse/src/integrationTest/java/org/apache/geode/tools/pulse/controllers/RegionDetailErrorMessageIntegrationTest.java new file mode 100644 index 000000000000..94aa800e6e61 --- /dev/null +++ b/geode-pulse/src/integrationTest/java/org/apache/geode/tools/pulse/controllers/RegionDetailErrorMessageIntegrationTest.java @@ -0,0 +1,155 @@ +/* + * 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.tools.pulse.controllers; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.when; +import static org.mockito.quality.Strictness.LENIENT; +import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; +import static org.springframework.http.MediaType.parseMediaType; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import java.security.Principal; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.MediaType; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.web.context.WebApplicationContext; + +import org.apache.geode.test.junit.categories.PulseTest; +import org.apache.geode.tools.pulse.internal.data.Cluster; +import org.apache.geode.tools.pulse.internal.data.Repository; + +/** + * Covers the region-detail error message end to end, from the {@code /pulseUpdate} request the + * Pulse UI posts through to the JSON it receives back. + */ +@Category({PulseTest.class}) +@RunWith(SpringJUnit4ClassRunner.class) +@WebAppConfiguration +@ContextConfiguration("classpath*:WEB-INF/pulse-servlet.xml") +@ActiveProfiles({"pulse.controller.test"}) +public class RegionDetailErrorMessageIntegrationTest { + + private static final String PATH_WITH_SPECIAL_CHARACTERS = "/orders<2026>&archive"; + private static final String ENCODED_MESSAGE = + "Region [/orders<2026>&archive] is not available"; + + private static final MediaType JSON_MEDIA_TYPE = parseMediaType(APPLICATION_JSON_VALUE); + private static final Principal PRINCIPAL = () -> "test-user"; + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @Rule + public MockitoRule mockitoRule = MockitoJUnit.rule().strictness(LENIENT); + + @Autowired + private WebApplicationContext wac; + + @Autowired + private Repository repository; + + @Mock + Cluster cluster; + + private MockMvc mockMvc; + + @Before + public void setup() { + when(repository.getCluster()).thenReturn(cluster); + when(cluster.getServerName()).thenReturn("mock-cluster"); + // The requested path resolves to no region, so the services take the error branch. + when(cluster.getClusterRegion(anyString())).thenReturn(null); + + mockMvc = MockMvcBuilders.webAppContextSetup(wac).build(); + } + + @Test + public void pulseUpdateEncodesSpecialCharactersForClusterSelectedRegion() throws Exception { + MvcResult result = mockMvc + .perform(post("/pulseUpdate") + .with(csrf()) + .param("pulseData", pulseData("ClusterSelectedRegion", PATH_WITH_SPECIAL_CHARACTERS)) + .principal(PRINCIPAL) + .accept(JSON_MEDIA_TYPE)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.ClusterSelectedRegion.selectedRegion.errorOnRegion") + .value(ENCODED_MESSAGE)) + .andReturn(); + + assertThat(result.getResponse().getContentAsString()) + .contains("/orders<2026>&archive"); + } + + @Test + public void pulseUpdateEncodesSpecialCharactersForClusterSelectedRegionsMember() + throws Exception { + MvcResult result = mockMvc + .perform(post("/pulseUpdate") + .with(csrf()) + .param("pulseData", + pulseData("ClusterSelectedRegionsMember", PATH_WITH_SPECIAL_CHARACTERS)) + .principal(PRINCIPAL) + .accept(JSON_MEDIA_TYPE)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.ClusterSelectedRegionsMember.selectedRegionsMembers.errorOnRegion") + .value(ENCODED_MESSAGE)) + .andReturn(); + + assertThat(result.getResponse().getContentAsString()) + .contains("/orders<2026>&archive"); + } + + @Test + public void pulseUpdateLeavesOrdinaryRegionPathUnchanged() throws Exception { + mockMvc + .perform(post("/pulseUpdate") + .with(csrf()) + .param("pulseData", pulseData("ClusterSelectedRegion", "/mock-region")) + .principal(PRINCIPAL) + .accept(JSON_MEDIA_TYPE)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.ClusterSelectedRegion.selectedRegion.errorOnRegion") + .value("Region [/mock-region] is not available")); + } + + /** Builds the {@code pulseData} body the Pulse frontend posts for the region-detail page. */ + private static String pulseData(String service, String regionFullPath) { + ObjectNode parameters = MAPPER.createObjectNode(); + parameters.put("regionFullPath", regionFullPath); + ObjectNode root = MAPPER.createObjectNode(); + root.set(service, parameters); + return root.toString(); + } +} diff --git a/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionService.java b/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionService.java index f99c50155045..74f55416ee1b 100644 --- a/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionService.java +++ b/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionService.java @@ -30,6 +30,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.text.StringEscapeUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.beans.factory.annotation.Autowired; @@ -222,7 +223,9 @@ private ObjectNode getSelectedRegionJson(Cluster cluster, String selectedRegionF return regionJSON; } else { ObjectNode responseJSON = mapper.createObjectNode(); - responseJSON.put("errorOnRegion", "Region [" + selectedRegionFullPath + "] is not available"); + responseJSON.put("errorOnRegion", + "Region [" + StringEscapeUtils.escapeHtml4(selectedRegionFullPath) + + "] is not available"); return responseJSON; } } diff --git a/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionsMemberService.java b/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionsMemberService.java index 238cdabc898c..6d1af501d2c2 100644 --- a/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionsMemberService.java +++ b/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionsMemberService.java @@ -25,6 +25,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import jakarta.servlet.http.HttpServletRequest; +import org.apache.commons.text.StringEscapeUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.beans.factory.annotation.Autowired; @@ -146,7 +147,9 @@ private ObjectNode getSelectedRegionsMembersJson(Cluster cluster, String selecte return regionMemberJSON; } else { ObjectNode responseJSON = mapper.createObjectNode(); - responseJSON.put("errorOnRegion", "Region [" + selectedRegionFullPath + "] is not available"); + responseJSON.put("errorOnRegion", + "Region [" + StringEscapeUtils.escapeHtml4(selectedRegionFullPath) + + "] is not available"); return responseJSON; } } diff --git a/geode-pulse/src/test/java/org/apache/geode/tools/pulse/internal/service/RegionErrorMessageEncodingTest.java b/geode-pulse/src/test/java/org/apache/geode/tools/pulse/internal/service/RegionErrorMessageEncodingTest.java new file mode 100644 index 000000000000..2d1b08674442 --- /dev/null +++ b/geode-pulse/src/test/java/org/apache/geode/tools/pulse/internal/service/RegionErrorMessageEncodingTest.java @@ -0,0 +1,132 @@ +/* + * 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.tools.pulse.internal.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.security.Principal; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import jakarta.servlet.http.HttpServletRequest; +import org.junit.Before; +import org.junit.Test; + +import org.apache.geode.tools.pulse.internal.data.Cluster; +import org.apache.geode.tools.pulse.internal.data.Repository; + +/** + * Tests the {@code errorOnRegion} message the region-detail services produce when the requested + * region path does not resolve. + * + *

+ * Paths containing characters such as {@code <} or {@code &} are encoded so the message displays + * as written. Lookup is unaffected and uses the path as supplied. + */ +public class RegionErrorMessageEncodingTest { + + private static final String PATH_WITH_SPECIAL_CHARACTERS = "/orders<2026>&archive"; + private static final String ENCODED_MESSAGE = + "Region [/orders<2026>&archive] is not available"; + private static final String ORDINARY_PATH = "/mock-region"; + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private Repository repository; + private Cluster cluster; + private HttpServletRequest request; + + @Before + public void setUp() { + repository = mock(Repository.class); + cluster = mock(Cluster.class); + request = mock(HttpServletRequest.class); + Principal principal = mock(Principal.class); + when(principal.getName()).thenReturn("admin"); + when(request.getUserPrincipal()).thenReturn(principal); + when(repository.getCluster()).thenReturn(cluster); + when(cluster.getServerName()).thenReturn("mock-cluster"); + // No region resolves, so every call below takes the error branch. + when(cluster.getClusterRegion(anyString())).thenReturn(null); + } + + @Test + public void clusterSelectedRegionEncodesSpecialCharactersInErrorMessage() throws Exception { + assertThat(selectedRegionError(PATH_WITH_SPECIAL_CHARACTERS)).isEqualTo(ENCODED_MESSAGE); + } + + @Test + public void clusterSelectedRegionsMemberEncodesSpecialCharactersInErrorMessage() + throws Exception { + assertThat(selectedRegionsMemberError(PATH_WITH_SPECIAL_CHARACTERS)).isEqualTo(ENCODED_MESSAGE); + } + + @Test + public void clusterSelectedRegionLeavesOrdinaryPathUnchanged() throws Exception { + assertThat(selectedRegionError(ORDINARY_PATH)) + .isEqualTo("Region [" + ORDINARY_PATH + "] is not available"); + } + + @Test + public void clusterSelectedRegionsMemberLeavesOrdinaryPathUnchanged() throws Exception { + assertThat(selectedRegionsMemberError(ORDINARY_PATH)) + .isEqualTo("Region [" + ORDINARY_PATH + "] is not available"); + } + + @Test + public void clusterSelectedRegionLooksTheRegionUpByTheSuppliedPath() throws Exception { + selectedRegionError(PATH_WITH_SPECIAL_CHARACTERS); + + verify(cluster).getClusterRegion(PATH_WITH_SPECIAL_CHARACTERS); + } + + @Test + public void clusterSelectedRegionsMemberLooksTheRegionUpByTheSuppliedPath() throws Exception { + selectedRegionsMemberError(PATH_WITH_SPECIAL_CHARACTERS); + + verify(cluster).getClusterRegion(PATH_WITH_SPECIAL_CHARACTERS); + } + + private String selectedRegionError(String regionFullPath) throws Exception { + when(request.getParameter("pulseData")) + .thenReturn(pulseData("ClusterSelectedRegion", regionFullPath)); + + ObjectNode json = new ClusterSelectedRegionService(repository).execute(request); + + return json.get("selectedRegion").get("errorOnRegion").asText(); + } + + private String selectedRegionsMemberError(String regionFullPath) throws Exception { + when(request.getParameter("pulseData")) + .thenReturn(pulseData("ClusterSelectedRegionsMember", regionFullPath)); + + ObjectNode json = new ClusterSelectedRegionsMemberService(repository).execute(request); + + return json.get("selectedRegionsMembers").get("errorOnRegion").asText(); + } + + /** Builds the {@code pulseData} body the Pulse frontend posts for the region-detail page. */ + private static String pulseData(String service, String regionFullPath) { + ObjectNode parameters = MAPPER.createObjectNode(); + parameters.put("regionFullPath", regionFullPath); + ObjectNode root = MAPPER.createObjectNode(); + root.set(service, parameters); + return root.toString(); + } +} From 7c165a4f7dee0a3bc5a1f532c9d030018f6d599c Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang <92374539+JinwooHwang@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:43:03 -0400 Subject: [PATCH 7/9] GEODE-10626: Add configurable output directories for export data (#8037) --- ...ExportDataCommandPermissionsDUnitTest.java | 227 ++++++++++++++++ ...portDataPathValidationIntegrationTest.java | 211 +++++++++++++++ .../ExportDataCommandPathValidationTest.java | 215 +++++++++++++++ .../ExportDataFunctionPathValidationTest.java | 248 ++++++++++++++++++ 4 files changed, 901 insertions(+) create mode 100644 geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandPermissionsDUnitTest.java create mode 100644 geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataPathValidationIntegrationTest.java create mode 100644 geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandPathValidationTest.java create mode 100644 geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/functions/ExportDataFunctionPathValidationTest.java diff --git a/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandPermissionsDUnitTest.java b/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandPermissionsDUnitTest.java new file mode 100644 index 000000000000..92324e04b34b --- /dev/null +++ b/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandPermissionsDUnitTest.java @@ -0,0 +1,227 @@ +/* + * 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.management.internal.cli.commands; + +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER; +import static org.apache.geode.management.internal.cli.functions.ExportDataFunction.EXPORT_DATA_DIRS_PROPERTY; +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Properties; + +import org.junit.After; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.TemporaryFolder; + +import org.apache.geode.cache.RegionShortcut; +import org.apache.geode.examples.SimpleSecurityManager; +import org.apache.geode.internal.cache.InternalCache; +import org.apache.geode.management.internal.security.ResourceConstants; +import org.apache.geode.test.dunit.IgnoredException; +import org.apache.geode.test.dunit.rules.ClusterStartupRule; +import org.apache.geode.test.dunit.rules.MemberVM; +import org.apache.geode.test.junit.categories.SecurityTest; +import org.apache.geode.test.junit.rules.GfshCommandRule; + +/** + * Tests which principals may run {@code export data} in a secured cluster. + * + *

+ * {@link SimpleSecurityManager} authorizes a user for exactly those permissions whose string form + * starts with the user name, and treats a comma separated user name as a set of roles. So + * "dataRead" holds DATA:READ alone, while "dataRead,clusterWrite" is the operator {@code + * export data} requires. + */ +@Category(SecurityTest.class) +public class ExportDataCommandPermissionsDUnitTest implements Serializable { + + private static final String REGION_NAME = "testRegion"; + private static final String READ_ONLY_USER = "dataRead"; + private static final String EXPORT_OPERATOR = "dataRead,clusterWrite"; + + @ClassRule + public static ClusterStartupRule cluster = new ClusterStartupRule(); + + @Rule + public GfshCommandRule gfsh = new GfshCommandRule(); + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private static MemberVM locator; + private static MemberVM server; + + /** The directory the server has been configured to permit exports into. */ + private Path permittedDir; + + /** Any other location on the server host. */ + private Path otherDir; + + @BeforeClass + public static void beforeClass() { + Properties locatorProps = new Properties(); + locatorProps.setProperty(SECURITY_MANAGER, SimpleSecurityManager.class.getName()); + locator = cluster.startLocatorVM(0, locatorProps); + + Properties serverProps = new Properties(); + serverProps.setProperty(ResourceConstants.USER_NAME, "clusterManage"); + serverProps.setProperty(ResourceConstants.PASSWORD, "clusterManage"); + server = cluster.startServerVM(1, serverProps, locator.getPort()); + + server.invoke(() -> { + InternalCache cache = ClusterStartupRule.getCache(); + assertThat(cache).isNotNull(); + cache.createRegionFactory(RegionShortcut.REPLICATE).create(REGION_NAME).put("key", "value"); + }); + } + + @Before + public void configurePermittedExportDirectory() throws Exception { + // Refusing an export is logged at error level on the member; that is the expected outcome of + // most of these tests, not a symptom of one going wrong. + IgnoredException.addIgnoredException("Cannot export to"); + + permittedDir = temporaryFolder.newFolder("permitted").toPath(); + otherDir = temporaryFolder.newFolder("other").toPath(); + + String permitted = permittedDir.toString(); + server.invoke(() -> System.setProperty(EXPORT_DATA_DIRS_PROPERTY, permitted)); + } + + @After + public void clearPermittedExportDirectory() { + server.invoke(() -> System.clearProperty(EXPORT_DATA_DIRS_PROPERTY)); + } + + private void connectAs(String user) throws Exception { + gfsh.secureConnectAndVerify(locator.getPort(), GfshCommandRule.PortType.locator, user, user); + } + + private String exportTo(String option, Path path) { + return "export data --member=" + server.getName() + " --region=" + REGION_NAME + " --" + + option + "=" + path; + } + + /** + * Read access to region data on its own does not permit an export. + */ + @Test + public void dataReadUserCannotExport() throws Exception { + connectAs(READ_ONLY_USER); + Path target = permittedDir.resolve("snapshot.gfd"); + + gfsh.executeAndAssertThat(exportTo("file", target)) + .statusIsError() + .containsOutput("not authorized for CLUSTER:WRITE"); + + assertThat(target).doesNotExist(); + } + + /** + * Permissions are checked before the path, so the target directory makes no difference. + */ + @Test + public void dataReadUserIsRefusedForAnyDirectory() throws Exception { + connectAs(READ_ONLY_USER); + Path target = otherDir.resolve("snapshot.gfd"); + + gfsh.executeAndAssertThat(exportTo("file", target)) + .statusIsError() + .containsOutput("not authorized for CLUSTER:WRITE"); + + assertThat(target).doesNotExist(); + } + + /** + * The command works for a principal holding both permissions. + */ + @Test + public void operatorWithClusterWriteCanExportIntoThePermittedDirectory() throws Exception { + connectAs(EXPORT_OPERATOR); + Path target = permittedDir.resolve("snapshot.gfd"); + + gfsh.executeAndAssertThat(exportTo("file", target)) + .statusIsSuccess() + .containsOutput("Data successfully exported"); + + assertThat(target).exists(); + } + + /** + * The directory restriction applies independently of the permission: even a permitted operator + * cannot place the snapshot anywhere it likes. + */ + @Test + public void operatorCannotExportOutsideThePermittedDirectory() throws Exception { + connectAs(EXPORT_OPERATOR); + Path target = otherDir.resolve("snapshot.gfd"); + + gfsh.executeAndAssertThat(exportTo("file", target)).statusIsError(); + + assertThat(target).doesNotExist(); + } + + /** + * Nor can the operator leave the permitted directory with "../". + */ + @Test + public void operatorCannotLeavePermittedDirectoryWithParentReference() throws Exception { + connectAs(EXPORT_OPERATOR); + Path withParentReference = permittedDir.resolve("..").resolve("other"); + + gfsh.executeAndAssertThat(exportTo("dir", withParentReference)).statusIsError(); + + assertThat(otherDir.resolve(REGION_NAME + ".gfd")).doesNotExist(); + } + + /** + * An existing file outside the permitted directory survives an export aimed at it. + */ + @Test + public void existingFileOutsideThePermittedDirectoryIsNotOverwritten() throws Exception { + connectAs(EXPORT_OPERATOR); + Path existingFile = otherDir.resolve("existing.gfd"); + String originalContent = "existing content"; + Files.write(existingFile, originalContent.getBytes(StandardCharsets.UTF_8)); + + gfsh.executeAndAssertThat(exportTo("file", existingFile)).statusIsError(); + + assertThat(new String(Files.readAllBytes(existingFile), StandardCharsets.UTF_8)) + .isEqualTo(originalContent); + } + + /** + * Confirms the read only grant really is read only, so the refusals above are the permission + * check taking effect rather than a misconfigured principal. + */ + @Test + public void readOnlyUserCanStillReadData() throws Exception { + connectAs(READ_ONLY_USER); + + gfsh.executeAndAssertThat("get --region=" + REGION_NAME + " --key=key").statusIsSuccess(); + gfsh.executeAndAssertThat("put --region=" + REGION_NAME + " --key=k --value=v") + .statusIsError() + .containsOutput("dataRead not authorized for DATA:WRITE"); + } +} diff --git a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataPathValidationIntegrationTest.java b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataPathValidationIntegrationTest.java new file mode 100644 index 000000000000..6b6add92a128 --- /dev/null +++ b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataPathValidationIntegrationTest.java @@ -0,0 +1,211 @@ +/* + * 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.management.internal.cli.commands; + +import static org.apache.geode.management.internal.cli.functions.ExportDataFunction.EXPORT_DATA_DIRS_PROPERTY; +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.stream.IntStream; + +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; +import org.junit.rules.TemporaryFolder; + +import org.apache.geode.cache.Region; +import org.apache.geode.cache.RegionShortcut; +import org.apache.geode.management.internal.cli.util.CommandStringBuilder; +import org.apache.geode.management.internal.i18n.CliStrings; +import org.apache.geode.test.junit.rules.GfshCommandRule; +import org.apache.geode.test.junit.rules.ServerStarterRule; + +/** + * End to end tests of the directories {@code export data} writes into: a live server exports into + * the directories it is configured to permit, and refuses paths that resolve outside them. + */ +public class ExportDataPathValidationIntegrationTest { + private static final String TEST_REGION_NAME = "testRegion"; + private static final int DATA_POINTS = 10; + + @ClassRule + public static ServerStarterRule server = new ServerStarterRule().withJMXManager() + .withRegion(RegionShortcut.PARTITION, TEST_REGION_NAME).withEmbeddedLocator(); + + @Rule + public GfshCommandRule gfsh = new GfshCommandRule(); + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Rule + public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + + /** The directory an operator has permitted this member to export into. */ + private Path permittedDir; + + /** Any other location on the server host. */ + private Path otherDir; + + private Region region; + + @Before + public void setup() throws Exception { + gfsh.connectAndVerify(server.getEmbeddedLocatorPort(), GfshCommandRule.PortType.locator); + region = server.getCache().getRegion(TEST_REGION_NAME); + IntStream.range(0, DATA_POINTS).forEach(i -> region.put("key" + i, "value" + i)); + + permittedDir = temporaryFolder.newFolder("permitted").toPath(); + otherDir = temporaryFolder.newFolder("other").toPath(); + System.setProperty(EXPORT_DATA_DIRS_PROPERTY, permittedDir.toString()); + } + + /** The gfsh table wraps long messages, so compare against whitespace normalized output. */ + private String normalizedOutput() { + return gfsh.getGfshOutput().replaceAll("\\s+", " "); + } + + /** + * Exports into a permitted directory work normally. + */ + @Test + public void exportIntoThePermittedDirectorySucceeds() { + Path target = permittedDir.resolve("snapshot.gfd"); + + gfsh.executeAndAssertThat(baseCommand() + .addOption(CliStrings.EXPORT_DATA__FILE, target.toString()) + .getCommandString()).statusIsSuccess(); + + assertThat(target).exists(); + assertThat(target.toFile().length()).isGreaterThan(0L); + } + + /** The --dir form works the same way. */ + @Test + public void exportIntoThePermittedDirectoryWithDirOptionSucceeds() { + gfsh.executeAndAssertThat(baseCommand() + .addOption(CliStrings.EXPORT_DATA__DIR, permittedDir.toString()) + .getCommandString()).statusIsSuccess(); + + assertThat(permittedDir.resolve(TEST_REGION_NAME + ".gfd")).exists(); + } + + /** + * An absolute path outside the permitted directories does not produce a file. + */ + @Test + public void exportToAnAbsolutePathOutsideThePermittedDirectoryIsRefused() { + Path target = otherDir.resolve("snapshot.gfd"); + + gfsh.executeAndAssertThat(baseCommand() + .addOption(CliStrings.EXPORT_DATA__FILE, target.toString()) + .getCommandString()).statusIsError(); + + assertThat(normalizedOutput()).contains("export directories configured for this member"); + assertThat(target).doesNotExist(); + } + + /** + * A "../" in --dir is refused, and nothing is written at the location it points to. + */ + @Test + public void exportWithParentReferenceInDirOptionIsRefused() { + String dirWithParentReference = permittedDir.resolve("..").resolve("other").toString(); + + gfsh.executeAndAssertThat(baseCommand() + .addOption(CliStrings.EXPORT_DATA__DIR, dirWithParentReference) + .getCommandString()).statusIsError(); + + assertThat(normalizedOutput()).contains("path segment"); + assertThat(otherDir.resolve(TEST_REGION_NAME + ".gfd")).doesNotExist(); + } + + /** + * The same for --file: it is caught before the export is sent to the member. + */ + @Test + public void exportWithParentReferenceInFileOptionIsRefused() { + String fileWithParentReference = permittedDir.resolve("..").resolve("other") + .resolve("snapshot.gfd").toString(); + + gfsh.executeAndAssertThat(baseCommand() + .addOption(CliStrings.EXPORT_DATA__FILE, fileWithParentReference) + .getCommandString()).statusIsError(); + + assertThat(normalizedOutput()).contains("path segment"); + assertThat(otherDir.resolve("snapshot.gfd")).doesNotExist(); + } + + /** + * An existing file outside the permitted directories keeps its contents. + */ + @Test + public void existingFileOutsideThePermittedDirectoryIsNotOverwritten() throws Exception { + Path existingFile = otherDir.resolve("existing.gfd"); + String originalContent = "existing content"; + Files.write(existingFile, originalContent.getBytes(StandardCharsets.UTF_8)); + + gfsh.executeAndAssertThat(baseCommand() + .addOption(CliStrings.EXPORT_DATA__FILE, existingFile.toString()) + .getCommandString()).statusIsError(); + + assertThat(new String(Files.readAllBytes(existingFile), StandardCharsets.UTF_8)) + .isEqualTo(originalContent); + } + + /** + * A parallel export does not create a directory tree outside the permitted directories. + */ + @Test + public void parallelExportOutsideThePermittedDirectoryCreatesNoDirectories() { + Path newTree = otherDir.resolve("created/by/export"); + + gfsh.executeAndAssertThat(baseCommand() + .addOption(CliStrings.EXPORT_DATA__DIR, newTree.toString()) + .addOption(CliStrings.EXPORT_DATA__PARALLEL, "true") + .getCommandString()).statusIsError(); + + assertThat(newTree).doesNotExist(); + } + + /** + * Without configuration the member permits only its own working directory. + */ + @Test + public void withoutConfigurationExportOutsideTheWorkingDirectoryIsRefused() { + System.clearProperty(EXPORT_DATA_DIRS_PROPERTY); + Path target = otherDir.resolve("snapshot.gfd"); + + gfsh.executeAndAssertThat(baseCommand() + .addOption(CliStrings.EXPORT_DATA__FILE, target.toString()) + .getCommandString()).statusIsError(); + + assertThat(target).doesNotExist(); + assertThat(normalizedOutput()) + .contains(new File(System.getProperty("user.dir")).getName()); + } + + private CommandStringBuilder baseCommand() { + return new CommandStringBuilder(CliStrings.EXPORT_DATA) + .addOption(CliStrings.MEMBER, server.getName()) + .addOption(CliStrings.EXPORT_DATA__REGION, TEST_REGION_NAME); + } +} diff --git a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandPathValidationTest.java b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandPathValidationTest.java new file mode 100644 index 000000000000..da3868806414 --- /dev/null +++ b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ExportDataCommandPathValidationTest.java @@ -0,0 +1,215 @@ +/* + * 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.management.internal.cli.commands; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +import java.io.File; +import java.util.Collections; + +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.mockito.ArgumentCaptor; + +import org.apache.geode.cache.execute.ResultCollector; +import org.apache.geode.distributed.DistributedMember; +import org.apache.geode.management.internal.functions.CliFunctionResult; +import org.apache.geode.security.NotAuthorizedException; +import org.apache.geode.security.ResourcePermission; +import org.apache.geode.security.ResourcePermission.Operation; +import org.apache.geode.security.ResourcePermission.Resource; +import org.apache.geode.test.junit.rules.GfshParserRule; + +/** + * Tests the path validation and the authorization {@code export data} applies before it sends any + * work to a member. + * + *

+ * The directory configuration is applied on the member; the command checks the option for a parent + * directory reference and asks for the permissions the operation needs. + * + * @see ExportDataCommandPermissionsDUnitTest for the permissions end to end in a secured + * cluster + */ +public class ExportDataCommandPathValidationTest { + + @ClassRule + public static GfshParserRule parser = new GfshParserRule(); + + private static final String REGION = "testRegion"; + /** + * On this branch the option carries no ConverterHint.REGION_PATH, so the command method sees the + * region name exactly as typed (on support/1.15 the converter prepends the separator). + */ + private static final String REGION_PATH = REGION; + + private ExportDataCommand command; + private ArgumentCaptor functionArgsCaptor; + + @Before + public void before() { + command = spy(ExportDataCommand.class); + + doNothing().when(command).authorize(any(Resource.class), any(Operation.class), anyString()); + doReturn(mock(DistributedMember.class)).when(command).getMember(anyString()); + + CliFunctionResult okResult = + new CliFunctionResult("server1", CliFunctionResult.StatusState.OK, "exported"); + ResultCollector collector = mock(ResultCollector.class); + doReturn(Collections.singletonList(okResult)).when(collector).getResult(); + + functionArgsCaptor = ArgumentCaptor.forClass(Object.class); + doReturn(collector).when(command).executeFunction(any(), functionArgsCaptor.capture(), + any(DistributedMember.class)); + } + + private String capturedExportPath() { + Object args = functionArgsCaptor.getValue(); + assertThat(args).isInstanceOf(String[].class); + return ((String[]) args)[1]; + } + + private void verifyNoExportWasRequested() { + verify(command, never()).executeFunction(any(), any(), any(DistributedMember.class)); + } + + /** + * A "../" element in --file is refused before anything is sent to a member. + */ + @Test + public void parentReferenceInFileOptionIsRejected() { + parser + .executeAndAssertThat(command, "export data --member=server1 --region=" + REGION + + " --file=../../../../var/tmp/snapshot.gfd") + .statusIsError() + .containsOutput("must not contain a \"..\" path segment"); + + verifyNoExportWasRequested(); + } + + /** + * A "../" buried in the middle of an otherwise absolute --file is refused too - the check looks + * at every element of the path, not just its start. + */ + @Test + public void parentReferenceInsideAnAbsoluteFilePathIsRejected() { + parser + .executeAndAssertThat(command, "export data --member=server1 --region=" + REGION + + " --file=/var/tmp/subdir/../../snapshot.gfd") + .statusIsError() + .containsOutput("must not contain a \"..\" path segment"); + + verifyNoExportWasRequested(); + } + + /** + * The --dir option is checked as well, even though its file name is generated rather than + * supplied. + */ + @Test + public void parentReferenceInDirOptionIsRejected() { + parser + .executeAndAssertThat(command, + "export data --member=server1 --region=" + REGION + " --dir=/tmp/subdir/../../var/tmp") + .statusIsError() + .containsOutput("must not contain a \"..\" path segment"); + + verifyNoExportWasRequested(); + } + + /** + * An ordinary path is forwarded unchanged, for the member to resolve. + */ + @Test + public void ordinaryPathIsForwardedToTheMember() { + parser + .executeAndAssertThat(command, + "export data --member=server1 --region=" + REGION + " --file=/var/tmp/snapshot.gfd") + .statusIsSuccess(); + + assertThat(capturedExportPath()).isEqualTo("/var/tmp/snapshot.gfd"); + } + + /** + * Same for --dir, with the generated file name appended. + */ + @Test + public void ordinaryDirectoryIsForwardedToTheMember() { + parser + .executeAndAssertThat(command, + "export data --member=server1 --region=" + REGION + " --dir=/var/tmp") + .statusIsSuccess(); + + assertThat(capturedExportPath()).isEqualTo(new File("/var/tmp", REGION + ".gfd").getPath()); + } + + /** + * The extension check still applies. + */ + @Test + public void fileExtensionIsValidated() { + parser + .executeAndAssertThat(command, + "export data --member=server1 --region=" + REGION + " --file=/var/tmp/snapshot.txt") + .statusIsError() + .containsOutput("Invalid file type, the file extension must be \".gfd\""); + + verifyNoExportWasRequested(); + } + + /** + * Writing a file on a member's host needs a cluster write permission, alongside read access to + * the data being exported. + */ + @Test + public void exportRequiresClusterWriteAndDataRead() { + parser + .executeAndAssertThat(command, + "export data --member=server1 --region=" + REGION + " --file=/var/tmp/snapshot.gfd") + .statusIsSuccess(); + + verify(command).authorize(Resource.DATA, Operation.READ, REGION_PATH); + verify(command).authorize(Resource.CLUSTER, Operation.WRITE, ResourcePermission.ALL); + } + + /** + * Permissions are checked before any work is done, so the export is not sent to the member. + */ + @Test + public void clusterWriteIsCheckedBeforeTheExportIsSent() { + doThrow(new NotAuthorizedException("dataRead not authorized for CLUSTER:WRITE")) + .when(command).authorize(eq(Resource.CLUSTER), eq(Operation.WRITE), anyString()); + + assertThatThrownBy( + () -> command.exportData("server1", REGION_PATH, "/var/tmp/snapshot.gfd", null, false)) + .isInstanceOf(NotAuthorizedException.class) + .hasMessageContaining("CLUSTER:WRITE"); + + verifyNoExportWasRequested(); + } +} diff --git a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/functions/ExportDataFunctionPathValidationTest.java b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/functions/ExportDataFunctionPathValidationTest.java new file mode 100644 index 000000000000..23a2a89b41fe --- /dev/null +++ b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/functions/ExportDataFunctionPathValidationTest.java @@ -0,0 +1,248 @@ +/* + * 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.management.internal.cli.functions; + +import static org.apache.geode.management.internal.cli.functions.ExportDataFunction.EXPORT_DATA_DIRS_PROPERTY; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; +import org.junit.rules.TemporaryFolder; +import org.mockito.ArgumentCaptor; + +import org.apache.geode.cache.Region; +import org.apache.geode.cache.execute.FunctionContext; +import org.apache.geode.cache.snapshot.RegionSnapshotService; +import org.apache.geode.cache.snapshot.SnapshotOptions.SnapshotFormat; +import org.apache.geode.distributed.internal.InternalDistributedSystem; +import org.apache.geode.distributed.internal.membership.InternalDistributedMember; +import org.apache.geode.internal.cache.InternalCache; +import org.apache.geode.internal.cache.InternalCacheForClientAccess; +import org.apache.geode.management.internal.functions.CliFunctionResult; + +/** + * Tests the directories a member permits {@code export data} to write into. + */ +public class ExportDataFunctionPathValidationTest { + + private static final String REGION = "testRegion"; + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Rule + public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + + private ExportDataFunction function; + private RegionSnapshotService snapshotService; + private FunctionContext context; + + @Before + @SuppressWarnings("unchecked") + public void before() { + function = new ExportDataFunction(); + + snapshotService = mock(RegionSnapshotService.class); + Region region = mock(Region.class); + when(region.getSnapshotService()).thenReturn(snapshotService); + + InternalCacheForClientAccess clientCache = mock(InternalCacheForClientAccess.class); + when(clientCache.getRegion(REGION)).thenReturn(region); + + InternalCache cache = mock(InternalCache.class); + when(cache.getCacheForProcessingClientRequests()).thenReturn(clientCache); + + InternalDistributedMember member = mock(InternalDistributedMember.class); + when(member.getHost()).thenReturn("localhost"); + InternalDistributedSystem system = mock(InternalDistributedSystem.class); + when(system.getDistributedMember()).thenReturn(member); + when(clientCache.getDistributedSystem()).thenReturn(system); + + context = mock(FunctionContext.class); + when(context.getCache()).thenReturn(cache); + when(context.getMemberName()).thenReturn("server1"); + } + + /** Permits exports into the temporary folder, in addition to the working directory. */ + private Path permitTemporaryFolder() { + Path permitted = temporaryFolder.getRoot().toPath(); + System.setProperty(EXPORT_DATA_DIRS_PROPERTY, permitted.toString()); + return permitted; + } + + private CliFunctionResult export(String requestedPath) throws Exception { + when(context.getArguments()) + .thenReturn(new String[] {REGION, requestedPath, Boolean.toString(false)}); + return function.executeFunction(context); + } + + private File captureExportFile() throws Exception { + ArgumentCaptor fileCaptor = ArgumentCaptor.forClass(File.class); + verify(snapshotService).save(fileCaptor.capture(), eq(SnapshotFormat.GEODE)); + return fileCaptor.getValue(); + } + + private void verifyNothingWasWritten() throws Exception { + verify(snapshotService, never()).save(any(File.class), eq(SnapshotFormat.GEODE)); + } + + /** + * An export into a permitted directory works normally. + */ + @Test + public void exportIntoAPermittedDirectorySucceeds() throws Exception { + Path permitted = permitTemporaryFolder(); + + CliFunctionResult result = export(permitted.resolve("snapshot.gfd").toString()); + + assertThat(result.isSuccessful()).isTrue(); + assertThat(captureExportFile().toPath()) + .isEqualTo(permitted.toRealPath().resolve("snapshot.gfd")); + } + + /** Sub-directories of a permitted directory are permitted too. */ + @Test + public void exportIntoASubdirectoryOfAPermittedDirectorySucceeds() throws Exception { + Path permitted = permitTemporaryFolder(); + + CliFunctionResult result = export(permitted.resolve("nested/snapshot.gfd").toString()); + + assertThat(result.isSuccessful()).isTrue(); + } + + /** + * An absolute path outside every permitted directory is refused. + */ + @Test + public void exportToAnAbsolutePathOutsideEveryPermittedDirectoryIsRefused() throws Exception { + permitTemporaryFolder(); + + assertThatThrownBy(() -> export("/var/tmp/snapshot.gfd")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("not within the export directories configured for this member"); + + verifyNothingWasWritten(); + } + + /** + * A "../" element that climbs out of a permitted directory is refused: the path is canonicalized + * before it is compared, so the comparison uses the location actually written to. + */ + @Test + public void parentReferenceOutOfAPermittedDirectoryIsRefused() throws Exception { + Path permitted = permitTemporaryFolder(); + + assertThatThrownBy(() -> export(permitted.resolve("../escaped.gfd").toString())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("not within the export directories configured for this member"); + + verifyNothingWasWritten(); + } + + /** + * A "../" element that stays inside a permitted directory is honoured, and is resolved before + * the write, so no "../" reaches the snapshot service. + */ + @Test + public void parentReferenceInsideAPermittedDirectoryIsResolvedBeforeTheWrite() + throws Exception { + Path permitted = permitTemporaryFolder(); + + CliFunctionResult result = + export(permitted.resolve("nested/../snapshot.gfd").toString()); + + assertThat(result.isSuccessful()).isTrue(); + File exportFile = captureExportFile(); + assertThat(exportFile.getPath()).doesNotContain(".."); + assertThat(exportFile.toPath()).isEqualTo(permitted.toRealPath().resolve("snapshot.gfd")); + } + + /** + * With no configuration, the only permitted directory is the member's working directory - which + * is where a relative export path lands. + */ + @Test + public void withoutConfigurationOnlyTheMemberWorkingDirectoryIsPermitted() throws Exception { + System.clearProperty(EXPORT_DATA_DIRS_PROPERTY); + Path workingDir = Paths.get(System.getProperty("user.dir")).toRealPath(); + + assertThat(export(workingDir.resolve("snapshot.gfd").toString()).isSuccessful()).isTrue(); + + assertThatThrownBy(() -> export(temporaryFolder.getRoot().toPath().resolve("x.gfd").toString())) + .isInstanceOf(IllegalArgumentException.class); + } + + /** + * The member's working directory stays permitted when other directories are configured, so a + * relative export path keeps working. + */ + @Test + public void theWorkingDirectoryRemainsPermittedWhenOtherDirectoriesAreConfigured() + throws Exception { + permitTemporaryFolder(); + + assertThat(export("snapshot.gfd").isSuccessful()).isTrue(); + assertThat(captureExportFile().toPath()) + .isEqualTo(Paths.get(System.getProperty("user.dir")).toRealPath().resolve("snapshot.gfd")); + } + + /** + * More than one directory can be permitted, which is how a deployment that exports to a + * dedicated backup location configures the member. + */ + @Test + public void severalDirectoriesCanBePermitted() throws Exception { + File backup = temporaryFolder.newFolder("backup"); + File other = temporaryFolder.newFolder("other"); + System.setProperty(EXPORT_DATA_DIRS_PROPERTY, + backup.getAbsolutePath() + File.pathSeparator + other.getAbsolutePath()); + + assertThat(export(new File(backup, "snapshot.gfd").getPath()).isSuccessful()).isTrue(); + assertThat(export(new File(other, "snapshot.gfd").getPath()).isSuccessful()).isTrue(); + + assertThatThrownBy(() -> export(temporaryFolder.getRoot().toPath().resolve("x.gfd").toString())) + .isInstanceOf(IllegalArgumentException.class); + } + + /** + * A directory whose name merely starts with a permitted directory's name is not inside it - the + * check compares path elements, not string prefixes. + */ + @Test + public void aSiblingDirectoryWithAMatchingNamePrefixIsNotPermitted() throws Exception { + File permitted = temporaryFolder.newFolder("exports"); + File sibling = temporaryFolder.newFolder("exports-archive"); + System.setProperty(EXPORT_DATA_DIRS_PROPERTY, permitted.getAbsolutePath()); + + assertThatThrownBy(() -> export(new File(sibling, "snapshot.gfd").getPath())) + .isInstanceOf(IllegalArgumentException.class); + + verifyNothingWasWritten(); + } +} From 949f77d9ee620159f0206dd4458dc6ef7474b0a7 Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang <92374539+JinwooHwang@users.noreply.github.com> Date: Fri, 4 Sep 2026 07:23:14 -0400 Subject: [PATCH 8/9] GEODE-10624: Validate interest result policy message part before decoding (#8053) Read the register-interest policy part in the fixed-identifier form the client sends it in, and refuse a part in any other form before decoding. Adds unit and integration coverage for the policy part handling. --- ...sterInterestPolicyPartIntegrationTest.java | 162 ++++++++++++++++++ .../cache/tier/sockets/BaseCommand.java | 38 ++++ .../sockets/command/RegisterInterest61.java | 6 +- .../command/RegisterInterestList66.java | 2 +- .../RegisterInterestObjectPartTest.java | 107 ++++++++++++ .../command/RegisterInterest61Test.java | 7 +- .../command/RegisterInterestList66Test.java | 7 +- 7 files changed, 325 insertions(+), 4 deletions(-) create mode 100644 geode-core/src/integrationTest/java/org/apache/geode/cache/client/internal/RegisterInterestPolicyPartIntegrationTest.java create mode 100644 geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/RegisterInterestObjectPartTest.java diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/client/internal/RegisterInterestPolicyPartIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/client/internal/RegisterInterestPolicyPartIntegrationTest.java new file mode 100644 index 000000000000..639e0fd89deb --- /dev/null +++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/client/internal/RegisterInterestPolicyPartIntegrationTest.java @@ -0,0 +1,162 @@ +/* + * 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.cache.client.internal; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.Serializable; + +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import org.apache.geode.cache.DataPolicy; +import org.apache.geode.cache.RegionShortcut; +import org.apache.geode.cache.client.PoolFactory; +import org.apache.geode.cache.client.PoolManager; +import org.apache.geode.internal.cache.tier.InterestType; +import org.apache.geode.internal.cache.tier.MessageType; +import org.apache.geode.internal.cache.tier.sockets.ChunkedMessage; +import org.apache.geode.internal.cache.tier.sockets.Message; +import org.apache.geode.internal.serialization.KnownVersion; +import org.apache.geode.test.junit.categories.ClientServerTest; +import org.apache.geode.test.junit.rules.ServerStarterRule; + +/** + * Exercises, over a real client connection to a running server, how the register-interest command + * reads the message part that carries its interest result policy. + * + *

+ * A client op builds a register-interest request whose policy part carries a type other than the + * policy argument, and sends it. The helper type records whether an instance of it is created on + * the server while the part is read. The server must read the part only as its expected policy type + * and refuse a part carrying any other type. + */ +@Category({ClientServerTest.class}) +public class RegisterInterestPolicyPartIntegrationTest { + + private static final String REGION_NAME = "region"; + + @Rule + public ServerStarterRule server = + new ServerStarterRule().withRegion(RegionShortcut.REPLICATE, REGION_NAME).withAutoStart(); + + private PoolImpl pool; + + @Before + public void setUp() { + OtherPartType.reset(); + final PoolFactory poolFactory = PoolManager.createFactory(); + poolFactory.addServer("localhost", server.getPort()); + poolFactory.setReadTimeout(10_000); + poolFactory.setMinConnections(1); + pool = (PoolImpl) poolFactory.create("testPool"); + } + + @After + public void tearDown() { + if (pool != null) { + pool.destroy(); + } + } + + @Test + public void serverDoesNotProduceAnotherTypeFromThePolicyPart() { + try { + pool.execute(new PolicyPartOfAnotherTypeOp(REGION_NAME)); + } catch (final Exception ignored) { + // The request does not complete: the point of interest is which type the server produced + // while reading the part, which is recorded independently below. + } + + assertThat(OtherPartType.instantiated) + .as("reading the policy part must not produce a type other than the policy on the server") + .isFalse(); + } + + /** + * A register-interest request whose policy part carries a type other than the policy argument. + * Sends the request and does not attempt to interpret the response. + */ + private static class PolicyPartOfAnotherTypeOp extends AbstractOp { + + PolicyPartOfAnotherTypeOp(final String region) { + super(MessageType.REGISTER_INTEREST, 7); + getMessage().addStringPart(region, true); + getMessage().addIntPart(InterestType.KEY.ordinal()); + getMessage().addObjPart(new OtherPartType()); + getMessage().addBytesPart(new byte[] {(byte) 0x00}); + getMessage().addStringOrObjPart("key"); + getMessage().addBytesPart(new byte[] {(byte) 0x00}); + getMessage().addBytesPart(new byte[] {(byte) DataPolicy.REPLICATE.ordinal(), (byte) 0x01}); + } + + @Override + protected Message createResponseMessage() { + return new ChunkedMessage(1, KnownVersion.CURRENT); + } + + @Override + protected Object processResponse(final Message msg) throws Exception { + // Drain the whole response so this op does not return until the server has finished + // handling the request. + final ChunkedMessage chunkedMessage = (ChunkedMessage) msg; + chunkedMessage.readHeader(); + do { + chunkedMessage.receiveChunk(); + } while (!chunkedMessage.isLastChunk()); + return null; + } + + @Override + protected boolean isErrorResponse(final MessageType msgType) { + return false; + } + + @Override + protected long startAttempt(final ConnectionStats stats) { + return 0; + } + + @Override + protected void endSendAttempt(final ConnectionStats stats, final long start) {} + + @Override + protected void endAttempt(final ConnectionStats stats, final long start) {} + } + + /** + * A serializable type other than the register-interest policy argument. It records whether an + * instance of it is created, so a test can tell which type a part produced. + */ + public static class OtherPartType implements Serializable { + private static final long serialVersionUID = 1L; + + static volatile boolean instantiated = false; + + static void reset() { + instantiated = false; + } + + private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException { + in.defaultReadObject(); + instantiated = true; + } + } +} diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java index 9000a5503c00..8ca2c7daea8e 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommand.java @@ -82,6 +82,8 @@ import org.apache.geode.internal.offheap.OffHeapHelper; import org.apache.geode.internal.security.SecurityService; import org.apache.geode.internal.sequencelog.EntryLogger; +import org.apache.geode.internal.serialization.DSCODE; +import org.apache.geode.internal.serialization.DataSerializableFixedID; import org.apache.geode.logging.internal.log4j.api.LogService; import org.apache.geode.security.GemFireSecurityException; import org.apache.geode.util.internal.GeodeGlossary; @@ -92,6 +94,9 @@ public abstract class BaseCommand implements Command { @Immutable private static final byte[] OK_BYTES = new byte[] {0}; + /** Length of the serialized form of an interest result policy: code, identifier, ordinal. */ + private static final int INTEREST_RESULT_POLICY_FORM_LENGTH = 3; + public static final int MAXIMUM_CHUNK_SIZE = Integer.getInteger("BridgeServer.MAXIMUM_CHUNK_SIZE", 100); @@ -873,6 +878,39 @@ static Message readRequest(final @NotNull ServerConnection servConn) { return requestMsg; } + /** + * Reads the interest result policy carried by the given message part. + * + *

+ * The policy is written by the client in the fixed-identifier form of + * {@link InterestResultPolicy}. Only that form is accepted here, so the part is read as a policy + * and a part in any other form is refused. + * + * @param policyPart the message part holding the interest result policy + * @return the policy the part describes + * @throws IOException if the part is not in the expected form + */ + protected static @NotNull InterestResultPolicy readInterestResultPolicy( + final @NotNull Part policyPart) throws IOException, ClassNotFoundException { + if (!hasInterestResultPolicyForm(policyPart)) { + throw new IOException("The interest result policy part is not in the expected form."); + } + return (InterestResultPolicy) policyPart.getObject(); + } + + private static boolean hasInterestResultPolicyForm(final @NotNull Part policyPart) { + if (!policyPart.isObject()) { + return false; + } + final byte[] serializedForm = policyPart.getSerializedForm(); + return serializedForm != null + && serializedForm.length == INTEREST_RESULT_POLICY_FORM_LENGTH + && serializedForm[0] == DSCODE.DS_FIXED_ID_BYTE.toByte() + && serializedForm[1] == DataSerializableFixedID.INTEREST_RESULT_POLICY + && serializedForm[2] >= InterestResultPolicy.NONE.getOrdinal() + && serializedForm[2] <= InterestResultPolicy.KEYS_VALUES.getOrdinal(); + } + protected static void fillAndSendRegisterInterestResponseChunks( final @Nullable LocalRegion region, final @NotNull Object riKey, final @NotNull InterestType interestType, diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterest61.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterest61.java index 86984deb1640..b8953abf6250 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterest61.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterest61.java @@ -78,7 +78,7 @@ public void cmdExecute(final @NotNull Message clientMessage, final InterestResultPolicy policy; try { - policy = (InterestResultPolicy) clientMessage.getPart(2).getObject(); + policy = readInterestResultPolicy(clientMessage.getPart(2)); } catch (Exception e) { writeChunkedException(clientMessage, e, serverConnection); serverConnection.setAsTrue(RESPONDED); @@ -113,6 +113,10 @@ public void cmdExecute(final @NotNull Message clientMessage, Object key; try { final Part keyPart = clientMessage.getPart(4); + if (interestType == InterestType.REGULAR_EXPRESSION && keyPart.isObject()) { + throw new IOException( + "The key part of a regular expression request is not in the expected form."); + } key = keyPart.getStringOrObject(); } catch (Exception e) { writeChunkedException(clientMessage, e, serverConnection); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66.java index a64197eb80da..c4d06db5ad3a 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66.java @@ -79,7 +79,7 @@ public void cmdExecute(final @NotNull Message clientMessage, // Retrieve the InterestResultPolicy final InterestResultPolicy policy; try { - policy = (InterestResultPolicy) clientMessage.getPart(1).getObject(); + policy = readInterestResultPolicy(clientMessage.getPart(1)); } catch (Exception e) { writeChunkedException(clientMessage, e, serverConnection); serverConnection.setAsTrue(RESPONDED); diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/RegisterInterestObjectPartTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/RegisterInterestObjectPartTest.java new file mode 100644 index 000000000000..b439d0a6f44a --- /dev/null +++ b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/RegisterInterestObjectPartTest.java @@ -0,0 +1,107 @@ +/* + * 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.internal.cache.tier.sockets; + +import static org.apache.geode.internal.cache.tier.sockets.BaseCommand.readInterestResultPolicy; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; + +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.Serializable; + +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import org.apache.geode.cache.InterestResultPolicy; +import org.apache.geode.internal.util.BlobHelper; +import org.apache.geode.test.junit.categories.ClientServerTest; + +/** + * Verifies how the register-interest commands read the message {@link Part} that carries the + * interest result policy. + * + *

+ * The part is read as an {@link InterestResultPolicy}: it is accepted only in the form the client + * writes it in, and a part carrying any other type is refused and that type is not produced. The + * helper type below records whether an instance of it is created while a part is read. + */ +@Category({ClientServerTest.class}) +public class RegisterInterestObjectPartTest { + + @Before + public void setUp() { + OtherPartType.reset(); + } + + @Test + public void policyPartOfAnotherTypeIsRefusedWithoutProducingThatType() throws Exception { + final byte[] objectPartBytes = BlobHelper.serializeToBlob(new OtherPartType()); + + final Part part = new Part(); + part.setPartState(objectPartBytes, true); + + assertThat(catchThrowable(() -> readInterestResultPolicy(part))) + .as("a policy part holding another type is refused") + .isInstanceOf(IOException.class); + + assertThat(OtherPartType.instantiated) + .as("reading the policy part must not produce a type other than the policy") + .isFalse(); + } + + @Test + public void nonObjectPolicyPartIsRefused() { + final Part part = new Part(); + part.setPartState(new byte[] {0x01, 0x25, 0x02}, false); + + assertThat(catchThrowable(() -> readInterestResultPolicy(part))) + .as("a policy part that is not object typed is refused") + .isInstanceOf(IOException.class); + } + + @Test + public void eachPolicyValueRoundTripsThroughThePart() throws Exception { + for (final InterestResultPolicy expected : new InterestResultPolicy[] { + InterestResultPolicy.NONE, InterestResultPolicy.KEYS, InterestResultPolicy.KEYS_VALUES}) { + final Part part = new Part(); + part.setPartState(BlobHelper.serializeToBlob(expected), true); + + assertThat(readInterestResultPolicy(part)) + .as("policy %s survives a write and read of the policy part", expected) + .isSameAs(expected); + } + } + + /** + * A serializable type other than the register-interest policy argument. It records whether an + * instance of it is created, so a test can tell which type a part produced. + */ + public static class OtherPartType implements Serializable { + private static final long serialVersionUID = 1L; + + static volatile boolean instantiated = false; + + static void reset() { + instantiated = false; + } + + private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException { + in.defaultReadObject(); + instantiated = true; + } + } +} diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterest61Test.java b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterest61Test.java index 6559f856cef5..cfec6dbf98fd 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterest61Test.java +++ b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterest61Test.java @@ -33,6 +33,7 @@ import org.mockito.MockitoAnnotations; import org.apache.geode.CancelCriterion; +import org.apache.geode.cache.InterestResultPolicy; import org.apache.geode.cache.operations.RegisterInterestOperationContext; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.internal.cache.LocalRegion; @@ -46,6 +47,7 @@ import org.apache.geode.internal.security.AuthorizeRequest; import org.apache.geode.internal.security.SecurityService; import org.apache.geode.internal.serialization.KnownVersion; +import org.apache.geode.internal.util.BlobHelper; import org.apache.geode.security.NotAuthorizedException; import org.apache.geode.security.ResourcePermission.Operation; import org.apache.geode.security.ResourcePermission.Resource; @@ -98,6 +100,9 @@ public void setUp() throws Exception { when(cache.getRegion(isA(String.class))).thenReturn(uncheckedCast(mock(LocalRegion.class))); when(cache.getCancelCriterion()).thenReturn(mock(CancelCriterion.class)); + final Part policyPart = new Part(); + policyPart.setPartState(BlobHelper.serializeToBlob(InterestResultPolicy.KEYS_VALUES), true); + when(durablePart.getObject()).thenReturn(DURABLE); when(interestTypePart.getInt()).thenReturn(0); @@ -107,7 +112,7 @@ public void setUp() throws Exception { when(message.getNumberOfParts()).thenReturn(6); when(message.getPart(eq(0))).thenReturn(regionNamePart); when(message.getPart(eq(1))).thenReturn(interestTypePart); - when(message.getPart(eq(2))).thenReturn(mock(Part.class)); + when(message.getPart(eq(2))).thenReturn(policyPart); when(message.getPart(eq(3))).thenReturn(durablePart); when(message.getPart(eq(4))).thenReturn(keyPart); when(message.getPart(eq(5))).thenReturn(notifyPart); diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66Test.java b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66Test.java index a3a6a0f6f501..3d4f1fc34eb6 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66Test.java +++ b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/RegisterInterestList66Test.java @@ -35,6 +35,7 @@ import org.mockito.MockitoAnnotations; import org.apache.geode.CancelCriterion; +import org.apache.geode.cache.InterestResultPolicy; import org.apache.geode.cache.operations.RegisterInterestOperationContext; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.internal.cache.LocalRegion; @@ -47,6 +48,7 @@ import org.apache.geode.internal.security.AuthorizeRequest; import org.apache.geode.internal.security.SecurityService; import org.apache.geode.internal.serialization.KnownVersion; +import org.apache.geode.internal.util.BlobHelper; import org.apache.geode.security.NotAuthorizedException; import org.apache.geode.security.ResourcePermission.Operation; import org.apache.geode.security.ResourcePermission.Resource; @@ -103,6 +105,9 @@ public void setUp() throws Exception { when(cache.getRegion(isA(String.class))).thenReturn(uncheckedCast(mock(LocalRegion.class))); when(cache.getCancelCriterion()).thenReturn(mock(CancelCriterion.class)); + final Part policyPart = new Part(); + policyPart.setPartState(BlobHelper.serializeToBlob(InterestResultPolicy.KEYS_VALUES), true); + when(durablePart.getObject()).thenReturn(DURABLE); when(interestTypePart.getInt()).thenReturn(0); @@ -111,7 +116,7 @@ public void setUp() throws Exception { when(message.getNumberOfParts()).thenReturn(6); when(message.getPart(eq(0))).thenReturn(regionNamePart); - when(message.getPart(eq(1))).thenReturn(interestTypePart); + when(message.getPart(eq(1))).thenReturn(policyPart); when(message.getPart(eq(2))).thenReturn(durablePart); when(message.getPart(eq(3))).thenReturn(keyPart); when(message.getPart(eq(4))).thenReturn(notifyPart); From f6b0709bbfcde9f5a59f8ddce40f7bfbbcf6894e Mon Sep 17 00:00:00 2001 From: Jinwoo Hwang <92374539+JinwooHwang@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:22:13 -0400 Subject: [PATCH 9/9] [GEODE-10623] Remediation of CVE-2026-64607: HttpClient5 5.4.4 to 5.6.4 (#8050) --- .../plugins/DependencyConstraints.groovy | 2 +- .../resources/assembly_content.txt | 2 +- .../resources/gfsh_dependency_classpath.txt | 2 +- ...lateClusterManagementServiceTransport.java | 31 ++++++++++++++++--- .../resources/dependency_classpath.txt | 2 +- 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy b/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy index a21a6e558fdb..483208269cae 100644 --- a/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy +++ b/build-tools/geode-dependency-management/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy @@ -181,7 +181,7 @@ class DependencyConstraints { api(group: 'org.apache.commons', name: 'commons-text', version: 1.9) api(group: 'org.apache.derby', name: 'derby', version: '10.14.2.0') // Apache HttpComponents 5.x - Modern HTTP client with HTTP/2 support - api(group: 'org.apache.httpcomponents.client5', name: 'httpclient5', version: '5.4.4') + api(group: 'org.apache.httpcomponents.client5', name: 'httpclient5', version: '5.6.4') api(group: 'org.apache.httpcomponents.core5', name: 'httpcore5', version: '5.4.3') api(group: 'org.apache.httpcomponents.core5', name: 'httpcore5-h2', version: '5.4.3') // Legacy HttpComponents 4.x (keep temporarily during migration, remove after complete) diff --git a/geode-assembly/src/integrationTest/resources/assembly_content.txt b/geode-assembly/src/integrationTest/resources/assembly_content.txt index 2f88d6a0f2c8..c2c6ae29523f 100644 --- a/geode-assembly/src/integrationTest/resources/assembly_content.txt +++ b/geode-assembly/src/integrationTest/resources/assembly_content.txt @@ -960,7 +960,7 @@ lib/geode-unsafe-0.0.0.jar lib/geode-wan-0.0.0.jar lib/gfsh-dependencies.jar lib/hibernate-validator-8.0.2.Final.jar -lib/httpclient5-5.4.4.jar +lib/httpclient5-5.6.4.jar lib/httpcore5-5.4.3.jar lib/httpcore5-h2-5.4.3.jar lib/istack-commons-runtime-4.1.1.jar diff --git a/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt b/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt index 8a26d0186bf1..17273bef851c 100644 --- a/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt +++ b/geode-assembly/src/integrationTest/resources/gfsh_dependency_classpath.txt @@ -60,7 +60,7 @@ lucene-analysis-common-9.12.3.jar lucene-queryparser-9.12.3.jar lucene-queries-9.12.3.jar lucene-core-9.12.3.jar -httpclient5-5.4.4.jar +httpclient5-5.6.4.jar httpcore5-h2-5.4.3.jar httpcore5-5.4.3.jar HikariCP-4.0.3.jar diff --git a/geode-management/src/main/java/org/apache/geode/management/api/RestTemplateClusterManagementServiceTransport.java b/geode-management/src/main/java/org/apache/geode/management/api/RestTemplateClusterManagementServiceTransport.java index dcb01f468945..6381001215fb 100644 --- a/geode-management/src/main/java/org/apache/geode/management/api/RestTemplateClusterManagementServiceTransport.java +++ b/geode-management/src/main/java/org/apache/geode/management/api/RestTemplateClusterManagementServiceTransport.java @@ -25,12 +25,14 @@ import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; +import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.io.HttpClientConnectionManager; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; +import org.apache.hc.client5.http.ssl.HostnameVerificationPolicy; import org.springframework.core.io.FileSystemResource; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; @@ -164,13 +166,13 @@ public void configureConnection(ConnectionConfig connectionConfig) { // Configure SSL context and hostname verifier (HttpClient 5.x approach) // Only configure SSL if we have a non-null SSL context if (connectionConfig.getSslContext() != null) { - SSLConnectionSocketFactory sslSocketFactory = new SSLConnectionSocketFactory( + DefaultClientTlsStrategy sslSocketFactory = createTlsStrategy( connectionConfig.getSslContext(), connectionConfig.getHostnameVerifier()); HttpClientConnectionManager connectionManager = PoolingHttpClientConnectionManagerBuilder.create() - .setSSLSocketFactory(sslSocketFactory) + .setTlsSocketStrategy(sslSocketFactory) .build(); clientBuilder.setConnectionManager(connectionManager); @@ -178,13 +180,13 @@ public void configureConnection(ConnectionConfig connectionConfig) { // If only hostname verifier is set without SSL context, we need to use the default SSL // context try { - SSLConnectionSocketFactory sslSocketFactory = new SSLConnectionSocketFactory( + DefaultClientTlsStrategy sslSocketFactory = createTlsStrategy( SSLContext.getDefault(), connectionConfig.getHostnameVerifier()); HttpClientConnectionManager connectionManager = PoolingHttpClientConnectionManagerBuilder.create() - .setSSLSocketFactory(sslSocketFactory) + .setTlsSocketStrategy(sslSocketFactory) .build(); clientBuilder.setConnectionManager(connectionManager); @@ -197,6 +199,25 @@ public void configureConnection(ConnectionConfig connectionConfig) { restTemplate.setRequestFactory(requestFactory); } + /** + * Builds the TLS strategy used for HTTPS connections. + * + *

+ * When the caller supplies a {@link HostnameVerifier}, that verifier alone decides whether the + * peer's certificate matches the endpoint, so the strategy is created with + * {@link HostnameVerificationPolicy#CLIENT}. Without an explicit verifier the strategy keeps the + * library's own endpoint identification. + *

+ */ + private static DefaultClientTlsStrategy createTlsStrategy(SSLContext sslContext, + HostnameVerifier hostnameVerifier) { + if (hostnameVerifier == null) { + return new DefaultClientTlsStrategy(sslContext); + } + return new DefaultClientTlsStrategy(sslContext, HostnameVerificationPolicy.CLIENT, + hostnameVerifier); + } + @Override public > ClusterManagementRealizationResult submitMessage( T configMessage, CommandType command) { diff --git a/geode-server-all/src/integrationTest/resources/dependency_classpath.txt b/geode-server-all/src/integrationTest/resources/dependency_classpath.txt index d0c9eccab8aa..fe2869ac30c4 100644 --- a/geode-server-all/src/integrationTest/resources/dependency_classpath.txt +++ b/geode-server-all/src/integrationTest/resources/dependency_classpath.txt @@ -24,7 +24,7 @@ jackson-dataformat-yaml-2.21.6.jar jackson-core-2.21.6.jar jackson-datatype-joda-2.21.6.jar jackson-databind-2.21.6.jar -httpclient5-5.4.4.jar +httpclient5-5.6.4.jar httpcore5-h2-5.4.3.jar httpcore5-5.4.3.jar HikariCP-4.0.3.jar