From dd309a32520937d4111a62af62532385ca68a03a Mon Sep 17 00:00:00 2001 From: Billal GHILAS Date: Sun, 11 Jan 2026 12:01:44 +0100 Subject: [PATCH] Compare auth tags in constant time --- lib/ex_srtp/cipher/aes_cm_hmac_sha1.ex | 11 +++--- native/ex_srtp/Cargo.lock | 1 + native/ex_srtp/Cargo.toml | 1 + native/ex_srtp/src/cipher/aes_cm_hmac_sha1.rs | 15 ++++---- native/ex_srtp/src/lib.rs | 25 ++++++++------ test/ex_srtp_test.exs | 34 +++++++++++++++---- 6 files changed, 59 insertions(+), 28 deletions(-) diff --git a/lib/ex_srtp/cipher/aes_cm_hmac_sha1.ex b/lib/ex_srtp/cipher/aes_cm_hmac_sha1.ex index 6175097..f97d1d2 100644 --- a/lib/ex_srtp/cipher/aes_cm_hmac_sha1.ex +++ b/lib/ex_srtp/cipher/aes_cm_hmac_sha1.ex @@ -78,9 +78,9 @@ defmodule ExSRTP.Cipher.AesCmHmacSha1 do header_size = byte_size(data) - byte_size(packet.payload) <> = data - new_tag = generate_srtp_auth_tag(cipher, encrypted_data, roc) + expected_tag = generate_srtp_auth_tag(cipher, encrypted_data, roc) - if tag == new_tag do + if :crypto.hash_equals(tag, expected_tag) do idx = packet.ssrc <<< 48 ||| roc <<< 16 ||| packet.sequence_number iv = bxor(cipher.rtp_salt, idx <<< 16) @@ -95,7 +95,7 @@ defmodule ExSRTP.Cipher.AesCmHmacSha1 do {:ok, %{packet | payload: payload}} else - {:error, :auth_failed} + {:error, :authentication_failed} end end @@ -119,10 +119,11 @@ defmodule ExSRTP.Cipher.AesCmHmacSha1 do <> = data - new_tag = generate_srtcp_auth_tag(cipher, binary_part(data, 0, authenticated_data_size)) + expected_tag = + generate_srtcp_auth_tag(cipher, binary_part(data, 0, authenticated_data_size)) cond do - new_tag != tag -> + not :crypto.hash_equals(tag, expected_tag) -> {:error, :authentication_failed} e == 0 -> diff --git a/native/ex_srtp/Cargo.lock b/native/ex_srtp/Cargo.lock index b9a3796..08d04ed 100644 --- a/native/ex_srtp/Cargo.lock +++ b/native/ex_srtp/Cargo.lock @@ -97,6 +97,7 @@ dependencies = [ "hmac", "rustler", "sha1", + "subtle", ] [[package]] diff --git a/native/ex_srtp/Cargo.toml b/native/ex_srtp/Cargo.toml index 2b5a303..497d027 100644 --- a/native/ex_srtp/Cargo.toml +++ b/native/ex_srtp/Cargo.toml @@ -14,6 +14,7 @@ aes = "0.8.4" ctr = "0.9.2" hmac = "0.12.1" rustler = "0.37.2" +subtle = "2.6.1" [target.'cfg(windows)'.dependencies] sha1 = "0.10.6" 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 8b502f2..8ad9641 100644 --- a/native/ex_srtp/src/cipher/aes_cm_hmac_sha1.rs +++ b/native/ex_srtp/src/cipher/aes_cm_hmac_sha1.rs @@ -1,6 +1,7 @@ use aes::cipher::{KeyIvInit, StreamCipher}; use hmac::Mac; use rustler::OwnedBinary; +use subtle::ConstantTimeEq; use crate::{ cipher::Cipher, key_derivation::aes_cm_key_derivation, protection_profile::ProtectionProfile, @@ -101,16 +102,18 @@ impl Cipher for AesCmHmacSha1Cipher { roc: u32, ) -> Result { let (encrypted_data, auth_tag) = payload.split_at(payload.len() - self.profile.tag_size()); - let expected_tag = - self.generate_rtp_auth_tag(&[&header[..], &encrypted_data[..], &roc.to_be_bytes()[..]]); + let expected_tag = &self.generate_rtp_auth_tag(&[ + &header[..], + &encrypted_data[..], + &roc.to_be_bytes()[..], + ]); - if auth_tag != expected_tag.as_slice() { + if auth_tag.ct_eq(expected_tag).unwrap_u8() != 1 { return Err("authentication_failed".to_string()); } let size = payload.len() - self.profile.tag_size(); let mut owned_binary = OwnedBinary::new(size).unwrap(); - // owned_binary.as_mut_slice()[..header.len()].copy_from_slice(header); owned_binary .as_mut_slice() .copy_from_slice(&payload[..payload.len() - self.profile.tag_size()]); @@ -148,8 +151,8 @@ impl Cipher for AesCmHmacSha1Cipher { let tag_size = self.profile.tag_size(); let (data, auth_tag) = compound_packet.split_at(compound_packet.len() - tag_size); - let expected_tag = self.generate_rtcp_auth_tag(data); - if auth_tag != expected_tag.as_slice() { + let expected_tag = &self.generate_rtcp_auth_tag(data); + if auth_tag.ct_eq(expected_tag).unwrap_u8() != 1 { return Err("authentication_failed".to_string()); } diff --git a/native/ex_srtp/src/lib.rs b/native/ex_srtp/src/lib.rs index 89c4812..9d29477 100644 --- a/native/ex_srtp/src/lib.rs +++ b/native/ex_srtp/src/lib.rs @@ -98,7 +98,7 @@ fn unprotect<'a>( state: ResourceArc, header: Binary<'a>, payload: Binary<'a>, -) -> Result, String> { +) -> Result, Atom> { let mut session = state.session.lock().unwrap(); let ssrc = u32::from_be_bytes(header.as_slice()[8..12].try_into().unwrap()); let seq = u16::from_be_bytes(header.as_slice()[2..4].try_into().unwrap()); @@ -109,13 +109,16 @@ fn unprotect<'a>( .or_insert_with(|| RTPContext::default()); let roc = ctx.estimate_roc(seq); - - let owned = session + match session .cipher - .decrypt_rtp(&header.as_slice(), &payload.as_slice(), roc)?; - - session.in_rtp_ctx.get_mut(&ssrc).unwrap().update_roc(seq); - return Ok(Binary::from_owned(owned, env)); + .decrypt_rtp(&header.as_slice(), &payload.as_slice(), roc) + { + Err(err) => Err(Atom::from_str(env, err.as_str()).unwrap()), + Ok(owned) => { + session.in_rtp_ctx.get_mut(&ssrc).unwrap().update_roc(seq); + Ok(Binary::from_owned(owned, env)) + } + } } #[rustler::nif] @@ -123,10 +126,12 @@ fn unprotect_rtcp<'a>( env: Env<'a>, state: ResourceArc, data: Binary<'a>, -) -> Result, String> { +) -> Result, Atom> { let mut session = state.session.lock().unwrap(); - let owned = session.cipher.decrypt_rtcp(&data.as_slice())?; - return Ok(Binary::from_owned(owned, env)); + match session.cipher.decrypt_rtcp(&data.as_slice()) { + Err(err) => return Err(Atom::from_str(env, err.as_str()).unwrap()), + Ok(owned) => Ok(Binary::from_owned(owned, env)), + } } #[rustler::nif] diff --git a/test/ex_srtp_test.exs b/test/ex_srtp_test.exs index 0b652e4..ccf2f51 100644 --- a/test/ex_srtp_test.exs +++ b/test/ex_srtp_test.exs @@ -86,22 +86,21 @@ defmodule ExSRTPTest do end describe "protect rtcp" do - test "Erlnag backend", %{srtp: srtp, compound_packet: compound_packet} do + setup do expected = <<128, 200, 0, 6, 137, 161, 255, 135, 235, 3, 169, 113, 236, 134, 217, 36, 127, 210, 78, 156, 66, 244, 203, 218, 58, 80, 24, 60, 28, 171, 30, 89, 192, 155, 19, 59, 128, 0, 0, 1, 139, 226, 152, 17, 40, 71, 251, 110, 11, 235>> + {:ok, expected: expected} + end + + test "Erlnag backend", %{srtp: srtp, compound_packet: compound_packet, expected: expected} do assert {:ok, ^expected, _srtp} = ExSRTP.protect_rtcp(compound_packet, srtp) assert {^expected, _srtp} = ExSRTP.protect_rtcp!(compound_packet, srtp) end - test "Rust backend", %{rust_srtp: srtp, compound_packet: compound_packet} do - expected = - <<128, 200, 0, 6, 137, 161, 255, 135, 235, 3, 169, 113, 236, 134, 217, 36, 127, 210, 78, - 156, 66, 244, 203, 218, 58, 80, 24, 60, 28, 171, 30, 89, 192, 155, 19, 59, 128, 0, 0, 1, - 139, 226, 152, 17, 40, 71, 251, 110, 11, 235>> - + test "Rust backend", %{rust_srtp: srtp, compound_packet: compound_packet, expected: expected} do assert {:ok, ^expected, _srtp} = RustCrypto.protect_rtcp(compound_packet, srtp) end end @@ -137,6 +136,15 @@ defmodule ExSRTPTest do assert {:ok, ^packet, srtp} = RustCrypto.unprotect(protected_packet, srtp) assert {:error, :replay} = RustCrypto.unprotect(protected_packet, srtp) end + + test "authentication failed", %{srtp: srtp, rust_srtp: rust_srtp} do + packet = + <<128, 96, 0, 1, 0, 1, 226, 64, 137, 161, 255, 135, 146, 221, 94, 142, 7, 197, 169, 172, + 155, 23, 74, 128, 181, 142, 46>> + + assert {:error, :authentication_failed} = ExSRTP.unprotect(packet, srtp) + assert {:error, :authentication_failed} = RustCrypto.unprotect(packet, rust_srtp) + end end describe "unprotect rtcp" do @@ -185,6 +193,18 @@ defmodule ExSRTPTest do ExSRTP.unprotect_rtcp!(protected_rtcp, srtp) end end + + test "authentication failed", %{srtp: srtp, rust_srtp: rust_srtp} do + protected_rtcp = + <<128, 200, 0, 6, 137, 161, 255, 135, 235, 3, 169, 113, 236, 134, 217, 36, 127, 210, 78, + 156, 66, 244, 203, 218, 58, 80, 24, 60, 28, 171, 30, 89, 192, 155, 19, 59, 128, 0, 0, 1, + 139, 226, 152, 17, 40, 70, 251, 110, 11, 235>> + + assert {:error, :authentication_failed} = ExSRTP.unprotect_rtcp(protected_rtcp, srtp) + + assert {:error, :authentication_failed} = + RustCrypto.unprotect_rtcp(protected_rtcp, rust_srtp) + end end for profile <- [:aes_cm_128_hmac_sha1_80, :aes_cm_128_hmac_sha1_32, :aes_gcm_128_16_auth] do