Page MenuHomePhorge
Files F85650089

D360.1787977110.diff
No OneTemporary
Actions

Size
19 KB
Referenced Files
None
Subscribers
None

D360.1787977110.diff
View Options

diff --git a/design-docs/undecryptable-event-fallback.md b/design-docs/undecryptable-event-fallback.md
--- a/design-docs/undecryptable-event-fallback.md
+++ b/design-docs/undecryptable-event-fallback.md
@@ -16,6 +16,7 @@
- `moe.kazv.mxc.errcode` The error code.
- `moe.kazv.mxc.error` The error message.
- `moe.kazv.mxc.raw` The raw data associated with the error.
+- For to-device events only, `moe.kazv.mxc.failed_times` The number of failed trials of decryption. After a certain amount of failed trials, libkazv will no longer attempt to decrypt the event by itself.
### Possible error codes
#### `MOE.KAZV.MXC_DECRYPT_ERROR`
diff --git a/src/client/actions/encryption.hpp b/src/client/actions/encryption.hpp
--- a/src/client/actions/encryption.hpp
+++ b/src/client/actions/encryption.hpp
@@ -17,7 +17,7 @@
ClientResult updateClient(ClientModel m, GenerateAndUploadOneTimeKeysAction a);
ClientResult processResponse(ClientModel m, UploadKeysResponse r);
- ClientModel tryDecryptEvents(ClientModel m);
+ [[nodiscard]] ClientModel tryDecryptEvents(ClientModel m);
Event decryptEvent(ClientModel &m, Event e);
std::optional<BaseJob> clientPerform(ClientModel m, QueryKeysAction a);
diff --git a/src/client/actions/encryption.cpp b/src/client/actions/encryption.cpp
--- a/src/client/actions/encryption.cpp
+++ b/src/client/actions/encryption.cpp
@@ -131,6 +131,7 @@
return { std::move(m), lager::noop };
}
+ static const std::string CANNOT_DECRYPT_MSGTYPE = "moe.kazv.mxc.cannot.decrypt";
static JsonWrap cannotDecryptEvent(
const std::string &reason,
const std::string &errcode,
@@ -139,7 +140,7 @@
return json{
{"type", "m.room.message"},
{"content", {
- {"msgtype","moe.kazv.mxc.cannot.decrypt"},
+ {"msgtype", CANNOT_DECRYPT_MSGTYPE},
{"body", "**This message cannot be decrypted due to " + reason + ".**"},
{"moe.kazv.mxc.error", reason},
{"moe.kazv.mxc.errcode", errcode},
@@ -253,6 +254,40 @@
return std::nullopt;
}
+ static constexpr int TO_DEVICE_EVENT_MAX_RETRY_COUNT = 10;
+
+ static int getEventFailedTimes(const Event &e)
+ {
+ // Safety guarantee: if the event is decrypted, then e.decrypted() will be true.
+ // So if someone sends us an encrypted m.room.message event with msgtype
+ // of <CANNOT_DECRYPT_MSGTYPE>, we will not consider it to be a failed-
+ // to-decrypt event.
+ if (!e.decrypted()
+ && e.encrypted()
+ && e.decryptedJson().get().contains("/content/msgtype"_json_pointer)
+ && e.decryptedJson().get()["content"]["msgtype"] == CANNOT_DECRYPT_MSGTYPE
+ && e.decryptedJson().get().contains("/content/moe.kazv.mxc.failed_times"_json_pointer)
+ && e.decryptedJson().get()["content"]["moe.kazv.mxc.failed_times"].is_number()) {
+ return e.decryptedJson().get()["content"]["moe.kazv.mxc.failed_times"].template get<int>();
+ } else {
+ return 0;
+ }
+ }
+
+ namespace
+ {
+ struct ReuseLastError {};
+
+ using GetPlainTextResult = std::variant<
+ // result from Crypto::decrypt()
+ MaybeString,
+ // result from cached decrypted (in events that cannot be verified)
+ json,
+ // ask to reuse last error (i.e. return e as-is)
+ ReuseLastError
+ >;
+ }
+
Event decryptEvent(ClientModel &m, Event e)
{
// no need for decryption
@@ -263,32 +298,66 @@
kzo.client.dbg() << "About to decrypt event: "
<< e.id() << std::endl;
- auto maybePlainText = m.withCrypto([&](Crypto &c) {
- return c.decrypt(e.originalJson().get());
- });
+ auto getPlainText = [&m](Event e) -> GetPlainTextResult {
+ auto fallbackDecrypt = [&m](Event ev) {
+ return m.withCrypto([&](Crypto &c) {
+ return c.decrypt(ev.originalJson().get());
+ });
+ };
+ if (e.decryptedJson().get().contains("/content/msgtype"_json_pointer)
+ && e.decryptedJson().get()["content"]["msgtype"] == CANNOT_DECRYPT_MSGTYPE
+ && e.decryptedJson().get().contains("/content/moe.kazv.mxc.errcode"_json_pointer)) {
+ auto errCode = e.decryptedJson().get()["content"]["moe.kazv.mxc.errcode"];
+ // We do not want to decrypt an event unnecessarily too many times
+ // olm events cannot be decrypted more than once
+ if (errCode == "MOE.KAZV.MXC_DECRYPT_ERROR"
+ || errCode == "MOE.KAZV.MXC_UNKNOWN_ALGORITHM") {
+ // Last time, this was unable to decrypt. We can try again.
+ return fallbackDecrypt(e);
+ } else if (errCode == "M_NOT_JSON") {
+ // Decrypted content is not json. Cannot recover.
+ return ReuseLastError{};
+ } else if (errCode == "MOE.KAZV.MXC_DEVICE_KEY_UNKNOWN"
+ || errCode == "MOE.KAZV.MXC_BAD_SENDER"
+ || errCode == "MOE.KAZV.MXC_BAD_RECIPIENT"
+ || errCode == "MOE.KAZV.MXC_BAD_RECIPIENT_KEYS"
+ || errCode == "MOE.KAZV.MXC_BAD_SENDER_KEYS"
+ || errCode == "MOE.KAZV.MXC_BAD_ROOM_ID"
+ || errCode == "M_BAD_JSON") {
+ // Last time the event was decrypted, but did not pass verification.
+ // Try to verify it again without re-decrypting it.
+ if (e.decryptedJson().get()["content"].contains("moe.kazv.mxc.raw")) {
+ return e.decryptedJson().get()["content"]["moe.kazv.mxc.raw"];
+ } else {
+ return fallbackDecrypt(e);
+ }
+ }
+ }
- if (! maybePlainText) {
- kzo.client.dbg() << "Cannot decrypt: " << maybePlainText.reason() << std::endl;
- return e.setDecryptedJson(
- cannotDecryptEvent(
- maybePlainText.reason(),
- "MOE.KAZV.MXC_DECRYPT_ERROR",
- json(nullptr)
- ),
- Event::NotDecrypted);
- } else {
+ return fallbackDecrypt(e);
+ };
+
+ auto r = getPlainText(e);
+ if (std::holds_alternative<ReuseLastError>(r)) {
+ return e;
+ }
+
+ json plainJson;
+
+ if (std::holds_alternative<MaybeString>(r)) {
+ auto maybePlainText = std::get<MaybeString>(r);
+ if (! maybePlainText) {
+ kzo.client.dbg() << "Cannot decrypt: " << maybePlainText.reason() << std::endl;
+ return e.setDecryptedJson(
+ cannotDecryptEvent(
+ maybePlainText.reason(),
+ "MOE.KAZV.MXC_DECRYPT_ERROR",
+ json(nullptr)
+ ),
+ Event::NotDecrypted);
+ }
try {
- auto plainJson = json::parse(maybePlainText.value());
- auto error = verifyEvent(m, e, plainJson);
- auto valid = !error.has_value();
- if (valid) {
- kzo.client.dbg() << "The decrypted event is valid." << std::endl;
- }
- return valid
- ? e.setDecryptedJson(plainJson, Event::Decrypted)
- : e.setDecryptedJson(
- error.value(),
- Event::NotDecrypted);
+ plainJson = json::parse(maybePlainText.value());
} catch (const std::exception &exception) {
return e.setDecryptedJson(
cannotDecryptEvent(
@@ -299,10 +368,24 @@
Event::NotDecrypted
);
}
+ } else {
+ // json
+ plainJson = std::get<json>(r);
}
+
+ auto error = verifyEvent(m, e, plainJson);
+ auto valid = !error.has_value();
+ if (valid) {
+ kzo.client.dbg() << "The decrypted event is valid." << std::endl;
+ }
+ return valid
+ ? e.setDecryptedJson(plainJson, Event::Decrypted)
+ : e.setDecryptedJson(
+ error.value(),
+ Event::NotDecrypted);
}
- ClientModel tryDecryptEvents(ClientModel m)
+ [[nodiscard]] ClientModel tryDecryptEvents(ClientModel m)
{
if (! m.crypto) {
kzo.client.dbg() << "We have no encryption enabled--ignoring decryption request" << std::endl;
@@ -311,7 +394,30 @@
kzo.client.dbg() << "Trying to decrypt events..." << std::endl;
- auto decryptFunc = [&](auto e) { return decryptEvent(m, e); };
+ auto decryptOneInRoom = [&](Event e) {
+ return decryptEvent(m, e);
+ };
+ auto decryptOneToDevice = [&](Event e) {
+ auto failedTimes = getEventFailedTimes(e);
+ // Only try to decrypt to-device events for limited times
+ if (failedTimes >= TO_DEVICE_EVENT_MAX_RETRY_COUNT) {
+ return e;
+ }
+ auto res = decryptEvent(m, e);
+ // if it still fails, add failed times
+ if (!res.decrypted()
+ && res.decryptedJson().get().contains("/content/msgtype"_json_pointer)
+ && res.decryptedJson().get()["content"]["msgtype"] == CANNOT_DECRYPT_MSGTYPE) {
+ auto decryptedJson = res.decryptedJson().get();
+ decryptedJson["content"]["moe.kazv.mxc.failed_times"] = failedTimes + 1;
+ return res.setDecryptedJson(
+ std::move(decryptedJson),
+ Event::NotDecrypted
+ );
+ } else {
+ return res;
+ }
+ };
auto takeOutRoomKeyEvents =
[&](auto e) {
@@ -369,11 +475,11 @@
m.toDevice = intoImmer(
EventList{},
- zug::map(decryptFunc)
+ zug::map(decryptOneToDevice)
| zug::filter(takeOutRoomKeyEvents),
std::move(m.toDevice));
- auto decryptEventInRoom =
+ auto decryptAllInRoom =
[&](auto id, auto room) {
if (! room.encrypted) {
return;
@@ -389,7 +495,7 @@
immer::flex_vector<std::string>{},
zug::filter([&](auto eventId) {
auto event = room.messages[eventId];
- auto decrypted = decryptFunc(event);
+ auto decrypted = decryptOneInRoom(event);
room.messages = std::move(room.messages)
.set(eventId, decrypted);
@@ -411,7 +517,7 @@
auto rooms = m.roomList.rooms;
for (auto [id, room]: rooms) {
- decryptEventInRoom(id, room);
+ decryptAllInRoom(id, room);
}
return m;
diff --git a/src/tests/client/encryption-test.cpp b/src/tests/client/encryption-test.cpp
--- a/src/tests/client/encryption-test.cpp
+++ b/src/tests/client/encryption-test.cpp
@@ -40,21 +40,24 @@
return json::parse(std::get<Bytes>(next.nextJobs[0].requestBody()))["device_keys"];
}
-static void queryEachOtherKeys(ClientModel &m1, ClientModel &m2)
+// Make a aware of b
+static void queryOne(ClientModel &a, ClientModel &b)
{
- auto queryOne = [](ClientModel &a, ClientModel &b) {
- auto queryKeysRespJson = json{
- {"device_keys", {{a.userId, {
- {a.deviceId, makeDeviceInfo(a)}
- }}}},
- };
- std::tie(b, std::ignore) = processResponse(b, QueryKeysResponse(
- makeResponse("QueryKeys", withResponseJsonBody(queryKeysRespJson)
- | withResponseDataKV("deviceKeys", json::object({
- {a.userId, {a.deviceId}}
- })))
- ));
+ auto queryKeysRespJson = json{
+ {"device_keys", {{b.userId, {
+ {b.deviceId, makeDeviceInfo(b)}
+ }}}},
};
+ std::tie(a, std::ignore) = processResponse(a, QueryKeysResponse(
+ makeResponse("QueryKeys", withResponseJsonBody(queryKeysRespJson)
+ | withResponseDataKV("deviceKeys", json::object({
+ {b.userId, {b.deviceId}}
+ })))
+ ));
+}
+
+static void queryEachOtherKeys(ClientModel &m1, ClientModel &m2)
+{
queryOne(m1, m2);
queryOne(m2, m1);
}
@@ -165,6 +168,17 @@
| withResponseDataKV("is", "incremental"));
}
+static json encryptToOneDevice(ClientModel &sender, const ClientModel &receiver, const json &plain)
+{
+ auto res = sender.olmEncryptSplit(Event(plain),
+ {{receiver.userId, {receiver.deviceId}}},
+ genRandomData(Crypto::encryptOlmMaxRandomSize()));
+
+ auto e = res[receiver.userId][receiver.deviceId].originalJson().get();
+ e["sender"] = sender.userId;
+ return e;
+}
+
TEST_CASE("PrepareForSharingRoomKeyAction: adds the encrypted event to pending events", "[client][encryption]")
{
ClientModel m;
@@ -383,13 +397,7 @@
};
auto encryptToReceiver = [receiver2](ClientModel &sender, const json &plain) {
- auto res = sender.olmEncryptSplit(Event(plain),
- {{receiver2.userId, {receiver2.deviceId}}},
- genRandomData(Crypto::encryptOlmMaxRandomSize()));
-
- auto e = res[receiver2.userId][receiver2.deviceId].originalJson().get();
- e["sender"] = sender.userId;
- return e;
+ return encryptToOneDevice(sender, receiver2, plain);
};
SECTION("Process forwarded key") {
@@ -539,31 +547,22 @@
TEST_CASE("tryDecryptEvents() rejects Olm-encrypted to-device event from unknown device, which causes QueryKeysAction to query relevant keys", "[client][encryption][olm]")
{
- // Bob: the current user, with crypto enabled
- auto bobCrypto = makeCrypto();
- bobCrypto.genOneTimeKeysWithRandom(genRandomData(Crypto::genOneTimeKeysRandomSize(1)), 1);
- auto bobOneTimeKeys = bobCrypto.unpublishedOneTimeKeys();
- bobCrypto.markOneTimeKeysAsPublished();
-
- auto bobClient = makeClient(withCrypto(bobCrypto));
- bobClient.userId = "@bob:example.com";
- bobClient.deviceId = "bobdevice";
-
- auto bobIdentityKey = bobCrypto.curve25519IdentityKey();
- auto bobOneTimeKey = std::string{};
- for (auto [id, key] : bobOneTimeKeys[CryptoConstants::curve25519].items()) {
- bobOneTimeKey = key;
- }
+ auto bobClient = makeClient(
+ withCrypto(makeCrypto())
+ | withAttr(&ClientModel::userId, "@bob:example.com")
+ | withAttr(&ClientModel::deviceId, "bobdevice")
+ );
- // Alice: a device NOT known to Bob (not in Bob's device list)
- auto aliceCrypto = makeCrypto();
- auto aliceIdentityKey = aliceCrypto.curve25519IdentityKey();
- auto aliceEdKey = aliceCrypto.ed25519IdentityKey();
+ auto aliceClient = makeClient(
+ withCrypto(makeCrypto())
+ | withAttr(&ClientModel::userId, "@alice:example.com")
+ | withAttr(&ClientModel::deviceId, "alicedevice")
+ );
+
+ queryOne(aliceClient, bobClient);
+ createAndClaimOneTimeKey(aliceClient, bobClient);
- // Alice creates an outbound session to Bob (using Bob's published one-time key)
- aliceCrypto.createOutboundSessionWithRandom(
- genRandomData(Crypto::createOutboundSessionRandomSize()),
- bobIdentityKey, bobOneTimeKey);
+ auto bobIdentityKey = bobClient.constCrypto().curve25519IdentityKey();
// Alice encrypts an m.room_key event for Bob
auto plainJson = json{
@@ -574,26 +573,28 @@
{"session_key", "somesessionkey"},
}},
{"keys", {
- {CryptoConstants::ed25519, aliceEdKey},
+ {CryptoConstants::ed25519, aliceClient.constCrypto().ed25519IdentityKey()},
}},
{"sender", "@alice:example.com"},
{"recipient", "@bob:example.com"},
{"recipient_keys", {
- {CryptoConstants::ed25519, bobCrypto.ed25519IdentityKey()},
+ {CryptoConstants::ed25519, bobClient.constCrypto().ed25519IdentityKey()},
}},
{"type", "m.room_key"},
};
- auto encryptedCiphertext = aliceCrypto.encryptOlmWithRandom(
- genRandomData(Crypto::encryptOlmMaxRandomSize()),
- plainJson, bobIdentityKey);
+ auto encryptedCiphertext = aliceClient.withCrypto([=](Crypto &c) {
+ return c.encryptOlmWithRandom(
+ genRandomData(Crypto::encryptOlmMaxRandomSize()),
+ plainJson, bobIdentityKey);
+ });
auto toDeviceJson = json{
{"sender", "@alice:example.com"},
{"type", "m.room.encrypted"},
{"content", {
{"algorithm", CryptoConstants::olmAlgo},
- {"sender_key", aliceIdentityKey},
+ {"sender_key", aliceClient.constCrypto().curve25519IdentityKey()},
{"ciphertext", encryptedCiphertext},
}},
};
@@ -627,6 +628,63 @@
{"@alice:example.com", json::array()},
};
REQUIRE(jsonBody["device_keys"] == expected);
+
+ // After B gets A's identity keys, the event should be recovered
+ queryOne(next, aliceClient);
+ next = tryDecryptEvents(next);
+
+ REQUIRE(next.toDevice.size() == 1);
+ processedEvent = next.toDevice[0];
+ REQUIRE(processedEvent.encrypted());
+ REQUIRE(processedEvent.decrypted());
+ REQUIRE(processedEvent.decryptedJson().get() == plainJson);
+}
+
+TEST_CASE("tryDecryptEvents() will not try to decrypt to-device events that failed too many times", "[client][encryption][olm]")
+{
+ auto bobClient = makeClient(
+ withCrypto(makeCrypto())
+ | withAttr(&ClientModel::userId, "@bob:example.com")
+ | withAttr(&ClientModel::deviceId, "bobdevice")
+ );
+
+ auto aliceClient = makeClient(
+ withCrypto(makeCrypto())
+ | withAttr(&ClientModel::userId, "@alice:example.com")
+ | withAttr(&ClientModel::deviceId, "alicedevice")
+ );
+
+ // only A knows B, but B does not have A's identity keys
+ // this makes B unable to verify A's event, thus rejecting it
+ queryOne(aliceClient, bobClient);
+ createAndClaimOneTimeKey(aliceClient, bobClient);
+
+ auto plainJson = json{
+ {"content", json::object()},
+ {"type", "moe.kazv.mxc.placeholder"},
+ };
+
+ auto encrypted = Event(encryptToOneDevice(aliceClient, bobClient, plainJson));
+
+ bobClient.toDevice = {encrypted};
+
+ auto decryptCycle = [](ClientModel &m, int expectedTimes) {
+ m = tryDecryptEvents(m);
+ REQUIRE(m.toDevice.size() == 1);
+ auto event = m.toDevice.at(0);
+ REQUIRE(event.encrypted());
+ REQUIRE(!event.decrypted());
+ REQUIRE(event.decryptedJson().get().at("content").at("moe.kazv.mxc.failed_times") == expectedTimes);
+ REQUIRE(event.decryptedJson().get().at("content").at("moe.kazv.mxc.errcode") == "MOE.KAZV.MXC_DEVICE_KEY_UNKNOWN");
+ };
+
+ int maxFail = 10;
+ for (int i = 0; i < maxFail; ++i) {
+ decryptCycle(bobClient, i + 1);
+ }
+
+ // the 11th time it encounters a failure, it should not attempt to decrypt this event
+ decryptCycle(bobClient, 10);
}
TEST_CASE("decryptEvent() handles Olm-encrypted to-device event", "[client][encryption][olm]")

File Metadata

Mime Type
text/plain
Expires
Fri, Aug 28, 9:18 PM (11 h, 15 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1737239
Default Alt Text
D360.1787977110.diff (19 KB)

Event Timeline