8006777: Improve TLS handling of invalid messages
authorxuelei
Thu, 07 Feb 2013 16:05:55 -0800
changeset 16113 946ec9b22004
parent 16112 fc9eb3e70734
child 16114 83963213d5fd
8006777: Improve TLS handling of invalid messages Reviewed-by: wetmore, ahgross
jdk/src/share/classes/sun/security/ssl/CipherBox.java
jdk/src/share/classes/sun/security/ssl/CipherSuite.java
jdk/src/share/classes/sun/security/ssl/EngineInputRecord.java
jdk/src/share/classes/sun/security/ssl/EngineOutputRecord.java
jdk/src/share/classes/sun/security/ssl/InputRecord.java
jdk/src/share/classes/sun/security/ssl/MAC.java
jdk/src/share/classes/sun/security/ssl/OutputRecord.java
jdk/src/share/classes/sun/security/ssl/SSLEngineImpl.java
jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java
--- a/jdk/src/share/classes/sun/security/ssl/CipherBox.java	Thu Feb 07 09:41:47 2013 -0800
+++ b/jdk/src/share/classes/sun/security/ssl/CipherBox.java	Thu Feb 07 16:05:55 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -392,7 +392,8 @@
      * uniformly use the bad_record_mac alert to hide the specific type of
      * the error.
      */
-    int decrypt(byte[] buf, int offset, int len) throws BadPaddingException {
+    int decrypt(byte[] buf, int offset, int len,
+            int tagLen) throws BadPaddingException {
         if (cipher == null) {
             return len;
         }
@@ -416,9 +417,10 @@
                         System.out);
                 } catch (IOException e) { }
             }
