keep track of event ID and timestamp of decrypted messages

This is to avoid false positives when detecting replay attacks.

fixes: vector-im/riot-web#3712

Signed-off-by: Hubert Chathi <hubert@uhoreg.ca>

spec/unit/crypto/algorithms/megolm.spec.js

@@ -181,5 +181,88 @@ describe("MegolmDecryption", function() {
                 expect(payload.content.session_key).toExist();
             });
         });
+
+        it("can detect replay attacks", function() {
+            // trying to decrypt two different messages (marked by different
+            // event IDs or timestamps) using the same (sender key, session id,
+            // message index) triple should result in an exception being thrown
+            // as it should be detected as a replay attack.
+            const sessionId = groupSession.session_id();
+            const cipherText = groupSession.encrypt(JSON.stringify({
+                room_id: ROOM_ID,
+                content: 'testytest',
+            }));
+            const event1 = new MatrixEvent({
+                type: 'm.room.encrypted',
+                room_id: ROOM_ID,
+                content: {
+                    algorithm: 'm.megolm.v1.aes-sha2',
+                    sender_key: "SENDER_CURVE25519",
+                    session_id: sessionId,
+                    ciphertext: cipherText,
+                },
+                event_id: "$event1",
+                origin_server_ts: 1507753886000,
+            });
+
+            const successHandler = expect.createSpy();
+            const failureHandler = expect.createSpy()
+                .andCall((err) => {
+                    expect(err.toString()).toMatch(
+                        /Duplicate message index, possible replay attack/,
+                    );
+                });
+
+            return megolmDecryption.decryptEvent(event1).then((res) => {
+                const event2 = new MatrixEvent({
+                    type: 'm.room.encrypted',
+                    room_id: ROOM_ID,
+                    content: {
+                        algorithm: 'm.megolm.v1.aes-sha2',
+                        sender_key: "SENDER_CURVE25519",
+                        session_id: sessionId,
+                        ciphertext: cipherText,
+                    },
+                    event_id: "$event2",
+                    origin_server_ts: 1507754149000,
+                });
+
+                return megolmDecryption.decryptEvent(event2);
+            }).then(
+                successHandler,
+                failureHandler,
+            ).then(() => {
+                expect(successHandler).toNotHaveBeenCalled();
+                expect(failureHandler).toHaveBeenCalled();
+            });
+        });
+
+        it("allows re-decryption of the same event", function() {
+            // in contrast with the previous test, if the event ID and
+            // timestamp are the same, then it should not be considered a
+            // replay attack
+            const sessionId = groupSession.session_id();
+            const cipherText = groupSession.encrypt(JSON.stringify({
+                room_id: ROOM_ID,
+                content: 'testytest',
+            }));
+            const event = new MatrixEvent({
+                type: 'm.room.encrypted',
+                room_id: ROOM_ID,
+                content: {
+                    algorithm: 'm.megolm.v1.aes-sha2',
+                    sender_key: "SENDER_CURVE25519",
+                    session_id: sessionId,
+                    ciphertext: cipherText,
+                },
+                event_id: "$event1",
+                origin_server_ts: 1507753886000,
+            });
+
+            return megolmDecryption.decryptEvent(event).then((res) => {
+                return megolmDecryption.decryptEvent(event);
+                // test is successful if no exception is thrown
+            });
+        });
     });
 });

src/crypto/OlmDevice.js

@@ -96,12 +96,18 @@ function OlmDevice(sessionStore) {
     // This partially mitigates a replay attack where a MITM resends a group
     // message into the room.
     //
-    // TODO: If we ever remove an event from memory we will also need to remove
+    // When we decrypt a message and the message index matches a previously
-    // it from this map. Otherwise if we download the event from the server we
+    // decrypted message, one possible cause of that is that we are decrypting
-    // will think that it is a duplicate.
+    // the same event, and may not indicate an actual replay attack.  For
+    // example, this could happen if we receive events, forget about them, and
+    // then re-fetch them when we backfill.  So we store the event ID and
+    // timestamp corresponding to each message index when we first decrypt it,
+    // and compare these against the event ID and timestamp every time we use
+    // that same index.  If they match, then we're probably decrypting the same
+    // event and we don't consider it a replay attack.
     //
     // Keys are strings of form "<senderKey>|<session_id>|<message_index>"
-    // Values are true.
+    // Values are objects containing the event ID and timestamp.
     this._inboundGroupSessionMessageIndexes = {};
 }
 
@@ -794,6 +800,8 @@ OlmDevice.prototype.importInboundGroupSession = async function(data) {
  * @param {string} senderKey base64-encoded curve25519 key of the sender
  * @param {string} sessionId session identifier
  * @param {string} body      base64-encoded body of the encrypted message
+ * @param {string} eventId   ID of the event being decrypted
+ * @param {Number} timestamp timestamp of the event being decrypted
  *
  * @return {null} the sessionId is unknown
  *
@@ -802,9 +810,10 @@ OlmDevice.prototype.importInboundGroupSession = async function(data) {
  *    keysClaimed: Object<string, string>}>}
  */
 OlmDevice.prototype.decryptGroupMessage = async function(
-    roomId, senderKey, sessionId, body,
+    roomId, senderKey, sessionId, body, eventId, timestamp,
 ) {
     const self = this;
+    const argumentsLength = arguments.length;
 
     function decrypt(session, sessionData) {
         const res = session.decrypt(body);
@@ -815,14 +824,23 @@ OlmDevice.prototype.decryptGroupMessage = async function(
             plaintext = res;
         } else {
             // Check if we have seen this message index before to detect replay attacks.
+            // If the event ID and timestamp are specified, and the match the event ID
+            // and timestamp from the last time we used this message index, then we
+            // don't consider it a replay attack.
             const messageIndexKey = senderKey + "|" + sessionId + "|" + res.message_index;
-            if (messageIndexKey in self._inboundGroupSessionMessageIndexes) {
+            if (messageIndexKey in self._inboundGroupSessionMessageIndexes
+                && (argumentsLength <= 4 // Compatibility for older old versions.
+                    || self._inboundGroupSessionMessageIndexes[messageIndexKey].id !== eventId
+                    || self._inboundGroupSessionMessageIndexes[messageIndexKey].timestamp !== timestamp)) {
                 throw new Error(
                     "Duplicate message index, possible replay attack: " +
                     messageIndexKey,
                 );
             }
-            self._inboundGroupSessionMessageIndexes[messageIndexKey] = true;
+            self._inboundGroupSessionMessageIndexes[messageIndexKey] = {
+                id: eventId,
+                timestamp: timestamp,
+            };
         }
 
         sessionData.session = session.pickle(self._pickleKey);

src/crypto/algorithms/megolm.js

@@ -628,6 +628,7 @@ MegolmDecryption.prototype.decryptEvent = async function(event) {
     try {
         res = await this._olmDevice.decryptGroupMessage(
             event.getRoomId(), content.sender_key, content.session_id, content.ciphertext,
+            event.getId(), event.getTs(),
         );
     } catch (e) {
         if (e.message === 'OLM.UNKNOWN_MESSAGE_INDEX') {