Hi Uwe, that post of mine surely took an unexpected & interesting turn. Thank you for the detailed explanation, it is always enjoyable to see the rationale / line of thought behind a decision :-) Do you know how the performance characteristics of statically compiled Groovy and the new Groovy 3.0 Java lambda support compare to Painless ? Cheers,mg
-------- Ursprüngliche Nachricht --------Von: Uwe Schindler <uschind...@apache.org> Datum: 02.04.18 12:11 (GMT+01:00) An: dev@groovy.apache.org, 'Jochen Theodorou' <blackd...@gmx.org>, d...@groovy.incubator.apache.org Betreff: RE: new MOP under Java9 module system findings Hi, > I think I found the article I was referring to: > https://www.compose.com/articles/elasticsearch-security-update-groovy- > scripting-dropped/ > (2015-03): > "After talking with the Groovy developers, Elasticsearch have decided > that Groovy will never be sufficiently safe in a sandbox and have > removed it from the list of sandboxed languages." > > ( I have used Solr, but not Elasticsearch. It seems they now have > "Painless", a Groovy-subset-compatible language as their main scripting > language: > https://www.elastic.co/guide/en/elasticsearch/painless/6.2/painless- > specification.html > ... ) Yes, that script language was designed from scratch with only invokedynamic and MethodHandles in mind. It also uses a pure whitelist-approach (explicit whitelisting what you can call, so no blacklists needed), but still allows most groovy syntax. As Elasticsearch is very performance critical (e.g. you may call a script for every search result which may be up to billion of times per search execution), we tried to avoid boxing and dynamic lookups by using extensive caching in the indy call sites. On the other hand it still allows the "def" type. The compiler tries to guess most types during compilation and only if it cannot guess the type it writes an invokedynamic. But nevertheless, by using invokedynamic with good caches (we go up to megamorphic if needed!) the slowdown by using invokedynamic for most scripts is neglectible!!! (less than 10% when full hotspot compilation is done). The main reason for creating a new script language was not only security (this was just good for selling it to users at that time), but also speed. Groovy behaves very bad when types are variying and especially in the "def" case it uses way too much reflection (the invokedynamic part has no good caches and still uses reflection all the time on the dynamic lookup in the call site type handler). The script language also supports lambdas and method references, but in contrast to Groovy, those are lambdas, not closures! So it uses LambdaMetaFactory (actually it uses it's own implementation, because LambdaMetaFactory is too strict with types and has some bugs in Java 9 that make it behave differently than the spec; java code does not see this, but dynamic languages break heavily with Java 9 when they use LambdaMetaFactory). As said before, the compiler tries to guess all types at compile time and passes the native types to the invokedynamic, unless it can directly generate a real method call (everything is known). For dynamic ("def") stuff it delegates everything to DefBootstrap (https://goo.gl/DdNJJG), which has several modes: - simple method calls: here it uses a polymorphic cache with up to 5 receiver items (the usual guardWithTest chain using MethodHandles). Once it sees more receivers, it reverts the whole cache and goes megamorphic (it builds a methodhandle that calls ClassValue.get on every call). This is slower, but better than resolving every time. Keep in mind: This one has no bugs with ClassValue, as all seen types are local and can be cleaned up by GC correctly. The problems of Groovy with ClassValue are homemade, sorry. ClassValue works perfectly, if you do not misuse it!!! Unfortunately, Groovy has a monomorphic cache only, which was one reason why groovy was behaving bad in Elasticsearch! - fields, array loads/stores: same as method calls, it just translates to a methodhandle like for simple method calls (including all caches), as mentioned above. - (binary-)operators: As you have mostly 2 variable types here, it's hard to build good caches. So it goes for a monomorphic cache here. In most cases this is not an issue, as operator constructs don't have varying types (unless both sides are DEF). Still, once seen at runtime it stays constant for most cases. - references (these are method references): Like in Java, lambdas (with/without captures) are also compiled to static/instance methods in the script class. Lambdas are also tried to be resolved at compile time, but you can also call a method taking a functional interface on the "def" type! In that case the above "references" variant is used (the DefBootstrap then delegates after type resolving to the LambdaBootstrap). At the end of an invokedynamic call, field access, array lookup, map lookup, list lookup, lambda lookup is just a compiled method handle including all MIC, PIC, Megamorphic caching logic (using those many methods in Java's "MethodHandles" class for combining MHs)! Except for Bootstrapping method references, no byte code is generated at runtime (of course java runtime does it, but we do not). We just combine methodhandles! Most static definitions that are used in created method handles are part of the "Def" (https://goo.gl/g7GD5t) and "DefMath" (https://goo.gl/AN4SJ1) classes. If you look at Def.java, you also see many cases where it backports some MethodHandles stuff only available in Java 9+, but falls back to Java 9 native code when Java 9 is used. Actually some methods missing in Java 8's MethodHandles implementation were added to Java 9, because of our feedback (e.g. array length lookup for implementing array[].length using invokedynamic). One thing: Because of the strict types and complexity/slowness with caching, Painless does not allow method overloading. The above whitelist uses alias names for methods that use overloading. One famous example are the group() methods on regex matchers (named, unnamed groups). Some users complained, but that's under control. The whitelist also adds addon methods (like Groovy) to some java classes in the same way like aliases. > > In my opinion that project is wrong, because the security manager > > mechanisms provide enough protection. The problem is that rarely > > anyone can use a security manager properly. Anyway... Groovy won't be > > able to do any call Java cannot do in principle in this version. That > > is not because of keeping security in mind, that is more because of > > the module system, that enforces this This is partly right, but also not always applicable in that simple form. Elasticsearch uses SecurityManager to encapsulate all of its plugins, script engines,... and also itsself. It prevents stuff like deep reflection (setAccessible is completely disallowed anywhere in Elasticsearch) or System.exit/halt(). File system is also restricted to not escape config/index folders. I think Elasticsearch's implementation of SecurityManager is one ogf those implementation that really work correctly. It opened a lot of bug reports in foreign projects to fix their code (e.g, missing doPrivileged or calls to setAccessible without proper try/catch). Of course this helps to make Elasticsearch safe for the typical security issues (code escapes sandbox). But on the other hand it does not help, if code in a script calls some public class in Elasticsearchs's core and executes a command to delete an index from inside a query script. Of course you could also guard all public APIs of Elasticsearch with securitymanager, but as you know, many stuff is very performance critical, so you cannot really guard everything. Of course in the future, all access to lower level "Apache Lucene" or internals can be shielded by the module system, but that's not yet possible (Elasticsearch has Java 8 as minimum requirement). Groovy has the functionality of blacklists (so you can exclude certain classes from being called by scripts), but Elasticsearch decided to design it's script language the other way round. It is solely "whitelist" based. In short, you cannot access anything from the Elasticsearch/Java code that is not explicitely whitelisted (that's called "Definition" in Painless): https://goo.gl/ehdjvh with those whitelists: https://goo.gl/adCU88 > Whatever the reason it will be more secure, my line of thought was, that > if some (clever) way to work around this exists, to maybe still not go > along that route, even though I don't like to see this feature go and > people who use this in tests will surely miss it... > (Of course that argument is mute if they could have fixed their problem > through proper security manager use). I agree with that! That was the main reason to redesign the script language in Painless. It was made mostly compatible to Groovy, but the backend and compiler was designed to use MethodHandles and Invokedynamic only. There is no reflection inside (some parts of the above definition use unreflect, but that's more on startup only when building the whitelist graph, because reflection allows to better inspect stuff at runtime, while MethodHandles are typed from the beginning - you cannot get a methodhandle without exact types). During execution of a script there is nowhere a call to any reflective field/method nor is there setAccessible anywhere. The Invokedynamic call site does everything (includes caching) with MethodHandles: DefBootstrap, LambdaBootstrap. BTW: There are also some goodies in this script engine: If it figures out at runtime, that Java 9 is used, the byte code generated does not use StringBuilder when concatting strings, instead it uses invokedynamic with StringConcatFactory. The code to do that is quite "generic" and applied with a subclass of ASM's GeneratorAdapter: https://github.com/elastic/elasticsearch/blob/6dadce47613a3c69d928940bcc1b2043e0a0184a/modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java#L238-L292 (if I have some time, I will extend it to use the better makeConcatWithConstants() to improve strings with many constant parts). Uwe
