From 7eb22c18db4bd81ef79c619173579cb54442fa9e Mon Sep 17 00:00:00 2001 From: Domenico Francesco Bruscino Date: Sat, 28 Nov 2020 23:10:05 +0100 Subject: [PATCH] ARTEMIS-3014 Fix JMX RBAC guard --- .../cli/factory/jmx/ManagementFactory.java | 2 + .../activemq/artemis/util/ServerUtil.java | 12 +- .../server/management/ManagementContext.java | 18 ++- tests/smoke-tests/pom.xml | 20 +++ .../resources/servers/jmx-rbac/management.xml | 52 +++++++ .../tests/smoke/jmxrbac/JmxRBACTest.java | 137 ++++++++++++++++++ 6 files changed, 231 insertions(+), 10 deletions(-) create mode 100644 tests/smoke-tests/src/main/resources/servers/jmx-rbac/management.xml create mode 100644 tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/jmxrbac/JmxRBACTest.java diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/factory/jmx/ManagementFactory.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/factory/jmx/ManagementFactory.java index 826dc8efb3..b41a1ef831 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/factory/jmx/ManagementFactory.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/factory/jmx/ManagementFactory.java @@ -134,6 +134,8 @@ public class ManagementFactory { context.setSecurityManager(securityManager); } + context.init(); + return context; } } diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/util/ServerUtil.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/util/ServerUtil.java index 7c8675b0b2..6d7cf8f5a0 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/util/ServerUtil.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/util/ServerUtil.java @@ -105,14 +105,22 @@ public class ServerUtil { } public static boolean waitForServerToStart(int id, int timeout) throws InterruptedException { - return waitForServerToStart("tcp://localhost:" + (61616 + id), timeout); + return waitForServerToStart(id, null, null, timeout); + } + + public static boolean waitForServerToStart(int id, String username, String password, int timeout) throws InterruptedException { + return waitForServerToStart("tcp://localhost:" + (61616 + id), username, password, timeout); } public static boolean waitForServerToStart(String uri, long timeout) throws InterruptedException { + return waitForServerToStart(uri, null, null, timeout); + } + + public static boolean waitForServerToStart(String uri, String username, String password, long timeout) throws InterruptedException { long realTimeout = System.currentTimeMillis() + timeout; while (System.currentTimeMillis() < realTimeout) { try (ActiveMQConnectionFactory cf = ActiveMQJMSClient.createConnectionFactory(uri, null)) { - cf.createConnection().close(); + cf.createConnection(username, password).close(); System.out.println("server " + uri + " started"); } catch (Exception e) { System.out.println("awaiting server " + uri + " start at "); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ManagementContext.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ManagementContext.java index f518c2da30..db308acdfc 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ManagementContext.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/ManagementContext.java @@ -34,6 +34,16 @@ public class ManagementContext implements ServiceComponent { private ArtemisMBeanServerGuard guardHandler; private ActiveMQSecurityManager securityManager; + public void init() { + if (accessControlList != null) { + //if we are configured then assume we want to use the guard so set the system property + System.setProperty("javax.management.builder.initial", ArtemisMBeanServerBuilder.class.getCanonicalName()); + guardHandler = new ArtemisMBeanServerGuard(); + guardHandler.setJMXAccessControlList(accessControlList); + ArtemisMBeanServerBuilder.setGuard(guardHandler); + } + } + @Override public void start() throws Exception { if (isStarted) { @@ -44,14 +54,6 @@ public class ManagementContext implements ServiceComponent { return; } isStarted = true; - if (accessControlList != null) { - //if we are configured then assume we want to use the guard so set the system property - System.setProperty("javax.management.builder.initial", ArtemisMBeanServerBuilder.class.getCanonicalName()); - guardHandler = new ArtemisMBeanServerGuard(); - guardHandler.setJMXAccessControlList(accessControlList); - ArtemisMBeanServerBuilder.setGuard(guardHandler); - } - if (jmxConnectorConfiguration != null) { mBeanServer = new ManagementConnector(jmxConnectorConfiguration, securityManager); mBeanServer.start(); diff --git a/tests/smoke-tests/pom.xml b/tests/smoke-tests/pom.xml index 5bf24ce546..3c5bd438dd 100644 --- a/tests/smoke-tests/pom.xml +++ b/tests/smoke-tests/pom.xml @@ -404,6 +404,26 @@ + + test-compile + create-createJMXRBAC + + create + + + amq + admin + admin + false + ${basedir}/target/jmx-rbac + ${basedir}/target/classes/servers/jmx-rbac + + + --java-options + -Djava.rmi.server.hostname=localhost + + + test-compile create-createAuditLogging diff --git a/tests/smoke-tests/src/main/resources/servers/jmx-rbac/management.xml b/tests/smoke-tests/src/main/resources/servers/jmx-rbac/management.xml new file mode 100644 index 0000000000..44e491eace --- /dev/null +++ b/tests/smoke-tests/src/main/resources/servers/jmx-rbac/management.xml @@ -0,0 +1,52 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/jmxrbac/JmxRBACTest.java b/tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/jmxrbac/JmxRBACTest.java new file mode 100644 index 0000000000..c25ecedf16 --- /dev/null +++ b/tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/jmxrbac/JmxRBACTest.java @@ -0,0 +1,137 @@ +/** + * 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.activemq.artemis.tests.smoke.jmxrbac; + +import javax.management.MBeanServerConnection; +import javax.management.MBeanServerInvocationHandler; +import javax.management.ObjectName; +import javax.management.remote.JMXConnector; +import javax.management.remote.JMXConnectorFactory; +import javax.management.remote.JMXServiceURL; + +import java.util.Collections; + +import org.apache.activemq.artemis.api.config.ActiveMQDefaultConfiguration; +import org.apache.activemq.artemis.api.core.management.ActiveMQServerControl; +import org.apache.activemq.artemis.api.core.management.ObjectNameBuilder; +import org.apache.activemq.artemis.tests.smoke.common.SmokeTestBase; +import org.apache.activemq.artemis.util.ServerUtil; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +public class JmxRBACTest extends SmokeTestBase { + // This test will use a smoke created by the pom on this project (smoke-tsts) + + private static final String JMX_SERVER_HOSTNAME = "localhost"; + private static final int JMX_SERVER_PORT = 10099; + + public static final String SERVER_NAME_0 = "jmx-rbac"; + + public static final String SERVER_ADMIN = "admin"; + public static final String SERVER_USER = "user"; + + + @Before + public void before() throws Exception { + cleanupData(SERVER_NAME_0); + disableCheckThread(); + startServer(SERVER_NAME_0, 0, 0); + ServerUtil.waitForServerToStart(0, SERVER_ADMIN, SERVER_ADMIN, 30000); + } + + @Test + public void testManagementRoleAccess() throws Exception { + // Without this, the RMI server would bind to the default interface IP (the user's local IP mostly) + System.setProperty("java.rmi.server.hostname", JMX_SERVER_HOSTNAME); + + // I don't specify both ports here manually on purpose. See actual RMI registry connection port extraction below. + String urlString = "service:jmx:rmi:///jndi/rmi://" + JMX_SERVER_HOSTNAME + ":" + JMX_SERVER_PORT + "/jmxrmi"; + + JMXServiceURL url = new JMXServiceURL(urlString); + JMXConnector jmxConnector; + + try { + //Connect using the admin. + jmxConnector = JMXConnectorFactory.connect(url, Collections.singletonMap( + "jmx.remote.credentials", new String[] {SERVER_ADMIN, SERVER_ADMIN})); + System.out.println("Successfully connected to: " + urlString); + } catch (Exception e) { + jmxConnector = null; + e.printStackTrace(); + Assert.fail(e.getMessage()); + } + + try { + //Create an user. + MBeanServerConnection mBeanServerConnection = jmxConnector.getMBeanServerConnection(); + String brokerName = "0.0.0.0"; // configured e.g. in broker.xml element + ObjectNameBuilder objectNameBuilder = ObjectNameBuilder.create(ActiveMQDefaultConfiguration.getDefaultJmxDomain(), brokerName, true); + ActiveMQServerControl activeMQServerControl = MBeanServerInvocationHandler.newProxyInstance(mBeanServerConnection, objectNameBuilder.getActiveMQServerObjectName(), ActiveMQServerControl.class, false); + ObjectName memoryObjectName = new ObjectName("java.lang:type=Memory"); + + try { + activeMQServerControl.removeUser(SERVER_USER); + } catch (Exception ignore) { + } + activeMQServerControl.addUser(SERVER_USER, SERVER_USER, "amq-user", true); + + activeMQServerControl.getVersion(); + + try { + mBeanServerConnection.invoke(memoryObjectName, "gc", null, null); + Assert.fail(SERVER_ADMIN + " should not access to " + memoryObjectName); + } catch (Exception e) { + Assert.assertEquals(SecurityException.class, e.getClass()); + } + } finally { + jmxConnector.close(); + } + + try { + //Connect using an user. + jmxConnector = JMXConnectorFactory.connect(url, Collections.singletonMap( + "jmx.remote.credentials", new String[] {SERVER_USER, SERVER_USER})); + System.out.println("Successfully connected to: " + urlString); + } catch (Exception e) { + jmxConnector = null; + e.printStackTrace(); + Assert.fail(e.getMessage()); + } + + + try { + MBeanServerConnection mBeanServerConnection = jmxConnector.getMBeanServerConnection(); + String brokerName = "0.0.0.0"; // configured e.g. in broker.xml element + ObjectNameBuilder objectNameBuilder = ObjectNameBuilder.create(ActiveMQDefaultConfiguration.getDefaultJmxDomain(), brokerName, true); + ActiveMQServerControl activeMQServerControl = MBeanServerInvocationHandler.newProxyInstance(mBeanServerConnection, objectNameBuilder.getActiveMQServerObjectName(), ActiveMQServerControl.class, false); + ObjectName memoryObjectName = new ObjectName("java.lang:type=Memory"); + + mBeanServerConnection.invoke(memoryObjectName, "gc", null, null); + + try { + activeMQServerControl.getVersion(); + Assert.fail(SERVER_USER + " should not access to " + objectNameBuilder.getActiveMQServerObjectName()); + } catch (Exception e) { + Assert.assertEquals(SecurityException.class, e.getClass()); + } + } finally { + jmxConnector.close(); + } + } +}