From 0514e551fe4fab3824be261461c61ca6389a0a37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?JB=20Onofr=C3=A9?= Date: Fri, 30 Oct 2026 08:03:22 +0100 Subject: [PATCH] Restrict feature:requirement-add and feature:requirement-remove to admin role feature:requirement-add adds a provisioning requirement that the resolver installs and starts exactly like feature:install -- a "bundle:" requirement downloads and starts an arbitrary bundle, and a "feature:" requirement installs that feature. feature:requirement-remove is the uninstall equivalent. Neither command had an ACL entry, so any SSH user with just the viewer role could reach RequirementAdd/RequirementRemove and have Karaf install, start or uninstall arbitrary attacker-supplied bundles and features, bypassing the admin-only ACL on feature:install/feature:uninstall (and bundle:install). This is the same class as the feature:repo-add -i / feature:repo-remove -u bypass fixed in #2883, on sibling commands that fix did not cover. Gate both commands to the admin role, mirroring install/uninstall. A plain name-only rule is enough: unlike repo-add/repo-remove (where only -i/-u are dangerous and need an option-specific regex), requirement-add and requirement-remove mutate provisioning state on every invocation. The ACL is updated in the three places it lives: the standard features descriptor that generates etc/org.apache.karaf.command.acl.feature.cfg in every distribution, the copy child instances receive, and the itest fixture. FeatureSshCommandSecurityTest gains a method asserting a viewer is denied both commands while admin is allowed, and the user guide is updated. --- .../standard/src/main/feature/feature.xml | 9 +++++++ .../org.apache.karaf.command.acl.feature.cfg | 9 +++++++ .../test/filtered-resources/etc/feature.xml | 2 ++ .../ssh/FeatureSshCommandSecurityTest.java | 26 +++++++++++++++++++ .../main/asciidoc/user-guide/security.adoc | 5 +++- 5 files changed, 50 insertions(+), 1 deletion(-) diff --git a/assemblies/features/standard/src/main/feature/feature.xml b/assemblies/features/standard/src/main/feature/feature.xml index 12b35e86a3e..bfbbe36ea76 100644 --- a/assemblies/features/standard/src/main/feature/feature.xml +++ b/assemblies/features/standard/src/main/feature/feature.xml @@ -149,6 +149,15 @@ uninstall = admin start = admin stop = admin # +# requirement-add/requirement-remove add or remove provisioning requirements. The resolver +# turns these into installing and starting (or uninstalling) bundles and features exactly as +# feature:install/feature:uninstall do -- a "bundle:" requirement downloads and starts an +# arbitrary bundle, and a "feature:" requirement installs that feature. They are gated to +# admin for the same reason as install/uninstall; otherwise they bypass those ACLs. +# +requirement-add = admin +requirement-remove = admin +# # repo-add/repo-remove stay open to the viewer role (the plain form only reads/lists # a repository), but installing/uninstalling every feature in it via -i/-u requires # admin. The bare "= viewer" fallback is required: a command with a regex-only ACL diff --git a/instance/src/main/resources/org/apache/karaf/instance/resources/etc/org.apache.karaf.command.acl.feature.cfg b/instance/src/main/resources/org/apache/karaf/instance/resources/etc/org.apache.karaf.command.acl.feature.cfg index cefaa7a7fc2..02d7bdcc2ad 100644 --- a/instance/src/main/resources/org/apache/karaf/instance/resources/etc/org.apache.karaf.command.acl.feature.cfg +++ b/instance/src/main/resources/org/apache/karaf/instance/resources/etc/org.apache.karaf.command.acl.feature.cfg @@ -25,6 +25,15 @@ uninstall = admin start = admin stop = admin # +# requirement-add/requirement-remove add or remove provisioning requirements. The resolver +# turns these into installing and starting (or uninstalling) bundles and features exactly as +# feature:install/feature:uninstall do -- a "bundle:" requirement downloads and starts an +# arbitrary bundle, and a "feature:" requirement installs that feature. They are gated to +# admin for the same reason as install/uninstall; otherwise they bypass those ACLs. +# +requirement-add = admin +requirement-remove = admin +# # repo-add/repo-remove stay open to the viewer role (the plain form only reads/lists # a repository), but installing/uninstalling every feature in it via -i/-u requires # admin. The bare "= viewer" fallback is required: a command with a regex-only ACL diff --git a/itests/test/src/test/filtered-resources/etc/feature.xml b/itests/test/src/test/filtered-resources/etc/feature.xml index 7a0c9d69ab5..ff520f578ad 100644 --- a/itests/test/src/test/filtered-resources/etc/feature.xml +++ b/itests/test/src/test/filtered-resources/etc/feature.xml @@ -108,6 +108,8 @@ uninstall = admin start = admin stop = admin + requirement-add = admin + requirement-remove = admin repo-add[/.*[-][i].*/] = admin repo-add = viewer repo-remove[/.*[-][u].*/] = admin diff --git a/itests/test/src/test/java/org/apache/karaf/itests/ssh/FeatureSshCommandSecurityTest.java b/itests/test/src/test/java/org/apache/karaf/itests/ssh/FeatureSshCommandSecurityTest.java index 288a0850266..5d0afab473d 100644 --- a/itests/test/src/test/java/org/apache/karaf/itests/ssh/FeatureSshCommandSecurityTest.java +++ b/itests/test/src/test/java/org/apache/karaf/itests/ssh/FeatureSshCommandSecurityTest.java @@ -94,4 +94,30 @@ public void testFeatureRepoCommandSecurityViaSsh() throws Exception { assertCommand("karaf", "feature:repo-add -i " + bogusUrl, Result.OK); assertCommand("karaf", "feature:repo-remove -u " + bogusUrl, Result.OK); } + + @Test + public void testFeatureRequirementCommandSecurityViaSsh() throws Exception { + // Skip on Windows where PTY output can be garbled, + // when upgrading to Junit5, this can be replaced with @DisabledOnOs(OS.WINDOWS) + // TODO: remove this once we have a better solution for PTY output on Windows + Assume.assumeFalse(System.getProperty("os.name", "").toLowerCase().contains("win")); + + String vieweruser = "viewer" + System.nanoTime() + "_requirements"; + + addViewer(vieweruser); + + // feature:requirement-add adds a provisioning requirement that the resolver installs and + // starts exactly like feature:install -- a "bundle:" requirement downloads and starts + // an arbitrary bundle. feature:requirement-remove is the uninstall equivalent. Both must be + // admin-only, like install/uninstall, so a viewer cannot bypass those ACLs. As plain + // name-only ACL rules (requirement-add = admin), the command guard hides them from the + // viewer, so the denial surfaces as "Command not found", same as feature:install. + assertCommand(vieweruser, "feature:requirement-add bundle:mvn:org.example/evil/1.0", Result.NOT_FOUND); + assertCommand(vieweruser, "feature:requirement-remove bundle:mvn:org.example/evil/1.0", Result.NOT_FOUND); + + // The admin role may run them. Use -t (simulate) with a non-existent requirement so nothing + // is actually downloaded, installed or persisted; this test only cares about the ACL decision. + assertCommand("karaf", "feature:requirement-add -t feature:karaf-test-nonexistent-requirement", Result.OK); + assertCommand("karaf", "feature:requirement-remove -t feature:karaf-test-nonexistent-requirement", Result.OK); + } } diff --git a/manual/src/main/asciidoc/user-guide/security.adoc b/manual/src/main/asciidoc/user-guide/security.adoc index fdc6817bbe0..a05f48c7ba3 100644 --- a/manual/src/main/asciidoc/user-guide/security.adoc +++ b/manual/src/main/asciidoc/user-guide/security.adoc @@ -492,7 +492,10 @@ By default, Apache Karaf defines the following commands ACLs: the users with the `manager` role can execute `config:*` commands. As `config:install` writes an arbitrary file into the `etc` folder, it is restricted to the users with the `admin` role. * `etc/org.apache.karaf.command.acl.feature.cfg` configuration file defines the ACL for `feature:*` commands. - Only the users with `admin` role can execute `feature:install`, `feature:uninstall`,`feature:start`, `feature:stop` and `feature:update` commands. `feature:repo-add` + Only the users with `admin` role can execute `feature:install`, `feature:uninstall`,`feature:start`, `feature:stop` and `feature:update` commands. + `feature:requirement-add` and `feature:requirement-remove` are also restricted to the `admin` role: they add or remove provisioning requirements that the + resolver turns into installing and starting (or uninstalling) bundles and features exactly as `feature:install`/`feature:uninstall` do, so without this a `viewer` + could bypass those ACLs (for instance a `bundle:` requirement downloads and starts an arbitrary bundle). `feature:repo-add` and `feature:repo-remove` require the `admin` role only when used with the `-i`/`--install` or `-u`/`--uninstall` option, which installs or uninstalls every feature in the repository; otherwise they only require the `viewer` role. The other `feature:*` commands can be executed by any user. * `etc/org.apache.karaf.command.acl.jaas.cfg` configuration file defines the ACL for `jaas:*` commands.