This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch karaf-4.4.x
in repository https://gitbox.apache.org/repos/asf/karaf.git
The following commit(s) were added to refs/heads/karaf-4.4.x by this push:
new 73c1df0104 [karaf-4.4.x] Restrict feature:repo-add -i and
feature:repo-remove -u to admin role (#2884)
73c1df0104 is described below
commit 73c1df0104b834319fcd3573f50b768e27c97734
Author: JB Onofré <[email protected]>
AuthorDate: Sat Sep 12 13:07:18 2026 +0200
[karaf-4.4.x] Restrict feature:repo-add -i and feature:repo-remove -u to
admin role (#2884)
* Restrict feature:repo-add -i and feature:repo-remove -u to admin role
feature:repo-add -i and feature:repo-remove -u call
FeaturesServiceImpl.addRepository()/removeRepository() with the
install/uninstall flag set, which installs or uninstalls every
feature in the repository. Neither command had an ACL entry, so
any SSH user with just the viewer role could reach this and have
Karaf install and start an arbitrary attacker-supplied feature
repository, bypassing the admin-only ACL on feature:install and
feature:uninstall.
Add option-specific ACL rules (same pattern already used for
bundle:refresh -f and friends) so only -i/-u require admin, while
plain repo-add/repo-remove stay open as before.
Backport of #2883 to karaf-4.4.x.
* Fix CI: add open fallback ACL rule for plain feature:repo-add/repo-remove
Same issue as apache/karaf#2883: the regex-conditioned ACL rules
(repo-add[/.*[-][i].*/], repo-remove[/.*[-][u].*/]) had no fallback entry
for
the plain command. Karaf's ACL guard treats a command name that has at least
one ACL entry but no rule matching the current invocation as "no roles
allowed" (empty role list), which denies everyone -- including admin --
rather than leaving it unrestricted. This broke feature:repo-add/
feature:repo-remove without -i/-u for all callers, causing FeatureTest and
FeatureSshCommandSecurityTest to fail in CI with CommandNotFoundException.
Add an explicit "repo-add = viewer" / "repo-remove = viewer" fallback,
mirroring the existing pattern used for bundle:refresh -f, so the plain
commands remain open while -i/-u stay admin-only.
* Restore viewer access to plain feature:repo-add/repo-remove
Same fix as apache/karaf#2883. The previous "Fix CI" commit here added
the "repo-add = viewer" / "repo-remove = viewer" fallback needed to keep
the plain command visible to non-admin sessions (Karaf's shell guard
decides command visibility before it knows the actual arguments, so a
command with only a regex/exact-arg ACL entry is otherwise treated as
requiring that entry's role for every invocation). That part fixed
FeatureSshCommandSecurityTest, but the pre-existing FeatureTest cases
that called repo-add/repo-remove with no role at all (fine previously,
since the commands had no ACL entry) still failed: once a command has
any ACL entry, invoking it requires holding a role.
Pass an explicit viewer RolePrincipal from those three FeatureTest cases,
and document the viewer/admin split in security.adoc.
---
.../features/standard/src/main/feature/feature.xml | 11 ++++++++++
.../etc/org.apache.karaf.command.acl.feature.cfg | 11 ++++++++++
.../src/test/filtered-resources/etc/feature.xml | 4 ++++
.../java/org/apache/karaf/itests/FeatureTest.java | 10 ++++-----
.../itests/ssh/FeatureSshCommandSecurityTest.java | 25 ++++++++++++++++++++++
manual/src/main/asciidoc/user-guide/security.adoc | 5 +++--
6 files changed, 59 insertions(+), 7 deletions(-)
diff --git a/assemblies/features/standard/src/main/feature/feature.xml
b/assemblies/features/standard/src/main/feature/feature.xml
index 0dab1e9c9e..697ba9a055 100644
--- a/assemblies/features/standard/src/main/feature/feature.xml
+++ b/assemblies/features/standard/src/main/feature/feature.xml
@@ -151,6 +151,17 @@ install = admin
uninstall = admin
start = admin
stop = 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
+# entry and no fallback is treated as admin-only for *every* invocation,
because the
+# command guard decides visibility before it knows the actual arguments.
+#
+repo-add[/.*[-][i].*/] = admin
+repo-add = viewer
+repo-remove[/.*[-][u].*/] = admin
+repo-remove = viewer
</config>
<bundle
start-level="30">mvn:org.apache.karaf.features/org.apache.karaf.features.command/${project.version}</bundle>
</conditional>
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 823fe3e2cc..cefaa7a7fc 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
@@ -24,3 +24,14 @@ install = admin
uninstall = admin
start = admin
stop = 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
+# entry and no fallback is treated as admin-only for *every* invocation,
because the
+# command guard decides visibility before it knows the actual arguments.
+#
+repo-add[/.*[-][i].*/] = admin
+repo-add = viewer
+repo-remove[/.*[-][u].*/] = admin
+repo-remove = viewer
diff --git a/itests/test/src/test/filtered-resources/etc/feature.xml
b/itests/test/src/test/filtered-resources/etc/feature.xml
index b34dfff46a..03726969db 100644
--- a/itests/test/src/test/filtered-resources/etc/feature.xml
+++ b/itests/test/src/test/filtered-resources/etc/feature.xml
@@ -110,6 +110,10 @@
uninstall = admin
start = admin
stop = admin
+ repo-add[/.*[-][i].*/] = admin
+ repo-add = viewer
+ repo-remove[/.*[-][u].*/] = admin
+ repo-remove = viewer
</config>
<bundle
start-level="30">mvn:org.apache.karaf.features/org.apache.karaf.features.command/${project.version}</bundle>
</conditional>
diff --git a/itests/test/src/test/java/org/apache/karaf/itests/FeatureTest.java
b/itests/test/src/test/java/org/apache/karaf/itests/FeatureTest.java
index 6d6a6595fd..d6e4755379 100644
--- a/itests/test/src/test/java/org/apache/karaf/itests/FeatureTest.java
+++ b/itests/test/src/test/java/org/apache/karaf/itests/FeatureTest.java
@@ -77,7 +77,7 @@ public class FeatureTest extends BaseTest {
@Test
public void listCommandFromRepository() {
- executeCommand("feature:repo-add
mvn:org.apache.karaf.cellar/apache-karaf-cellar/3.0.0/xml/features");
+ executeCommand("feature:repo-add
mvn:org.apache.karaf.cellar/apache-karaf-cellar/3.0.0/xml/features", new
RolePrincipal("viewer"));
String repositoryName = "karaf-cellar-3.0.0";
String featureListOutput = executeCommand("feature:list --repository "
+ repositoryName);
assertFalse(featureListOutput.isEmpty());
@@ -151,17 +151,17 @@ public class FeatureTest extends BaseTest {
@Test
public void repoAddRemoveCommand() throws Exception {
- System.out.println(executeCommand("feature:repo-add
mvn:org.apache.karaf.cellar/apache-karaf-cellar/3.0.0/xml/features"));
+ System.out.println(executeCommand("feature:repo-add
mvn:org.apache.karaf.cellar/apache-karaf-cellar/3.0.0/xml/features", new
RolePrincipal("viewer")));
assertContains("apache-karaf-cellar",
executeCommand("feature:repo-list"));
- System.out.println(executeCommand("feature:repo-remove
mvn:org.apache.karaf.cellar/apache-karaf-cellar/3.0.0/xml/features"));
+ System.out.println(executeCommand("feature:repo-remove
mvn:org.apache.karaf.cellar/apache-karaf-cellar/3.0.0/xml/features", new
RolePrincipal("viewer")));
assertContainsNot("apache-karaf-cellar",
executeCommand("feature:repo-list"));
}
@Test
public void repoAddRemoveCommandWithRegex() throws Exception {
- System.out.println(executeCommand("feature:repo-add
mvn:org.apache.karaf.cellar/apache-karaf-cellar/3.0.0/xml/features"));
+ System.out.println(executeCommand("feature:repo-add
mvn:org.apache.karaf.cellar/apache-karaf-cellar/3.0.0/xml/features", new
RolePrincipal("viewer")));
assertContains("apache-karaf-cellar",
executeCommand("feature:repo-list"));
- System.out.println(executeCommand("feature:repo-remove
'.*apache-karaf-cellar.*'"));
+ System.out.println(executeCommand("feature:repo-remove
'.*apache-karaf-cellar.*'", new RolePrincipal("viewer")));
assertContainsNot("apache-karaf-cellar",
executeCommand("feature:repo-list"));
}
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 662b7c14e2..77eb84e77a 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
@@ -55,4 +55,29 @@ public class FeatureSshCommandSecurityTest extends
SshCommandTestBase {
Assert.assertFalse(feature + " feature should have been uninstalled",
r5.contains(feature));
}
+
+ @Test
+ public void testFeatureRepoCommandSecurityViaSsh() throws Exception {
+ String vieweruser = "viewer" + System.nanoTime() + "_repos";
+ // Deliberately non-existent so repo-add/repo-remove never actually
add, install, remove
+ // or uninstall anything; this test only cares about the ACL decision,
not the outcome of
+ // the underlying operation.
+ // Note: the ACL regex matches against the whole argument list, so
this URL must not
+ // itself contain "-i" or "-u" or it would coincidentally match the
option-specific rules.
+ String bogusUrl = "file:///nonexistent/karaf-test-repo-features.xml";
+
+ addViewer(vieweruser);
+
+ // repo-add/repo-remove without options are not gated by any role,
same as before this ACL
+ // was introduced: only the -i/-u options (which install/uninstall
every feature in the
+ // repository) must require admin.
+ assertCommand(vieweruser, "feature:repo-add " + bogusUrl, Result.OK);
+ assertCommand(vieweruser, "feature:repo-remove " + bogusUrl,
Result.OK);
+
+ assertCommand(vieweruser, "feature:repo-add -i " + bogusUrl,
Result.NO_CREDENTIALS);
+ assertCommand(vieweruser, "feature:repo-remove -u " + bogusUrl,
Result.NO_CREDENTIALS);
+
+ assertCommand("karaf", "feature:repo-add -i " + bogusUrl, Result.OK);
+ assertCommand("karaf", "feature:repo-remove -u " + bogusUrl,
Result.OK);
+ }
}
diff --git a/manual/src/main/asciidoc/user-guide/security.adoc
b/manual/src/main/asciidoc/user-guide/security.adoc
index 7a736b02a1..fdc6817bbe 100644
--- a/manual/src/main/asciidoc/user-guide/security.adoc
+++ b/manual/src/main/asciidoc/user-guide/security.adoc
@@ -492,8 +492,9 @@ 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. The other `feature:*`
- commands can be executed by any user.
+ Only the users with `admin` role can execute `feature:install`,
`feature:uninstall`,`feature:start`, `feature:stop` and `feature:update`
commands. `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.
Only the users with `admin` role can execute `jaas:update` command. The other
`jaas:*` commands can be executed by any
user.
