-
Notifications
You must be signed in to change notification settings - Fork 25.4k
Remove java.xml from system modules #133671
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
Conversation
java.xml is part of the jdk, but it's really a utility module that shouldn't have direct access to network or files. This commit excludes java.xml from system modules. Note that since it is part of the jdk, it does need access to read jdk classes, so a new internal entitlement is also added to allow reading jrt urls.
Pinging @elastic/es-core-infra (Team:Core/Infra) |
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.
Some optional javadoc suggestions. Mergeable as is I think.
@@ -610,5 +612,12 @@ static void javaDesktopFileAccess() throws Exception { | |||
new FileImageInputStream(file.toFile()).close(); | |||
} | |||
|
|||
@EntitlementTest(expectedAccess = ALWAYS_DENIED) | |||
static void javaXmlFileRequest() throws Exception { | |||
// java.xml is part of the jdk, but not a system module. this checks it can't access the network |
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.
*filesystem?
/** | ||
* Internal entitlement to read code from the jdk, ie jrt urls | ||
*/ | ||
public class ReadJdkCodeEntitlement implements Entitlement { |
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.
Ooh a new entitlement
package org.elasticsearch.entitlement.runtime.policy.entitlements; | ||
|
||
/** | ||
* Internal entitlement to read code from the jdk, ie jrt urls |
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.
This description could use a little refinement, I think. Anyone seeing ReadJdkCodeEntitlement
can be trusted to infer that it's an entitlement for reading JDK code, but their question would be: what does that actually mean? I think it means allowing the opening of connections with URLs using the jrt
protocol.
The actual implementation does not seem to be limited to just reading resources within the jar, but I suppose writes to jrt
URLs would be forbidden by other means?
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.
yeah the jdk images are read only. I updated the javadoc.
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.
I also tweaked the entitlement name to make it a little clearer what can be read
Looks like this needs further entitlements to read any jar on the classpath.
|
Looks like granting java.xml read access to the
but I suppose |
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.
Looking good besides above mentioned test issues
Hi @rjernst, I've created a changelog YAML for you. |
java.xml is part of the jdk, but it's really a utility module that shouldn't have direct access to network or files. This commit excludes java.xml from system modules. Note that since it is part of the jdk, it does need access to read jdk classes, so a new internal entitlement is also added to allow reading jrt urls.
java.xml is part of the jdk, but it's really a utility module that shouldn't have direct access to network or files. This commit excludes java.xml from system modules. Note that since it is part of the jdk, it does need access to read jdk classes, so a new internal entitlement is also added to allow reading jrt urls.
💔 Backport failed
You can use sqren/backport to manually backport by running |
java.xml is part of the jdk, but it's really a utility module that shouldn't have direct access to network or files. This commit excludes java.xml from system modules. Note that since it is part of the jdk, it does need access to read jdk classes, so a new internal entitlement is also added to allow reading jrt urls.
java.xml is part of the jdk, but it's really a utility module that shouldn't have direct access to network or files. This commit excludes java.xml from system modules. Note that since it is part of the jdk, it does need access to read jdk classes, so a new internal entitlement is also added to allow reading jrt urls.
java.xml is part of the jdk, but it's really a utility module that shouldn't have direct access to network or files. This commit excludes java.xml from system modules. Note that since it is part of the jdk, it does need access to read jdk classes, so a new internal entitlement is also added to allow reading jrt urls.
java.xml is part of the jdk, but it's really a utility module that shouldn't have direct access to network or files. This commit excludes java.xml from system modules. Note that since it is part of the jdk, it does need access to read jdk classes, so a new internal entitlement is also added to allow reading jrt urls.
java.xml is part of the jdk, but it's really a utility module that shouldn't have direct access to network or files. This commit excludes java.xml from system modules. Note that since it is part of the jdk, it does need access to read jdk classes, so a new internal entitlement is also added to allow reading jrt urls.