Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Specify DTLS handshake cipher suite? #1333

Open
achingbrain opened this issue Feb 12, 2025 · 5 comments · May be fixed by #1335
Open

Specify DTLS handshake cipher suite? #1333

achingbrain opened this issue Feb 12, 2025 · 5 comments · May be fixed by #1335

Comments

@achingbrain
Copy link
Contributor

achingbrain commented Feb 12, 2025

I'm trying to integrate with webtrc-rs, a rewrite of Pion in Rust.

I can dial webrtc-rs -> libdatachannel ok, that works.

If I dial libdatachannel -> webrtc-rs it gets stuck on the DTLS handshake.

The logs I see on the Rust side are:

2025-02-12T07:39:54.935156Z  INFO webrtc_ice::agent::agent_internal: [controlled]: Setting new connection state: Connected    
2025-02-12T07:39:54.935206Z  INFO webrtc::peer_connection: ICE connection state changed: connected    
2025-02-12T07:39:54.937267Z  WARN webrtc_dtls::handshake::handshake_message_client_hello: Unsupported Extension Type 0 35    
2025-02-12T07:39:54.937275Z  WARN webrtc_dtls::handshake::handshake_message_client_hello: Unsupported Extension Type 0 22    
2025-02-12T07:39:54.937311Z  WARN webrtc_dtls::handshake::handshake_message_client_hello: Unsupported Extension Type 0 35    
2025-02-12T07:39:54.937315Z  WARN webrtc_dtls::handshake::handshake_message_client_hello: Unsupported Extension Type 0 22    
2025-02-12T07:39:54.937321Z DEBUG webrtc_dtls::flight::flight0: [handshake:server] use cipher suite: TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA    
2025-02-12T07:39:54.939337Z  WARN webrtc_dtls::handshake::handshake_message_client_hello: Unsupported Extension Type 0 35    
2025-02-12T07:39:54.939345Z  WARN webrtc_dtls::handshake::handshake_message_client_hello: Unsupported Extension Type 0 22    
2025-02-12T07:39:54.939359Z  WARN webrtc_dtls::handshake::handshake_message_client_hello: Unsupported Extension Type 0 35    
2025-02-12T07:39:54.939362Z  WARN webrtc_dtls::handshake::handshake_message_client_hello: Unsupported Extension Type 0 22    
2025-02-12T07:39:54.941897Z DEBUG webrtc_dtls::conn: server: CipherSuite not initialized, queuing packet    
2025-02-12T07:39:54.941903Z DEBUG webrtc_dtls::conn: server: received packet of next epoch, queuing packet    
2025-02-12T07:39:54.943042Z DEBUG webrtc_dtls::conn: server: decrypt failed: invalid mac    
2025-02-12T07:39:55.940353Z DEBUG webrtc_dtls::conn: server: decrypt failed: invalid mac
...invalid mac message repeats

Interestingly if I dial webrtc-rs -> webrtc-rs it works but I see a different cipher suite:

2025-02-12T07:21:32.425318Z DEBUG webrtc_dtls::flight::flight0: [handshake:server] use cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256    

Chrome causes the same cipher suite to be logged:

2025-02-12T09:50:48.695521Z DEBUG webrtc_dtls::flight::flight0: [handshake:server] use cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256   

...as does Firefox:

2025-02-12T09:55:15.163634Z DEBUG webrtc_dtls::flight::flight0: [handshake:server] use cipher suite: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256  

I'd like to try aligning the cipher suite used by libdatachannel with the one used by webrtc-rs to rule out this being the cause of the incompatibility, but I can't see an option.

Is it possible to specify what cipher suite to use?

@paullouisageneau
Copy link
Owner

There is no exposed option for DTLS ciphers, only an option for certificate type. For debugging purposes, you could change the DTLS cipher list in the code. For instance, it is set here for GnuTLS and here for OpenSSL.

@achingbrain
Copy link
Contributor Author

Aha, thanks. Everything works if I specify the cipher manually:

diff --git a/src/impl/dtlstransport.cpp b/src/impl/dtlstransport.cpp
index e2c389bf..3ef82d98 100644
--- a/src/impl/dtlstransport.cpp
+++ b/src/impl/dtlstransport.cpp
@@ -761,7 +761,7 @@ DtlsTransport::DtlsTransport(shared_ptr<IceTransport> lower, certificate_ptr cer
                                   CertificateCallback);
                SSL_CTX_set_verify_depth(mCtx, 1);
 
-               openssl::check(SSL_CTX_set_cipher_list(mCtx, "ALL:!LOW:!EXP:!RC4:!MD5:@STRENGTH"),
+               openssl::check(SSL_CTX_set_cipher_list(mCtx, "ECDHE-ECDSA-AES128-GCM-SHA256"),
                               "Failed to set SSL priorities");
 
 #if OPENSSL_VERSION_NUMBER >= 0x30000000

Would you accept a PR that makes it configurable?

@achingbrain
Copy link
Contributor Author

achingbrain commented Feb 12, 2025

There's some interesting discussion here too - it seems the default list of ciphers is big enough to cause fragmentation of the Client Hello message - I haven't verified this but if that's the case it might be sensible to restrict the list of supported ciphers?

Chrome's list is here, though this list is slightly smaller and doesn't cause fragmentation:

SSL_CTX_set_cipher_list(
      ctx, "ALL:!SHA256:!SHA384:!aPSK:!ECDSA+SHA1:!ADH:!LOW:!EXP:!MD5:!3DES:!SSLv3:!TLSv1");

This list would probably not need to be configurable.

@paullouisageneau
Copy link
Owner

Thank you for investigating, I think it would make sense to restrict the list. Making it configurable would be a bit complex because the setting would need to be generic and translated for each TLS library.

achingbrain added a commit to achingbrain/libdatachannel that referenced this issue Feb 15, 2025
Reduces the number of supported ciphers to ensure Client Hello
messages are not fragmented, `ECDHE-ECDSA-AES128-GCM-SHA256`
takes priority and older insecure ciphers are not used.

Fixes paullouisageneau#1333
@achingbrain achingbrain linked a pull request Feb 15, 2025 that will close this issue
@achingbrain
Copy link
Contributor Author

Great, I've opened #1335

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants