From 88ccf54275888072aa8e95686d3c3caa3848bc58 Mon Sep 17 00:00:00 2001 From: jbogdahn Date: Mon, 12 Nov 2018 13:15:31 +0100 Subject: Bug 388055 - Secure Storage uses PBE with MD5/DES as default algorithms + added IV parameter to CryptoData class, needed by specific Ciphers + added test to test detection of PBE Ciphers. Test is checking if roundtrips work for the PBE ciphers provided by JVM Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=388055 Change-Id: Ic6b564b0217f4ab83d4d4e6cdc7d402a294bb8ba Signed-off-by: jbogdahn --- .../META-INF/MANIFEST.MF | 2 +- bundles/org.eclipse.equinox.security.tests/pom.xml | 2 +- .../tests/storage/DetectPBECiphersTest.java | 67 ++++++++++++++++++++++ .../equinox/security/tests/AllSecurityTests.java | 6 +- .../META-INF/MANIFEST.MF | 2 +- bundles/org.eclipse.equinox.security/pom.xml | 2 +- .../internal/security/storage/CryptoData.java | 30 ++++++++-- .../internal/security/storage/JavaEncryption.java | 22 +++++-- .../security/storage/SecurePreferences.java | 8 ++- 9 files changed, 123 insertions(+), 18 deletions(-) create mode 100644 bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/internal/security/tests/storage/DetectPBECiphersTest.java diff --git a/bundles/org.eclipse.equinox.security.tests/META-INF/MANIFEST.MF b/bundles/org.eclipse.equinox.security.tests/META-INF/MANIFEST.MF index 18b645c06..759bb863b 100644 --- a/bundles/org.eclipse.equinox.security.tests/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.equinox.security.tests/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: Equinox security tests Bundle-SymbolicName: org.eclipse.equinox.security.tests;singleton:=true -Bundle-Version: 1.1.0.qualifier +Bundle-Version: 1.1.100.qualifier Bundle-Activator: org.eclipse.equinox.internal.security.tests.SecurityTestsActivator Bundle-RequiredExecutionEnvironment: JavaSE-1.8 Bundle-Vendor: Eclipse.org diff --git a/bundles/org.eclipse.equinox.security.tests/pom.xml b/bundles/org.eclipse.equinox.security.tests/pom.xml index eb4f6e995..ee0caa6a7 100644 --- a/bundles/org.eclipse.equinox.security.tests/pom.xml +++ b/bundles/org.eclipse.equinox.security.tests/pom.xml @@ -19,7 +19,7 @@ org.eclipse.equinox org.eclipse.equinox.security.tests - 1.1.0-SNAPSHOT + 1.1.100-SNAPSHOT eclipse-test-plugin true diff --git a/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/internal/security/tests/storage/DetectPBECiphersTest.java b/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/internal/security/tests/storage/DetectPBECiphersTest.java new file mode 100644 index 000000000..a0a3bfa6e --- /dev/null +++ b/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/internal/security/tests/storage/DetectPBECiphersTest.java @@ -0,0 +1,67 @@ +/******************************************************************************* + * Copyright (c) 2018 Inno-Tec Innovative Technologies GmbH. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Inno-Tec (Juergen Bogdahn) - Fix for Bug 388055 + * + *******************************************************************************/ +package org.eclipse.equinox.internal.security.tests.storage; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.security.Provider; +import java.security.Security; +import java.util.*; +import org.eclipse.equinox.internal.security.storage.JavaEncryption; +import org.junit.Test; + +/** + * In this test the number of PBE ciphers in the VM are determined and compared + * to the available ciphers in the secure storage implementation. + * + * This test is only interested in PBE Ciphers for the secure storage. + * + * !IMPORTANT! It can only pass if crypto.policy is set to 'unlimited' !IMPORTANT! + * + * The test "rebuilds" the logic of searching for keyFactories and matching PBE ciphers. + * For these ciphers the "roundtrip" has to be successful, otherwise the SecureStorage + * implementation does not use the cipher correctly and it cannot be used. + */ +public class DetectPBECiphersTest { + + @SuppressWarnings("rawtypes") + @Test + public void testPBEDetect() { + int cipherJVMCount = 0; + Set keyFactories = new HashSet<>(); + Provider[] providers = Security.getProviders(); + for (Provider p : providers) { //find all key factories + for (Provider.Service service : p.getServices()) { + if (service.getType().equals("SecretKeyFactory") && service.getAlgorithm().indexOf(' ') == -1) {// skips properties like "[Cipher.ABC SupportedPaddings]") + keyFactories.add(service.getAlgorithm()); + } + } + } + for (Provider p : providers) { //find all ciphers matching a key factory and start with PBE + for (Provider.Service service : p.getServices()) { + if (service.getType().equals("Cipher") && service.getAlgorithm().startsWith("PBE") && keyFactories.contains(service.getAlgorithm())) { + cipherJVMCount++; + } + } + } + + JavaEncryption encryption = new JavaEncryption(); + HashMap detectedCiphers = encryption.detect(); + + assertTrue(detectedCiphers.size() > 0); + assertEquals(cipherJVMCount, detectedCiphers.size()); + } +} diff --git a/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/security/tests/AllSecurityTests.java b/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/security/tests/AllSecurityTests.java index 2cf6233ee..28f4da04d 100644 --- a/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/security/tests/AllSecurityTests.java +++ b/bundles/org.eclipse.equinox.security.tests/src/org/eclipse/equinox/security/tests/AllSecurityTests.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008 IBM Corporation and others. + * Copyright (c) 2008, 2018 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -10,6 +10,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Inno-Tec Innovative Technologies GmbH - Fix for Bug 388055 + * *******************************************************************************/ package org.eclipse.equinox.security.tests; @@ -23,7 +25,7 @@ import org.junit.runners.Suite.SuiteClasses; * As such this tests should be run in a headless mode. */ @RunWith(Suite.class) -@SuiteClasses({Base64Test.class, SlashEncodeTest.class, DefaultPreferencesTest.class, DynamicPreferencesTest.class, WinPreferencesTest.class}) +@SuiteClasses({Base64Test.class, DetectPBECiphersTest.class, SlashEncodeTest.class, DefaultPreferencesTest.class, DynamicPreferencesTest.class, WinPreferencesTest.class}) public class AllSecurityTests { //see @SuiteClasses } diff --git a/bundles/org.eclipse.equinox.security/META-INF/MANIFEST.MF b/bundles/org.eclipse.equinox.security/META-INF/MANIFEST.MF index fff17024a..4ca6438c1 100644 --- a/bundles/org.eclipse.equinox.security/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.equinox.security/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.equinox.security;singleton:=true -Bundle-Version: 1.3.0.qualifier +Bundle-Version: 1.3.100.qualifier Bundle-Vendor: %providerName Bundle-Localization: plugin Bundle-Activator: org.eclipse.equinox.internal.security.auth.AuthPlugin diff --git a/bundles/org.eclipse.equinox.security/pom.xml b/bundles/org.eclipse.equinox.security/pom.xml index 39551f1c3..efdd45563 100644 --- a/bundles/org.eclipse.equinox.security/pom.xml +++ b/bundles/org.eclipse.equinox.security/pom.xml @@ -19,7 +19,7 @@ org.eclipse.equinox org.eclipse.equinox.security - 1.3.0-SNAPSHOT + 1.3.100-SNAPSHOT eclipse-plugin diff --git a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/CryptoData.java b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/CryptoData.java index b8a87bc24..c7e98a6d3 100644 --- a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/CryptoData.java +++ b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/CryptoData.java @@ -7,9 +7,11 @@ * https://www.eclipse.org/legal/epl-2.0/ * * SPDX-License-Identifier: EPL-2.0 - * + * * Contributors: * IBM Corporation - initial API and implementation + * Inno-Tec Innovative Technologies GmbH - Fix for Bug 388055 + * *******************************************************************************/ package org.eclipse.equinox.internal.security.storage; @@ -24,15 +26,18 @@ public class CryptoData { * Separates salt from the data; this must not be a valid Base64 character. */ static private final char SALT_SEPARATOR = ','; + static private final char IV_SEPARATOR = ';'; final private String moduleID; final private byte[] salt; + final private byte[] iv; final private byte[] encryptedData; - public CryptoData(String moduleID, byte[] salt, byte[] data) { + public CryptoData(String moduleID, byte[] salt, byte[] data, byte[] iv) { this.moduleID = moduleID; this.salt = salt; this.encryptedData = data; + this.iv = iv; } public String getModuleID() { @@ -47,6 +52,10 @@ public class CryptoData { return encryptedData; } + public byte[] getIV() { + return iv; + } + public CryptoData(String data) throws StorageException { // separate moduleID int pos = data.indexOf(MODULE_ID_SEPARATOR); @@ -61,10 +70,18 @@ public class CryptoData { encrypted = data.substring(pos + 1); } + // separate IV + int ivPos = encrypted.indexOf(IV_SEPARATOR); + if (ivPos != -1) { + iv = Base64.decode(encrypted.substring(0, ivPos)); + } else { // this data does not provide an IV + iv = null; + } + // separate salt and data int saltPos = encrypted.indexOf(SALT_SEPARATOR); if (saltPos != -1) { - salt = Base64.decode(encrypted.substring(0, saltPos)); + salt = Base64.decode(encrypted.substring(ivPos + 1, saltPos)); encryptedData = Base64.decode(encrypted.substring(saltPos + 1)); } else { // this is a "null" value if (encrypted.length() != 0) // double check that this is not a broken entry @@ -78,8 +95,13 @@ public class CryptoData { public String toString() { StringBuffer encryptedText = (moduleID == null) ? new StringBuffer() : new StringBuffer(moduleID); encryptedText.append(MODULE_ID_SEPARATOR); - if (salt != null) + if (iv != null) { + encryptedText.append(Base64.encode(iv)); + } + if (salt != null) { + encryptedText.append(IV_SEPARATOR); encryptedText.append(Base64.encode(salt)); + } if (encryptedData != null) { encryptedText.append(SALT_SEPARATOR); encryptedText.append(Base64.encode(encryptedData)); diff --git a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/JavaEncryption.java b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/JavaEncryption.java index d45a59dab..39c88138e 100644 --- a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/JavaEncryption.java +++ b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/JavaEncryption.java @@ -7,9 +7,11 @@ * https://www.eclipse.org/legal/epl-2.0/ * * SPDX-License-Identifier: EPL-2.0 - * + * * Contributors: * IBM Corporation - initial API and implementation + * Inno-Tec Innovative Technologies GmbH - Fix for Bug 388055 + * *******************************************************************************/ package org.eclipse.equinox.internal.security.storage; @@ -18,8 +20,7 @@ import java.security.spec.InvalidKeySpecException; import java.util.*; import java.util.Map.Entry; import javax.crypto.*; -import javax.crypto.spec.PBEKeySpec; -import javax.crypto.spec.PBEParameterSpec; +import javax.crypto.spec.*; import org.eclipse.core.runtime.jobs.ILock; import org.eclipse.core.runtime.jobs.Job; import org.eclipse.core.runtime.preferences.ConfigurationScope; @@ -144,9 +145,10 @@ public class JavaEncryption { Cipher c = Cipher.getInstance(cipherAlgorithm); c.init(Cipher.ENCRYPT_MODE, key, entropy); + byte[] iv = c.getIV(); byte[] result = c.doFinal(clearText); - return new CryptoData(passwordExt.getModuleID(), salt, result); + return new CryptoData(passwordExt.getModuleID(), salt, result, iv); } catch (InvalidKeyException e) { handle(e, StorageException.ENCRYPTION_ERROR); return null; @@ -181,7 +183,17 @@ public class JavaEncryption { SecretKeyFactory keyFactory = SecretKeyFactory.getInstance(keyFactoryAlgorithm); SecretKey key = keyFactory.generateSecret(passwordExt.getPassword()); - PBEParameterSpec entropy = new PBEParameterSpec(encryptedData.getSalt(), SALT_ITERATIONS); + IvParameterSpec ivParamSpec = null; + if (encryptedData.getIV() != null) { + ivParamSpec = new IvParameterSpec(encryptedData.getIV()); + } + + PBEParameterSpec entropy = null; + if (ivParamSpec != null) { + entropy = new PBEParameterSpec(encryptedData.getSalt(), SALT_ITERATIONS, ivParamSpec); + } else { + entropy = new PBEParameterSpec(encryptedData.getSalt(), SALT_ITERATIONS); + } Cipher c = Cipher.getInstance(cipherAlgorithm); c.init(Cipher.DECRYPT_MODE, key, entropy); diff --git a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferences.java b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferences.java index f31f259a5..d9b7f273c 100644 --- a/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferences.java +++ b/bundles/org.eclipse.equinox.security/src/org/eclipse/equinox/internal/security/storage/SecurePreferences.java @@ -7,9 +7,11 @@ * https://www.eclipse.org/legal/epl-2.0/ * * SPDX-License-Identifier: EPL-2.0 - * + * * Contributors: * IBM Corporation - initial API and implementation + * Inno-Tec Innovative Technologies GmbH - Fix for Bug 388055 + * *******************************************************************************/ package org.eclipse.equinox.internal.security.storage; @@ -218,7 +220,7 @@ public class SecurePreferences { checkRemoved(); if (!encrypt || value == null) { - CryptoData clearValue = new CryptoData(null, null, StorageUtils.getBytes(value)); + CryptoData clearValue = new CryptoData(null, null, StorageUtils.getBytes(value), null); internalPut(key, clearValue.toString()); // uses Base64 to encode byte sequences markModified(); return; @@ -228,7 +230,7 @@ public class SecurePreferences { if (passwordExt == null) { boolean storeDecrypted = !CallbacksProvider.getDefault().runningUI() || InternalExchangeUtils.isJUnitApp(); if (storeDecrypted) { // for JUnits and headless runs we store value as clear text and log a error - CryptoData clearValue = new CryptoData(null, null, StorageUtils.getBytes(value)); + CryptoData clearValue = new CryptoData(null, null, StorageUtils.getBytes(value), null); internalPut(key, clearValue.toString()); markModified(); // Make this as visible as possible. Both print out the output and log a error -- cgit v1.2.3