Skip to content
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

MSC2757: Sign Events #2757

Open
wants to merge 4 commits into
base: old_master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 167 additions & 0 deletions proposals/2757-sign-events.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
# MSC2757: Sign Events

Currently the homeserver is trusted to not modify events in cleartext rooms. To resolve this you can
turn on end-to-end encryption in a given room. That, however, adds a lot of overhead and is pointless
for big, public rooms. As such, it would be nice to be able to sign your own events you send, so that
the recipient can ensure that the homeserver did not tamper with the message itself.

## Proposal

The general idea is to sign the `content` of each event you send out with your own ed25519 key and a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered how redactions will be handled?

If the new content signature is covered by the existing hash/signature then it will not be able to be redacted when the ”content” is removed. If it isn’t covered by the existing hash/signature then it is also forgeable by the homeserver.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, redactions have actually not be considered yet. However, why would they have to be considered specially? The homeserver can just clear out the content alltogether for redactions, like it currently does.

A server will alwasy be able to forge an event being redacted or simply not deliver an event properly. Soru is not sure what a preventable attack vector would be here.

new `message_signing` key. Signing with your own ed25519 key is not enough, as that makes the signature
unverifyable after you log out of your own device. Only using a `message_signing` key is not enough,
as signing with your own device key is a nice mitigation for a client that does not support cross-signing yet

### The `message_signing` key

The `message_signing` key would be an ed25519 key similar to the cross-sigining `user_signing` and
`self-signing` keys: It is signed by your own master key, uploaded and distributed via the same
endpoints.

As `usage` it is expected to have `message_signing` listed.

#### POST `/_matrix/client/r0/keys/signatures/upload`

A new key, `message_signing_key`, is introduced, which uploads the message singing key, signed by the
master key. As usual, when a user uploads or changes a message singing key, the user ID of that person
should appear in the `changed` field of the `/sync` reply of other users. An example payload could
look as following:

```json
{
"master_key": {
"user_id": "@alice:example.com",
"usage": ["master"],
"keys": {
"ed25519:base64+master+public+key": "base64+master+public+key",
}
},
"self_signing_key": {
"user_id": "@alice:example.com",
"usage": ["self_signing"],
"keys": {
"ed25519:base64+self+signing+public+key": "base64+self+signing+public+key",
},
"signatures": {
"@alice:example.com": {
"ed25519:base64+master+public+key": "signature+of+self+signing+key"
}
}
},
"user_signing_key": {
"user_id": "@alice:example.com",
"usage": ["user_signing"],
"keys": {
"ed25519:base64+user+signing+public+key": "base64+user+signing+public+key",
},
"signatures": {
"@alice:example.com": {
"ed25519:base64+master+public+key": "signature+of+user+signing+key"
}
}
},
"message_signing_key": {
"user_id": "@alice:example.com",
"usage": ["message_signing"],
"keys": {
"ed25519:base64+message+signing+public+key": "base64+message+signing+key"
},
"signatures": {
"@alice:example.com": {
"ed25519:base64+master+public+key": "signature+of+message+signing+key"
}
}
}
}
```

#### POST `/_matrix/client/r0/keys/query`

Similar to `user_signing` keys etc. a new dict, `message_signing_keys` is introduced.

#### Visibility

Similarly to the master key, everyone should be able to see a persons message signing key, if they
share a room with them. This is required to be abe to verify signatures of messages.

#### Storage in SSSS

The private key can be stored base64-encoded in SSSS under the key `m.message_signing`
(`m.cross_signing.message_signin`? Calling these keys "cross-signing" was prolly not too descriptive...).

### Signing messages
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's worthwhile defining signing of state events

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would that be needed? This MSC outlines signing events in general, and that should also include state events already?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The title strongly implies this just signs messages. Also, you may want to add the state_key to the part that gets signed, similar to how the event type is handled.

Copy link
Contributor

@Half-Shot Half-Shot Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't, because you do not include the state_key in the stripped content. You'd need to define that. (argh message races :|)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So like event_type + state_key + canonical_json, where if state_key is a blank string we have "normal" PDUs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess one aspect is if that you banned person A state_key=@alice:example.com, and the server changed that to say state_key=@bob:example.com, it would look to others like you had banned bob.

