From d9faa377aaed594848f43e7b73018d17906545ab Mon Sep 17 00:00:00 2001 From: JinwooHwang Date: Wed, 21 Oct 2026 13:39:03 -0400 Subject: [PATCH] GEODE-10679: Refactor access checks in MBeanServerWrapper --- .../security/MBeanServerWrapperJUnitTest.java | 118 ++++++++++++++++++ .../internal/security/MBeanServerWrapper.java | 14 +++ 2 files changed, 132 insertions(+) create mode 100644 geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/MBeanServerWrapperJUnitTest.java diff --git a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/MBeanServerWrapperJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/MBeanServerWrapperJUnitTest.java new file mode 100644 index 000000000000..ffa5feb0c1f2 --- /dev/null +++ b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/MBeanServerWrapperJUnitTest.java @@ -0,0 +1,118 @@ +/* + * 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.security; + +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import javax.management.MBeanServer; +import javax.management.MBeanServerConnection; +import javax.management.ObjectName; + +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import org.apache.geode.management.internal.MBeanJMXAdapter; +import org.apache.geode.security.TestSecurityManager; +import org.apache.geode.test.junit.categories.SecurityTest; +import org.apache.geode.test.junit.rules.ConnectionConfiguration; +import org.apache.geode.test.junit.rules.MBeanServerConnectionRule; +import org.apache.geode.test.junit.rules.ServerStarterRule; + +/** + * Exercises MBean creation, registration and invocation through the JMX connector for users with + * different resource permissions, drawn from {@code cacheServer.json}. + */ +@Category({SecurityTest.class}) +public class MBeanServerWrapperJUnitTest { + + private static final String DISTRIBUTED_SYSTEM_MBEAN = + "GemFire:service=System,type=Distributed"; + + private static final ObjectName OTHER_DOMAIN_BEAN = objectName("other:type=timer"); + + @ClassRule + public static ServerStarterRule server = new ServerStarterRule().withJMXManager() + .withProperty(SECURITY_MANAGER, TestSecurityManager.class.getName()) + .withProperty(TestSecurityManager.SECURITY_JSON, + "org/apache/geode/management/internal/security/cacheServer.json") + .withAutoStart(); + + @Rule + public MBeanServerConnectionRule connectionRule = + new MBeanServerConnectionRule(server::getJmxPort); + + private static ObjectName objectName(String name) { + try { + return new ObjectName(name); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + @BeforeClass + public static void registerBeanForInvocation() throws Exception { + MBeanServer platform = MBeanJMXAdapter.mbeanServer; + if (!platform.isRegistered(OTHER_DOMAIN_BEAN)) { + platform.createMBean("javax.management.timer.Timer", OTHER_DOMAIN_BEAN); + } + } + + @Test + @ConnectionConfiguration(user = "stranger", password = "1234567") + public void memberCountReadRequiresClusterRead() throws Exception { + MBeanServerConnection con = connectionRule.getMBeanServerConnection(); + assertThatThrownBy( + () -> con.getAttribute(new ObjectName(DISTRIBUTED_SYSTEM_MBEAN), "MemberCount")) + .hasMessageContaining("CLUSTER:READ"); + } + + @Test + @ConnectionConfiguration(user = "cluster-reader", password = "1234567") + public void clusterReaderCanReadMemberCount() throws Exception { + MBeanServerConnection con = connectionRule.getMBeanServerConnection(); + Object memberCount = con.getAttribute(new ObjectName(DISTRIBUTED_SYSTEM_MBEAN), "MemberCount"); + assertThat(((Number) memberCount).intValue()).isGreaterThanOrEqualTo(1); + } + + @Test + @ConnectionConfiguration(user = "stranger", password = "1234567") + public void createMBeanInOtherDomainRequiresClusterManage() throws Exception { + MBeanServerConnection con = connectionRule.getMBeanServerConnection(); + ObjectName name = objectName("other:type=created"); + assertThatThrownBy(() -> con.createMBean("javax.management.timer.Timer", name)) + .satisfiesAnyOf( + t -> assertThat(t).isInstanceOf(SecurityException.class), + t -> assertThat(t).hasMessageContaining("not authorized"), + t -> assertThat(t).hasMessageContaining("Access Denied")); + } + + @Test + @ConnectionConfiguration(user = "stranger", password = "1234567") + public void invokeInOtherDomainRequiresClusterManage() throws Exception { + MBeanServerConnection con = connectionRule.getMBeanServerConnection(); + assertThatThrownBy( + () -> con.invoke(OTHER_DOMAIN_BEAN, "removeAllNotifications", new Object[] {}, + new String[] {})) + .satisfiesAnyOf( + t -> assertThat(t).isInstanceOf(SecurityException.class), + t -> assertThat(t).hasMessageContaining("not authorized"), + t -> assertThat(t).hasMessageContaining("Access Denied")); + } +} diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java b/geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java index 9c5cccba0504..688dcfee43d9 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java @@ -78,10 +78,15 @@ private void checkDomain(ObjectName name) { } } + private void checkClusterManageResource() { + securityService.authorize(new ResourcePermission(Resource.CLUSTER, Operation.MANAGE)); + } + @Override public ObjectInstance createMBean(String className, ObjectName name) throws ReflectionException, InstanceAlreadyExistsException, MBeanException, NotCompliantMBeanException { checkDomain(name); + checkClusterManageResource(); return mbs.createMBean(className, name); } @@ -90,6 +95,7 @@ public ObjectInstance createMBean(String className, ObjectName name, ObjectName throws ReflectionException, InstanceAlreadyExistsException, MBeanException, NotCompliantMBeanException, InstanceNotFoundException { checkDomain(name); + checkClusterManageResource(); return mbs.createMBean(className, name, loaderName); } @@ -98,6 +104,7 @@ public ObjectInstance createMBean(String className, ObjectName name, Object[] pa String[] signature) throws ReflectionException, InstanceAlreadyExistsException, MBeanException, NotCompliantMBeanException { checkDomain(name); + checkClusterManageResource(); return mbs.createMBean(className, name, params, signature); } @@ -107,6 +114,7 @@ public ObjectInstance createMBean(String className, ObjectName name, ObjectName throws ReflectionException, InstanceAlreadyExistsException, MBeanException, NotCompliantMBeanException, InstanceNotFoundException { checkDomain(name); + checkClusterManageResource(); return mbs.createMBean(className, name, loaderName, params, signature); } @@ -115,6 +123,7 @@ public ObjectInstance registerMBean(Object object, ObjectName name) throws InstanceAlreadyExistsException, MBeanRegistrationException, NotCompliantMBeanException { checkDomain(name); + checkClusterManageResource(); return mbs.registerMBean(object, name); } @@ -122,6 +131,7 @@ public ObjectInstance registerMBean(Object object, ObjectName name) public void unregisterMBean(ObjectName name) throws InstanceNotFoundException, MBeanRegistrationException { checkDomain(name); + checkClusterManageResource(); mbs.unregisterMBean(name); } @@ -235,6 +245,10 @@ public Object invoke(ObjectName name, String operationName, Object[] params, Str throws InstanceNotFoundException, MBeanException, ReflectionException { ResourcePermission ctx = getOperationContext(name, operationName, true); + if (ctx == null + && !ManagementConstants.OBJECTNAME__DEFAULTDOMAIN.equals(name.getDomain())) { + ctx = new ResourcePermission(Resource.CLUSTER, Operation.MANAGE); + } securityService.authorize(ctx); return mbs.invoke(name, operationName, params, signature);