Reject shell metacharacters in instance javaOpts and add missing instance ACL - #2878
Merged
jbonofre merged 2 commits intoSep 12, 2026
Merged
Conversation
…ance ACL InstanceServiceImpl builds the child-instance launch command by concatenating the caller-supplied javaOpts unquoted into a string that is ultimately handed to /bin/sh (via ScriptUtils/ProcessImpl), so a javaOpts value containing shell metacharacters was executed as an OS command rather than passed to the JVM. This was reachable through instance:create, instance:start, instance:restart and instance:change-opts (and the corresponding JMX operations). Reject any javaOpts value that isn't valid JVM-option syntax before it is used to build the launch command, in both doStart() and the self-restart branch of restartInstance(). Also ship org.apache.karaf.command.acl.instance.cfg, since the instance subshell had no ACL config at all and therefore failed open: any authenticated shell user could invoke instance:* commands regardless of role. The new ACL restricts state-changing operations to admin and read-only ones (list/status) to viewer, matching the pattern used by the other command ACLs.
2 tasks
…ance ACL The new org.apache.karaf.command.acl.instance.cfg restricts instance:* commands to the admin (and list/status to viewer) role. InstanceTest was invoking these commands without any role principal, so the secured shell rejected them as CommandNotFoundException, failing CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
InstanceServiceImplbuilds the child-instance launch command by concatenating the caller-suppliedjavaOptsunquoted into a single string that is ultimately executed via/bin/sh(ScriptUtils/ProcessImpl). AjavaOptsvalue containing shell metacharacters (;,|,`,$(...), etc.) is therefore interpreted by the shell rather than passed to the JVM as an option, allowing arbitrary OS command execution as the Karaf process user. This is reachable throughinstance:create,instance:start,instance:restart,instance:change-opts, and the equivalentInstancesMBeanJMX operations.Separately, the
instanceshell scope ships with noorg.apache.karaf.command.acl.instance.cfgat all (unlikebundle/config/feature/jaas/kar/scope_bundle/shell/system), so with no ACL configured for that scope, any authenticated shell user — not justadmin— can invokeinstance:*commands by default.Changes
InstanceServiceImpl: reject anyjavaOptsvalue that isn't valid JVM-option syntax (letters, digits, whitespace, and_-+.,:=/@*) before it is used to build the launch command line, in bothdoStart()and the self-restart branch ofrestartInstance(). This closes the injection at the two points where the shell command string is actually constructed, independent of which caller supplied the value.org.apache.karaf.command.acl.instance.cfg(new resource, wired into the child-instance scaffolding copy list) and the matching<config name="org.apache.karaf.command.acl.instance">block inassemblies/features/standard/src/main/feature/feature.xml, so theinstancescope is no longer unauthenticated-by-default. State-changing operations requireadmin;list/statusrequireviewer.Test plan
mvn -pl instance,util testpasses, including tests that launch a real JVM subprocess throughdoStart/ProcessImplwith legitimatejavaOpts(e.g.-Dprop="key"), confirming the validation doesn't reject normal usage.assemblies/features/standard/src/main/feature/feature.xmlis still well-formed XML.instance:create -o '-Xmx64m; id' xnow throwsIllegalArgumentExceptioninstead of spawning a shell.