Java Cipher - PBE thread security issue

Question

It seems like I'm having a thread safety issue with Cipher and / or PBEKeySpec.

• JDK: 1.8.0_102, 1.8.0_151 and 9.0.1 + 11
• PBKDF2 Algorithm: PBKDF2WithHmacSHA1
• Encryption Algorithm: AES / CFB / NoPadding
• Key Algorithms: AES

I know that these classes are not protected from the tread if we use the same instances, but this is not the case, I get a new instance on each decoder. But even this, sometimes decoding fails, there is no exception, just an unexpected decoded value.

I was able to reproduce the problem:

@Test public void shouldBeThreadSafe() { final byte[] encoded = { 27, 26, 18, 88, 84, -87, -40, -91, 70, -74, 87, -21, -124, -114, -44, -24, 7, -7, 104, -26, 45, 96, 119, 45, -74, 51 }; final String expected = "dummy data"; final Charset charset = StandardCharsets.UTF_8; final String salt = "e47312da-bc71-4bde-8183-5e25db6f0987"; final String passphrase = "dummy-passphrase"; // Crypto configuration final int iterationCount = 10; final int keyStrength = 128; final String pbkdf2Algorithm = "PBKDF2WithHmacSHA1"; final String cipherAlgorithm = "AES/CFB/NoPadding"; final String keyAlgorithm = "AES"; // Counters final AtomicInteger succeedCount = new AtomicInteger(0); final AtomicInteger failedCount = new AtomicInteger(0); // Test System.setProperty("java.util.concurrent.ForkJoinPool.common.parallelism", "10"); IntStream.range(0, 1000000).parallel().forEach(i -> { try { SecretKeyFactory factory = SecretKeyFactory.getInstance(pbkdf2Algorithm); KeySpec spec = new PBEKeySpec(passphrase.toCharArray(), salt.getBytes(charset), iterationCount, keyStrength); SecretKey tmp = factory.generateSecret(spec); SecretKeySpec key = new SecretKeySpec(tmp.getEncoded(), keyAlgorithm); Cipher cipher = Cipher.getInstance(cipherAlgorithm); int blockSize = cipher.getBlockSize(); IvParameterSpec iv = new IvParameterSpec(Arrays.copyOf(encoded, blockSize)); byte[] dataToDecrypt = Arrays.copyOfRange(encoded, blockSize, encoded.length); cipher.init(Cipher.DECRYPT_MODE, key, iv); byte[] utf8 = cipher.doFinal(dataToDecrypt); String decoded = new String(utf8, charset); if (!expected.equals(decoded)) { System.out.println("Try #" + i + " | Unexpected decoded value: [" + decoded + "]"); failedCount.incrementAndGet(); } else { succeedCount.incrementAndGet(); } } catch (Exception e) { System.out.println("Try #" + i + " | Decode failed"); e.printStackTrace(); failedCount.incrementAndGet(); } }); System.out.println(failedCount.get() + " of " + (succeedCount.get() + failedCount.get()) + " decodes failed"); } 

Output:

 Try #656684 | Unexpected decoded value: [ jE |S   ] Try #33896 | Unexpected decoded value: [ jE |S   ] 2 of 1000000 decodes failed 

I do not understand how this code can fail, is there an error in the Cipher and / or PBEKeySpec classes? Or did I miss something in my test?

Any help would be greatly appreciated.

---

UPDATE

OpenJDK issue: https://bugs.openjdk.java.net/browse/JDK-8191177

Answer 1

This is really a JDK error in the PBKDF2KeyImpl.getEncoded() method.

More details in the error report https://bugs.openjdk.java.net/browse/JDK-8191177 and related problem https://bugs.openjdk.java.net/browse/JDK-8191002 .

It was fixed and shipped in the January 2018 Java CPU release.

UPDATE: This has been fixed for JDK 9 and later using reachabilityFence ().

Due to the lack of this fence in the EALER JDK version, you should use a workaround: " as Hans Böhm first discovered, it happens that one of the ways to implement the reachability equivalent is Fence (x) even now is" synchronized "(x) {}" "

In our case, the workaround is:

 SecretKeyFactory factory = SecretKeyFactory.getInstance(pbkdf2Algorithm); KeySpec spec = new PBEKeySpec(passphrase.toCharArray(), salt.getBytes(charset), iterationCount, keyStrength); SecretKey secret = factory.generateSecret(spec); SecretKeySpec key; //noinspection SynchronizationOnLocalVariableOrMethodParameter synchronized(secret) { key = new SecretKeySpec(secret.getEncoded(), keyAlgorithm); } 

Answer 2

I am inclined to believe that this is most likely a manifestation of a JVM error related to finalization and arrays. Below is a more general test case. Run with java -Xmx10m -cp . UnexpectedArrayContents java -Xmx10m -cp . UnexpectedArrayContents , the smaller the heap, the greater the chance of failure. Not sure if the calls to clone() really matter, just tried to be close to the source fragment.

 // Omitting package and imports for brevity // ... public class UnexpectedArrayContents { void demonstrate() { IntStream.range(0, 20000000).parallel().forEach(i -> { String expected = randomAlphaNumeric(10); byte[] expectedBytes = expected.getBytes(StandardCharsets.UTF_8); ArrayHolder holder = new ArrayHolder(expectedBytes); byte[] actualBytes = holder.getBytes(); String actual = new String(actualBytes, StandardCharsets.UTF_8); if (!Objects.equals(expected, actual)) { System.err.println("attempt#" + i + " failed; expected='" + expected + "' actual='" + actual + "'"); System.err.println("actual bytes: " + DatatypeConverter.printHexBinary(actualBytes)); } }); } static class ArrayHolder { private byte[] _bytes; ArrayHolder(final byte[] bytes) { _bytes = bytes.clone(); } byte[] getBytes() { return _bytes.clone(); } @Override protected void finalize() throws Throwable { if (_bytes != null) { Arrays.fill(_bytes, (byte) 'z'); _bytes = null; } super.finalize(); } } private static final String ALPHA_NUMERIC_STRING = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; private static final Random RND = new Random(); static String randomAlphaNumeric(int count) { final StringBuilder sb = new StringBuilder(); while (count-- != 0) { int character = RND.nextInt(ALPHA_NUMERIC_STRING.length()); sb.append(ALPHA_NUMERIC_STRING.charAt(character)); } return sb.toString(); } public static void main(String[] args) throws Exception { new UnexpectedArrayContents().demonstrate(); } } 

Update

Now the error is tracked as JDK-8191002 . Affected Versions: 8,9,10.