support POST requests to preview_url

[boontifex]

Hello,
we reviewed our homeserver and reverse proxy logs and discovered the following issues that may effect integrity of matrix.
At first we figured out that the element client calls the endpoint preview_url with GET method for encrypted rooms even if the endpoint was disabled on synapse server. This happens every time a user sends a message that only contains a url.

We reported that behavior as an issue under matrix-org/synapse#11591.
While discussing this @squahtx told that the the synapse homeserver replaces (redact) sensitive data like access_token and clients_access within the logging string.

In our opinion, redacting these events in homeserver isn't the solution. Sensitive data like access_token, client_access or urls shouldn't have to send with GET, because this will expose potential sensible data to the synapse server, the reverse proxy and every services that parse these logs.

First thoughts:

• change GET /_matrix/media/v3/preview_url to POST
• promote activated features to clients (aka smtp ehlo)
• remove endpoint preview_url endpoint
• disable support of sending access_token via get

best regards

[dkasak]

Note that while the spec allows passing the access token in a GET query string, it strongly discourages from using this mechanism, instead passing the access token in an Authorization header, even for GET requests. This is what Element does.

[richvdh]

It's also worth noting that access tokens as they currently stand are planned to be phased out as part of a migration toOAuth2 (see matrix-org/matrix-spec-proposals#2964). In the meantime, I suggest filing bugs against any clients which put access tokens in query parameters.

promote activated features to clients (aka smtp ehlo)
remove endpoint preview_url endpoint

I'm afraid I don't understand what either of these mean.

[boontifex]

@dkasak yes, i read understand this part. While its state of the art is to encrypt communications per default, authentication data should not appear in plain text in the logs. Yes in fact, it's possible to redact Events in Logfiles but in my opinion, the specification should allow only "secure" methods.

@richvdh we found preview_urls in plain-text in our proxy and homeserver logs even if preview_url was disabled in our synapse server configuration. These are only some first thoughts how to deal with that:

1. In my opinion, users relay on the confidentiality that came with encryption and it shouldn't matter if the user sends a normal message or a url. So preview_url should use POST.
2. the clients shouldn't not use the preview_url endpoint if its disabled on server side. To do this, the clients need to know which features are available and activated on the server.
3. If neither of the two proposed solutions is applicable, the endpoint preview_url should be deleted from the specification because it weakens privacy. The usefulness of this feature does not justify the damage caused by loss of confidentiality.

[dkasak]

In my opinion, users relay on the confidentiality that came with encryption and it shouldn't matter if the user sends a normal message or a url. So preview_url should use POST.

I don't think this does much for privacy since HTTP servers can choose to log POST data too. Though I agree that typically configured HTTP servers are less likely to log POST data than URLs, so it would at least reduce accidental logging which is a welcome addition. Such a change would require going through the MSC process however and is unlikely to be a priority right now unless someone steps up to write a proposal.

the clients shouldn't not use the preview_url endpoint if its disabled on server side. To do this, the clients need to know which features are available and activated on the server.

Similarly to the above: sounds nice, but needs an MSC and then people have to implement it, so there are no guarantees.

If neither of the two proposed solutions is applicable, the endpoint preview_url should be deleted from the specification because it weakens privacy. The usefulness of this feature does not justify the damage caused by loss of confidentiality.

It should be noted that at least Element Web/Desktop disable URL previews by default in encrypted rooms. A user has to explicitly turn it on for a particular room in order for URL preview requests to get sent.

[richvdh]

There are an awful lot of words here, but:

• sending access_tokens in query params is a client bug. The fact that the spec even mentions it is questionable, but hard to get excited about when OIDC is on its way and will enforce this anyway.
• so I think this amounts to "preview_url should allow POST".