Two-byte RTP header extension encryption does not work

cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)

Other

1.22k stars 475 forks source link

Two-byte RTP header extension encryption does not work #490

Closed lgrahl closed 4 years ago

lgrahl commented 4 years ago

Hi folks, I'm confused by the logical and with 0x1fff. In order to just ignore the last 4 (application) bits, following RFC 8285, sec 4.3, shouldn't this logical and with 0xfff0? Or am I missing something?

https://github.com/cisco/libsrtp/blob/90b3124060ef8ba257cbd933a5a0b96dbdcf1efa/srtp/srtp.c#L1426

paulej commented 4 years ago

At first glance, this does appear to be wrong. In addition to the logical and you suggest, the result needs to be shifted 4 bits to the right to compare against 0x100.

lgrahl commented 4 years ago

Yep. That being said, I questioned my sanity multiple times reading that section of the RFC. And, apparently, I haven't been the only one. This explains why libsrtp refused to do anything with two-byte header extensions.

So, IIRC, this line needs to be:

} else if ((ntohs(xtn_hdr->profile_specific) & 0xfff0) == 0x1000) {

Having it patched that way, it finally does encrypt two-byte header extensions (and the application bits are being ignored).

lgrahl commented 4 years ago

Perhaps a small note on security: I've found this bug in relation with libwebrtc but since a) header extension encryption in general is currently broken in libwebrtc (I'm preparing a patch) and b) this library fails explicitly and no packets are being sent following an error in libwebrtc, it's not something that would demand filing a CVE.