This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch karaf-4.4.x
in repository https://gitbox.apache.org/repos/asf/karaf.git
The following commit(s) were added to refs/heads/karaf-4.4.x by this push:
new 6c8c15672a [4.4.x] Apply the JMX RBAC guard to MBean lifecycle
operations (#2866)
6c8c15672a is described below
commit 6c8c15672a55116e701c489f1a2015f842b1789d
Author: JB Onofré <[email protected]>
AuthorDate: Fri Sep 11 14:53:44 2026 +0200
[4.4.x] Apply the JMX RBAC guard to MBean lifecycle operations (#2866)
* fix(management): apply the JMX RBAC guard to MBean lifecycle operations
The KarafMBeanServerGuard only intercepted invoke, getAttribute(s) and
setAttribute(s). createMBean, registerMBean and unregisterMBean went
straight to the delegate MBeanServer, so they were not subject to any
role check and behaved inconsistently with the rest of the guarded API.
Route these three operations through the guard as well:
* MBeanInvocationHandler adds them to the guarded operation list.
* KarafMBeanServerGuard resolves the required roles from the jmx.acl*
configuration by ObjectName and operation name (the target MBean is
not, or not yet, registered so no MBeanInfo introspection is done).
For createMBean/registerMBean the class name is passed as the
operation argument so ACL rules can match on it, e.g.
createMBean(java.lang.String)[/javax\.management\.loading\..*/] = admin
* A null ObjectName falls back to the generic jmx.acl configuration.
* The default jmx.acl.cfg maps createMBean/registerMBean/unregisterMBean
to the admin role (the existing "* = admin" already covered them).
(cherry picked from commit 87c14269fe561a5289b48d12df54da61eca49535)
* Fix Javadoc comment prematurely terminated by regex example
The ACL regex example in the handleRegistration Javadoc contained the
literal sequence */ (from \..*/]), which closed the block comment early
and broke compilation of org.apache.karaf.management.server. Escape the
trailing slash as / so the comment terminates where intended.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_017MZQYyrYzSQHdHeD6AEGXT
---------
Co-authored-by: Jean-Baptiste Onofré <[email protected]>
Co-authored-by: Claude Sonnet 5 <[email protected]>
---
.../features/standard/src/main/feature/feature.xml | 5 +
.../karaf/instance/resources/etc/jmx.acl.cfg | 5 +
.../karaf/management/KarafMBeanServerGuard.java | 86 +++++++++++++-
.../internal/MBeanInvocationHandler.java | 2 +-
.../management/KarafMBeanServerGuardTest.java | 129 +++++++++++++++++++++
.../src/main/asciidoc/user-guide/monitoring.adoc | 11 ++
6 files changed, 231 insertions(+), 7 deletions(-)
diff --git a/assemblies/features/standard/src/main/feature/feature.xml
b/assemblies/features/standard/src/main/feature/feature.xml
index c52c66c891..3d9d474aca 100644
--- a/assemblies/features/standard/src/main/feature/feature.xml
+++ b/assemblies/features/standard/src/main/feature/feature.xml
@@ -1088,6 +1088,11 @@ list* = viewer
get* = viewer
is* = viewer
set* = admin
+# MBean lifecycle operations (creating, registering and unregistering MBeans)
are
+# restricted to the admin role.
+createMBean = admin
+registerMBean = admin
+unregisterMBean = admin
* = admin
</config>
<config name="jmx.acl.org.apache.karaf.security.jmx">
diff --git
a/instance/src/main/resources/org/apache/karaf/instance/resources/etc/jmx.acl.cfg
b/instance/src/main/resources/org/apache/karaf/instance/resources/etc/jmx.acl.cfg
index b4899bd7e9..b6e47d751e 100644
---
a/instance/src/main/resources/org/apache/karaf/instance/resources/etc/jmx.acl.cfg
+++
b/instance/src/main/resources/org/apache/karaf/instance/resources/etc/jmx.acl.cfg
@@ -70,4 +70,9 @@ list* = viewer
get* = viewer
is* = viewer
set* = admin
+# MBean lifecycle operations (creating, registering and unregistering MBeans)
are
+# restricted to the admin role.
+createMBean = admin
+registerMBean = admin
+unregisterMBean = admin
* = admin
\ No newline at end of file
diff --git
a/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java
b/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java
index d567f544a6..405cbbad94 100644
---
a/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java
+++
b/management/server/src/main/java/org/apache/karaf/management/KarafMBeanServerGuard.java
@@ -85,9 +85,6 @@ public class KarafMBeanServerGuard implements
InvocationHandler {
if (method.getParameterTypes().length == 0)
return null;
- if (!ObjectName.class.isAssignableFrom(method.getParameterTypes()[0]))
- return null;
-
MBeanServer mbs = (MBeanServer) proxy;
if (mbs != null && Proxy.getInvocationHandler(mbs) instanceof
MBeanInvocationHandler) {
mbs = ((MBeanInvocationHandler)
Proxy.getInvocationHandler(mbs)).getDelegate();
@@ -96,6 +93,28 @@ public class KarafMBeanServerGuard implements
InvocationHandler {
mbs = ((EventAdminMBeanServerWrapper) mbs).getDelegate();
}
+ // MBean lifecycle operations don't take the ObjectName as the first
argument (or, for
+ // registerMBean, don't provide a class name at all), so they are
handled separately from
+ // the getAttribute/setAttribute/invoke operations below.
+ switch (method.getName()) {
+ case "createMBean":
+ // createMBean(String className, ObjectName name, ...)
+ handleRegistration("createMBean", (ObjectName) args[1],
(String) args[0]);
+ return null;
+ case "registerMBean":
+ // registerMBean(Object object, ObjectName name)
+ handleRegistration("registerMBean", (ObjectName) args[1],
+ args[0] != null ? args[0].getClass().getName() : null);
+ return null;
+ case "unregisterMBean":
+ // unregisterMBean(ObjectName name)
+ handleRegistration("unregisterMBean", (ObjectName) args[0],
null);
+ return null;
+ }
+
+ if (!ObjectName.class.isAssignableFrom(method.getParameterTypes()[0]))
+ return null;
+
ObjectName objectName = (ObjectName) args[0];
switch (method.getName()) {
case "getAttribute":
@@ -319,6 +338,11 @@ public class KarafMBeanServerGuard implements
InvocationHandler {
}
private boolean canBypassRBAC(BulkRequestContext context, ObjectName
objectName, String operationName) {
+ if (objectName == null) {
+ // an ObjectName-less invocation (e.g. createMBean with a null
name) can't be matched
+ // against the ObjectName-based whitelist
+ return false;
+ }
List<String> allBypassObjectName = new ArrayList<>();
List<Dictionary<String, Object>> configs =
context.getWhitelistProperties();
@@ -385,6 +409,48 @@ public class KarafMBeanServerGuard implements
InvocationHandler {
throw se;
}
+ /**
+ * Enforce RBAC for the MBean lifecycle operations
(<code>createMBean</code>, <code>registerMBean</code> and
+ * <code>unregisterMBean</code>). Unlike {@link #handleInvoke}, the target
MBean is not (or not yet) registered,
+ * so the required roles are resolved purely from the {@link ObjectName}
and the operation name against the
+ * <code>jmx.acl*</code> configurations. When a class name is available
(<code>createMBean</code> and
+ * <code>registerMBean</code>) it is passed as the single argument so that
ACL rules can match on it, e.g.
+ *
<code>createMBean(java.lang.String)[/javax\.management\.loading\..*/] =
admin</code>.
+ *
+ * @param operationName the lifecycle operation name.
+ * @param objectName the ObjectName the MBean is (being) registered under.
+ * @param className the MBean class name, or {@code null} when not
available.
+ * @throws IOException if the ConfigAdmin lookup fails.
+ */
+ void handleRegistration(String operationName, ObjectName objectName,
String className) throws IOException {
+ Object[] params;
+ String[] signature;
+ if (className != null) {
+ params = new Object[] { className };
+ signature = new String[] { String.class.getName() };
+ } else {
+ params = new Object[] {};
+ signature = new String[] {};
+ }
+
+ BulkRequestContext context =
BulkRequestContext.newContext(configAdmin);
+ if (canBypassRBAC(context, objectName, operationName)) {
+ return;
+ }
+ for (String role : getRequiredRoles(context, objectName,
operationName, params, signature)) {
+ if (JaasHelper.currentUserHasRole(role))
+ return;
+ }
+ if (Boolean.parseBoolean(System.getProperty(JMX_ACL_DETAILED_MESSAGE,
"false"))) {
+ printDetailedMessage(context, objectName, operationName, params,
signature);
+ }
+ SecurityException se = new SecurityException("Insufficient
roles/credentials for operation");
+ if (logger != null) {
+ logger.log(INVOKE, INVOKE_SIG, null, se, objectName,
operationName, signature, params);
+ }
+ throw se;
+ }
+
private void printDetailedMessage(BulkRequestContext context, ObjectName
objectName,
String operationName, Object[] params,
String[] signature) throws IOException {
StringBuilder expectedRoles = new StringBuilder();
@@ -406,8 +472,11 @@ public class KarafMBeanServerGuard implements
InvocationHandler {
currentRoles = new StringBuilder(p.getName());
}
}
+ List<String> pids = objectName == null
+ ? Collections.singletonList(JMX_ACL_PID_PREFIX)
+ : iterateDownPids(getNameSegments(objectName));
String matchedPid = null;
- for (String pid : iterateDownPids(getNameSegments(objectName))) {
+ for (String pid : pids) {
String generalPid = getGeneralPid(context.getAllPids(), pid);
if (generalPid.length() > 0) {
Dictionary<String, Object> config =
context.getConfiguration(generalPid);
@@ -421,7 +490,7 @@ public class KarafMBeanServerGuard implements
InvocationHandler {
}
if (matchedPid == null) {
//can't find the matched PID, use the most specific one
- matchedPid = iterateDownPids(getNameSegments(objectName)).get(0);
+ matchedPid = pids.get(0);
}
LOG.debug("The current roles are \'" + currentRoles
+ "\', however the expected roles are \'"
@@ -445,7 +514,12 @@ public class KarafMBeanServerGuard implements
InvocationHandler {
}
List<String> getRequiredRoles(BulkRequestContext context, ObjectName
objectName, String methodName, Object[] params, String[] signature) throws
IOException {
- for (String pid : iterateDownPids(getNameSegments(objectName))) {
+ // an ObjectName-less invocation (e.g. createMBean with a null name)
can only be matched
+ // against the generic jmx.acl configuration
+ List<String> pids = objectName == null
+ ? Collections.singletonList(JMX_ACL_PID_PREFIX)
+ : iterateDownPids(getNameSegments(objectName));
+ for (String pid : pids) {
String generalPid = getGeneralPid(context.getAllPids(), pid);
if (generalPid.length() > 0) {
Dictionary<String, Object> config =
context.getConfiguration(generalPid);
diff --git
a/management/server/src/main/java/org/apache/karaf/management/internal/MBeanInvocationHandler.java
b/management/server/src/main/java/org/apache/karaf/management/internal/MBeanInvocationHandler.java
index a779a45902..05266a24dd 100644
---
a/management/server/src/main/java/org/apache/karaf/management/internal/MBeanInvocationHandler.java
+++
b/management/server/src/main/java/org/apache/karaf/management/internal/MBeanInvocationHandler.java
@@ -33,7 +33,7 @@ public class MBeanInvocationHandler implements
InvocationHandler {
private final InvocationHandler guard;
- private final List<String> guarded =
Collections.unmodifiableList(Arrays.asList("invoke", "getAttribute",
"getAttributes", "setAttribute", "setAttributes"));
+ private final List<String> guarded =
Collections.unmodifiableList(Arrays.asList("invoke", "getAttribute",
"getAttributes", "setAttribute", "setAttributes", "createMBean",
"registerMBean", "unregisterMBean"));
public MBeanInvocationHandler(MBeanServer mBeanServer, InvocationHandler
guard) {
wrapped = mBeanServer;
diff --git
a/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java
b/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java
index 53d2e37f2a..2aa8e7dbc3 100644
---
a/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java
+++
b/management/server/src/test/java/org/apache/karaf/management/KarafMBeanServerGuardTest.java
@@ -522,6 +522,135 @@ public class KarafMBeanServerGuardTest extends TestCase {
});
}
+ public void testCreateRegisterUnregisterMBeanRequireRole() throws
Throwable {
+ Dictionary<String, Object> configuration = new Hashtable<>();
+ configuration.put("createMBean", "admin");
+ configuration.put("registerMBean", "admin");
+ configuration.put("unregisterMBean", "admin");
+ configuration.put("*", "admin");
+ ConfigurationAdmin ca = getMockConfigAdmin(configuration);
+
+ final KarafMBeanServerGuard guard = new KarafMBeanServerGuard();
+ guard.setConfigAdmin(ca);
+
+ final Method createMBean = MBeanServer.class.getMethod("createMBean",
String.class, ObjectName.class);
+ final Method registerMBean =
MBeanServer.class.getMethod("registerMBean", Object.class, ObjectName.class);
+ final Method unregisterMBean =
MBeanServer.class.getMethod("unregisterMBean", ObjectName.class);
+ final ObjectName on = ObjectName.getInstance("foo.bar:type=Test");
+
+ Subject viewer = loginWithTestRoles("viewer");
+ Subject.doAs(viewer, (PrivilegedAction<Void>) () -> {
+ try {
+ guard.invoke(null, createMBean, new
Object[]{"javax.management.loading.MLet", on});
+ fail("createMBean should be blocked for a non-admin user");
+ } catch (SecurityException se) {
+ // good
+ } catch (Throwable th) {
+ throw new RuntimeException(th);
+ }
+ try {
+ guard.invoke(null, registerMBean, new Object[]{new Object(),
on});
+ fail("registerMBean should be blocked for a non-admin user");
+ } catch (SecurityException se) {
+ // good
+ } catch (Throwable th) {
+ throw new RuntimeException(th);
+ }
+ try {
+ guard.invoke(null, unregisterMBean, new Object[]{on});
+ fail("unregisterMBean should be blocked for a non-admin user");
+ } catch (SecurityException se) {
+ // good
+ } catch (Throwable th) {
+ throw new RuntimeException(th);
+ }
+ return null;
+ });
+
+ Subject admin = loginWithTestRoles("admin");
+ Subject.doAs(admin, (PrivilegedAction<Void>) () -> {
+ try {
+ // none of these should throw for an admin user
+ guard.invoke(null, createMBean, new
Object[]{"javax.management.loading.MLet", on});
+ guard.invoke(null, registerMBean, new Object[]{new Object(),
on});
+ guard.invoke(null, unregisterMBean, new Object[]{on});
+ return null;
+ } catch (Throwable th) {
+ throw new RuntimeException(th);
+ }
+ });
+ }
+
+ public void testCreateMBeanWithNullObjectName() throws Throwable {
+ Dictionary<String, Object> configuration = new Hashtable<>();
+ configuration.put(Constants.SERVICE_PID, "jmx.acl");
+ configuration.put("createMBean", "admin");
+ configuration.put("*", "admin");
+ ConfigurationAdmin ca = getMockConfigAdmin2(configuration);
+
+ final KarafMBeanServerGuard guard = new KarafMBeanServerGuard();
+ guard.setConfigAdmin(ca);
+
+ final Method createMBean = MBeanServer.class.getMethod("createMBean",
String.class, ObjectName.class);
+
+ Subject viewer = loginWithTestRoles("viewer");
+ Subject.doAs(viewer, (PrivilegedAction<Void>) () -> {
+ try {
+ // a null ObjectName falls back to the generic jmx.acl
configuration
+ guard.invoke(null, createMBean, new
Object[]{"javax.management.loading.MLet", null});
+ fail("createMBean with a null name should be blocked for a
non-admin user");
+ } catch (SecurityException se) {
+ // good
+ } catch (Throwable th) {
+ throw new RuntimeException(th);
+ }
+ return null;
+ });
+
+ Subject admin = loginWithTestRoles("admin");
+ Subject.doAs(admin, (PrivilegedAction<Void>) () -> {
+ try {
+ guard.invoke(null, createMBean, new
Object[]{"javax.management.loading.MLet", null});
+ return null;
+ } catch (Throwable th) {
+ throw new RuntimeException(th);
+ }
+ });
+ }
+
+ public void testCreateMBeanClassNameMatch() throws Throwable {
+ Dictionary<String, Object> configuration = new Hashtable<>();
+
configuration.put("createMBean(java.lang.String)[/javax\\.management\\.loading\\..*/]",
"admin");
+ configuration.put("createMBean", "viewer");
+ ConfigurationAdmin ca = getMockConfigAdmin(configuration);
+
+ final KarafMBeanServerGuard guard = new KarafMBeanServerGuard();
+ guard.setConfigAdmin(ca);
+
+ final Method createMBean = MBeanServer.class.getMethod("createMBean",
String.class, ObjectName.class);
+ final ObjectName on = ObjectName.getInstance("foo.bar:type=Test");
+
+ Subject viewer = loginWithTestRoles("viewer");
+ Subject.doAs(viewer, (PrivilegedAction<Void>) () -> {
+ try {
+ // a "regular" class only requires the viewer role
+ guard.invoke(null, createMBean, new
Object[]{"com.example.Foo", on});
+ } catch (Throwable th) {
+ throw new RuntimeException(th);
+ }
+ try {
+ // a JMX classloader MBean requires the admin role
+ guard.invoke(null, createMBean, new
Object[]{"javax.management.loading.MLet", on});
+ fail("createMBean of a JMX classloader should be blocked for a
non-admin user");
+ } catch (SecurityException se) {
+ // good
+ } catch (Throwable th) {
+ throw new RuntimeException(th);
+ }
+ return null;
+ });
+ }
+
public void testGetAttributeIs() throws Throwable {
final ObjectName on = ObjectName.getInstance("foo.bar:type=Test");
diff --git a/manual/src/main/asciidoc/user-guide/monitoring.adoc
b/manual/src/main/asciidoc/user-guide/monitoring.adoc
index 019825d4a5..a6e9a02be6 100644
--- a/manual/src/main/asciidoc/user-guide/monitoring.adoc
+++ b/manual/src/main/asciidoc/user-guide/monitoring.adoc
@@ -264,11 +264,22 @@ doesn't have a specific configuration:
get* = viewer
is* = viewer
set* = admin
+ createMBean = admin
+ registerMBean = admin
+ unregisterMBean = admin
* = admin
----
By default, all "read-only" operations (`list*`, `get*`, `is*`) can be
performed by a `viewer`, whereas the "read-write" operations can be performed
only by an `admin`.
+The MBean lifecycle operations (`createMBean`, `registerMBean` and
`unregisterMBean`) are also guarded by RBAC and restricted to the `admin` role
by default.
+For `createMBean` (and `registerMBean`), the MBean class name is passed as the
operation argument, so it's possible to write more specific rules, for example
to restrict the creation of JMX class loader MBeans:
+
+----
+# etc/jmx.acl.cfg
+ createMBean(java.lang.String)[/javax\.management\.loading\..*/] = admin
+----
+
The `org.apache.karaf:type=security,area=jmx` MBean can be used to check
whether the current user can access a certain MBean or invoke a specific
operation on it.
This MBean can be used by management clients (monitoring tools, etc) to decide
whether to show certain MBeans or operations to the end user.
