Skip to content

Conversation

DanielOaks
Copy link
Member

This adds the if-host-match and port-if-match keys, allowing a network like freenode (which has third-party domain aliases pointing towards their servers) to advertise STS. I've kept this basically along the lines of the conversation we've had, and think it's pretty simple. I opted to use a space \s to separate patterns, as we're sure that character isn't allowed in hosts and it should already be escaped properly by the key escaping.

@DanielOaks
Copy link
Member Author

We considered requiring that the client send USER <user> <hostname-they-are-connecting-to> and using that supplied hostname to programmatically advertise STS to the client or not based on that information. However:

  • That would require the client send USER before requesting CAP.
  • There are already two conflicting definitions for USER and oh my god let's not have an extra one.
  • Just, no.

Which is why this method is preferred.

@DanielOaks
Copy link
Member Author

May be worth checking out the few clients that do support STS to ensure they handle STS being advertised with no port sanely (i.e. silently ignoring it). If I check any out I'll note the results here.

@jesopo
Copy link

jesopo commented Jun 10, 2019

for what it's worth; this change would have KeyErrored BitBot prior to bitbot-irc/bitbot@807e239

@Alexendoo
Copy link

It might be worth explicitly mentioning that *.example.com matches irc.ipv6.example.com or some other example, as it could be mistaken for the TLS style wildcards where it only applies within a single domain name component

@Alexendoo
Copy link

I'm curious on the point about advertising the connecting hostname via USER - is there a way to offer the information that's less problematic? It seems like the server not sending the STS key itself would work pretty well if it's not present.

@jwheare
Copy link
Member

jwheare commented Jul 23, 2019

Given the option for a server to decide whether to advertise a persistence policy (duration) on a secure connection by checking the SNI host, I think this is less necessary.

SNI isn't available on insecure connections, so an upgrade policy (port) would still have to be served in that case. But if no duration key is present after the upgrade, client implementations won't fail the connection irrevocably.

@Alexendoo
Copy link

My understanding is this is entirely focused on the insecure case, as a way to not cause breakage. The secure connection isn't ever reached (because the cert is invalid) so it doesn't really play a part here

@jwheare
Copy link
Member

jwheare commented Jul 23, 2019

This is about preventing an upgrade to the secure case, because of the perceived risk that it would then fail. But if no duration key is sent on the secure connection, then failure is unlikely, since clients do not on the whole prevent connecting to hosts with invalid/self-signed/mismatched-host certificates.

So, my point is that there is no real reason to prevent upgrading from the insecure case.

@Alexendoo
Copy link

Both clients I use correctly verify certificates (Hexchat, weechat). I don't think the existence of some clients that fail to do so is reason enough to break clients that do

@jwheare
Copy link
Member

jwheare commented Jul 23, 2019

Both of those clients allow certificate verification to be disabled, which is a requirement for connecting to hobby servers with self signed certs. So this isn't a new form of breakage, it's something clients can already deal with.

@Alexendoo
Copy link

It's a new form of breakage as one day the connection will be working for a user, the next it will be broken. If they're going to have to change configuration they could just change it to the right hostname anyway, but the reason this was introduced in the first place was to avoid such a situation.

Not everybody would know what to do to fix the issue, I would image most would not in fact since it's not exactly obvious what's going on.

@jwheare
Copy link
Member

jwheare commented Jul 23, 2019

The motivation for this issue is that the breakage would fall under strict sts rules, i.e. not be easy for the user to work around. But that isn't the case if you don't advertise an sts persistance duration. If there's also a fear around a different sort of change, fine, but the goal posts have been shifted.

Bad certs are common enough in IRC that I suspect people are more familiar with this than you state. I personally feel this change in severity moves the needle further away from "user hostile" and closer to "just enough friction to make people fix their dns deployments and default host lists". But it's a spectrum and I'm sure we've got a range of opinions.

@DanielOaks
Copy link
Member Author

