Skip to content

Commit

Permalink
Fixes HMAC return on authenticated AESCBC decrypt (iss #223)
Browse files Browse the repository at this point in the history
  • Loading branch information
vdzhuvinov committed Jun 1, 2017
1 parent 1ec634d commit 6a29f10
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 24 deletions.
33 changes: 10 additions & 23 deletions src/main/java/com/nimbusds/jose/crypto/AESCBC.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
*
* @author Vladimir Dzhuvinov
* @author Axel Nennker
* @version 2015-05-26
* @version 2017-05-30
*/
@ThreadSafe
class AESCBC {
Expand Down Expand Up @@ -351,21 +351,11 @@ public static byte[] decryptAuthenticated(final SecretKey secretKey,

byte[] expectedAuthTag = Arrays.copyOf(hmac, compositeKey.getTruncatedMACByteLength());

boolean macCheckPassed = true;

if (! ConstantTimeUtils.areEqual(expectedAuthTag, authTag)) {
// Thwart timing attacks by delaying exception until after decryption
macCheckPassed = false;
}

byte[] plainText = decrypt(compositeKey.getAESKey(), iv, cipherText, ceProvider);

if (! macCheckPassed) {

throw new JOSEException("MAC check failed");
}

return plainText;
return decrypt(compositeKey.getAESKey(), iv, cipherText, ceProvider);
}


Expand Down Expand Up @@ -413,26 +403,23 @@ public static byte[] decryptWithConcatKDF(final JWEHeader header,

epv = new Base64URL((String)header.getCustomParam("epv")).decode();
}

SecretKey cekAlt = LegacyConcatKDF.generateCEK(secretKey, header.getEncryptionMethod(), epu, epv);

final byte[] plainText = AESCBC.decrypt(cekAlt, iv.decode(), cipherText.decode(), ceProvider);


SecretKey cik = LegacyConcatKDF.generateCIK(secretKey, header.getEncryptionMethod(), epu, epv);

String macInput = header.toBase64URL().toString() + "." +
encryptedKey.toString() + "." +
iv.toString() + "." +
cipherText.toString();

byte[] mac = HMAC.compute(cik, macInput.getBytes(), macProvider);

if (! ConstantTimeUtils.areEqual(authTag.decode(), mac)) {

throw new JOSEException("HMAC integrity check failed");
throw new JOSEException("MAC check failed");
}

return plainText;
SecretKey cekAlt = LegacyConcatKDF.generateCEK(secretKey, header.getEncryptionMethod(), epu, epv);

return AESCBC.decrypt(cekAlt, iv.decode(), cipherText.decode(), ceProvider);
}


Expand Down
68 changes: 67 additions & 1 deletion src/test/java/com/nimbusds/jose/crypto/AESCBCTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;

import com.nimbusds.jose.EncryptionMethod;
import com.nimbusds.jose.JOSEException;
import com.nimbusds.jose.JWEAlgorithm;
import com.nimbusds.jose.JWEHeader;
import com.nimbusds.jose.util.Base64URL;
import junit.framework.TestCase;

import org.junit.Assert;
Expand All @@ -31,7 +36,7 @@
* vectors from draft-ietf-jose-json-web-algorithms-10, appendix C.
*
* @author Vladimir Dzhuvinov
* @version 2015-05-17
* @version 2017-05-30
*/
public class AESCBCTest extends TestCase {

Expand Down Expand Up @@ -177,4 +182,65 @@ public void testAuthenticatedEncryption512()
Assert.assertArrayEquals("Cipher text", CIPHER_TEXT_512, act.getCipherText());
Assert.assertArrayEquals("Auth tag", AUTH_TAG_512, act.getAuthenticationTag());
}


public void testCBCPaddingOracleAttack()
throws Exception {

SecretKey inputKey = new SecretKeySpec(INPUT_KEY_256, "AES");

Assert.assertArrayEquals("Input key", INPUT_KEY_256, inputKey.getEncoded());

AuthenticatedCipherText act = AESCBC.encryptAuthenticated(inputKey, IV, PLAIN_TEXT, AAD, null, null);

byte[] cipherText = act.getCipherText();

// Now change the cipher text to make CBC padding invalid.
cipherText[cipherText.length - 1] ^= 0x01;

try {
AESCBC.decryptAuthenticated(inputKey, IV, cipherText, AAD, act.getAuthenticationTag(), null, null);
} catch (JOSEException e) {
assertEquals("MAC check failed", e.getMessage());
}
}


public void testCBCPaddingOracleAttackOldConcatKDF()
throws Exception {

SecretKey inputKey = new SecretKeySpec(INPUT_KEY_256, "AES");

Assert.assertArrayEquals("Input key", INPUT_KEY_256, inputKey.getEncoded());

AuthenticatedCipherText act = AESCBC.encryptWithConcatKDF(
new JWEHeader(JWEAlgorithm.RSA1_5, EncryptionMethod.A128CBC_HS256_DEPRECATED),
new SecretKeySpec(INPUT_KEY_256, "AES"),
Base64URL.encode(INPUT_KEY_256), // mock
IV,
PLAIN_TEXT,
null,
null
);

byte[] cipherText = act.getCipherText();

// Now change the cipher text to make CBC padding invalid.
cipherText[cipherText.length - 1] ^= 0x01;

try {
AESCBC.decryptWithConcatKDF(
new JWEHeader(JWEAlgorithm.RSA1_5, EncryptionMethod.A128CBC_HS256_DEPRECATED),
new SecretKeySpec(INPUT_KEY_256, "AES"),
Base64URL.encode(INPUT_KEY_256), // mock
Base64URL.encode(IV),
Base64URL.encode(cipherText),
Base64URL.encode(act.getAuthenticationTag()),
null,
null
);
} catch (JOSEException e) {
assertEquals("MAC check failed", e.getMessage());
}
}
}

0 comments on commit 6a29f10

Please sign in to comment.