From 5e9de4447c95cd9ed332ec012b4d73ad7a8ef8b2 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 6 May 2016 12:54:35 +0200 Subject: external-provider: only allow permission check for caller package names --- .../keychain/remote/ApiPermissionHelper.java | 51 ++++++++++------------ .../keychain/remote/KeychainExternalProvider.java | 32 +++++++++++--- 2 files changed, 47 insertions(+), 36 deletions(-) (limited to 'OpenKeychain') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPermissionHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPermissionHelper.java index 3af8e70dd..47ecdb21f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPermissionHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPermissionHelper.java @@ -18,6 +18,10 @@ package org.sufficientlysecure.keychain.remote; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.Arrays; + import android.annotation.SuppressLint; import android.app.PendingIntent; import android.content.Context; @@ -37,11 +41,6 @@ import org.sufficientlysecure.keychain.provider.ApiDataAccessObject; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.util.Log; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; - /** * Abstract service class for remote APIs that handle app registration and user input. @@ -234,35 +233,29 @@ public class ApiPermissionHelper { private boolean isPackageAllowed(String packageName) throws WrongPackageCertificateException { Log.d(Constants.TAG, "isPackageAllowed packageName: " + packageName); - ArrayList allowedPkgs = mApiDao.getRegisteredApiApps(); - Log.d(Constants.TAG, "allowed: " + allowedPkgs); + byte[] storedPackageCert = mApiDao.getApiAppCertificate(packageName); - // check if package is allowed to use our service - if (allowedPkgs.contains(packageName)) { - Log.d(Constants.TAG, "Package is allowed! packageName: " + packageName); + boolean isKnownPackage = storedPackageCert != null; + if (!isKnownPackage) { + Log.d(Constants.TAG, "Package is NOT allowed! packageName: " + packageName); + return false; + } + Log.d(Constants.TAG, "Package is allowed! packageName: " + packageName); - // check package signature - byte[] currentCert; - try { - currentCert = getPackageCertificate(packageName); - } catch (NameNotFoundException e) { - throw new WrongPackageCertificateException(e.getMessage()); - } + byte[] currentPackageCert; + try { + currentPackageCert = getPackageCertificate(packageName); + } catch (NameNotFoundException e) { + throw new WrongPackageCertificateException(e.getMessage()); + } - byte[] storedCert = mApiDao.getApiAppCertificate(packageName); - if (Arrays.equals(currentCert, storedCert)) { - Log.d(Constants.TAG, - "Package certificate is correct! (equals certificate from database)"); - return true; - } else { - throw new WrongPackageCertificateException( - "PACKAGE NOT ALLOWED! Certificate wrong! (Certificate not " + - "equals certificate from database)"); - } + boolean packageCertMatchesStored = Arrays.equals(currentPackageCert, storedPackageCert); + if (packageCertMatchesStored) { + Log.d(Constants.TAG,"Package certificate matches expected."); + return true; } - Log.d(Constants.TAG, "Package is NOT allowed! packageName: " + packageName); - return false; + throw new WrongPackageCertificateException("PACKAGE NOT ALLOWED DUE TO CERTIFICATE MISMATCH!"); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java index 7eb5d558f..455857f00 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java @@ -24,12 +24,14 @@ import java.util.HashMap; import android.content.ContentProvider; import android.content.ContentValues; +import android.content.Context; import android.content.UriMatcher; import android.database.Cursor; import android.database.DatabaseUtils; import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteQueryBuilder; import android.net.Uri; +import android.os.Binder; import android.support.annotation.NonNull; import android.text.TextUtils; @@ -206,16 +208,13 @@ public class KeychainExternalProvider extends ContentProvider implements SimpleC break; } - case API_APPS: { - qb.setTables(Tables.API_APPS); - - break; - } - case API_APPS_BY_PACKAGE_NAME: { + String requestedPackageName = uri.getLastPathSegment(); + checkIfPackageBelongsToCaller(getContext(), requestedPackageName); + qb.setTables(Tables.API_APPS); qb.appendWhere(ApiApps.PACKAGE_NAME + " = "); - qb.appendWhereEscapeString(uri.getLastPathSegment()); + qb.appendWhereEscapeString(requestedPackageName); break; } @@ -248,6 +247,25 @@ public class KeychainExternalProvider extends ContentProvider implements SimpleC return cursor; } + private void checkIfPackageBelongsToCaller(Context context, String requestedPackageName) { + int callerUid = Binder.getCallingUid(); + String[] callerPackageNames = context.getPackageManager().getPackagesForUid(callerUid); + if (callerPackageNames == null) { + throw new IllegalStateException("Failed to retrieve caller package name, this is an error!"); + } + + boolean packageBelongsToCaller = false; + for (String p : callerPackageNames) { + if (p.equals(requestedPackageName)) { + packageBelongsToCaller = true; + break; + } + } + if (!packageBelongsToCaller) { + throw new SecurityException("ExternalProvider may only check status of caller package!"); + } + } + @Override public Uri insert(@NonNull Uri uri, ContentValues values) { throw new UnsupportedOperationException(); -- cgit v1.2.3