Repository navigation
Conversation
…min role feature:requirement-add adds a provisioning requirement that the resolver installs and starts exactly like feature:install -- a "bundle:<uri>" requirement downloads and starts an arbitrary bundle, and a "feature:<name>" 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 apache#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.
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
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.
Problem
feature:requirement-addadds a provisioning requirement that the resolver installs and starts exactly likefeature:install— abundle:<uri>requirement downloads and starts an arbitrary bundle, and afeature:<name>requirement installs that feature.feature:requirement-removeis the uninstall equivalent.Neither command had an entry in
etc/org.apache.karaf.command.acl.feature.cfg, so an SSH user with only theviewerrole could reachRequirementAdd/RequirementRemoveand have Karaf download, install, start or uninstall arbitrary attacker-supplied bundles and features — bypassing the admin-only ACL onfeature:install,feature:uninstallandbundle:install. The installed bundle then runs with the Karaf process's authority (CWE-863).This is the same class as the
feature:repo-add -i/feature:repo-remove -ubypass fixed in #2883, on sibling commands that fix did not cover.Fix
Gate both commands to the
adminrole, mirroringinstall/uninstall. A plain name-only rule is sufficient: unlikerepo-add/repo-remove(where only-i/-uare dangerous and need an option-specific regex),requirement-addandrequirement-removemutate provisioning state on every invocation.The ACL is updated in the three places it lives:
assemblies/features/standard/src/main/feature/feature.xml— the standard features descriptor that generatesetc/org.apache.karaf.command.acl.feature.cfgin every distributioninstance/.../etc/org.apache.karaf.command.acl.feature.cfg— the copy child instances receiveitests/test/src/test/filtered-resources/etc/feature.xml— the itest fixture (kept in sync, as in Restrict feature:repo-add -i and feature:repo-remove -u to admin role #2883)Scope
FeaturesServiceMBeanexposes norequirement-*operation, so nothing to gate there.requirement-list) and non-install commands are left open, keeping the change tightly scoped like Restrict feature:repo-add -i and feature:repo-remove -u to admin role #2883.Testing
FeatureSshCommandSecurityTestgainstestFeatureRequirementCommandSecurityViaSsh, asserting avieweris denied both commands (surfaces asCommand not found, same asfeature:install) whileadminis allowed. The user guide ACL section is updated.Verified locally against a freshly rebuilt distribution: