CVE-2013-5679

@@ -65,7 +65,9 @@ public final class CipherText implements Serializable {
     //       reflected in the class CipherTextSerializer.
     // This should be *same* version as in CipherTextSerializer and KeyDerivationFunction.
 	// If one changes, the other should as well to accommodate any differences.
-	public  static final int cipherTextVersion = 20110203; // Format: YYYYMMDD, max is 99991231.
+	//		Previous versions:	20110203 - Original version (ESAPI releases 2.0 & 2.0.1)
+	//						    20130830 - Fix to issue #306 (release 2.1.0)
+	public  static final int cipherTextVersion = 20130830; // Format: YYYYMMDD, max is 99991231.
 		// Required by Serializable classes.
 	private static final long serialVersionUID = cipherTextVersion; // Format: YYYYMMDD
 	
@@ -435,21 +437,23 @@ void storeSeparateMAC(byte[] macValue) {
 	 * @return True if the ciphertext has not be tampered with, and false otherwise.
 	 */
 	public boolean validateMAC(SecretKey authKey) {
-	    boolean usesMAC = ESAPI.securityConfiguration().useMACforCipherText();
+	    boolean requiresMAC = ESAPI.securityConfiguration().useMACforCipherText();
 
-	    if (  usesMAC && macComputed() ) {  // Uses MAC and it was computed
+	    if (  requiresMAC && macComputed() ) {  // Uses MAC and it was computed
 	        // Calculate MAC from HMAC-SHA1(nonce, IV + plaintext) and
 	        // compare to stored value (separate_mac_). If same, then return true,
 	        // else return false.
 	        byte[] mac = computeMAC(authKey);
 	        assert mac.length == separate_mac_.length : "MACs are of differnt lengths. Should both be the same.";
 	        return CryptoHelper.arrayCompare(mac, separate_mac_); // Safe compare!!!
-	    } else if ( ! usesMAC ) {           // Doesn't use MAC
+	    } else if ( ! requiresMAC ) {           // Doesn't require a MAC
 	        return true;
-	    } else {                            // Uses MAC but it has not been computed / stored.
-	        logger.warning(Logger.SECURITY_FAILURE, "Cannot validate MAC as it was never computed and stored. " +
-	        "Decryption result may be garbage even when decryption succeeds.");
-	        return true;    // Need to return 'true' here because of encrypt() / decrypt() methods don't support this.
+	    } else {
+	    		// This *used* to be the case (for versions 2.0 and 2.0.1) where we tried to
+	    		// accomodate the deprecated decrypt() method from ESAPI 1.4. Unfortunately,
+	    		// that was an EPIC FAIL. (See Google Issue # 306 for details.)
+	        logger.warning(Logger.SECURITY_FAILURE, "MAC may have been tampered with (e.g., length set to 0).");
+	        return false;    // Deprecated decrypt() method removed, so now return false.
 	    }
 	}
 	
@@ -474,8 +478,8 @@ public byte[] asPortableSerializedByteArray() throws EncryptionException {
 	    
 	    // If we are supposed to be using a (separate) MAC, also make sure
 	    // that it has been computed/stored.
-	    boolean usesMAC = ESAPI.securityConfiguration().useMACforCipherText();
-	    if (  usesMAC && ! macComputed() ) {
+	    boolean requiresMAC = ESAPI.securityConfiguration().useMACforCipherText();
+	    if (  requiresMAC && ! macComputed() ) {
 	        String msg = "Programming error: MAC is required for this cipher mode (" +
 	                     getCipherMode() + "), but MAC has not yet been " +
 	                     "computed and stored. Call the method " +

@@ -32,7 +32,9 @@
 public class CipherTextSerializer {
     // This should be *same* version as in CipherText & KeyDerivationFunction.
 	// If one changes, the other should as well to accommodate any differences.
-	public  static final  int cipherTextSerializerVersion = 20110203; // Format: YYYYMMDD, max is 99991231.
+	//		Previous versions:	20110203 - Original version (ESAPI releases 2.0 & 2.0.1)
+	//						    20130830 - Fix to issue #306 (release 2.1.0)
+	public  static final  int cipherTextSerializerVersion = 20130830; // Format: YYYYMMDD, max is 99991231.
     private static final long serialVersionUID = cipherTextSerializerVersion;
 
     private static final Logger logger = ESAPI.getLogger("CipherTextSerializer");
@@ -234,7 +236,7 @@ private CipherText convertToCipherText(byte[] cipherTextSerializedBytes)
             debug("kdfPrf: " + kdfPrf);
             assert kdfPrf >= 0 && kdfPrf <= 15 : "kdfPrf == " + kdfPrf + " must be between 0 and 15.";
             int kdfVers = ( kdfInfo & 0x07ffffff);
-            assert kdfVers > 0 && kdfVers <= 99991231 : "KDF Version (" + kdfVers + ") out of range."; // Really should be >= 20110203 (earliest).
+            assert kdfVers >= 20110203 && kdfVers <= 99991231 : "KDF Version (" + kdfVers + ") out of range."; // 20110203 (orig version).
             debug("convertToCipherText: kdfPrf = " + kdfPrf + ", kdfVers = " + kdfVers);
             if ( kdfVers != CipherText.cipherTextVersion ) {
                 // NOTE: In future, support backward compatibility via this mechanism. When we do this

@@ -49,12 +49,15 @@ public class KeyDerivationFunction {
 	 * {@link CipherTextSerializer#cipherTextSerializerVersion} to make sure
 	 * that these classes are all kept in-sync in order to support backward
 	 * compatibility of previously encrypted data.
-	 * 
+	 * <pre>
+	 * Previous versions:	20110203 - Original version (ESAPI releases 2.0 & 2.0.1)
+	 *					    20130830 - Fix to issue #306 (release 2.1.0)
+	 * </pre>
 	 * @see CipherTextSerializer#asSerializedByteArray()
 	 * @see CipherText#asPortableSerializedByteArray()
 	 * @see CipherText#fromPortableSerializedBytes(byte[])
 	 */
-	public  static final int  kdfVersion       = 20110203;   // Format: YYYYMMDD, max is 99991231.
+	public  static final int  kdfVersion       = 20130830;   // Format: YYYYMMDD, max is 99991231.
 	private static final long serialVersionUID = kdfVersion; // Format: YYYYMMDD
 	
     // Pseudo-random function algorithms suitable for NIST KDF in counter mode.

@@ -105,75 +105,12 @@ public interface Encryptor {
 	 */

 	String hash(String plaintext, String salt, int iterations) throws EncryptionException;

 	

-	/**

-	 * Encrypts the provided plaintext and returns a ciphertext string using the

-	 * master secret key and default cipher transformation.

-	 * </p><p>

-     * <b>Compatibility with earlier ESAPI versions:</b> The symmetric encryption

-     * in ESAPI 2.0 and later is not compatible with the encryption in ESAPI 1.4

-     * or earlier. Not only are the interfaces slightly different, but they format

-     * of the serialized encrypted data is incompatible. Therefore, if you have

-     * encrypted data with ESAPI 1.4 or earlier, you must first encrypt it and

-     * then re-encrypt it with ESAPI 2.0. Backward compatibility with ESAPI 1.4

-     * was proposed to both the ESAPI Developers and ESAPI Users mailing lists

-     * and voted down. More details are available in the ESAPI document

-     * <a href="http://owasp-esapi-java.googlecode.com/svn/trunk/documentation/esapi4java-core-2.0-readme-crypto-changes.html">

-     * Why Is OWASP Changing ESAPI Encryption?</a>

-	 * </p><p>

-	 * <b>Why this method is deprecated:</b> Most cryptographers strongly suggest

-	 * that if you are creating crypto functionality for general-purpose use,

-	 * at a minimum you should ensure that it provides authenticity, integrity,

-	 * and confidentiality. This method only provides confidentiality, but not

-	 * authenticity or integrity. Therefore, you are encouraged to use

-	 * one of the other encryption methods referenced below. Because this

-	 * method provides neither authenticity nor integrity, it may be

-	 * removed in some future ESAPI Java release. Note: there are some cases

-	 * where authenticity / integrity are not that important. For instance, consider

-	 * a case where the encrypted data is never out of your application's control. For

-	 * example, if you receive data that your application is encrypting itself and then

-	 * storing the encrypted data in its own database for later use (and no other

-	 * applications can query or update that column of the database), providing

-	 * confidentiality alone might be sufficient. However, if there are cases

-	 * where your application will be sending or receiving already encrypted data

-	 * over an insecure, unauthenticated channel, in such cases authenticity and

-	 * integrity of the encrypted data likely is important and this method should

-	 * be avoided in favor of one of the other two.

-	 * 

-	 * @param plaintext

-	 *      the plaintext {@code String} to encrypt. Note that if you are encrypting

-	 *      general bytes, you should encypt that byte array to a String using

-	 *      "UTF-8" encoding.

-	 * 

-	 * @return 

-	 * 		the encrypted, base64-encoded String representation of 'plaintext' plus

-	 * 		the random IV used.

-	 * 

-	 * @throws EncryptionException

-	 *      if the specified encryption algorithm could not be found or another problem exists with 

-	 *      the encryption of 'plaintext'

-	 * 

-	 * @see #encrypt(PlainText)

-	 * @see #encrypt(SecretKey, PlainText)

-	 * 

-	 * @deprecated As of 1.4.2; use {@link #encrypt(PlainText)} instead, which

-	 *			   also ensures message authenticity. This method will be

-	 *             completely removed as of the next major release or point

-	 *             release (3.0 or 2.1, whichever comes first) as per OWASP

-	 *             deprecation policy.

-	 */

-	@Deprecated String encrypt(String plaintext) throws EncryptionException;

-

 	/**

 	 * Encrypts the provided plaintext bytes using the cipher transformation

 	 * specified by the property <code>Encryptor.CipherTransformation</code>

 	 * and the <i>master encryption key</i> as specified by the property

 	 * {@code Encryptor.MasterKey} as defined in the <code>ESAPI.properties</code> file.

-	 * </p><p>

-	 * This method is preferred over {@link #encrypt(String)} because it also

-	 * allows encrypting of general byte streams rather than simply strings and

-	 * also because it returns a {@code CipherText} object and thus supports

-	 * cipher modes that require an Initialization Vector (IV), such as

-	 * Cipher Block Chaining (CBC).

+	 * </p>

 	 * 

 	 * @param plaintext	The {@code PlainText} to be encrypted.

 	 * @return the {@code CipherText} object from which the raw ciphertext, the

@@ -217,68 +154,11 @@ public interface Encryptor {
 	 CipherText encrypt(SecretKey key, PlainText plaintext)

 	 		throws EncryptionException;

 

-	/**

-	 * Decrypts the provided ciphertext and returns a plaintext string using the

-	 * master secret key and default cipher transformation.

-	 * </p><p>

-	 * <b>Compatibility with earlier ESAPI versions:</b> The symmetric encryption

-	 * in ESAPI 2.0 and later is not compatible with the encryption in ESAPI 1.4

-	 * or earlier. Not only are the interfaces slightly different, but they format

-	 * of the serialized encrypted data is incompatible. Therefore, if you have

-	 * encrypted data with ESAPI 1.4 or earlier, you must first encrypt it and

-	 * then re-encrypt it with ESAPI 2.0. Backward compatibility with ESAPI 1.4

-	 * was proposed to both the ESAPI Developers and ESAPI Users mailing lists

-	 * and voted down. More details are available in the ESAPI document

-	 * <a href="http://owasp-esapi-java.googlecode.com/svn/trunk/documentation/esapi4java-core-2.0-readme-crypto-changes.html">

-	 * Why Is OWASP Changing ESAPI Encryption?</a>

-	 * </p><p>

-	 * <b>Why this method is deprecated:</b> Most cryptographers strongly suggest

-	 * that if you are creating crypto functionality for general-purpose use,

-	 * at a minimum you should ensure that it provides authenticity, integrity,

-	 * and confidentiality. This method only provides confidentiality, but not

-	 * authenticity or integrity. Therefore, you are encouraged to use

-	 * one of the other encryption methods referenced below. Because this

-	 * method provides neither authenticity nor integrity, it may be

-	 * removed in some future ESAPI Java release. Note: there are some cases

-	 * where authenticity / integrity are not that important. For instance, consider

-	 * a case where the encrypted data is never out of your application's control. For

-	 * example, if you receive data that your application is encrypting itself and then

-	 * storing the encrypted data in its own database for later use (and no other

-	 * applications can query or update that column of the database), providing

-	 * confidentiality alone might be sufficient. However, if there are cases

-	 * where your application will be sending or receiving already encrypted data

-	 * over an insecure, unauthenticated channel, in such cases authenticity and

-	 * integrity of the encrypted data likely is important and this method should

-	 * be avoided in favor of one of the other two.

-	 *

-	 * @param ciphertext

-	 *      the ciphertext (the encrypted plaintext) that resulted from

-	 *      encrypting using the method {@link #encrypt(String)}.

-	 * 

-	 * @return 

-	 * 		the decrypted ciphertext (i.e., the corresponding plaintext).

-	 * 

-	 * @throws EncryptionException

-	 *      if the specified encryption algorithm could not be found or another problem exists with 

-	 *      the decryption of 'plaintext'

-	 *

-	 * @deprecated As of 1.4.2; use {@link #decrypt(CipherText)} instead, which

-     *             also ensures message authenticity. This method will be

-     *             completely removed as of the next major release or point

-     *             release (3.0 or 2.1, whichever comes first) as per OWASP

-     *             deprecation policy.

-	 */

-	 @Deprecated String decrypt(String ciphertext) throws EncryptionException;

-

 	/**

 	 * Decrypts the provided {@link CipherText} using the information from it

 	 * and the <i>master encryption key</i> as specified by the property

 	 * {@code Encryptor.MasterKey} as defined in the {@code ESAPI.properties}

 	 * file.

-	 * </p><p>

-	 * This decrypt method is to be preferred over the deprecated

-	 * {@link #decrypt(String)} method because this method can handle plaintext

-	 * bytes that were encrypted with cipher modes requiring IVs, such as CBC.

 	 * </p>

 	 * @param ciphertext The {@code CipherText} object to be decrypted.

 	 * @return The {@code PlainText} object resulting from decrypting the specified

@@ -329,28 +329,6 @@ public String hash(String plaintext, String salt, int iterations) throws Encrypt
 			throw new EncryptionException("Internal error", "Can't find encoding for " + encoding, ex);

 		}

 	}

-	

-	/**

-	 * Convenience method that encrypts plaintext strings the new way (default

-	 * is CBC mode and PKCS5 padding). This encryption method uses the master

-	 * encryption key specified by the {@code Encryptor.MasterKey} property

-	 * in {@code ESAPI.properties}.

-	 * 

-	 * @param plaintext	A String to be encrypted

-	 * @return	A base64-encoded combination of IV + raw ciphertext

-	 * @throws EncryptionException	Thrown when something goes wrong with the

-	 * 								encryption.

-	 * 

-	 * @see org.owasp.esapi.Encryptor#encrypt(PlainText)

-	 */

-	@Deprecated public String encrypt(String plaintext) throws EncryptionException

-	{

-        logWarning("encrypt", "Calling deprecated encrypt() method.");

-		CipherText ct = null;

-		ct = encrypt(new PlainText(plaintext) );

-		return ct.getEncodedIVCipherText();

-	}

-

 

 	/**

 	* {@inheritDoc}

@@ -366,10 +344,15 @@ public CipherText encrypt(PlainText plaintext) throws EncryptionException {
 	 public CipherText encrypt(SecretKey key, PlainText plain)

 	 			throws EncryptionException

 	 {

+		 if ( key == null ) {

+			 throw new IllegalArgumentException("(Master) encryption key arg may not be null. Is Encryptor.MasterKey set?");

+		 }

+		 if ( plain == null ) {

+			 throw new IllegalArgumentException("PlainText may arg not be null");

+		 }

 		 byte[] plaintext = plain.asBytes();

 		 boolean overwritePlaintext = ESAPI.securityConfiguration().overwritePlainText();

-		 assert key != null : "(Master) encryption key may not be null";

-		 

+

 		 boolean success = false;	// Used in 'finally' clause.

 		 String xform = null;

 		 int keySize = key.getEncoded().length * 8;	// Convert to # bits

@@ -569,53 +552,6 @@ public CipherText encrypt(SecretKey key, PlainText plain)
 	}

 	 }

 

-	/**

-	  * Convenience method that decrypts previously encrypted plaintext strings

-	  * that were encrypted using the new encryption mechanism (with CBC mode and

-	  * PKCS5 padding by default).  This decryption method uses the master

-	  * encryption key specified by the {@code Encryptor.MasterKey} property

-	  * in {@code ESAPI.properties}.

-	  * 

-	  * @param b64IVCiphertext	A base64-encoded representation of the

-	  * 							IV + raw ciphertext string to be decrypted with

-	  * 							the default master key.

-	  * @return	The plaintext string prior to encryption.

-	  * @throws EncryptionException When something fails with the decryption.

-	  * 

-	  * @see org.owasp.esapi.Encryptor#decrypt(CipherText)

-	  */

-	 @Deprecated public String decrypt(String b64IVCiphertext) throws EncryptionException

-	 {

-	     logWarning("decrypt", "Calling deprecated decrypt() method.");

-		 CipherText ct = null;

-		 try {

-			 // We assume that the default cipher transform was used to encrypt this.

-			 ct = new CipherText();

-

-			 // Need to base64 decode the IV+ciphertext and extract the IV to set it in CipherText object.

-			 byte[] ivPlusRawCipherText = ESAPI.encoder().decodeFromBase64(b64IVCiphertext);

-			 int blockSize = ct.getBlockSize();	// Size in bytes.

-			 byte[] iv = new byte[ blockSize ];

-			 CryptoHelper.copyByteArray(ivPlusRawCipherText, iv, blockSize);	// Copy the first blockSize bytes into iv array

-			 int cipherTextSize = ivPlusRawCipherText.length - blockSize;

-			 byte[] rawCipherText = new byte[ cipherTextSize ];

-			 System.arraycopy(ivPlusRawCipherText, blockSize, rawCipherText, 0, cipherTextSize);

-			 ct.setIVandCiphertext(iv, rawCipherText);

-

-			 // Now the CipherText object should be prepared to use it to decrypt.

-			 PlainText plaintext = decrypt(ct);

-			 return plaintext.toString();	// Convert back to a Java String

-		 } catch (UnsupportedEncodingException e) {

-			 // Should never happen; UTF-8 should be in rt.jar.

-			 logger.error(Logger.SECURITY_FAILURE, "UTF-8 encoding not available! Decryption failed.", e);

-			 return null;	// CHECKME: Or re-throw or what? Could also use native encoding, but that's

-			 // likely to cause unexpected and undesired effects far downstream.

-		 } catch (IOException e) {

-			 logger.error(Logger.SECURITY_FAILURE, "Base64 decoding of IV+ciphertext failed. Decryption failed.", e);

-			 return null;

-		 }

-	 }

-

 	/**

 	* {@inheritDoc}

 	*/