javax.crypto fixes
Marco Trudel
mtrudel@gmx.ch
Sat Sep 30 18:28:00 GMT 2006
I added another test for empty input. It will correctly handle empty
files or empty data...
Marco
Marco Trudel wrote:
> 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
>
> Cipher, CipherOutputStream and CipherInputStream now work as expected.
>
>
> 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...
>
> Ah, and finally: this is actually classpath code, can someone check that
> in there too, as soon as it's ok for gcj?
>
>
> 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,19 @@
> engineInit(opmode, key, spec, random);
> }
>
> - protected byte[] engineUpdate(byte[] input, int off, int len)
> + protected byte[] engineUpdate(byte[] input, int inOff, int inLen)
> {
> 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 +400,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 +457,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;
> - }
> }
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: crypto-patch2.txt
URL: <http://gcc.gnu.org/pipermail/java-patches/attachments/20060930/cc390b69/attachment.txt>
More information about the Java-patches
mailing list