From 0f9da2a4b368ff81bba6fced06a20ddff1b29db1 Mon Sep 17 00:00:00 2001 From: Billal GHILAS Date: Thu, 15 Jan 2026 12:18:33 +0100 Subject: [PATCH] Add sanity checks --- lib/ex_srtp/backend/crypto.ex | 7 ++-- lib/ex_srtp/backend/rust_crypto.ex | 13 +++---- lib/ex_srtp/cipher/aes_cm_hmac_sha1.ex | 5 +-- lib/ex_srtp/helper.ex | 12 +++++-- native/ex_srtp/src/cipher/aes_cm_hmac_sha1.rs | 35 +++++++++++++------ native/ex_srtp/src/cipher/aes_gcm.rs | 30 ++++++++++------ test/ex_srtp_test.exs | 10 ++++++ 7 files changed, 77 insertions(+), 35 deletions(-) diff --git a/lib/ex_srtp/backend/crypto.ex b/lib/ex_srtp/backend/crypto.ex index 5d0225b..539b0a4 100644 --- a/lib/ex_srtp/backend/crypto.ex +++ b/lib/ex_srtp/backend/crypto.ex @@ -95,10 +95,11 @@ defmodule ExSRTP.Backend.Crypto do @impl true def unprotect_rtcp(data, session) do tag_size = Cipher.tag_size(session.cipher) - {ssrc, index} = Helper.rtcp_index(tag_size, data) - ctx = session.in_rtcp_contexts[ssrc] || RTCPContext.new(session.rtcp_replay_window_size) - with {:ok, ctx} <- RTCPContext.check_replay(ctx, index), + with {:ok, ssrc, index} <- Helper.rtcp_index(tag_size, data), + ctx <- + session.in_rtcp_contexts[ssrc] || RTCPContext.new(session.rtcp_replay_window_size), + {:ok, ctx} <- RTCPContext.check_replay(ctx, index), {:ok, decrypted_data} <- Cipher.decrypt_rtcp(session.cipher, data), {:ok, packets} <- CompoundPacket.decode(decrypted_data) do session = %{ diff --git a/lib/ex_srtp/backend/rust_crypto.ex b/lib/ex_srtp/backend/rust_crypto.ex index f9e59d8..26e5bbd 100644 --- a/lib/ex_srtp/backend/rust_crypto.ex +++ b/lib/ex_srtp/backend/rust_crypto.ex @@ -121,12 +121,9 @@ defmodule ExSRTP.Backend.RustCrypto do :aes_gcm_128_16_auth -> 0 end - {ssrc, index} = ExSRTP.Helper.rtcp_index(tag_size, protected_packet) - - replay_list = - session.rtcp_replay_list[ssrc] || ReplayList.new(session.rtcp_replay_window_size) - - with {:ok, replay_list} <- ReplayList.check_and_update(replay_list, index), + with {:ok, ssrc, index} <- ExSRTP.Helper.rtcp_index(tag_size, protected_packet), + replay_list <- rtcp_replay_list(session, ssrc), + {:ok, replay_list} <- ReplayList.check_and_update(replay_list, index), {:ok, unprotected_data} <- Native.unprotect_rtcp(native, protected_packet), {:ok, packets} <- CompoundPacket.decode(unprotected_data) do session = %{ @@ -137,4 +134,8 @@ defmodule ExSRTP.Backend.RustCrypto do {:ok, packets, session} end end + + defp rtcp_replay_list(session, ssrc) do + session.rtcp_replay_list[ssrc] || ReplayList.new(session.rtcp_replay_window_size) + end end diff --git a/lib/ex_srtp/cipher/aes_cm_hmac_sha1.ex b/lib/ex_srtp/cipher/aes_cm_hmac_sha1.ex index f97d1d2..60070d4 100644 --- a/lib/ex_srtp/cipher/aes_cm_hmac_sha1.ex +++ b/lib/ex_srtp/cipher/aes_cm_hmac_sha1.ex @@ -75,12 +75,11 @@ defmodule ExSRTP.Cipher.AesCmHmacSha1 do def decrypt_rtp(cipher, data, packet, roc) do tag_size = tag_size(cipher) - header_size = byte_size(data) - byte_size(packet.payload) - <> = data expected_tag = generate_srtp_auth_tag(cipher, encrypted_data, roc) if :crypto.hash_equals(tag, expected_tag) do + header_size = byte_size(data) - byte_size(packet.payload) idx = packet.ssrc <<< 48 ||| roc <<< 16 ||| packet.sequence_number iv = bxor(cipher.rtp_salt, idx <<< 16) @@ -146,6 +145,7 @@ defmodule ExSRTP.Cipher.AesCmHmacSha1 do end end + @compile {:inline, generate_srtp_auth_tag: 3} defp generate_srtp_auth_tag(cipher, data, roc) do :crypto.macN( :hmac, @@ -160,6 +160,7 @@ defmodule ExSRTP.Cipher.AesCmHmacSha1 do :crypto.macN(:hmac, :sha, cipher.rtcp_auth_key, data, tag_size(cipher)) end + @compile {:inline, tag_size: 1} def tag_size(%{profile: :aes_cm_128_hmac_sha1_80}), do: 10 def tag_size(%{profile: :aes_cm_128_hmac_sha1_32}), do: 4 end diff --git a/lib/ex_srtp/helper.ex b/lib/ex_srtp/helper.ex index 72f16da..83cbca8 100644 --- a/lib/ex_srtp/helper.ex +++ b/lib/ex_srtp/helper.ex @@ -1,9 +1,15 @@ defmodule ExSRTP.Helper do @moduledoc false - @spec rtcp_index(integer(), binary()) :: {non_neg_integer(), non_neg_integer()} + @spec rtcp_index(integer(), binary()) :: + {:ok, non_neg_integer(), non_neg_integer()} | {:error, :not_enough_data} def rtcp_index(tag_size, <<_::32, ssrc::32, rest::binary>>) do - <<_::binary-size(byte_size(rest) - tag_size - 4), _e::1, index::31, _tag::binary>> = rest - {ssrc, index} + case rest do + <<_::binary-size(byte_size(rest) - tag_size - 4), _e::1, index::31, _tag::binary>> -> + {:ok, ssrc, index} + + _ -> + {:error, :not_enough_data} + end end end diff --git a/native/ex_srtp/src/cipher/aes_cm_hmac_sha1.rs b/native/ex_srtp/src/cipher/aes_cm_hmac_sha1.rs index 8ad9641..c6c4a44 100644 --- a/native/ex_srtp/src/cipher/aes_cm_hmac_sha1.rs +++ b/native/ex_srtp/src/cipher/aes_cm_hmac_sha1.rs @@ -8,6 +8,9 @@ use crate::{ Aes128Ctr, HmacSha1, }; +const RTCP_INDEX_SIZE: usize = 4; +const RTCP_HEADER_SIZE: usize = 8; + pub(crate) struct AesCmHmacSha1Cipher { profile: ProtectionProfile, rtp_session_key: Vec, @@ -101,6 +104,10 @@ impl Cipher for AesCmHmacSha1Cipher { payload: &[u8], roc: u32, ) -> Result { + if payload.len() < self.profile.tag_size() { + return Err("not_enough_data".to_string()); + } + let (encrypted_data, auth_tag) = payload.split_at(payload.len() - self.profile.tag_size()); let expected_tag = &self.generate_rtp_auth_tag(&[ &header[..], @@ -130,25 +137,31 @@ impl Cipher for AesCmHmacSha1Cipher { let mut index_bytes = index.to_be_bytes(); index_bytes[0] |= 0x80; - let size = compound_packet.len() + self.profile.tag_size() + 4; + let size = compound_packet.len() + self.profile.tag_size() + RTCP_INDEX_SIZE; let mut owned_binary = OwnedBinary::new(size).unwrap(); let slice = owned_binary.as_mut_slice(); slice[..compound_packet.len()].copy_from_slice(compound_packet); - slice[compound_packet.len()..compound_packet.len() + 4].copy_from_slice(&index_bytes); + slice[compound_packet.len()..compound_packet.len() + RTCP_INDEX_SIZE] + .copy_from_slice(&index_bytes); let iv = Self::rtcp_initialization_vector(&self.rtcp_salt, ssrc, index); Aes128Ctr::new(self.rtcp_session_key.as_slice().into(), &iv.into()) - .apply_keystream(&mut slice[8..compound_packet.len()]); + .apply_keystream(&mut slice[RTCP_HEADER_SIZE..compound_packet.len()]); - let auth_tag = self.generate_rtcp_auth_tag(&slice[..compound_packet.len() + 4]); + let auth_tag = + self.generate_rtcp_auth_tag(&slice[..compound_packet.len() + RTCP_INDEX_SIZE]); - slice[compound_packet.len() + 4..].copy_from_slice(&auth_tag); - return owned_binary; + slice[compound_packet.len() + RTCP_INDEX_SIZE..].copy_from_slice(&auth_tag); + owned_binary } fn decrypt_rtcp(&mut self, compound_packet: &[u8]) -> Result { let tag_size = self.profile.tag_size(); + if compound_packet.len() < tag_size + RTCP_HEADER_SIZE + RTCP_INDEX_SIZE { + return Err("not_enough_data".to_string()); + } + let (data, auth_tag) = compound_packet.split_at(compound_packet.len() - tag_size); let expected_tag = &self.generate_rtcp_auth_tag(data); @@ -156,8 +169,8 @@ impl Cipher for AesCmHmacSha1Cipher { return Err("authentication_failed".to_string()); } - let (header, rest) = data.split_at(8); - let (encrypted_data, index_bytes) = rest.split_at(rest.len() - 4); + let (header, rest) = data.split_at(RTCP_HEADER_SIZE); + let (encrypted_data, index_bytes) = rest.split_at(rest.len() - RTCP_INDEX_SIZE); let ssrc = u32::from_be_bytes(header[4..8].try_into().unwrap()); let mut index = u32::from_be_bytes(index_bytes.try_into().unwrap()); @@ -171,12 +184,12 @@ impl Cipher for AesCmHmacSha1Cipher { let size = header.len() + encrypted_data.len(); let mut owned_binary = OwnedBinary::new(size).unwrap(); - owned_binary.as_mut_slice()[..header.len()].copy_from_slice(header); - owned_binary.as_mut_slice()[header.len()..].copy_from_slice(encrypted_data); + owned_binary.as_mut_slice()[..RTCP_HEADER_SIZE].copy_from_slice(header); + owned_binary.as_mut_slice()[RTCP_HEADER_SIZE..].copy_from_slice(encrypted_data); let iv = Self::rtcp_initialization_vector(&self.rtcp_salt, ssrc, index); Aes128Ctr::new(self.rtcp_session_key.as_slice().into(), &iv.into()) - .apply_keystream(&mut owned_binary.as_mut_slice()[header.len()..]); + .apply_keystream(&mut owned_binary.as_mut_slice()[RTCP_HEADER_SIZE..]); Ok(owned_binary) } diff --git a/native/ex_srtp/src/cipher/aes_gcm.rs b/native/ex_srtp/src/cipher/aes_gcm.rs index 4b81237..ff42510 100644 --- a/native/ex_srtp/src/cipher/aes_gcm.rs +++ b/native/ex_srtp/src/cipher/aes_gcm.rs @@ -1,6 +1,7 @@ use rustler::OwnedBinary; const RTCP_INDEX_SIZE: usize = 4; +const RTCP_HEADER_SIZE: usize = 8; use crate::{ cipher::Cipher, key_derivation::aes_cm_key_derivation, protection_profile::ProtectionProfile, @@ -80,6 +81,10 @@ impl Cipher for AesGcmCipher { payload: &[u8], roc: u32, ) -> Result { + if payload.len() < self.profile.tag_size() { + return Err("not_enough_data".to_string()); + } + let tag_size = self.profile.tag_size(); let mut owned_binary = OwnedBinary::new(payload.len() - tag_size).unwrap(); let slice = owned_binary.as_mut_slice(); @@ -100,22 +105,23 @@ impl Cipher for AesGcmCipher { fn encrypt_rtcp(&mut self, compound_packet: &[u8], index: u32) -> OwnedBinary { let ssrc = u32::from_be_bytes(compound_packet[4..8].try_into().unwrap()); - let size = compound_packet.len() + self.profile.tag_size() + 4; + let size = compound_packet.len() + self.profile.tag_size() + RTCP_INDEX_SIZE; let mut owned_binary = OwnedBinary::new(size).unwrap(); owned_binary.as_mut_slice()[..compound_packet.len()].copy_from_slice(compound_packet); - let (header, remaining) = owned_binary.as_mut_slice().split_at_mut(8); - let (plain_text, remaining) = remaining.split_at_mut(compound_packet.len() - 8); + let (header, remaining) = owned_binary.as_mut_slice().split_at_mut(RTCP_HEADER_SIZE); + let (plain_text, remaining) = + remaining.split_at_mut(compound_packet.len() - RTCP_HEADER_SIZE); let (auth_tag, index_bytes) = remaining.split_at_mut(self.profile.tag_size()); index_bytes.copy_from_slice(&index.to_be_bytes()); index_bytes[0] |= 0x80; let iv = self.rtcp_initialization_vector(ssrc, index); - let mut aad = Vec::::with_capacity(12); - aad.extend_from_slice(&header); - aad.extend_from_slice(&index_bytes); + let mut aad = [0; 12]; + aad[..RTCP_HEADER_SIZE].copy_from_slice(&header); + aad[RTCP_HEADER_SIZE..].copy_from_slice(index_bytes); self.rtcp_c .encrypt(&iv, &aad, plain_text, auth_tag.try_into().unwrap()); @@ -124,6 +130,10 @@ impl Cipher for AesGcmCipher { } fn decrypt_rtcp(&mut self, compound_packet: &[u8]) -> Result { + if compound_packet.len() < self.profile.tag_size() + RTCP_INDEX_SIZE + RTCP_HEADER_SIZE { + return Err("not_enough_data".to_string()); + } + let ssrc = u32::from_be_bytes(compound_packet[4..8].try_into().unwrap()); let tag_size = self.profile.tag_size(); let size = compound_packet.len() - tag_size - RTCP_INDEX_SIZE; @@ -133,7 +143,7 @@ impl Cipher for AesGcmCipher { .as_mut_slice() .copy_from_slice(&compound_packet[..size]); - let (header, cipher_text) = owned_binary.as_mut_slice().split_at_mut(8); + let (header, cipher_text) = owned_binary.as_mut_slice().split_at_mut(RTCP_HEADER_SIZE); let auth_tag = &compound_packet[size..size + tag_size]; let index_bytes = &compound_packet[compound_packet.len() - RTCP_INDEX_SIZE..]; @@ -141,9 +151,9 @@ impl Cipher for AesGcmCipher { index &= 0x7FFFFFFF; let iv = self.rtcp_initialization_vector(ssrc, index); - let mut aad = Vec::::with_capacity(12); - aad.extend_from_slice(&header); - aad.extend_from_slice(&index_bytes); + let mut aad = [0; 12]; + aad[..RTCP_HEADER_SIZE].copy_from_slice(&header); + aad[RTCP_HEADER_SIZE..].copy_from_slice(index_bytes); self.rtcp_c .decrypt(&iv, &aad, cipher_text, auth_tag) diff --git a/test/ex_srtp_test.exs b/test/ex_srtp_test.exs index ccf2f51..ee1b493 100644 --- a/test/ex_srtp_test.exs +++ b/test/ex_srtp_test.exs @@ -145,6 +145,11 @@ defmodule ExSRTPTest do assert {:error, :authentication_failed} = ExSRTP.unprotect(packet, srtp) assert {:error, :authentication_failed} = RustCrypto.unprotect(packet, rust_srtp) end + + test "short packets", %{rust_srtp: rust_srtp} do + packet = <<128, 96, 0, 1, 0, 1, 226, 64, 137, 161, 255, 135, 146>> + assert {:error, :not_enough_data} = RustCrypto.unprotect(packet, rust_srtp) + end end describe "unprotect rtcp" do @@ -205,6 +210,11 @@ defmodule ExSRTPTest do assert {:error, :authentication_failed} = RustCrypto.unprotect_rtcp(protected_rtcp, rust_srtp) end + + test "short packets", %{rust_srtp: rust_srtp} do + packet = <<128, 200, 0, 6, 137, 161, 255, 135, 235>> + assert {:error, :not_enough_data} = RustCrypto.unprotect_rtcp(packet, rust_srtp) + end end for profile <- [:aes_cm_128_hmac_sha1_80, :aes_cm_128_hmac_sha1_32, :aes_gcm_128_16_auth] do