From 7c3a53d1496a376bdaaa3bf7c79c77fe6d90a3d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 30 Dec 2013 19:16:21 +0100 Subject: remote service: package signature verification, use string for service instead of getClass.getName --- .../keychain/provider/KeychainContract.java | 1 + .../keychain/provider/KeychainDatabase.java | 13 +- .../keychain/provider/ProviderHelper.java | 25 ++++ .../exception/WrongPackageSignatureException.java | 10 ++ .../keychain/service/remote/AppSettings.java | 12 +- .../service/remote/AppSettingsFragment.java | 23 +++- .../keychain/service/remote/RemoteService.java | 142 +++++++++++++++------ .../service/remote/RemoteServiceActivity.java | 4 +- 8 files changed, 182 insertions(+), 48 deletions(-) create mode 100644 OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/WrongPackageSignatureException.java (limited to 'OpenPGP-Keychain/src/org/sufficientlysecure') diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/KeychainContract.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/KeychainContract.java index ea4ca377c..82bb473f6 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/KeychainContract.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/KeychainContract.java @@ -55,6 +55,7 @@ public class KeychainContract { interface ApiAppsColumns { String PACKAGE_NAME = "package_name"; + String PACKAGE_SIGNATURE = "package_signature"; String KEY_ID = "key_id"; // not a database id String ENCRYPTION_ALGORITHM = "encryption_algorithm"; String HASH_ALORITHM = "hash_algorithm"; diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/KeychainDatabase.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/KeychainDatabase.java index 0f962967d..60c5c91a8 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/KeychainDatabase.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/KeychainDatabase.java @@ -31,7 +31,7 @@ import android.provider.BaseColumns; public class KeychainDatabase extends SQLiteOpenHelper { private static final String DATABASE_NAME = "apg.db"; - private static final int DATABASE_VERSION = 5; + private static final int DATABASE_VERSION = 6; public interface Tables { String KEY_RINGS = "key_rings"; @@ -66,9 +66,10 @@ public class KeychainDatabase extends SQLiteOpenHelper { private static final String CREATE_API_APPS = "CREATE TABLE IF NOT EXISTS " + Tables.API_APPS + " (" + BaseColumns._ID + " INTEGER PRIMARY KEY AUTOINCREMENT, " - + ApiAppsColumns.PACKAGE_NAME + " TEXT UNIQUE, " + ApiAppsColumns.KEY_ID + " INT64, " - + ApiAppsColumns.ENCRYPTION_ALGORITHM + " INTEGER, " + ApiAppsColumns.HASH_ALORITHM - + " INTEGER, " + ApiAppsColumns.COMPRESSION + " INTEGER)"; + + ApiAppsColumns.PACKAGE_NAME + " TEXT UNIQUE, " + ApiAppsColumns.PACKAGE_SIGNATURE + + " BLOB, " + ApiAppsColumns.KEY_ID + " INT64, " + ApiAppsColumns.ENCRYPTION_ALGORITHM + + " INTEGER, " + ApiAppsColumns.HASH_ALORITHM + " INTEGER, " + + ApiAppsColumns.COMPRESSION + " INTEGER)"; KeychainDatabase(Context context) { super(context, DATABASE_NAME, null, DATABASE_VERSION); @@ -110,6 +111,10 @@ public class KeychainDatabase extends SQLiteOpenHelper { break; case 4: db.execSQL(CREATE_API_APPS); + case 5: + // new column: package_signature + db.execSQL("DROP TABLE IF EXISTS " + Tables.API_APPS); + db.execSQL(CREATE_API_APPS); default: break; diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/ProviderHelper.java index 7ef61c15b..f12048277 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -742,6 +742,7 @@ public class ProviderHelper { private static ContentValues contentValueForApiApps(AppSettings appSettings) { ContentValues values = new ContentValues(); values.put(ApiApps.PACKAGE_NAME, appSettings.getPackageName()); + values.put(ApiApps.PACKAGE_SIGNATURE, appSettings.getPackageSignature()); values.put(ApiApps.KEY_ID, appSettings.getKeyId()); values.put(ApiApps.COMPRESSION, appSettings.getCompression()); values.put(ApiApps.ENCRYPTION_ALGORITHM, appSettings.getEncryptionAlgorithm()); @@ -770,6 +771,8 @@ public class ProviderHelper { settings = new AppSettings(); settings.setPackageName(cur.getString(cur .getColumnIndex(KeychainContract.ApiApps.PACKAGE_NAME))); + settings.setPackageSignature(cur.getBlob(cur + .getColumnIndex(KeychainContract.ApiApps.PACKAGE_SIGNATURE))); settings.setKeyId(cur.getLong(cur.getColumnIndex(KeychainContract.ApiApps.KEY_ID))); settings.setCompression(cur.getInt(cur .getColumnIndexOrThrow(KeychainContract.ApiApps.COMPRESSION))); @@ -781,4 +784,26 @@ public class ProviderHelper { return settings; } + + public static byte[] getApiAppSignature(Context context, String packageName) { + Uri queryUri = KeychainContract.ApiApps.buildByPackageNameUri(packageName); + + String[] projection = new String[] { ApiApps.PACKAGE_SIGNATURE }; + + ContentResolver cr = context.getContentResolver(); + Cursor cursor = cr.query(queryUri, projection, null, null, null); + + byte[] signature = null; + if (cursor != null && cursor.moveToFirst()) { + int signatureCol = 0; + + signature = cursor.getBlob(signatureCol); + } + + if (cursor != null) { + cursor.close(); + } + + return signature; + } } diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/WrongPackageSignatureException.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/WrongPackageSignatureException.java new file mode 100644 index 000000000..cef002265 --- /dev/null +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/exception/WrongPackageSignatureException.java @@ -0,0 +1,10 @@ +package org.sufficientlysecure.keychain.service.exception; + +public class WrongPackageSignatureException extends Exception { + + private static final long serialVersionUID = -8294642703122196028L; + + public WrongPackageSignatureException(String message) { + super(message); + } +} \ No newline at end of file diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/AppSettings.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/AppSettings.java index 381a4065c..9da4c8392 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/AppSettings.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/AppSettings.java @@ -23,6 +23,7 @@ import org.sufficientlysecure.keychain.Id; public class AppSettings { private String packageName; + private byte[] packageSignature; private long keyId = Id.key.none; private int encryptionAlgorithm; private int hashAlgorithm; @@ -32,9 +33,10 @@ public class AppSettings { } - public AppSettings(String packageName) { + public AppSettings(String packageName, byte[] packageSignature) { super(); this.packageName = packageName; + this.packageSignature = packageSignature; // defaults: this.encryptionAlgorithm = PGPEncryptedData.AES_256; this.hashAlgorithm = HashAlgorithmTags.SHA512; @@ -49,6 +51,14 @@ public class AppSettings { this.packageName = packageName; } + public byte[] getPackageSignature() { + return packageSignature; + } + + public void setPackageSignature(byte[] packageSignature) { + this.packageSignature = packageSignature; + } + public long getKeyId() { return keyId; } diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/AppSettingsFragment.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/AppSettingsFragment.java index 942f8eba8..e592f5d57 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/AppSettingsFragment.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/AppSettingsFragment.java @@ -17,8 +17,12 @@ package org.sufficientlysecure.keychain.service.remote; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; + import org.spongycastle.openpgp.PGPSecretKey; import org.spongycastle.openpgp.PGPSecretKeyRing; +import org.spongycastle.util.encoders.Hex; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.Id; import org.sufficientlysecure.keychain.R; @@ -67,6 +71,8 @@ public class AppSettingsFragment extends Fragment { private Spinner mEncryptionAlgorithm; private Spinner mHashAlgorithm; private Spinner mCompression; + private TextView mPackageName; + private TextView mPackageSignature; KeyValueSpinnerAdapter encryptionAdapter; KeyValueSpinnerAdapter hashAdapter; @@ -79,6 +85,19 @@ public class AppSettingsFragment extends Fragment { public void setAppSettings(AppSettings appSettings) { this.appSettings = appSettings; setPackage(appSettings.getPackageName()); + mPackageName.setText(appSettings.getPackageName()); + + try { + MessageDigest md = MessageDigest.getInstance("SHA-256"); + md.update(appSettings.getPackageSignature()); + byte[] digest = md.digest(); + String signature = new String(Hex.encode(digest)); + + mPackageSignature.setText(signature); + } catch (NoSuchAlgorithmException e) { + Log.e(Constants.TAG, "Should not happen!", e); + } + updateSelectedKeyView(appSettings.getKeyId()); mEncryptionAlgorithm.setSelection(encryptionAdapter.getPosition(appSettings .getEncryptionAlgorithm())); @@ -110,6 +129,8 @@ public class AppSettingsFragment extends Fragment { .findViewById(R.id.api_app_settings_encryption_algorithm); mHashAlgorithm = (Spinner) view.findViewById(R.id.api_app_settings_hash_algorithm); mCompression = (Spinner) view.findViewById(R.id.api_app_settings_compression); + mPackageName = (TextView) view.findViewById(R.id.api_app_settings_package_name); + mPackageSignature = (TextView) view.findViewById(R.id.api_app_settings_package_signature); AlgorithmNames algorithmNames = new AlgorithmNames(getActivity()); @@ -182,7 +203,7 @@ public class AppSettingsFragment extends Fragment { public void onClick(View v) { if (mAdvancedSettingsContainer.getVisibility() == View.VISIBLE) { mAdvancedSettingsContainer.startAnimation(invisibleAnimation); - mAdvancedSettingsContainer.setVisibility(View.INVISIBLE); + mAdvancedSettingsContainer.setVisibility(View.GONE); mAdvancedSettingsButton.setText(R.string.api_settings_show_advanced); } else { mAdvancedSettingsContainer.startAnimation(visibleAnimation); diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteService.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteService.java index 4e8c4678a..0f28d96f6 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteService.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteService.java @@ -18,18 +18,24 @@ package org.sufficientlysecure.keychain.service.remote; import java.util.ArrayList; +import java.util.Arrays; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.TimeUnit; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.ProviderHelper; +import org.sufficientlysecure.keychain.service.exception.WrongPackageSignatureException; import org.sufficientlysecure.keychain.util.Log; import org.sufficientlysecure.keychain.util.PausableThreadPoolExecutor; import android.app.Service; import android.content.Context; import android.content.Intent; +import android.content.pm.PackageInfo; +import android.content.pm.PackageManager; +import android.content.pm.PackageManager.NameNotFoundException; +import android.content.pm.Signature; import android.net.Uri; import android.os.Binder; import android.os.Bundle; @@ -38,7 +44,7 @@ import android.os.Message; import android.os.Messenger; /** - * Abstract service for remote APIs that handle app registration and user input. + * Abstract service class for remote APIs that handle app registration and user input. */ public abstract class RemoteService extends Service { Context mContext; @@ -98,32 +104,57 @@ public abstract class RemoteService extends Service { * @param r */ protected void checkAndEnqueue(Runnable r) { - if (isCallerAllowed(false)) { - mThreadPool.execute(r); - - Log.d(Constants.TAG, "Enqueued runnable…"); - } else { - String[] callingPackages = getPackageManager() - .getPackagesForUid(Binder.getCallingUid()); - - Log.e(Constants.TAG, "Not allowed to use service! Starting activity for registration!"); - Bundle extras = new Bundle(); - // TODO: currently simply uses first entry - extras.putString(RemoteServiceActivity.EXTRA_PACKAGE_NAME, callingPackages[0]); - - RegisterActivityCallback callback = new RegisterActivityCallback(); - - pauseAndStartUserInteraction(RemoteServiceActivity.ACTION_REGISTER, callback, extras); - - if (callback.isAllowed()) { + try { + if (isCallerAllowed(false)) { mThreadPool.execute(r); + Log.d(Constants.TAG, "Enqueued runnable…"); } else { - Log.d(Constants.TAG, "User disallowed app!"); + String[] callingPackages = getPackageManager().getPackagesForUid( + Binder.getCallingUid()); + // TODO: currently simply uses first entry + String packageName = callingPackages[0]; + + byte[] packageSignature; + try { + packageSignature = getPackageSignature(packageName); + } catch (NameNotFoundException e) { + Log.e(Constants.TAG, "Should not happen, returning!", e); + return; + } + Log.e(Constants.TAG, + "Not allowed to use service! Starting activity for registration!"); + Bundle extras = new Bundle(); + extras.putString(RemoteServiceActivity.EXTRA_PACKAGE_NAME, packageName); + extras.putByteArray(RemoteServiceActivity.EXTRA_PACKAGE_SIGNATURE, packageSignature); + RegisterActivityCallback callback = new RegisterActivityCallback(); + + pauseAndStartUserInteraction(RemoteServiceActivity.ACTION_REGISTER, callback, + extras); + + if (callback.isAllowed()) { + mThreadPool.execute(r); + Log.d(Constants.TAG, "Enqueued runnable…"); + } else { + Log.d(Constants.TAG, "User disallowed app!"); + } } + } catch (WrongPackageSignatureException e) { + // TODO: Inform user about wrong signature! + Log.e(Constants.TAG, "RemoteService", e); } } + private byte[] getPackageSignature(String packageName) throws NameNotFoundException { + PackageInfo pkgInfo = getPackageManager().getPackageInfo(packageName, + PackageManager.GET_SIGNATURES); + Signature[] signatures = pkgInfo.signatures; + // TODO: Only first signature?! + byte[] packageSignature = signatures[0].toByteArray(); + + return packageSignature; + } + /** * Locks current thread and pauses execution of runnables and starts activity for user input * @@ -200,15 +231,20 @@ public abstract class RemoteService extends Service { packageName = msg.getData().getString(PACKAGE_NAME); // resume threads - if (isPackageAllowed(packageName, false)) { - synchronized (userInputLock) { - userInputLock.notifyAll(); + try { + if (isPackageAllowed(packageName)) { + synchronized (userInputLock) { + userInputLock.notifyAll(); + } + mThreadPool.resume(); + } else { + // Should not happen! + Log.e(Constants.TAG, "Should not happen! Emergency shutdown!"); + mThreadPool.shutdownNow(); } - mThreadPool.resume(); - } else { - // Should not happen! - Log.e(Constants.TAG, "Should not happen! Emergency shutdown!"); - mThreadPool.shutdownNow(); + } catch (WrongPackageSignatureException e) { + // TODO: Inform user about wrong signature! + Log.e(Constants.TAG, "RemoteService", e); } } else { allowed = false; @@ -230,15 +266,28 @@ public abstract class RemoteService extends Service { * @param allowOnlySelf * allow only Keychain app itself * @return true if process is allowed to use this service + * @throws WrongPackageSignatureException */ - private boolean isCallerAllowed(boolean allowOnlySelf) { - String[] callingPackages = getPackageManager().getPackagesForUid(Binder.getCallingUid()); + private boolean isCallerAllowed(boolean allowOnlySelf) throws WrongPackageSignatureException { + return isUidAllowed(Binder.getCallingUid(), allowOnlySelf); + } + + private boolean isUidAllowed(int uid, boolean allowOnlySelf) + throws WrongPackageSignatureException { + if (android.os.Process.myUid() == uid) { + return true; + } + if (allowOnlySelf) { // barrier + return false; + } + + String[] callingPackages = getPackageManager().getPackagesForUid(uid); // is calling package allowed to use this service? for (int i = 0; i < callingPackages.length; i++) { String currentPkg = callingPackages[i]; - if (isPackageAllowed(currentPkg, allowOnlySelf)) { + if (isPackageAllowed(currentPkg)) { return true; } } @@ -248,28 +297,39 @@ public abstract class RemoteService extends Service { } /** - * Checks if packageName is a registered app for the API. + * Checks if packageName is a registered app for the API. Does not return true for own package! * * @param packageName - * @param allowOnlySelf - * allow only Keychain app itself * @return + * @throws WrongPackageSignatureException */ - private boolean isPackageAllowed(String packageName, boolean allowOnlySelf) { + private boolean isPackageAllowed(String packageName) throws WrongPackageSignatureException { Log.d(Constants.TAG, "packageName: " + packageName); - ArrayList allowedPkgs = ProviderHelper.getRegisteredApiApps(mContext); + ArrayList allowedPkgs = ProviderHelper.getRegisteredApiApps(this); Log.d(Constants.TAG, "allowed: " + allowedPkgs); // check if package is allowed to use our service - if (allowedPkgs.contains(packageName) && (!allowOnlySelf)) { + if (allowedPkgs.contains(packageName)) { Log.d(Constants.TAG, "Package is allowed! packageName: " + packageName); - return true; - } else if (Constants.PACKAGE_NAME.equals(packageName)) { - Log.d(Constants.TAG, "Package is OpenPGP Keychain! -> allowed!"); + // check package signature + byte[] currentSig; + try { + currentSig = getPackageSignature(packageName); + } catch (NameNotFoundException e) { + throw new WrongPackageSignatureException(e.getMessage()); + } - return true; + byte[] storedSig = ProviderHelper.getApiAppSignature(this, packageName); + if (Arrays.equals(currentSig, storedSig)) { + Log.d(Constants.TAG, + "Package signature is correct! (equals signature from database)"); + return true; + } else { + throw new WrongPackageSignatureException( + "PACKAGE NOT ALLOWED! Signature wrong! (Signature not equals signature from database)"); + } } return false; diff --git a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteServiceActivity.java b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteServiceActivity.java index 2c4bb4e97..bba8176b1 100644 --- a/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteServiceActivity.java +++ b/OpenPGP-Keychain/src/org/sufficientlysecure/keychain/service/remote/RemoteServiceActivity.java @@ -55,6 +55,7 @@ public class RemoteServiceActivity extends SherlockFragmentActivity { public static final String EXTRA_SECRET_KEY_ID = "secret_key_id"; // register action public static final String EXTRA_PACKAGE_NAME = "package_name"; + public static final String EXTRA_PACKAGE_SIGNATURE = "package_signature"; // select pub keys action public static final String EXTRA_SELECTED_MASTER_KEY_IDS = "master_key_ids"; public static final String EXTRA_MISSING_USER_IDS = "missing_user_ids"; @@ -110,6 +111,7 @@ public class RemoteServiceActivity extends SherlockFragmentActivity { */ if (ACTION_REGISTER.equals(action)) { final String packageName = extras.getString(EXTRA_PACKAGE_NAME); + final byte[] packageSignature = extras.getByteArray(EXTRA_PACKAGE_SIGNATURE); // Inflate a "Done"/"Cancel" custom action bar view ActionBarHelper.setDoneCancelView(getSupportActionBar(), R.string.api_register_allow, @@ -166,7 +168,7 @@ public class RemoteServiceActivity extends SherlockFragmentActivity { mSettingsFragment = (AppSettingsFragment) getSupportFragmentManager().findFragmentById( R.id.api_app_settings_fragment); - AppSettings settings = new AppSettings(packageName); + AppSettings settings = new AppSettings(packageName, packageSignature); mSettingsFragment.setAppSettings(settings); } else if (ACTION_CACHE_PASSPHRASE.equals(action)) { long secretKeyId = extras.getLong(EXTRA_SECRET_KEY_ID); -- cgit v1.2.3