shinyoshiaki / werift-webrtc

WebRTC Implementation for TypeScript (Node.js), includes ICE/DTLS/SCTP/RTP/SRTP/WEBM/MP4
MIT License
469 stars 30 forks source link

missing dtls ciphers? #253

Open koush opened 2 years ago

koush commented 2 years ago

I'm trying to connect to Tuya's webrtc endpoint. This works fine in any browser. However, it is failing in werift due to missing ciphers. Here's the supported cipher list from Tuya's client:

  49162, 49172, 49187,
  49191, 49161, 49171,
  49167, 49157, 49193,
  49166, 49189, 49156,
    255

This seems to be a combination of various CBC/SHA1 ciphers, which seem to be weak/deprecated. Werift doesn't support any CBC ciphers. I tried to add support for some of these but am getting alert fatal error with description of 50 while trying to handshake.

Cipher suite list: https://docs.microsoft.com/en-us/dotnet/api/system.net.security.tlsciphersuite?view=net-6.0

shinyoshiaki commented 2 years ago

Show me the wip branch where you are implementing CBC support

shinyoshiaki commented 2 years ago

alert 50 is decode_error https://www.ietf.org/rfc/rfc5246.html#section-7.2

koush commented 2 years ago

alert 50 is decode_error https://www.ietf.org/rfc/rfc5246.html#section-7.2

Yep, I saw that from your AlertDesc enum.

Here's the branch. The change is hacked in at the moment to see if I could get it working with one of the Tuya supported ciphers. I'm unsure what to use instead of AEAD_AES_128_GCM. As far as I know, that should work, but there should be a counter retained somewhere I think?

https://github.com/koush/werift-webrtc/commit/23750417e5804868be010f0979a86e557497a9d8

koush commented 2 years ago

I think I may need to retain the cipher instance or grab the IV out of the cipher for reconstruction for CBC, since it can't reuse the same IV for subsequent messages?

shinyoshiaki commented 2 years ago

I'm unsure what to use instead of AEAD_AES_128_GCM

Here is the suite that chrome supports

49195, 49199, 52393, 52392, 49161, 49171, 49162, 49172, 156, 47, 53

Of these, the following four will be supported by the Tuya

49161, 49171, 49162, 49172,

For now, I think it would be better to support "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA | 49161" first

koush commented 2 years ago

For now, I think it would be better to support "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA | 49161" first

I also tried implementing that one first but got the same alert error. Any suggestions for the code I have thus far?

shinyoshiaki commented 2 years ago

Changing a few parameters is not enough to support

Supporting CBC ciphers is tough; We need to implement the following parts of DTLS. https://www.rfc-editor.org/rfc/rfc5246#section-6.2.3.2

Specifically, We need to implement CBCCihper as in the following AEADCipher. https://github.com/shinyoshiaki/werift-webrtc/blob/498c724029c4572dfef3ae5b02e13ca1897aa823/packages/dtls/src/cipher/suites/aead.ts

this is my wip branch https://github.com/shinyoshiaki/werift-webrtc/tree/feature/dtls-cipher-cbc

I can't guarantee that I will continue to work on it myself as it may not be worth the effort.

koush commented 2 years ago

Understood. I'm unfamiliar with the inner workings of DTLS, out of my element here. I can try picking up where you left off. I'll see if I can pressure Tuya to implement other ciphers as CBC has been deprecated in favor of GCM anyways from what I understand. It's on their 2.0 roadmap, but I don't know when that is.