charybdis-ircd / charybdis

Scalable IRCv3.2 server for large, community-oriented networks
GNU General Public License v2.0
231 stars 102 forks source link

IRCv3 STS #350

Open kaniini opened 4 years ago

kaniini commented 4 years ago

Minimal STS implementation. Does not support cap-notify.

kaniini commented 4 years ago

any reason why this hasn't been reviewed yet?

aaronmdjones commented 4 years ago

I was going to do this tomorrow.


This will not be merged as-is because it is not complete.

Namely, it does not support Server Name Indication. This needs to be extended to take a hostname (ideally as part of the top-level token name, a la connect "irc.server.name" {}), and be allowed to be specified multiple times (again like connect). It should only advertise the capability to the client if the client's SNI-provided hostname matches a configured sts block.

Without this, it would be simply stupid to ever enable it, because STS requires clients to validate the server's certificate, but you can't control which hostnames point at your IRCd.

Also, the enabled key is redundant; simply commenting out the block in the default configuration would be preferred, with the understanding that if you uncomment it, you are enabling it.

aaronmdjones commented 4 years ago

Upon further thought, the design I envisage is that whether or not to advertise a port to non-TLS clients should be decided by the listen block (with a listen::stsport key, parsing conditional on listen::sslport being given, and the port given in listen::stsport matching that list of ports), and whether or not to advertise a persistence policy to TLS clients should be, as above, decided by SNI.