Skip to content
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

java.io.FilePermission is no longer allowed for custom plug-in. #69464

Closed
plebedev opened this issue Feb 23, 2021 · 5 comments · Fixed by #69643
Closed

java.io.FilePermission is no longer allowed for custom plug-in. #69464

plebedev opened this issue Feb 23, 2021 · 5 comments · Fixed by #69643
Labels
>bug :Core/Infra/Plugins Plugin API and infrastructure Team:Core/Infra Meta label for core/infra team

Comments

@plebedev
Copy link
Contributor

Elasticsearch version (bin/elasticsearch --version):
Version: 7.11.1, Build: default/tar/ff17057114c2199c9c1bbecc727003a907c0db7a/2021-02-15T13:44:09.394032Z, JVM: 11.0.6

Plugins installed: []

JVM version (java -version):
java version "11.0.6" 2020-01-14 LTS

OS version (uname -a if on a Unix-like system):
18.7.0 Darwin Kernel Version 18.7.0: Fri Oct 30 12:37:06 PDT 2020; root:xnu-4903.278.44.0.2~1/RELEASE_X86_64 x86_64

Description of the problem including expected versus actual behavior:
We manage our own ES cluster on k8s, and have developed a custom plug-in that needs to read a secret installed on a pod. For this we added the following to plugin-security.policy file:

permission java.io.FilePermission "/var/run/secrets/kubernetes.io/serviceaccount/token", "read";

With 7.10, this works as expected. However, the installation of the plugin fails with 7.11.1:

Exception in thread "main" java.lang.IllegalArgumentException: plugin policy [/Users/plebedev/elasticsearch-7.11.1/plugins/.installing-7543768861641787116/plugin-security.policy] contains illegal permission ("java.io.FilePermission" "/var/run/secrets/kubernetes.io/serviceaccount/token#plus" "read") in global grant

(see stack trace below).

Here is the full file:

grant {
   permission java.net.SocketPermission "*", "resolve, connect";
   permission java.lang.RuntimePermission "getClassLoader";
   permission java.lang.RuntimePermission "setContextClassLoader";
   permission java.util.PropertyPermission "guava.concurrent.generate_cancellation_cause", "read";
   permission java.io.FilePermission "/var/run/secrets/kubernetes.io/serviceaccount/token", "read";
   permission java.lang.RuntimePermission "setFactory";
   permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
   permission java.lang.RuntimePermission "accessDeclaredMembers";
};

In 7.11, the following file was added:
https://github.com/elastic/elasticsearch/blob/v7.11.0/server/src/main/java/org/elasticsearch/bootstrap/PolicyUtil.java
that explicitly list permissions allowed for plug-ins, and java.io.FilePermission is not part of it.
If I simply remove this grant I will get this error:

Caused by: java.security.AccessControlException: access denied ("java.io.FilePermission" "/var/run/secrets/kubernetes.io/serviceaccount/token" "read")
	at java.security.AccessControlContext.checkPermission(AccessControlContext.java:472) ~[?:?]
	at java.security.AccessController.checkPermission(AccessController.java:897) ~[?:?]
	at java.lang.SecurityManager.checkPermission(SecurityManager.java:322) ~[?:?]
	at java.lang.SecurityManager.checkRead(SecurityManager.java:661) ~[?:?]
	at java.io.File.canRead(File.java:764) ~[?:?]

If this is a bug, this needs to be fixed. If this is an intended behavior, the documentation needs to be updated that this is a breaking change, and the appropriate alternative provided.

Steps to reproduce:
You can use https://github.com/elastic/elasticsearch/tree/v7.11.1/plugins/examples/painless-whitelist as a baseline, add plugin-security.policy file (as described https://www.elastic.co/guide/en/elasticsearch/plugins/current/plugin-authors.html#plugin-authors-jsm) with the permission above and try to install on 7.11.1.

Provide logs (if relevant):

