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

Deprecate STARTTLS #211

Merged
merged 2 commits into from Feb 2, 2019

Conversation

Projects
None yet
7 participants
@lol768
Copy link
Contributor

commented Jan 1, 2019

Has already been some discussion on this in #ircv3. I'm in favour of deprecating it for the reasons I outlined there, but happy to see more discussion on the matter.


It's been quite rightly pointed out I should elaborate on reasoning here, so:

  • The upgrade message can be silently dropped by an active MitM. Some clients, which are purely opportunistic about the upgrade, will continue connecting in plaintext -- whereas a "proper" TLS-only connection would fail if an insufficient level of security could be established. @dequis has pointed out that this is addressed in the spec, but it still is subject to TOFU (vs e.g. STS preloading, or an explicit option to connect using TLS on port 6697)
  • Whilst STARTTLS can provide some protection against a passive Eve, I feel that it's existence is harmful as it offers a false sense of security to users who may not fully appreciate the difference between STARTTLS and TLS (especially given the misleadingly named CAP key).
  • We should be pushing STS instead (and preload mechanisms to prevent plaintext connections at the start). We should also push for TLS-only connections in the longer term.
@DanielOaks

This comment has been minimized.

Copy link
Member

commented Jan 1, 2019

+1 on deprecating STARTTLS in favour of properly pushing STS instead. If we do go through with this, we should add some explanatory text into the spec itself as well, and some other cleanups.

## [STARTTLS]({{site.baseurl}}/specs/extensions/tls-3.1.html)

STARTTLS allows clients to upgrade their plaintext connections to use TLS
encryption. It is recommended that clients instead implement STS support.

This comment has been minimized.

Copy link
@lol768

lol768 Jan 1, 2019

Author Contributor

This is a bit of a crappy explanation, and should be expanded with more reasoning.

@Herringway

This comment has been minimized.

Copy link

commented Jan 1, 2019

This article has a decent explanation of the issue as well as some real-world examples of attacks on STARTTLS in SMTP. I've also heard this attack referred to as STRIPTLS in the past.

@dequis

This comment has been minimized.

Copy link
Contributor

commented Jan 1, 2019

While I agree with deprecating STARTTLS in favor of STS (we've been informally recommending against implementing it for a long while already), it should be noted that the current text of the spec covers downgrade attacks:

If the user agrees, all future connections to the server must use TLS via STARTTLS. If TLS cannot be negotiated, it is a hard error and the client must abort the connection. This is to make sure that the connection cannot be downgraded to plaintext by a man-in-the-middle once it has been established that TLS support is available.

It's more of an afterthought, in contrast to how STS was designed around persisting the policy, but it's there.

@Herringway

This comment has been minimized.

Copy link

commented Jan 1, 2019

Be sure to remove the STARTTLS rows from the support tables as well.
EDIT: never mind it seems to be done already

@grawity

This comment has been minimized.

Copy link
Contributor

commented Jan 2, 2019

The upgrade message can be silently dropped by an active MitM. Some clients, which are purely opportunistic about the upgrade, will continue connecting in plaintext -- whereas a "proper" TLS-only connection would fail if an insufficient level of security could be established.

The problem is the opportunistic behavior itself, not the upgrade process. It's the same with TLS on separate port – you can equally have an opportunistic client that tries port 6697 and is forced to fall back to insecure by active MitM. (The only difference is that separate ports make it much simpler to implement a strict client, since it's the default behavior already.)

Regardless of that, I agree that STARTTLS on IRC doesn't offer anything useful compared to direct TLS and should be deprecated. It might be useful to reference RFC 8314 as part of the explanation.

@FauxFaux

This comment has been minimized.

Copy link

commented Jan 2, 2019

What would a server, which only wanted to speak TLS, offer on the plain port? I was expecting it to offer CAP negotiation, then drop the connection (hopefully the client displays the message) if the client doesn't request the STARTTLS cap. Maybe send one of the many deprecated REDIRECT opcodes?

(Similar to how many sites redirect http://example.com/foo/ -> https://example.com/foo/ , and if you don't redirect you're doomed.)

@Zarthus

This comment has been minimized.

Copy link
Contributor

commented Jan 2, 2019

What would a server, which only wanted to speak TLS, offer on the plain port? I was expecting it to offer CAP negotiation, then drop the connection (hopefully the client displays the message) if the client doesn't request the STARTTLS cap. Maybe send one of the many deprecated REDIRECT opcodes?

(Similar to how many sites redirect http://example.com/foo/ -> https://example.com/foo/ , and if you don't redirect you're doomed.)

You can use sts with the port=6697 to redirect clients (and presumably close the connection. if they don't support it, they're SOOL just like if you dont implement starttls).

but a server should never advertise a plaintext port if you don't support it, imho.

@lol768

This comment has been minimized.

Copy link
Contributor Author

commented Jan 2, 2019

Probably the best approach is what @Zarthus suggested along with a NOTICE explaining why the connection is being terminated (for clients that lack sts support).

@lol768

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2019

I've expanded the explanation in the Markdown file a little to try and give some context around the deprecation. Thoughts welcomed

@Zarthus

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

ping @DanielOaks, is there anything else needed here to progress this issue?

@lol768

This comment has been minimized.

Copy link
Contributor Author

commented Feb 1, 2019

There's broad consensus here I think with regard to the change itself, and I've not received any further wording related suggestions.

Can we get this merged please @jwheare?

@DanielOaks DanielOaks merged commit bc9f698 into ircv3:master Feb 2, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify netlify/deploy
Details

@lol768 lol768 referenced this pull request Feb 2, 2019

Merged

Deprecate STARTTLS #355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.