Skip to content

Commit

Permalink
Fixed issue with OMEMO keys being overwritten #beagleim-354
Browse files Browse the repository at this point in the history
  • Loading branch information
hantu85 committed Feb 26, 2021
1 parent 7652854 commit 1e22f52
Showing 1 changed file with 33 additions and 30 deletions.
63 changes: 33 additions & 30 deletions BeagleIM/database/DBOMEMOStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import TigaseSQLite3

extension Query {
static let omemoKeyPairForAccount = Query("SELECT key FROM omemo_identities WHERE account = :account AND name = :name AND device_id = :deviceId AND own = 1");
static let omemoKeyPairUpdate = Query("UPDATE omemo_identities SET key = :key, fingerprint = :fingerprint, own = :own WHERE account = :account AND name = :name AND device_id = :deviceId");
static let omemoKeyPairExists = Query("SELECT count(1) FROM omemo_identities WHERE account = :account AND name = :name AND fingerprint = :fingerprint");
static let omemoKeyPairInsert = Query("INSERT INTO omemo_identities (account, name, device_id, key, fingerprint, own, status) VALUES (:account,:name,:deviceId,:key,:fingerprint,:own,:status) ON CONFLICT(account, name, fingerprint) DO UPDATE SET device_id = :deviceId, status = :status");
static let omemoKeyPairLoadStatus = Query("SELECT status FROM omemo_identities WHERE account = :account AND name = :name AND device_id = :deviceId");
static let omemoKeyPairUpdateStatus = Query("UPDATE omemo_identities SET status = :status WHERE account = :account AND name = :name AND device_id = :deviceId");
Expand Down Expand Up @@ -99,52 +99,55 @@ class DBOMEMOStore {
}

func save(identity: SignalAddress, key: SignalIdentityKeyProtocol?, forAccount account: BareJID, own: Bool = false) -> Bool {
guard key != nil else {
guard let key = key else {
// should we remove this key?
return false;
}
guard let publicKeyData = key?.publicKey else {
guard let publicKeyData = key.publicKey else {
return false;
}

let fingerprint: String = publicKeyData.map { (byte) -> String in
return String(format: "%02x", byte)
}.joined();
var params: [String: Any?] = ["account": account, "name": identity.name, "deviceId": identity.deviceId, "key": key!.serialized(), "fingerprint": fingerprint, "own": (own ? 1 : 0)];

let fingerprint: String = self.fingerprint(publicKey: publicKeyData);

defer {
_ = self.setStatus(.verifiedActive, forIdentity: identity, andAccount: account);
}
return try! Database.main.writer({ database -> Int in
try database.update(query: .omemoKeyPairUpdate, params: params);
if database.changes == 0 {
params["status"] = IdentityStatus.verifiedActive.rawValue;
try database.insert(query: .omemoKeyPairInsert, params: params);
}
return database.changes;
}) != 0;

return save(identity: identity, fingerprint: fingerprint, own: own, data: key.serialized(), forAccount: account);
}

func fingerprint(publicKey: Data) -> String {
return publicKey.map { (byte) -> String in
return String(format: "%02x", byte)
}.joined();
}

func save(identity: SignalAddress, publicKeyData: Data?, forAccount account: BareJID) -> Bool {
guard publicKeyData != nil else {
guard let publicKeyData = publicKeyData else {
// should we remove this key?
return false;
}

let fingerprint: String = publicKeyData!.map { (byte) -> String in
return String(format: "%02x", byte)
}.joined();
var params: [String: Any?] = ["account": account, "name": identity.name, "deviceId": identity.deviceId, "key": publicKeyData!, "fingerprint": fingerprint, "own": 0];
print("updating identity for account \(account) and address \(identity) with fingerprint \(fingerprint)");
return try! Database.main.writer({ database -> Int in
try database.update(query: .omemoKeyPairUpdate, params: params);
if database.changes == 0 {
print("inserting identity for account \(account) and address \(identity) with fingerprint \(fingerprint)");
params["status"] = IdentityStatus.trustedActive.rawValue;
try database.insert(query: .omemoKeyPairInsert, params: params);
let fingerprint: String = self.fingerprint(publicKey: publicKeyData);
return save(identity: identity, fingerprint: fingerprint, own: false, data: publicKeyData, forAccount: account);
}

private func save(identity: SignalAddress, fingerprint: String, own: Bool, data: Data?, forAccount account: BareJID) -> Bool {
return try! Database.main.writer({ database -> Bool in
let paramsCount: [String: Any?] = ["account": account, "name": identity.name, "fingerprint": fingerprint];
guard try database.count(query: .omemoKeyPairExists, params: paramsCount) == 0 else {
return true;
}
return database.changes;
}) == 0;

print("inserting identity for account \(account) and address \(identity) with fingerprint \(fingerprint)");
var params: [String: Any?] = paramsCount;
params["deviceId"] = identity.deviceId;
params["key"] = data;
params["own"] = own ? 1 : 0;
params["status"] = IdentityStatus.trustedActive.rawValue;
try database.insert(query: .omemoKeyPairInsert, params: params);
return true;
});
}

func setStatus(_ status: IdentityStatus, forIdentity identity: SignalAddress, andAccount account: BareJID) -> Bool {
Expand Down

2 comments on commit 1e22f52

@michaelblyons
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any detail you can share on this? It sounds significant.

@hantu85
Copy link
Contributor Author

@hantu85 hantu85 commented on 1e22f52 Mar 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In rare cases, it was possible, that Beagle IM/Siskin IM could overwrite their device key (public + private) with only a public key making further OMEMO decryption fail on this device.

If you have not seen an issue then it is not important. The fix was backported from Siskin IM as the issue was found there when OMEMO was used in MUC, but as Beagle IM doesn't have MUC OMEMO, the issue has not manifested.

Please sign in to comment.