Bug 509604 - backward compatibility improvement
- Fixed a backward compatibility issue in core
- Added regression testing
Reviewed-by: Lukas Jungmann <lukas.jungmann@oracle.com>
Signed-off-by: David Minsky <david.minsky@oracle.com>
diff --git a/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/security/SecurableBackwardsCompatibilityTest.java b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/security/SecurableBackwardsCompatibilityTest.java
new file mode 100644
index 0000000..f29721c
--- /dev/null
+++ b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/security/SecurableBackwardsCompatibilityTest.java
@@ -0,0 +1,170 @@
+/*******************************************************************************
+ * Copyright (c) 1998, 2016 Oracle and/or its affiliates. All rights reserved.
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
+ * which accompanies this distribution.
+ * The Eclipse Public License is available at http://www.eclipse.org/legal/epl-v10.html
+ * and the Eclipse Distribution License is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * Contributors:
+ * dminsky - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.persistence.testing.tests.security;
+
+import java.io.ByteArrayOutputStream;
+import java.io.ObjectOutputStream;
+
+import javax.crypto.Cipher;
+import javax.crypto.CipherOutputStream;
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.DESKeySpec;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.eclipse.persistence.exceptions.ValidationException;
+import org.eclipse.persistence.internal.helper.Helper;
+import org.eclipse.persistence.internal.security.JCEEncryptor;
+import org.eclipse.persistence.internal.security.Securable;
+
+import org.eclipse.persistence.testing.framework.ReflectionHelper;
+import org.eclipse.persistence.testing.framework.TestCase;
+
+/**
+ * Regression test suite for the EclipseLink reference implementation of the Securable interface
+ * @author dminsky
+ */
+public class SecurableBackwardsCompatibilityTest extends TestCase {
+
+ public SecurableBackwardsCompatibilityTest(String testMethod) {
+ super();
+ setName(testMethod);
+ }
+
+ @Override
+ public void test() throws Throwable {
+ ReflectionHelper.invokeMethod(getName(), this, new Class[0], new Object[0]);
+ }
+
+ /**
+ * Test the decryption of a String encrypted with DES ECB.
+ * @throws Exception
+ */
+ public void testStringDecryption_DES_ECB() throws Exception {
+ String plainTextString = "welcome123_des_ecb";
+
+ String testString = encryptString_DES_ECB(plainTextString);
+ assertFalse("Strings should not match.", plainTextString.equals(testString));
+
+ Securable securable = new JCEEncryptor();
+ String decryptedString = securable.decryptPassword(testString);
+ assertEquals("Strings should match.", plainTextString, decryptedString);
+ }
+
+ /**
+ * Test the decryption of a String encrypted with AES CBC.
+ * @throws Exception
+ */
+ public void testStringDecryption_AES_CBC() throws Exception {
+ String plainTextString = "welcome123_aes_cbc";
+
+ Securable securable = new JCEEncryptor();
+ String testString = securable.encryptPassword(plainTextString);
+ assertFalse("Strings should not match.", plainTextString.equals(testString));
+
+ String decryptedString = securable.decryptPassword(testString);
+ assertEquals("Strings should match.", plainTextString, decryptedString);
+ }
+
+ /**
+ * Test the decryption of a String encrypted with AES ECB.
+ * @throws Exception
+ */
+ public void testStringDecryption_AES_ECB() throws Exception {
+ String plainTextString = "welcome123_aes_ecb";
+
+ String testString = encryptString_AES_ECB(plainTextString);
+ assertFalse("Strings should not match.", plainTextString.equals(testString));
+
+ Securable securable = new JCEEncryptor();
+ String decryptedString = securable.decryptPassword(testString);
+ assertEquals("Strings should match.", plainTextString, decryptedString);
+ }
+
+ /**
+ * Test the decryption/processing of a plaintext String.
+ * @throws Exception
+ */
+ public void testStringDecryption_PlainText() throws Exception {
+ String plainTextString = "welcome123_plaintext";
+
+ Securable securable = new JCEEncryptor();
+ String decryptedString = securable.decryptPassword(plainTextString);
+ assertEquals("Passwords should match.", plainTextString, decryptedString);
+ }
+
+ /**
+ * Test the decryption/processing of a null parameter.
+ * @throws Exception
+ */
+ public void testNullParameterDecryption() throws Exception {
+ Securable securable = new JCEEncryptor();
+ String returnValue = securable.decryptPassword(null);
+ assertNull("Null should be returned when decrypting a null value", returnValue);
+ }
+
+ /**
+ * Test the encryption of a null parameter.
+ * @throws Exception
+ */
+ public void testNullParameterEncryption() throws Exception {
+ ValidationException expectedException = null;
+ try {
+ Securable securable = new JCEEncryptor();
+ securable.encryptPassword(null);
+ } catch (ValidationException ve) {
+ expectedException = ve;
+ }
+ assertNotNull("A ValidationException should be thrown when encrypting a null value", expectedException);
+ }
+
+ /*
+ * Internal test utility:
+ * Return a DES ECB encrypted version of the String parameter, using the legacy encryption code.
+ */
+ private String encryptString_DES_ECB(String aString) throws Exception {
+ final byte[] bytes = Helper.buildBytesFromHexString("E60B80C7AEC78038");
+
+ Cipher cipher = Cipher.getInstance("DES/ECB/PKCS5Padding");
+ cipher.init(Cipher.ENCRYPT_MODE, SecretKeyFactory.getInstance("DES").generateSecret(new DESKeySpec(bytes)));
+
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ CipherOutputStream cos = new CipherOutputStream(baos, cipher);
+ ObjectOutputStream oos = new ObjectOutputStream(cos);
+ oos.writeObject(aString);
+ oos.flush();
+ oos.close();
+
+ return Helper.buildHexStringFromBytes(baos.toByteArray());
+ }
+
+ /*
+ * Internal test utility:
+ * Return an AES ECB encrypted version of the String parameter, using the legacy encryption code.
+ */
+ private String encryptString_AES_ECB(String aString) throws Exception {
+ final byte[] bytes = Helper.buildBytesFromHexString("3E7CFEF156E712906E1F603B59463C67");
+
+ Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5Padding");
+ cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(bytes, "AES"));
+
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ CipherOutputStream cos = new CipherOutputStream(baos, cipher);
+ ObjectOutputStream oos = new ObjectOutputStream(cos);
+ oos.writeObject(aString);
+ oos.flush();
+ oos.close();
+
+ return Helper.buildHexStringFromBytes(baos.toByteArray());
+ }
+
+}
diff --git a/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/security/SecurityTestModel.java b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/security/SecurityTestModel.java
index ab83a15..232971a 100644
--- a/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/security/SecurityTestModel.java
+++ b/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/security/SecurityTestModel.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 1998, 2015 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2016 Oracle and/or its affiliates. All rights reserved.
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
* which accompanies this distribution.
@@ -44,6 +44,7 @@ public void addTests() {
addTest(getJCETestSuite());
addTest(getValidationSecurityTestSuite());
addTest(new DatabaseLoginWithNoEncryptorTest());
+ addTest(getSecurableBackwardsCompatibilityTestSuite());
}
public static TestSuite getJCETestSuite() {
@@ -76,6 +77,21 @@ public static TestSuite getValidationSecurityTestSuite() {
return suite;
}
+
+ public static TestSuite getSecurableBackwardsCompatibilityTestSuite() {
+ TestSuite suite = new TestSuite();
+ suite.setName("Securable Backward Compatibility Tests");
+ suite.setDescription("Tests the backwards compatibility of the (default) Securable reference implementation");
+
+ suite.addTest(new SecurableBackwardsCompatibilityTest("testStringDecryption_PlainText"));
+ suite.addTest(new SecurableBackwardsCompatibilityTest("testStringDecryption_DES_ECB"));
+ suite.addTest(new SecurableBackwardsCompatibilityTest("testStringDecryption_AES_CBC"));
+ suite.addTest(new SecurableBackwardsCompatibilityTest("testStringDecryption_AES_ECB"));
+ suite.addTest(new SecurableBackwardsCompatibilityTest("testNullParameterDecryption"));
+ suite.addTest(new SecurableBackwardsCompatibilityTest("testNullParameterEncryption"));
+
+ return suite;
+ }
/**
* Return the JUnit suite to allow JUnit runner to find it.
diff --git a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/security/JCEEncryptor.java b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/security/JCEEncryptor.java
index 9adc3c0..3c9e418 100644
--- a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/security/JCEEncryptor.java
+++ b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/security/JCEEncryptor.java
@@ -12,7 +12,12 @@
******************************************************************************/
package org.eclipse.persistence.internal.security;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+
import javax.crypto.Cipher;
+import javax.crypto.CipherInputStream;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.DESKeySpec;
@@ -24,20 +29,21 @@
import org.eclipse.persistence.internal.helper.Helper;
/**
- * TopLink reference implementation for password encryption.
+ * EclipseLink reference implementation for password encryption.
*
* @author Guy Pelletier
*/
public class JCEEncryptor implements Securable {
- // Legacy decrypt cipher used for backwards compatibility only.
- private static final String DES = "DES/ECB/PKCS5Padding";
- private final Cipher decryptCipherDES;
+
+ // Legacy DES ECB cipher used for backwards compatibility decryption only.
+ private static final String DES_ECB = "DES/ECB/PKCS5Padding";
+ private final Cipher decryptCipherDES_ECB;
- // Legacy AES cipher used for backwards compatibility only.
- private static final String AES = "AES/ECB/PKCS5Padding";
- private final Cipher decryptCipherAES;
+ // Legacy AES ECB cipher used for backwards compatibility decryption only.
+ private static final String AES_ECB = "AES/ECB/PKCS5Padding";
+ private final Cipher decryptCipherAES_ECB;
- // All encryption is done through the AES cipher.
+ // All encryption is done through the AES CBC cipher.
private static final String AES_CBC = "AES/CBC/PKCS5Padding";
private final Cipher encryptCipherAES_CBC;
private final Cipher decryptCipherAES_CBC;
@@ -54,11 +60,11 @@ public JCEEncryptor() throws Exception {
*
* Confusing??? Well, don't move this code before talking to Guy first!
*/
- decryptCipherDES = Cipher.getInstance(DES);
- decryptCipherDES.init(Cipher.DECRYPT_MODE, Synergizer.getDESMultitasker());
+ decryptCipherDES_ECB = Cipher.getInstance(DES_ECB);
+ decryptCipherDES_ECB.init(Cipher.DECRYPT_MODE, Synergizer.getDESMultitasker());
- decryptCipherAES = Cipher.getInstance(AES);
- decryptCipherAES.init(Cipher.DECRYPT_MODE, Synergizer.getAESMultitasker());
+ decryptCipherAES_ECB = Cipher.getInstance(AES_ECB);
+ decryptCipherAES_ECB.init(Cipher.DECRYPT_MODE, Synergizer.getAESMultitasker());
SecretKey sk = Synergizer.getAESCBCMultitasker();
IvParameterSpec iv = Synergizer.getIvSpec();
@@ -79,7 +85,6 @@ public synchronized String encryptPassword(String password) {
} catch (Exception e) {
throw ValidationException.errorEncryptingPassword(e);
}
-
}
/**
@@ -88,32 +93,50 @@ public synchronized String encryptPassword(String password) {
*/
@Override
public synchronized String decryptPassword(String encryptedPswd) {
- String password = null;
+ if (encryptedPswd == null) {
+ return null;
+ }
- if (encryptedPswd != null) {
- byte[] bytePassword = new byte[0];
+ String password = null;
+ byte[] bytePassword = new byte[0];
+
+ try {
+ bytePassword = Helper.buildBytesFromHexString(encryptedPswd);
+ // try AES/CBC first
+ password = new String(decryptCipherAES_CBC.doFinal(bytePassword), "UTF-8");
+ } catch (ConversionException ce) {
+ // buildBytesFromHexString failed, assume clear text
+ password = encryptedPswd;
+ } catch (Exception e) {
+ ObjectInputStream oisAes = null;
try {
- bytePassword = Helper.buildBytesFromHexString(encryptedPswd);
- password = new String(decryptCipherAES_CBC.doFinal(bytePassword), "UTF-8");
- } catch (ConversionException ce) {
- // Never prepared (buildBytesFromHexString failed), assume clear text
- password = encryptedPswd;
+ // try AES/ECB second
+ oisAes = new ObjectInputStream(new CipherInputStream(new ByteArrayInputStream(bytePassword), decryptCipherAES_ECB));
+ password = (String)oisAes.readObject();
} catch (Exception f) {
+ ObjectInputStream oisDes = null;
try {
- // try AES/ECB
- password = new String(decryptCipherAES.doFinal(bytePassword));
- } catch (Exception exc) {
- // Catch all exceptions when trying to decrypt using AES and try the
- // old DES decryptor before deciding what to do.
- try {
- password = new String(decryptCipherDES.doFinal(bytePassword));
- } catch (ArrayIndexOutOfBoundsException e) {
- // JCE 1.2.1 couldn't decrypt it, assume clear text
- password = encryptedPswd;
- } catch (Exception e) {
- throw ValidationException.errorDecryptingPassword(e);
+ // try DES/ECB third
+ oisDes = new ObjectInputStream(new CipherInputStream(new ByteArrayInputStream(bytePassword), decryptCipherDES_ECB));
+ password = (String)oisDes.readObject();
+ } catch (ArrayIndexOutOfBoundsException g) {
+ // JCE 1.2.1 couldn't decrypt it, assume clear text
+ password = encryptedPswd;
+ } catch (Exception h) {
+ throw ValidationException.errorDecryptingPassword(h);
+ } finally {
+ if (oisDes != null) {
+ try {
+ oisDes.close();
+ } catch (IOException e2) {}
}
}
+ } finally {
+ if (oisAes != null) {
+ try {
+ oisAes.close();
+ } catch (IOException e1) {}
+ }
}
}
