I fixed some stuff in javax.crypto:
1. decryption with padding was broken/wrong handled
2. CipherOutputStream was completly broken/unusuable
3. PKCS7 did an unnecessary test
1. decryption with padding needs to keep back the last block for final
unpadding when doFinal() is called. This wasn't done. doFinal() lead to
an exception.
Actually, padded decrypting only worked correct when all data was passed
by doFinal(byte[]) or when there where update() calls that filled the
data to a multiple of the blocklengh and doFinal was called with the
rest of the data.
This fixes CiperInputStream as well, because it relies on the correct
doFinal() handling of the cipher class.
2. CipherOutputstream had a lot of code that did nothing except leading
to a NullPointerException when calling write(...) (outBuffer was never
initialized). It looks to me like the code should have worked around the
bugs in CipherAdapter. But that should have been done in CipherInputStream?!
3. PKCS7 unpadding tested a value that was just read with itself. Fixed
it because I was already reading it... Nothing big...
Any comments? Hints?
I have no committing rights and the copyright assignment papers are not
yet arrived with mail. This might be a problem...
Marco
Index: classpath/gnu/javax/crypto/jce/cipher/CipherAdapter.java
===================================================================
--- classpath/gnu/javax/crypto/jce/cipher/CipherAdapter.java (revision
117329)
+++ classpath/gnu/javax/crypto/jce/cipher/CipherAdapter.java (working copy)
@@ -373,14 +373,21 @@
engineInit(opmode, key, spec, random);
}
- protected byte[] engineUpdate(byte[] input, int off, int len)
+ protected byte[] engineUpdate(byte[] input, int inOff, int inLen)
{
+ if (inLen == 0) // nothing to process
+ return new byte[0];
final int blockSize = mode.currentBlockSize();
- final int count = (partLen + len) / blockSize;
- final byte[] out = new byte[count * blockSize];
+ int blockCount = (partLen + inLen) / blockSize;
+
+ // always keep data for unpadding in padded decryption mode; might even be
a complete block
+ if(pad != null && ((Integer) attributes.get(IMode.STATE)).intValue() ==
IMode.DECRYPTION &&
+ (partLen + inLen) % blockSize == 0) blockCount--;
+
+ final byte[] out = new byte[blockCount * blockSize];
try
{
- engineUpdate(input, off, len, out, 0);
+ engineUpdate(input, inOff, inLen, out, 0);
}
catch (ShortBufferException x) // should not happen
{
@@ -395,7 +402,12 @@
if (inLen == 0) // nothing to process
return 0;
final int blockSize = mode.currentBlockSize();
- final int blockCount = (partLen + inLen) / blockSize;
+ int blockCount = (partLen + inLen) / blockSize;
+
+ // always keep data for unpadding in padded decryption mode; might even be
a complete block
+ if(pad != null && ((Integer) attributes.get(IMode.STATE)).intValue() ==
IMode.DECRYPTION &&
+ (partLen + inLen) % blockSize == 0) blockCount--;
+
final int result = blockCount * blockSize;
if (result > out.length - outOff)
throw new ShortBufferException();
@@ -447,16 +459,20 @@
break;
case IMode.DECRYPTION:
int padLen;
+ byte[] buf3 = new byte[buf.length + partLen];
try
{
- padLen = pad.unpad(buf, 0, buf.length);
+ if(partLen != 16) throw new WrongPaddingException();
+ System.arraycopy(buf, 0, buf3, 0, buf.length);
+ mode.update(partBlock, 0, buf3, buf.length);
+ padLen = pad.unpad(buf3, 0, buf3.length);
}
catch (WrongPaddingException wpe)
{
throw new BadPaddingException(wpe.getMessage());
}
- result = new byte[buf.length - padLen];
- System.arraycopy(buf, 0, result, 0, result.length);
+ result = new byte[buf3.length - padLen];
+ System.arraycopy(buf3, 0, result, 0, result.length);
break;
default:
throw new IllegalStateException();
Index: classpath/gnu/javax/crypto/pad/PKCS7.java
===================================================================
--- classpath/gnu/javax/crypto/pad/PKCS7.java (revision 117306)
+++ classpath/gnu/javax/crypto/pad/PKCS7.java (working copy)
@@ -100,8 +100,8 @@
throws WrongPaddingException
{
int limit = offset + length;
- int result = in[limit - 1] & 0xFF;
- for (int i = 0; i < result; i++)
+ int result = in[--limit] & 0xFF;
+ for (int i = 0; i < result-1; i++)
if (result != (in[--limit] & 0xFF))
throw new WrongPaddingException();
if (Configuration.DEBUG)
Index: classpath/javax/crypto/CipherOutputStream.java
===================================================================
--- classpath/javax/crypto/CipherOutputStream.java (revision 117306)
+++ classpath/javax/crypto/CipherOutputStream.java (working copy)
@@ -50,30 +50,10 @@
*/
public class CipherOutputStream extends FilterOutputStream
{
-
- // Fields.
- // ------------------------------------------------------------------------
-
/** The underlying cipher. */
private Cipher cipher;
- private byte[][] inBuffer;
- private int inLength;
-
- private byte[] outBuffer;
-
- private static final int FIRST_TIME = 0;
- private static final int SECOND_TIME = 1;
- private static final int SEASONED = 2;
- private int state;
-
- /** True if the cipher is a stream cipher (blockSize == 1) */
- private boolean isStream;
-
- // Constructors.
- // ------------------------------------------------------------------------
-
/**
* Create a new cipher output stream. The cipher argument must have
* already been initialized.
@@ -84,20 +64,7 @@
public CipherOutputStream(OutputStream out, Cipher cipher)
{
super(out);
- if (cipher != null)
- {
- this.cipher = cipher;
- if (!(isStream = cipher.getBlockSize() == 1))
- {
- inBuffer = new byte[2][];
- inBuffer[0] = new byte[cipher.getBlockSize()];
- inBuffer[1] = new byte[cipher.getBlockSize()];
- inLength = 0;
- state = FIRST_TIME;
- }
- }
- else
- this.cipher = new NullCipher();
+ this.cipher = (cipher != null) ? cipher : new NullCipher();
}
/**
@@ -110,8 +77,6 @@
super(out);
}
- // Instance methods.
- // ------------------------------------------------------------------------
/**
* Close this output stream, and the sink output stream.
@@ -126,30 +91,17 @@
public void close() throws IOException
{
try
- {
- int len;
- if (state != FIRST_TIME)
- {
- len = cipher.update(inBuffer[0], 0, inBuffer[0].length, outBuffer);
- out.write(outBuffer, 0, len);
- }
- len = cipher.doFinal(inBuffer[0], 0, inLength, outBuffer);
- out.write(outBuffer, 0, len);
- }
- catch (javax.crypto.IllegalBlockSizeException ibse)
- {
- throw new IOException(ibse.toString());
- }
- catch (javax.crypto.BadPaddingException bpe)
- {
- throw new IOException(bpe.toString());
- }
- catch (ShortBufferException sbe)
- {
- throw new IOException(sbe.toString());
- }
- out.flush();
- out.close();
+ {
+ out.write(cipher.doFinal());
+ out.flush();
+ out.close();
+ } catch (javax.crypto.IllegalBlockSizeException ibse)
+ {
+ throw new IOException(ibse.toString());
+ } catch (javax.crypto.BadPaddingException bpe)
+ {
+ throw new IOException(bpe.toString());
+ }
}
/**
@@ -172,23 +124,7 @@
*/
public void write(int b) throws IOException
{
- if (isStream)
- {
- byte[] buf = new byte[] { (byte) b };
- try
- {
- cipher.update(buf, 0, 1, buf, 0);
- }
- catch (ShortBufferException sbe)
- {
- throw new IOException(sbe.toString());
- }
- out.write(buf);
- return;
- }
- inBuffer[1][inLength++] = (byte) b;
- if (inLength == inBuffer[1].length)
- process();
+ write(new byte[] { (byte) b }, 0, 1);
}
/**
@@ -216,53 +152,6 @@
*/
public void write(byte[] buf, int off, int len) throws IOException
{
- if (isStream)
- {
- out.write(cipher.update(buf, off, len));
- return;
- }
- int count = 0;
- while (count < len)
- {
- int l = Math.min(inBuffer[1].length - inLength, len - count);
- System.arraycopy(buf, off+count, inBuffer[1], inLength, l);
- count += l;
- inLength += l;
- if (inLength == inBuffer[1].length)
- process();
- }
+ out.write(cipher.update(buf, off, len));
}
-
- // Own method.
- // -------------------------------------------------------------------------
-
- private void process() throws IOException
- {
- if (state == SECOND_TIME)
- {
- state = SEASONED;
- }
- else
- {
- byte[] temp = inBuffer[0];
- inBuffer[0] = inBuffer[1];
- inBuffer[1] = temp;
- }
- if (state == FIRST_TIME)
- {
- inLength = 0;
- state = SECOND_TIME;
- return;
- }
- try
- {
- cipher.update(inBuffer[0], 0, inBuffer[0].length, outBuffer);
- }
- catch (ShortBufferException sbe)
- {
- throw new IOException(sbe.toString());
- }
- out.write(outBuffer);
- inLength = 0;
- }
}
