-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add an exclusive parameter for files entitlements #123087
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
4d2f850
add dedup to file access tree
jdconrad 6002f1f
add exclusive to FilesEntitlement
jdconrad c79fb72
add global exclusive variable
jdconrad 415a744
build and validate exclusive paths in PolicyManager
jdconrad 8897f0b
add exclusive paths to FileAccessTree constructor
jdconrad 8da4660
build and throw for exclusive path access
jdconrad 381ed11
add some basic tests
jdconrad 6f14915
[CI] Auto commit changes from spotless
c5b9f2a
Merge branch 'main' into exclusive2
jdconrad fef7dc5
remove conflict for main
jdconrad 0978447
Merge branch 'main' into exclusive2
jdconrad 857ba64
update to use path instead of string and component and module for
jdconrad a39b3a9
checkpoint
jdconrad b86d434
add withExclusive to FileData
jdconrad e839df5
Merge branch 'main' into exclusive2
jdconrad 2b9570a
updates
jdconrad 580c4a3
fix test
jdconrad a283e0d
add more tests
jdconrad e36364f
update tests
jdconrad 7ecd086
Merge branch 'main' into exclusive2
jdconrad b255a2c
Merge branch 'main' into exclusive2
jdconrad c5dbb64
add policy manager tests for exclusive
jdconrad 3b648b8
[CI] Auto commit changes from spotless
b78ce2c
more tests
jdconrad 39b9ff7
Merge branch 'main' into exclusive2
jdconrad d9de8c7
response to pr comments
jdconrad cc6cd44
Merge branch 'main' into exclusive2
jdconrad f0b4136
Merge branch 'main' into exclusive2
jdconrad 7383ea8
fix error messages for parsing types
jdconrad File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ | |
| import org.elasticsearch.core.SuppressForbidden; | ||
| import org.elasticsearch.entitlement.instrumentation.InstrumentationService; | ||
| import org.elasticsearch.entitlement.runtime.api.NotEntitledException; | ||
| import org.elasticsearch.entitlement.runtime.policy.FileAccessTree.ExclusiveFileEntitlement; | ||
| import org.elasticsearch.entitlement.runtime.policy.FileAccessTree.ExclusivePath; | ||
| import org.elasticsearch.entitlement.runtime.policy.entitlements.CreateClassLoaderEntitlement; | ||
| import org.elasticsearch.entitlement.runtime.policy.entitlements.Entitlement; | ||
| import org.elasticsearch.entitlement.runtime.policy.entitlements.ExitVMEntitlement; | ||
|
|
@@ -32,6 +34,7 @@ | |
| import java.lang.module.ModuleFinder; | ||
| import java.lang.module.ModuleReference; | ||
| import java.nio.file.Path; | ||
| import java.util.ArrayList; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
@@ -91,7 +94,7 @@ ModuleEntitlements defaultEntitlements(String componentName) { | |
| } | ||
|
|
||
| // pkg private for testing | ||
| ModuleEntitlements policyEntitlements(String componentName, List<Entitlement> entitlements) { | ||
| ModuleEntitlements policyEntitlements(String componentName, String moduleName, List<Entitlement> entitlements) { | ||
| FilesEntitlement filesEntitlement = FilesEntitlement.EMPTY; | ||
| for (Entitlement entitlement : entitlements) { | ||
| if (entitlement instanceof FilesEntitlement) { | ||
|
|
@@ -101,7 +104,7 @@ ModuleEntitlements policyEntitlements(String componentName, List<Entitlement> en | |
| return new ModuleEntitlements( | ||
| componentName, | ||
| entitlements.stream().collect(groupingBy(Entitlement::getClass)), | ||
| FileAccessTree.of(filesEntitlement, pathLookup) | ||
| FileAccessTree.of(componentName, moduleName, filesEntitlement, pathLookup, exclusivePaths) | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -143,6 +146,13 @@ private static Set<Module> findSystemModules() { | |
| */ | ||
| private final Module entitlementsModule; | ||
|
|
||
| /** | ||
| * Paths that are only allowed for a single module. Used to generate | ||
| * structures to indicate other modules aren't allowed to use these | ||
| * files in {@link FileAccessTree}s. | ||
| */ | ||
| private final List<ExclusivePath> exclusivePaths; | ||
|
|
||
| public PolicyManager( | ||
| Policy serverPolicy, | ||
| List<Entitlement> apmAgentEntitlements, | ||
|
|
@@ -162,25 +172,40 @@ public PolicyManager( | |
| this.apmAgentPackageName = apmAgentPackageName; | ||
| this.entitlementsModule = entitlementsModule; | ||
| this.pathLookup = requireNonNull(pathLookup); | ||
| this.defaultFileAccess = FileAccessTree.of(FilesEntitlement.EMPTY, pathLookup); | ||
| this.defaultFileAccess = FileAccessTree.of( | ||
| UNKNOWN_COMPONENT_NAME, | ||
| UNKNOWN_COMPONENT_NAME, | ||
| FilesEntitlement.EMPTY, | ||
| pathLookup, | ||
| List.of() | ||
| ); | ||
| this.mutedClasses = suppressFailureLogClasses; | ||
|
|
||
| List<ExclusiveFileEntitlement> exclusiveFileEntitlements = new ArrayList<>(); | ||
| for (var e : serverEntitlements.entrySet()) { | ||
| validateEntitlementsPerModule(SERVER_COMPONENT_NAME, e.getKey(), e.getValue()); | ||
| validateEntitlementsPerModule(SERVER_COMPONENT_NAME, e.getKey(), e.getValue(), exclusiveFileEntitlements); | ||
| } | ||
| validateEntitlementsPerModule(APM_AGENT_COMPONENT_NAME, "unnamed", apmAgentEntitlements); | ||
| validateEntitlementsPerModule(APM_AGENT_COMPONENT_NAME, ALL_UNNAMED, apmAgentEntitlements, exclusiveFileEntitlements); | ||
| for (var p : pluginsEntitlements.entrySet()) { | ||
| for (var m : p.getValue().entrySet()) { | ||
| validateEntitlementsPerModule(p.getKey(), m.getKey(), m.getValue()); | ||
| validateEntitlementsPerModule(p.getKey(), m.getKey(), m.getValue(), exclusiveFileEntitlements); | ||
| } | ||
| } | ||
| List<ExclusivePath> exclusivePaths = FileAccessTree.buildExclusivePathList(exclusiveFileEntitlements, pathLookup); | ||
| FileAccessTree.validateExclusivePaths(exclusivePaths); | ||
| this.exclusivePaths = exclusivePaths; | ||
| } | ||
|
|
||
| private static Map<String, List<Entitlement>> buildScopeEntitlementsMap(Policy policy) { | ||
| return policy.scopes().stream().collect(toUnmodifiableMap(Scope::moduleName, Scope::entitlements)); | ||
| } | ||
|
|
||
| private static void validateEntitlementsPerModule(String componentName, String moduleName, List<Entitlement> entitlements) { | ||
| private static void validateEntitlementsPerModule( | ||
| String componentName, | ||
| String moduleName, | ||
| List<Entitlement> entitlements, | ||
| List<ExclusiveFileEntitlement> exclusiveFileEntitlements | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this is an "out parameter"? That probably deserves a javadoc.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added. |
||
| ) { | ||
| Set<Class<? extends Entitlement>> found = new HashSet<>(); | ||
| for (var e : entitlements) { | ||
| if (found.contains(e.getClass())) { | ||
|
|
@@ -189,6 +214,9 @@ private static void validateEntitlementsPerModule(String componentName, String m | |
| ); | ||
| } | ||
| found.add(e.getClass()); | ||
| if (e instanceof FilesEntitlement fe) { | ||
| exclusiveFileEntitlements.add(new ExclusiveFileEntitlement(componentName, moduleName, fe)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -498,7 +526,7 @@ private ModuleEntitlements computeEntitlements(Class<?> requestingClass) { | |
|
|
||
| if (requestingModule.isNamed() == false && requestingClass.getPackageName().startsWith(apmAgentPackageName)) { | ||
| // The APM agent is the only thing running non-modular in the system classloader | ||
| return policyEntitlements(APM_AGENT_COMPONENT_NAME, apmAgentEntitlements); | ||
| return policyEntitlements(APM_AGENT_COMPONENT_NAME, ALL_UNNAMED, apmAgentEntitlements); | ||
| } | ||
|
|
||
| return defaultEntitlements(UNKNOWN_COMPONENT_NAME); | ||
|
|
@@ -513,7 +541,7 @@ private ModuleEntitlements getModuleScopeEntitlements( | |
| if (entitlements == null) { | ||
| return defaultEntitlements(componentName); | ||
| } | ||
| return policyEntitlements(componentName, entitlements); | ||
| return policyEntitlements(componentName, moduleName, entitlements); | ||
| } | ||
|
|
||
| private static boolean isServerModule(Module requestingModule) { | ||
|
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this is pretty subtle.
Assuming the incoming list of paths has been sorted by
PATH_ORDER, that does not ensure that a child is always immediately after its parent; but it does ensure that any list of paths containing one or more parent-child pairs will have at least one of those pairs adjacent, and that's enough!For example:
The child
/a/cis not next to its parent/a, but for the sake of validation, that's ok, because to have something else (like/a/b) interleave, that must itself be a child (or descendant) of/a, and so the validation will fail on that.I'm 95% sure this reasoning works and we're not missing a case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be path order and since that moves file separators to be the first character this should work. This works similarly to pruning and I think has the same guarantees here. Thanks for checking it :)