KeyChain: Refactor DB handling + tests
Extract database interaction in KeyChain to its own class,
GrantsDatabase.
Add Robolectric tests for the new class, to make sure existing
functionality works and is well-tested.
This change will make it easier to test new functionality that
will be added to the GrantsDatabase.
No functional changes.
Bug: 65624467
Test: New Robolectric unit tests, also tested on-device KeyChain isn't
broken. Run with 'm -j RunKeyChainRoboTests'
Change-Id: I2add6b18e0bfa65ad7a7c4a1ffdebf386b8cdc36
diff --git a/robotests/src/com/android/keychain/internal/GrantsDatabaseTest.java b/robotests/src/com/android/keychain/internal/GrantsDatabaseTest.java
new file mode 100644
index 0000000..074b376
--- /dev/null
+++ b/robotests/src/com/android/keychain/internal/GrantsDatabaseTest.java
@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.keychain.internal;
+
+import com.android.keychain.TestConfig;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.RuntimeEnvironment;
+import org.robolectric.annotation.Config;
+
+/** Unit tests for {@link com.android.keychain.internal.GrantsDatabase}. */
+@RunWith(RobolectricTestRunner.class)
+@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
+public final class GrantsDatabaseTest {
+ private static final String DUMMY_ALIAS = "dummy_alias";
+ private static final String DUMMY_ALIAS2 = "another_dummy_alias";
+ private static final int DUMMY_UID = 1000;
+ private static final int DUMMY_UID2 = 1001;
+
+ private GrantsDatabase mGrantsDB;
+
+ @Before
+ public void setUp() {
+ mGrantsDB = new GrantsDatabase(RuntimeEnvironment.application);
+ }
+
+ @Test
+ public void testSetGrant_notMixingUIDs() {
+ mGrantsDB.setGrant(DUMMY_UID, DUMMY_ALIAS, true);
+ Assert.assertFalse(mGrantsDB.hasGrant(DUMMY_UID2, DUMMY_ALIAS));
+ }
+
+ @Test
+ public void testSetGrant_notMixingAliases() {
+ mGrantsDB.setGrant(DUMMY_UID, DUMMY_ALIAS, true);
+ Assert.assertFalse(mGrantsDB.hasGrant(DUMMY_UID, DUMMY_ALIAS2));
+ }
+
+ @Test
+ public void testSetGrantTrue() {
+ Assert.assertFalse(mGrantsDB.hasGrant(DUMMY_UID, DUMMY_ALIAS));
+ mGrantsDB.setGrant(DUMMY_UID, DUMMY_ALIAS, true);
+ Assert.assertTrue(mGrantsDB.hasGrant(DUMMY_UID, DUMMY_ALIAS));
+ }
+
+ @Test
+ public void testSetGrantFalse() {
+ mGrantsDB.setGrant(DUMMY_UID, DUMMY_ALIAS, false);
+ Assert.assertFalse(mGrantsDB.hasGrant(DUMMY_UID, DUMMY_ALIAS));
+ }
+
+ @Test
+ public void testSetGrantTrueThenFalse() {
+ mGrantsDB.setGrant(DUMMY_UID, DUMMY_ALIAS, true);
+ Assert.assertTrue(mGrantsDB.hasGrant(DUMMY_UID, DUMMY_ALIAS));
+ mGrantsDB.setGrant(DUMMY_UID, DUMMY_ALIAS, false);
+ Assert.assertFalse(mGrantsDB.hasGrant(DUMMY_UID, DUMMY_ALIAS));
+ }
+
+ @Test
+ public void testRemoveGrantsForAlias() {
+ mGrantsDB.setGrant(DUMMY_UID, DUMMY_ALIAS, true);
+ mGrantsDB.setGrant(DUMMY_UID2, DUMMY_ALIAS, true);
+ Assert.assertTrue(mGrantsDB.hasGrant(DUMMY_UID, DUMMY_ALIAS));
+ mGrantsDB.removeGrantsForAlias(DUMMY_ALIAS);
+ Assert.assertFalse(mGrantsDB.hasGrant(DUMMY_UID, DUMMY_ALIAS));
+ Assert.assertFalse(mGrantsDB.hasGrant(DUMMY_UID2, DUMMY_ALIAS));
+ }
+
+ @Test
+ public void testRemoveAllGrants() {
+ mGrantsDB.setGrant(DUMMY_UID, DUMMY_ALIAS, true);
+ mGrantsDB.setGrant(DUMMY_UID2, DUMMY_ALIAS, true);
+ mGrantsDB.setGrant(DUMMY_UID, DUMMY_ALIAS2, true);
+ mGrantsDB.removeAllGrants();
+ Assert.assertFalse(mGrantsDB.hasGrant(DUMMY_UID, DUMMY_ALIAS));
+ Assert.assertFalse(mGrantsDB.hasGrant(DUMMY_UID2, DUMMY_ALIAS));
+ Assert.assertFalse(mGrantsDB.hasGrant(DUMMY_UID, DUMMY_ALIAS2));
+ }
+}
diff --git a/src/com/android/keychain/KeyChainService.java b/src/com/android/keychain/KeyChainService.java
index f45a1e6..f4366ee 100644
--- a/src/com/android/keychain/KeyChainService.java
+++ b/src/com/android/keychain/KeyChainService.java
@@ -37,6 +37,7 @@
import android.security.KeyChain;
import android.security.KeyStore;
import android.util.Log;
+import com.android.keychain.internal.GrantsDatabase;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.security.cert.CertificateException;
@@ -54,25 +55,8 @@
private static final String TAG = "KeyChain";
- private static final String DATABASE_NAME = "grants.db";
- private static final int DATABASE_VERSION = 1;
- private static final String TABLE_GRANTS = "grants";
- private static final String GRANTS_ALIAS = "alias";
- private static final String GRANTS_GRANTEE_UID = "uid";
-
/** created in onCreate(), closed in onDestroy() */
- public DatabaseHelper mDatabaseHelper;
-
- private static final String SELECTION_COUNT_OF_MATCHING_GRANTS =
- "SELECT COUNT(*) FROM " + TABLE_GRANTS
- + " WHERE " + GRANTS_GRANTEE_UID + "=? AND " + GRANTS_ALIAS + "=?";
-
- private static final String SELECT_GRANTS_BY_UID_AND_ALIAS =
- GRANTS_GRANTEE_UID + "=? AND " + GRANTS_ALIAS + "=?";
-
- private static final String SELECTION_GRANTS_BY_UID = GRANTS_GRANTEE_UID + "=?";
-
- private static final String SELECTION_GRANTS_BY_ALIAS = GRANTS_ALIAS + "=?";
+ public GrantsDatabase mGrantsDb;
public KeyChainService() {
super(KeyChainService.class.getSimpleName());
@@ -80,14 +64,14 @@
@Override public void onCreate() {
super.onCreate();
- mDatabaseHelper = new DatabaseHelper(this);
+ mGrantsDb = new GrantsDatabase(this);
}
@Override
public void onDestroy() {
super.onDestroy();
- mDatabaseHelper.close();
- mDatabaseHelper = null;
+ mGrantsDb.destroy();
+ mGrantsDb = null;
}
private final IKeyChainService.Stub mIKeyChainService = new IKeyChainService.Stub() {
@@ -124,7 +108,7 @@
}
final int callingUid = getCallingUid();
- if (!hasGrantInternal(mDatabaseHelper.getReadableDatabase(), callingUid, alias)) {
+ if (!mGrantsDb.hasGrant(callingUid, alias)) {
throw new IllegalStateException("uid " + callingUid
+ " doesn't have permission to access the requested alias");
}
@@ -203,7 +187,7 @@
if (!Credentials.deleteAllTypesForAlias(mKeyStore, alias)) {
return false;
}
- removeGrantsForAlias(alias);
+ mGrantsDb.removeGrantsForAlias(alias);
broadcastKeychainChange();
broadcastLegacyStorageChange();
return true;
@@ -217,7 +201,7 @@
@Override public boolean reset() {
// only Settings should be able to reset
checkSystemCaller();
- removeAllGrants(mDatabaseHelper.getWritableDatabase());
+ mGrantsDb.removeAllGrants();
boolean ok = true;
synchronized (mTrustedCertificateStore) {
// delete user-installed CA certs
@@ -283,12 +267,12 @@
@Override public boolean hasGrant(int uid, String alias) {
checkSystemCaller();
- return hasGrantInternal(mDatabaseHelper.getReadableDatabase(), uid, alias);
+ return mGrantsDb.hasGrant(uid, alias);
}
@Override public void setGrant(int uid, String alias, boolean value) {
checkSystemCaller();
- setGrantInternal(mDatabaseHelper.getWritableDatabase(), uid, alias, value);
+ mGrantsDb.setGrant(uid, alias, value);
broadcastPermissionChange(uid, alias, value);
broadcastLegacyStorageChange();
}
@@ -359,60 +343,6 @@
}
};
- private boolean hasGrantInternal(final SQLiteDatabase db, final int uid, final String alias) {
- final long numMatches = DatabaseUtils.longForQuery(db, SELECTION_COUNT_OF_MATCHING_GRANTS,
- new String[]{String.valueOf(uid), alias});
- return numMatches > 0;
- }
-
- private void setGrantInternal(final SQLiteDatabase db,
- final int uid, final String alias, final boolean value) {
- if (value) {
- if (!hasGrantInternal(db, uid, alias)) {
- final ContentValues values = new ContentValues();
- values.put(GRANTS_ALIAS, alias);
- values.put(GRANTS_GRANTEE_UID, uid);
- db.insert(TABLE_GRANTS, GRANTS_ALIAS, values);
- }
- } else {
- db.delete(TABLE_GRANTS, SELECT_GRANTS_BY_UID_AND_ALIAS,
- new String[]{String.valueOf(uid), alias});
- }
- }
-
- private void removeGrantsForAlias(String alias) {
- final SQLiteDatabase db = mDatabaseHelper.getWritableDatabase();
- db.delete(TABLE_GRANTS, SELECTION_GRANTS_BY_ALIAS, new String[] {alias});
- }
-
- private void removeAllGrants(final SQLiteDatabase db) {
- db.delete(TABLE_GRANTS, null /* whereClause */, null /* whereArgs */);
- }
-
- private class DatabaseHelper extends SQLiteOpenHelper {
- public DatabaseHelper(Context context) {
- super(context, DATABASE_NAME, null /* CursorFactory */, DATABASE_VERSION);
- }
-
- @Override
- public void onCreate(final SQLiteDatabase db) {
- db.execSQL("CREATE TABLE " + TABLE_GRANTS + " ( "
- + GRANTS_ALIAS + " STRING NOT NULL, "
- + GRANTS_GRANTEE_UID + " INTEGER NOT NULL, "
- + "UNIQUE (" + GRANTS_ALIAS + "," + GRANTS_GRANTEE_UID + "))");
- }
-
- @Override
- public void onUpgrade(final SQLiteDatabase db, int oldVersion, final int newVersion) {
- Log.e(TAG, "upgrade from version " + oldVersion + " to version " + newVersion);
-
- if (oldVersion == 1) {
- // the first upgrade step goes here
- oldVersion++;
- }
- }
- }
-
@Override public IBinder onBind(Intent intent) {
if (IKeyChainService.class.getName().equals(intent.getAction())) {
return mIKeyChainService;
@@ -423,35 +353,7 @@
@Override
protected void onHandleIntent(final Intent intent) {
if (Intent.ACTION_PACKAGE_REMOVED.equals(intent.getAction())) {
- purgeOldGrants();
- }
- }
-
- private void purgeOldGrants() {
- final PackageManager packageManager = getPackageManager();
- final SQLiteDatabase db = mDatabaseHelper.getWritableDatabase();
- Cursor cursor = null;
- db.beginTransaction();
- try {
- cursor = db.query(TABLE_GRANTS,
- new String[]{GRANTS_GRANTEE_UID}, null, null, GRANTS_GRANTEE_UID, null, null);
- while (cursor.moveToNext()) {
- final int uid = cursor.getInt(0);
- final boolean packageExists = packageManager.getPackagesForUid(uid) != null;
- if (packageExists) {
- continue;
- }
- Log.d(TAG, "deleting grants for UID " + uid
- + " because its package is no longer installed");
- db.delete(TABLE_GRANTS, SELECTION_GRANTS_BY_UID,
- new String[]{Integer.toString(uid)});
- }
- db.setTransactionSuccessful();
- } finally {
- if (cursor != null) {
- cursor.close();
- }
- db.endTransaction();
+ mGrantsDb.purgeOldGrants(getPackageManager());
}
}
diff --git a/src/com/android/keychain/internal/GrantsDatabase.java b/src/com/android/keychain/internal/GrantsDatabase.java
new file mode 100644
index 0000000..591d0a5
--- /dev/null
+++ b/src/com/android/keychain/internal/GrantsDatabase.java
@@ -0,0 +1,174 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.keychain.internal;
+
+import android.content.ContentValues;
+import android.content.Context;
+import android.content.pm.PackageManager;
+import android.database.Cursor;
+import android.database.DatabaseUtils;
+import android.database.sqlite.SQLiteDatabase;
+import android.database.sqlite.SQLiteOpenHelper;
+import android.util.Log;
+
+public class GrantsDatabase {
+ private static final String TAG = "KeyChain";
+
+ private static final String DATABASE_NAME = "grants.db";
+ private static final int DATABASE_VERSION = 1;
+ private static final String TABLE_GRANTS = "grants";
+ private static final String GRANTS_ALIAS = "alias";
+ private static final String GRANTS_GRANTEE_UID = "uid";
+
+ private static final String SELECTION_COUNT_OF_MATCHING_GRANTS =
+ "SELECT COUNT(*) FROM "
+ + TABLE_GRANTS
+ + " WHERE "
+ + GRANTS_GRANTEE_UID
+ + "=? AND "
+ + GRANTS_ALIAS
+ + "=?";
+
+ private static final String SELECT_GRANTS_BY_UID_AND_ALIAS =
+ GRANTS_GRANTEE_UID + "=? AND " + GRANTS_ALIAS + "=?";
+
+ private static final String SELECTION_GRANTS_BY_UID = GRANTS_GRANTEE_UID + "=?";
+
+ private static final String SELECTION_GRANTS_BY_ALIAS = GRANTS_ALIAS + "=?";
+
+ public DatabaseHelper mDatabaseHelper;
+
+ private class DatabaseHelper extends SQLiteOpenHelper {
+ public DatabaseHelper(Context context) {
+ super(context, DATABASE_NAME, null /* CursorFactory */, DATABASE_VERSION);
+ }
+
+ @Override
+ public void onCreate(final SQLiteDatabase db) {
+ db.execSQL(
+ "CREATE TABLE "
+ + TABLE_GRANTS
+ + " ( "
+ + GRANTS_ALIAS
+ + " STRING NOT NULL, "
+ + GRANTS_GRANTEE_UID
+ + " INTEGER NOT NULL, "
+ + "UNIQUE ("
+ + GRANTS_ALIAS
+ + ","
+ + GRANTS_GRANTEE_UID
+ + "))");
+ }
+
+ @Override
+ public void onUpgrade(final SQLiteDatabase db, int oldVersion, final int newVersion) {
+ Log.e(TAG, "upgrade from version " + oldVersion + " to version " + newVersion);
+
+ if (oldVersion == 1) {
+ // the first upgrade step goes here
+ oldVersion++;
+ }
+ }
+ }
+
+ public GrantsDatabase(Context context) {
+ mDatabaseHelper = new DatabaseHelper(context);
+ }
+
+ public void destroy() {
+ mDatabaseHelper.close();
+ mDatabaseHelper = null;
+ }
+
+ boolean hasGrantInternal(final SQLiteDatabase db, final int uid, final String alias) {
+ final long numMatches =
+ DatabaseUtils.longForQuery(
+ db,
+ SELECTION_COUNT_OF_MATCHING_GRANTS,
+ new String[] {String.valueOf(uid), alias});
+ return numMatches > 0;
+ }
+
+ public boolean hasGrant(final int uid, final String alias) {
+ final SQLiteDatabase db = mDatabaseHelper.getWritableDatabase();
+ return hasGrantInternal(db, uid, alias);
+ }
+
+ public void setGrant(final int uid, final String alias, final boolean value) {
+ final SQLiteDatabase db = mDatabaseHelper.getWritableDatabase();
+ if (value) {
+ if (!hasGrantInternal(db, uid, alias)) {
+ final ContentValues values = new ContentValues();
+ values.put(GRANTS_ALIAS, alias);
+ values.put(GRANTS_GRANTEE_UID, uid);
+ db.insert(TABLE_GRANTS, GRANTS_ALIAS, values);
+ }
+ } else {
+ db.delete(
+ TABLE_GRANTS,
+ SELECT_GRANTS_BY_UID_AND_ALIAS,
+ new String[] {String.valueOf(uid), alias});
+ }
+ }
+
+ public void removeGrantsForAlias(String alias) {
+ final SQLiteDatabase db = mDatabaseHelper.getWritableDatabase();
+ db.delete(TABLE_GRANTS, SELECTION_GRANTS_BY_ALIAS, new String[] {alias});
+ }
+
+ public void removeAllGrants() {
+ final SQLiteDatabase db = mDatabaseHelper.getWritableDatabase();
+ db.delete(TABLE_GRANTS, null /* whereClause */, null /* whereArgs */);
+ }
+
+ public void purgeOldGrants(PackageManager pm) {
+ final SQLiteDatabase db = mDatabaseHelper.getWritableDatabase();
+ Cursor cursor = null;
+ db.beginTransaction();
+ try {
+ cursor =
+ db.query(
+ TABLE_GRANTS,
+ new String[] {GRANTS_GRANTEE_UID},
+ null,
+ null,
+ GRANTS_GRANTEE_UID,
+ null,
+ null);
+ while (cursor.moveToNext()) {
+ final int uid = cursor.getInt(0);
+ final boolean packageExists = pm.getPackagesForUid(uid) != null;
+ if (packageExists) {
+ continue;
+ }
+ Log.d(TAG, String.format(
+ "deleting grants for UID %d because its package is no longer installed",
+ uid));
+ db.delete(
+ TABLE_GRANTS,
+ SELECTION_GRANTS_BY_UID,
+ new String[] {Integer.toString(uid)});
+ }
+ db.setTransactionSuccessful();
+ } finally {
+ if (cursor != null) {
+ cursor.close();
+ }
+ db.endTransaction();
+ }
+ }
+}