fix(#728): scope the camel-core jmx.acl to Karaf's role conventions - #737
fix(#728): scope the camel-core jmx.acl to Karaf's role conventions#737oscerd wants to merge 3 commits into
Conversation
…ions The camel-core feature seeded jmx.acl.org.apache.camel with a single "* = *" entry. Karaf resolves ACL PIDs most specific first, so that domain level PID is consulted before the root jmx.acl PID and replaces Karaf's stock per-operation mapping for every Camel MBean: read-only introspection and mutating operations end up treated identically. Replace it with the deviations only. Karaf falls through to the next PID when no rule in the current one matches, so listing just the lifecycle operations leaves get*/list*/is* at the stock viewer mapping and everything else - sendBody*, requestBody*, createEndpoint, removeEndpoints, addOrUpdateRoutes*, and dump*, which resolves property placeholders - at the stock admin mapping. Lifecycle goes to manager, matching how Karaf maps bundle lifecycle in its own jmx.acl.org.apache.karaf.bundle. The comment being removed said the entry was there to let the karaf commands reach the Camel MBeans. That does not hold: KarafMBeanServerGuard is installed as a proxy on the MBeanServer handed to the JMX connector (ConnectorServerFactory:293-295), so it applies to remote connections only. The camel:* commands read CamelContext from the OSGi service registry, and the one command that touches the MBeanServer (ContextInflight) does so in-VM, bypassing the guard either way. Verified against Karaf 4.4.8's own ACLConfigurationParser rather than by inspection: read-only stays viewer, lifecycle resolves to manager, and the injection/mutation/dump operations resolve to admin. Karaf seeds a config only when absent, so an existing installation keeps its current file on upgrade; documented as an operator check in the security model.
Co-authored-by: JB Onofré <jbonofre@apache.org>
…are safe to expose Builds on the committed suggestion. browse* = viewer is kept as reviewed; the dump* wildcard is replaced by an explicit list, because as a wildcard it grants more than intended and fails open. Reading the MBeans is what settled it: - dumpRoutesAsXml has four overloads and dumpRoutesAsYaml five, plus the route level twins. The first boolean is resolvePlaceholders in every family, and every no-arg form passes false - verified from the bytecode, dumpRoutesAsXml() calls dumpRoutesAsXml(false, true). So the no-arg dumps are safe to expose and the boolean overloads are the ones that can print resolved credentials. - dump* also matches ManagedBacklogTracerMBean.dumpAllTracedMessagesAsXml and dumpTracedMessagesAsXml, which return traced message bodies, and ManagedDumpRoutesStrategyMBean.dumpRoutes(String), which writes files. Pinning arguments instead (dumpRoutesAsXml(boolean)[true] = admin) would cover today's overloads but fails open: an overload added by a future Camel matches no argument rule, falls back to the dump* wildcard and is granted to viewer. Since repackaging each new Camel release is what this repository does, that is routine rather than hypothetical. Listing the safe operations fails the other way - an unrecognised operation stays at Karaf's admin default until someone reviews it. Verified by running the mapping through Karaf 4.4.8's ACLConfigurationParser, driven the way KarafMBeanServerGuard.getRequiredRoles drives it: domain PID first, fall through to the stock root jmx.acl on NO_MATCH. 24 expectations, all holding, including a hypothetical dumpRoutesAsXml(b,b,b,b) landing on admin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@jbonofre before this lands — one line in it contradicts the rationale the same PR documents, and I would rather raise it now than after the merge. The
The Four lines above it, though:
So on any browsable endpoint in the container — Worth noting it was not in the original issue either — #728 was about the Two consistent ways out, both fine by me:
Everything else in the PR I checked against the real MBean interfaces: all 15 Claude Code on behalf of Andrea Cosentino |
2 participants
Fixes #728
What
The
camel-corefeature seededjmx.acl.org.apache.camelwith a single* = *entry. Karaf resolves ACL PIDs most-specific-first, so this domain-levelPID is consulted before the root
jmx.aclPID and replaces Karaf's stockper-operation mapping for every MBean in the
org.apache.cameldomain — soread-only introspection and mutating operations are treated identically.
How
Karaf falls through to the next-less-specific PID when no rule in the current
PID matches the operation. That means the domain file only needs to list the
operations that should differ from the stock
jmx.acldefaults, exactly asKaraf's own
jmx.acl.org.apache.karaf.bundle.cfgdoes (it lists lifecycle onlyand lets
get*/list*fall through).So the entry becomes:
which leaves:
get*/list*/is*at the stock viewer mappingsendBody*,requestBody*,createEndpoint,removeEndpoints,addOrUpdateRoutes*, anddump*(which resolves propertyplaceholders) — at the stock admin mapping
The comment that was removed was wrong
The entry carried
<!-- allow camel to access its own mbeans for karaf commands and other needs -->. That justification does not hold:KarafMBeanServerGuardis installed as aProxyaround theMBeanServerpassed to
JMXConnectorServerFactory.newJMXConnectorServer(...)(
ConnectorServerFactory:293-295), so it applies to remote JMX connectionsonly.
camel:*commands readCamelContextstraight from the OSGi serviceregistry (
CamelCommandSupport.getCamelContexts()).ContextInflight, gets itfrom
agent.getMBeanServer()in-VM — which bypasses the guard regardless ofwhat any ACL says.
So this config never had any effect on the shell commands; it only ever applied
to remote JMX principals.
Verification
Rather than reasoning about the semantics, I ran the new mapping through
Karaf 4.4.8's own
ACLConfigurationParser, driving it the wayKarafMBeanServerGuard.getRequiredRolesdoes (domain PID first, fall through tothe stock root
jmx.aclonNO_MATCH):(The
[*]role in the OLD column is why every row reads ALLOWED: Karaf'sJaasHelper.currentUserHasRoletreats a required role of*as satisfiedbefore it looks at any principal.)
camel-features.xmlis still well-formed XML.Upgrade note
Karaf seeds a
<config>only when the file does not already exist, so anexisting installation keeps its current
etc/jmx.acl.org.apache.camel.cfgon upgrade. Added an operator bullet to thesecurity model telling operators to check that file.
Open question for reviewers
The tiering above is a judgement call. The alternative is to drop the
<config>block entirely and let the whole domain fall through to Karaf'sdefaults — simpler, but then route lifecycle needs
adminover remote JMX,which is inconsistent with Karaf treating bundle lifecycle as
manager. Happyto switch if you prefer strict fall-through.
Claude Code on behalf of Andrea Cosentino