Add getKeysProved and getKeysClaimed methods to MatrixEvent. #206
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These list the keys that sender of the event must have ownership of and the keys of that the sender claims ownership of. All olm and megolm messages prove ownership of a curve25519 key. All new olm and megolm message will now claim ownership of a ed25519 key. This allows us to detect if an attacker claims ownership of a curve25519 key they don't own when advertising their device keys, because when we receive an event from the original user it will have a different ed25519 key to the attackers.
|
@matrixbot retest this please |
|
This should allow us to fix element-hq/element-web#2215 |
|
looks plausible |
richvdh
reviewed
Sep 16, 2016
| @@ -569,7 +571,12 @@ OlmDevice.prototype._getInboundGroupSession = function( | |||
| var session = new Olm.InboundGroupSession(); | |||
| try { | |||
| session.unpickle(this._pickleKey, r.session); | |||
| return {sessionExists: true, result: func(session)}; | |||
| return { | |||
richvdh
reviewed
Sep 16, 2016
| * @param {string} sessionId | ||
| * @param {Olm.InboundGroupSession} session | ||
| * @param {object} keysClaimed Other keys the sender claims. |
There was a problem hiding this comment.
- It's a shame that this doesn't specify the format of the object. But that doesn't matter too much, because it's a private method.
- It does't appear to be set by either
addInboundGroupSessionordecryptGroupMessage.
| @@ -200,7 +211,13 @@ OlmDecryption.prototype.decryptEvent = function(event) { | |||
| // TODO: Check the sender user id matches the sender key. | |||
| // TODO: check the room_id and fingerprint | |||
| if (payloadString !== null) { | |||
| return {result: JSON.parse(payloadString), sessionExists: true}; | |||
| var payload = JSON.parse(payloadString); | |||
| return { | |||
There was a problem hiding this comment.
(see also algorithms/base.js which has yet another copy of the jsdoc. sorry.)
richvdh
reviewed
Sep 16, 2016
| if (r.sessionExists) { | ||
| return r.result; | ||
| return payload; |
| @@ -233,12 +233,28 @@ module.exports.MatrixEvent.prototype = { | |||
| return Boolean(this._clearEvent.type); | |||
| }, | |||
|
|
|||
| /** | |||
| * The curve25519 key that sent this event | |||
| * @return {string} | |||
| * @return {object} | ||
| */ | ||
| getKeysProved: function() { | ||
| return this._clearEvent.keysProved || {}; |
There was a problem hiding this comment.
I'm not happy with messing about with the contents of _clearEvent. It's supposed to be exactly what was sent on the wire. keysProved and keysClaimed should be in different properties.
This was referenced Sep 19, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These list the keys that sender of the event must have ownership
of and the keys of that the sender claims ownership of.
All olm and megolm messages prove ownership of a curve25519 key.
All new olm and megolm message will now claim ownership of a
ed25519 key.
This allows us to detect if an attacker claims ownership of a curve25519
key they don't own when advertising their device keys, because when we
receive an event from the original user it will have a different ed25519 key
to the attackers.