Fix DTLS packets do not honor configured DTLS MTU (attempt 3)

[ibc]

• Fixes DTLS fragment exceeds MTU size if the certificate is large. #1100
• Based on PR Make DTLS fragment stay within MTU size range #1156 which was based on PR fix/make dtls fragment stay within mtu size range #1143

Details

This PR is the same as PR #1156. However that PR introduced a memory leak (see issue #1340). This PR fixes that leak by following the discussion and research in associated issues and PRs, specially here: #1340 (comment)

To prove that this PR works at DTLS level, here the DTLS messages sent from mediasoup to browser while doing the DTLS handshake:

RTC::DtlsTransport::SendDtlsData() | 1349 bytes of DTLS data ready to be sent
RTC::DtlsTransport::SendDtlsData() | 1349 bytes of DTLS data ready to be sent
RTC::DtlsTransport::SendDtlsData() | 1349 bytes of DTLS data ready to be sent
RTC::DtlsTransport::SendDtlsData() | 1349 bytes of DTLS data ready to be sent
RTC::DtlsTransport::SendDtlsData() | 1349 bytes of DTLS data ready to be sent
RTC::DtlsTransport::SendDtlsData() | 242 bytes of DTLS data ready to be sent
RTC::DtlsTransport::SendDtlsData() | 67 bytes of DTLS data ready to be sent
RTC::DtlsTransport::SendDtlsData() | 129 bytes of DTLS data ready to be sent
RTC::DtlsTransport::SendDtlsData() | 385 bytes of DTLS data ready to be sent
RTC::DtlsTransport::SendDtlsData() | 45 bytes of DTLS data ready to be sent

which clearly corresponds to what we see in Wireshark (there is a huge certificate so it's splitted into N DTLS Certificate fragments):

Note that sizes of each message match the DTLS data sent by mediasoup plus UDP and IP encapsulation (32 bytes always).

[ibc]

CC @gkrol-katmai @pnts-se

[jmillan]

👍 👍

[gkrol-katmai]

The code looks good to me! I think we now have a good grasp of what's going on. I know that adding the 'reset' in that function will fix the leak. I haven't tested this PR.

Not sure what the coding standards are for comments, but I would suggest we add comments to onSslBioOut and DtlsTransport::SendDtlsData (above the implementation). Those should describe the effect of the function and when/why it gets called. Preferably /** */ style. We're doing some subtle things here so we need to be extra clear.

[ibc]

I know that adding the 'reset' in that function will fix the leak. I haven't tested this PR.

I think this is also proven here #1340 (comment) where it's clear that the onSslBioOut is always called with len matching the previous mem_write() call.

Not sure what the coding standards are for comments, but I would suggest we add comments to onSslBioOut and DtlsTransport::SendDtlsData (above the implementation). Those should describe the effect of the function and when/why it gets called. Preferably /** */ style. We're doing some subtle things here so we need to be extra clear.

Done here: e827248

[ibc]

BTW I've added some logs and Wireshark capture in the PR description above proving that everything makes sense.

[ibc]

Let's merge and release. Thanks a lot.