Some clarifications and style fixes in E2EE-related areas #1088

dkasak · 2022-05-26T14:06:10Z

Changes include:

Link to correct Megolm session sharing formats:
- m.room_key -> session sharing format
- m.forwarded_room_key -> session export format
- Server-side Megolm session backups -> session export format
- Megolm session file exports -> session export format
Remove some points of ambiguity regarding terminology.
Adding links to other parts of the spec.
Stylistic fixes (rewording for consistency, capitalization, etc).
Fix for Broken link in "Protocol definitions" section #1080.

Preview: https://pr1088--matrix-spec-previews.netlify.app

The explanation is copied from another place for consistency. There's now a link to the Signing JSON section of the spec.

The term "identity key" is ambiguous given it's somewhat common to refer to the Curve25519 key as the identity key. So this commit just removes the word "identity" to remove the ambiguity.

The Megolm session backups don't actually use the session sharing format, but the session export format (which is the same except for having no signature at the end).

turt2live · 2022-05-28T02:03:04Z

content/client-server-api/modules/end_to_end_encryption.md

-| -----------| -----------|--------------------------------------------------------------------------------------------------|
-| public_key | string     | **Required.** The curve25519 public key used to encrypt the backups, encoded in unpadded base64. |
-| signatures | Signatures | Optional. Signatures of the ``auth_data``, as Signed JSON                                        |
+| Parameter  | Type       | Description                                                                                                           |


we have ways of generating these tables which we should start using for the crypto section too. See #1062 for how we described m.relates_to, for example.

Unfortunately, I don't have time to figure this one out right now. Mind if we postpone it to a later PR?

Preferably not, as otherwise it won't ever get fixed.

I'd prefer to convert the tables too, but really can't afford to fiddle with this now. But the changes are useful on their own and unrelated to the table generation method, no?

The changes might be useful, but we really can't afford to keep putting off the tech debt.

we're aware that contributing to the spec isn't as easy as it could be, but not converting things to OpenAPI definitions doesn't help us fix that :(

If you're okay with it, I can add this to my list of things to look at/take over to do the conversion. No promises on when or how quickly that will happen though.

Yeah, if you do get around to it before me, feel free to do it. Thanks!

I'd rather get these changes landed than block them behind a refactoring. Ideally we'd separate refactoring from wording changes anyway.

we can separate them by commit, but I really don't think we should be putting effort into maintaining things we shouldn't be maintaining.

ok, I have made these changes in #1166. Unfortunately it is going to introduce conflicts with this PR which will now need to be resolved.

turt2live · 2022-05-28T02:42:53Z

while in the area, can I suggest a change to fix #1080 ? 😇

Closes matrix-org#1080.

richvdh

looks like a significant improvement - a few nits to clean up though

richvdh · 2022-06-14T14:38:38Z

content/client-server-api/modules/end_to_end_encryption.md

+| Parameter  | Type       | Description                                                                                                           |
+| -----------| -----------|---------------------------------------------------------------------------------------------------------------------- |
+| public_key | string     | **Required.** The curve25519 public key used to encrypt the backups, encoded in unpadded base64.                      |
+| signatures | Signatures | *Optional.* Signatures of the ``auth_data``. The signature is calculated using the process described at Signing JSON. |


Suggested change

| signatures | Signatures | *Optional.* Signatures of the ``auth_data``. The signature is calculated using the process described at Signing JSON. |

