From 7d2b356d1cb8f637053643840cf9a37c96aab961 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 17 Jan 2018 14:41:10 +0100 Subject: [PATCH] Further small optimizations to Autocrypt logic --- .../AutocryptPeerDataAccessObject.java | 159 +++++++++--------- .../remote/KeychainExternalProvider.java | 45 ++--- 2 files changed, 102 insertions(+), 102 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/AutocryptPeerDataAccessObject.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/AutocryptPeerDataAccessObject.java index 7fa579894..dd6f3d05e 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/AutocryptPeerDataAccessObject.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/AutocryptPeerDataAccessObject.java @@ -18,15 +18,16 @@ package org.sufficientlysecure.keychain.provider; +import java.util.ArrayList; import java.util.Date; -import java.util.HashMap; -import java.util.Map; +import java.util.List; import android.content.ContentResolver; import android.content.ContentValues; import android.content.Context; import android.database.Cursor; import android.net.Uri; +import android.support.annotation.Nullable; import android.text.format.DateUtils; import org.sufficientlysecure.keychain.provider.KeychainContract.ApiAutocryptPeer; @@ -198,85 +199,14 @@ public class AutocryptPeerDataAccessObject { queryInterface.delete(ApiAutocryptPeer.buildByPackageNameAndAutocryptId(packageName, autocryptId), null, null); } - public Map getAutocryptState(String... autocryptIds) { - /* -Determine if encryption is possible -If there is no peers[to-addr], then set ui-recommendation to disable, and terminate. + public List determineAutocryptRecommendations(String... autocryptIds) { + List result = new ArrayList<>(autocryptIds.length); -For the purposes of the rest of this recommendation, if either public_key or gossip_key is revoked, expired, or otherwise known to be unusable for encryption, then treat that key as though it were null (not present). - -If both public_key and gossip_key are null, then set ui-recommendation to disable and terminate. - -Otherwise, we derive the recommendation using a two-phase algorithm. The first phase computes the preliminary-recommendation. - -Preliminary Recommendation -If public_key is null, then set target-keys[to-addr] to gossip_key and set preliminary-recommendation to discourage and skip to the Deciding to Encrypt by Default. - -Otherwise, set target-keys[to-addr] to public_key. - -If autocrypt_timestamp is more than 35 days older than last_seen, set preliminary-recommendation to discourage. - -Otherwise, set preliminary-recommendation to available. - */ - - Map result = new HashMap<>(); - StringBuilder selection = new StringBuilder(ApiAutocryptPeer.IDENTIFIER + " IN (?"); - for (int i = 1; i < autocryptIds.length; i++) { - selection.append(",?"); - } - selection.append(")"); - - Cursor cursor = queryInterface.query(ApiAutocryptPeer.buildByPackageName(packageName), - PROJECTION_AUTOCRYPT_QUERY, selection.toString(), autocryptIds, null); + Cursor cursor = queryAutocryptPeerData(autocryptIds); try { while (cursor.moveToNext()) { - String autocryptId = cursor.getString(INDEX_IDENTIFIER); - - boolean hasKey = !cursor.isNull(INDEX_MASTER_KEY_ID); - boolean isRevoked = cursor.getInt(INDEX_KEY_IS_REVOKED) != 0; - boolean isExpired = cursor.getInt(INDEX_KEY_IS_EXPIRED) != 0; - if (hasKey && !isRevoked && !isExpired) { - long masterKeyId = cursor.getLong(INDEX_MASTER_KEY_ID); - long lastSeen = cursor.getLong(INDEX_LAST_SEEN); - long lastSeenKey = cursor.getLong(INDEX_LAST_SEEN_KEY); - boolean isVerified = cursor.getInt(INDEX_KEY_IS_VERIFIED) != 0; - if (lastSeenKey < (lastSeen - AUTOCRYPT_DISCOURAGE_THRESHOLD_MILLIS)) { - AutocryptPeerStateResult peerResult = new AutocryptPeerStateResult( - AutocryptState.DISCOURAGED_OLD, masterKeyId, isVerified); - result.put(autocryptId, peerResult); - continue; - } - - boolean isMutual = cursor.getInt(INDEX_STATE) != 0; - if (isMutual) { - AutocryptPeerStateResult peerResult = new AutocryptPeerStateResult( - AutocryptState.MUTUAL, masterKeyId, isVerified); - result.put(autocryptId, peerResult); - continue; - } else { - AutocryptPeerStateResult peerResult = new AutocryptPeerStateResult( - AutocryptState.AVAILABLE, masterKeyId, isVerified); - result.put(autocryptId, peerResult); - continue; - } - } - - boolean gossipHasKey = !cursor.isNull(INDEX_GOSSIP_MASTER_KEY_ID); - boolean gossipIsRevoked = cursor.getInt(INDEX_GOSSIP_KEY_IS_REVOKED) != 0; - boolean gossipIsExpired = cursor.getInt(INDEX_GOSSIP_KEY_IS_EXPIRED) != 0; - boolean isVerified = cursor.getInt(INDEX_GOSSIP_KEY_IS_VERIFIED) != 0; - if (gossipHasKey && !gossipIsRevoked && !gossipIsExpired) { - long masterKeyId = cursor.getLong(INDEX_GOSSIP_MASTER_KEY_ID); - AutocryptPeerStateResult peerResult = - new AutocryptPeerStateResult( - AutocryptState.DISCOURAGED_GOSSIP, masterKeyId, isVerified); - result.put(autocryptId, peerResult); - continue; - } - - AutocryptPeerStateResult peerResult = new AutocryptPeerStateResult( - AutocryptState.DISABLE, null, false); - result.put(autocryptId, peerResult); + AutocryptRecommendationResult peerResult = determineAutocryptRecommendation(cursor); + result.add(peerResult); } } finally { cursor.close(); @@ -285,12 +215,81 @@ Otherwise, set preliminary-recommendation to available. return result; } - public static class AutocryptPeerStateResult { + /** Determines Autocrypt "ui-recommendation", according to spec. + * See https://autocrypt.org/level1.html#recommendations-for-single-recipient-messages + */ + private AutocryptRecommendationResult determineAutocryptRecommendation(Cursor cursor) { + String peerId = cursor.getString(INDEX_IDENTIFIER); + + AutocryptRecommendationResult keyRecommendation = determineAutocryptKeyRecommendation(peerId, cursor); + if (keyRecommendation != null) return keyRecommendation; + + AutocryptRecommendationResult gossipRecommendation = determineAutocryptGossipRecommendation(peerId, cursor); + if (gossipRecommendation != null) return gossipRecommendation; + + return new AutocryptRecommendationResult(peerId, AutocryptState.DISABLE, null, false); + } + + @Nullable + private AutocryptRecommendationResult determineAutocryptKeyRecommendation(String peerId, Cursor cursor) { + boolean hasKey = !cursor.isNull(INDEX_MASTER_KEY_ID); + boolean isRevoked = cursor.getInt(INDEX_KEY_IS_REVOKED) != 0; + boolean isExpired = cursor.getInt(INDEX_KEY_IS_EXPIRED) != 0; + if (!hasKey || isRevoked || isExpired) { + return null; + } + + long masterKeyId = cursor.getLong(INDEX_MASTER_KEY_ID); + long lastSeen = cursor.getLong(INDEX_LAST_SEEN); + long lastSeenKey = cursor.getLong(INDEX_LAST_SEEN_KEY); + boolean isVerified = cursor.getInt(INDEX_KEY_IS_VERIFIED) != 0; + if (lastSeenKey < (lastSeen - AUTOCRYPT_DISCOURAGE_THRESHOLD_MILLIS)) { + return new AutocryptRecommendationResult(peerId, AutocryptState.DISCOURAGED_OLD, masterKeyId, isVerified); + } + + boolean isMutual = cursor.getInt(INDEX_STATE) != 0; + if (isMutual) { + return new AutocryptRecommendationResult(peerId, AutocryptState.MUTUAL, masterKeyId, isVerified); + } else { + return new AutocryptRecommendationResult(peerId, AutocryptState.AVAILABLE, masterKeyId, isVerified); + } + } + + @Nullable + private AutocryptRecommendationResult determineAutocryptGossipRecommendation(String peerId, Cursor cursor) { + boolean gossipHasKey = !cursor.isNull(INDEX_GOSSIP_MASTER_KEY_ID); + boolean gossipIsRevoked = cursor.getInt(INDEX_GOSSIP_KEY_IS_REVOKED) != 0; + boolean gossipIsExpired = cursor.getInt(INDEX_GOSSIP_KEY_IS_EXPIRED) != 0; + boolean isVerified = cursor.getInt(INDEX_GOSSIP_KEY_IS_VERIFIED) != 0; + + if (!gossipHasKey || gossipIsRevoked || gossipIsExpired) { + return null; + } + + long masterKeyId = cursor.getLong(INDEX_GOSSIP_MASTER_KEY_ID); + return new AutocryptRecommendationResult(peerId, AutocryptState.DISCOURAGED_GOSSIP, masterKeyId, isVerified); + } + + private Cursor queryAutocryptPeerData(String[] autocryptIds) { + StringBuilder selection = new StringBuilder(ApiAutocryptPeer.IDENTIFIER + " IN (?"); + for (int i = 1; i < autocryptIds.length; i++) { + selection.append(",?"); + } + selection.append(")"); + + return queryInterface.query(ApiAutocryptPeer.buildByPackageName(packageName), + PROJECTION_AUTOCRYPT_QUERY, selection.toString(), autocryptIds, null); + } + + public static class AutocryptRecommendationResult { + public final String peerId; public final Long masterKeyId; public final AutocryptState autocryptState; public final boolean isVerified; - AutocryptPeerStateResult(AutocryptState autocryptState, Long masterKeyId, boolean isVerified) { + AutocryptRecommendationResult(String peerId, AutocryptState autocryptState, Long masterKeyId, + boolean isVerified) { + this.peerId = peerId; this.autocryptState = autocryptState; this.masterKeyId = masterKeyId; this.isVerified = isVerified; 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 d0b17bb0f..6bd7179da 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java @@ -22,7 +22,6 @@ import java.security.AccessControlException; import java.util.Arrays; import java.util.HashMap; import java.util.List; -import java.util.Map; import android.content.ContentProvider; import android.content.ContentValues; @@ -41,7 +40,7 @@ import org.sufficientlysecure.keychain.BuildConfig; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.provider.ApiDataAccessObject; import org.sufficientlysecure.keychain.provider.AutocryptPeerDataAccessObject; -import org.sufficientlysecure.keychain.provider.AutocryptPeerDataAccessObject.AutocryptPeerStateResult; +import org.sufficientlysecure.keychain.provider.AutocryptPeerDataAccessObject.AutocryptRecommendationResult; import org.sufficientlysecure.keychain.provider.AutocryptPeerDataAccessObject.AutocryptState; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.KeychainContract.ApiApps; @@ -249,8 +248,8 @@ public class KeychainExternalProvider extends ContentProvider implements SimpleC AutocryptStatus.UID_ADDRESS + " TEXT, " + AutocryptStatus.UID_MASTER_KEY_ID + " INT, " + AutocryptStatus.UID_CANDIDATES + " INT, " + + AutocryptStatus.AUTOCRYPT_PEER_STATE + " INT DEFAULT " + AutocryptStatus.AUTOCRYPT_PEER_DISABLED + ", " + AutocryptStatus.AUTOCRYPT_KEY_STATUS + " INT, " + - AutocryptStatus.AUTOCRYPT_PEER_STATE + " INT, " + AutocryptStatus.AUTOCRYPT_MASTER_KEY_ID + " INT" + ");"); ContentValues cv = new ContentValues(); @@ -278,7 +277,9 @@ public class KeychainExternalProvider extends ContentProvider implements SimpleC throw new UnsupportedOperationException("Cannot wildcard-query autocrypt results!"); } if (!isWildcardSelector && queriesAutocryptResult) { - fillTempTableWithAutocryptResult(selectionArgs, db, callingPackageName, cv); + AutocryptPeerDataAccessObject autocryptPeerDao = + new AutocryptPeerDataAccessObject(this, callingPackageName); + fillTempTableWithAutocryptRecommendations(db, autocryptPeerDao, selectionArgs); } qb.setTables(TEMP_TABLE_QUERIED_ADDRESSES); @@ -338,30 +339,30 @@ public class KeychainExternalProvider extends ContentProvider implements SimpleC return cursor; } - private void fillTempTableWithAutocryptResult(String[] selectionArgs, SQLiteDatabase db, String callingPackageName, - ContentValues cv) { - AutocryptPeerDataAccessObject autocryptPeerDao = - new AutocryptPeerDataAccessObject(this, callingPackageName); - Map autocryptStates = autocryptPeerDao.getAutocryptState(selectionArgs); + private void fillTempTableWithAutocryptRecommendations(SQLiteDatabase db, + AutocryptPeerDataAccessObject autocryptPeerDao, String[] peerIds) { + List autocryptStates = + autocryptPeerDao.determineAutocryptRecommendations(peerIds); - for (String autocryptId : selectionArgs) { + fillTempTableWithAutocryptRecommendations(db, autocryptStates); + } + + private void fillTempTableWithAutocryptRecommendations(SQLiteDatabase db, + List autocryptRecommendations) { + ContentValues cv = new ContentValues(); + for (AutocryptRecommendationResult peerResult : autocryptRecommendations) { cv.clear(); - AutocryptPeerStateResult peerResult = autocryptStates.get(autocryptId); - if (peerResult == null) { - cv.put(AutocryptStatus.AUTOCRYPT_PEER_STATE, AutocryptStatus.AUTOCRYPT_PEER_DISABLED); - } else { - cv.put(AutocryptStatus.AUTOCRYPT_PEER_STATE, getPeerStateValue(peerResult.autocryptState)); - if (peerResult.masterKeyId != null) { - cv.put(AutocryptStatus.AUTOCRYPT_MASTER_KEY_ID, peerResult.masterKeyId); - cv.put(AutocryptStatus.AUTOCRYPT_KEY_STATUS, peerResult.isVerified ? - KeychainExternalContract.KEY_STATUS_VERIFIED : - KeychainExternalContract.KEY_STATUS_UNVERIFIED); - } + cv.put(AutocryptStatus.AUTOCRYPT_PEER_STATE, getPeerStateValue(peerResult.autocryptState)); + if (peerResult.masterKeyId != null) { + cv.put(AutocryptStatus.AUTOCRYPT_MASTER_KEY_ID, peerResult.masterKeyId); + cv.put(AutocryptStatus.AUTOCRYPT_KEY_STATUS, peerResult.isVerified ? + KeychainExternalContract.KEY_STATUS_VERIFIED : + KeychainExternalContract.KEY_STATUS_UNVERIFIED); } db.update(TEMP_TABLE_QUERIED_ADDRESSES, cv,TEMP_TABLE_COLUMN_ADDRES + "=?", - new String[] { autocryptId }); + new String[] { peerResult.peerId }); } }