From 0e07fc12081c7757fcb6d15ef13cceafd151b02e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?I=C3=B1aki=20Baz=20Castillo?= Date: Wed, 13 Sep 2023 13:29:08 +0200 Subject: [PATCH 1/2] Make DTLS fragment stay within MTU size range - Fixes #1100 - All credits to @pnts-se and his PR #1143 --- worker/include/RTC/DtlsTransport.hpp | 3 +- worker/src/RTC/DtlsTransport.cpp | 75 +++++++++++++--------------- 2 files changed, 38 insertions(+), 40 deletions(-) diff --git a/worker/include/RTC/DtlsTransport.hpp b/worker/include/RTC/DtlsTransport.hpp index 9cec6ae8f6..93b81bbd7c 100644 --- a/worker/include/RTC/DtlsTransport.hpp +++ b/worker/include/RTC/DtlsTransport.hpp @@ -152,6 +152,8 @@ namespace RTC return this->localRole; } void SendApplicationData(const uint8_t* data, size_t len); + // This method must be public since it's called within an OpenSSL callback. + void SendDtlsData(const uint8_t* data, size_t len); private: bool IsRunning() const @@ -173,7 +175,6 @@ namespace RTC } void Reset(); bool CheckStatus(int returnCode); - void SendPendingOutgoingDtlsData(); bool SetTimeout(); bool ProcessHandshake(); bool CheckRemoteFingerprint(); diff --git a/worker/src/RTC/DtlsTransport.cpp b/worker/src/RTC/DtlsTransport.cpp index 04c4d4b179..8d4d5895fb 100644 --- a/worker/src/RTC/DtlsTransport.cpp +++ b/worker/src/RTC/DtlsTransport.cpp @@ -53,6 +53,31 @@ inline static unsigned int onSslDtlsTimer(SSL* /*ssl*/, unsigned int timerUs) return 2 * timerUs; } +inline static long onSslBioOut( + BIO* bio, + int operationType, + const char* argp, + size_t len, + int /*argi*/, + long /*argl*/, + int ret, + size_t* /*processed*/) +{ + long resultOfcallback = (operationType == BIO_CB_RETURN) ? static_cast(ret) : 1; + + if (operationType == BIO_CB_WRITE && argp && len > 0) + { + MS_DEBUG_DEV("%zu bytes of DTLS data ready to be sent", len); + + RTC::DtlsTransport* dtlsTransport = + reinterpret_cast(BIO_get_callback_arg(bio)); + + dtlsTransport->SendDtlsData(reinterpret_cast(argp), len); + } + + return resultOfcallback; +} + namespace RTC { /* Static. */ @@ -633,6 +658,8 @@ namespace RTC goto error; } + BIO_set_callback_ex(this->sslBioToNetwork, onSslBioOut); + BIO_set_callback_arg(this->sslBioToNetwork, reinterpret_cast(this)); SSL_set_bio(this->ssl, this->sslBioFromNetwork, this->sslBioToNetwork); // Set the MTU so that we don't send packets that are too large with no fragmentation. @@ -673,7 +700,6 @@ namespace RTC { // Send close alert to the peer. SSL_shutdown(this->ssl); - SendPendingOutgoingDtlsData(); } if (this->ssl) @@ -776,7 +802,6 @@ namespace RTC SSL_set_connect_state(this->ssl); SSL_do_handshake(this->ssl); - SendPendingOutgoingDtlsData(); SetTimeout(); break; @@ -847,9 +872,6 @@ namespace RTC // Must call SSL_read() to process received DTLS data. read = SSL_read(this->ssl, static_cast(DtlsTransport::sslReadBuffer), SslReadBufferSize); - // Send data if it's ready. - SendPendingOutgoingDtlsData(); - // Check SSL status and return if it is bad/closed. if (!CheckStatus(read)) return; @@ -910,9 +932,14 @@ namespace RTC MS_WARN_TAG( dtls, "OpenSSL SSL_write() wrote less (%d bytes) than given data (%zu bytes)", written, len); } + } - // Send data. - SendPendingOutgoingDtlsData(); + void DtlsTransport::SendDtlsData(const uint8_t* data, size_t len) + { + MS_TRACE(); + + // Notify the listener. + this->listener->OnDtlsTransportSendData(this, data, len); } void DtlsTransport::Reset() @@ -929,9 +956,8 @@ namespace RTC // Stop the DTLS timer. this->timer->Stop(); - // We need to reset the SSL instance so we need to "shutdown" it, but we - // don't want to send a Close Alert to the peer, so just don't call - // SendPendingOutgoingDTLSData(). + // NOTE: We need to reset the SSL instance so we need to "shutdown" it, but we + // don't want to send a Close Alert to the peer. However this is gonna happen. SSL_shutdown(this->ssl); this->localRole.reset(); @@ -1044,32 +1070,6 @@ namespace RTC } } - inline void DtlsTransport::SendPendingOutgoingDtlsData() - { - MS_TRACE(); - - if (BIO_eof(this->sslBioToNetwork)) - return; - - int64_t read; - char* data{ nullptr }; - - read = BIO_get_mem_data(this->sslBioToNetwork, &data); // NOLINT - - if (read <= 0) - return; - - MS_DEBUG_DEV("%" PRIu64 " bytes of DTLS data ready to sent to the peer", read); - - // Notify the listener. - this->listener->OnDtlsTransportSendData( - this, reinterpret_cast(data), static_cast(read)); - - // Clear the BIO buffer. - // NOTE: the (void) avoids the -Wunused-value warning. - (void)BIO_reset(this->sslBioToNetwork); - } - inline bool DtlsTransport::SetTimeout() { MS_TRACE(); @@ -1512,9 +1512,6 @@ namespace RTC if (ret == 1) { - // If required, send DTLS data. - SendPendingOutgoingDtlsData(); - // Set the DTLS timer again. SetTimeout(); } From e2c9fbf8f8e27950f2564759c06b9609fefd3877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?I=C3=B1aki=20Baz=20Castillo?= Date: Wed, 13 Sep 2023 13:30:05 +0200 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2619eedf3b..76f1412a91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * Add optional `rtcpListenInfo` in `PlainTransportOptions` ([PR #1099](https://github.com/versatica/mediasoup/pull/1099)). * Add pause/resume API in `DataProducer` and `DataConsumer` ([PR #1104](https://github.com/versatica/mediasoup/pull/1104)). * DataChannel subchannels feature ([PR #1152](https://github.com/versatica/mediasoup/pull/1152)). +* `Worker`: Make DTLS fragment stay within MTU size range ([PR #1156](https://github.com/versatica/mediasoup/pull/1156), based on [PR #1143](https://github.com/versatica/mediasoup/pull/1143) by @vpnts-se). ### 3.12.12