+
             if (blockSize != 0) {
-                newLen = removePadding(buf, offset, newLen,
-                             blockSize, protocolVersion);
+                newLen = removePadding(
+                    buf, offset, newLen, tagLen, blockSize, protocolVersion);
 
                 if (protocolVersion.v >= ProtocolVersion.TLS11.v) {
                     if (newLen < blockSize) {
@@ -448,7 +450,7 @@
      *
      *  @see decrypt(byte[], int, int)
      */
-    int decrypt(ByteBuffer bb) throws BadPaddingException {
+    int decrypt(ByteBuffer bb, int tagLen) throws BadPaddingException {
 
         int len = bb.remaining();
 
@@ -471,7 +473,6 @@
             }
 
             if (debug != null && Debug.isOn("plaintext")) {
-                bb.position(pos);
                 try {
                     HexDumpEncoder hd = new HexDumpEncoder();
 
@@ -479,7 +480,8 @@
                         "Padded plaintext after DECRYPTION:  len = "
                         + newLen);
 
-                    hd.encodeBuffer(bb, System.out);
+                    hd.encodeBuffer(
+                        (ByteBuffer)bb.duplicate().position(pos), System.out);
                 } catch (IOException e) { }
             }
 
@@ -488,7 +490,8 @@
              */
             if (blockSize != 0) {
                 bb.position(pos);
-                newLen = removePadding(bb, blockSize, protocolVersion);
+                newLen = removePadding(
+                    bb, tagLen, blockSize, protocolVersion);
 
                 if (protocolVersion.v >= ProtocolVersion.TLS11.v) {
                     if (newLen < blockSize) {
@@ -590,6 +593,65 @@
         return newlen;
     }
 
+    /*
+     * A constant-time check of the padding.
+     *
+     * NOTE that we are checking both the padding and the padLen bytes here.
+     *
+     * The caller MUST ensure that the len parameter is a positive number.
+     */
+    private static int[] checkPadding(
+            byte[] buf, int offset, int len, byte pad) {
+
+        if (len <= 0) {
+            throw new RuntimeException("padding len must be positive");
+        }
+
+        // An array of hits is used to prevent Hotspot optimization for
+        // the purpose of a constant-time check.
+        int[] results = {0, 0};    // {missed #, matched #}
+        for (int i = 0; i <= 256;) {
+            for (int j = 0; j < len && i <= 256; j++, i++) {     // j <= i
+                if (buf[offset + j] != pad) {
+                    results[0]++;       // mismatched padding data
+                } else {
+                    results[1]++;       // matched padding data
+                }
+            }
+        }
+
+        return results;
+    }
+
+    /*
+     * A constant-time check of the padding.
+     *
+     * NOTE that we are checking both the padding and the padLen bytes here.
+     *
+     * The caller MUST ensure that the bb parameter has remaining.
+     */
+    private static int[] checkPadding(ByteBuffer bb, byte pad) {
+
+        if (!bb.hasRemaining()) {
+            throw new RuntimeException("hasRemaining() must be positive");
+        }
+
+        // An array of hits is used to prevent Hotspot optimization for
+        // the purpose of a constant-time check.
+        int[] results = {0, 0};    // {missed #, matched #}
+        bb.mark();
+        for (int i = 0; i <= 256; bb.reset()) {
+            for (; bb.hasRemaining() && i <= 256; i++) {
+                if (bb.get() != pad) {
+                    results[0]++;       // mismatched padding data
+                } else {
+                    results[1]++;       // matched padding data
+                }
+            }
+        }
+
+        return results;
+    }
 
     /*
      * Typical TLS padding format for a 64 bit block cipher is as follows:
@@ -602,86 +664,95 @@
      * as it makes the data a multiple of the block size
      */
     private static int removePadding(byte[] buf, int offset, int len,
-            int blockSize, ProtocolVersion protocolVersion)
-            throws BadPaddingException {
+            int tagLen, int blockSize,
+            ProtocolVersion protocolVersion) throws BadPaddingException {
+
         // last byte is length byte (i.e. actual padding length - 1)
         int padOffset = offset + len - 1;
-        int pad = buf[padOffset] & 0x0ff;
+        int padLen = buf[padOffset] & 0xFF;
 
-        int newlen = len - (pad + 1);
-        if (newlen < 0) {
-            throw new BadPaddingException("Padding length invalid: " + pad);
+        int newLen = len - (padLen + 1);
+        if ((newLen - tagLen) < 0) {
+            // If the buffer is not long enough to contain the padding plus
+            // a MAC tag, do a dummy constant-time padding check.
+            //
+            // Note that it is a dummy check, so we won't care about what is
+            // the actual padding data.
+            checkPadding(buf, offset, len, (byte)(padLen & 0xFF));
+
+            throw new BadPaddingException("Invalid Padding length: " + padLen);
         }
 
+        // The padding data should be filled with the padding length value.
+        int[] results = checkPadding(buf, offset + newLen,
+                        padLen + 1, (byte)(padLen & 0xFF));
         if (protocolVersion.v >= ProtocolVersion.TLS10.v) {
-            for (int i = 1; i <= pad; i++) {
-                int val = buf[padOffset - i] & 0xff;
-                if (val != pad) {
-                    throw new BadPaddingException
-                                        ("Invalid TLS padding: " + val);
-                }
+            if (results[0] != 0) {          // padding data has invalid bytes
+                throw new BadPaddingException("Invalid TLS padding data");
             }
         } else { // SSLv3
             // SSLv3 requires 0 <= length byte < block size
             // some implementations do 1 <= length byte <= block size,
             // so accept that as well
             // v3 does not require any particular value for the other bytes
-            if (pad > blockSize) {
-                throw new BadPaddingException("Invalid SSLv3 padding: " + pad);
+            if (padLen > blockSize) {
+                throw new BadPaddingException("Invalid SSLv3 padding");
             }
         }
-        return newlen;
+        return newLen;
     }
 
     /*
      * Position/limit is equal the removed padding.
      */
     private static int removePadding(ByteBuffer bb,
-            int blockSize, ProtocolVersion protocolVersion)
-            throws BadPaddingException {
+            int tagLen, int blockSize,
+            ProtocolVersion protocolVersion) throws BadPaddingException {
 
         int len = bb.remaining();
         int offset = bb.position();
 
         // last byte is length byte (i.e. actual padding length - 1)
         int padOffset = offset + len - 1;
-        int pad = bb.get(padOffset) & 0x0ff;
+        int padLen = bb.get(padOffset) & 0xFF;
 
-        int newlen = len - (pad + 1);
-        if (newlen < 0) {
-            throw new BadPaddingException("Padding length invalid: " + pad);
+        int newLen = len - (padLen + 1);
+        if ((newLen - tagLen) < 0) {
+            // If the buffer is not long enough to contain the padding plus
+            // a MAC tag, do a dummy constant-time padding check.
+            //
+            // Note that it is a dummy check, so we won't care about what is
+            // the actual padding data.
+            checkPadding(bb.duplicate(), (byte)(padLen & 0xFF));
+
+            throw new BadPaddingException("Invalid Padding length: " + padLen);
         }
 
-        /*
-         * We could zero the padding area, but not much useful
-         * information there.
-         */
+        // The padding data should be filled with the padding length value.
+        int[] results = checkPadding(
+                (ByteBuffer)bb.duplicate().position(offset + newLen),
+                (byte)(padLen & 0xFF));
         if (protocolVersion.v >= ProtocolVersion.TLS10.v) {
-            bb.put(padOffset, (byte)0);         // zero the padding.
-            for (int i = 1; i <= pad; i++) {
-                int val = bb.get(padOffset - i) & 0xff;
-                if (val != pad) {
-                    throw new BadPaddingException
-                                        ("Invalid TLS padding: " + val);
-                }
+            if (results[0] != 0) {          // padding data has invalid bytes
+                throw new BadPaddingException("Invalid TLS padding data");
             }
         } else { // SSLv3
             // SSLv3 requires 0 <= length byte < block size
             // some implementations do 1 <= length byte <= block size,
             // so accept that as well
             // v3 does not require any particular value for the other bytes
-            if (pad > blockSize) {
-                throw new BadPaddingException("Invalid SSLv3 padding: " + pad);
+            if (padLen > blockSize) {
+                throw new BadPaddingException("Invalid SSLv3 padding");
             }
         }
 
         /*
          * Reset buffer limit to remove padding.
          */
-        bb.position(offset + newlen);
-        bb.limit(offset + newlen);
+        bb.position(offset + newLen);
+        bb.limit(offset + newLen);
 
-        return newlen;
+        return newLen;
     }
 
     /*
@@ -708,4 +779,45 @@
     boolean isCBCMode() {
         return isCBCMode;
     }
+
+    /**
+     * Is the cipher null?
+     *
+     * @return true if the cipher is null, false otherwise.
+     */
+    boolean isNullCipher() {
+        return cipher == null;
+    }
+
+    /**
+     * Sanity check the length of a fragment before decryption.
+     *
+     * In CBC mode, check that the fragment length is one or multiple times
+     * of the block size of the cipher suite, and is at least one (one is the
+     * smallest size of padding in CBC mode) bigger than the tag size of the
+     * MAC algorithm except the explicit IV size for TLS 1.1 or later.
+     *
+     * In non-CBC mode, check that the fragment length is not less than the
+     * tag size of the MAC algorithm.
+     *
+     * @return true if the length of a fragment matches above requirements
+     */
+    boolean sanityCheck(int tagLen, int fragmentLen) {
+        if (!isCBCMode) {
+            return fragmentLen >= tagLen;
+        }
+
+        if ((fragmentLen % blockSize) == 0) {
+            int minimal = tagLen + 1;
+            minimal = (minimal >= blockSize) ? minimal : blockSize;
+            if (protocolVersion.v >= ProtocolVersion.TLS11.v) {
+                minimal += blockSize;   // plus the size of the explicit IV
+            }
+
+            return (fragmentLen >= minimal);
+        }
+
+        return false;
+    }
+
 }
--- a/jdk/src/share/classes/sun/security/ssl/CipherSuite.java	Thu Feb 07 09:41:47 2013 -0800
+++ b/jdk/src/share/classes/sun/security/ssl/CipherSuite.java	Thu Feb 07 16:05:55 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2002, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -549,9 +549,18 @@
         // size of the MAC value (and MAC key) in bytes
         final int size;
 
-        MacAlg(String name, int size) {
+        // block size of the underlying hash algorithm
+        final int hashBlockSize;
+
+        // minimal padding size of the underlying hash algorithm
+        final int minimalPaddingSize;
+
+        MacAlg(String name, int size,
+                int hashBlockSize, int minimalPaddingSize) {
             this.name = name;
             this.size = size;
+            this.hashBlockSize = hashBlockSize;
+            this.minimalPaddingSize = minimalPaddingSize;
         }
 
         /**
@@ -596,11 +605,11 @@
                         new BulkCipher(CIPHER_AES,     32, 16, true);
 
     // MACs
-    final static MacAlg M_NULL = new MacAlg("NULL", 0);
-    final static MacAlg M_MD5  = new MacAlg("MD5", 16);
-    final static MacAlg M_SHA  = new MacAlg("SHA", 20);
-    final static MacAlg M_SHA256  = new MacAlg("SHA256", 32);
-    final static MacAlg M_SHA384  = new MacAlg("SHA384", 48);
+    final static MacAlg M_NULL    = new MacAlg("NULL",     0,   0,   0);
+    final static MacAlg M_MD5     = new MacAlg("MD5",     16,  64,   9);
+    final static MacAlg M_SHA     = new MacAlg("SHA",     20,  64,   9);
+    final static MacAlg M_SHA256  = new MacAlg("SHA256",  32,  64,   9);
+    final static MacAlg M_SHA384  = new MacAlg("SHA384",  48, 128,  17);
 
     /**
      * PRFs (PseudoRandom Function) from TLS specifications.
--- a/jdk/src/share/classes/sun/security/ssl/EngineInputRecord.java	Thu Feb 07 09:41:47 2013 -0800
+++ b/jdk/src/share/classes/sun/security/ssl/EngineInputRecord.java	Thu Feb 07 16:05:55 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2007, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -178,71 +178,6 @@
     }
 
     /*
-     * Verifies and removes the MAC value.  Returns true if
-     * the MAC checks out OK.
-     *
-     * On entry:
-     *     position = beginning of app/MAC data
-     *     limit = end of MAC data.
-     *
-     * On return:
-     *     position = beginning of app data
-     *     limit = end of app data
-     */
-    boolean checkMAC(MAC signer, ByteBuffer bb) {
-        if (internalData) {
-            return checkMAC(signer);
-        }
-
-        int len = signer.MAClen();
-        if (len == 0) { // no mac
-            return true;
-        }
-
-        /*
-         * Grab the original limit
-         */
-        int lim = bb.limit();
-
-        /*
-         * Delineate the area to apply a MAC on.
-         */
-        int macData = lim - len;
-        bb.limit(macData);
-
-        byte[] mac = signer.compute(contentType(), bb);
-
-        if (len != mac.length) {
-            throw new RuntimeException("Internal MAC error");
-        }
-
-        /*
-         * Delineate the MAC values, position was already set
-         * by doing the compute above.
-         *
-         * We could zero the MAC area, but not much useful information
-         * there anyway.
-         */
-        bb.position(macData);
-        bb.limit(lim);
-
-        try {
-            for (int i = 0; i < len; i++) {
-                if (bb.get() != mac[i]) {  // No BB.equals(byte []); !
-                    return false;
-                }
-            }
-            return true;
-        } finally {
-            /*
-             * Position to the data.
-             */
-            bb.rewind();
-            bb.limit(macData);
-        }
-    }
-
-    /*
      * Pass the data down if it's internally cached, otherwise
      * do it here.
      *
@@ -251,21 +186,164 @@
      * If external data(app), return a new ByteBuffer with data to
      * process.
      */
-    ByteBuffer decrypt(CipherBox box, ByteBuffer bb)
-            throws BadPaddingException {
+    ByteBuffer decrypt(MAC signer,
+            CipherBox box, ByteBuffer bb) throws BadPaddingException {
 
         if (internalData) {
-            decrypt(box);
+            decrypt(signer, box);   // MAC is checked during decryption
             return tmpBB;
         }
 
-        box.decrypt(bb);
-        bb.rewind();
+        BadPaddingException reservedBPE = null;
+        int tagLen = signer.MAClen();
+        int cipheredLength = bb.remaining();
+
+        if (!box.isNullCipher()) {
+            // sanity check length of the ciphertext
+            if (!box.sanityCheck(tagLen, cipheredLength)) {
+                throw new BadPaddingException(
+                    "ciphertext sanity check failed");
+            }
+
+            try {
+                // Note that the CipherBox.decrypt() does not change
+                // the capacity of the buffer.
+                box.decrypt(bb, tagLen);
+            } catch (BadPaddingException bpe) {
+                // RFC 2246 states that decryption_failed should be used
+                // for this purpose. However, that allows certain attacks,
+                // so we just send bad record MAC. We also need to make
+                // sure to always check the MAC to avoid a timing attack
+                // for the same issue. See paper by Vaudenay et al and the
+                // update in RFC 4346/5246.
+                //
+                // Failover to message authentication code checking.
+                reservedBPE = bpe;
+            } finally {
+                bb.rewind();
+            }
+        }
+
+        if (tagLen != 0) {
+            int macOffset = bb.limit() - tagLen;
+
+            // Note that although it is not necessary, we run the same MAC
+            // computation and comparison on the payload for both stream
+            // cipher and CBC block cipher.
+            if (bb.remaining() < tagLen) {
+                // negative data length, something is wrong
+                if (reservedBPE == null) {
+                    reservedBPE = new BadPaddingException("bad record");
+                }
+
+                // set offset of the dummy MAC
+                macOffset = cipheredLength - tagLen;
+                bb.limit(cipheredLength);
+            }
+
+            // Run MAC computation and comparison on the payload.
+            if (checkMacTags(contentType(), bb, signer, false)) {
+                if (reservedBPE == null) {
+                    reservedBPE = new BadPaddingException("bad record MAC");
+                }
+            }
+
+            // Run MAC computation and comparison on the remainder.
+            //
+            // It is only necessary for CBC block cipher.  It is used to get a
+            // constant time of MAC computation and comparison on each record.
+            if (box.isCBCMode()) {
+                int remainingLen = calculateRemainingLen(
+                                        signer, cipheredLength, macOffset);
+
+                // NOTE: here we use the InputRecord.buf because I did not find
+                // an effective way to work on ByteBuffer when its capacity is
+                // less than remainingLen.
+
+                // NOTE: remainingLen may be bigger (less than 1 block of the
+                // hash algorithm of the MAC) than the cipheredLength. However,
+                // We won't need to worry about it because we always use a
+                // maximum buffer for every record.  We need a change here if
+                // we use small buffer size in the future.
+                if (remainingLen > buf.length) {
+                    // unlikely to happen, just a placehold
+                    throw new RuntimeException(
+                        "Internal buffer capacity error");
+                }
+
+                // Won't need to worry about the result on the remainder. And
+                // then we won't need to worry about what's actual data to
+                // check MAC tag on.  We start the check from the header of the
+                // buffer so that we don't need to construct a new byte buffer.
+                checkMacTags(contentType(), buf, 0, remainingLen, signer, true);
+            }
+
+            bb.limit(macOffset);
+        }
+
+        // Is it a failover?
+        if (reservedBPE != null) {
+            throw reservedBPE;
+        }
 
         return bb.slice();
     }
 
     /*
+     * Run MAC computation and comparison
+     *
+     * Please DON'T change the content of the ByteBuffer parameter!
+     */
+    private static boolean checkMacTags(byte contentType, ByteBuffer bb,
+            MAC signer, boolean isSimulated) {
+
+        int tagLen = signer.MAClen();
+        int lim = bb.limit();
+        int macData = lim - tagLen;
+
+        bb.limit(macData);
+        byte[] hash = signer.compute(contentType, bb, isSimulated);
+        if (hash == null || tagLen != hash.length) {
+            // Something is wrong with MAC implementation.
+            throw new RuntimeException("Internal MAC error");
+        }
+
+        bb.position(macData);
+        bb.limit(lim);
+        try {
+            int[] results = compareMacTags(bb, hash);
+            return (results[0] != 0);
+        } finally {
+            bb.rewind();
+            bb.limit(macData);
+        }
+    }
+
+    /*
+     * A constant-time comparison of the MAC tags.
+     *
+     * Please DON'T change the content of the ByteBuffer parameter!
+     */
+    private static int[] compareMacTags(ByteBuffer bb, byte[] tag) {
+
+        // An array of hits is used to prevent Hotspot optimization for
+        // the purpose of a constant-time check.
+        int[] results = {0, 0};     // {missed #, matched #}
+
+        // The caller ensures there are enough bytes available in the buffer.
+        // So we won't need to check the remaining of the buffer.
+        for (int i = 0; i < tag.length; i++) {
+            if (bb.get() != tag[i]) {
+                results[0]++;       // mismatched bytes
+            } else {
+                results[1]++;       // matched bytes
+            }
+        }
+
+        return results;
+    }
+
+    /*
      * Override the actual write below.  We do things this way to be
      * consistent with InputRecord.  InputRecord may try to write out
      * data to the peer, and *then* throw an Exception.  This forces
--- a/jdk/src/share/classes/sun/security/ssl/EngineOutputRecord.java	Thu Feb 07 09:41:47 2013 -0800
+++ b/jdk/src/share/classes/sun/security/ssl/EngineOutputRecord.java	Thu Feb 07 16:05:55 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -118,7 +118,7 @@
             throws IOException {
 
         if (signer.MAClen() != 0) {
-            byte[] hash = signer.compute(contentType(), bb);
+            byte[] hash = signer.compute(contentType(), bb, false);
 
             /*
              * position was advanced to limit in compute above.
--- a/jdk/src/share/classes/sun/security/ssl/InputRecord.java	Thu Feb 07 09:41:47 2013 -0800
+++ b/jdk/src/share/classes/sun/security/ssl/InputRecord.java	Thu Feb 07 16:05:55 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2008, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -133,44 +133,174 @@
         return handshakeHash;
     }
 
-    /*
-     * Verify and remove the MAC ... used for all records.
-     */
-    boolean checkMAC(MAC signer) {
-        int len = signer.MAClen();
-        if (len == 0) { // no mac
-            return true;
+    void decrypt(MAC signer, CipherBox box) throws BadPaddingException {
+
+        BadPaddingException reservedBPE = null;
+        int tagLen = signer.MAClen();
+        int cipheredLength = count - headerSize;
+
+        if (!box.isNullCipher()) {
+            // sanity check length of the ciphertext
+            if (!box.sanityCheck(tagLen, cipheredLength)) {
+                throw new BadPaddingException(
+                    "ciphertext sanity check failed");
+            }
+
+            try {
+                // Note that the CipherBox.decrypt() does not change
+                // the capacity of the buffer.
+                count = headerSize +
+                        box.decrypt(buf, headerSize, cipheredLength, tagLen);
+            } catch (BadPaddingException bpe) {
+                // RFC 2246 states that decryption_failed should be used
+                // for this purpose. However, that allows certain attacks,
+                // so we just send bad record MAC. We also need to make
+                // sure to always check the MAC to avoid a timing attack
+                // for the same issue. See paper by Vaudenay et al and the
+                // update in RFC 4346/5246.
+                //
+                // Failover to message authentication code checking.
+                reservedBPE = bpe;
+            }
         }
 
-        int offset = count - len;
+        if (tagLen != 0) {
+            int macOffset = count - tagLen;
+            int contentLen = macOffset - headerSize;
+
+            // Note that although it is not necessary, we run the same MAC
+            // computation and comparison on the payload for both stream
+            // cipher and CBC block cipher.
+            if (contentLen < 0) {
+                // negative data length, something is wrong
+                if (reservedBPE == null) {
+                    reservedBPE = new BadPaddingException("bad record");
+                }
+
+                // set offset of the dummy MAC
+                macOffset = headerSize + cipheredLength - tagLen;
+                contentLen = macOffset - headerSize;
+            }
+
+            count -= tagLen;  // Set the count before any MAC checking
+                              // exception occurs, so that the following
+                              // process can read the actual decrypted
+                              // content (minus the MAC) in the fragment
+                              // if necessary.
 
-        if (offset < headerSize) {
-            // data length would be negative, something is wrong
-            return false;
+            // Run MAC computation and comparison on the payload.
+            if (checkMacTags(contentType(),
+                    buf, headerSize, contentLen, signer, false)) {
+                if (reservedBPE == null) {
+                    reservedBPE = new BadPaddingException("bad record MAC");
+                }
+            }
+
+            // Run MAC computation and comparison on the remainder.
+            //
+            // It is only necessary for CBC block cipher.  It is used to get a
+            // constant time of MAC computation and comparison on each record.
+            if (box.isCBCMode()) {
+                int remainingLen = calculateRemainingLen(
+                                        signer, cipheredLength, contentLen);
+
+                // NOTE: remainingLen may be bigger (less than 1 block of the
+                // hash algorithm of the MAC) than the cipheredLength. However,
+                // We won't need to worry about it because we always use a
+                // maximum buffer for every record.  We need a change here if
+                // we use small buffer size in the future.
+                if (remainingLen > buf.length) {
+                    // unlikely to happen, just a placehold
+                    throw new RuntimeException(
+                        "Internal buffer capacity error");
+                }
+
+                // Won't need to worry about the result on the remainder. And
+                // then we won't need to worry about what's actual data to
+                // check MAC tag on.  We start the check from the header of the
+                // buffer so that we don't need to construct a new byte buffer.
+                checkMacTags(contentType(), buf, 0, remainingLen, signer, true);
+            }
         }
 
-        byte[] mac = signer.compute(contentType(), buf,
-            headerSize, offset - headerSize);
+        // Is it a failover?
+        if (reservedBPE != null) {
+            throw reservedBPE;
+        }
+    }
 
-        if (len != mac.length) {
+    /*
+     * Run MAC computation and comparison
+     *
+     * Please DON'T change the content of the byte buffer parameter!
+     */
+    static boolean checkMacTags(byte contentType, byte[] buffer,
+            int offset, int contentLen, MAC signer, boolean isSimulated) {
+
+        int tagLen = signer.MAClen();
+        byte[] hash = signer.compute(
+                contentType, buffer, offset, contentLen, isSimulated);
+        if (hash == null || tagLen != hash.length) {
+            // Something is wrong with MAC implementation.
             throw new RuntimeException("Internal MAC error");
         }
 
-        for (int i = 0; i < len; i++) {
-            if (buf[offset + i] != mac[i]) {
-                return false;
+        int[] results = compareMacTags(buffer, offset + contentLen, hash);
+        return (results[0] != 0);
+    }
+
+    /*
+     * A constant-time comparison of the MAC tags.
+     *
+     * Please DON'T change the content of the byte buffer parameter!
+     */
+    private static int[] compareMacTags(
+            byte[] buffer, int offset, byte[] tag) {
+
+        // An array of hits is used to prevent Hotspot optimization for
+        // the purpose of a constant-time check.
+        int[] results = {0, 0};    // {missed #, matched #}
+
+        // The caller ensures there are enough bytes available in the buffer.
+        // So we won't need to check the length of the buffer.
+        for (int i = 0; i < tag.length; i++) {
+            if (buffer[offset + i] != tag[i]) {
+                results[0]++;       // mismatched bytes
+            } else {
+                results[1]++;       // matched bytes
             }
         }
-        count -= len;
-        return true;
+
+        return results;
     }
 
-    void decrypt(CipherBox box) throws BadPaddingException {
-        int len = count - headerSize;
-        count = headerSize + box.decrypt(buf, headerSize, len);
+    /*
+     * Calculate the length of a dummy buffer to run MAC computation
+     * and comparison on the remainder.
+     *
+     * The caller MUST ensure that the fullLen is not less than usedLen.
+     */
+    static int calculateRemainingLen(
+            MAC signer, int fullLen, int usedLen) {
+
+        int blockLen = signer.hashBlockLen();
+        int minimalPaddingLen = signer.minimalPaddingLen();
+
+        // (blockLen - minimalPaddingLen) is the maximum message size of
+        // the last block of hash function operation. See FIPS 180-4, or
+        // MD5 specification.
+        fullLen += 13 - (blockLen - minimalPaddingLen);
+        usedLen += 13 - (blockLen - minimalPaddingLen);
+
+        // Note: fullLen is always not less than usedLen, and blockLen
+        // is always bigger than minimalPaddingLen, so we don't worry
+        // about negative values. 0x01 is added to the result to ensure
+        // that the return value is positive.  The extra one byte does
+        // not impact the overall MAC compression function evaluations.
+        return 0x01 + (int)(Math.ceil(fullLen/(1.0d * blockLen)) -
+                Math.ceil(usedLen/(1.0d * blockLen))) * signer.hashBlockLen();
     }
 
-
     /*
      * Well ... hello_request messages are _never_ hashed since we can't
      * know when they'd appear in the sequence.
--- a/jdk/src/share/classes/sun/security/ssl/MAC.java	Thu Feb 07 09:41:47 2013 -0800
+++ b/jdk/src/share/classes/sun/security/ssl/MAC.java	Thu Feb 07 16:05:55 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -43,8 +43,8 @@
  * provide integrity protection for SSL messages.  The MAC is actually
  * one of several keyed hashes, as associated with the cipher suite and
  * protocol version.  (SSL v3.0 uses one construct, TLS uses another.)
- *
- * <P>NOTE: MAC computation is the only place in the SSL protocol that the
+ * <P>
+ * NOTE: MAC computation is the only place in the SSL protocol that the
  * sequence number is used.  It's also reset to zero with each change of
  * a cipher spec, so this is the only place this state is needed.
  *
@@ -58,6 +58,9 @@
     // Value of the null MAC is fixed
     private static final byte nullMAC[] = new byte[0];
 
+    // internal identifier for the MAC algorithm
+    private final MacAlg        macAlg;
+
     // stuff defined by the kind of MAC algorithm
     private final int           macSize;
 
@@ -82,6 +85,7 @@
 
     private MAC() {
         macSize = 0;
+        macAlg = M_NULL;
         mac = null;
         block = null;
     }
@@ -91,6 +95,7 @@
      */
     MAC(MacAlg macAlg, ProtocolVersion protocolVersion, SecretKey key)
             throws NoSuchAlgorithmException, InvalidKeyException {
+        this.macAlg = macAlg;
         this.macSize = macAlg.size;
 
         String algorithm;
@@ -128,15 +133,31 @@
     }
 
     /**
+     * Returns the hash function block length of the MAC alorithm.
+     */
+    int hashBlockLen() {
+        return macAlg.hashBlockSize;
+    }
+
+    /**
+     * Returns the hash function minimal padding length of the MAC alorithm.
+     */
+    int minimalPaddingLen() {
+        return macAlg.minimalPaddingSize;
+    }
+
+    /**
      * Computes and returns the MAC for the data in this byte array.
      *
      * @param type record type
      * @param buf compressed record on which the MAC is computed
      * @param offset start of compressed record data
      * @param len the size of the compressed record
+     * @param isSimulated if true, simulate the the MAC computation
      */
-    final byte[] compute(byte type, byte buf[], int offset, int len) {
-        return compute(type, null, buf, offset, len);
+    final byte[] compute(byte type, byte buf[],
+            int offset, int len, boolean isSimulated) {
+        return compute(type, null, buf, offset, len, isSimulated);
     }
 
     /**
@@ -149,9 +170,10 @@
      * @param type record type
      * @param bb a ByteBuffer in which the position and limit
      *          demarcate the data to be MAC'd.
+     * @param isSimulated if true, simulate the the MAC computation
      */
-    final byte[] compute(byte type, ByteBuffer bb) {
-        return compute(type, bb, null, 0, bb.remaining());
+    final byte[] compute(byte type, ByteBuffer bb, boolean isSimulated) {
+        return compute(type, bb, null, 0, bb.remaining(), isSimulated);
     }
 
     /**
@@ -204,18 +226,21 @@
      * or buf/offset/len.
      */
     private byte[] compute(byte type, ByteBuffer bb, byte[] buf,
-            int offset, int len) {
+            int offset, int len, boolean isSimulated) {
 
         if (macSize == 0) {
             return nullMAC;
         }
 
-        block[BLOCK_OFFSET_TYPE] = type;
-        block[block.length - 2]  = (byte)(len >> 8);
-        block[block.length - 1]  = (byte)(len     );
+        // MUST NOT increase the sequence number for a simulated computation.
+        if (!isSimulated) {
+            block[BLOCK_OFFSET_TYPE] = type;
+            block[block.length - 2]  = (byte)(len >> 8);
+            block[block.length - 1]  = (byte)(len     );
 
-        mac.update(block);
-        incrementSequenceNumber();
+            mac.update(block);
+            incrementSequenceNumber();
+        }
 
         // content
         if (bb != null) {
--- a/jdk/src/share/classes/sun/security/ssl/OutputRecord.java	Thu Feb 07 09:41:47 2013 -0800
+++ b/jdk/src/share/classes/sun/security/ssl/OutputRecord.java	Thu Feb 07 16:05:55 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -205,7 +205,7 @@
         }
         if (signer.MAClen() != 0) {
             byte[] hash = signer.compute(contentType, buf,
-                    headerSize, count - headerSize);
+                    headerSize, count - headerSize, false);
             write(hash);
         }
     }
--- a/jdk/src/share/classes/sun/security/ssl/SSLEngineImpl.java	Thu Feb 07 09:41:47 2013 -0800
+++ b/jdk/src/share/classes/sun/security/ssl/SSLEngineImpl.java	Thu Feb 07 16:05:55 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -958,35 +958,15 @@
              * throw a fatal alert if the integrity check fails.
              */
             try {
-                decryptedBB = inputRecord.decrypt(readCipher, readBB);
+                decryptedBB = inputRecord.decrypt(readMAC, readCipher, readBB);
             } catch (BadPaddingException e) {
-                // RFC 2246 states that decryption_failed should be used
-                // for this purpose. However, that allows certain attacks,
-                // so we just send bad record MAC. We also need to make
-                // sure to always check the MAC to avoid a timing attack
-                // for the same issue. See paper by Vaudenay et al.
-                //
-                // rewind the BB if necessary.
-                readBB.rewind();
-
-                inputRecord.checkMAC(readMAC, readBB);
-
-                // use the same alert types as for MAC failure below
                 byte alertType = (inputRecord.contentType() ==
                     Record.ct_handshake) ?
                         Alerts.alert_handshake_failure :
                         Alerts.alert_bad_record_mac;
-                fatal(alertType, "Invalid padding", e);
+                fatal(alertType, e.getMessage(), e);
             }
 
-            if (!inputRecord.checkMAC(readMAC, decryptedBB)) {
-                if (inputRecord.contentType() == Record.ct_handshake) {
-                    fatal(Alerts.alert_handshake_failure,
-                        "bad handshake record MAC");
-                } else {
-                    fatal(Alerts.alert_bad_record_mac, "bad record MAC");
-                }
-            }
 
             // if (!inputRecord.decompress(c))
             //     fatal(Alerts.alert_decompression_failure,
--- a/jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java	Thu Feb 07 09:41:47 2013 -0800
+++ b/jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java	Thu Feb 07 16:05:55 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -986,29 +986,13 @@
              * throw a fatal alert if the integrity check fails.
              */
             try {
-                r.decrypt(readCipher);
+                r.decrypt(readMAC, readCipher);
             } catch (BadPaddingException e) {
-                // RFC 2246 states that decryption_failed should be used
-                // for this purpose. However, that allows certain attacks,
-                // so we just send bad record MAC. We also need to make
-                // sure to always check the MAC to avoid a timing attack
-                // for the same issue. See paper by Vaudenay et al.
-                r.checkMAC(readMAC);
-                // use the same alert types as for MAC failure below
                 byte alertType = (r.contentType() == Record.ct_handshake)
                                         ? Alerts.alert_handshake_failure
                                         : Alerts.alert_bad_record_mac;
-                fatal(alertType, "Invalid padding", e);
+                fatal(alertType, e.getMessage(), e);
             }
-            if (!r.checkMAC(readMAC)) {
-                if (r.contentType() == Record.ct_handshake) {
-                    fatal(Alerts.alert_handshake_failure,
-                        "bad handshake record MAC");
-                } else {
-                    fatal(Alerts.alert_bad_record_mac, "bad record MAC");
-                }
-            }
-
 
             // if (!r.decompress(c))
             //     fatal(Alerts.alert_decompression_failure,