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

Confirm OpenSearch is running with a strongly configured JVM security manager by default #528

Closed
mikemccand opened this issue Apr 12, 2021 · 10 comments
Labels
enhancement Enhancement or improvement to existing feature or request

Comments

@mikemccand
Copy link

Spinoff from #524:

The JVM security manager is an awesome security tool! It is effectively a self-imposed jail, allowing you to express what things your JVM process is and is not allowed to do. Tutorial: https://docs.oracle.com/javase/tutorial/essential/environment/security.html

I think Elasticsearch enabled the security manager by default, but I'm not certain. @rmuir might know?

For this issue let's 1) confirm we are still enabling the security manager by default, and 2) try to tighten its rules (how strong the jail is) based on OpenSearch specific changes like removing Elasticsearch's sneaky "phone home" feature.

This can improve future security by preventing certain categories of dangerous operations.

@mikemccand mikemccand added the enhancement Enhancement or improvement to existing feature or request label Apr 12, 2021
@rmuir
Copy link
Contributor

rmuir commented Apr 12, 2021

@mikemccand I took a look, it seems to be unconditionally enabled (also in tests). Look for calls to System.setSecurityManager, that is what turns it on. This is because policy processing is complex (e.g. involves user-configurable paths from yaml configs etc).

few notes about how it works from my memory:

  • The core policy files are here: https://github.com/opensearch-project/OpenSearch/tree/main/server/src/main/resources/org/opensearch/bootstrap
  • There's a BootstrapForTesting that should be turning the stuff on when running unit tests, obviously this is critical.
  • Security setup for tests may be complicated, they may be running in "crazy" environments such as IDEs.
  • Other modules/plugins can have their own policy requesting additional permissions. But in order for this to work, they have to properly use AccessController.doPrivileged etc.
  • Some of the more dangerous code such as scripting, tika, etc has explicit sandboxes that drop more privs to run that code.
  • there's a seccomp sandbox too (disabling fork/exec). Check /proc/<pid>/status and look for Seccomp: 2 to confirm its on. Similar stuff happens on windows/mac/bsd
  • Recommended reading: https://www.oracle.com/java/technologies/javase/seccodeguide.html

@rmuir
Copy link
Contributor

rmuir commented Apr 12, 2021

I will try to do a pass on this stuff tonight and really confirm policies etc are good. cc @uschindler

@uschindler
Copy link
Contributor

uschindler commented Apr 12, 2021

I can definitely say that in the main security policy the folowing lines are obsolete, according to: https://github.com/apache/lucene/blob/releases/lucene-solr%2F8.8.1/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java#L385

The other stuff there is from pre Java9 early access releases. This is IMHO obsolete:

// java 9 "package"
permission java.lang.RuntimePermission "accessClassInPackage.jdk.internal.ref";
// NOTE: also needed for RAMUsageEstimator size calculations
permission java.lang.RuntimePermission "accessDeclaredMembers";

The second one may be moved to tests. Lucene core no longer used ramusageestimator.

@uschindler
Copy link
Contributor

BTW, the policy files have wrong license header, @nknize

@rmuir
Copy link
Contributor

rmuir commented Apr 12, 2021

I took a pass through all the files (core policies mentioned above, and every plugin-security.policy) and didn't see anything unexpected.

If you are worried about network connectivity, you can just git grep SocketPermission to your own satisfaction, but it looks reasonable to me.

@nknize
Copy link
Collaborator

nknize commented Apr 13, 2021

Opened PR #531 to fix the license header on the security policy files.

@setiah
Copy link
Contributor

setiah commented Apr 22, 2021

I took a pass through all the files (core policies mentioned above, and every plugin-security.policy) and didn't see anything unexpected.

Thanks @rmuir for looking into the policy files.

I am taking a look into OpenSearch security (in general) to develop a better understanding and document it somewhere. Here are some of the initial findings.

OpenSearch security initialization flow during bootstrap

  • JVM starts
  • Apply system call filter - another level of security outside of JSM, to prevent OpenSearch from forking any processes.
  • Native system calls and adds shutdown hooks - ProcessProbe, OsProbe, JvmProbe
  • Enable security manager - all security policies are applied.
  • Load plugins
  • Bootstrap checks - like SystemCallFilterCheck, G1GCCheck AllPermissionCheck, DiscoveryConfiguredCheck, etc.
  • Enables network - for cluster formation

