Compare auth tags in constant time (#14)

Author: gBillal

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)
 
       <<encrypted_data::binary-size(byte_size(data) - tag_size), tag::binary>> = 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
 
       <<rtcp_data::binary-size(authenticated_data_size - 4), e::1, index::31, tag::binary>> = 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 ->

native/ex_srtp/Cargo.lock

@@ -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"

native/ex_srtp/Cargo.toml

@@ -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<OwnedBinary, String> {
         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());
         }
 

native/ex_srtp/src/cipher/aes_cm_hmac_sha1.rs

@@ -98,7 +98,7 @@ fn unprotect<'a>(
     state: ResourceArc<State>,
     header: Binary<'a>,
     payload: Binary<'a>,
-) -> Result<Binary<'a>, String> {
+) -> Result<Binary<'a>, 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,24 +109,29 @@ 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]
 fn unprotect_rtcp<'a>(
     env: Env<'a>,
     state: ResourceArc<State>,
     data: Binary<'a>,
-) -> Result<Binary<'a>, String> {
+) -> Result<Binary<'a>, 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]

native/ex_srtp/src/lib.rs

@@ -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