matrix-org / matrix-federation-tester

Tester for matrix federation written in golang.
77 stars 17 forks source link

Federation Tester should check content-type of /_matrix/key/v2/server #148

Open herkulessi opened 5 months ago

herkulessi commented 5 months ago

The federation tester currently doesn't check the content-type of the responses, while servers do and the Spec mandates application/json

richvdh commented 5 months ago

Citation needed, for both claims.

I see this text in the spec:

The Content-Type for this response SHOULD be application/json, however servers parsing the response should assume that the body is JSON regardless of type.

(https://spec.matrix.org/v1.9/server-server-api/#getwell-knownmatrixserver)

herkulessi commented 5 months ago

Claim No1:

I have tested it both with the matrix.org hosted version of the federation tester, as well as the federationtester built from main seconds ago, and both attested me a working federation. I tested it with my current project (which is not a real homeserver). It's hard to prove without revealing the domain, but I got the following output during testing just now:

❯ curl 'http://127.0.0.1:8080/api/federation-ok?server_name=<my.test.domain>'
GOOD
❯ curl https://<my.test.domain>/_matrix/key/v2/server -v 2>&1 | grep content-type
< content-type: application/octet-stream

I have removed the domain, as it was a very temporary setup that will not be consistently reachable anyways.

Claim No2:

You are right, it seems like the well-known is exempt from the content-type rule (as per your quoted part of the spec), however I would argue the /_matrix/key/v2/server endpoint would have to set a content-type of application/json.

all endpoints in this specification require the destination server to return a JSON object. Servers must include a Content-Type header of application/json for all JSON responses. (https://spec.matrix.org/v1.9/server-server-api/#api-standards)

herkulessi commented 5 months ago

Synapse does actually check the content-type at the key endpoint but not at the well-known endpoint.

2024-03-07 22:21:39,759 - synapse.http.federation.well_known_resolver - 197 - INFO - ServerKeyFetcher-1036- - Response from .well-known: {'m.server': '<my.test.domain>:443'}
2024-03-07 22:21:39,832 - synapse.http.matrixfederationclient - 341 - WARNING - ServerKeyFetcher-1036 - {GET-O-4890} [<my.test.domain>] Error reading response GET matrix-federation://<my.test.domain>/_matrix/key/v2/server: Failed to send request: RuntimeError: Remote server sent Content-Type header of 'application/octet-stream', not 'application/json'
richvdh commented 5 months ago

sorry, the "both claims" I was referring to were that (1) the spec and (2) servers require application/json for the .well-known file.

I think you're right about it for /_matrix/key/v2/server. The relevant bit of spec is here:

Servers must include a Content-Type header of application/json for all JSON responses.

[edit: which you already linked! sorry]