cthrace/open-keychain
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Store secret keys in private storage instead of database

Browse Source
This commit is contained in:
Vincent Breitmoser
2018-06-15 14:34:38 +02:00
parent a3fd1609df
commit 59c9f52e85
9 changed files with 102 additions and 140 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
View File

@@ -419,7 +419,6 @@ public abstract class OperationResult implements Parcelable {
// import secret
MSG_IS(LogLevel.START, R.string.msg_is),
MSG_IS_BAD_TYPE_PUBLIC (LogLevel.WARN, R.string.msg_is_bad_type_public),
MSG_IS_DB_EXCEPTION (LogLevel.DEBUG, R.string.msg_is_db_exception),
MSG_IS_ERROR_IO_EXC(LogLevel.DEBUG, R.string.msg_is_error_io_exc),
MSG_IS_MERGE_PUBLIC (LogLevel.DEBUG, R.string.msg_is_merge_public),
MSG_IS_MERGE_SECRET (LogLevel.DEBUG, R.string.msg_is_merge_secret),

22
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeyRepository.java
View File

@@ -56,24 +56,28 @@ public class KeyRepository {
final ContentResolver contentResolver;
final LocalPublicKeyStorage mLocalPublicKeyStorage;
final LocalSecretKeyStorage localSecretKeyStorage;
OperationLog mLog;
int mIndent;
public static KeyRepository create(Context context) {
ContentResolver contentResolver = context.getContentResolver();
LocalPublicKeyStorage localPublicKeyStorage = LocalPublicKeyStorage.getInstance(context);
LocalSecretKeyStorage localSecretKeyStorage = LocalSecretKeyStorage.getInstance(context);
return new KeyRepository(contentResolver, localPublicKeyStorage);
return new KeyRepository(contentResolver, localPublicKeyStorage, localSecretKeyStorage);
}
private KeyRepository(ContentResolver contentResolver, LocalPublicKeyStorage localPublicKeyStorage) {
this(contentResolver, localPublicKeyStorage, new OperationLog(), 0);
private KeyRepository(ContentResolver contentResolver, LocalPublicKeyStorage localPublicKeyStorage,
LocalSecretKeyStorage localSecretKeyStorage) {
this(contentResolver, localPublicKeyStorage, localSecretKeyStorage, new OperationLog(), 0);
}
KeyRepository(ContentResolver contentResolver, LocalPublicKeyStorage localPublicKeyStorage,
OperationLog log, int indent) {
LocalSecretKeyStorage localSecretKeyStorage, OperationLog log, int indent) {
this.contentResolver = contentResolver;
mLocalPublicKeyStorage = localPublicKeyStorage;
this.localSecretKeyStorage = localSecretKeyStorage;
mIndent = indent;
mLog = log;
}
@@ -326,14 +330,12 @@ public class KeyRepository {
}
public final byte[] loadSecretKeyRingData(long masterKeyId) throws NotFoundException {
byte[] data = (byte[]) getGenericDataOrNull(KeychainContract.KeyRingData.buildSecretKeyRingUri(masterKeyId),
KeyRingData.KEY_RING_DATA, FIELD_TYPE_BLOB);
if (data == null) {
try {
return localSecretKeyStorage.readSecretKey(masterKeyId);
} catch (IOException e) {
Timber.e(e, "Error reading public key from storage!");
throw new NotFoundException();
}
return data;
}
public static class NotFoundException extends Exception {

47
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeyWritableRepository.java
View File

@@ -85,28 +85,31 @@ public class KeyWritableRepository extends KeyRepository {
private final Context context;
private final LastUpdateInteractor lastUpdateInteractor;
private DatabaseNotifyManager databaseNotifyManager;
private final DatabaseNotifyManager databaseNotifyManager;
public static KeyWritableRepository create(Context context) {
LocalPublicKeyStorage localPublicKeyStorage = LocalPublicKeyStorage.getInstance(context);
LastUpdateInteractor lastUpdateInteractor = LastUpdateInteractor.create(context);
LocalSecretKeyStorage localSecretKeyStorage = LocalSecretKeyStorage.getInstance(context);
DatabaseNotifyManager databaseNotifyManager = DatabaseNotifyManager.create(context);
return new KeyWritableRepository(context, localPublicKeyStorage, lastUpdateInteractor,
databaseNotifyManager);
}
LastUpdateInteractor lastUpdateInteractor = LastUpdateInteractor.create(context);
return new KeyWritableRepository(context, localPublicKeyStorage, localSecretKeyStorage, databaseNotifyManager,
lastUpdateInteractor);
}
@VisibleForTesting
KeyWritableRepository(Context context, LocalPublicKeyStorage localPublicKeyStorage,
LastUpdateInteractor lastUpdateInteractor, DatabaseNotifyManager databaseNotifyManager) {
this(context, localPublicKeyStorage, lastUpdateInteractor, new OperationLog(), 0,
databaseNotifyManager);
KeyWritableRepository(Context context,
LocalPublicKeyStorage localPublicKeyStorage,
LocalSecretKeyStorage localSecretKeyStorage,
DatabaseNotifyManager databaseNotifyManager,
LastUpdateInteractor lastUpdateInteractor) {
this(context, localPublicKeyStorage, localSecretKeyStorage, databaseNotifyManager, lastUpdateInteractor,
new OperationLog(), 0);
}
private KeyWritableRepository(Context context, LocalPublicKeyStorage localPublicKeyStorage,
LastUpdateInteractor lastUpdateInteractor, OperationLog log, int indent,
DatabaseNotifyManager databaseNotifyManager) {
super(context.getContentResolver(), localPublicKeyStorage, log, indent);
LocalSecretKeyStorage localSecretKeyStorage, DatabaseNotifyManager databaseNotifyManager,
LastUpdateInteractor lastUpdateInteractor, OperationLog log, int indent) {
super(context.getContentResolver(), localPublicKeyStorage, localSecretKeyStorage, log, indent);
this.context = context;
this.databaseNotifyManager = databaseNotifyManager;
@@ -601,21 +604,15 @@ public class KeyWritableRepository extends KeyRepository {
operations.add(ContentProviderOperation.newInsert(uri).withValues(values).build());
}
private Uri writeSecretKeyRing(CanonicalizedSecretKeyRing keyRing, long masterKeyId) throws IOException {
private void writeSecretKeyRing(CanonicalizedSecretKeyRing keyRing, long masterKeyId) throws IOException {
byte[] encodedKey = keyRing.getEncoded();
ContentValues values = new ContentValues();
values.put(KeyRingData.MASTER_KEY_ID, masterKeyId);
values.put(KeyRingData.KEY_RING_DATA, encodedKey);
// insert new version of this keyRing
Uri uri = KeyRingData.buildSecretKeyRingUri(masterKeyId);
return contentResolver.insert(uri, values);
localSecretKeyStorage.writeSecretKey(masterKeyId, encodedKey);
}
public boolean deleteKeyRing(long masterKeyId) {
try {
mLocalPublicKeyStorage.deletePublicKey(masterKeyId);
localSecretKeyStorage.deleteSecretKey(masterKeyId);
} catch (IOException e) {
Timber.e(e, "Could not delete file!");
return false;
@@ -683,11 +680,7 @@ public class KeyWritableRepository extends KeyRepository {
// save secret keyring
try {
Uri insertedUri = writeSecretKeyRing(keyRing, masterKeyId);
if (insertedUri == null) {
log(LogType.MSG_IS_DB_EXCEPTION);
return SaveKeyringResult.RESULT_ERROR;
}
writeSecretKeyRing(keyRing, masterKeyId);
} catch (IOException e) {
Timber.e(e, "Failed to encode key!");
log(LogType.MSG_IS_ERROR_IO_EXC);

14
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainContract.java
View File

@@ -136,7 +136,6 @@ public class KeychainContract {
public static final String PATH_BY_SIGNER = "signer";
public static final String PATH_PUBLIC = "public";
public static final String PATH_SECRET = "secret";
public static final String PATH_USER_IDS = "user_ids";
public static final String PATH_LINKED_IDS = "linked_ids";
public static final String PATH_KEYS = "keys";
@@ -239,19 +238,6 @@ public class KeychainContract {
public static Uri buildPublicKeyRingUri(Uri uri) {
return CONTENT_URI.buildUpon().appendPath(uri.getPathSegments().get(1)).appendPath(PATH_PUBLIC).build();
}
public static Uri buildSecretKeyRingUri() {
return CONTENT_URI.buildUpon().appendPath(PATH_SECRET).build();
}
public static Uri buildSecretKeyRingUri(long masterKeyId) {
return CONTENT_URI.buildUpon().appendPath(Long.toString(masterKeyId)).appendPath(PATH_SECRET).build();
}
public static Uri buildSecretKeyRingUri(Uri uri) {
return CONTENT_URI.buildUpon().appendPath(uri.getPathSegments().get(1)).appendPath(PATH_SECRET).build();
}
}
public static class Keys implements KeysColumns, BaseColumns {

94
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java
View File

@@ -24,7 +24,7 @@ import java.io.FileOutputStream;
import java.io.IOException;
import android.content.Context;
import android.database.SQLException;
import android.database.Cursor;
import android.database.sqlite.SQLiteDatabase;
import android.database.sqlite.SQLiteException;
import android.database.sqlite.SQLiteOpenHelper;
@@ -55,12 +55,11 @@ import timber.log.Timber;
*/
public class KeychainDatabase extends SQLiteOpenHelper {
private static final String DATABASE_NAME = "openkeychain.db";
private static final int DATABASE_VERSION = 25;
private static final int DATABASE_VERSION = 26;
private Context mContext;
public interface Tables {
String KEY_RINGS_PUBLIC = "keyrings_public";
String KEY_RINGS_SECRET = "keyrings_secret";
String KEYS = "keys";
String UPDATED_KEYS = "updated_keys";
String KEY_SIGNATURES = "key_signatures";
@@ -78,14 +77,6 @@ public class KeychainDatabase extends SQLiteOpenHelper {
+ KeyRingsColumns.KEY_RING_DATA + " BLOB"
+ ")";
private static final String CREATE_KEYRINGS_SECRET =
"CREATE TABLE IF NOT EXISTS keyrings_secret ("
+ KeyRingsColumns.MASTER_KEY_ID + " INTEGER PRIMARY KEY,"
+ KeyRingsColumns.KEY_RING_DATA + " BLOB, "
+ "FOREIGN KEY(" + KeyRingsColumns.MASTER_KEY_ID + ") "
+ "REFERENCES keyrings_public(" + KeyRingsColumns.MASTER_KEY_ID + ") ON DELETE CASCADE"
+ ")";
private static final String CREATE_KEYS =
"CREATE TABLE IF NOT EXISTS " + Tables.KEYS + " ("
+ KeysColumns.MASTER_KEY_ID + " INTEGER, "
@@ -222,7 +213,6 @@ public class KeychainDatabase extends SQLiteOpenHelper {
Timber.w("Creating database...");
db.execSQL(CREATE_KEYRINGS_PUBLIC);
db.execSQL(CREATE_KEYRINGS_SECRET);
db.execSQL(CREATE_KEYS);
db.execSQL(CREATE_USER_PACKETS);
db.execSQL(CREATE_CERTS);
@@ -302,37 +292,37 @@ public class KeychainDatabase extends SQLiteOpenHelper {
db.execSQL("DROP TABLE IF EXISTS certs");
db.execSQL("DROP TABLE IF EXISTS user_ids");
db.execSQL("CREATE TABLE IF NOT EXISTS user_packets("
+ "master_key_id INTEGER, "
+ "type INT, "
+ "user_id TEXT, "
+ "attribute_data BLOB, "
+ "master_key_id INTEGER, "
+ "type INT, "
+ "user_id TEXT, "
+ "attribute_data BLOB, "
+ "is_primary INTEGER, "
+ "is_revoked INTEGER, "
+ "rank INTEGER, "
+ "is_primary INTEGER, "
+ "is_revoked INTEGER, "
+ "rank INTEGER, "
+ "PRIMARY KEY(master_key_id, rank), "
+ "FOREIGN KEY(master_key_id) REFERENCES "
+ "PRIMARY KEY(master_key_id, rank), "
+ "FOREIGN KEY(master_key_id) REFERENCES "
+ "keyrings_public(master_key_id) ON DELETE CASCADE"
+ ")");
+ ")");
db.execSQL("CREATE TABLE IF NOT EXISTS certs("
+ "master_key_id INTEGER,"
+ "rank INTEGER, " // rank of certified uid
+ "master_key_id INTEGER,"
+ "rank INTEGER, " // rank of certified uid
+ "key_id_certifier INTEGER, " // certifying key
+ "type INTEGER, "
+ "verified INTEGER, "
+ "creation INTEGER, "
+ "key_id_certifier INTEGER, " // certifying key
+ "type INTEGER, "
+ "verified INTEGER, "
+ "creation INTEGER, "
+ "data BLOB, "
+ "data BLOB, "
+ "PRIMARY KEY(master_key_id, rank, "
+ "PRIMARY KEY(master_key_id, rank, "
+ "key_id_certifier), "
+ "FOREIGN KEY(master_key_id) REFERENCES "
+ "FOREIGN KEY(master_key_id) REFERENCES "
+ "keyrings_public(master_key_id) ON DELETE CASCADE,"
+ "FOREIGN KEY(master_key_id, rank) REFERENCES "
+ "FOREIGN KEY(master_key_id, rank) REFERENCES "
+ "user_packets(master_key_id, rank) ON DELETE CASCADE"
+ ")");
+ ")");
case 9:
// do nothing here, just consolidate
case 10:
@@ -380,11 +370,11 @@ public class KeychainDatabase extends SQLiteOpenHelper {
}
*/
case 20:
db.execSQL(
"CREATE TABLE IF NOT EXISTS overridden_warnings ("
+ "_id INTEGER PRIMARY KEY AUTOINCREMENT, "
+ "identifier TEXT NOT NULL UNIQUE "
+ ")");
db.execSQL(
"CREATE TABLE IF NOT EXISTS overridden_warnings ("
+ "_id INTEGER PRIMARY KEY AUTOINCREMENT, "
+ "identifier TEXT NOT NULL UNIQUE "
+ ")");
case 21:
try {
@@ -403,7 +393,7 @@ public class KeychainDatabase extends SQLiteOpenHelper {
+ "master_key_id INTEGER NULL, "
+ "PRIMARY KEY(package_name, identifier), "
+ "FOREIGN KEY(package_name) REFERENCES api_apps(package_name) ON DELETE CASCADE"
+ ")");
+ ")");
case 23:
db.execSQL("CREATE TABLE IF NOT EXISTS key_signatures ("
@@ -413,7 +403,7 @@ public class KeychainDatabase extends SQLiteOpenHelper {
+ "FOREIGN KEY(master_key_id) REFERENCES keyrings_public(master_key_id) ON DELETE CASCADE"
+ ")");
case 24:
case 24: {
try {
db.beginTransaction();
db.execSQL("ALTER TABLE api_autocrypt_peers RENAME TO tmp");
@@ -456,9 +446,33 @@ public class KeychainDatabase extends SQLiteOpenHelper {
db.execSQL("CREATE INDEX IF NOT EXISTS uids_by_email ON user_packets (email);");
db.execSQL("DROP INDEX keys_by_rank");
db.execSQL("CREATE INDEX keys_by_rank ON keys(rank, master_key_id);");
}
case 25: {
try {
migrateSecretKeysFromDbToLocalStorage(db);
} catch (IOException e) {
throw new IllegalStateException("Error migrating secret keys! This is bad!!");
}
}
}
}
private void migrateSecretKeysFromDbToLocalStorage(SQLiteDatabase db) throws IOException {
LocalSecretKeyStorage localSecretKeyStorage = LocalSecretKeyStorage.getInstance(mContext);
Cursor cursor = db.rawQuery("SELECT master_key_id, key_ring_data FROM keyrings_secret", null);
while (cursor.moveToNext()) {
long masterKeyId = cursor.getLong(0);
byte[] secretKeyBlob = cursor.getBlob(1);
localSecretKeyStorage.writeSecretKey(masterKeyId, secretKeyBlob);
}
cursor.close();
// we'll keep this around for now, but make sure to delete when migration looks ok!!
// db.execSQL("DROP TABLE keyrings_secret");
}
@Override
public void onDowngrade(SQLiteDatabase db, int oldVersion, int newVersion) {
// Downgrade is ok for the debug version, makes it easier to work with branches

51
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java
View File

@@ -60,14 +60,12 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe
private static final int KEY_RINGS_UNIFIED = 101;
private static final int KEY_RINGS_PUBLIC = 102;
private static final int KEY_RINGS_SECRET = 103;
private static final int KEY_RINGS_USER_IDS = 104;
private static final int KEY_RING_UNIFIED = 200;
private static final int KEY_RING_KEYS = 201;
private static final int KEY_RING_USER_IDS = 202;
private static final int KEY_RING_PUBLIC = 203;
private static final int KEY_RING_SECRET = 204;
private static final int KEY_RING_CERTS = 205;
private static final int KEY_RING_CERTS_SPECIFIC = 206;
private static final int KEY_RING_LINKED_IDS = 207;
@@ -118,9 +116,6 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe
matcher.addURI(authority, KeychainContract.BASE_KEY_RINGS
+ "/" + KeychainContract.PATH_PUBLIC,
KEY_RINGS_PUBLIC);
matcher.addURI(authority, KeychainContract.BASE_KEY_RINGS
+ "/" + KeychainContract.PATH_SECRET,
KEY_RINGS_SECRET);
matcher.addURI(authority, KeychainContract.BASE_KEY_RINGS
+ "/" + KeychainContract.PATH_USER_IDS,
KEY_RINGS_USER_IDS);
@@ -180,9 +175,6 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe
matcher.addURI(authority, KeychainContract.BASE_KEY_RINGS + "/*/"
+ KeychainContract.PATH_PUBLIC,
KEY_RING_PUBLIC);
matcher.addURI(authority, KeychainContract.BASE_KEY_RINGS + "/*/"
+ KeychainContract.PATH_SECRET,
KEY_RING_SECRET);
matcher.addURI(authority, KeychainContract.BASE_KEY_RINGS + "/*/"
+ KeychainContract.PATH_CERTS,
KEY_RING_CERTS);
@@ -268,9 +260,6 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe
case KEY_RING_USER_IDS:
return UserPackets.CONTENT_TYPE;
case KEY_RING_SECRET:
return KeyRings.CONTENT_ITEM_TYPE;
case UPDATED_KEYS:
return UpdatedKeys.CONTENT_TYPE;
@@ -349,9 +338,10 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe
projectionMap.put(KeyRings.VERIFIED, Tables.CERTS + "." + Certs.VERIFIED);
projectionMap.put(KeyRings.HAS_SECRET, Tables.KEYS + "." + KeyRings.HAS_SECRET);
projectionMap.put(KeyRings.HAS_ANY_SECRET,
"(EXISTS (SELECT * FROM " + Tables.KEY_RINGS_SECRET + " WHERE "
+ Tables.KEYS + "." + Keys.MASTER_KEY_ID + " = "
+ Tables.KEY_RINGS_SECRET + "." + KeyRingData.MASTER_KEY_ID
"(EXISTS (SELECT * FROM " + Tables.KEYS + " AS k WHERE "
+ "k." + Keys.HAS_SECRET + " != 0"
+ " AND k." + Keys.MASTER_KEY_ID + " = "
+ Tables.KEYS + "." + KeyRingData.MASTER_KEY_ID
+ ")) AS " + KeyRings.HAS_ANY_SECRET);
projectionMap.put(KeyRings.HAS_ENCRYPT,
"kE." + Keys.KEY_ID + " AS " + KeyRings.HAS_ENCRYPT);
@@ -641,24 +631,6 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe
break;
}
case KEY_RINGS_SECRET:
case KEY_RING_SECRET: {
HashMap<String, String> projectionMap = new HashMap<>();
projectionMap.put(KeyRingData._ID, Tables.KEY_RINGS_SECRET + ".oid AS _id");
projectionMap.put(KeyRingData.MASTER_KEY_ID, KeyRingData.MASTER_KEY_ID);
projectionMap.put(KeyRingData.KEY_RING_DATA, KeyRingData.KEY_RING_DATA);
qb.setProjectionMap(projectionMap);
qb.setTables(Tables.KEY_RINGS_SECRET);
if(match == KEY_RING_SECRET) {
qb.appendWhere(KeyRings.MASTER_KEY_ID + " = ");
qb.appendWhereEscapeString(uri.getPathSegments().get(1));
}
break;
}
case KEY_RING_CERTS:
case KEY_RING_CERTS_SPECIFIC:
case KEY_RING_LINKED_ID_CERTS: {
@@ -885,11 +857,6 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe
keyId = values.getAsLong(KeyRings.MASTER_KEY_ID);
break;
}
case KEY_RING_SECRET: {
db.insertOrThrow(Tables.KEY_RINGS_SECRET, null, values);
keyId = values.getAsLong(KeyRings.MASTER_KEY_ID);
break;
}
case KEY_RING_KEYS: {
db.insertOrThrow(Tables.KEYS, null, values);
keyId = values.getAsLong(Keys.MASTER_KEY_ID);
@@ -982,6 +949,7 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe
count = db.delete(Tables.KEY_RINGS_PUBLIC, null, null);
break;
}
case KEY_RING_PUBLIC: {
@SuppressWarnings("ConstantConditions") // ensured by uriMatcher above
String selection = KeyRings.MASTER_KEY_ID + " = " + uri.getPathSegments().get(1);
@@ -992,15 +960,6 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe
count = db.delete(Tables.KEY_RINGS_PUBLIC, selection, selectionArgs);
break;
}
case KEY_RING_SECRET: {
@SuppressWarnings("ConstantConditions") // ensured by uriMatcher above
String selection = KeyRings.MASTER_KEY_ID + " = " + uri.getPathSegments().get(1);
if (!TextUtils.isEmpty(additionalSelection)) {
selection += " AND (" + additionalSelection + ")";
}
count = db.delete(Tables.KEY_RINGS_SECRET, selection, selectionArgs);
break;
}
case AUTOCRYPT_PEERS_BY_PACKAGE_NAME_AND_TRUST_ID: {
String packageName = uri.getPathSegments().get(2);

0
OpenKeychain/src/main/sqldelight/org/sufficientlysecure/keychain/ApiAllowedKeys.sq Normal file
View File

8
OpenKeychain/src/main/sqldelight/org/sufficientlysecure/keychain/ApiApps.sq Normal file
View File

@@ -0,0 +1,8 @@
CREATE TABLE IF NOT EXISTS api_apps (
_id INTEGER PRIMARY KEY AUTOINCREMENT,
package_name TEXT NOT NULL UNIQUE,
package_signature BLOB
);
getAllowedKeys:
SELECT

5
OpenKeychain/src/test/java/org/sufficientlysecure/keychain/provider/InteropTest.java
View File

@@ -244,8 +244,9 @@ public class InteropTest {
KeyWritableRepository helper = new KeyWritableRepository(RuntimeEnvironment.application,
LocalPublicKeyStorage.getInstance(RuntimeEnvironment.application),
LastUpdateInteractor.create(RuntimeEnvironment.application),
DatabaseNotifyManager.create(RuntimeEnvironment.application)) {
LocalSecretKeyStorage.getInstance(RuntimeEnvironment.application),
DatabaseNotifyManager.create(RuntimeEnvironment.application),
LastUpdateInteractor.create(RuntimeEnvironment.application)) {
@Override
public CachedPublicKeyRing getCachedPublicKeyRing(Uri queryUri) throws PgpKeyNotFoundException {