Synapse: Federation uses incorrect Host: header when SRV record is present

[WGH-]

Consider:

• A Matrix homeserver example.org
• DNS record _matrix._tcp.example.org has SRV record 1 1 443 matrix.example.org.

Synapse sends federation requests to https://matrix.example.org, which is correct, but it uses wrong hostname (example.org instead of matrix.example.org) and also doesn't use SNI (which is a known issue, though, #1491).

The problem is here:

https://github.com/matrix-org/synapse/blob/6c1bb1601e43c89637ae5bd8720c255646ca8141/synapse/http/matrixfederationclient.py#L143

MatrixFederationHttpClient uses original server name, before DNS resolution.

[richvdh]

I'm failing to understand why this isn't a dup of #1491

[WGH-]

Ooops, it indeed is. The comments of that issue mostly discussed SNI, and I failed to read the original post carefully.

[richvdh]

This specific point was discussed in the context of dendrite today; the conclusion was that the current behaviour is actually correct.

Here are our reasons:

• SRV records are in some way analogous to CNAMEs; in the case of CNAMEs, we base the Host: header on the original host, not the CNAME target.
• DNS is insecure. Mallory could spoof a DNS response to Alice who's trying to contact Bob telling her to contact Mallory instead. if the SNI was based on the DNS response, Alice would then verify she's talking to Mallory, not Bob, leaving a gaping hole in the protocol.
  
  
  
  • We can draw a parallel with DANE, which has an RFC (7673) which happens to describe how to verify TLS when SRV is insecure and there's no TLSA records (i.e. DANE isn't being used) - which is our situation here. The recommendation is to verify the server_name part (the "source address") and not what the SRV record points to.)
  
  
• from a purely practical perspective, it seems easier to vhost based on the server name than the intermediate dns entry for whichever host the client happened to pick from the SRV records.

[army1349]

This decision is kind of security by obscurity. Use DNSSEC instead.