matrix-org / matrix-federation-tester

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

SRV records should be looked up for root domain, not server hostname #34

Closed karthanistyr closed 5 years ago

karthanistyr commented 5 years ago

On a configuration with both SRV and .well-known set up, the tester should return the correct information for both delegation mechanisms.

What happened

Given the recommended setup for a delegated matrix host on a domain:

DNS results ( from https://matrix.org/federationtester/api/report?server_name=draak.fr) came back with the following:

"DNSResult": {
  "SRVCName": "",
  "SRVRecords": null,
  "SRVError": null,
  "Hosts": {
    "matrix.draak.fr": {
      "CName": "matrix.draak.fr.",
      "Addrs": [
        "5.196.94.91"
      ],
      "Error": null
    }
  },
  "Addrs": [
    "5.196.94.91:8448"
  ]
}

Notice the "null" SRVCName and SRVRecords.

What should happen

The SRV record information should be populated with the correct information, with both SRVCName and SRVRecords filled in.

Some investigation

It looks like there is a logic error in the tester's handling of root domain vs. server hostname when looking up .well-known and SRV records.

// Host address of the server (can be different from the serverName through SRV/well-known)
serverHost := serverName

// Check for .well-known
var err error
var wellKnown *gomatrixserverlib.WellKnownResult
if wellKnown, err = gomatrixserverlib.LookupWellKnown(serverName); err == nil {
    // Use well-known as new host
    serverHost = wellKnown.NewAddress
}
var report ServerReport
dnsResult, err := gomatrixserverlib.LookupServer(serverHost)
if err != nil {
    return nil, err
}

When a .well-known is successfully detected and read, the serverHost changes to that of the actual server's host, and then the tester goes on to try to lookup _matrix._tcp.<serverHost> which may (most probably) not exist. DNS results should only ever be tested against the root domain (i.e. serverName) for the purpose of reading SRV records.

In case .well-known and SRV record both exist but differ in values, .well-known should probably be the source of truth and the discrepancy should be conveyed that the SRV should match .well-known.

Perflyst commented 5 years ago

This is probably also why the federation tester shows self signed for draak.fr even if it is Let's Encrypt. https://matrix.org/federationtester#draak.fr

richvdh commented 5 years ago

DNS results should only ever be tested against the root domain (i.e. serverName) for the purpose of reading SRV records.

No, this is not what the spec says: https://matrix.org/docs/spec/server_server/r0.1.1.html#resolving-server-names. If your well-known delegates to matrix.draak.fr, it is correct to then look for a SRV record at _matrix._tcp.matrix.draak.fr. The SRV record at _matrix._tcp.draak.fr will be ignored, except by legacy Synapse versions.

karthanistyr commented 5 years ago

Well then the problem is that the tester results are misleading at best.

There is an active recommendation to keep the SRV record on the root domain to support legacy systems, and so indefinitely (at least for the moment).

To show that none exist is not what I expected, when I took explicit steps for one to exist.

Le 9 févr. 2019 à 15:38, Richard van der Hoff notifications@github.com a écrit :

DNS results should only ever be tested against the root domain (i.e. serverName) for the purpose of reading SRV records.

No, this is not what the spec says: https://matrix.org/docs/spec/server_server/r0.1.1.html#resolving-server-names. If your well-known delegates to matrix.draak.fr, it is correct to then look for a SRV record at _matrix._tcp.matrix.draak.fr. The SRV record at _matrix._tcp.draak.fr will be ignored, except by legacy Synapse versions.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

richvdh commented 5 years ago

I've opened a new issue at #37