Exception in thread "main" java.lang.IllegalArgumentException: plugin policy [/Users/plebedev/elasticsearch-7.11.1/plugins/.installing-7543768861641787116/plugin-security.policy] contains illegal permission ("java.io.FilePermission" "/var/run/secrets/kubernetes.io/serviceaccount/token#plus" "read") in global grant
	at org.elasticsearch.bootstrap.PolicyUtil.validatePolicyPermissionsForJar(PolicyUtil.java:300)
	at org.elasticsearch.bootstrap.PolicyUtil.validatePolicyPermissions(PolicyUtil.java:310)
	at org.elasticsearch.bootstrap.PolicyUtil.getPluginPolicyInfo(PolicyUtil.java:321)
	at org.elasticsearch.plugins.InstallPluginCommand.installPlugin(InstallPluginCommand.java:861)
	at org.elasticsearch.plugins.InstallPluginCommand.execute(InstallPluginCommand.java:245)
	at org.elasticsearch.plugins.InstallPluginCommand.execute(InstallPluginCommand.java:215)
	at org.elasticsearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:75)
	at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:116)
	at org.elasticsearch.cli.MultiCommand.execute(MultiCommand.java:80)
	at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:116)
	at org.elasticsearch.cli.Command.main(Command.java:79)
	at org.elasticsearch.plugins.PluginCli.main(PluginCli.java:36)
@plebedev plebedev added >bug needs:triage Requires assignment of a team area label labels Feb 23, 2021
@jaymode jaymode added the :Core/Infra/Plugins Plugin API and infrastructure label Feb 24, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Feb 24, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@jaymode jaymode removed the needs:triage Requires assignment of a team area label label Feb 24, 2021
@rjernst
Copy link
Member

rjernst commented Feb 24, 2021

@plebedev Thanks for the detailed report, and sorry for the situation. This is working as intended, at least in part. We are making plugins more restricted with system access to narrow the surface area they can affect if compromised.
However, the caveat to this is that we know certain files like the one you need access to are sometimes necessary (see this recent example with a similar problem). The idea that I had before implementing the policy restrictions was to give all plugins automatic read/readlink permissions to their configuration directory (a directory under /etc/elasticsearch named for the plugin). I didn't yet implement this because I thought the global read,readlink we gave to the config directory would suffice for now, but now realize readlink only works there at the top level, not recursively (at least, I think).
I will work on this new behavior soon, but in the meantime I can't think of any workaround. I'll post here when I get a PR up to fix the issue, at which point you can add a symlink as follows:

mkdir -p /etc/elasticsearch/NAME_OF_YOUR_PLUGIN
ln -s /var/run/secrets/kubernetes.io/serviceaccount/token /etc/elasticsearch/NAME_OF_YOUR_PLUGIN/kubernetes-token

Note that this will require wrapping access to the file in AccessController.doPrivileged, but that is no different than was already necessary.

@plebedev
Copy link
Contributor Author

@rjernst - thanks for the comment. In our current implementation we do wrap the file access in AccessController.doPrivileged, so this won't be a new thing.

I'm curious if it is possible to allow FilePermission to support existing plug-ins until the proper solution is implemented. There is a bug introduced in 7.9 that is fixed in 7.11, that is not related to plug-ins (#69401) but we can't move to 7.11 because of this issue.

rjernst added a commit to rjernst/elasticsearch that referenced this issue Feb 26, 2021
This commit adds back allowing FilePermission for reading files in
plugins. This is a temporary measure until plugins are automatically
granted read permissions for files within their own configuration
directory.

closes elastic#69464
rjernst added a commit that referenced this issue Feb 27, 2021
This commit adds back allowing FilePermission for reading files in
plugins. This is a temporary measure until plugins are automatically
granted read permissions for files within their own configuration
directory.

closes #69464
rjernst added a commit that referenced this issue Feb 27, 2021
This commit adds back allowing FilePermission for reading files in
plugins. This is a temporary measure until plugins are automatically
granted read permissions for files within their own configuration
directory.

closes #69464
rjernst added a commit that referenced this issue Feb 27, 2021
This commit adds back allowing FilePermission for reading files in
plugins. This is a temporary measure until plugins are automatically
granted read permissions for files within their own configuration
directory.

closes #69464
rjernst added a commit that referenced this issue Feb 27, 2021
This commit adds back allowing FilePermission for reading files in
plugins. This is a temporary measure until plugins are automatically
granted read permissions for files within their own configuration
directory.

closes #69464
@rjernst
Copy link
Member

rjernst commented Feb 27, 2021

@plebedev Thanks again for the report. I've restored the ability to grant FilePermission read from plugins for now. We will restrict this again in the future, as I described above, so I suggest you do switch to using a symlink, and then the FilePermission in your own policy can just disappear in the future when that change occurs.

@plebedev
Copy link
Contributor Author

plebedev commented Mar 1, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Plugins Plugin API and infrastructure Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants