Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jdk21 com/sun/crypto/provider/Cipher/ChaCha20/ChaCha20NoReuse Must use either a different key or iv #17632

Closed
pshipton opened this issue Jun 21, 2023 · 10 comments · Fixed by ibmruntimes/openj9-openjdk-jdk21#50 or adoptium/aqa-tests#4817

Comments

@pshipton
Copy link
Member

https://openj9-jenkins.osuosl.org/job/Grinder_testList_2/46/ - amac jdk21
com/sun/crypto/provider/Cipher/ChaCha20/ChaCha20NoReuse.java

15:08:52  java.lang.IllegalStateException: Must use either a different key or iv
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher$EngineStreamOnly.doUpdate(NativeChaCha20Cipher.java:924)
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher$EngineStreamOnly.doFinal(NativeChaCha20Cipher.java:933)
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher.engineDoFinal(NativeChaCha20Cipher.java:728)
15:08:52  	at java.base/javax.crypto.Cipher.doFinal(Cipher.java:2244)
15:08:52  	at ChaCha20NoReuse$6.run(ChaCha20NoReuse.java:392)
15:08:52  	at ChaCha20NoReuse.main(ChaCha20NoReuse.java:597)
15:08:52  	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
15:08:52  	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
15:08:52  	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333)
15:08:52  	at java.base/java.lang.Thread.run(Thread.java:1595)
15:08:52  java.lang.IllegalStateException: Attempted to update AAD on uninitialized Cipher
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher.engineUpdateAAD(NativeChaCha20Cipher.java:454)
15:08:52  	at java.base/javax.crypto.Cipher.updateAAD(Cipher.java:2796)
15:08:52  	at java.base/javax.crypto.Cipher.updateAAD(Cipher.java:2750)
15:08:52  	at ChaCha20NoReuse$6.run(ChaCha20NoReuse.java:390)
15:08:52  	at ChaCha20NoReuse.main(ChaCha20NoReuse.java:597)
15:08:52  	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
15:08:52  	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
15:08:52  	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333)
15:08:52  	at java.base/java.lang.Thread.run(Thread.java:1595)
15:08:52  java.lang.IllegalStateException: Attempted to update AAD on uninitialized Cipher
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher.engineUpdateAAD(NativeChaCha20Cipher.java:454)
15:08:52  	at java.base/javax.crypto.Cipher.updateAAD(Cipher.java:2796)
15:08:52  	at java.base/javax.crypto.Cipher.updateAAD(Cipher.java:2750)
15:08:52  	at ChaCha20NoReuse$7.run(ChaCha20NoReuse.java:444)
15:08:52  	at ChaCha20NoReuse.main(ChaCha20NoReuse.java:597)
15:08:52  	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
15:08:52  	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
15:08:52  	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333)
15:08:52  	at java.base/java.lang.Thread.run(Thread.java:1595)
15:08:52  java.security.InvalidKeyException: Matching key and nonce from previous initialization
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher.checkKeyAndNonce(NativeChaCha20Cipher.java:624)
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher.init(NativeChaCha20Cipher.java:560)
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher.engineInit(NativeChaCha20Cipher.java:358)
15:08:52  	at java.base/javax.crypto.Cipher.init(Cipher.java:1464)
15:08:52  	at java.base/javax.crypto.Cipher.init(Cipher.java:1393)
15:08:52  	at ChaCha20NoReuse$9.run(ChaCha20NoReuse.java:569)
15:08:52  	at ChaCha20NoReuse.main(ChaCha20NoReuse.java:597)
15:08:52  	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
15:08:52  	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
15:08:52  	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333)
15:08:52  	at java.base/java.lang.Thread.run(Thread.java:1595)
15:08:52  java.security.InvalidKeyException: Matching key and nonce from previous initialization
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher.checkKeyAndNonce(NativeChaCha20Cipher.java:624)
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher.init(NativeChaCha20Cipher.java:560)
15:08:52  	at java.base/com.sun.crypto.provider.NativeChaCha20Cipher.engineInit(NativeChaCha20Cipher.java:358)
15:08:52  	at java.base/javax.crypto.Cipher.init(Cipher.java:1464)
15:08:52  	at java.base/javax.crypto.Cipher.init(Cipher.java:1393)
15:08:52  	at ChaCha20NoReuse$9.run(ChaCha20NoReuse.java:569)
15:08:52  	at ChaCha20NoReuse.main(ChaCha20NoReuse.java:597)
15:08:52  	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
15:08:52  	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
15:08:52  	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333)
15:08:52  	at java.base/java.lang.Thread.run(Thread.java:1595)
15:08:52  java.lang.RuntimeException: Not all tests passed.  See output for failure info
15:08:52  	at ChaCha20NoReuse.main(ChaCha20NoReuse.java:609)
15:08:52  	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
15:08:52  	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
15:08:52  	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333)
15:08:52  	at java.base/java.lang.Thread.run(Thread.java:1595)
@pshipton
Copy link
Member Author

@jasonkatonica fyi

@pshipton
Copy link
Member Author

Still fails on the latest jdk21 OpenJ9.
Passes on the latest Temurin nightly build.

Passes on OpenJ9 with -Djdk.nativeChaCha20=false

@pshipton
Copy link
Member Author

Tentatively setting as a blocker, unless we can explain why this failure doesn't matter.

@KostasTsiounis
Copy link
Contributor

The tests that are failing are part of a specific scenario. The scenario involves the reuse of the Cipher after a decryption. The test involves three use cases that are failing:

  • Reinitialize with same key + nonce
  • Update AAD after a successful doFinal() without reinitializing
  • Update AAD after an AEADBadTagException

According to the specification and original implementation in OpenJDK21, all of those are allowed.

To accommodate this expected behaviour, I created a fix: https://github.com/KostasTsiounis/openj9-openjdk-jdk21/tree/chacha20_reuse

The failing test now succeeds: https://hyc-runtimes-jenkins.swg-devops.com/view/Test_grinder/job/Grinder/34861/

Only problem is that this particular behaviour is only expected in later versions of Java.

More specifically, the equivalent test in Java 17 expects to get an Exception in all of the use cases:

The equivalent test in Java 11 doesn't even check for the first use case and also expects an Exception for each of the other two:

@jasonkatonica
Copy link
Contributor

Hi Kostas, Reading above it seems like we could do one of the following:

  1. Match the behavior of the Sun providers. In this solution we allow repeat key + nonce during decryption in JDK 21 and higher and disallow this in JDK 17 and older releases. We clear the key the same way as the Sun providers in the various releases. We also have same reinitalization behavior.
  2. Enable the use of the same key + nonce during decryption in all releases. This will diverge from what the Sun providers are doing currently in releases less then JDK 21. In this case we make our code ( the openssl code ) look the same everywhere and then need to change the tests in older releases to match this specific behavior since the code would act differently then the sun providers.

Id vote for option 1 above to maintain behavior as close to Sun providers as possible there is a good reason otherwise. @pshipton Do you have an opinion here one way or another?

@0xdaryl
Copy link
Contributor

0xdaryl commented Sep 26, 2023

@pshipton : fyi, your opinion was requested above.

Kostas (@KostasTsiounis ), could you post a current status update on this blocking JDK 21 issue please? Thanks.

@pshipton
Copy link
Member Author

Option 1 works for me. Matching the OpenJDK behavior is what users expect.

@KostasTsiounis
Copy link
Contributor

A PR has already been put in for OpenJDKnext: ibmruntimes/openj9-openjdk-jdk#661

Once this is merged, I will back port to 21, and this issue will be resolved.

@babsingh
Copy link
Contributor

@babsingh babsingh reopened this Oct 17, 2023
babsingh added a commit to babsingh/aqa-tests that referenced this issue Oct 17, 2023
JDK-next (JDK22) fix: ibmruntimes/openj9-openjdk-jdk#661
JDK21 fix: ibmruntimes/openj9-openjdk-jdk21#50

Closes: eclipse-openj9/openj9#17632

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh added a commit to babsingh/aqa-tests that referenced this issue Oct 17, 2023
JDK-next (JDK22) fix: ibmruntimes/openj9-openjdk-jdk#661
JDK21 fix: ibmruntimes/openj9-openjdk-jdk21#50

Closes: eclipse-openj9/openj9#17632

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
@babsingh
Copy link
Contributor

Opened adoptium/aqa-tests#4817 to re-enable the ChaCha20NoReuse test. After this PR is merged, the test excluded label should be removed, and this issue should be closed.

smlambert pushed a commit to adoptium/aqa-tests that referenced this issue Oct 17, 2023
JDK-next (JDK22) fix: ibmruntimes/openj9-openjdk-jdk#661
JDK21 fix: ibmruntimes/openj9-openjdk-jdk21#50

Closes: eclipse-openj9/openj9#17632

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
llxia pushed a commit to llxia/aqa-tests that referenced this issue Nov 22, 2023
JDK-next (JDK22) fix: ibmruntimes/openj9-openjdk-jdk#661
JDK21 fix: ibmruntimes/openj9-openjdk-jdk21#50

Closes: eclipse-openj9/openj9#17632

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
llxia pushed a commit to llxia/aqa-tests that referenced this issue Nov 22, 2023
JDK-next (JDK22) fix: ibmruntimes/openj9-openjdk-jdk#661
JDK21 fix: ibmruntimes/openj9-openjdk-jdk21#50

Closes: eclipse-openj9/openj9#17632

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
pshipton pushed a commit to adoptium/aqa-tests that referenced this issue Nov 22, 2023
JDK-next (JDK22) fix: ibmruntimes/openj9-openjdk-jdk#661
JDK21 fix: ibmruntimes/openj9-openjdk-jdk21#50

Closes: eclipse-openj9/openj9#17632

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment