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.

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

Remove _xmppconnect DNS method from XEP-0156 and add warnings #1158

Merged
merged 1 commit into from Mar 8, 2022

Conversation

moparisthebest
Copy link
Contributor

xep-0156.xml Outdated Show resolved Hide resolved
@horazont horazont added the Needs Council The affected XEP has the Council as Approving Body and it needs to decide on the change. label Feb 15, 2022
@moparisthebest moparisthebest changed the title Obsolete XEP-0156 and add warnings Remove _xmppconnect DNS method from XEP-0156 and add warnings Feb 22, 2022
xep-0156.xml Outdated
@@ -212,8 +179,9 @@ _xmppconnect IN TXT "_xmpp-client-websocket=wss://web.example.com:443/ws"
<section2 topic='Business Rules' anchor='httpbizrules'>
<p>The following business rules apply:</p>
<ol start='1'>
<li>host-meta files MUST be fetched only over HTTPS, and MUST ignore any connection URLs that are not secure, eg not 'https://' or 'wss://', this provides secure delegation, meaning you SHOULD send SNI matching the host of the URL from the connection URL and validate that the certificate is valid for that host *or* the the XMPP domain</li>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having (negated) positive examples, I think it would be better to make explicit negative examples: "e.g. 'http://' or 'ws://'", otherwise uneducated users may decide they consider http without TLS secure enough.

nits:

  • "and clients MUST ignore any..."
  • "e.g." instead of "eg"
  • Add full stop before "this provides secure delegation"
  • "the the"
  • Add full stop at end of item

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, I've re-worded it and fixed the other issues you identified, but I really want to keep the "allow these" wording vs "do not allow these" as it encourages implementations to implement a list of allowed protocols which is more secure than a list of disallowed ones.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this way is even better, given that as of now, https and wss are effectively the only schemes that should be supported, as bosh and websockets are the only alternative connection methods you can discover with the XEP.

Only other nit:
The markdownish *or* doesn't really fit here. We have <em>or</em> or <strong>or</strong> for this. :)

@moparisthebest moparisthebest force-pushed the obsolete-xep-0156 branch 2 times, most recently from b9ccd0f to b518d7e Compare February 23, 2022 13:48
@horazont horazont merged commit 0bb4676 into xsf:master Mar 8, 2022
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Council The affected XEP has the Council as Approving Body and it needs to decide on the change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants