Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Proposal to change default annotation processing policy in JDK 23

mail.openjdk.org

Post reply on HN
Print view View original

21–30 of 108 posts

Re: Proposal to change default annotation processing policy in JDK 23

#21

Post by vips7L »

jillesvangurp wrote: post #18
vips7L wrote: post #17

Earlier quoted context omitted.

> 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.

In 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.

Sure. 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 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”.

Re: Proposal to change default annotation processing policy in JDK 23

#22

Post by giamma »

"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.

Re: Proposal to change default annotation processing policy in JDK 23

#24

Post by frant-hartm »

rzwitserloot wrote: post #5
MarkSweep wrote: post #2

I 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?

How 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 sne…

There are multiple attack vectors via the supply chain that this new settings prevents.

Sure, if the compromised library is one of your core libraries that runs during build, test and runtime, then this does not help. But there are

  test libraries only
  compile time only libraries (hello lombok?)
  transitive dependencies that may not be used during runtime (or run only in rare code paths)
Sometimes compromising the build environment is more valuable than the app's runtime environment - e.g. it may allow the attacker to compromise all apps.

Explicitly enabling a particular annotation processor I want to run is small price to pay for the increased security.

Re: Proposal to change default annotation processing policy in JDK 23

#25

Post by xxs »

vips7L wrote: post #16
kaba0 wrote: post #15

Earlier quoted context omitted.

> this is a silly update in OpenJDK's war against things like Project Lombok Project Lombok doesn’t use the normal annotation processing system, as that is deliberately “add-only” - you can’t change a class’s implementation, unlike what lombok does. They instead hack into the javac compiler to be able to modify class files, which is a very different mechanism (and prone to break with any javac update, which they don’…

He knows what Lombok does. He’s the author.

Kind of - the point remains. Lombok requires to alter the compiler (hook into), not (just) the annotation processing, itself. I'd be okayish if Lombok was a mere post compilation/enhancement too, but it isn't. It's a per-compilation step

Re: Proposal to change default annotation processing policy in JDK 23

#26

Post by foobarkey »

Mixed 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

Re: Proposal to change default annotation processing policy in JDK 23

#28

Post by kaba0 »

vips7L wrote: post #16
kaba0 wrote: post #15

Earlier quoted context omitted.

> this is a silly update in OpenJDK's war against things like Project Lombok Project Lombok doesn’t use the normal annotation processing system, as that is deliberately “add-only” - you can’t change a class’s implementation, unlike what lombok does. They instead hack into the javac compiler to be able to modify class files, which is a very different mechanism (and prone to break with any javac update, which they don’…

He knows what Lombok does. He’s the author.

Well, then he just prefers telling lies. Annotation processors can’t modify classes, this is a fact. The primary purpose of Lombok is adding new methods like getX and setX to the same class based on fields. It’s pretty easy to conclude that Lombok is thus not an ordinary annotation processor, and actually uses sun.misc.unsafe to go into the private internals of javac to modify the AST, which has become possible only by specifying some additional arguments, notifying the end-user that some libraries on the class path might do something that can’t be promised to work ad eternity. This is a completely reasonable decision on the Java team’s part (and quite narcissistic to conclude that it is due to your lib..), as many of the breaking changes between 8 and 9 were actually due to libraries doing exactly that. These changes helps uphold Java’s strong backwards compatibility guarantees.

Re: Proposal to change default annotation processing policy in JDK 23

#29

Post by usrusr »

giamma wrote: post #22

"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 sup…

> 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)

Re: Proposal to change default annotation processing policy in JDK 23

#30

Post by josefx »

pjmlp wrote: post #14
rzwitserloot wrote: post #5

Earlier quoted context omitted.

How 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 sne…

Had some developers not starting to Monkey patch this wouldn't be needed, Java isn't Ruby. Go is now going to do a similar approach, because internals and not being public symbols apparently isn't clear enough.

Public/private does nothing to change the problems a developer has to deal with. On the JVM side the most widely "hacked" private API was sun.misc.Unsafe, which just could not be implemented using pure Java and Oracle did not design public replacement APIs until the restrictions on it where anounced.

> because internals and not being public symbols apparently isn't clear enough.

You might as well tell a starving person that eating bread is illegal.

Post reply on HN