# HG changeset patch
# User xuelei
# Date 1360281955 28800
# Node ID 946ec9b2200484111b98f481d90a198cf1042661
# Parent fc9eb3e707342ccf97343cf43337c48c9b8edb99
8006777: Improve TLS handling of invalid messages
Reviewed-by: wetmore, ahgross
diff -r fc9eb3e70734 -r 946ec9b22004 jdk/src/share/classes/sun/security/ssl/CipherBox.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;
+ }
+
}
diff -r fc9eb3e70734 -r 946ec9b22004 jdk/src/share/classes/sun/security/ssl/CipherSuite.java
--- 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.
diff -r fc9eb3e70734 -r 946ec9b22004 jdk/src/share/classes/sun/security/ssl/EngineInputRecord.java
--- 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
diff -r fc9eb3e70734 -r 946ec9b22004 jdk/src/share/classes/sun/security/ssl/EngineOutputRecord.java
--- 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.
diff -r fc9eb3e70734 -r 946ec9b22004 jdk/src/share/classes/sun/security/ssl/InputRecord.java
--- 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.
diff -r fc9eb3e70734 -r 946ec9b22004 jdk/src/share/classes/sun/security/ssl/MAC.java
--- 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.)
- *
- * NOTE: MAC computation is the only place in the SSL protocol that the
+ *
+ * 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) {
diff -r fc9eb3e70734 -r 946ec9b22004 jdk/src/share/classes/sun/security/ssl/OutputRecord.java
--- 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);
}
}
diff -r fc9eb3e70734 -r 946ec9b22004 jdk/src/share/classes/sun/security/ssl/SSLEngineImpl.java
--- 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,
diff -r fc9eb3e70734 -r 946ec9b22004 jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java
--- 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,