6 ms·

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

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 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
tadfisher 2y ago
Annotation processors on the compile classpath are not automatically added to the build output's runtime classpath. The status quo doesn't change, because annotation processors also don't do anything when running the app (e.g. with java and not javac), on any Java version.
- The_Colonel 2y ago
  Annotation processor can generate code which will be included in the build output runtime classpath..
  - dtech 2y ago
    which is completely irrelevant for the context of this specific change: automatically picking up annotation processors. If you don't like the (generated) code of a lib don't use it.
    - The_Colonel 2y ago
      It's not irrelevant. A rogue (automatically run) annotation processor will generate exploit code bundled into the build output artifact, which will then run on the production systems. Besides, annotation processor can attack the build system as well.
MichaelNolan 2y ago
> 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 I’ve been out of the Java world recently, but my understanding is that an application developer can still do anything they want. All of these new restrictions are for library developers. To be clear the library developer can still do anything, they just have to make it explicit. This is the opposite of a backwards compatibility breaking crusade. This is a crusade to make sure application developers don’t accidentally depend on libraries that are either breaking encapsulation or depending on JDK internals. This should improve backwards compatibility.
GauntletWizard 2y ago
This seems a real vulnerability if you're using legacy infrastructure - If you're running your build process on a highly privileged build machine, like a single large Jenkins instance. These machines might have a bunch of subprojects - and a bunch of credentials to login to other prod systems for deployment purposes. This is not the reason that I prefer containerized build solutions, but it is a real concern, outside of the little bubble that is the startup ecosystem. Edit: It occurs to me that since I just gave a talk on this, it behooves me to link it: https://youtu.be/dswPHnfGwlY https://youtu.be/dswPHnfGwlY
pjmlp 2y ago
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.
- josefx 2y ago
  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.
  - pron 2y ago
    > Oracle did not design public replacement APIs until the restrictions on it where anounced. The very opposite is true. All the remaining required replacements for Unsafe were put in place in JDK 22, and access to Unsafe has been unrestricted and unencumbered in any way until the upcoming JDK 23 [1]. It is precisely because we know people have come to depend on Unsafe that we had not started to restrict its use until all replacements were delivered. > You might as well tell a starving person that eating bread is illegal. Also no, because no functionality has been taken away. The only thing that has been taken away is the ability of a library (possibly a deep transitive dependency) to unilaterally use internals (again, not Unsafe, which wasn't encapsulated) -- something that has a global effect on the application -- without the application's knowledge and consent. In other words, doing the thing is not illegal; what's illegal is doing it without the application's permission. If a library is, indeed, trusted by the application to do things that carry special risk, then it's easy for the application to grant it permission; if it isn't trusted, then surely there is no argument that the trust should be implicitly taken rather than given. [1]: https://openjdk.org/jeps/471 https://openjdk.org/jeps/471
    - dtech 2y ago
      Private APIs have been starting to be restricted in previous versions already, requiring command line flags like add-exports=jdk.compiler/com.sun.tools.javac.api
kaba0 2y ago
> 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’t control) and I think it’s quite easy to see why it’s not loved (though this “war” bullshit is just propaganda from the creator’s ego or whatever). Also, default being strict is a good stance (both in case of the reflection restrictions) — you can access everything with just a few command line flags, so I don’t really see all the complaints. The point is, you have to know about whether some module in your system accesses another in a non-standard way. Like, are firewalls overly strict because they only allow traffic through port 22 when specified to do so? Should they start with allow-all?
- vips7L 2y ago
  He knows what Lombok does. He’s the author.
  - xxs 2y ago
    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
    - vips7L 2y ago
      I’m not refuting the point. Just giving kaba0 context that they are explaining how a library works to its author.
  - kaba0 2y ago
    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.
metadat 2y ago
Is the OpenJDK behavior different from Oracle JDK?
- pjmlp 2y ago
  They are basically the same, other than support and packaged set of GC configurations.
martypitt 2y ago
Slightly OT, but sincere thanks from me to you for creating Lombok. It's a tool that sparks strong opinions, which is a testament to its significance and the impact it has made. You've created something that people are passionate about, whether they're for or against it. Personally, I'm firmly in Team Lombok. I believe the negative feedback it receives is disproportionate. For those of us who use it, Lombok significantly improves our coding experience. For those who don't, it's entirely optional and doesn't interfere with their workflow. Thanks again for creating something that made me enjoy coding in Java :)
- pjmlp 2y ago
  Those of us that are not on team Lombok still have to deal with it due to transitive dependencies.
  - frant-hartm 2y ago
    Complain to the author of your direct dependencies (or better send a PR if open source). There is absolutely no reason why lombok should get pulled in as transitive dependency apart from ignorance on library author side.
    - pjmlp 2y ago
      I rather get rid of them when possible, instead of indirectly contributing to lombok's existence.
      - beeboobaa3 2y ago
        It's clear you are just spiteful and have absolutely no clue what you are talking about. Lombok does not get pulled in as a transitive dependency unless the author of your dependency fucked up their build. RTFM and you're good. Even if they fucked up, you can just exclude the transitive dependency. Easy. Lombok produces bytecode that could have been written by hand. You will not be able to tell the difference when using these compiled classes.
frant-hartm 2y ago
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.
- beeboobaa3 2y ago
  > test libraries only Typically runs during build (unit tests), just like annotation processors. > compile time only libraries (hello lombok?) Right, annotation processors. This is what we're discussing. > transitive dependencies that may not be used during runtime (or run only in rare code paths) Irrelevant. If they are compromised they will just set themselves up as an SPI and run on JVM start.
pron 2y ago
> OpenJDK's backwards-compatibility breaking crusade. First, Java has always been very explicit about where it offers backward compatibility and where it does not. You cannot break backward compatibility where it must not be expected. These are classes that carry a warning saying: these are internal classes that offer no kind of backward compatibility and can be changed at any time and without warning; by depending on them you are accepting upon yourself the responsibility to respond to any change. Nevertheless, in most situations we are offering plenty of advance warning. Second, the "crusade" isn't done to break anything or against anyone, but because these internal and not-backward-compatibility-breaking changes are necessary to offer Java users the features they're asking for, which, in turn, rely on integrity [1]. We're talking about changes to the very core assumptions of the platform, which could violate any invariant and have a global impact, that could have been made by any code in any transitive library. In order to offer certain features the platform must know which of its invariants it may trust (for example, the JIT compiler cannot perform certain optimisations that assume strings are immutable because even though it is an invariant of the platform, some third-level dependency could have decided that actually strings would be mutable in any application that consumes this library). Nevertheless, the platform does not prevent code from choosing to violate integrity invariants. It just prevents libraries from doing so -- which has a global effect on the application -- without the application's knowledge. > "For security" without being particularly clear about how that update contributes to security. The changes I was referring to above (deep reflection, Unsafe, JNI/FFM, dynamic agents) are about integrity, not security (you can think of integrity as a generalisation of memory safety, which is a special case of integrity; it is not security in itself, but it can make security easier). We are very clear both about that and about the relationship between integrity and security [1]. As far as this particular change to annotation processing, however -- a change I'm not personally involved with and don't know the specifics of -- the email does mention security as the motivation. It has been a long-standing policy of the JDK team (as well as that of many other projects) not to disclose any specifics about any vulnerabilities involved. OpenJDK has a specialised vulnerability group [2], made up of people from multiple companies, and they are given access to the vulnerabilities. > 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. Yeah, one, it's not about security but about integrity -- as explained in the motivation [1] -- which is, indeed, essential for security but for other important things as well, and two, your security analysis is just wrong. That is why for security is best to rely on security experts and not on people unfamiliar with the field who go by what seems to make sense to them. > if you are running malicious code inside a JVM, we've got much, much bigger problems. Benevolent, well-meaning code with unintended vulnerabilities is a far bigger security problem than malicious code in server applications. Malicious code is a common problem on the client, but benevolent code is the bigger danger on the server, so if you start thinking about what it is that malicious code could do you know you're thinking in the wrong direction. [1]: https://openjdk.org/jeps/8305968 https://openjdk.org/jeps/8305968 [2]: https://openjdk.org/groups/vulnerability/ https://openjdk.org/groups/vulnerability/

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

