opensearch-project / OpenSearch

🔎 Open source distributed and RESTful search engine.
https://opensearch.org/docs/latest/opensearch/index/
Apache License 2.0
9.12k stars 1.69k forks source link

[RFC] Replace Java Security Manager (JSM) #1687

Open reta opened 2 years ago

reta commented 2 years ago

Is your feature request related to a problem? Please describe. It has been announced a while ago that SecurityManager is going to be phased out from the JDK. The first step, the deprecation of the SecurityManager (JEP-411), has been landed in JDK 17 and issues the following warnings on OpenSearch builds or server startup:

WARNING: System::setSecurityManager will be removed in a future release

The JDK 18 pushes it even further and now fails on startup (see please https://bugs.openjdk.java.net/browse/JDK-8270380), running OpenSearch builds or server on JDK 18 EA fails with:

Caused by: java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release
    at java.base/java.lang.System.setSecurityManager(System.java:416)

It now requires JVM command line option to enable it explicitly using (see please [1]):

-Djava.security.manager=allow 

Describe the solution you'd like There is no alternative or replacement for the SecurityManager (to understand why, Project Loom is to "blame"), see please [2]. One of the options is to just drop it, it sounds risky but combined with Plugin Sandbox (see please [3], [4]) it may sounds like a viable option. Other options include (but not limited to): bytecode instrumentation, java agent, custom classloader.

Describe alternatives you've considered We could keep it as long as we can, but once removed from the JDK, it will be a problem.

Additional context See please links.

[1] https://inside.java/2021/12/06/quality-heads-up/ [2] https://inside.java/2021/04/23/security-and-sandboxing-post-securitymanager/ [3] https://github.com/opensearch-project/OpenSearch/issues/1572 [4] https://github.com/opensearch-project/OpenSearch/issues/1422 [5] A possible JEP to replace SecurityManager after JEP 411

dblock commented 2 years ago

@nknize suggested we remove security manager in 2.0, labelling issue as such - once we have agreed here on what to do for this issue let's open a campaign parent issue in https://github.com/opensearch-project/opensearch-plugins/

reta commented 2 years ago

@dblock would you mind if I submit a small patch for 1.3.x+ so it could be run on JDK 18? Thank you

PS: To clarify why, JDK 18 is scheduled to be released in March, right around 1.4.x (planned) release, I suspect a number of people may give it a try. The change is only adding the command line property, non breaking.

dblock commented 2 years ago

@dblock would you mind if I submit a small patch for 1.3.x+ so it could be run on JDK 18? Thank you

PS: To clarify why, JDK 18 is scheduled to be released in March, right around 1.4.x (planned) release, I suspect a number of people may give it a try. The change is only adding the command line property, non breaking.

I'm A-OK with anything non-breaking on 1.x.

nknize commented 2 years ago

You mean something like adding support to disable the security manager via -Djava.security.manager=disable? (EDIT: I should've read past the first line :) )

I suspect tests will blow up since the test infrastructure leverages a custom SecurityManger via SecureSM. That's going to be more impactful. I'd love some thoughts from @rmuir or @uschindler on this as they are much closer to the JDK security bits than I.

rmuir commented 2 years ago

I think the issue is written up correctly. You'll want to set -Djava.security.manager=allow from startup scripts (e.g. .bat/.sh), and from gradle when running tests? Otherwise System.setSecurityManager() will fail.

Lucene uses a custom security manager too, no issues on JDK18. we just initialize it differently than opensearch, right at JVM startup time: -Djava.security.manager=org.apache.lucene.util.TestSecurityManager.

But in your case here, it is a little different because system starts up with no security manager, then parses some config files and maybe does a few evil things on startup, then it installs security manager via System.setSecurityManager(). That's the difference, the deferred initialization. So now for JDK18 you have to set "allow" property for that call to not fail.

rmuir commented 2 years ago

Separately, as far as alternatives, I can suggest a few things:

  1. Keep the SystemCallFilter. This is unrelated to security manager and will stop RCE dead in its tracks, as it disables fork()/exec() etc completely in an irreversible way.
  2. Look into enhancing the systemd unit to compensate. You can do a lot here, such as allow/block lists of filesystem paths, and more. Recommended introduction. Especially file paths would be great, if you have a directory traversal vulnerability, it is way better to fail with a filesystem error than to transfer some private files. But in addition to file paths, you can also do fancy stuff such as system-call filtering (except for fork/exec which is why you still need to keep part 1), capability drops, etc.
  3. consider hardening Docker environment too. current entrypoint just runs the shell script, maybe it could instead use the systemd unit, to also benefit from work already done above.
  4. adjust existing classloader filtering: example. The filtering-classloader currently integrates with security manager, just as a convenient way to provide a list of allowable classes, but it doesn't have to work this way. It can be changed to get its list of allowed classes some other way, and then things like scripting languages at least keep that protection.

I don't recommend directly going the LSM route (AppArmor, SELinux, etc). There's a lot of complexity to those, and its so system-specific which if any are even available. I'd start with systemd which is basically universal now on linux systems, and it gets you the biggest wins anyway (e.g. filtering filesystem and so on).

rmuir commented 2 years ago

Another win for stuff like ingest-attachment would be to just run the tika server (separate service/container) and have this plugin call out to it with a REST call. IMO it would be better security for using tika and they provide such a server these days. Then the tika could run in its own stricter separate sandbox.

but that strategy won't work for all the code: There's no one-size/fits-all solution. For example, things like analysis modules/plugins are extremely performance sensitive, and really need to just be passed to IndexWriter. At the same time, these plugins have less security risk (compared to e.g. Tika or scripting languages), so it's not a huge deal: they are just exposing lucene analyzers :)

reta commented 2 years ago

Thank you very much, @rmuir

I think the issue is written up correctly. You'll want to set -Djava.security.manager=allow from startup scripts (e.g. .bat/.sh), and from gradle when running tests? Otherwise System.setSecurityManager() will fail.

That is right.

rmuir commented 2 years ago

I've also made my opinion loudly clear on twitter that removing SecurityManager without replacement is a bad idea for java right now. At least providing a "replacement" first (ideally enabled by default), to help protect server-side apps against the worst vulnerabilities, is really needed. Java is filled with security landmines.

Doubt anything will change on the java side, but I tried. I don't have the resources/energy to write up JEP proposals or anything to try to make real change here though, sorry.

reta commented 2 years ago

Thanks @rmuir , I think the large part with respect to "what the replacement should be" is still unknown, as it is dictated by Project Loom that is not there yet. But I do 💯 agree on the point: removing SecurityManager without replacement is a bad idea.

rmuir commented 2 years ago

if you think of the entire internet (not just opensearch), i really do feel that something similar to the openbsd pledge() api would be at least a minimal replacement. process-wide: drop permissions to fork/exec (RCE), maybe drop network connect() permissions to hosts you don't need, maybe drop permissions to file paths you don't need. In many cases, perhaps the OS can enforce the functionality, in other cases, maybe java needs to do it.

but there's also the separate problem that java includes insecure functionality like JDNI ("landmines"), by default. Besides sandboxing, we need to get good secure defaults here and disable dangerous crap by default. it is a multi-pronged approach.

Pallavi-AWS commented 2 years ago

Do we have a decision on whether OpenSearch will deprecate SecurityManager in a future release or will command line option be used? If it will be deprecated, will there be a replacement? @dblock @nknize @rmuir. thanks,

reta commented 2 years ago

@Pallavi-AWS the recent (one of many) discussions on OpenJDK mailing list hint there won't be replacements for SecurityManager (very likely, at least) as well as there won't be suitable mechanisms provided for implementing your own. For JDK-18, we explicitly allow SecurityManager but there is no official decision being made on deprecation since no replacement is available.

[1] https://mail.openjdk.java.net/pipermail/security-dev/2022-April/029643.html

rmuir commented 2 years ago

i recommend to keep using it until it completely stops working. why would you voluntarily disable a security feature unless you have to?

nknize commented 2 years ago

Do we have a decision on whether OpenSearch will deprecate SecurityManager

It's already deprecated in the jdk and can be found in the build logs: WARNING: System::setSecurityManager will be removed in a future release.

will there be a replacement?

This is still being worked and there are already some great suggestions on this issue. In the meantime, we planned to keep using it until it stops working and will converge on a plan before upgrading to a jdk that removes it completely.

dblock commented 1 year ago

@davidlago Do you have a list of CVEs that were mitigated since the fork by JSM? For example, for the log4j RCE Security Manager + JDK >8 prevented LDAP/RMI connections. I think it will be useful to evaluate any replacement.

davidlago commented 1 year ago

Log4j is the big one that comes to mind. I'll take a pass at some of the ones we've seen and see if I can find others. Regardless... it only takes one :) I mean, even just a high severity/critical one averted/mitigated by it is a good reason to not lower our guard there.

varun-lodaya commented 1 year ago

Jumping onto this old thread.. Generally speaking, JSM definitely adds to the defence in depth. JSM provided some protection, if not complete, for some of the recent CVEs, log4j being one as you'll mentioned. However, given the deprecation path of JSM and operational overhead of maintaining JSM (as called out in the JEP (https://mail.openjdk.org/pipermail/security-dev/2022-April/029643.html), I liked the alternatives mentioned by @rmuir.

Class loader protection (also mentioned in JEP): We are investigating into this to add better access controls

SystemCallFilter as mentioned by @rmuir again sounds promising

SeLINUX: This is a very powerful tool and can help get much better protection at system level, then JSM. However, given that it works at kernel level, we need to figure out how we enable/provide this in our OpenSearch bundles.

uschindler commented 1 year ago

I general, like in Lucene core, we should keep support for SecurityManager/AccessController as long as possible. If it gets "disabled" in JDK (by making everything a NOOP), we do not need to care. If it gets really removed (that may not happen before JDK 21 LTS), we have to deal with that, e.g., using MethodHandles in Lucene's core. In Lucene, my idea is to replace all AccessController#doPrivileged calls by a MethodHandle that is replaced by a noop in recent JDKs. MethodHandles keep the call stack, so the caller-sensitive method is preserving its use. Anothe ridea would be a functional interface.

rmuir commented 1 year ago

no need for methodhandles or any crazy shit like that. The JEP tells you that they won't remove stuff in this way, they will make things no-ops: read it.

uschindler commented 1 year ago

Please read what I said: "If it gets really removed (that may not happen before JDK 21 LTS), we have to deal with that, e.g., using MethodHandles in Lucene's core"

Thanks.

dblock commented 1 year ago

I general, like in Lucene core, we should keep support for SecurityManager/AccessController as long as possible.

My question is still why? That's why I asked @davidlago of what actual CVEs JSM mitigated. There's some effort to "move security into core", with that, @rmuir @uschindler what is your argument for not removing JSM?

uschindler commented 1 year ago

I was talking about Lucene. We can't remove the AccessController.doPrivileged blocks, as otherwise downstream projects like yours won't be able to differentiate between actions triggered by Lucene or malicious code. So Lucene will remove our parts later. We don't do security, we just provide the plugin points like any other Java library should have done, too (there are some which ignored AccessController, but most of them added correct doPrivileged blocks).

So in short: Lucene will keep its blocks forever (although they may get noops). Most other libraries do the same. At moment where java 11 and java 17 are still used in wide range, please, please leave current code enabled. The "why" was explained before.

Some additional idea that came to me in summer while talking on conferences, improving the class loader / instrumentation variant: Forbiddenapis may be used as JVM agent (or for plugins/scripts in their classloader). If there's need, the Forbiddenapis signature files could be packaged with an application and through bytecode instrumentation it could deny loading classes (also bytecode generated at runtime) that has specific signatures to JNDI or Java Serialization. I think that can be done easily with some code around Forbiddenapis to invoke Checker.

rmuir commented 1 year ago

My question is still why? That's why I asked @davidlago of what actual CVEs JSM mitigated. There's some effort to "move security into core", with that, @rmuir @uschindler what is your argument for not removing JSM?

Because currently I don't see replacements for a lot of the functionality. Meanwhile the protection still works so why discard the only security mechanism that you have? I think I explained above, but to summarize for specific vulnerabilities that are of concern (e.g. have happened before), in a world without a security manager, I think the easiest win is to harden the systemd service, Currently it is very weak and insecure: https://github.com/opensearch-project/OpenSearch/blob/main/distribution/packages/src/common/systemd/opensearch.service

These are some of the historically problematic issues that the security manager prevents... aka the worst-of-the-worst:

I recommend looking at a secure systemd service as an example, and comparing it to the current one, here is a good one: https://github.com/archlinux/svntogit-community/blob/packages/dnscrypt-proxy/trunk/dnscrypt-proxy.service

You may also use systemd-analyze security opensearch.service to track your progress, it will suggest improvements.

rmuir commented 1 year ago

I dropped in the current opensearch.service and analyzed it so you can see what I mean:

→ Overall exposure level for opensearch.service: 8.8 EXPOSED 🙁 Full output: opensearch-analysis.txt

Compare this to e.g. my nginx.service: → Overall exposure level for nginx.service: 0.9 SAFE 😀 Full output: nginx.analysis.txt

reta commented 1 year ago

Thanks @rmuir, hardening systemd would make sense in any case, w/o JSM, may be you could create an issue for that for OpenSearch? We also have to support Windows and other distributions / platforms where systemd is not available, so it leaves us mostly with JSM only (for now at least).

rmuir commented 1 year ago

We also have to support Windows and other distributions / platforms where systemd is not available, so it leaves us mostly with JSM only (for now at least).

Are you sure about that? https://devblogs.microsoft.com/commandline/systemd-support-is-now-available-in-wsl/

reta commented 1 year ago

We also have to support Windows and other distributions / platforms where systemd is not available, so it leaves us mostly with JSM only (for now at least).

Are you sure about that? https://devblogs.microsoft.com/commandline/systemd-support-is-now-available-in-wsl/

I mean WSL technically is not Windows native deployment model, plus not everyone keen on using systemd even when it is supported (I am not devops guy, but I frequently have seen debates on the matter).

uschindler commented 1 year ago

We also have to support Windows and other distributions / platforms where systemd is not available, so it leaves us mostly with JSM only (for now at least).

Are you sure about that? https://devblogs.microsoft.com/commandline/systemd-support-is-now-available-in-wsl/

I mean WSL technically is not Windows native deployment model, plus not everyone keen on using systemd even when it is supported (I am not devops guy, but I frequently have seen debates on the matter).

The problem is that systemd is only supported in the WSL2, so actually you need to run Opensearch as a Ubuntu Linux application inside WSL's Ubuntu distribution (there may be other distributions, but that's the default one shipped by Microsoft). There's no native support in Windows Win32 NT kernel subsystem. So running Opensearch on Windows with Java for Windows as Windows Service can't use it.

