| author | Matthew Wild
<mwild1@gmail.com> 2025-05-16 10:22:47 UTC |
| committer | Stephen Paul Weber
<singpolyma@singpolyma.net> 2025-09-29 13:43:04 UTC |
| parent | 8579964b021428d01353a3307227a6cec388cb72 |
| doc/OMEMO.md | +1 | -1 |
| snikket/Client.hx | +4 | -4 |
| snikket/EncryptionInfo.hx | +22 | -45 |
| snikket/Message.hx | +4 | -2 |
| snikket/MessageSync.hx | +3 | -1 |
| snikket/OMEMO.hx | +56 | -22 |
diff --git a/doc/OMEMO.md b/doc/OMEMO.md index 69fcc0c..c130206 100644 --- a/doc/OMEMO.md +++ b/doc/OMEMO.md @@ -16,9 +16,9 @@ compile with the NO_OMEMO flag. - [x] Encrypt outgoing messages to the sending account's other devices - [x] Persistence: IndexedDB (for web) - [x] Use cache for remote contact devices +- [x] Fix that encryption status reported by the API can be forged by sender - [ ] Persistence: SQLite backend (for native) - [ ] API to control encryption of outgoing messages - [ ] API to determine cryptographic identity of message sender -- [ ] Fix that encryption status reported by the API can be forged by sender - [ ] Group chat support diff --git a/snikket/Client.hx b/snikket/Client.hx index f11b598..c98c300 100644 --- a/snikket/Client.hx +++ b/snikket/Client.hx @@ -371,9 +371,9 @@ class Client extends EventEmitter { } #if !NO_OMEMO if(stanza.hasChild("encrypted", NS.OMEMO)) { - omemo.decryptMessage(stanza).then((decryptedStanza) -> { + omemo.decryptMessage(stanza).then((decryptionResult) -> { trace("OMEMO: Decrypted message, now processing..."); - processLiveMessage(decryptedStanza); + processLiveMessage(decryptionResult.stanza, decryptionResult.encryptionInfo); return true; }); return EventHandled; @@ -596,7 +596,7 @@ class Client extends EventEmitter { } @:allow(snikket) - private function processLiveMessage(stanza:Stanza):Void { + private function processLiveMessage(stanza:Stanza, ?encryptionInfo:EncryptionInfo):Void { final from = stanza.attr.get("from") == null ? null : JID.parse(stanza.attr.get("from")); if (stanza.attr.get("type") == "error" && from != null) { @@ -620,7 +620,7 @@ class Client extends EventEmitter { if (chat == null && stanza.attr.get("type") != "groupchat") chat = getDirectChat(builder.chatId()); if (chat == null) return builder; return chat.prepareIncomingMessage(builder, stanza); - }); + }, encryptionInfo); switch (message.parsed) { case ChatMessageStanza(chatMessage): diff --git a/snikket/EncryptionInfo.hx b/snikket/EncryptionInfo.hx index c5315ee..df661b6 100644 --- a/snikket/EncryptionInfo.hx +++ b/snikket/EncryptionInfo.hx @@ -46,53 +46,30 @@ class EncryptionInfo { return el; } + // Typically used to deduce an EncryptionInfo when none has been provided + // May return null if the stanza is not recognizably encrypted. static public function fromStanza(stanza:Stanza):Null<EncryptionInfo> { - final decryptionStatus = stanza.getChild("decryption-status", "https://snikket.org/protocol/sdk"); final emeElement = stanza.getChild("encryption", "urn:xmpp:eme:0"); - - if(decryptionStatus != null) { - if(decryptionStatus.attr.get("result") == "failure") { - // We attempted to decrypt this stanza, but something - // went wrong. The decryption-status element contains - // more information. - final ns = decryptionStatus.attr.get("encryption"); - return new EncryptionInfo( - DecryptionFailure, - ns??"unknown", - ns != null?knownEncryptionSchemes.get(ns):"Unknown encryption", - decryptionStatus.getChildText("reason"), // Machine-readable reason (depends on encryption method) - decryptionStatus.getChildText("text"), // Human-readable explanation - ); - } else { - final encryptionMethod = decryptionStatus.attr.get("encryption")??"unknown"; - return new EncryptionInfo( - DecryptionSuccess, - encryptionMethod, - knownEncryptionSchemes.get(encryptionMethod)??"Unknown encryption" - ); - } - } else { - // We did not decrypt this stanza, so check for any signs - // that it was encrypted in the first place... - var ns = null, name = null; - if(emeElement != null) { - ns = emeElement.attr.get("namespace"); - name = emeElement.attr.get("name"); - } else if(stanza.getChild("encrypted", "eu.siacs.conversations.axolotl") != null) { - // Special handling for OMEMO without EME, just because it is - // so widely used. - ns = "eu.siacs.conversations.axolotl"; - } - if(ns != null) { - return new EncryptionInfo( - DecryptionFailure, - ns??"unknown", - knownEncryptionSchemes.get(ns)??name??"Unknown encryption", - "unsupported-encryption", - "Unsupported encryption method" - ); - } - } + // We did not decrypt this stanza, so check for any signs + // that it was encrypted in the first place... + var ns = null, name = null; + if(emeElement != null) { + ns = emeElement.attr.get("namespace"); + name = emeElement.attr.get("name"); + } else if(stanza.getChild("encrypted", "eu.siacs.conversations.axolotl") != null) { + // Special handling for OMEMO without EME, just because it is + // so widely used. + ns = "eu.siacs.conversations.axolotl"; + } + if(ns != null) { + return new EncryptionInfo( + DecryptionFailure, + ns??"unknown", + knownEncryptionSchemes.get(ns)??name??"Unknown encryption", + "unsupported-encryption", + "Unsupported encryption method" + ); + } return null; // Probably not encrypted } } diff --git a/snikket/Message.hx b/snikket/Message.hx index c307446..f3e8835 100644 --- a/snikket/Message.hx +++ b/snikket/Message.hx @@ -48,10 +48,12 @@ class Message { this.encryption = encryption; } - public static function fromStanza(stanza:Stanza, localJid:JID, ?addContext: (ChatMessageBuilder, Stanza)->ChatMessageBuilder):Message { + public static function fromStanza(stanza:Stanza, localJid:JID, ?addContext: (ChatMessageBuilder, Stanza)->ChatMessageBuilder, ?encryptionInfo:EncryptionInfo):Message { final fromAttr = stanza.attr.get("from"); final from = fromAttr == null ? localJid.domain : fromAttr; - final encryptionInfo = EncryptionInfo.fromStanza(stanza); + if(encryptionInfo==null) { + encryptionInfo = EncryptionInfo.fromStanza(stanza); + } if (stanza.attr.get("type") == "error") { return new Message(from, from, null, ErrorMessageStanza(stanza), encryptionInfo); diff --git a/snikket/MessageSync.hx b/snikket/MessageSync.hx index 2fe08ec..70229d8 100644 --- a/snikket/MessageSync.hx +++ b/snikket/MessageSync.hx @@ -93,12 +93,14 @@ class MessageSync { if (originalMessage.hasChild("encrypted", NS.OMEMO)) { #if !NO_OMEMO trace("MAM: Processing OMEMO message from " + originalMessage.attr.get("from")); - promisedMessages.push(client.omemo.decryptMessage(originalMessage).then((decryptedStanza) -> { + promisedMessages.push(client.omemo.decryptMessage(originalMessage).then((decryptionResult) -> { + final decryptedStanza = decryptionResult.stanza; trace("MAM: Decrypted stanza: "+decryptedStanza); final msg = Message.fromStanza(decryptedStanza, client.jid, (builder, stanza) -> { builder.serverId = result.attr.get("id"); builder.serverIdBy = serviceJID; + builder.encryption = decryptionResult.encryptionInfo; if (timestamp != null && builder.timestamp == null) builder.timestamp = timestamp; return contextHandler(builder, stanza); }).parsed; diff --git a/snikket/OMEMO.hx b/snikket/OMEMO.hx index f28440f..b496948 100644 --- a/snikket/OMEMO.hx +++ b/snikket/OMEMO.hx @@ -304,6 +304,16 @@ class OMEMOEncryptionResult { } } +class OMEMODecryptionResult { + public final stanza:Stanza; + public final encryptionInfo:EncryptionInfo; + + public function new(stanza:Stanza, encryptionInfo:EncryptionInfo) { + this.stanza = stanza; + this.encryptionInfo = encryptionInfo; + } +} + //@:nullSafety(Strict) class OMEMO { private final client: Client; @@ -840,48 +850,72 @@ class OMEMO { }); } - public function decryptMessage(stanza: Stanza):Promise<Stanza> { + public function decryptMessage(stanza: Stanza):Promise<OMEMODecryptionResult> { final header = OMEMOPayload.fromMessageStanza(stanza); return client.omemo.getDeviceId().then((deviceId:Int) -> { final deviceKey = header.findKey(deviceId); if(deviceKey == null) { trace("OMEMO: Message not encrypted for our device (looked for "+deviceId+")"); stanza.removeChildren("encrypted", NS.OMEMO); - return Promise.resolve(stanza); + return Promise.resolve( + new OMEMODecryptionResult( + stanza, + new EncryptionInfo( + DecryptionFailure, + NS.OMEMO, + "missing-key", + "Sender did not include this device in recipients" + ) + ) + ); } // FIXME: Identify correct JID for group chats trace("OMEMO: Decrypting payload..."); final from = JID.parse(stanza.attr.get("from")).asBare(); final promPayload = decryptPayload(deviceId, deviceKey, from.asString(), header); return promPayload.then((decryptedPayload:BytesData) -> { - if(decryptedPayload != null) { - stanza.removeChildren("body"); - // FIXME: Verify valid UTF-8, etc. - stanza.textTag("body", Bytes.ofData(decryptedPayload).toString()); - stanza.tag("decryption-status", { - xmlns: "https://snikket.org/protocol/sdk", - result: "success", - encryption: "eu.siacs.conversations.axolotl", - }); - trace("OMEMO: Payload decrypted OK!"); - } else { + if(decryptedPayload == null) { trace("OMEMO: Decrypted payload is null?"); + return Promise.resolve(new OMEMODecryptionResult( + stanza, + new EncryptionInfo( + DecryptionFailure, + NS.OMEMO, + "invalid-payload", + "The encrypted message was malformed" + ) + )); } - return Promise.resolve(stanza); + + stanza.removeChildren("body"); + // FIXME: Verify valid UTF-8, etc. + stanza.textTag("body", Bytes.ofData(decryptedPayload).toString()); + trace("OMEMO: Payload decrypted OK!"); + return Promise.resolve(new OMEMODecryptionResult( + stanza, + new EncryptionInfo( + DecryptionSuccess, + NS.OMEMO, + ) + )); }, (err:Any) -> { trace("OMEMO: Failed to decrypt message: " + err); - stanza.tag("decryption-status", { - xmlns: "https://snikket.org/protocol/sdk", - result: "failure", - encryption: "eu.siacs.conversations.axolotl", - reason: "generic", - text: err, - }); + // FIXME: Rebuilding the session should not be unconditional, as this + // can be triggered by a MITM and effectively bypass the double ratchet + // part of the protocol. buildSession(deviceId, from.asString(), header.sid).then((session) -> { // Broken session? Send key to start new session... sendKeyExchange(deviceId, from.asString(), header.sid); }); - return Promise.resolve(stanza); + return Promise.resolve(new OMEMODecryptionResult( + stanza, + new EncryptionInfo( + DecryptionFailure, + NS.OMEMO, + "generic", + err, + ) + )); }); }); }