-
-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Encrypt credentials in memory #2723
base: master
Are you sure you want to change the base?
Changes from 28 commits
77934f1
b6cf455
c137fab
6547246
8fdcbf6
9722643
0996c07
f1201a9
daf20a3
9c54fe2
d149226
ebb96fd
db3e33a
5a45854
d328a05
ecb938b
d8f3b6c
021f2d9
7bfd5af
8c74193
4e137b6
275be76
ce84ea1
8609871
28493bf
6c5caf0
efe1908
26f6e1f
5c6f8fd
8c15e6b
6a805fc
8a33928
cd31c2b
8a97d37
6edd01d
13ffe39
ce30b99
ff2102d
1e9326c
b1af601
7d66b9f
48aa5a5
af8194a
db7ef66
403fff4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -136,42 +136,50 @@ dependencies { | |
} | ||
|
||
ext { | ||
// Obtained from ~/.gradle/gradle.properties on build server (mobile agent), or your local | ||
// Obtained from ~/.gradle/gradle.properties on build server (mobile agent), or your local | ||
// ~/.gradle/gradle.properties file, or loads default empty strings if neither is present | ||
MAPBOX_SDK_API_KEY = project.properties['MAPBOX_SDK_API_KEY'] ?: "" | ||
ANALYTICS_TRACKING_ID_DEV = project.properties['ANALYTICS_TRACKING_ID_DEV'] ?: "" | ||
ANALYTICS_TRACKING_ID_LIVE = project.properties['ANALYTICS_TRACKING_ID_LIVE'] ?: "" | ||
GOOGLE_PLAY_MAPS_API_KEY = project.properties['GOOGLE_PLAY_MAPS_API_KEY'] ?: "" | ||
RELEASE_STORE_FILE = project.properties['RELEASE_STORE_FILE'] ?: "." | ||
RELEASE_STORE_PASSWORD = project.properties['RELEASE_STORE_PASSWORD'] ?: "" | ||
RELEASE_KEY_ALIAS = project.properties['RELEASE_KEY_ALIAS'] ?: "" | ||
RELEASE_KEY_PASSWORD = project.properties['RELEASE_KEY_PASSWORD'] ?: "" | ||
MAPBOX_SDK_API_KEY = project.properties['MAPBOX_SDK_API_KEY'] ?: '' | ||
ANALYTICS_TRACKING_ID_DEV = project.properties['ANALYTICS_TRACKING_ID_DEV'] ?: '' | ||
ANALYTICS_TRACKING_ID_LIVE = project.properties['ANALYTICS_TRACKING_ID_LIVE'] ?: '' | ||
GOOGLE_PLAY_MAPS_API_KEY = project.properties['GOOGLE_PLAY_MAPS_API_KEY'] ?: '' | ||
RELEASE_STORE_FILE = project.properties['RELEASE_STORE_FILE'] ?: '.' | ||
RELEASE_STORE_PASSWORD = project.properties['RELEASE_STORE_PASSWORD'] ?: '' | ||
RELEASE_KEY_ALIAS = project.properties['RELEASE_KEY_ALIAS'] ?: '' | ||
RELEASE_KEY_PASSWORD = project.properties['RELEASE_KEY_PASSWORD'] ?: '' | ||
TRUSTED_SOURCE_PUBLIC_KEY = project.properties['TRUSTED_SOURCE_PUBLIC_KEY'] ?: | ||
"MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHiuy2ULV4pobkuQN2TEjmR1tn" + | ||
"HJ+F335hm/lVdaFQzvBmeq64MUMbumheVLDJaSUiAVzqSHDKJWH01ZQRowqBYjwo" + | ||
"ycVSQSeO2glc6XZZ+CJudAPXe8iFWLQp3kBBnBmVcBXCOQFO7aLgQMv4nqKZsLW0" + | ||
"HaAJkjpnc165Os+aYwIDAQAB" | ||
GOOGLE_SERVICES_API_KEY = project.properties['GOOGLE_SERVICES_API_KEY'] ?: "" | ||
QA_BETA_APP_ID = "" | ||
STANDALONE_APP_ID = "" | ||
LTS_APP_ID = "" | ||
COMMCARE_APP_ID = "" | ||
HQ_API_USERNAME = project.properties['HQ_API_USERNAME'] ?: "" | ||
HQ_API_PASSWORD = project.properties['HQ_API_PASSWORD'] ?: "" | ||
TEST_BUILD_TYPE = project.properties['TEST_BUILD_TYPE'] ?: "debug" | ||
FIREBASE_DATABASE_URL = project.properties['FIREBASE_DATABASE_URL'] ?: "" | ||
GOOGLE_SERVICES_API_KEY = project.properties['GOOGLE_SERVICES_API_KEY'] ?: '' | ||
QA_BETA_APP_ID = '' | ||
STANDALONE_APP_ID = '' | ||
LTS_APP_ID = '' | ||
COMMCARE_APP_ID = '' | ||
HQ_API_USERNAME = project.properties['HQ_API_USERNAME'] ?: '' | ||
HQ_API_PASSWORD = project.properties['HQ_API_PASSWORD'] ?: '' | ||
TEST_BUILD_TYPE = project.properties['TEST_BUILD_TYPE'] ?: 'debug' | ||
FIREBASE_DATABASE_URL = project.properties['FIREBASE_DATABASE_URL'] ?: '' | ||
|
||
// properties related to Service providers part of the Java SPI pattern | ||
SERVICE_PROVIDERS_REL_DIR = 'META-INF/services' | ||
/** | ||
* Service provider implementations and respective service interfaces should be added to this | ||
* map. The task registerServiceProviders is responsible for iterating over it and create the | ||
* configuration files under META-INF/services | ||
*/ | ||
SERVICE_PROVIDERS = ['org.commcare.util.IEncryptionKeyProvider' : 'org.commcare.utils.EncryptionKeyProvider'] | ||
} | ||
|
||
afterEvaluate { | ||
// Hack to get assets to show up in robolectric tests; try to eventually remove this | ||
preCommcareDebugUnitTestBuild.dependsOn mergeCommcareDebugAssets | ||
processStandaloneDebugGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile | ||
processStandaloneReleaseGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile | ||
processLtsDebugGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile | ||
processLtsReleaseGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile | ||
processCommcareDebugGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile | ||
processCommcareReleaseGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile | ||
preCommcareDebugUnitTestBuild.dependsOn mergeCommcareDebugAssets, registerServiceProviders | ||
processStandaloneDebugGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile, registerServiceProviders | ||
processStandaloneReleaseGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile, registerServiceProviders | ||
processLtsDebugGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile, registerServiceProviders | ||
processLtsReleaseGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile, registerServiceProviders | ||
processCommcareDebugGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile, registerServiceProviders | ||
processCommcareReleaseGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile, registerServiceProviders | ||
} | ||
|
||
/** | ||
|
@@ -632,3 +640,17 @@ downloadLicenses { | |
includeProjectDependencies = true | ||
dependencyConfiguration = 'compile' | ||
} | ||
|
||
task registerServiceProviders { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why add it at build time instead of just having a static file ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shubham1g5 that was the initial approach but I thought about making it easy for future additions and also to put this on others radar, a static file can very easily be overlooked. So, not necessarily strong reasons. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, I would still prefer a static file if we go with service provider approach as this adds an unnecessary build step without any strong advantages. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted |
||
doLast { | ||
def servProvAbsPath = android.sourceSets.main.java.srcDirs[0].path + File.separator + project.ext.SERVICE_PROVIDERS_REL_DIR | ||
project.ext.SERVICE_PROVIDERS.each { servProv -> | ||
println(servProvAbsPath + File.separator + "$servProv.key") | ||
def file = new File(servProvAbsPath + File.separator + "$servProv.key") | ||
if(!file.getParentFile().exists()) { | ||
file.getParentFile().mkdirs() | ||
} | ||
file.write("$servProv.value") | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,6 +140,9 @@ | |
import okhttp3.MultipartBody; | ||
import okhttp3.RequestBody; | ||
|
||
import static org.commcare.util.EncryptionUtils.USER_CREDENTIALS_KEY_ALIAS; | ||
import static org.commcare.util.EncryptionUtils.getEncryptionKeyProvider; | ||
|
||
public class CommCareApplication extends MultiDexApplication { | ||
|
||
private static final String TAG = CommCareApplication.class.getSimpleName(); | ||
|
@@ -228,6 +231,8 @@ public void onCreate() { | |
setRoots(); | ||
prepareTemporaryStorage(); | ||
|
||
getEncryptionKeyProvider().generateCryptographicKeyInKeyStore(USER_CREDENTIALS_KEY_ALIAS); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this key is not user specific, can we call it something general like cc_in_memory_encryption_key ? |
||
|
||
if (LegacyInstallUtils.checkForLegacyInstall(this)) { | ||
dbState = STATE_LEGACY_DETECTED; | ||
} else { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
package org.commcare.utils; | ||
|
||
import android.os.Build; | ||
import android.security.KeyPairGeneratorSpec; | ||
import android.security.keystore.KeyGenParameterSpec; | ||
import android.security.keystore.KeyProperties; | ||
|
||
import org.commcare.CommCareApplication; | ||
import org.commcare.util.EncryptionKeyAndTransformation; | ||
import org.commcare.util.EncryptionUtils; | ||
import org.commcare.util.IEncryptionKeyProvider; | ||
|
||
import java.io.IOException; | ||
import java.math.BigInteger; | ||
import java.security.InvalidAlgorithmParameterException; | ||
import java.security.Key; | ||
import java.security.KeyPairGenerator; | ||
import java.security.KeyStore; | ||
import java.security.KeyStoreException; | ||
import java.security.NoSuchAlgorithmException; | ||
import java.security.NoSuchProviderException; | ||
import java.security.Security; | ||
import java.security.UnrecoverableEntryException; | ||
import java.security.cert.CertificateException; | ||
import java.util.GregorianCalendar; | ||
|
||
import javax.crypto.KeyGenerator; | ||
import javax.security.auth.x500.X500Principal; | ||
|
||
import androidx.annotation.RequiresApi; | ||
|
||
import static org.commcare.utils.GlobalConstants.KEYSTORE_NAME; | ||
|
||
/** | ||
* Class for providing encryption keys backed by Android Keystore | ||
* | ||
* @author dviggiano | ||
*/ | ||
public class EncryptionKeyProvider implements IEncryptionKeyProvider { | ||
|
||
@RequiresApi(api = Build.VERSION_CODES.M) | ||
private static final String ALGORITHM = KeyProperties.KEY_ALGORITHM_AES; | ||
@RequiresApi(api = Build.VERSION_CODES.M) | ||
private static final String BLOCK_MODE = KeyProperties.BLOCK_MODE_GCM; | ||
@RequiresApi(api = Build.VERSION_CODES.M) | ||
private static final String PADDING = KeyProperties.ENCRYPTION_PADDING_NONE; | ||
private static KeyStore keystoreSingleton = null; | ||
|
||
private static KeyStore getKeyStore() throws KeyStoreException, CertificateException, | ||
IOException, NoSuchAlgorithmException { | ||
if (keystoreSingleton == null) { | ||
keystoreSingleton = KeyStore.getInstance(KEYSTORE_NAME); | ||
keystoreSingleton.load(null); | ||
} | ||
return keystoreSingleton; | ||
} | ||
|
||
@Override | ||
public EncryptionKeyAndTransformation retrieveKeyFromKeyStore(String keyAlias, | ||
EncryptionUtils.CryptographicOperation operation) | ||
throws KeyStoreException, UnrecoverableEntryException, NoSuchAlgorithmException, | ||
CertificateException, IOException { | ||
Key key; | ||
if (getKeyStore().containsAlias(keyAlias)) { | ||
KeyStore.Entry keyEntry = getKeyStore().getEntry(keyAlias, null); | ||
if (keyEntry instanceof KeyStore.PrivateKeyEntry) { | ||
if (operation == EncryptionUtils.CryptographicOperation.Encryption) { | ||
key = ((KeyStore.PrivateKeyEntry)keyEntry).getCertificate().getPublicKey(); | ||
} else { | ||
key = ((KeyStore.PrivateKeyEntry)keyEntry).getPrivateKey(); | ||
} | ||
} else { | ||
key = ((KeyStore.SecretKeyEntry)keyEntry).getSecretKey(); | ||
} | ||
} else { | ||
throw new KeyStoreException("Key not found in KeyStore"); | ||
} | ||
if (key != null) { | ||
return new EncryptionKeyAndTransformation(key, getTransformationString(key.getAlgorithm())); | ||
} else { | ||
return null; | ||
} | ||
} | ||
|
||
// Generates a cryptrographic key and adds it to the Android KeyStore | ||
public void generateCryptographicKeyInKeyStore(String keyAlias) { | ||
if (isKeyStoreAvailable()) { | ||
try { | ||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { | ||
KeyGenerator keyGenerator = KeyGenerator | ||
.getInstance(getAESKeyAlgorithmRepresentation(), KEYSTORE_NAME); | ||
KeyGenParameterSpec keyGenParameterSpec = new KeyGenParameterSpec.Builder(keyAlias, | ||
KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT) | ||
.setBlockModes(BLOCK_MODE) | ||
.setEncryptionPaddings(PADDING) | ||
.build(); | ||
keyGenerator.init(keyGenParameterSpec); | ||
keyGenerator.generateKey(); | ||
} else { | ||
// Because KeyGenParameterSpec was introduced in Android SDK 23, prior versions | ||
// need to resource to KeyPairGenerator which only generates asymmetric keys, | ||
// hence the need to switch to a correspondent algorithm as well, RSA | ||
// TODO: Add link to StackOverflow page | ||
KeyPairGenerator keyGenerator = KeyPairGenerator | ||
.getInstance(getRSAKeyAlgorithmRepresentation(), KEYSTORE_NAME); | ||
GregorianCalendar start = new GregorianCalendar(); | ||
GregorianCalendar end = new GregorianCalendar(); | ||
end.add(GregorianCalendar.YEAR, 100); | ||
|
||
KeyPairGeneratorSpec keySpec = new KeyPairGeneratorSpec.Builder(CommCareApplication.instance()) | ||
// Key alias to be used to retrieve it from the KeyStore | ||
.setAlias(keyAlias) | ||
// The subject used for the self-signed certificate of the generated pair | ||
.setSubject(new X500Principal(String.format("CN=%s", keyAlias))) | ||
// The serial number used for the self-signed certificate of the | ||
// generated pair | ||
.setSerialNumber(BigInteger.valueOf(1337)) | ||
// Date range of validity for the generated pair | ||
.setStartDate(start.getTime()) | ||
.setEndDate(end.getTime()) | ||
.build(); | ||
|
||
keyGenerator.initialize(keySpec); | ||
keyGenerator.generateKeyPair(); | ||
} | ||
|
||
} catch (NoSuchAlgorithmException | NoSuchProviderException | | ||
InvalidAlgorithmParameterException e) { | ||
throw new RuntimeException(e); | ||
} | ||
} else { | ||
throw new RuntimeException("KeyStore not available"); | ||
} | ||
} | ||
|
||
@Override | ||
public boolean isKeyStoreAvailable() { | ||
return Security.getProvider(KEYSTORE_NAME) != null; | ||
} | ||
|
||
@Override | ||
public String getAESKeyAlgorithmRepresentation() { | ||
return ALGORITHM; | ||
} | ||
|
||
@Override | ||
public String getRSAKeyAlgorithmRepresentation() { | ||
return "RSA"; | ||
} | ||
|
||
@Override | ||
public String getTransformationString(String algorithm) { | ||
String transformation = null; | ||
if (algorithm.equals(getRSAKeyAlgorithmRepresentation())) { | ||
transformation = "RSA/ECB/PKCS1Padding"; | ||
} else if (algorithm.equals(getAESKeyAlgorithmRepresentation())) { | ||
transformation = String.format("%s/%s/%s", algorithm, BLOCK_MODE, PADDING); | ||
} | ||
// This will cause an error if null | ||
return transformation; | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reasoning behind specifying it here instead of Java -
registerServiceKeyProvider(new EncryptionKeyProvider()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 this is just to create the static configuration files, the instantiation is done by the JRE using the
ServiceLoader
. The reasoning here is to decouple the service interface from its implementations, more concretely, it removes any dependency betweencommcare-android
andcommcare-core
when it comes to IEncryptionKeyProvider implementations.