Skip to content

Add more validation and logging to policy server signature responses.#19807

Draft
reivilibre wants to merge 8 commits into
developfrom
rei/fix_ps_signature_overwrite
Draft

Add more validation and logging to policy server signature responses.#19807
reivilibre wants to merge 8 commits into
developfrom
rei/fix_ps_signature_overwrite

Conversation

@reivilibre

@reivilibre reivilibre commented May 27, 2026

Copy link
Copy Markdown
Contributor

Was working on this to fix #19796 but didn't notice that #19797 was already opened to fix it.

Have now merged in develop to build on top.

Notable changes:

  • Now avoid overwriting any signature at all, even if it's from the same server name as the policy server.
    (It didn't sit right with me to overwrite an existing signature)
    • We now log in this case
  • Validate that the policy server responds with the spec-mandated key ID. The spec only applies this to ed25519 keys, awkwardly, so the validation is equally awkward, but non-zero.
  • Validation on the response body format of the policy server response in general.
  • Introduces a Pydantic-based approach for validating federation responses.
    • Now raises a 502 like you'd expect, rather than 500 with an unhandled exception.
    • I'd like to see this machinery used more widely in the federation client.

Review guidance: Commit-by-commit review is generally fine, just beware that 'Validate policy server response.....' is partially conflicting with concurrent work on develop and the merge conflict resolves the difference. It may be easier to review that alongside the merge commit at the same time?

Original commit schedule, with full messages:

  1. Promote InvalidResponseError to a 'real' error type

  2. pydantic_models: Fix broken link, add RootModel equivalent

  3. federation_client: Add Pydantic validation machinery

  4. Validate policy server response and prevent overwriting signatures

  5. Merge branch 'develop' into rei/fix_ps_signature_overwrite

  6. Add a log for the case when the policy server tries to overwrite

@reivilibre reivilibre force-pushed the rei/fix_ps_signature_overwrite branch from d333007 to bc4538e Compare May 27, 2026 11:16
@MadLittleMods MadLittleMods added the A-Abuse Reports, media quarantine, policy servers, etc label May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Abuse Reports, media quarantine, policy servers, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Synapse doesn't merge signatures correctly when a policy server is running on the same domain

2 participants