JDK-7142509 : Cipher.doFinal(ByteBuffer,ByteBuffer) fails to process when in.remaining() == 0
  • Type: Bug
  • Component: security-libs
  • Sub-Component: javax.crypto
  • Affected Version: 7
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • OS: windows_xp
  • CPU: x86
  • Submitted: 2012-02-03
  • Updated: 2012-09-12
  • Resolved: 2012-09-12
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other JDK 6 JDK 7 JDK 8 Other
5.0u35,OpenJDK6Fixed 6u32Fixed 7u4Fixed 8 b27Fixed OpenJDK6Fixed
Description
FULL PRODUCT VERSION :
java version "1.7.0_02"
Java(TM) SE Runtime Environment (build 1.7.0_02-b13)
Java HotSpot(TM) Client VM (build 22.0-b10, mixed mode, sharing)
- OR -
Java HotSpot(TM) Server VM (build 22.0-b10, mixed mode)

ADDITIONAL OS VERSION INFORMATION :
Microsoft Windows XP [Version 5.1.2600]

A DESCRIPTION OF THE PROBLEM :
When invoking Cipher.doFinal(ByteBuffer, ByteBuffer) with input.remaining() == 0 and at least one of the buffers is a direct buffer, the method fails to process any data internally buffered in the cipher and store it in the output buffer.

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
1. Create and initialize a cipher, e.g. for the DES/CBC/PKCS5Padding transformation.
2. Allocate two separate byte buffers inBuffer and outBuffer, at least one of which is a direct buffer, of sufficient size, and put some random data into inBuffer.
3. Invoke cipher.update(inBuffer, outBuffer), which processes all data except for the last (partial or complete) block. inBuffer.hasRemaining() should now return false.
4. Invoke cipher.doFinal(inBuffer, outBuffer) to process the last block which is buffered in the cipher, applying padding if necessary.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
outBuffer should now contain all encrypted blocks, including the last one that was processed by doFinal(ByteBuffer, ByteBuffer).
Subsequently calling doFinal() [no arguments] twice will yield byte arrays with equal contents, the ciphertext for the empty message.
ACTUAL -
outBuffer contains all blocks EXCEPT the last one that should have been processed by doFinal(ByteBuffer, ByteBuffer).
Subsequently calling doFinal() [no arguments] twice will yield byte arrays with DIFFERENT contents, as the first invocation finally processes the data internally buffered in the cipher.

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
package javax.crypto.test;

import java.nio.ByteBuffer;
import java.security.SecureRandom;
import java.util.Arrays;
import java.util.Random;

import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;

/**
 * Simple test case to show that {@link javax.crypto.Cipher#doFinal(ByteBuffer, ByteBuffer)} fails to process the data
 * internally buffered in the cipher when <tt>input.remaining() == 0<tt> and at least one buffer is a direct buffer.
 */
public class BrokenDoFinal {

	public static void main(String args[]) throws Exception {
		Random random = new SecureRandom();

		byte[] keyBytes = new byte[8];
		random.nextBytes(keyBytes);
		SecretKey key = new SecretKeySpec(keyBytes, "DES");

		Cipher cipher = Cipher.getInstance("DES/CBC/PKCS5Padding");
		cipher.init(Cipher.ENCRYPT_MODE, key);

		int size = 12;

		byte[] testdata = new byte[size];
		random.nextBytes(testdata);

		byte[] expected = cipher.doFinal(testdata);

		// first case: both buffers are heap buffers
		System.out.print("Test case 1 (only heap buffers)       : ");
		ByteBuffer in1 = ByteBuffer.allocate(size);
		ByteBuffer out1 = ByteBuffer.allocate(cipher.getOutputSize(size));

		// this succeeds
		try {
			encrypt(in1, out1, cipher, testdata, expected);
			System.out.println("ok");
		} catch (AssertionError e) {
			System.out.println("FAILED -- " + e.getMessage());
		}
		if (!Arrays.equals(cipher.doFinal(), cipher.doFinal())) {
			System.out.println("  Internal buffers still held data!");
		}

		// second case: input buffer is direct buffer, output buffer is heap buffer
		System.out.print("Test case 2 (input buffer is direct)  : ");
		ByteBuffer in2 = ByteBuffer.allocateDirect(size);
		ByteBuffer out2 = ByteBuffer.allocate(cipher.getOutputSize(size));

		// this fails
		try {
			encrypt(in2, out2, cipher, testdata, expected);
			System.out.println("ok");
		} catch (AssertionError e) {
			System.out.println("FAILED -- " + e.getMessage());
		}
		if (!Arrays.equals(cipher.doFinal(), cipher.doFinal())) {
			System.out.println("  Internal buffers still held data!");
		}

		// third case: input buffer is heap buffer, output buffer is direct buffer
		System.out.print("Test case 3 (output buffer is direct) : ");
		ByteBuffer in3 = ByteBuffer.allocate(size);
		ByteBuffer out3 = ByteBuffer.allocateDirect(cipher.getOutputSize(size));

		// this fails, too
		try {
			encrypt(in3, out3, cipher, testdata, expected);
			System.out.println("ok");
		} catch (AssertionError e) {
			System.out.println("FAILED -- " + e.getMessage());
		}
		if (!Arrays.equals(cipher.doFinal(), cipher.doFinal())) {
			System.out.println("  Internal buffers still held data!");
		}

		// fourth case: both buffers are direct buffers
		System.out.print("Test case 4 (both buffers are direct) : ");
		ByteBuffer in4 = ByteBuffer.allocateDirect(size);
		ByteBuffer out4 = ByteBuffer.allocateDirect(cipher.getOutputSize(size));

		// and this of course also fails
		try {
			encrypt(in4, out4, cipher, testdata, expected);
			System.out.println("ok");
		} catch (AssertionError e) {
			System.out.println("FAILED -- " + e.getMessage());
		}
		if (!Arrays.equals(cipher.doFinal(), cipher.doFinal())) {
			System.out.println("  Internal buffers still held data!");
		}
	}

	private static void encrypt(ByteBuffer in, ByteBuffer out, Cipher cipher, byte[] testdata, byte[] expected)
			throws Exception {
		in.put(testdata);
		in.flip();

		// process all data
		cipher.update(in, out);
		if (in.hasRemaining()) {
			throw new RuntimeException("buffer not empty");
		}

		// finish encryption and process all data buffered in the cipher
		cipher.doFinal(in, out);
		out.flip();

		// validate output size
		if (out.remaining() != expected.length) {
			throw new AssertionError("incomplete encryption output, expected " + expected.length
					+ " bytes but was only " + out.remaining() + " bytes");
		}

		// validate output data
		byte[] encrypted = new byte[out.remaining()];
		out.get(encrypted);
		if (Arrays.equals(expected, encrypted) == false) {
			throw new AssertionError("bad encryption output");
		}
	}

}
---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Use Cipher.doFinal() [no arguments] to retrieve a byte array with the last block instead. If necessary for channel I/O, put the contents of the byte array into the output buffer.

Comments
WORK AROUND Reserve some bytes for calling doFinal().
09-02-2012

EVALUATION Yep, code is broken here: while (inLen > 0) { ...deleted... engineDoFinal(inArray, 0, chunk, outArray, outOfs); ...deleted... } The remaining 4 bytes are stuck in the implementation buffer.
09-02-2012

EVALUATION My tests on PKCS11 on solaris-i586 works fine. Likely a problem with the CipherSpi.java manipulations for implementations that don't override the ByteBuffer methods. Problem duplicated on SunJCE, should be able to isolate fairly quickly. Submitter reports: sure! It's telling "SunJCE version 1.7", as it is a fairly fresh JDK installation with no extension libs installed. The problem seems to be reproducable regardless of cipher algorithm, cipher mode (even for stream cipher modes, not only block cipher modes), or padding. I've also done a quick test using a dynamically registered Bouncy Castle JCE provider (version 1.46), with the same results. Security.addProvider(new BouncyCastleProvider()); Cipher cipher = Cipher.getInstance("DES/CBC/PKCS5Padding", "BC");
08-02-2012

EVALUATION Haven't verified this, but if there is no ByteBuffer implementation in the Cipher, we do a bunch of our own manipulations on this to change around to ByteBuffer. There could be a bug in this, or could be a specific provider where this is seen. Have asked the submitter for more information, will try to duplicate this soon.
04-02-2012