Skip to content

Commit

Permalink
Fix srtp_unprotect_rtcp_mki when RTP auth != RTCP
Browse files Browse the repository at this point in the history 
srtp_get_session_keys, which is used by both srtp_unprotect_mki and
srtp_unprotect_rtcp_mki, determines the tag len from rtp_auth.

This fails when rtp_auth differ from rtcp_auth. E.g. when SRTP is used
with weak authentication but SRTCP must not (RFC 3711).

This commit splits the function in two:
srtp_get_session_keys_rtp
srtp_get_session_keys_rtcp

And adds a short auth policy test to test/srtp_driver.
  • Loading branch information
@vopatek
vopatek committed Dec 6, 2024
1 parent 3a399ab commit 63a19f4
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 16 deletions.
62 changes: 46 additions & 16 deletions srtp/srtp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1638,24 +1638,16 @@ static void srtp_calc_aead_iv(srtp_session_keys_t *session_keys,
v128_xor(iv, &in, &salt);
}

srtp_session_keys_t *srtp_get_session_keys(srtp_stream_ctx_t *stream,
const uint8_t *hdr,
unsigned int pkt_octet_len,
unsigned int *mki_size)
static srtp_session_keys_t *srtp_get_session_keys(srtp_stream_ctx_t *stream,
const uint8_t *hdr,
unsigned int pkt_octet_len,
unsigned int *mki_size,
unsigned int tag_len)
{
unsigned int base_mki_start_location = pkt_octet_len;
unsigned int mki_start_location = 0;
unsigned int tag_len = 0;
unsigned int i = 0;

// Determine the authentication tag size
if (stream->session_keys[0].rtp_cipher->algorithm == SRTP_AES_GCM_128 ||
stream->session_keys[0].rtp_cipher->algorithm == SRTP_AES_GCM_256) {
tag_len = 0;
} else {
tag_len = srtp_auth_get_tag_length(stream->session_keys[0].rtp_auth);
}

if (tag_len > base_mki_start_location) {
*mki_size = 0;
return NULL;
Expand All @@ -1680,6 +1672,44 @@ srtp_session_keys_t *srtp_get_session_keys(srtp_stream_ctx_t *stream,
return NULL;
}

static srtp_session_keys_t *srtp_get_session_keys_rtp(
srtp_stream_ctx_t *stream,
const uint8_t *hdr,
unsigned int pkt_octet_len,
unsigned int *mki_size)
{
unsigned int tag_len = 0;

// Determine the authentication tag size
if (stream->session_keys[0].rtp_cipher->algorithm == SRTP_AES_GCM_128 ||
stream->session_keys[0].rtp_cipher->algorithm == SRTP_AES_GCM_256) {
tag_len = 0;
} else {
tag_len = srtp_auth_get_tag_length(stream->session_keys[0].rtp_auth);
}

return srtp_get_session_keys(stream, hdr, pkt_octet_len, mki_size, tag_len);
}

static srtp_session_keys_t *srtp_get_session_keys_rtcp(
srtp_stream_ctx_t *stream,
const uint8_t *hdr,
unsigned int pkt_octet_len,
unsigned int *mki_size)
{
unsigned int tag_len = 0;

// Determine the authentication tag size
if (stream->session_keys[0].rtcp_cipher->algorithm == SRTP_AES_GCM_128 ||
stream->session_keys[0].rtcp_cipher->algorithm == SRTP_AES_GCM_256) {
tag_len = 0;
} else {
tag_len = srtp_auth_get_tag_length(stream->session_keys[0].rtcp_auth);
}

return srtp_get_session_keys(stream, hdr, pkt_octet_len, mki_size, tag_len);
}

static srtp_err_status_t srtp_estimate_index(srtp_rdbx_t *rdbx,
uint32_t roc,
srtp_xtd_seq_num_t *est,
Expand Down Expand Up @@ -2583,8 +2613,8 @@ srtp_err_status_t srtp_unprotect_mki(srtp_ctx_t *ctx,
/* Determine if MKI is being used and what session keys should be used */
if (use_mki) {
session_keys =
srtp_get_session_keys(stream, (const uint8_t *)hdr,
(unsigned int)*pkt_octet_len, &mki_size);
srtp_get_session_keys_rtp(stream, (const uint8_t *)hdr,
(unsigned int)*pkt_octet_len, &mki_size);

if (session_keys == NULL)
return srtp_err_status_bad_mki;
Expand Down Expand Up @@ -4293,7 +4323,7 @@ srtp_err_status_t srtp_unprotect_rtcp_mki(srtp_t ctx,
* Determine if MKI is being used and what session keys should be used
*/
if (use_mki) {
session_keys = srtp_get_session_keys(
session_keys = srtp_get_session_keys_rtcp(
stream, (uint8_t *)hdr, (unsigned int)*pkt_octet_len, &mki_size);

if (session_keys == NULL)
Expand Down
33 changes: 33 additions & 0 deletions test/srtp_driver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4279,6 +4279,38 @@ const srtp_policy_t aes_256_hmac_policy = {
NULL
};

const srtp_policy_t aes_256_hmac_32_policy = {
{ ssrc_any_outbound, 0 }, /* SSRC */
{
/* SRTP policy */
SRTP_AES_ICM_256, /* cipher type */
SRTP_AES_ICM_256_KEY_LEN_WSALT, /* cipher key length in octets */
SRTP_HMAC_SHA1, /* authentication func type */
20, /* auth key length in octets */
4, /* auth tag length in octets */
sec_serv_conf_and_auth /* security services flag */
},
{
/* SRTCP policy */
SRTP_AES_ICM_256, /* cipher type */
SRTP_AES_ICM_256_KEY_LEN_WSALT, /* cipher key length in octets */
SRTP_HMAC_SHA1, /* authentication func type */
20, /* auth key length in octets */
10, /* auth tag length in octets.
80 bits per RFC 3711. */
sec_serv_conf_and_auth /* security services flag */
},
NULL,
(srtp_master_key_t **)test_256_keys,
2, /* indicates the number of Master keys */
NULL, /* indicates that EKT is not in use */
128, /* replay window size */
0, /* retransmission not allowed */
NULL, /* no encrypted extension headers */
0, /* list of encrypted extension headers is empty */
NULL
};

char ekt_test_policy = 'x';

const srtp_policy_t hmac_only_with_ekt_policy = {
Expand Down Expand Up @@ -4333,6 +4365,7 @@ const srtp_policy_t *policy_array[] = {
#endif
&null_policy,
&aes_256_hmac_policy,
&aes_256_hmac_32_policy,
NULL
};
// clang-format on
Expand Down

0 comments on commit 63a19f4

Please sign in to comment.