If the server was clever enough to hide that fact from you somehow, it could look, at least for a bit, like you had done something malicious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would still be signed as state_key=@alice:example.com, so the signature would not match

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't get soru wrong, event type and state key are included in the signature as per this MSC, there is just no separater, so it is just event_type + state_key + canonical_json

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right. Yes, I don't think you can really attack it. I defer to a spec team grey-beard on whether they want separators because I think it's mostly a style thing at this point.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even with separators you could always construct event types / state keys to mimic those.

Depends how you design the separators, you could of course sign:

{
  "type": "something",
  "state_key": "blub",
  "content": {...}
}

Is that an attack vector at all?

I don't think so.


To sign a message you strip the `signatures` and `unsigned` dicts off of the `content` (if present),
Copy link
Contributor

@Half-Shot Half-Shot Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

content.unsigned is a new dict for storing things you do not wish to sign? Could you define it's purpose formally in another paragraph. I'm actually not too sure why you wouldn't want to sign part of your content?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its purpose mostly exists to stay in-line with everything else that is signable in the matrix spec so far

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Righto, so in the C-S api representation of events, the unsigned section is for the local server to drop in bits of related information that hasn't been signed by the sending server (usually because it's an age value, or prev_state, which is mutable).

In this case we don't expect any third parties to drop in bits of information into content.unsigned because servers have a section for that. Since the client is sending immutable information, I'd expect they will want to sign everything so the section feels superfluous to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be easiest to include it already in case we have usecases for it in the future. Will note in the MSC that it doesn't really have any usecase yet. Does that seem OK?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think until someone comes up with a valid use case for it, it's probably not worth speccing it (given these MSCs take ages to go through anyway, there should be ample time for someone to shout).

I'm just thinking that because content is guaranteed to be immutable, there is little argument for including it. Sorry :(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is little argument for including it.

Greater code simplicity as signing code clients have will already strip signatures and unsigned

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a valid usecase: A proxy (like pantalaimon, only pan doesn't make sense as pan does e2ee, but a different kind of proxy) could add additional information into the unsigned block.

encode it with canonical json and prepend the event type. Then you sign it with the message_signing
and device ed25519 key.

For example, a message of type `m.room.message` with the following content:
```json
{
"msgtype": "m.text",
"body": "foxies!",
"unsigned": {
"super secret": "wha!"
}
}
```

Would yield the following string needing to be signed: `m.room.message{"body":"foxies!","msgtype":"m.text"}`

Prepending the event type is done to rule out attack vectors where the server could modify the type of
an event.

After that, the generated signatures are added in a signatures dict to the content, similar as done
elsewhere:

```json
{
"msgtype": "m.text",
"body": "foxies!",
"unsigned": {
"super secret": "wha!"
},
"signatures": {
"@alice:example.com": {
"ed25519:base64+message+signing+key": "signature+of+message+signing+key",
"ed25519:device+id": "signature+of+device+id"
}
}
}
```

This new content is then used to send messages via `/_matrix/client/r0/rooms/{roomId}/send/{eventType}/{txnId}`.

### Signing messages in encrypted rooms

To add additional verification and authenticity it is a good idea to also sign messages in encrypted
rooms. The encrypted content should be signed.

Sorunome marked this conversation as resolved.
Show resolved Hide resolved
### Verifying message signatures

If signatures are present and bad, a client MUST at least display a warning, or completely hide that
message.

If signatures are present and good, clients MAY indicate that in the UI somehow, e.g. by a green
checkmark.

## Potential issues
Sorunome marked this conversation as resolved.
Show resolved Hide resolved

The introduction of a new message_signing key will force users to enter their recovery passphrase
to enable this feature, as the master key is typically not cached. Additionally, the message_signin
Sorunome marked this conversation as resolved.
Show resolved Hide resolved
key should be cached on devices, so that the user doesn't have to enter their recovery key every time
they want to send a message.

## Alternatives

Instead of introducing a new message_signing key, the self_signing key could be re-used for that
purpose, as the visibility of those should be the same. The two keys, however, have different
purposes. Additionally, the message_signing key should be cached, making it potentially more vulnerable
to being stolen. As such, it is a good idea to keep the two separate, to be able to revoke them
independently.

## Security considerations

The message_signing key should be cached by clients, but it shouldn't be stolen by a third party.
Thus, clients will have to think about themselves how to resolve this issue, e.g. by using a securly
encrypted store provided by their platform.
Sorunome marked this conversation as resolved.
Show resolved Hide resolved