-
-
Notifications
You must be signed in to change notification settings - Fork 585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
keep track of event ID and timestamp of decrypted messages #555
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
// it from this map. Otherwise if we download the event from the server we | ||
// will think that it is a duplicate. | ||
// When we decrypt a message and the message index matches a previously | ||
// decrypted message, one possible cause of that is that we are decrypting | ||
// 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wouldn't worry about this |
||
|| self._inboundGroupSessionMessageIndexes[messageIndexKey].id !== eventId | ||
|| self._inboundGroupSessionMessageIndexes[messageIndexKey].timestamp !== timestamp)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to fix the lint, pull out a local var as @krombel suggested in riot-dev. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know why I called it "suggestion" here 😎 |
||
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); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you specify the keys here?
"values are objects of the format
{id: <event id>, timestamp: <ts>}
"