tadfisher 2y ago

Annotation processors on the compile classpath are not automatically added to the build output's runtime classpath. The status quo doesn't change, because annotation processors also don't do anything when running the app (e.g. with java and not javac), on any Java version.

The_Colonel 2y ago
Annotation processor can generate code which will be included in the build output runtime classpath..
- dtech 2y ago
  which is completely irrelevant for the context of this specific change: automatically picking up annotation processors. If you don't like the (generated) code of a lib don't use it.
  - The_Colonel 2y ago
    It's not irrelevant. A rogue (automatically run) annotation processor will generate exploit code bundled into the build output artifact, which will then run on the production systems. Besides, annotation processor can attack the build system as well.

MichaelNolan 2y ago

> 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 I’ve been out of the Java world recently, but my understanding is that an application developer can still do anything they want. All of these new restrictions are for library developers. To be clear the library developer can still do anything, they just have to make it explicit. This is the opposite of a backwards compatibility breaking crusade. This is a crusade to make sure application developers don’t accidentally depend on libraries that are either breaking encapsulation or depending on JDK internals. This should improve backwards compatibility.

GauntletWizard 2y ago

This seems a real vulnerability if you're using legacy infrastructure - If you're running your build process on a highly privileged build machine, like a single large Jenkins instance. These machines might have a bunch of subprojects - and a bunch of credentials to login to other prod systems for deployment purposes. This is not the reason that I prefer containerized build solutions, but it is a real concern, outside of the little bubble that is the startup ecosystem. Edit: It occurs to me that since I just gave a talk on this, it behooves me to link it: https://youtu.be/dswPHnfGwlY https://youtu.be/dswPHnfGwlY

pjmlp 2y ago

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.

josefx 2y ago
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.
- pron 2y ago
  > Oracle did not design public replacement APIs until the restrictions on it where anounced. The very opposite is true. All the remaining required replacements for Unsafe were put in place in JDK 22, and access to Unsafe has been unrestricted and unencumbered in any way until the upcoming JDK 23 [1]. It is precisely because we know people have come to depend on Unsafe that we had not started to restrict its use until all replacements were delivered. > You might as well tell a starving person that eating bread is illegal. Also no, because no functionality has been taken away. The only thing that has been taken away is the ability of a library (possibly a deep transitive dependency) to unilaterally use internals (again, not Unsafe, which wasn't encapsulated) -- something that has a global effect on the application -- without the application's knowledge and consent. In other words, doing the thing is not illegal; what's illegal is doing it without the application's permission. If a library is, indeed, trusted by the application to do things that carry special risk, then it's easy for the application to grant it permission; if it isn't trusted, then surely there is no argument that the trust should be implicitly taken rather than given. [1]: https://openjdk.org/jeps/471 https://openjdk.org/jeps/471
  - dtech 2y ago
    Private APIs have been starting to be restricted in previous versions already, requiring command line flags like add-exports=jdk.compiler/com.sun.tools.javac.api

kaba0 2y ago

> 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’t control) and I think it’s quite easy to see why it’s not loved (though this “war” bullshit is just propaganda from the creator’s ego or whatever). Also, default being strict is a good stance (both in case of the reflection restrictions) — you can access everything with just a few command line flags, so I don’t really see all the complaints. The point is, you have to know about whether some module in your system accesses another in a non-standard way. Like, are firewalls overly strict because they only allow traffic through port 22 when specified to do so? Should they start with allow-all?

vips7L 2y ago
He knows what Lombok does. He’s the author.
- xxs 2y ago
  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
  - vips7L 2y ago
    I’m not refuting the point. Just giving kaba0 context that they are explaining how a library works to its author.
- kaba0 2y ago
  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.

metadat 2y ago

Is the OpenJDK behavior different from Oracle JDK?

pjmlp 2y ago
They are basically the same, other than support and packaged set of GC configurations.

martypitt 2y ago

