-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Federation .well-known requests don't follow 308 redirects #8098
Comments
It's also worth noting that the Kubernetes Nginx Ingress controller uses Also note that the Matrix spec explicitly states "30x redirects should be followed" in the Resolving server names section (point #3). As for fixing this issue, I guess the best approach is opening an issue/PR with twisted? |
It likely has just never been implemented. I searched Twisted briefly and didn't find any ticket requesting it be implemented.
Thanks for the context!
That would be great! Seems like it should be fairly easy to support there. We could also attempt to fix it in the Synapse code-base until an updated version of Twisted is released. |
Thanks @clokep. I've opened a ticket with Twisted, and made a quick PR. This is a very small change so hopefully they can push it through quickly! |
Added a note to the docs regarding this issue, with some temporary workarounds: #8120 |
Quick update: This issue has been fixed at the Twisted repo (twisted/twisted#1376). Now, we just need to wait for a release, then update the Twisted version being used by Synapse! However, because this issue originates with the destination server during a federation attempt, all servers must update to the newer version of Twisted before the issue is fully resolved. If you're using 308 redirects on your homeserver, you will not be able to federate with any servers that are using an older version of Twisted. It could be 6-12 months before the majority of servers are migrated to the newer version. If you're facing this problem, you should probably just use a work-around (see here). |
Twisted released a new version (21.2.0) last month, so 308 redirects are now officially supported! It looks like Synapse is currently requiring Twisted 18.9.0. Bumping the version to 21.2.0 would ensure that users are getting the fix, but it feels like that could be a large change. @clokep do you think it's worth me making a PR to bump the Twisted version, or is that a project that would be chosen/handled by the Synapse team? |
I could go either way on this. Normally we don't bump our dependency versions as a matter of course, to allow freedom to our downstream packages to use whatever version is most appropriate for them. But arguably Synapse is actually buggy with older Twisted versions so we should require 21.2 to fix the bug. What do other folks on @matrix-org/synapse-core think? As an aside: it would be good to see updates to the spec, and additions to the Complement or Sytest test suites, to ensure that other homeserver implementations don't forget about 308. |
I think the spec already says 30x redirects should be followed for .well-known resolution. Correct me if I'm wrong though. |
it does, though I thought it might be nice to enumerate exactly which codes that covers. If we forgot 308, there are chances that others might do too. @ryanc-me I think it's as simple as a PR against https://github.com/matrix-org/matrix-doc/blob/master/specification/server_server_api.rst (though there might be some rearrangement afoot in that repo, so the PR might sit for a while.)
the thing is, I don't think either of those are reason enough to break compat with old Twisted. On the other hand, not honouring the federation spec (and thus making it incompatible with other servers) is enough reason, imho. So yes, @ryanc-me, I think a PR to bump the Twisted version would be appreciated. |
Thanks for the feedback @richvdh. FWIW, I think that spec compatibility is a good reason too, especially considering that Synapse is the de-facto reference server. This was a very difficult error to track down. I'll work on some PRs over the weekend! |
Description
Synapse does not follow HTTP 308 (Permanent Redirect) redirects on .well-known files when resolving hosts for federation.
The problem seems to be that Synapse uses Twisted's RedirectAgent for handling .well-known redirects, which for whatever reason doesn't follow 308 redirects
Steps to reproduce
Expected result: federation works fine
Actual result: other servers will fail to fetch the .well-known and fall back to 8448, leading to federation not working
Version information
The text was updated successfully, but these errors were encountered: