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

MSC2723: Forwarded message metadata #2723

Open
wants to merge 7 commits into
base: old_master
Choose a base branch
from
Open
Changes from 6 commits
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
101 changes: 101 additions & 0 deletions proposals/2723-add-forward-info.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# MSC2723: Add forward information to forwarded messages

Currently a forwarded message is not easily recognized as a forwarded message. While for messages of `msgtype` `m.text`, `m.emote` and `m.notice` clients could do something in the `formatted_body` of `content`, for all other message types the forward highlighting would be very poor. To get around this and provide a guideline to clients which information should go with a forward, we suggest adding the "forward info" explicitly. See also https://github.com/vector-im/riot-web/issues/4747.

## Proposal

### Providing m.forward
hummlbach marked this conversation as resolved.
Show resolved Hide resolved

Leaning onto edits (adding `m.new_content` inside `content`), we want to suggest to add `m.forwarded` to `content` to forwarded messages. The required information would cover at least the original `senden`, `room_id` and `origin_server_ts`:
hummlbach marked this conversation as resolved.
Show resolved Hide resolved

```
{
"content": {
"body": "Big Ben, London, UK",
"geo_uri": "geo:51.5008,0.1247",
"m.forwarded": {
"event_id": "$123275682943PhrSn:example.org",
"room_id": "!jEsTZKDJdhfrheTzSU:example.org",

Choose a reason for hiding this comment

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

In some cases rooms are "private by knowledge" in this case sharing the room_id may inadvertently let point join. This has to be considered carefully and I think this MSC should at least include a warning that implementing clients should warn users.

"sender": "@someone:example.org",
"origin_server_ts": 1432735824141
},
"msgtype": "m.location"
},
"event_id": "$143273582443PhrSn:example.org",
"origin_server_ts": 1432735824653,
"room_id": "!jEsUZKDJdhlrceRyVU:example.org",
"sender": "@example:example.org",
"type": "m.room.message",
"unsigned": {
"age": 1234
}
}
```

Additional the original `event_id` could be added.
hummlbach marked this conversation as resolved.
Show resolved Hide resolved

## Potential issues

### Resolving display name and avatar
Since the receiver (of a forward) may not be in the room, the message has originally been posted to, he may not be able to get the original sender's `displayname` and `avatar_url` from `/_matrix/client/r0/rooms/{roomId}/members`.

We see two possible solutions at the moment:

1. The forwarder adds `displayname` and `avatar_url` to `m.forwarded`.
2. The receiving client resolves the `displayname` and the `avatar_url` from the user id given by `sender` using `/_matrix/client/r0/profile/{userId}`.

Both solutions have a drawback. In case of 1., changing the display name or the avatar would not be reflected in forwards. And the avatar URL may even become invalid(?). The second solution may cause confusion is the original sender has set different display names and avatars for different rooms. I.e. if the original sender is in the room where the message is forwarded to, it may appear there under a different display name and avatar.

### Clients can fake forwards
Should we care of/can we avoid "fake forwards"? Does it make sense/is it possible at all to only add the original `event_id` when sending a forward and assign the server the responsibility of adding the forward information?

## Alternatives
Copy link
Member

@tulir tulir Aug 12, 2020

Choose a reason for hiding this comment

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

Has including the whole federation event data been considered (i.e. include the signatures and everything)? Having the signatures and exact content would allow the recipient servers to validate the event and thus prevent faking forwards.

The only drawbacks I can think of are:

  • it needs server support to send and validate forwards
  • forwarding big events (close to the 64 KiB federation limit) wouldn't be possible

I might be missing something, but even if it's not a viable option, the reasoning should be documented.

Copy link
Author

Choose a reason for hiding this comment

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

No, we haven't considered that yet (at least not me).

Copy link
Contributor

Choose a reason for hiding this comment

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

Then the client would need to verify the federation signatures, though, can it even do that?

Copy link
Contributor

Choose a reason for hiding this comment

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

The server could verify that the signatures / content of the included event were signed by the homeserver that originally sent it. The homeserver could include a key in unsigned to notify the client about it's trustworthyness.

Copy link
Member

Choose a reason for hiding this comment

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

I went ahead and drafted something for this: #2730

Choose a reason for hiding this comment

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

The problem with forwarding the whole event with signatures is that it becomes impossible to redact data that should not be shared. This means that we are committing to including all data covered by the signature in a forward which may be more information than was intended to be shared.


### Extending info

```
{
"content": {
"body": "Big Ben, London, UK",
"geo_uri": "geo:51.5008,0.1247",
"info": {
"forward_info": {
"event_id": "$123275682943PhrSn:example.org",
"room_id": "!jEsTZKDJdhfrheTzSU:example.org",
"sender": "@someone:example.org",
"origin_server_ts": 1432735824141
}
},
"msgtype": "m.location"
},
"event_id": "$143273582443PhrSn:example.org",
"origin_server_ts": 1432735824653,
"room_id": "!jEsUZKDJdhlrceRyVU:example.org",
"sender": "@example:example.org",
"type": "m.room.message",
"unsigned": {
"age": 1234
}
}
```

### Discarded: Using m.relates_to
We've also discussed and discarded usind `m_relates_to` for highlighting the message as forward, like the following:

```
"m.relates_to": {
"rel_type": "m.forwarded",
"event_id": "!1234:server.abc",
}
```

We discarded this idea for two reasons:

1. The idea of `m.relates_to` seems to be that related messages belong to the same room.
2. Its unclear who should fetch the event from a different room she/he/it is potentially not in and how this could be done at all.

## Unstable prefix
Clients can implement this feature with the unstable prefix `com.famedly.app.forwarded` onstead of `m.forwarded` until this MSC gets merged.
hummlbach marked this conversation as resolved.
Show resolved Hide resolved


See also our discussion here https://gitlab.com/famedly/app/-/issues/320.
hummlbach marked this conversation as resolved.
Show resolved Hide resolved