From e2d506c96a0a72b7a903d28d9b3a858430700266 Mon Sep 17 00:00:00 2001 From: Andreas Straub Date: Sat, 5 Sep 2015 17:25:46 +0200 Subject: [PATCH] Never build a session with oneself If we detect our own ID is not in our own devicelist on receiving an update, we reannounce ourselves. This used to have the side effect of modifying the list of devices we thought were in the update set, causing us to accidentally build a session with ourselves. This lead to our own key being set to TRUSTED_INACTIVE, resulting in red lock icons on messages sent by the own device. We fix this by having publishOwnDeviceId() operate on a copy of the original set. This commit also includes a db migration which deletes sessions with oneself and sets own keys back to TRUSTED. --- .../crypto/axolotl/AxolotlService.java | 14 ++-- .../crypto/axolotl/SQLiteAxolotlStore.java | 7 +- .../persistance/DatabaseBackend.java | 67 ++++++++++++++++--- 3 files changed, 68 insertions(+), 20 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java b/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java index 2ce62c9ad..77c9d9d7e 100644 --- a/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java +++ b/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java @@ -337,9 +337,10 @@ public class AxolotlService { } public void publishOwnDeviceId(Set deviceIds) { - if (!deviceIds.contains(getOwnDeviceId())) { + Set deviceIdsCopy = new HashSet<>(deviceIds); + if (!deviceIdsCopy.contains(getOwnDeviceId())) { Log.d(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Own device " + getOwnDeviceId() + " not in PEP devicelist."); - if (deviceIds.isEmpty()) { + if (deviceIdsCopy.isEmpty()) { if (numPublishTriesOnEmptyPep >= publishTriesThreshold) { Log.w(Config.LOGTAG, getLogprefix(account) + "Own device publish attempt threshold exceeded, aborting..."); pepBroken = true; @@ -351,8 +352,8 @@ public class AxolotlService { } else { numPublishTriesOnEmptyPep = 0; } - deviceIds.add(getOwnDeviceId()); - IqPacket publish = mXmppConnectionService.getIqGenerator().publishDeviceIds(deviceIds); + deviceIdsCopy.add(getOwnDeviceId()); + IqPacket publish = mXmppConnectionService.getIqGenerator().publishDeviceIds(deviceIdsCopy); mXmppConnectionService.sendIqPacket(account, publish, new OnIqPacketReceived() { @Override public void onIqPacketReceived(Account account, IqPacket packet) { @@ -490,7 +491,10 @@ public class AxolotlService { } private void buildSessionFromPEP(final AxolotlAddress address) { - Log.i(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Building new sesstion for " + address.getDeviceId()); + Log.i(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Building new sesstion for " + address.toString()); + if (address.getDeviceId() == getOwnDeviceId()) { + throw new AssertionError("We should NEVER build a session with ourselves. What happened here?!"); + } try { IqPacket bundlesPacket = mXmppConnectionService.getIqGenerator().retrieveBundlesForDevice( diff --git a/src/main/java/eu/siacs/conversations/crypto/axolotl/SQLiteAxolotlStore.java b/src/main/java/eu/siacs/conversations/crypto/axolotl/SQLiteAxolotlStore.java index 190eb88a8..4d7793023 100644 --- a/src/main/java/eu/siacs/conversations/crypto/axolotl/SQLiteAxolotlStore.java +++ b/src/main/java/eu/siacs/conversations/crypto/axolotl/SQLiteAxolotlStore.java @@ -89,15 +89,14 @@ public class SQLiteAxolotlStore implements AxolotlStore { private IdentityKeyPair loadIdentityKeyPair() { String ownName = account.getJid().toBareJid().toString(); - IdentityKeyPair ownKey = mXmppConnectionService.databaseBackend.loadOwnIdentityKeyPair(account, - ownName); + IdentityKeyPair ownKey = mXmppConnectionService.databaseBackend.loadOwnIdentityKeyPair(account); if (ownKey != null) { return ownKey; } else { - Log.i(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Could not retrieve axolotl key for account " + ownName); + Log.i(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Could not retrieve own IdentityKeyPair"); ownKey = generateIdentityKeyPair(); - mXmppConnectionService.databaseBackend.storeOwnIdentityKeyPair(account, ownName, ownKey); + mXmppConnectionService.databaseBackend.storeOwnIdentityKeyPair(account, ownKey); } return ownKey; } diff --git a/src/main/java/eu/siacs/conversations/persistance/DatabaseBackend.java b/src/main/java/eu/siacs/conversations/persistance/DatabaseBackend.java index 9fe47512b..44063a547 100644 --- a/src/main/java/eu/siacs/conversations/persistance/DatabaseBackend.java +++ b/src/main/java/eu/siacs/conversations/persistance/DatabaseBackend.java @@ -42,7 +42,7 @@ public class DatabaseBackend extends SQLiteOpenHelper { private static DatabaseBackend instance = null; private static final String DATABASE_NAME = "history"; - private static final int DATABASE_VERSION = 16; + private static final int DATABASE_VERSION = 17; private static String CREATE_CONTATCS_STATEMENT = "create table " + Contact.TABLENAME + "(" + Contact.ACCOUNT + " TEXT, " @@ -301,6 +301,20 @@ public class DatabaseBackend extends SQLiteOpenHelper { db.execSQL("ALTER TABLE " + Message.TABLENAME + " ADD COLUMN " + Message.CARBON + " INTEGER"); } + if (oldVersion < 17 && newVersion >= 17) { + List accounts = getAccounts(db); + for (Account account : accounts) { + String ownDeviceIdString = account.getKey(SQLiteAxolotlStore.JSONKEY_REGISTRATION_ID); + if ( ownDeviceIdString == null ) { + continue; + } + int ownDeviceId = Integer.valueOf(ownDeviceIdString); + AxolotlAddress ownAddress = new AxolotlAddress(account.getJid().toBareJid().toString(), ownDeviceId); + deleteSession(db, account, ownAddress); + IdentityKeyPair identityKeyPair = loadOwnIdentityKeyPair(db, account); + setIdentityKeyTrust(db, account, identityKeyPair.getPublicKey().getFingerprint().replaceAll("\\s", ""), XmppAxolotlSession.Trust.TRUSTED ); + } + } } public static synchronized DatabaseBackend getInstance(Context context) { @@ -414,8 +428,12 @@ public class DatabaseBackend extends SQLiteOpenHelper { } public List getAccounts() { - List list = new ArrayList<>(); SQLiteDatabase db = this.getReadableDatabase(); + return getAccounts(db); + } + + private List getAccounts(SQLiteDatabase db) { + List list = new ArrayList<>(); Cursor cursor = db.query(Account.TABLENAME, null, null, null, null, null, null); while (cursor.moveToNext()) { @@ -602,8 +620,12 @@ public class DatabaseBackend extends SQLiteOpenHelper { } public List getSubDeviceSessions(Account account, AxolotlAddress contact) { - List devices = new ArrayList<>(); final SQLiteDatabase db = this.getReadableDatabase(); + return getSubDeviceSessions(db, account, contact); + } + + private List getSubDeviceSessions(SQLiteDatabase db, Account account, AxolotlAddress contact) { + List devices = new ArrayList<>(); String[] columns = {SQLiteAxolotlStore.DEVICE_ID}; String[] selectionArgs = {account.getUuid(), contact.getName()}; @@ -642,6 +664,10 @@ public class DatabaseBackend extends SQLiteOpenHelper { public void deleteSession(Account account, AxolotlAddress contact) { SQLiteDatabase db = this.getWritableDatabase(); + deleteSession(db, account, contact); + } + + private void deleteSession(SQLiteDatabase db, Account account, AxolotlAddress contact) { String[] args = {account.getUuid(), contact.getName(), Integer.toString(contact.getDeviceId())}; @@ -790,15 +816,24 @@ public class DatabaseBackend extends SQLiteOpenHelper { } private Cursor getIdentityKeyCursor(Account account, String name, boolean own) { - return getIdentityKeyCursor(account, name, own, null); + final SQLiteDatabase db = this.getReadableDatabase(); + return getIdentityKeyCursor(db, account, name, own); + } + + private Cursor getIdentityKeyCursor(SQLiteDatabase db, Account account, String name, boolean own) { + return getIdentityKeyCursor(db, account, name, own, null); } private Cursor getIdentityKeyCursor(Account account, String fingerprint) { - return getIdentityKeyCursor(account, null, null, fingerprint); + final SQLiteDatabase db = this.getReadableDatabase(); + return getIdentityKeyCursor(db, account, fingerprint); } - private Cursor getIdentityKeyCursor(Account account, String name, Boolean own, String fingerprint) { - final SQLiteDatabase db = this.getReadableDatabase(); + private Cursor getIdentityKeyCursor(SQLiteDatabase db, Account account, String fingerprint) { + return getIdentityKeyCursor(db, account, null, null, fingerprint); + } + + private Cursor getIdentityKeyCursor(SQLiteDatabase db, Account account, String name, Boolean own, String fingerprint) { String[] columns = {SQLiteAxolotlStore.TRUSTED, SQLiteAxolotlStore.KEY}; ArrayList selectionArgs = new ArrayList<>(4); @@ -825,9 +860,15 @@ public class DatabaseBackend extends SQLiteOpenHelper { return cursor; } - public IdentityKeyPair loadOwnIdentityKeyPair(Account account, String name) { + public IdentityKeyPair loadOwnIdentityKeyPair(Account account) { + SQLiteDatabase db = getReadableDatabase(); + return loadOwnIdentityKeyPair(db, account); + } + + private IdentityKeyPair loadOwnIdentityKeyPair(SQLiteDatabase db, Account account) { + String name = account.getJid().toBareJid().toString(); IdentityKeyPair identityKeyPair = null; - Cursor cursor = getIdentityKeyCursor(account, name, true); + Cursor cursor = getIdentityKeyCursor(db, account, name, true); if(cursor.getCount() != 0) { cursor.moveToFirst(); try { @@ -911,6 +952,10 @@ public class DatabaseBackend extends SQLiteOpenHelper { public boolean setIdentityKeyTrust(Account account, String fingerprint, XmppAxolotlSession.Trust trust) { SQLiteDatabase db = this.getWritableDatabase(); + return setIdentityKeyTrust(db, account, fingerprint, trust); + } + + private boolean setIdentityKeyTrust(SQLiteDatabase db, Account account, String fingerprint, XmppAxolotlSession.Trust trust) { String[] selectionArgs = { account.getUuid(), fingerprint @@ -928,8 +973,8 @@ public class DatabaseBackend extends SQLiteOpenHelper { storeIdentityKey(account, name, false, identityKey.getFingerprint().replaceAll("\\s", ""), Base64.encodeToString(identityKey.serialize(), Base64.DEFAULT)); } - public void storeOwnIdentityKeyPair(Account account, String name, IdentityKeyPair identityKeyPair) { - storeIdentityKey(account, name, true, identityKeyPair.getPublicKey().getFingerprint().replaceAll("\\s", ""), Base64.encodeToString(identityKeyPair.serialize(), Base64.DEFAULT), XmppAxolotlSession.Trust.TRUSTED); + public void storeOwnIdentityKeyPair(Account account, IdentityKeyPair identityKeyPair) { + storeIdentityKey(account, account.getJid().toBareJid().toString(), true, identityKeyPair.getPublicKey().getFingerprint().replaceAll("\\s", ""), Base64.encodeToString(identityKeyPair.serialize(), Base64.DEFAULT), XmppAxolotlSession.Trust.TRUSTED); } public void recreateAxolotlDb() {