Skip to content

Commit e207720

Browse files
wmathurinwmathurin
authored
Merge pull request #2819 from wmathurin/dev
Never recreating the key pair used to encrypt symmetric encryption keys
2 parents e958716 + 47f5b94 commit e207720

2 files changed

Lines changed: 129 additions & 14 deletions

File tree

‎libs/SalesforceSDK/src/com/salesforce/androidsdk/security/SalesforceKeyGenerator.java‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ public class SalesforceKeyGenerator {
5959
private static final String SHARED_PREF_FILE = "identifier.xml";
6060
private static final String ENCRYPTED_ID_SHARED_PREF_KEY = "encrypted_%s";
6161
private static final String ID_PREFIX = "id_";
62-
private static final String KEYSTORE_ALIAS = "com.salesforce.androidsdk.security.KEYPAIR";
62+
protected static final String LEGACY_KEYPAIR_ALIAS = "com.salesforce.androidsdk.security.KEYPAIR";
63+
protected static final String MSDK_KEYPAIR_ALIAS = "com.salesforce.androidsdk.security.MSDK_KEYPAIR";
6364
private static final String SHA256 = "SHA-256";
6465
private static final String AES = "AES";
6566

@@ -160,10 +161,17 @@ private synchronized static String generateUniqueIdIfNoneStored(String name, int
160161
// Checks if we have a unique identifier stored.
161162
final String encryptedUniqueId = readFromSharedPrefs(ID_PREFIX + name);
162163
if (encryptedUniqueId != null) {
163-
final PrivateKey privateKey = KeyStoreWrapper.getInstance().getRSAPrivateKey(KEYSTORE_ALIAS);
164-
uniqueId = Encryptor.decryptWithRSA(privateKey, encryptedUniqueId, Encryptor.CipherMode.RSA_OAEP_SHA256);
164+
final PrivateKey msdkPrivateKey = KeyStoreWrapper.getInstance().getRSAPrivateKey(MSDK_KEYPAIR_ALIAS);
165+
uniqueId = Encryptor.decryptWithRSA(msdkPrivateKey, encryptedUniqueId, Encryptor.CipherMode.RSA_OAEP_SHA256);
166+
// Decryption failed - must have been encrypted with legacy key (LEGACY_KEYPAIR_ALIAS)
165167
if (uniqueId == null) {
166-
uniqueId = Encryptor.decryptWithRSA(privateKey, encryptedUniqueId, Encryptor.CipherMode.RSA_PKCS1);
168+
final PrivateKey privateKey = KeyStoreWrapper.getInstance().getRSAPrivateKey(LEGACY_KEYPAIR_ALIAS);
169+
uniqueId = Encryptor.decryptWithRSA(privateKey, encryptedUniqueId, Encryptor.CipherMode.RSA_OAEP_SHA256);
170+
// Decryption failed - must have been encrypted with legacy key with old cipher mode
171+
if (uniqueId == null) {
172+
uniqueId = Encryptor.decryptWithRSA(privateKey, encryptedUniqueId, Encryptor.CipherMode.RSA_PKCS1);
173+
}
174+
// We need to store it with thew new key (MSDK_KEYPAIR_ALIAS)
167175
storeUniqueId = true;
168176
}
169177
}
@@ -174,15 +182,9 @@ private synchronized static String generateUniqueIdIfNoneStored(String name, int
174182
storeUniqueId = true;
175183
}
176184

177-
// Encrypt and store unique id if it was just created, or if it had to be decrypted with old cipher mode
185+
// Encrypt and store unique id if it was just created, or if it had to be decrypted with old key
178186
if (storeUniqueId) {
179-
// Check if existing key supports OAEP padding, recreate if not
180-
if (!KeyStoreWrapper.getInstance().keySupportsOAEPPadding(KEYSTORE_ALIAS)) {
181-
SalesforceSDKLogger.i(TAG, "Key doesn't support OAEP padding, recreating key pair with OAEP support");
182-
KeyStoreWrapper.getInstance().deleteKey(KEYSTORE_ALIAS);
183-
}
184-
185-
final PublicKey publicKey = KeyStoreWrapper.getInstance().getRSAPublicKey(KEYSTORE_ALIAS);
187+
final PublicKey publicKey = KeyStoreWrapper.getInstance().getRSAPublicKey(MSDK_KEYPAIR_ALIAS);
186188
final String encryptedKey = Encryptor.encryptWithRSA(publicKey, uniqueId, Encryptor.CipherMode.RSA_OAEP_SHA256);
187189
storeInSharedPrefs(ID_PREFIX + name, encryptedKey);
188190
}
@@ -208,12 +210,12 @@ private static String createUniqueId(int length) {
208210
return uniqueId;
209211
}
210212

211-
private static String readFromSharedPrefs(String key) {
213+
protected static String readFromSharedPrefs(String key) {
212214
final SharedPreferences prefs = SalesforceSDKManager.getInstance().getAppContext().getSharedPreferences(SHARED_PREF_FILE, 0);
213215
return prefs.getString(getSharedPrefKey(key), null);
214216
}
215217

216-
private synchronized static void storeInSharedPrefs(String key, String value) {
218+
protected synchronized static void storeInSharedPrefs(String key, String value) {
217219
final SharedPreferences prefs = SalesforceSDKManager.getInstance().getAppContext().getSharedPreferences(SHARED_PREF_FILE, 0);
218220
prefs.edit().putString(getSharedPrefKey(key), value).apply();
219221
}

‎libs/test/SalesforceSDKTest/src/com/salesforce/androidsdk/security/SalesforceKeyGeneratorTest.java‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,11 @@
2828

2929
import android.app.Application;
3030
import android.app.Instrumentation;
31+
import android.content.SharedPreferences;
3132

3233
import com.salesforce.androidsdk.TestForceApp;
34+
import com.salesforce.androidsdk.analytics.security.Encryptor;
35+
import com.salesforce.androidsdk.app.SalesforceSDKManager;
3336

3437
import org.junit.Assert;
3538
import org.junit.Before;
@@ -40,6 +43,9 @@
4043
import androidx.test.filters.SmallTest;
4144
import androidx.test.platform.app.InstrumentationRegistry;
4245

46+
import java.security.PrivateKey;
47+
import java.security.PublicKey;
48+
4349
/**
4450
* Tests for {@link SalesforceKeyGenerator}.
4551
*
@@ -87,4 +93,111 @@ public void testGetEncryptionKey() {
8793
Assert.assertEquals("Encryption keys with the same name should be the same", id1Again, id1);
8894
Assert.assertNotSame("Encryption keys with different names should be different", id2, id1);
8995
}
96+
97+
@Test
98+
public void testGetUniqueIdStoredUsingLegacyKeyPairAndOldCipherMode() {
99+
encryptAndStoreInPrefs("test_name", "test_value", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1);
100+
Assert.assertEquals("test_value", decryptFromPrefs("test_name", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
101+
102+
// Now calling getUniqueId
103+
Assert.assertEquals("test_value", SalesforceKeyGenerator.getUniqueId("test_name"));
104+
105+
// The value should have been re-encrypted
106+
// - it should not be decryptable with the legacy key pair
107+
// - it should be decryptable with the msdk key pair
108+
Assert.assertNull(decryptFromPrefs("test_name", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
109+
Assert.assertEquals("test_value", decryptFromPrefs("test_name", SalesforceKeyGenerator.MSDK_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256));
110+
}
111+
112+
@Test
113+
public void testGetUniqueIdStoredUsingLegacyKeyPairAndNewCipherMode() {
114+
encryptAndStoreInPrefs("test_name", "test_value", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256);
115+
Assert.assertEquals("test_value", decryptFromPrefs("test_name", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256));
116+
117+
// Now calling getUniqueId
118+
Assert.assertEquals("test_value", SalesforceKeyGenerator.getUniqueId("test_name"));
119+
120+
// The value should have been re-encrypted
121+
// - it should not be decryptable with the legacy key pair
122+
// - it should be decryptable with the msdk key pair
123+
Assert.assertNull(decryptFromPrefs("test_name", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256));
124+
Assert.assertEquals("test_value", decryptFromPrefs("test_name", SalesforceKeyGenerator.MSDK_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256));
125+
}
126+
127+
@Test
128+
public void testMultipleGetUniqueIdStoredUsingLegacyKeyPair() {
129+
encryptAndStoreInPrefs("test_name_1", "test_value_1", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1);
130+
encryptAndStoreInPrefs("test_name_2", "test_value_2", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1);
131+
encryptAndStoreInPrefs("test_name_3", "test_value_3", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1);
132+
Assert.assertEquals("test_value_1", decryptFromPrefs("test_name_1", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
133+
Assert.assertEquals("test_value_2", decryptFromPrefs("test_name_2", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
134+
Assert.assertEquals("test_value_3", decryptFromPrefs("test_name_3", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
135+
136+
// Now calling getUniqueId for the first one
137+
Assert.assertEquals("test_value_1", SalesforceKeyGenerator.getUniqueId("test_name_1"));
138+
139+
// The value should have been re-encrypted
140+
// - it should not be decryptable with the legacy key pair
141+
// - it should be decryptable with the msdk key pair
142+
Assert.assertNull(decryptFromPrefs("test_name_1", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
143+
Assert.assertEquals("test_value_1", decryptFromPrefs("test_name_1", SalesforceKeyGenerator.MSDK_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256));
144+
145+
// Other values should not have been re-encrypted
146+
Assert.assertEquals("test_value_2", decryptFromPrefs("test_name_2", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
147+
Assert.assertEquals("test_value_3", decryptFromPrefs("test_name_3", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
148+
149+
// Now calling getUniqueId for the second one
150+
Assert.assertEquals("test_value_2", SalesforceKeyGenerator.getUniqueId("test_name_2"));
151+
152+
// The value should have been re-encrypted
153+
// - it should not be decryptable with the legacy key pair
154+
// - it should be decryptable with the msdk key pair
155+
Assert.assertNull(decryptFromPrefs("test_name_2", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
156+
Assert.assertEquals("test_value_2", decryptFromPrefs("test_name_2", SalesforceKeyGenerator.MSDK_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256));
157+
158+
// The already re-encrypted value should have been left alone
159+
Assert.assertNull(decryptFromPrefs("test_name_1", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
160+
Assert.assertEquals("test_value_1", decryptFromPrefs("test_name_1", SalesforceKeyGenerator.MSDK_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256));
161+
162+
// The third one should not have been re-encrypted
163+
Assert.assertEquals("test_value_3", decryptFromPrefs("test_name_3", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
164+
165+
// Now calling getUniqueId for the third one
166+
Assert.assertEquals("test_value_3", SalesforceKeyGenerator.getUniqueId("test_name_3"));
167+
168+
// The value should have been re-encrypted
169+
// - it should not be decryptable with the legacy key pair
170+
// - it should be decryptable with the msdk key pair
171+
Assert.assertNull(decryptFromPrefs("test_name_3", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
172+
Assert.assertEquals("test_value_3", decryptFromPrefs("test_name_3", SalesforceKeyGenerator.MSDK_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256));
173+
174+
// The already re-encrypted values should have been left alone
175+
Assert.assertNull(decryptFromPrefs("test_name_1", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
176+
Assert.assertEquals("test_value_1", decryptFromPrefs("test_name_1", SalesforceKeyGenerator.MSDK_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256));
177+
Assert.assertNull(decryptFromPrefs("test_name_2", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1));
178+
Assert.assertEquals("test_value_2", decryptFromPrefs("test_name_2", SalesforceKeyGenerator.MSDK_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_OAEP_SHA256));
179+
}
180+
181+
182+
@Test
183+
public void testMakeSureLegacyKeyPairNotRecreated() {
184+
encryptAndStoreInPrefs("test_name", "test_value", SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS, Encryptor.CipherMode.RSA_PKCS1);
185+
PublicKey legacyPublicKey = KeyStoreWrapper.getInstance().getRSAPublicKey(SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS);
186+
// Now calling getUniqueId
187+
Assert.assertEquals("test_value", SalesforceKeyGenerator.getUniqueId("test_name"));
188+
// The legacy key pair should NOT have been deleted or recreated
189+
Assert.assertEquals(legacyPublicKey.toString(), KeyStoreWrapper.getInstance().getRSAPublicKey(SalesforceKeyGenerator.LEGACY_KEYPAIR_ALIAS).toString());
190+
}
191+
192+
private void encryptAndStoreInPrefs(String name, String value, String keyPairAlias, Encryptor.CipherMode cipherMode) {
193+
PublicKey publicKey = KeyStoreWrapper.getInstance().getRSAPublicKey(keyPairAlias);
194+
String encryptedKey = Encryptor.encryptWithRSA(publicKey, value, cipherMode);
195+
SalesforceKeyGenerator.storeInSharedPrefs("id_" + name, encryptedKey);
196+
}
197+
198+
private String decryptFromPrefs(String name, String keyPairAlias, Encryptor.CipherMode cipherMode) {
199+
PrivateKey privateKey = KeyStoreWrapper.getInstance().getRSAPrivateKey(keyPairAlias);
200+
String encryptedValue = SalesforceKeyGenerator.readFromSharedPrefs("id_" + name);
201+
return Encryptor.decryptWithRSA(privateKey, encryptedValue, cipherMode);
202+
}
90203
}

0 commit comments

Comments
 (0)