| signatures | Signatures | *Optional.* Signatures of the ``auth_data``. The signature is calculated using the process described at [Signing JSON](/appendices/#signing-json). |

richvdh · 2022-06-14T14:52:08Z

content/client-server-api/modules/end_to_end_encryption.md

-| -----------| -----------|--------------------------------------------------------------------------------------------------|
-| public_key | string     | **Required.** The curve25519 public key used to encrypt the backups, encoded in unpadded base64. |
-| signatures | Signatures | Optional. Signatures of the ``auth_data``, as Signed JSON                                        |
+| Parameter  | Type       | Description                                                                                                           |


I'd rather get these changes landed than block them behind a refactoring. Ideally we'd separate refactoring from wording changes anyway.

richvdh · 2022-06-14T14:52:45Z

content/client-server-api/modules/end_to_end_encryption.md

-| sender_claimed_keys             | {string: string}  | **Required.** A map from algorithm name (`ed25519`) to the identity key for the sending device.                                                                             |
-| session_key                     | string            | **Required.** Unpadded base64-encoded session key in [session-sharing format](https://gitlab.matrix.org/matrix-org/olm/blob/master/docs/megolm.md#session-sharing-format).  |
+| sender_key                      | string            | **Required.** Unpadded base64-encoded device Curve25519 key.                                                                                                                |
+| sender_claimed_keys             | {string: string}  | **Required.** A map from the algorithm name (`ed25519`) to the corresponding device key of the sending device.                                                              |


I think it's better to be explicit, even if it is repetitive:

Suggested change

| sender_claimed_keys | {string: string} | **Required.** A map from the algorithm name (`ed25519`) to the corresponding device key of the sending device. |

| sender_claimed_keys | {string: string} | **Required.** A map from the algorithm name (`ed25519`) to the Ed25519 signing key of the sending device. |

richvdh · 2022-06-14T14:56:10Z

content/client-server-api/modules/end_to_end_encryption.md

+| sender_key                        | string           | **Required.** The Curve25519 key of the device which initiated the session originally.                                                    |
+| sender_claimed_keys               | {string: string} | **Required.** The Ed25519 key of the device which initiated the session originally.                                                       |
+| session_id                        | string           | **Required.** The Megolm session ID.                                                                                                      |
+| session_key                       | string           | **Required.** The Megolm session key in the [session export format](https://gitlab.matrix.org/matrix-org/olm/blob/master/docs/megolm.md#session-export-format).|


It'd be nice if this table could use the same wording as in the "Server-side key backups" section (I think they are the same thing).

(possibly an argument in favour of moving it out to yaml objects, but we can just repeat it for now)

richvdh · 2022-06-14T14:57:36Z

content/client-server-api/modules/end_to_end_encryption.md

@@ -1372,7 +1372,7 @@ Example:
        "room_id": "!Cuyf34gef24t:localhost",
        "sender_key": "RF3s+E7RkTQTGF2d8Deol0FkQvgII2aJDf3/Jp5mxVU",
        "sender_claimed_keys": {
-            "ed25519": "<device ed25519 identity key>",
+            "ed25519": "<device ed25519 key>",


According to https://spec.matrix.org/v1.2/client-server-api/#device-keys this is called the "signing key", so:

Suggested change

"ed25519": "<device ed25519 key>",

"ed25519": "<device ed25519 signing key>",

ymmv though

richvdh · 2022-06-14T15:02:46Z

data/event-schemas/schema/m.room.encrypted.yaml

+  within a room (in which case it will have all of the [Room Event
+  fields](/client-server-api/#room-event-format)), or as


Suggested change

within a room (in which case it will have all of the [Room Event

fields](/client-server-api/#room-event-format)), or as

within a room (in which case it will have all of the [Room Event

fields](/client-server-api/#room-event-format) when it is returned

from a homeserver to a client via the Client-Server API), or as

anoadragon453 · 2022-10-04T08:55:27Z

Hi @dkasak, do you have time to finish this up? The changes look valuable!

dkasak · 2022-10-04T10:40:59Z

Hi, yes, haven't forgotten about it, just the disclosure was keeping me busy. Will be coming back to this shortly.

richvdh · 2024-02-27T12:48:58Z

I'm going to go ahead and close this for now. @dkasak, if you have time to return to it in the future, that would be very much appreciated and we'll be delighted to reopen it!

dkasak added 10 commits May 26, 2022 12:33

Bold "Required" in key export format for consistency.

c436576

Clarify some key export format and session backup fields.

b994072

Stylistic fixes.

73a7c7e

Explain what is meant by Signed JSON.

6e4159b

The explanation is copied from another place for consistency. There's now a link to the Signing JSON section of the spec.

Remove ambiguity regarding ed25519 device key reference.

79633a1

The term "identity key" is ambiguous given it's somewhat common to refer to the Curve25519 key as the identity key. So this commit just removes the word "identity" to remove the ambiguity.

Stylistic fixes.

7122ceb

Consistently capitalise "Olm" and "Megolm".

b5b25c2

Fix a lie regarding the Megolm session in backups.

8007f07

The Megolm session backups don't actually use the session sharing format, but the session export format (which is the same except for having no signature at the end).

Link to Megolm session sharing/export format in more places.

ed89f4c

Add changelog.

cce8e57

turt2live reviewed May 28, 2022

View reviewed changes

Fix link to Room Event format.

f52fde4

Closes matrix-org#1080.

dkasak marked this pull request as ready for review May 31, 2022 11:05

dkasak requested a review from a team as a code owner May 31, 2022 11:05

turt2live self-requested a review May 31, 2022 21:47

richvdh reviewed Jun 14, 2022

View reviewed changes

richvdh mentioned this pull request Jul 12, 2022

Move various e2e defintions out to yaml files #1166

Merged

dkasak self-assigned this Oct 4, 2022

richvdh closed this Feb 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Some clarifications and style fixes in E2EE-related areas #1088

Some clarifications and style fixes in E2EE-related areas #1088

dkasak commented May 26, 2022 •

edited

Loading

turt2live May 28, 2022

dkasak May 31, 2022

turt2live May 31, 2022

dkasak May 31, 2022

turt2live May 31, 2022

turt2live May 31, 2022

dkasak Jun 7, 2022

richvdh Jun 14, 2022

turt2live Jun 14, 2022

richvdh Jul 12, 2022

turt2live commented May 28, 2022

richvdh left a comment

richvdh Jun 14, 2022

richvdh Jun 14, 2022

richvdh Jun 14, 2022

richvdh Jun 14, 2022

richvdh Jun 14, 2022

richvdh Jun 14, 2022

anoadragon453 commented Oct 4, 2022

dkasak commented Oct 4, 2022

richvdh commented Feb 27, 2024

	\| signatures \| Signatures \| Optional. Signatures of the ``auth_data``. The signature is calculated using the process described at Signing JSON. \|
	\| signatures \| Signatures \| Optional. Signatures of the ``auth_data``. The signature is calculated using the process described at [Signing JSON](/appendices/#signing-json). \|

	\| sender_claimed_keys \| {string: string} \| Required. A map from the algorithm name (`ed25519`) to the corresponding device key of the sending device. \|
	\| sender_claimed_keys \| {string: string} \| Required. A map from the algorithm name (`ed25519`) to the Ed25519 signing key of the sending device. \|

	"ed25519": "<device ed25519 key>",
	"ed25519": "<device ed25519 signing key>",

		within a room (in which case it will have all of the [Room Event
		fields](/client-server-api/#room-event-format)), or as

Some clarifications and style fixes in E2EE-related areas #1088

Some clarifications and style fixes in E2EE-related areas #1088

Conversation

dkasak commented May 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turt2live commented May 28, 2022

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anoadragon453 commented Oct 4, 2022

dkasak commented Oct 4, 2022

richvdh commented Feb 27, 2024

dkasak commented May 26, 2022 •

edited

Loading