Skip to content

[4.4.x] Reject shell metacharacters in instance javaOpts and add missing instance ACL - #2879

Merged
jbonofre merged 2 commits into
apache:karaf-4.4.xfrom
jbonofre:backport/2878-instance-javaopts-command-injection-4.4.x
Sep 12, 2026
Merged

[4.4.x] Reject shell metacharacters in instance javaOpts and add missing instance ACL#2879
jbonofre merged 2 commits into
apache:karaf-4.4.xfrom
jbonofre:backport/2878-instance-javaopts-command-injection-4.4.x

Conversation

@jbonofre

Copy link
Copy Markdown
Member

Backport of #2878 to karaf-4.4.x.

Summary

InstanceServiceImpl builds the child-instance launch command by concatenating the caller-supplied javaOpts unquoted into a single string that is ultimately executed via /bin/sh (ScriptUtils/ProcessImpl). A javaOpts value 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 through instance:create, instance:start, instance:restart, instance:change-opts, and the equivalent InstancesMBean JMX operations.

Separately, the instance shell scope ships with no org.apache.karaf.command.acl.instance.cfg at all (unlike bundle/config/feature/jaas/kar/scope_bundle/shell/system), so with no ACL configured for that scope, any authenticated shell user — not just admin — can invoke instance:* commands by default.

Changes

  • InstanceServiceImpl: reject any javaOpts value that isn't valid JVM-option syntax (letters, digits, whitespace, and _-+.,:=/@*) before it is used to build the launch command line, in both doStart() and the self-restart branch of restartInstance().
  • Add 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 in assemblies/features/standard/src/main/feature/feature.xml, so the instance scope is no longer unauthenticated-by-default. State-changing operations require admin; list/status require viewer.

Backport notes

Cherry-picked from 89cfc9b480 (main). One conflict, in InstanceServiceImpl.java: doStart() is a non-static instance method on this branch (it was made static later on main) — resolved by keeping the non-static signature and adding the new validateJavaOpts() helper as private static alongside it. No other differences.

Test plan

  • mvn -pl instance,util test passes on karaf-4.4.x, including tests that launch a real JVM subprocess through doStart/ProcessImpl with legitimate javaOpts (e.g. -Dprop="key"), confirming the validation doesn't reject normal usage.
  • Verified assemblies/features/standard/src/main/feature/feature.xml is still well-formed XML.

…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.
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Test Results

  656 files  ±0    656 suites  ±0   1h 2m 1s ⏱️ - 4m 46s
1 006 tests ±0    963 ✅ +1   43 💤 ±0  0 ❌  - 1 
2 750 runs  ±0  2 630 ✅ +2  120 💤 ±0  0 ❌  - 2 

Results for commit 2ee9c81. ± Comparison against base commit da2a8de.

♻️ This comment has been updated with latest results.

…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.
@jbonofre
jbonofre merged commit 4576eaa into apache:karaf-4.4.x Sep 12, 2026
7 checks passed
@jbonofre
jbonofre deleted the backport/2878-instance-javaopts-command-injection-4.4.x branch September 12, 2026 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant