9 ms·
Proposal to change default annotation processing policy in JDK 23
- MarkSweep 2y agoI assume this is to prevent a file-dropping attack, similar to DLL injection. How hard is this to exploit in practice? Does javac include the current directory in the class path? Does it look in other directories that are easy for other users to drop files in? Also, how much are people running javac directly? I would guess a lot of people use build tools like Gradle or Ant that limit the class path, right?
- senorrib 2y agoUsing Gradle, ant or Maven still means, in the end, you’re calling javac. All it takes for this to be exploited is for the files to be dropped anywhere in the classpath.
- jillesvangurp 2y agoIt's an annotation processor. They generally require annotations in your own source code to kick in. As a security risk this is pretty minor. About on the same level as any software project with any dependencies risking running arbitrary code unless you audit those dependencies. This is of course a very real risk and it has affected a bunch of projects. But it's not really stopping people from using things like cargo, npm, etc. Overall, it makes sense to make the use of annotation processors a bit more explicit. With Kotlin this is kind of how it works as well. You have things like ksp that you have to configure explicitly if you want to use them. Additionally there are compiler plugins that you can configure if you need them. It's not a big deal to configure this explicitly. I actually prefer it over magic discovery mechanisms that are hard to debug when they don't work. So, good change that probably simplifies the build process a little.
- vips7L 2y ago> They generally require annotations in your own source code to kick in Right but you don’t know which annotation processor will actually run. Anybody could look for javax.persistence.Entity and do something. There’s no guarantee only your JPA provider will be running and looking at them.
- jillesvangurp 2y agoIn exactly the same way, unless you audit your dependencies, you have no idea what you are going to run. It all boils down to whether you trust your dependencies. The only difference here is that this is a compile time dependency, not a run-time dependency. But unless you checked it, there are no guarantees.
- vips7L 2y agoSure. Hypothetically, I have checked them. I have checked what I’m actually using, but the compiler is still executing things I didn’t know about. I only use CatUtils from org.catpache.commons. I’ve audited this single class that I use. I know it’s safe. It only contains a Map<String, String> of Latin cat names to their common English name, but what I didn’t know was the compiler magically running an annotation processor behind my back and it’s now modified all of my classes to throw MeowException whenever toString returns “dog”.
- kaba0 2y agoIt’s pretty easy to statically verify that, say, no dependency (even transitive ones) contain a System.exit call. It’s basically impossible to determine what your annotation processors will output besides running them yourselves (and their output may not be deterministic to begin with).
- marginalia_nu 2y agoNot sure how much that would help. As far as I understand if you have access to putting stuff in the class path surely you can just override Java classes and run arbitrary code that way.
- derefr 2y agoI think the difference is that annotation processors run arbitrary code at compile time. An org might have e.g. a CI with a build environment that’s not as well-sandboxed as the test environment for the built app, because a Java compiler isn’t generally expected to (and other than through annotations, usually doesn’t) expose arbitrary code execution abilities to the payload of code being compiled.
- hedora 2y agoIs it actually common practice these days to have Java repositories that do not contain the build scripts, packaging, etc? The last time I looked at maven, it seemed easy enough to have it run arbitrary code directly during the build.
- kaba0 2y agoThis is mostly about supply chain attacks, where transitive dependencies are at play. Those are mostly just already built jar files, that can still contain annotation processors, but not build scripts.
- derefr 2y ago> Is it actually common practice these days to have Java repositories that do not contain the build scripts, packaging, etc? It's not that they don't contain these things; but rather that you can (and people often do) set things up so that the build scripts + packaging can be "more trusted" than the source files. If you've ever tried to set up CI on e.g. Github for an open-source project, then you might be familiar with the concept of a "PR attack" — where an external contributor forks your project, submits a PR that adds malicious code to your build scripts (to e.g. exfiltrate your build-time secrets); and then your CI "helpfully" runs those build scripts (in order to e.g. evaluate that the PR compiles + passes tests + lints in order to determine whether it should be blocked from merging or not.) GitHub and others have come up with ways around "PR attacks", that involve treating triggered automation workflows for external PRs differently: in these workflow runs, the core of the workflow — the workflow manifest file — is sourced not from the contributor's branch, but rather from the base branch that the PR aims to merge to. Now, it's up to you, as a repo maintainer, to come up with a way to bootstrap that little bit of safety (protected workflow manifest) into whole-repo "PR attack" arbitrary-code-execution protection. But usually doing so involves: 1. moving as much of the logic for executing your build as possible out of the repo itself and into buildpacks / "action" repos; and 2. sourcing any scripts you do need for the build, not from the worktree of the external PR, but rather by having the workflow environment also check out the base branch, and then running those scripts against your worktree. (IIRC the most common pattern for this is that you check out the base branch worktree, blow away its src/ dir, symbolic-link the PR branch's worktree's src/ into the base branch as src/, and then run your build in the context of the base branch.) This approach is incomplete, however, if the PR's source files can themselves be the source of arbitrary code execution.
- rzwitserloot 2y agoHow hard is this to exploit in practice? Very - this is a silly update in OpenJDK's war against things like Project Lombok. It _seems_ easy to exploit: Just.. get any jar file containing an annotation processor on the classpath and it will be executed as part of `javac` - and almost every java build tool calls javac under the hood. However, this is misleading: _if_ somebody with malicious intent manages to either sneak a jar file into the build dependencies somehow, or manages to libxz-style take control of a commonly used dependency, the damage is done. That jar will also be on the classpath when running the app. So now we're running compromised code. Java is not a sandboxed thing, running malicious code inside a JVM is a bit like an XSS web attack: The game is lost. Totally and utterly. The fact that javac runs annotation processors by default until JDK23 does mean that a compromised build chain now runs _during the build_, whereas starting with JDK23 they'll only run when you run the app. This doesn't seem impactful to me; I'm having a hard time figuring out scenarios where this is meaningfully less bad. Developers tend to run the app they are writing. The odds a developer will build a source tree and never actually run what they built, seems insufficient to consider this a meaningful contribution to security. CI servers might be a useful place to look for 'systems that will run the build but will not run the app', except - no. Just about every CI tool will run some tests, thus, running the app, thus, running the malicious code. Which gets us back to: OpenJDK's backwards-compatibility breaking crusade. The OpenJDK team has also broken reflection (you can no longer access anything in another module that wasn't explicitly exported without command line switches, even though reflection used to be able to do this. Reflection has 'CARE! You are accessing APis that were not designed to be messed with!' written on the tin. It's.. the point of it). - same reasoning. "For security" without being particularly clear about how that update contributes to security. It's not about 'it is impossible to use reflection to cause serious damage' (it is very possible to do that, in fact). It's more about: .... if you are running malicious code inside a JVM, we've got much, much bigger problems. It's sort of like stating that security is improved by ensuring that it is no longer possible to open the front door from inside the house without a key. Seems nice - but, they're... already inside the house.
- j16sdiz 2y ago> That jar will also be on the classpath when running the app. No. In many maven config, runtime classpath is not the same as compile time
- vbezhenar 2y agoI assume this is to have more control over compilation process. Without annotation processor, you can expect your code to be compiled and behaved in an obvious way. With annotation processor all bets are off, your code and code that results from compilation are completely different entities. So with this switch being explicit, you might enforce politics like lack of annotation processors for better clarity. While security theoretically might be better, in practice with modern build tools there are enough ways to cause code execution, so it probably doesn't matter much.
- giamma 2y ago"This policy of implicitly running annotation processors by default may have been reasonable when the feature was introduced in JDK 6 circa 2006, but from a current perspective, in the interest of making build output more robust against annotation processors unintentionally being placed on the class path, the policy should not be the default anymore." This makes sense, especially in light of the growing number of supply chain attacks. However, given how many libraries a typical Java application has in the classpath, most of which are transitive dependencies of popular frameworks, I foresee many build jobs not producing working binaries anymore when JDK23 is adopted. Plus, I'd love to be proven wrong, but I could bet that most people will fix the issue by simply adding the "all-on" switch to the command line instead of carefully evaluating which annotation processors are really needed by their codebase. So I am afraid that in the long term this change will not improve security or reduce build times. Unless maybe JDK 23 comes with some tool to print a catalogue of the annotation processors found in the classpath, with their respective purpose and documentation, so that developers can make an educated guess about what they need or don't need.
- usrusr 2y ago> but I could bet that most people will fix the issue by simply adding the "all-on" switch to the command line Definitely this. This is a dev experience problem. > Unless maybe JDK 23 comes with some tool to print a catalogue of the annotation processors found in the classpath, with their respective purpose and documentation, so that developers can make an educated guess about what they need or don't need. My thoughts on as to how a solution to the dev-x problem might look like go in the same direction: have a way to provide a file with an accept list, another file with a deny list (vetoing accept list entries when matching both) and, this would be the key devx feature, a mode to automatically populate the accept list file from the full scan. Teams who'd be tempted to run the "all-on" mode could keep it running on auto-populate accept file, put the auto-populated accept file in versioning and see additions to the processor zoo in commits. And de-trusting a processor (or speeding up the build) would be as easy as copying a line to the deny file. Minor improvement that would probably come up at some point: the auto-generated accept file should better not automatically exclude entries exluded by the deny file so that replacing the deny with a subset from accept remains a straight-forward option, but chances are you might encounter some situations where some dev environments have a processor that other build environments don't have (this would not exist in a perfect world..) and that would cause undesirable changes to the auto-generated accept file. So you'd want a stronger deny option that forbids already during the scan, either with some special syntax in the deny file or (better I think) an optional file for "even higher priority deny". As to auto-generating documentation for the educated guess: clearly a trade-off between content and conciseness. My vote would be offering a way for processors that want to be helpful (or misleading, in the attack scenario!) to supply text for trailing line comments in the auto-generated accept file. For teams who do want to use the auto option. (trailing line comments are such an under-utilized magic compromise for conciseness/volume conflicts, probably because they aren't for readers running their editors with auto-wrap)
- twic 2y ago> As of the April 2024 JDK security updates, support for the "-proc:full" option has been backported to 17u (17.0.11) and 11u (11.0.23) for both Oracle JDK and OpenJDK distributions. Someone tell me again how LTS isn't a think for OpenJDK.
- usrusr 2y agoI read this as "the updated versions can now tolerate the option that makes future versions behave the same without calling out an error: invalid flag" This is exactly how you want an LTS to be supported long term.
- kaba0 2y agoOracleJDK has LTS defined. OpenJDK doesn’t, but as changes made to forks have to be migrated back to the original codebase, it does tend to be a “de facto” LTS version.
- foobarkey 2y agoMixed feelings about this one, now every project has to start tweaking compiler options for lombok etc, I kind of like how it works out of the box. I guess if you have a crazy project and a lot of transitive dependencies there is a small chance some processor lands on the classpath accidently and starts processing things, but I have not ran into this in practice
- nfrankel 2y agoThe Java team is still breaking things despite numerous pleas of the community pleading not to do so? Color me surprised. The Security Manager send its regards.
- pron 2y agoYou mean pleas from some members of the community not to do so and many more pleas from many more members of the community to the contrary. It is impossible to do "the community's bidding" when different parts of the community demand contradictory things. So we try to cater to the majority while giving the minority sufficient time to adapt. SecurityManager is a case in point. You and a few others claimed that removing it would have a large harmful impact. We proceeded in our usual cautious manner to first test the more widely believed hypothesis to the contrary before doing anything irreversible and put a warning in place in JDK 17. A lot of people have adopted JDK 17 or later over the last few years and those of them who are using SecurityManager have seen the warning about its planned removal. As most people believed, the warning did not uncover some hidden widespread use of the feature (and your campaign didn't manage to find widespread support for your position). It is perfectly alright to be dissatisfied when a decision doesn't go your way, but it's not alright to present it as if the decision went against "numerous pleas" without mentioning that, however numerous (which, in this case, was fewer than ten people), those pleas came from a small minority. Had we gone the other way, far more people would have been dissatisfied.
- beeboobaa3 2y agoNever seen anyone ask for you to get rid of security manager. Fix and improve it, sure. But no, it had to go and we did not get a suitable replacement. RIP.
- pron 2y agoOf course you did -- many, many, many times -- you just didn't know that that's what you were seeing. People rarely directly asks for an old feature that they're not using to be removed. Rather, they ask for new features that the old feature interferes with. SecurityManager imposed a significant tax on nearly every new feature, as it interacts with just about everything, slowing down much of the work on the JDK. SecurityManager was (actually, still is as it has not been removed yet) simultaneously one of the most expensive features and least used features in the JDK. Everyone who asked for new significant features effectively asked for SM to be removed, as they could not be delivered -- certainly not as expeditiously -- with it in place. In fact, as early as JDK 8, new features (such as streams) had to work around SM (simply documenting that it could not be effectively used in combination with the new feature; few people noticed because few people used SM). With time, it became ridiculous to say that SM is ineffective when used in combination with more and more new features (e.g. virtual threads, and the same would have been the case for Leyden [1]), and it still kept exacting a tax on most uses, so it had to be removed if we were to give people the features they were asking for. The fact that more robust, effective, and relevant security mechanisms are now not only available but have grown far more popular in Java applications than SM ever was only meant that the whatever arguments there may have been to keep it and delay and significantly complicate new features were weak. [1]: I shudder to think how Leyden could be done while keeping SM in some functional state.
- bloat 2y agoLombok has come up a bit in this discussion. Are there any other popular Java libraries or frameworks that are affected?
- kaba0 2y agoNo other comes to mind. Most other libraries are proper annotation processors, meaning they abide by the rules and are only additive, generating new classes. One such would be mapstruct which is pretty frequently used.
- the-alchemist 2y agoLombok is not affected by this, as it is not an annotation processor. The most popular annotation processor that I've seen "in the wild" is the Hibernate Metamodel Generator (https://hibernate.org/orm/tooling/ https://hibernate.org/orm/tooling/). Also, Immutables (https://immutables.github.io/ https://immutables.github.io/), my favorite Lombok alternative, is affected. Of note, you can bypass this more-security-concious approach by just passing `-prof:full` to javac.
- lucumo 2y ago> Lombok is not affected by this, as it is not an annotation processor. Lombok uses an annotation processor to bootstrap itself.
- lucumo 2y agoI know of these: - Immutables - Autovalue - Mapstruct - Checker Framework There's quite a list here https://github.com/gunnarmorling/awesome-annotation-processing?tab=readme-ov-file#existing-annotation-processors https://github.com/gunnarmorling/awesome-annotation-processi... (Though I don't think Error Prone is actually an annotation processor, but rather a javac plugin.) There's some irony in that Immutables and Autovalue are often named as alternatives for people that dislike Lombok's implementation but do like (some of) Lombok's features.
- the-alchemist 2y agoDon't quite understand the hate. This is very reasonable behavior for a compiler in 2024. I would even go so far as to say that a compiler should never execute any user code by default. Imagine if gcc could automatically, by default, download some shared library and include it in the build because of a special macro included by a random header file on the system. Of course, this change is optional (just add `-proc:full` to the command line) if you want the old, less secure behavior. Also, Lombok is not affected because it's not an annotation processor. (Also, look at Immutables: https://immutables.github.io/ https://immutables.github.io/).
- cies 2y agoI find it a great improvement. Secure by default. To me the hate started when Java went from a statically compiled languages, to a half way dynamic language that does a fair bit of compilation at application startup, based on a bunch of annotations. Suddenly many Java projects introduced Ruby-on-Rails level "magic" (yes looking at you Spring(Boot)). I'm not totally against annotations, but it's easier to over use them just to remove a bit of boiler plate. To me Kotlin's approach makes more sense: reduce boilerplate by making the language more expressive, adding KClass and KFunction (https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect) while mostly avoiding annotations and exceptions. Kotlin also has a nice story for immutability.