rmuir commented 1 year ago

Well, java announced it is dropping its os-independent sandboxing tool. And you guys aggressively want to remove it without nothing in place and are asking me to defend why you wouldnt do that?

It is clear you haven't thought this through. I think its enough to just run well on linux and then run as container on mac/windows. Probably controversial but easy way to reduce complexity and testing.

I've been doing this stuff a long time and never actually seen any serious server running on windows.

You'll have to change something, if you want to keep it secure, seems like the right tradeoff to me.

dblock commented 1 year ago

Well, java announced it is dropping its os-independent sandboxing tool. And you guys aggressively want to remove it without nothing in place and are asking me to defend why you wouldnt do that? It is clear you haven't thought this through.

Me personally, I am asking because I value your and other people's opinion, @rmuir, and am trying to think it through before doing anything. 😅

pfirmstone commented 1 year ago

I just stumbled on this interesting conversation.

Remember Java's deserialization flaws, the source of gadget attacks? Had the SecurityManager API been utilised properly, these could have been simply avoided, let me explain:

  1. We use our own deserialization framework, it has a security check, which checks whether the caller has permission to deserialize. We also have another permission for class loading. Java should have had these by default.
  2. We use TLS network connections, the calling thread always runs with the remotely authenticated calling subject. No authentication, no connection.
  3. If the calling subject doesn't have permission to deserialize, that's it, no Object data is deserialized over the connection.
  4. If the subject has permission to deserialize (trusted), do they have permission to load classes? Not necessarily.
  5. We have a public API for Serialization, it's intended to allow serialization frameworks to access object state, for serialization and deserialization.
  6. We have an input validation API for deserialization, it only uses constructors, so serialization graphs with circular references cannot be deserialized directly. In effect we have implemented a more secure subset of Java serialization. Graphs are deserialized from leaf nodes, to branches, to the trunk, if any object in the graph cannot be validated, an exception is thrown, prior to Object's default constructor being called. Object creation is atomic, if input validation fails, the object is not created and cannot be reached by a finalization thread, therefore preventing many gadget attacks.
  7. We reimplemented the Java Policy Provider - with our own high scaling, low overhead policy provider.
  8. We have dynamic permissions - permissions can be granted dynamically at runtime, during authentication for example. We also have revocable permissions, however not all permissions are revocable as some allow the objects they defend to escape. Some permissions like deserialization are revocable at runtime.
  9. We also have a class with similar methods to AccessController, this class has existed since 2004. This implementation encapsulates AccessController, so provides a way to abstract the implementation of AccessController.
  10. We have a policy file generation tool - run your code in a simulation environment, it generates policy files based on least privilege principles, you then edit these files, adjusting your permission model as necessary.
  11. Our code is available under an AL2 license.
  12. It won't require a lot of effort to reimplement AccessController's stack walk.
  13. Java's public API can be instrumented. This is the largest task.
  14. Access to Java implementation API can be controlled by Java's modular framework.
  15. Unfortunately AccessController.doPrivileged methods will eventually be removed, we would need to develop strategies around dealing with that.
  16. We can instrument Guard.checkGuard() calls, currently they call the SecurityManager. The Permission and ProtectionDomain API's will remain. This will allow client code to continue using it.
  17. OpenJDK has been receptive to giving Java's implementation code ProtectionDomain's a CodeSource with a meaningful URL, allowing us to reduce the size of the trusted codebase. It was a mistake for Java's trusted codebase to become so large and encouraging people to use AllPermission so sparingly, this breaks least privilege principles.
  18. OpenJDK claims that ProtectionDomain's were for untrusted code, we didn't use them this way. In our software ProtectionDomain's often represent a Server Subject, by way of a service proxy, that's loaded into a ClassLoader (even if it's only a dynamically generated instance of java.lang.reflect.Proxy). A Proxy is a representative of a Service. For example a number of servers may participate in a transaction using their proxy's, the proxy ProtectionDomains appear on the call stack. If one ProtectionDomain doesn't have necessary permission, the transaction will fail atomically. This is all simple stuff that works very well, unfortunately the work that Li Gong started was never properly maintained.
  19. So we have this service based distributed server infrastructure. For us, the SecurityManager API is an effective access control layer, since we've already reimplemented components of the SecurityManager API and use it to it's full potential, it's probably a good place to start for reimplementation.

The code can be found here: https://github.com/pfirmstone/JGDMS

JGDMS is a large codebase, so security code has been isolated here, and there's also some initial reimplementation of the AccessController API: https://github.com/pfirmstone/HighPerformanceSecurity

pfirmstone commented 1 year ago

It's probably also worth noting that we use SM API's to establish our TLS connections, when I last checked, there was no replacement.

reta commented 1 year ago

Thanks a lot @pfirmstone for your thoughts

  1. Java's public API can be instrumented. This is the largest task.

That would require Java Agent or similar approach, is my understanding correct? (consulted https://github.com/pfirmstone/HighPerformanceSecurity#unfortunate-news-regarding-java-library-support-for-authorization-hooks)

  1. Unfortunately AccessController.doPrivileged methods will eventually be removed, we would need to develop strategies around dealing with that.

AFAIU the framework you are referring to uses SM under the hood, right?

On the separate subject, how does it fit into the JVM world when Project Loom becomes mainstream? (I believe SM was a major blocker for it).

pfirmstone commented 1 year ago

Hi Andriy,

Replies inline below.

On 9/02/2023 7:47 am, Andriy Redko wrote:

Thanks a lot @pfirmstone https://github.com/pfirmstone for your thoughts

13. Java's public API can be instrumented. This is the largest task.

That would require Java Agent or similar approach, is my understanding correct? (consulted https://github.com/pfirmstone/HighPerformanceSecurity#unfortunate-news-regarding-java-library-support-for-authorization-hooks)

Yes, that's correct.

15. Unfortunately AccessController.doPrivileged methods will
    eventually be removed, we would need to develop strategies
    around dealing with that.

AFAIU the framework you are referring to https://github.com/pfirmstone/HighPerformanceSecurity uses SM under the hood, right?

Correct, it has been in use for some time, about a decade, long before JEP411, it currently relies on AccessController, AccessControlContext, DomainController and Subject functionality.

It provides a replacement SecurityManager implementation with a non blocking cache, to avoid repeated security checks for the same context.  It also has a high scaling policy provider, that uses thread confinement to minimise blocking.

It provides a policy file generation tool, you set up your deployment environment, you run through all the deployment workflows and it records the permissions required.   You give it a list of system properties and it replaces matching string targets with properties to make the policy files more generic.   Then review, and edit the policy files and deploy.   This is a very locked down environment, if anyone tries doing anything that wasn't in the deployment workflow, they'll hit a SecurityException.

The performance impact is less than 1%.   Java's default implementations kill scalability, the Policy has a PermissionCollection cache that blocks, there's no tool to assist policy file generation, generally their assessment of SM in JEP411, is an assessment of their own implementation, which was practically unchanged since Li Gong implemented it over two decades ago.  I did offer to donate the code to OpenJDK, but one of the contributors had passed away and OpenJDK wouldn't accept an AL2.0 license.  Today we use the stack walk algorithm in AccessController, it has a low performance overhead.   Reading JEP411, there's a lot they don't know about Java authorization as they don't use it.   I've run the tests on heavily loaded hardware.

JGDMS is a modern evolution of Jini, put simply, it's services using authenticated secure remote method invocations.   It doesn't use RMI, instead it uses JERI (Jini Extensible Remote Invocation), which replaced RMI.    Jini was constrained to local networks due to IPv4 Network address translation, as it requires end to end connectivity.   Today it's used with IPv6, and has multicast global service discovery.

On the separate subject, how does it fit into the JVM world when Project Loom becomes mainstream? (I believe SM was a major blocker for it).

This is something that the OpenJDK team denied, at least when I asked them, I had suspected there may be an element of truth to it.

They didn't want to preserve the context for every thread, because it's mutable, would consume more memory.

Lightweight threads don't have privileges, which is a good thing, it means you can't attain privileges, or do anything privileged with them.

This means that any lightweight threads will have to call an Executor to perform any privileged tasks.   You can't use lightweight threads for establishing secure network connections, which is a pity, this would be a good use case, as network connections are blocking.  Maybe in future they'll modify the new Subject methods so you can.

Currently we use AccessControlContext for obtaining the Subject for establishing TLS connections.  I'm not sure whether the new Subject methods will preserve the Subject for TLS connections, currently they don't.    Blocking network calls are a big issue for thread pools, so I think they might do something.   In JGDMS remote calls are executed with their authenticated Subject, restricting their permissions.  So we can have any number of JVM's communicating over the network to each other, anywhere in the world and all are authenticating each other and restricting permissions based on who they authenticated.

Basically JEP411 is ripping out the good with the bad, I don't care about the SecurityManager implementation or the Policy provider implementation, neither matter.

These are the things that matter, that they're ripping out, these components were reasonably well designed and didn't hurt performance or scalability:

AccessController, AccessControlContext, DomainController, Subject::{doAsPrivileged, getSubject}

I'm hoping that we can at least have them retained, so we can instrument them too if we have to.   At least get them removed from deprecated for removal, so people don't remove their use from library code.

OpenJDK recently released this video https://www.youtube.com/watch?v=3HnH6G_zcP0

Cheers,

Peter.

— Reply to this email directly, view it on GitHub https://github.com/opensearch-project/OpenSearch/issues/1687#issuecomment-1423281487, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACYWKMRDPJWM32T2ICFGXZTWWQH57ANCNFSM5JWWDSKA. You are receiving this because you were mentioned.Message ID: @.***>

reta commented 1 year ago

Super insightful, thanks a lot @pfirmstone (yeah, I saw this video), more choice to consider but there are high risks AccessController & Co are going to be phased out.

pfirmstone commented 1 year ago

Yes, it does look that way.

We could make identical API's in another namespace and encourage people to migrate their code, initially this API could simply encapsulate the Java SM API's, then replace them at a later date with an implementation.

It needs to be a fairly low bar for developers, so it's just a namespace change.

For older binary code we could then create a tool to replace the classes using ASM.

Replicating the stack walk functionality relatively straightforward.

The difficulty is getting everyone to keep their privileged calls and their Subject::doAsPrivileged code.

Any suggestions for a namespace?

-- Regards,

Peter Firmstone

On 9/02/2023 11:04 am, Andriy Redko wrote:

Super insightful, thanks a lot @pfirmstone https://github.com/pfirmstone (yeah, I saw this video), more choice to consider but there are high risks AccessController & Co are going to be phased out.

— Reply to this email directly, view it on GitHub https://github.com/opensearch-project/OpenSearch/issues/1687#issuecomment-1423459841, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACYWKMQME2HI3THWG5DP7X3WWQ7DDANCNFSM5JWWDSKA. You are receiving this because you were mentioned.Message ID: @.***>

reta commented 1 year ago

The difficulty is getting everyone to keep their privileged calls and their Subject::doAsPrivileged code.

I agree, it is difficult even these days, but once the OpenJDK gets rid of SM, it would become nearly impossible to convince people to keep this "dead" checks.

Any suggestions for a namespace?

That's a good one, I have not thought about it to be fair :-)

pfirmstone commented 1 year ago

On 9/02/2023 11:26 am, Andriy Redko wrote:

The difficulty is getting everyone to keep their privileged calls
and their Subject::doAsPrivileged code.

I agree, it is difficult even these days, but once the OpenJDK gets rid of SM, it would become nearly impossible to convince people to keep this "dead" checks.

Yes, I can think of three types of developers:

  1. The ambivalent, who removes it only when they have to, it's on a todo list, they have other priorities.
  2. The bleeding edge developer who removes it as soon as it's deprecated by OpenJDK.
  3. Those who need it and want to replace it.

I think if we can make #1 a simple search and replace, add a new dependency, it reduces the work needed, changing code can introduce new unforeseen bugs, we'll pick up these developers if we can get the message out there.

2 is already gone.

3 will research any and all available solutions, if we provide it, they

will find us.

These calls are important to capture, as they prevent viral permissions.  If we have to grant a permission to all ProtectionDomain's on a call stack, because the developer didn't make a privileged call, without leaking sensitive information, that's a viral permission that does nothing for security, because it has to be granted to make the software work.

Any suggestions for a namespace?

That's a good one, I have not thought about it to be fair :-)

— Reply to this email directly, view it on GitHub https://github.com/opensearch-project/OpenSearch/issues/1687#issuecomment-1423477440, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACYWKMXSYJ2E3HMMOEPK5BDWWRBVTANCNFSM5JWWDSKA. You are receiving this because you were mentioned.

The first order of the day is to think of a suitable new namespace, the second, to implement a wrapper API, that can be directly substituted.   If someone owns a domain and is happy for us to use it?

I think security is a moving target, with implied liability, so rather than calling it security, name it by it's functionality instead?

eg. org.opensearch.authorize.*

I'm not a big fan of Authority, people usually end up abusing their authority power, authorize implies that you are giving consent to authorize someone to do something on your system.

For now I think we avoid substituting SecurityManager and Policy calls, Maybe we provide a Policy API, or some common subset of it, not sure about that one, might be better to wait for requests, to see what people need, it might be the case that we just simply provide the implementation, it's not easy to implement, in a scalable and secure manner.

SecurityManager sm = SecurityManager.getSecurityManager();

if (sm != null) sm.checkPermission(Permission p);

Can be replaced with:

Permission::checkGuard, so we can instrument Guard::checkGuard.

The classes in high performance security originate from these packages (maintained), it would be nice if I can pull the packages into an external dependency (we have users who use OSGi, so we try to avoid splitting packages):

net.jini.security

net.jini.security.policy

org.apache.river.api.security

org.apache.river.api.net

This Security class uses a delegation pattern, that encapsulates SecurityManager, AccessController and AccessControlContext.

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-platform/src/main/java/net/jini/security/Security.java

SecureClassLoader causes unwanted DNS calls, so we have RFC3986ClassLoader to prevent that.

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-platform/src/main/java/org/apache/river/api/net/RFC3986URLClassLoader.java

CodeSource::implies also causes unwanted DNS calls, so we have an RFC3986 based URI::implies to replace that.   Java's policy provider is susceptible to DNS cache poisoning attacks.

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-platform/src/main/java/org/apache/river/api/net/Uri.java

That last one wipes the floor with Java's crappy URI implementation.   Under the hood, it's using bit shift operations during normalization, that strictly comply with RFC3986 and RFC 5952

We have a ScalableNestedPolicy interface, that allows policy's to be nested, each policy can decorate, to allow policies to be broken up into separate concerns.

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-platform/src/main/java/org/apache/river/api/security/ScalableNestedPolicy.java

Message ID: @.***>

reta commented 1 year ago

I am continue looking into https://github.com/pfirmstone/HighPerformanceSecurity codebase (just trying to learn what you have implemented, impressive). From API perspective - I got it, but it seems like there is no agent implementation (or alike) available yet - is that right?

Why I am asking, I think with the approach you have described (and similar ones) we could instrument the core components, but this is only a small part of it. A slightly larger issue are plugins - the extensibility mechanism we do not have control over (from implementation perspective). If lights go off for SM today - we have a problem. The agent or similar instrumentation would be the only guardrail to prevent possibly harmful behaviour. Yes, we could ask plugin developers to use certain APIs for permission checks, but we could not enforce it. That's the most uncertain area, here is a black-box JAR - deal with it, I think the JGDMS at this point may not cover us, would it?

rmuir commented 1 year ago

The difficulty is getting everyone to keep their privileged calls and their Subject::doAsPrivileged code.

I agree, this is a real problem. From the library perspective, as soon as users see the "warnings" of deprecation, they are eager to remove anything related to it. Apache Lucene example: https://github.com/apache/lucene/issues/11801

pfirmstone commented 1 year ago

Thanks for the link, I'll mention it on OpenJDK.

-- Regards,

Peter

On 10/02/2023 1:55 am, Robert Muir wrote:

The difficulty is getting everyone to keep their privileged calls
and their Subject::doAsPrivileged code.

I agree, this is a real problem. From the library perspective, as soon as users see the "warnings" of deprecation, they are eager to remove anything related to it. Apache Lucene example: apache/lucene#11801 https://github.com/apache/lucene/issues/11801

— Reply to this email directly, view it on GitHub https://github.com/opensearch-project/OpenSearch/issues/1687#issuecomment-1424420455, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACYWKMRTNBTKTMTS4QH2RU3WWUHOJANCNFSM5JWWDSKA. You are receiving this because you were mentioned.Message ID: @.***>

pfirmstone commented 1 year ago

Some thoughts on AccessController, AccessControlContext, DomainController and Subject:

  1. Before removing these classes / methods, OpenJDK will create another JEP.   We need to retain as many uses of these methods inside OpenJDK as possible.   At that point in time, we can campaign for these classes and their uses, to be retained as no ops or null object patterns.  They may just ignore us, as they did with JEP 411, but this time, their excuse that it creates a maintenance burden for OpenJDK won't cut it, so we make enough noise on social media to get their attention.
  2. In the interim, we create an alternative API that developers can switch to.
  3. Future versions of Java will be insecure, at this time, SecurityManager has only been deprecated, they haven't started dismantling it yet.

I think Nicolas puts the risks into perspective, it's inevitable that bad actors will take advantage of powerful Java features at some point in time, perhaps it will be like Java Serialization Gadget attacks, Java Serialization was insecure for over a decade before it was really taken advantage of and became widely known, the consequences of that was goodbye applets, goodbye Java client market.   Maybe it's in the interests of bad actors to allow vulnerabilities to be widely deployed before taking advantage of them?

https://www.youtube.com/watch?v=uVob-4aXbxY

-- Regards,

Peter

On 9/02/2023 12:27 pm, Peter Firmstone wrote:

On 9/02/2023 11:26 am, Andriy Redko wrote:

The difficulty is getting everyone to keep their privileged calls
and their Subject::doAsPrivileged code.

I agree, it is difficult even these days, but once the OpenJDK gets rid of SM, it would become nearly impossible to convince people to keep this "dead" checks.

Yes, I can think of three types of developers:

  1. The ambivalent, who removes it only when they have to, it's on a todo list, they have other priorities.
  2. The bleeding edge developer who removes it as soon as it's deprecated by OpenJDK.
  3. Those who need it and want to replace it.

I think if we can make #1 a simple search and replace, add a new dependency, it reduces the work needed, changing code can introduce new unforeseen bugs, we'll pick up these developers if we can get the message out there.

2 is already gone.

3 will research any and all available solutions, if we provide it,

they will find us.

These calls are important to capture, as they prevent viral permissions.  If we have to grant a permission to all ProtectionDomain's on a call stack, because the developer didn't make a privileged call, without leaking sensitive information, that's a viral permission that does nothing for security, because it has to be granted to make the software work.

Any suggestions for a namespace?

That's a good one, I have not thought about it to be fair :-)

— Reply to this email directly, view it on GitHub https://github.com/opensearch-project/OpenSearch/issues/1687#issuecomment-1423477440, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACYWKMXSYJ2E3HMMOEPK5BDWWRBVTANCNFSM5JWWDSKA. You are receiving this because you were mentioned.

The first order of the day is to think of a suitable new namespace, the second, to implement a wrapper API, that can be directly substituted.   If someone owns a domain and is happy for us to use it?

I think security is a moving target, with implied liability, so rather than calling it security, name it by it's functionality instead?

eg. org.opensearch.authorize.*

I'm not a big fan of Authority, people usually end up abusing their authority power, authorize implies that you are giving consent to authorize someone to do something on your system.

For now I think we avoid substituting SecurityManager and Policy calls, Maybe we provide a Policy API, or some common subset of it, not sure about that one, might be better to wait for requests, to see what people need, it might be the case that we just simply provide the implementation, it's not easy to implement, in a scalable and secure manner.

SecurityManager sm = SecurityManager.getSecurityManager();

if (sm != null) sm.checkPermission(Permission p);

Can be replaced with:

Permission::checkGuard, so we can instrument Guard::checkGuard.

The classes in high performance security originate from these packages (maintained), it would be nice if I can pull the packages into an external dependency (we have users who use OSGi, so we try to avoid splitting packages):

net.jini.security

net.jini.security.policy

org.apache.river.api.security

org.apache.river.api.net

This Security class uses a delegation pattern, that encapsulates SecurityManager, AccessController and AccessControlContext.

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-platform/src/main/java/net/jini/security/Security.java

SecureClassLoader causes unwanted DNS calls, so we have RFC3986ClassLoader to prevent that.

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-platform/src/main/java/org/apache/river/api/net/RFC3986URLClassLoader.java

CodeSource::implies also causes unwanted DNS calls, so we have an RFC3986 based URI::implies to replace that.   Java's policy provider is susceptible to DNS cache poisoning attacks.

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-platform/src/main/java/org/apache/river/api/net/Uri.java

That last one wipes the floor with Java's crappy URI implementation.   Under the hood, it's using bit shift operations during normalization, that strictly comply with RFC3986 and RFC 5952

We have a ScalableNestedPolicy interface, that allows policy's to be nested, each policy can decorate, to allow policies to be broken up into separate concerns.

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-platform/src/main/java/org/apache/river/api/security/ScalableNestedPolicy.java

Message ID: @.***>

reta commented 1 year ago

Maybe it's in the interests of bad actors to allow vulnerabilities to be widely deployed before taking advantage of them?

I really hope this is not the case BUT, to be fair, this is a real risk.

uhlhosting commented 1 year ago
Apr 07 12:14:42 wazu systemd-entrypoint[1455556]: WARNING: A terminally deprecated method in java.lang.System has been called
Apr 07 12:14:42 wazu systemd-entrypoint[1455556]: WARNING: System::setSecurityManager has been called by org.opensearch.bootstrap.OpenSearch (file:/usr/share/wazuh-indexer/lib/opensearch-2.4.1.jar)
Apr 07 12:14:42 wazu systemd-entrypoint[1455556]: WARNING: Please consider reporting this to the maintainers of org.opensearch.bootstrap.OpenSearch
Apr 07 12:14:42 wazu systemd-entrypoint[1455556]: WARNING: System::setSecurityManager will be removed in a future release
Apr 07 12:14:44 wazu systemd-entrypoint[1455556]: WARNING: A terminally deprecated method in java.lang.System has been called
Apr 07 12:14:44 wazu systemd-entrypoint[1455556]: WARNING: System::setSecurityManager has been called by org.opensearch.bootstrap.Security (file:/usr/share/wazuh-indexer/lib/opensearch-2.4.1.jar)
Apr 07 12:14:44 wazu systemd-entrypoint[1455556]: WARNING: Please consider reporting this to the maintainers of org.opensearch.bootstrap.Security
Apr 07 12:14:44 wazu systemd-entrypoint[1455556]: WARNING: System::setSecurityManager will be removed in a future release

Is there any workaround for this yet?

reta commented 1 year ago

Is there any workaround for this yet?

No but this is just an warning (as per https://bugs.openjdk.org/browse/JDK-8264713, it cannot be suppressed), although it is annoying

reta commented 1 year ago

The GraalVM is taking interesting approach to snadboxing the guest languages (one of those could be Java):

peternied commented 1 year ago

Application scopes are being built that require explicitly opt-in of plugins and extensions to access different capabilities of OpenSearch, not a replacement for JSM but might address some of the associated problems

reta commented 1 year ago

Yet another issue with SecurityManager, the JDK-21 is coming with GAed virtual threads, however the OpenSearch won't be able to fully use them, as per https://openjdk.org/jeps/444:

Virtual threads have no permissions when running with a SecurityManager set.

And

Virtual threads are not active members of thread groups. When invoked on a virtual thread, [Thread.getThreadGroup()](https://download.java.net/java/early_access/jdk21/docs/api/java.base/java/lang/Thread.html#getThreadGroup()) returns a placeholder thread group with the name "VirtualThreads". The Thread.Builder API does not define a method to set the thread group of a virtual thread.

uschindler commented 1 year ago

I don't think there is a reason to use virtual threads. They are contra productive for Lucene workloads. And as all networking code in Opensearch is selector based, not even the Network layer needs virtual threads.