Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-7160837

DigestOutputStream does not turn off digest calculation when "close()" is called

    Details

    • Subcomponent:
    • Resolved In Build:
      b94
    • CPU:
      x86
    • OS:
      windows_7
    • Verification:
      Verified

      Backports

        Description

        FULL PRODUCT VERSION :
        java version "1.7.0_03"
        Java(TM) SE Runtime Environment (build 1.7.0_03-b05)
        Java HotSpot(TM) Client VM (build 22.1-b02, mixed mode, sharing)

        ADDITIONAL OS VERSION INFORMATION :
        Microsoft Windows [Version 6.1.7601]

        EXTRA RELEVANT SYSTEM CONFIGURATION :
        CPU: Intel Core i7 2600K
        RAM: 8GB

        A DESCRIPTION OF THE PROBLEM :
        As DigestOutputStream does not implement the FilterOutputStream.close() method, it has no awareness of the stream being actually closed. When the stream is closed and digest calculation is left "on", this object continues updating its MessageDigest on subsequent calls on any of its write() methods.

        This causes problem in case of nested OutputStreams. In particular, nesting an ObjectOutputStream over a CipherOutputStream (with block ciphers such as AES) which is on top of a DigestOutputStream reveals the problem, that is, when we are finished with the inner most stream (the ObjectOutputSteam in this case), we have to call on its close method. This call, in turn, forces a flush on CipherOutputStream which DOES NOT FLUSH its unprocessed buffers. Then comes the turn for CipherOutputSteam being closed, which first performs a flush and then a final output of its residue buffers to the underlying stream (DigestOutputStream), followed by a flush and close on DigestOutputStream.

        Things are pretty clear up to this point, but for some strange reasons, CipherOutputSteam decides to do a flush and re-write its latest buffer values after it has closed its underlying stream. Since DigestOutputStream has not taken last "close()" call seriously, it accepts the "write()" call and updates its digest AND THEN passes the call to the underlying stream which reasonably throws a "Stream Closed" IOExcepion, but since the CipherOutputStram is already closed, it will not re-throw the exception and no one on the outside will eve be told of this. But, the one who is left in the dark is the poor DigestOutputStream which has already updated its digest WITHOUT first taking into account its state and to a lesser degree WITHOUT waiting for an underlying "write()" to succeed.

        It might be more of a problem by CipherOutputStream calling "write()" after being closed, but more or less, it is also a flaw in DigestOutputStream.


        STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
        1. Create a FileOutputStream
        2. Create a DigestOutputStream (SHA-1) over it
        3. Initialize a block-cipher (AES-128) and create a CipherOutputStream over the previous one
        4. Create an ObjectOutputStream
        5. Write some data to the ObjectOutputStream
        6. Close the ObjectOutputStream
        7. Close the CipherOutputSteam (if necessary)
        8. Close the DigestOutputStream (if necessary)
        9. Close the FileOutputStream (if necessary)
        10. Call "digest()" on the MessageDigest object of the DigestOutputStream and store the result.

        EXPECTED VERSUS ACTUAL BEHAVIOR :
        EXPECTED -
        The result stored in step 10 must be equal to an independently calculated SHA-1 checksum of the file contents, which is surprisingly not, as mentioned earlier.
        ACTUAL -
        A different checksum, due to a late call to "write()" method which INCORRECTLY updated the message digest.

        ERROR MESSAGES/STACK TRACES THAT OCCUR :
        no error message is produces here

        REPRODUCIBILITY :
        This bug can be reproduced always.

        ---------- BEGIN SOURCE ----------
        import java.io.FileInputStream;
        import java.io.FileOutputStream;
        import java.io.ObjectOutputStream;
        import java.security.DigestOutputStream;
        import java.security.MessageDigest;
        import java.util.Arrays;

        import javax.crypto.Cipher;
        import javax.crypto.CipherOutputStream;
        import javax.crypto.KeyGenerator;
        import javax.crypto.SecretKey;

        public class Test
        {
        public static void main(String[] args) throws Exception
        {
        /**
        * Producing the problem
        */
        MessageDigest sha1 = MessageDigest.getInstance("SHA1");
        KeyGenerator aesKeyGen = KeyGenerator.getInstance("AES");
        aesKeyGen.init(128);
        SecretKey aesKey = aesKeyGen.generateKey();
        Cipher aes = Cipher.getInstance("AES");
        aes.init(Cipher.ENCRYPT_MODE, aesKey);
        try (FileOutputStream fos = new FileOutputStream("test.bin");
        DigestOutputStream dos = new DigestOutputStream(fos, sha1);
        CipherOutputStream cos = new CipherOutputStream(dos, aes);
        ObjectOutputStream oos = new ObjectOutputStream(cos))
        {
        oos.writeObject("This is a sample data to be AES encrypted.");
        }
        byte[] incorrectSha1 = sha1.digest();

        /**
        * Calculating the correct SHA1 for the file
        */
        byte[] correctSha1 = null;
        sha1.reset();
        try (FileInputStream fis = new FileInputStream("test.bin"))
        {
        byte[] buffer = new byte[1024];
        int count;
        count = fis.read(buffer);

        sha1.update(buffer, 0, count);
        correctSha1 = sha1.digest();
        }

        if(Arrays.equals(correctSha1, incorrectSha1) == false)
        System.out.println("The flaw still exists!");
        else
        System.out.println("The problem is resolved.");
        }
        }

        ---------- END SOURCE ----------

        CUSTOMER SUBMITTED WORKAROUND :
        The simplest solution to work this around is to subclass DigestOutputStream, override the "close()" method and TURN ITS CALCULATIONS OFF by calling "on(false)" after calling the "super.close()". Following snippet demonstrate it:

        public static class CorrectDigestOutputStream extends DigestOutputStream
        {

        public CorrectDigestOutputStream(OutputStream stream, MessageDigest digest)
        {
        super(stream, digest);
        }

        @Override
        public void close() throws IOException
        {
        super.close();
        this.on(false);
        }
        }

        We might be able to fix it by overriding the "write()" methods, but this one is more concise.

          Attachments

            Issue Links

              Activity

                People

                • Assignee:
                  ascarpino Anthony Scarpino
                  Reporter:
                  webbuggrp Webbug Group
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  3 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Imported:
                    Indexed: