From d0947e8377283f8780b5e7555dd321950a2de147 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Tue, 1 Apr 2014 18:44:36 +0200 Subject: [PATCH] Only allow private keys associated to accounts of an app --- .../keychain/pgp/PgpDecryptVerify.java | 23 ++++++++++--------- .../keychain/provider/ProviderHelper.java | 20 ++++++++++++++-- .../keychain/remote/OpenPgpService.java | 12 ++++++---- .../keychain/remote/RemoteService.java | 2 +- 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java index 29255fbbe..003db632d 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java @@ -67,6 +67,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.security.SignatureException; import java.util.Iterator; +import java.util.Set; /** * This class uses a Builder pattern! @@ -79,7 +80,7 @@ public class PgpDecryptVerify { private ProgressDialogUpdater mProgressDialogUpdater; private boolean mAllowSymmetricDecryption; private String mPassphrase; - private long mEnforcedKeyId; + private Set mAllowedKeyIds; private PgpDecryptVerify(Builder builder) { // private Constructor can only be called from Builder @@ -90,7 +91,7 @@ public class PgpDecryptVerify { this.mProgressDialogUpdater = builder.mProgressDialogUpdater; this.mAllowSymmetricDecryption = builder.mAllowSymmetricDecryption; this.mPassphrase = builder.mPassphrase; - this.mEnforcedKeyId = builder.mEnforcedKeyId; + this.mAllowedKeyIds = builder.mAllowedKeyIds; } public static class Builder { @@ -103,7 +104,7 @@ public class PgpDecryptVerify { private ProgressDialogUpdater mProgressDialogUpdater = null; private boolean mAllowSymmetricDecryption = true; private String mPassphrase = null; - private long mEnforcedKeyId = 0; + private Set mAllowedKeyIds = null; public Builder(Context context, InputData data, OutputStream outStream) { this.mContext = context; @@ -127,14 +128,14 @@ public class PgpDecryptVerify { } /** - * Allow this key id alone for decryption. - * This means only ciphertexts encrypted for this private key can be decrypted. + * Allow these key ids alone for decryption. + * This means only ciphertexts encrypted for one of these private key can be decrypted. * - * @param enforcedKeyId + * @param allowedKeyIds * @return */ - public Builder enforcedKeyId(long enforcedKeyId) { - this.mEnforcedKeyId = enforcedKeyId; + public Builder allowedKeyIds(Set allowedKeyIds) { + this.mAllowedKeyIds = allowedKeyIds; return this; } @@ -236,16 +237,16 @@ public class PgpDecryptVerify { // secret key exists in database // allow only a specific key for decryption? - if (mEnforcedKeyId != 0) { + if (mAllowedKeyIds != null) { // TODO: improve this code! get master key directly! PGPSecretKeyRing secretKeyRing = ProviderHelper.getPGPSecretKeyRingByKeyId(mContext, encData.getKeyID()); long masterKeyId = PgpKeyHelper.getMasterKey(secretKeyRing).getKeyID(); Log.d(Constants.TAG, "encData.getKeyID():" + encData.getKeyID()); - Log.d(Constants.TAG, "enforcedKeyId: " + mEnforcedKeyId); + Log.d(Constants.TAG, "allowedKeyIds: " + mAllowedKeyIds); Log.d(Constants.TAG, "masterKeyId: " + masterKeyId); - if (mEnforcedKeyId != masterKeyId) { + if (!mAllowedKeyIds.contains(masterKeyId)) { throw new PgpGeneralException( mContext.getString(R.string.error_no_secret_key_found)); } diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java index b971143ae..13dc2af2b 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -48,6 +48,8 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Date; +import java.util.HashSet; +import java.util.Set; public class ProviderHelper { @@ -867,10 +869,10 @@ public class ProviderHelper { return settings; } - public static AccountSettings getApiAccountSettings(Context context, Uri uri) { + public static AccountSettings getApiAccountSettings(Context context, Uri accountUri) { AccountSettings settings = null; - Cursor cur = context.getContentResolver().query(uri, null, null, null, null); + Cursor cur = context.getContentResolver().query(accountUri, null, null, null, null); if (cur != null && cur.moveToFirst()) { settings = new AccountSettings(); @@ -889,6 +891,20 @@ public class ProviderHelper { return settings; } + public static Set getAllKeyIdsForApp(Context context, Uri uri) { + Set keyIds = new HashSet(); + + Cursor cursor = context.getContentResolver().query(uri, null, null, null, null); + if (cursor != null) { + int keyIdColumn = cursor.getColumnIndex(KeychainContract.ApiAccounts.KEY_ID); + while (cursor.moveToNext()) { + keyIds.add(cursor.getLong(keyIdColumn)); + } + } + + return keyIds; + } + public static byte[] getApiAppSignature(Context context, String packageName) { Uri queryUri = ApiApps.buildByPackageNameUri(packageName); diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java index 1cd5862e7..b00c012b8 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -45,6 +45,7 @@ import org.sufficientlysecure.keychain.util.Log; import java.io.InputStream; import java.io.OutputStream; import java.util.ArrayList; +import java.util.Set; public class OpenPgpService extends RemoteService { @@ -276,7 +277,7 @@ public class OpenPgpService extends RemoteService { } private Intent decryptAndVerifyImpl(Intent data, ParcelFileDescriptor input, - ParcelFileDescriptor output, AccountSettings accSettings) { + ParcelFileDescriptor output, Set allowedKeyIds) { try { // Get Input- and OutputStream from ParcelFileDescriptor InputStream is = new ParcelFileDescriptor.AutoCloseInputStream(input); @@ -291,7 +292,7 @@ public class OpenPgpService extends RemoteService { PgpDecryptVerify.Builder builder = new PgpDecryptVerify.Builder(this, inputData, os); builder.allowSymmetricDecryption(false) // no support for symmetric encryption - .enforcedKeyId(accSettings.getKeyId()) // allow only the private key for this app for decryption + .allowedKeyIds(allowedKeyIds) // allow only private keys associated with accounts of this app .passphrase(passphrase); // TODO: currently does not support binary signed-only content @@ -299,7 +300,7 @@ public class OpenPgpService extends RemoteService { if (PgpDecryptVerifyResult.KEY_PASSHRASE_NEEDED == decryptVerifyResult.getStatus()) { // get PendingIntent for passphrase input, add it to given params and return to client - Intent passphraseBundle = getPassphraseBundleIntent(data, accSettings.getKeyId()); + Intent passphraseBundle = getPassphraseBundleIntent(data, decryptVerifyResult.getKeyIdPassphraseNeeded()); return passphraseBundle; } else if (PgpDecryptVerifyResult.SYMMETRIC_PASSHRASE_NEEDED == decryptVerifyResult.getStatus()) { throw new PgpGeneralException("Decryption of symmetric content not supported by API!"); @@ -452,7 +453,10 @@ public class OpenPgpService extends RemoteService { } else if (OpenPgpApi.ACTION_SIGN_AND_ENCRYPT.equals(action)) { return encryptAndSignImpl(data, input, output, accSettings, true); } else if (OpenPgpApi.ACTION_DECRYPT_VERIFY.equals(action)) { - return decryptAndVerifyImpl(data, input, output, accSettings); + String currentPkg = getCurrentCallingPackage(); + Set allowedKeyIds = + ProviderHelper.getAllKeyIdsForApp(mContext, KeychainContract.ApiAccounts.buildBaseUri(currentPkg)); + return decryptAndVerifyImpl(data, input, output, allowedKeyIds); } else if (OpenPgpApi.ACTION_GET_KEY.equals(action)) { return getKeyImpl(data); } else if (OpenPgpApi.ACTION_GET_KEY_IDS.equals(action)) { diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/remote/RemoteService.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/remote/RemoteService.java index 59a04a10e..3e72eec17 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/remote/RemoteService.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/remote/RemoteService.java @@ -127,7 +127,7 @@ public abstract class RemoteService extends Service { * * @return package name */ - private String getCurrentCallingPackage() { + protected String getCurrentCallingPackage() { // TODO: // callingPackages contains more than one entry when sharedUserId has been used... String[] callingPackages = getPackageManager().getPackagesForUid(Binder.getCallingUid());