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

missing dtls ciphers? #253

Open
koush opened this issue Sep 4, 2022 · 8 comments
Open

missing dtls ciphers? #253

koush opened this issue Sep 4, 2022 · 8 comments
Labels
enhancement New feature or request

Comments

@koush
Copy link
Collaborator

koush commented Sep 4, 2022

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
Copy link
Owner

Show me the wip branch where you are implementing CBC support

@shinyoshiaki
Copy link
Owner

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

@koush
Copy link
Collaborator Author

koush commented Sep 4, 2022

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?

koush@2375041

@koush
Copy link
Collaborator Author

koush commented Sep 4, 2022

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
Copy link
Owner

shinyoshiaki commented Sep 6, 2022

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
Copy link
Collaborator Author

koush commented Sep 6, 2022

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
Copy link
Owner

shinyoshiaki commented Sep 6, 2022

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
Copy link
Collaborator Author

koush commented Sep 6, 2022

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.

@shinyoshiaki shinyoshiaki added the enhancement New feature or request label Jan 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants