Repository navigation
Check room for the SRTP trailer in rtpp_dtls_conn_rtp_send() - #180
Merged
Merged
Conversation
srtp_protect() and srtp_protect_rtcp() encrypt in place and append the auth tag (and the SRTCP index) right after the packet, and libsrtp requires the caller to leave SRTP_MAX_TRAILER_LEN (+4 for SRTCP) writable bytes there. Packets are received into the whole 8192-byte data.buf, so a datagram that comes within a tag's length of filling it (larger ones are truncated to 8192 bytes) and is relayed from the plain leg into a DTLS-SRTP leg had its tag written past data.buf, over the wi pointer and rinfo in rtp_packet_priv. The resulting 8202-byte packet was then sent from the 8192-byte buffer. Drop such packets instead of protecting them. SRTP_MAX_SRTCP_TRAILER_LEN only exists in libsrtp2 2.6.0 and later, so define it the way older headers document it. Add a dtls_gw/basic sub-test that sends an oversized datagram into the plain side of Fwd between two normal ones and expects it to be dropped. Validated with libsrtp2 2.7.0: before the change Fwd relays the packet and emits an 8202-byte SRTP datagram, after it the packet is counted in ndropped and the normal ones around it are still relayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Repro: bridge a plain RTP leg to a DTLS-SRTP leg with dtls_gw and send a 9000-byte UDP datagram to the plain side. recvfrom() truncates it to the full 8192-byte
data.buf, Fwd relays it, and tcpdump shows an 8202-byte SRTP datagram leaving on the DTLS leg (libsrtp2 2.7.0).Cause:
rtpp_dtls_conn_rtp_send()callssrtp_protect()/srtp_protect_rtcp()in place without leaving theSRTP_MAX_TRAILER_LEN(+4 for SRTCP) bytes that libsrtp requires after the packet, so the tag is written pastdata.bufintortp_packet_priv.Fix: drop the packet when it doesn't leave room for the trailer.
With an 80-bit tag the overwrite turns
pvt.wifrom NULL into tag bytes andrinfo.data_size(whatpkt->parsedpoints at) from 8180 into 36072, and SRTCP runs 14 bytes over. The sender only has to reach the plain leg's RTP port, e.g. the SIP-side peer of a WebRTC call.SRTP_MAX_SRTCP_TRAILER_LENfirst appeared in libsrtp2 2.6.0, hence the fallback for older headers such as the 2.4.2 in Ubuntu 22.04.The new dtls_gw/basic sub-test sends normal/oversized/normal datagrams into Fwd's plain side and expects
npkts_ina=3 nrelayed=2 ndropped=1; on master it getsnrelayed=3 ndropped=0. dtls_gw/basic, startstop/startstop1, startstop/nodebug and command_parser/ul_subc pass locally.