Slightly OT, but sincere thanks from me to you for creating Lombok. It's a tool that sparks strong opinions, which is a testament to its significance and the impact it has made. You've created something that people are passionate about, whether they're for or against it. Personally, I'm firmly in Team Lombok. I believe the negative feedback it receives is disproportionate. For those of us who use it, Lombok significantly improves our coding experience. For those who don't, it's entirely optional and doesn't interfere with their workflow. Thanks again for creating something that made me enjoy coding in Java :)

pjmlp 2y ago
Those of us that are not on team Lombok still have to deal with it due to transitive dependencies.
- frant-hartm 2y ago
  Complain to the author of your direct dependencies (or better send a PR if open source). There is absolutely no reason why lombok should get pulled in as transitive dependency apart from ignorance on library author side.
  - pjmlp 2y ago
    I rather get rid of them when possible, instead of indirectly contributing to lombok's existence.
    - beeboobaa3 2y ago
      It's clear you are just spiteful and have absolutely no clue what you are talking about. Lombok does not get pulled in as a transitive dependency unless the author of your dependency fucked up their build. RTFM and you're good. Even if they fucked up, you can just exclude the transitive dependency. Easy. Lombok produces bytecode that could have been written by hand. You will not be able to tell the difference when using these compiled classes.

frant-hartm 2y ago

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.

beeboobaa3 2y ago
> test libraries only Typically runs during build (unit tests), just like annotation processors. > compile time only libraries (hello lombok?) Right, annotation processors. This is what we're discussing. > transitive dependencies that may not be used during runtime (or run only in rare code paths) Irrelevant. If they are compromised they will just set themselves up as an SPI and run on JVM start.

pron 2y ago

> OpenJDK's backwards-compatibility breaking crusade. First, Java has always been very explicit about where it offers backward compatibility and where it does not. You cannot break backward compatibility where it must not be expected. These are classes that carry a warning saying: these are internal classes that offer no kind of backward compatibility and can be changed at any time and without warning; by depending on them you are accepting upon yourself the responsibility to respond to any change. Nevertheless, in most situations we are offering plenty of advance warning. Second, the "crusade" isn't done to break anything or against anyone, but because these internal and not-backward-compatibility-breaking changes are necessary to offer Java users the features they're asking for, which, in turn, rely on integrity [1]. We're talking about changes to the very core assumptions of the platform, which could violate any invariant and have a global impact, that could have been made by any code in any transitive library. In order to offer certain features the platform must know which of its invariants it may trust (for example, the JIT compiler cannot perform certain optimisations that assume strings are immutable because even though it is an invariant of the platform, some third-level dependency could have decided that actually strings would be mutable in any application that consumes this library). Nevertheless, the platform does not prevent code from choosing to violate integrity invariants. It just prevents libraries from doing so -- which has a global effect on the application -- without the application's knowledge. > "For security" without being particularly clear about how that update contributes to security. The changes I was referring to above (deep reflection, Unsafe, JNI/FFM, dynamic agents) are about integrity, not security (you can think of integrity as a generalisation of memory safety, which is a special case of integrity; it is not security in itself, but it can make security easier). We are very clear both about that and about the relationship between integrity and security [1]. As far as this particular change to annotation processing, however -- a change I'm not personally involved with and don't know the specifics of -- the email does mention security as the motivation. It has been a long-standing policy of the JDK team (as well as that of many other projects) not to disclose any specifics about any vulnerabilities involved. OpenJDK has a specialised vulnerability group [2], made up of people from multiple companies, and they are given access to the vulnerabilities. > 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. Yeah, one, it's not about security but about integrity -- as explained in the motivation [1] -- which is, indeed, essential for security but for other important things as well, and two, your security analysis is just wrong. That is why for security is best to rely on security experts and not on people unfamiliar with the field who go by what seems to make sense to them. > if you are running malicious code inside a JVM, we've got much, much bigger problems. Benevolent, well-meaning code with unintended vulnerabilities is a far bigger security problem than malicious code in server applications. Malicious code is a common problem on the client, but benevolent code is the bigger danger on the server, so if you start thinking about what it is that malicious code could do you know you're thinking in the wrong direction. [1]: https://openjdk.org/jeps/8305968 https://openjdk.org/jeps/8305968 [2]: https://openjdk.org/groups/vulnerability/ https://openjdk.org/groups/vulnerability/