OpenSearch w/ Java Security Manager

  • OpenSearch uses a custom security manager, an extension over the default, to work around these
    design flaws in Java security.
  • The Policy class is extended to include static and dynamic permissions into account.
  • The JVM doesn't start with security manager, instead the JSM is turned on later for the two reasons explained in detail here - assigning file permissions, and running native code.
  • Scripts
    • Groovy based scripts, assigned minimal permissions, but the sandboxing is not adequate enough for reasons. (Something we could potentially look into improving?)
  • Plugins
    • are restricted with their own security policy and have their own bootstrap checks. The plugin security policy is defined in plugin-security.policy. This could mean any external plugins may come with unrestricted permissions.

Other security constructs

  • There's a bootstrap check that doesn't allow to run the opensearch process as root user.

@rmuir
Copy link
Contributor

rmuir commented Apr 22, 2021

@setiah yes, thats a great summary! To answer some of your questions:

Groovy based scripts, assigned minimal permissions, but the sandboxing is not adequate enough for reasons. (Something we could potentially look into improving?)

I don't see a module/plugin for groovy scripts anymore. This is a very good thing. So that comment is just outdated/obselete. Looks to me like the only scripting languages here are expressions, painless, and mustache which are all relatively safe in comparison.

Note: there's also a ClassLoader-based filtering mechanism, that can be used with scripts to filter which classes can be "seen", example: https://github.com/opensearch-project/OpenSearch/blob/main/modules/lang-expression/src/main/java/org/opensearch/script/expression/ExpressionScriptEngine.java#L197-L204 . You can see how the restrictions integrate into the policy: https://github.com/opensearch-project/OpenSearch/blob/main/modules/lang-expression/src/main/plugin-metadata/plugin-security.policy#L37-L44 . I just mention it as it doesn't actually depend on securitymanager, its really just a classloader thing. It just utilizes a custom permission (ClassPermission) for convenience/integration.

Plugins are restricted with their own security policy and have their own bootstrap checks. The plugin security policy is defined in plugin-security.policy. This could mean any external plugins may come with unrestricted permissions.

It is by design that core server should have least-privilege, and modules/plugins do any sketchy stuff that requires additional permissions. This keeps any elevated stuff contained to a smaller area (example: only networking module allowed to do network calls). Module/Plugin must declare it in their security file and properly use AccessController.doPrivileged. It used to have android-style confirmation of the additional permissions in the CLI when you install them! (Not sure if this is still happening).

Given that securitymanager looks to be deprecated and removed from the jvm completely (unless someone complains): https://openjdk.java.net/jeps/411, other steps should to be taken to secure this thing IMO. I will list a few ideas, I realize these will likely be unpopular, but something must be done:

None of this stuff is easy, especially testing. And sandboxing with the OS is a lot harder than via security manager because you have to take care of JDK internals (what system calls does it make, what files does it use, etc). But I think its a bad idea to just remove security manager and do nothing at all and pretend like everything is OK :)

@setiah
Copy link
Contributor

setiah commented Apr 23, 2021

Thanks @rmuir for providing this wealth of information and useful context.

It used to have android-style confirmation of the additional permissions in the CLI when you install them! (Not sure if this is still happening).

Right, that's still happening for plugin installation reference

Given that securitymanager looks to be deprecated and removed from the jvm completely (unless someone complains): https://openjdk.java.net/jeps/411, other steps should to be taken to secure this thing IMO. I will list a few ideas, I realize these will likely be unpopular, but something must be done:

Interesting point on Security Manager deprecation and thanks for pointing this out. These are all great ideas, and probably we should dig deeper into each of these. This seems like a BIG topic in itself. Shall we create a separate issue "Security model for OpenSearch" to discuss path forward on this?
Maybe we can close this issue since you have confirmed the JSM policies look secure. WDYT? cc: @mikemccand @nknize

  • auth should be mandatory to hit any of the APIs. It is 2021 :)

+1, I think authentication is a must have. Probably we can still keep authorization as optional (plugin-supported). WDYT?

@minalsha
Copy link
Contributor

minalsha commented Sep 7, 2021

Closing this issue as confirmed by @rmuir that he does not see any issue.

"I took a pass through all the files (core policies mentioned above, and every plugin-security.policy) and didn't see anything unexpected."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request
Projects
None yet
Development

No branches or pull requests

6 participants