Now that I'm back, there was a ton of discussion on this through the IRC channel and elsewhere, specifically around particular sentences from the STS spec and certain (under/not-defined) cases. @jwheare @edk0 where did we end up with this, is this PR still required or are we waiting on more testing of client implementations in the wild to decide or should I close this PR?

@SadieCat
Copy link
Contributor

If this is still a proposal people want i'd be interested in implementing it.

@slingamn
Copy link
Contributor

Here's a sketch of an alternative proposal: a redirect key for the sts cap value.

  1. As in the current if-host-match proposal, a server MUST NOT advertise both port and redirect
  2. The value of redirect is something with the semantic content of (for example) ircs://chat.freenode.net:6697 (it could literally be an IRC URL, which we could specify as part of this effort, or it could have some other syntax, but either way it carries sufficient information for the client to disconnect and make a new verified TLS connection)
  3. If a client currently connecting "insecurely" (plaintext or unverified TLS) sees this, they MUST disconnect and connect to the new address over verified TLS. If they see an STS policy on the new connection, they MUST store it associated with the new address only, not the old address. (The old address remains unprotected against future MITM, but if-host-match doesn't solve this problem either.)
  4. If a client currently connecting "securely" (verified TLS) sees this, and the address doesn't match the domain name they're connecting to (but the certificate validates anyway because of SANs), they MAY disconnect and reconnect to the new address, but if they don't, they MUST NOT store the STS policy associated with the domain name they're currently connecting to.

@jwheare
Copy link
Member

jwheare commented Feb 19, 2020

A few concerns I have with this:

  • Seems considerably more fiddly to implement for clients, especially point 4.
  • Plays a more active (perhaps disruptive) role in upgrading CNAMEd connections to a canonical address that the server may wish to simply leave alone.
  • Buries/couples generic redirect behaviour within sts when it's arguably useful elsewhere. I would prefer this being a separate CAP, but I'm still uncomfortable with abusing CAP with more non-negotiated passive keys.

@slingamn
Copy link
Contributor

From discussion in #ircv3, I'd like to revise point 4 to something more like, "if a client connecting over verified TLS sees a redirect policy and the address doesn't match the domain name they're connecting to, they MUST ignore it and continue with registration."

@slingamn
Copy link
Contributor

One case where redirect falls short of if-host-match: people who are connecting to a verifiable SAN (like irc.freenode.net) in plaintext. These people will never be served a persistent upgrade policy that they can trust (but if there's no active attacker, they will get redirected to TLS every time).

@jwheare
Copy link
Member

jwheare commented Feb 19, 2020

Not persisting a policy for irc.freenode.net feels like a dealbreaker to me.

Apart from that, we discussed that given the change to point 4, where the "redirect" is ignored on secure connections, a better key might be upgrade-host=chat.freenode.net:6697 (or upgrade-host=chat.freenode.net:6697;upgrade-port=6697 although that could be more confusing given the existing port key is an upgrade port) but this may now be moot.

@slingamn
Copy link
Contributor

I withdraw the suggestion of redirection as an alternative to the current proposal.

As I understand it, the current proposal is very closely tailored to Freenode's needs, to the point that its "clunky" quality was described as a feature, not a bug (because any network who isn't Freenode will know to steer clear of it). If the problem being solved here really is specific to a single network, I want to encourage people to think about interventions targeted directly to the network (e.g., collecting additional statistics on current plaintext use of Freenode).

@SadieCat
Copy link
Contributor

InspIRCd implementation: inspircd/inspircd@insp3...SadieCat:insp3+sts

progval added a commit to progval/Limnoria that referenced this pull request Jun 20, 2020
@jwheare jwheare added this to the Roadmap milestone Feb 16, 2021
@DanielOaks
Copy link
Member Author

DanielOaks commented Feb 24, 2021

Since this is going to take more time and be re-visited later, we may just want to add this line to the spec for now:

If any required key does not exist, clients MUST silently ignore the policy.

as that behaviour is kinda necessary to let us this sort of future work down the line.

@jwheare
Copy link
Member

jwheare commented Feb 28, 2021

Done in #443

@jwheare jwheare removed this from the Roadmap milestone Feb 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants