CVE-2017-12973: Improper Validation of Integrity Check Value (CWE-354)
value: Nimbus JOSE+JWT before 4.39 proceeds improperly after detection of an invalid HMAC in authenticated AES-CBC decryption, which allows attackers to conduct a padding oracle attack.
CWE description: The product does not validate or incorrectly validates the integrity check values or "checksums" of a message. This may prevent it from detecting if the data has been modified or corrupted in transmission.
CVSS v3 base score: 3.1
Published: 2017-08-20T16:29Z
Repository: https://bitbucket.org/connect2id/nimbus-jose-jwt
Language: Java
Files changed: src/main/java/com/nimbusds/jose/crypto/AESCBC.java, src/test/java/com/nimbusds/jose/crypto/AESCBCTest.java
Commit date: 2017-06-01 10:40:56 +0300
Commit message: Fixes HMAC return on authenticated AESCBC decrypt (iss #223)

Diff: diff --git a/src/main/java/com/nimbusds/jose/crypto/AESCBC.java b/src/main/java/com/nimbusds/jose/crypto/AESCBC.java
index 98c5b8c..5962695 100644
--- a/src/main/java/com/nimbusds/jose/crypto/AESCBC.java
+++ b/src/main/java/com/nimbusds/jose/crypto/AESCBC.java
@@ -44,11 +44,11 @@ import net.jcip.annotations.ThreadSafe;
  *
  * <p>See RFC 7518 (JWA), section 5.2.
  *
  * @author Vladimir Dzhuvinov
  * @author Axel Nennker
- * @version 2015-05-26
+ * @version 2017-05-30
  */
 @ThreadSafe
 class AESCBC {
 
 
@@ -349,25 +349,15 @@ class AESCBC {
 			array();
 		byte[] hmac = HMAC.compute(compositeKey.getMACKey(), hmacInput, macProvider);
 
 		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);
 	}
 
 
 	/**
 	 * Decrypts the specified cipher text using the deprecated concat KDF
@@ -411,30 +401,27 @@ class AESCBC {
 
 		if (header.getCustomParam("epv") instanceof String) {
 
 			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);
 	}
 
 
 	/**
 	 * Prevents public instantiation.
diff --git a/src/test/java/com/nimbusds/jose/crypto/AESCBCTest.java b/src/test/java/com/nimbusds/jose/crypto/AESCBCTest.java
index 088a3d5..004ff26 100644
--- a/src/test/java/com/nimbusds/jose/crypto/AESCBCTest.java
+++ b/src/test/java/com/nimbusds/jose/crypto/AESCBCTest.java
@@ -19,21 +19,26 @@ package com.nimbusds.jose.crypto;
 
 
 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;
 
 
 /**
  * Tests the authenticated AES/CBC encryption and decryption methods. Uses test 
  * 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 {
 
 
 	private static final byte[] INPUT_KEY_256 = 
@@ -175,6 +180,67 @@ public class AESCBCTest extends TestCase {
 		AuthenticatedCipherText act = AESCBC.encryptAuthenticated(inputKey, IV, PLAIN_TEXT, AAD, null, null);
 
 		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());
+		}
+	}
 }
\ No newline at end of file

Vulnerable code:  * @version 2015-05-26
		boolean macCheckPassed = true;

			// Thwart timing attacks by delaying exception until after decryption
			macCheckPassed = false;
		}

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

		if (! macCheckPassed) {

		return plainText;

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

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





			throw new JOSEException("HMAC integrity check failed");
		return plainText;
 * @version 2015-05-17
Fixed code:  * @version 2017-05-30
		return decrypt(compositeKey.getAESKey(), iv, cipherText, ceProvider);
		
		
		
		
			throw new JOSEException("MAC check failed");
		SecretKey cekAlt = LegacyConcatKDF.generateCEK(secretKey, header.getEncryptionMethod(), epu, epv);

		return AESCBC.decrypt(cekAlt, iv.decode(), cipherText.decode(), ceProvider);
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;
 * @version 2017-05-30
	
	
	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());
		}
	}