paullouisageneau / libdatachannel

C/C++ WebRTC network library featuring Data Channels, Media Transport, and WebSockets
https://libdatachannel.org/
Mozilla Public License 2.0
1.81k stars 366 forks source link

Compile issue when mbedtls is enabled and NO_MEDIA is set. #1283

Closed Nemirtingas closed 1 month ago

Nemirtingas commented 1 month ago

Here: https://github.com/paullouisageneau/libdatachannel/blob/3c33ea0f49fff2630be080b2043052b4d3de5786/src/impl/dtlstransport.cpp#L373-L376

and here: https://github.com/paullouisageneau/libdatachannel/blob/3c33ea0f49fff2630be080b2043052b4d3de5786/src/impl/dtlstransport.cpp#L414

srtp is used even if NO_MEDIA is set, making a compile error on undefined types.

Nemirtingas commented 1 month ago

I applied a patch locally:

diff --git a/src/impl/dtlstransport.cpp b/src/impl/dtlstransport.cpp 
index caad9c71..b09ff174 100644 
--- a/src/impl/dtlstransport.cpp 
+++ b/src/impl/dtlstransport.cpp 
@@ -369,10 +369,12 @@ int DtlsTransport::TimeoutCallback(gnutls_transport_ptr_t ptr, unsigned int /* m 

 #elif USE_MBEDTLS 

+#if RTC_ENABLE_MEDIA 
 const mbedtls_ssl_srtp_profile srtpSupportedProtectionProfiles[] = { 
     MBEDTLS_TLS_SRTP_AES128_CM_HMAC_SHA1_80, 
     MBEDTLS_TLS_SRTP_UNSET, 
 }; 
+#endif 

 DtlsTransport::DtlsTransport(shared_ptr<IceTransport> lower, certificate_ptr certificate, 
                              optional<size_t> mtu, 
@@ -409,7 +411,9 @@ DtlsTransport::DtlsTransport(shared_ptr<IceTransport> lower, certificate_ptr cer 
                mbedtls::check(mbedtls_ssl_conf_own_cert(&mConf, crt.get(), pk.get())); 

                mbedtls_ssl_conf_dtls_cookies(&mConf, NULL, NULL, NULL); 
+#if RTC_ENABLE_MEDIA 
                mbedtls_ssl_conf_dtls_srtp_protection_profiles(&mConf, srtpSupportedProtectionProfiles); 
+#endif 

                mbedtls::check(mbedtls_ssl_setup(&mSsl, &mConf)); 
paullouisageneau commented 1 month ago

I get where you're coming from here, MbedTLS is often not built by maintainers with DTLS-SRTP support as the flag MBEDTLS_SSL_DTLS_SRTP is not set by default.

However, the WebRTC spec (specifically RFC 8827) mandates the support for DTLS-SRTP irrelevant if SRTP is used or not. Removing it would break compatibility with zealous implementations which check support during DTLS handshake (for instance Pion), even if you don't use media afterwards. You could still connect to other libdatachannel agents and browsers, which are lenient enough, but it wouldn't be standard-compliant anymore.

Therefore, DTLS-SRTP support is still needed for compatibility even with NO_MEDIA.

Nemirtingas commented 1 month ago

Hmmm, I need to change vcpkg port then. :'(, got so many ports to fix for my cross-compilation stuff x)