Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MSC1711: X.509 certificate verification for federation connections #1711

Merged
merged 9 commits into from
Jan 13, 2019
Merged

MSC1711: X.509 certificate verification for federation connections #1711

merged 9 commits into from
Jan 13, 2019

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Nov 7, 2018 •
edited
Loading

@richvdh richvdh changed the title proposal for requiring signed certs for federation MSC1711: X.509 certificate verification for federation connections Nov 7, 2018
@richvdh richvdh added proposal A matrix spec change proposal proposal-wip labels Nov 7, 2018
@richvdh richvdh mentioned this pull request Nov 7, 2018
Closed
@ara4n
Copy link
Member

ara4n commented Nov 8, 2018

This generally looks good to me, although it's a bit sad that people who have got used to self-signed certs magically working (like me on arasphere) will be forced to pull their LE lives together.

Is there any way to (optionally) fall back to tofu when you see a self-signed cert? (asking somewhat rhetorically, given it feels it makes it too easy for an attacker to MITM new connections to servers via a self-signed cert)

@jevolk
Copy link
Contributor

jevolk commented Nov 8, 2018

related #1685

@richvdh
Copy link
Member Author

richvdh commented Dec 6, 2018

Note for anyone reading this: we see #1708 as a pre-requisite. It is fair to say that #1708 has had a less than rapturous reception.

@jcgruenhage jcgruenhage mentioned this pull request Dec 10, 2018
Closed
@richvdh richvdh mentioned this pull request Dec 14, 2018
Merged
@richvdh richvdh mentioned this pull request Jan 7, 2019
Open
@richvdh
Copy link
Member Author

richvdh commented Jan 7, 2019

right, I have updated this, and moved out the MSC1708 section because really that belongs to MSC1708. Everyone seems to be basically on board with the principle of this MSC, so I'm going to propose a FCP.

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Jan 7, 2019 •
edited by KitsuneRal
Loading

Team member @richvdh has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@mscbot
Copy link
Collaborator

mscbot commented Jan 8, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@richvdh richvdh added the r0 P1 label Jan 8, 2019
@richvdh richvdh self-assigned this Jan 8, 2019
@mscbot mscbot added finished-final-comment-period and removed final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Jan 13, 2019
@mscbot
Copy link
Collaborator

mscbot commented Jan 13, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

@turt2live turt2live merged commit 87bb1a6 into master Jan 13, 2019
Superceded by https://github.com/orgs/matrix-org/projects/8 automation moved this from Review to Done - Operations Jan 13, 2019
This was referenced Jan 15, 2019
Closed
Closed
@neilisfragile neilisfragile moved this from Done - Operations to Done - Planned Project in Superceded by https://github.com/orgs/matrix-org/projects/8 Jan 16, 2019
@richvdh richvdh removed their assignment Jan 17, 2019
turt2live added a commit that referenced this pull request Jan 31, 2019
@turt2live
Specify .well-known s2s discovery and X.509 validation
0347e87 
Original proposals:
* #1708 (note: the JSON requirements were softened by #1824)
* #1711

Implementation proofs:
* matrix-org/synapse#4489
* No explicit PRs for MSC1711 could be found, however Synapse is known to implement it.

There are no intentional changes which differ from the proposals in this commit, however the author has relied upon various historical conversations outside of the proposals to gain the required context. Inaccuracies introduced by the author are purely accidental.
@turt2live turt2live mentioned this pull request Jan 31, 2019
Merged
@mherrb
Copy link

mherrb commented Feb 27, 2019 •
edited
Loading

when using an SRV DNS record to point to, let's say example.com, does it mean that 'example.com' needs to be include in the list of subjectAltNames in the certificate ?
(the laas.fr domain is setup with a SRV record, but fails the certificate check on https://matrix.org/federationtester/ - I'm wondering if the missing laas.fr domain is the cause.

@jcgruenhage
Copy link
Contributor

@mherrb You will want to either have your server name in your DNS SANs, or have a .well-known record pointing to the other server. https://github.com/matrix-org/synapse/blob/master/docs/MSC1711_certificates_FAQ.md has more info on what you can do.

@mherrb
Copy link

mherrb commented Feb 27, 2019

Thanks.

@erikjohnston erikjohnston mentioned this pull request Mar 6, 2019
Merged
@turt2live
Copy link
Member

This is merged via #1830

@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed finished-final-comment-period labels May 24, 2019
@anoadragon453 anoadragon453 mentioned this pull request Jul 23, 2019
Closed
@turt2live turt2live added kind:maintenance MSC which clarifies/updates existing spec and removed proposal-pr labels Apr 20, 2020
@afranke afranke deleted the rav/proposal/x509-for-federation branch September 22, 2021 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uhoreg uhoreg uhoreg left review comments

@ara4n ara4n ara4n left review comments

@hawkowl hawkowl hawkowl left review comments

@jcgruenhage jcgruenhage jcgruenhage requested changes

@4nd3r 4nd3r 4nd3r left review comments

@maxidorius maxidorius maxidorius left review comments

Assignees
No one assigned
Labels
disposition-merge kind:maintenance MSC which clarifies/updates existing spec merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal
Projects
No open projects
Superceded by https://github.com/orgs...
  
Done - Planned Project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet