java.lang.NoSuchMethodError: java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer; in InsecureNonceChaCha20Poly1305Base on JDK8

[lauredogit]

Hi,

We are using Tink for Java.

One of our projects is still using Java 8.

We noticed a binary-compatibility issue when upgrading from Tink 1.18.0 to 1.19.0.

Hereunder is the stacktrace:

java.lang.NoSuchMethodError: java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer;
	at com.google.crypto.tink.aead.internal.InsecureNonceChaCha20Poly1305Base.encrypt(InsecureNonceChaCha20Poly1305Base.java:98)
	at com.google.crypto.tink.aead.internal.InsecureNonceXChaCha20Poly1305.encrypt(InsecureNonceXChaCha20Poly1305.java:26)
	at com.google.crypto.tink.subtle.XChaCha20Poly1305.rawEncrypt(XChaCha20Poly1305.java:63)
	at com.google.crypto.tink.subtle.XChaCha20Poly1305.encrypt(XChaCha20Poly1305.java:70)

It seems to come from the fact that in Java 8, ByteBuffer does not override the #position(int) method of Buffer, while on Java 11 it does.

Therefore, it would seem that Tink 1.19.0 has been compiled with JDK11 (or beyond) with target level set to 1.8.

However, while the Java sources are the same, the bytecode generated is not the same as before and links to the ByteBuffer#position(int) method of JDK11 which does not exist on JDK8.

Maybe this should be seen as a compiler bug?

A "silly" fix to keep the compatibility with JDK8 would be to force linking to the right method with

    chacha20.encrypt(output, nonce, plaintext);
    ((Buffer) output).position(firstPosition);

[tholenst]

Unfortunately, all the tooling available to us made it very hard to keep supporting JDK 8 (AFAIU Bazel doesn't support building for Java 8 anymore, for example). This is why since 1.19.0 we don't officially support JDK 8 anymore, see https://github.com/tink-crypto/tink-java/releases/tag/v1.19.0.

That being said, I actually tried to help you a bit: e357c85 -- it seems I missed some instances. As I said, it's hard to maintain this.

I would be willing to accept similar PRs, but I don't think we would have time to backport these to earlier Tink versions. If you decide to go this way, you probably want to use Tink at head and compile it yourself (so that you can find issues early).

[lauredogit]

We don't need backporting to earlier versions, as far as I know.

Thank you very much for the changes!

[tholenst]

Just to clarify (and I guess somewhat disappoint you, sorry): The above change I linked was already present in 1.19.0 and is not sufficient. At that point I just had access to some Java 8 runtime and could test things, so I did a last effort to try and help people like you who are still using Java 8.

However, I will not attempt to fix XChaCha20Poly1305. The reason for this is simply that I think the probabillity of me managing in a single try is roughly 1% -- I remember that I had a fairly long feedback loop until I could test my previous changes before we released 1.19.0, and it was already hard to get all instances. Trying to find instances now is close to impossible. It will not work reliably since I have no way of testing this at the moment. I could of course fix InsecureNonceChaCha20Poly1305Base.java:98 but I am very sure I would miss something without a direct feedback loop, which at the moment I cannot set up.

What's more is that we don't really have a way to ensure we don't use newer APIs. I'm sorry, but I don't know how else I can help you.

[lauredogit]

Ok, pretty clear, thank you.

Therefore, we will tell colleagues and customers to stick with Tink 1.18.0 for their projects still on Java 8...

[Sineaggi]

If compatibility with 8 is still desired, it should still be possible to compile against java 8 using the --release 8 but with a newer compiler.
For java 8 compatibility with XChaCha20Poly1305, a change like this is necessary.

java_library(
    name = "insecure_nonce_cha_cha20_poly1305_base",
    srcs = ["InsecureNonceChaCha20Poly1305Base.java"],
+   javacopts = ["--release 8"],
    deps = [
        ":insecure_nonce_cha_cha20_base",
        ":poly1305",
        "//src/main/java/com/google/crypto/tink/config/internal:tink_fips_util",
    ],
)

This would avoid the need for workarounds like e357c85

EDIT: Or a bazel macro

def java_8_library(name, javacops = [], **kwargs):
  java_library(
    name = name,
    javacopts = javacops + ["--release", "8"],
    **kwargs,
  )

load("//:internal/java_8_library.bzl", java_library = "java_8_library")
# use java_library as normal

[Sineaggi]

Not sure if there's any interest, but this branch does a quick first pass at making the code compile against java 8 main...Sineaggi:tink-java:support-building-against-java-8
The only changes are adding a new Maps helper that mirrors the api of Map::entry and Map::ofEntries from java 9.

[tholenst]

@Sineaggi Thank you for this. I will discuss this internally, but I'm in favor.

One consideration is what can go wrong with this since we will not run the tests with a java 8 runtime most likely (though if you know of an easy way to do that with Bazel, let me know).

Do you mind if I just replicate the change? I will also consider changing all instances of "java_library" to "java_8_library" since this is all produced by a script. Do you know what happens if I forget to replace a single "java_library" with "java_8_library"?

[tholenst]

Another thing I fail to understand is whether the java_lite_proto_library will be built for Java8... :/

[Sineaggi]

@tholenst I think there's an easy enough way to test against java 8, adding something like build:java8 --java_runtime_version=8 to .bazelrc and then running tests with bazel test --config=java8 //....

I wasn't able to get the android tests running on my machine, but hopefully that's unaffected.

Do you mind if I just replicate the change?

Sure

Another thing I fail to understand is whether the java_lite_proto_library will be built for Java8... :/

I don't think so. Adding --release 8 to the build flags in .bazelrc caused issues around UnsafeUtil (--release 8 does not let you compile against unsupported apis)

UnsafeUtil.java:456:20: could not resolve sun.misc.Unsafe
    MemoryAccessor(sun.misc.Unsafe unsafe) {

After adding

build --javacopt="--release 8"

to .bazelrc

[tholenst]

During investigation of this, I noticed that actually we are building the Java Library wrong :( While our toolchain was Java 11, we still had wrong Java 8 and local_jdk referenced in .bazelrc.

I fixed this in 864c915.

I prefer to do this, because the previous setting was clearly wrong and I cannot be sure of the impact it has.

However, I also fear that now Java 8 will simply not work at all anymore. I will investigate some more whether we can do this properly though.

[Sineaggi]

@tholenst are you asking if you can cross-compile from java 11 to target java 8? Because you can, that's explicitly what I'm doing in this branch main...Sineaggi:tink-java:support-building-against-java-8, and what JEP 247 is about. https://openjdk.org/jeps/247

[tholenst]

The problem is that AFAICT Bazel doesn't support this and we are building with Bazel. Another problem is that we need to probably build the protobuf generated classes with this too, and I don't know how to achieve this.

I didn't have time to look into this further :(

It might also be possible to just use the source jar (which we generate using bazel) and then use that to build the Java8 bytecode without Bazel. I think this might be easier even...

[Sineaggi]

So unless I'm mistaken, it should be fine to leave that option on 8, considering proto still guarantees support for java 8. Then for the code built in this library, using release would prevent Apis that are unavailable from being used.