jbonofre opened a new pull request, #2879:
URL: https://github.com/apache/karaf/pull/2879

   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
   
   - [x] `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.
   - [x] Verified `assemblies/features/standard/src/main/feature/feature.xml` 
is still well-formed XML.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to