simplex-chat / simplexmq

⚙️ SimpleXMQ - A reference implementation of the SimpleX Messaging Protocol for simplex queues over public networks.
https://simplex.chat
GNU Affero General Public License v3.0
512 stars 56 forks source link

RFC 9266: Channel Bindings for TLS 1.3 support #486

Open Neustradamus opened 2 years ago

Neustradamus commented 2 years ago

Can you add the support of RFC 9266: Channel Bindings for TLS 1.3?

Little details, to know easily:

Thanks in advance.

epoberezkin commented 2 years ago

We have - we use Peer Finished message from TLS handshake (tlsunique binding), as per this RFC and it is used as a session ID in each command, signed over with per-queue key.

Neustradamus commented 2 years ago

@epoberezkin: Thanks for your answer! tls-unique does not exist for TLS = 1.3...

epoberezkin commented 2 years ago

I am not sure it’s correct, the way tlsunique is defined still makes sense - it’s a Finished message of the client, sent as part of TLS handshake - it exists in TLS 1.3.

Whether it sufficiently protects TLS 1.3 is another question. This RFC refers to triple handshake vulnerability of TLS 1.3, but if I understood it correctly it requires session resumption, and we disabled it.

Anyway, we will analyse whether we should switch to tls-exporter, and how important it is - thanks for the tip!

epoberezkin commented 2 years ago

Just confirming – 3shake attack requires client certificates and session resumption – SimpleX doesn't use these. So tlsunique channel binding that SimpleX uses currently appears to be robust for both TLS 1.2 and 1.3.

Neustradamus commented 2 years ago

Hi @epoberezkin,

Please note that tls-unique is only for TLS =< 1.2 (RFC5929 which has been removed in TLS 1.3) and tls-exporter is for TLS = 1.3 (RFC9266, this RFC has been released few days ago).

epoberezkin commented 2 years ago

Please note that tls-unique is only for TLS =< 1.2 (RFC5929 which has been removed in TLS 1.3)

I am not sure this is correct.

What RFC9266 says is the following:

The "tls-unique" channel binding type defined in [RFC5929] was found to be susceptible to the "triple handshake vulnerability"...

So the motivation to introduce tls-exporter as the default channel binding was triple handshake vulnerability. As I wrote, triple handshake vulnerability is only relevant when session resumption is used together with client certificates. As we use neither, I don't see the problem to continue using tis-unique binding with both TLS 1.2 and 1.3 for now.

Neustradamus commented 2 years ago

tls-unique is used with TLS =< 1.2 and tls-exporter is used with TLS = 1.3.

tls-unique does not work with TLS 1.3.

epoberezkin commented 2 years ago

Sorry, can you please explain what “doesn’t work” mean. RFC doesn’t say that.

Neustradamus commented 2 years ago

The RFC5929 "tls-unique" does not work with TLS 1.3, it is for this there is now the RFC9266 "tls-exporter".

For example:

You can see the code in Mellium SASL by the author of the RFC9266:

Prosody IM has been updated:

Miranda NG has been updated:

GNU SASL (GSASL) has been updated:

glib/glib-networking has been updated, it was compatible with draft before:

epoberezkin commented 2 years ago

I understood the statement, and other libraries/tools may have needed to update, e.g. if they allowed session resumption.

I am simply asking for the clarification of what “doesn’t work” mean in your statement. Options I can think of:

Once Haskell TLS library is updated - we may contribute - we will switch, but I explained why I don’t see it as urgent - it’s not a vulnerability in our case.

Neustradamus commented 11 months ago

@epoberezkin: I think that you have seen the jabber.ru MITM and Channel Binding is the solution:

ydylla commented 5 months ago

Hi @epoberezkin, a switch to "tls-exporter" would also benefit SMP implementations in Rust that use rustls. I was experimenting a bit and this seems to be a major blocker for now. I got x448 and ed448 kind of working by using a custom CryptoProvider & ServerCertVerifier. But rustls currently only supports "tls-exporter" and they seem reluctant to add "tls-unique" for TLS 1.3 because it is not specified or rather got replaced by RFC9266 (which I find reasonable). See https://github.com/rustls/rustls/issues/995#issuecomment-1026041350 and https://github.com/rustls/rustls/issues/1089.

Neustradamus commented 5 months ago

@ydylla: Thanks for your comment!

I can specify that there is a ticket for "tls-server-end-point" in rustls repository too: