From 8f10fac3b8df5eba60e8d588804d584e391a85f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Tue, 19 Dec 2023 12:44:25 +0100 Subject: [PATCH 1/2] remove use of pointers to 32bit values There is no reason to use pointers to 32bit values, all operations are based on bytes. So This removes alot of extra casting and potential pointer arithmetic errors. Also provide some functions for calculating header values to reduce duplication and improve readability. --- srtp/srtp.c | 222 ++++++++++++++++++++++++++-------------------------- 1 file changed, 109 insertions(+), 113 deletions(-) diff --git a/srtp/srtp.c b/srtp/srtp.c index 2d9f6155b..dddf6b7e7 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -74,39 +74,50 @@ srtp_debug_module_t mod_srtp = { }; #define octets_in_rtp_header 12 -#define uint32s_in_rtp_header 3 #define octets_in_rtcp_header 8 -#define uint32s_in_rtcp_header 2 -#define octets_in_rtp_extn_hdr 4 +#define octets_in_rtp_xtn_hdr 4 + +static uint32_t srtp_get_rtp_hdr_len(const srtp_hdr_t *hdr) +{ + return octets_in_rtp_header + 4 * hdr->cc; +} + +static srtp_hdr_xtnd_t *srtp_get_rtp_xtn_hdr(srtp_hdr_t *hdr) +{ + uint32_t rtp_xtn_hdr_start = srtp_get_rtp_hdr_len(hdr); + return (srtp_hdr_xtnd_t *)((uint8_t *)hdr + rtp_xtn_hdr_start); +} + +static uint32_t srtp_get_rtp_xtn_hdr_len(const srtp_hdr_xtnd_t *xtn_hdr) +{ + return (ntohs(xtn_hdr->length) + 1) * 4; +} static srtp_err_status_t srtp_validate_rtp_header(const void *rtp_hdr, - int pkt_octet_len) + uint32_t pkt_octet_len) { const srtp_hdr_t *hdr = (const srtp_hdr_t *)rtp_hdr; - int rtp_header_len; + uint32_t rtp_header_len; if (pkt_octet_len < octets_in_rtp_header) return srtp_err_status_bad_param; /* Check RTP header length */ - rtp_header_len = octets_in_rtp_header + 4 * hdr->cc; - if (hdr->x == 1) - rtp_header_len += octets_in_rtp_extn_hdr; - + rtp_header_len = srtp_get_rtp_hdr_len(hdr); if (pkt_octet_len < rtp_header_len) return srtp_err_status_bad_param; - /* Verifing profile length. */ + /* Verifying profile length. */ if (hdr->x == 1) { - const srtp_hdr_xtnd_t *xtn_hdr = - (const srtp_hdr_xtnd_t *)((const uint32_t *)hdr + - uint32s_in_rtp_header + hdr->cc); - int profile_len = ntohs(xtn_hdr->length); - rtp_header_len += profile_len * 4; - /* profile length counts the number of 32-bit words */ + if (pkt_octet_len < rtp_header_len + octets_in_rtp_xtn_hdr) + return srtp_err_status_bad_param; + + rtp_header_len += srtp_get_rtp_xtn_hdr_len( + (const srtp_hdr_xtnd_t *)((const uint8_t *)hdr + rtp_header_len)); if (pkt_octet_len < rtp_header_len) return srtp_err_status_bad_param; } + return srtp_err_status_ok; } @@ -1464,7 +1475,7 @@ static srtp_err_status_t srtp_process_header_encryption( srtp_err_status_t status; uint8_t keystream[257]; /* Maximum 2 bytes header + 255 bytes data. */ int keystream_pos; - uint8_t *xtn_hdr_data = ((uint8_t *)xtn_hdr) + octets_in_rtp_extn_hdr; + uint8_t *xtn_hdr_data = ((uint8_t *)xtn_hdr) + octets_in_rtp_xtn_hdr; uint8_t *xtn_hdr_end = xtn_hdr_data + (ntohs(xtn_hdr->length) * sizeof(uint32_t)); @@ -1769,7 +1780,7 @@ static srtp_err_status_t srtp_protect_aead(srtp_ctx_t *ctx, unsigned int use_mki) { srtp_hdr_t *hdr = (srtp_hdr_t *)rtp_hdr; - uint32_t *enc_start; /* pointer to start of encrypted portion */ + uint8_t *enc_start; /* pointer to start of encrypted portion */ int enc_octet_len = 0; /* number of octets in encrypted portion */ srtp_xtd_seq_num_t est; /* estimated xtd_seq_num_t of *hdr */ int delta; /* delta of local pkt idx and that in hdr */ @@ -1809,16 +1820,15 @@ static srtp_err_status_t srtp_protect_aead(srtp_ctx_t *ctx, * extension, if present; otherwise, it starts after the last csrc, * if any are present */ - enc_start = (uint32_t *)hdr + uint32s_in_rtp_header + hdr->cc; + enc_start = (uint8_t *)hdr + srtp_get_rtp_hdr_len(hdr); if (hdr->x == 1) { - xtn_hdr = (srtp_hdr_xtnd_t *)enc_start; - enc_start += (ntohs(xtn_hdr->length) + 1); + xtn_hdr = srtp_get_rtp_xtn_hdr(hdr); + enc_start += srtp_get_rtp_xtn_hdr_len(xtn_hdr); } /* note: the passed size is without the auth tag */ - if (!((uint8_t *)enc_start <= (uint8_t *)hdr + *pkt_octet_len)) + if (!(enc_start <= (uint8_t *)hdr + *pkt_octet_len)) return srtp_err_status_parse_err; - enc_octet_len = - (int)(*pkt_octet_len - ((uint8_t *)enc_start - (uint8_t *)hdr)); + enc_octet_len = (int)(*pkt_octet_len - (enc_start - (uint8_t *)hdr)); if (enc_octet_len < 0) return srtp_err_status_parse_err; @@ -1891,7 +1901,7 @@ static srtp_err_status_t srtp_protect_aead(srtp_ctx_t *ctx, /* * Set the AAD over the RTP header */ - aad_len = (uint32_t)((uint8_t *)enc_start - (uint8_t *)hdr); + aad_len = (uint32_t)(enc_start - (uint8_t *)hdr); status = srtp_cipher_set_aad(session_keys->rtp_cipher, (uint8_t *)hdr, aad_len); if (status) { @@ -1899,7 +1909,7 @@ static srtp_err_status_t srtp_protect_aead(srtp_ctx_t *ctx, } /* Encrypt the payload */ - status = srtp_cipher_encrypt(session_keys->rtp_cipher, (uint8_t *)enc_start, + status = srtp_cipher_encrypt(session_keys->rtp_cipher, enc_start, (unsigned int *)&enc_octet_len); if (status) { return srtp_err_status_cipher_fail; @@ -1908,9 +1918,8 @@ static srtp_err_status_t srtp_protect_aead(srtp_ctx_t *ctx, * If we're doing GCM, we need to get the tag * and append that to the output */ - status = - srtp_cipher_get_tag(session_keys->rtp_cipher, - (uint8_t *)enc_start + enc_octet_len, &tag_len); + status = srtp_cipher_get_tag(session_keys->rtp_cipher, + enc_start + enc_octet_len, &tag_len); if (status) { return (srtp_err_status_cipher_fail); } @@ -1945,7 +1954,7 @@ static srtp_err_status_t srtp_unprotect_aead(srtp_ctx_t *ctx, int advance_packet_index) { srtp_hdr_t *hdr = (srtp_hdr_t *)srtp_hdr; - uint32_t *enc_start; /* pointer to start of encrypted portion */ + uint8_t *enc_start; /* pointer to start of encrypted portion */ unsigned int enc_octet_len = 0; /* number of octets in encrypted portion */ v128_t iv; srtp_err_status_t status; @@ -1993,19 +2002,18 @@ static srtp_err_status_t srtp_unprotect_aead(srtp_ctx_t *ctx, * extension, if present; otherwise, it starts after the last csrc, * if any are present */ - enc_start = (uint32_t *)hdr + uint32s_in_rtp_header + hdr->cc; + enc_start = (uint8_t *)hdr + srtp_get_rtp_hdr_len(hdr); if (hdr->x == 1) { - xtn_hdr = (srtp_hdr_xtnd_t *)enc_start; - enc_start += (ntohs(xtn_hdr->length) + 1); + xtn_hdr = srtp_get_rtp_xtn_hdr(hdr); + enc_start += srtp_get_rtp_xtn_hdr_len(xtn_hdr); } - if (!((uint8_t *)enc_start <= - (uint8_t *)hdr + (*pkt_octet_len - tag_len - mki_size))) + if (!(enc_start <= (uint8_t *)hdr + (*pkt_octet_len - tag_len - mki_size))) return srtp_err_status_parse_err; /* * We pass the tag down to the cipher when doing GCM mode */ enc_octet_len = (unsigned int)(*pkt_octet_len - mki_size - - ((uint8_t *)enc_start - (uint8_t *)hdr)); + (enc_start - (uint8_t *)hdr)); /* * Sanity check the encrypted payload length against @@ -2037,7 +2045,7 @@ static srtp_err_status_t srtp_unprotect_aead(srtp_ctx_t *ctx, /* * Set the AAD for AES-GCM, which is the RTP header */ - aad_len = (uint32_t)((uint8_t *)enc_start - (uint8_t *)hdr); + aad_len = (uint32_t)(enc_start - (uint8_t *)hdr); status = srtp_cipher_set_aad(session_keys->rtp_cipher, (uint8_t *)hdr, aad_len); if (status) { @@ -2149,8 +2157,8 @@ srtp_err_status_t srtp_protect_mki(srtp_ctx_t *ctx, unsigned int mki_index) { srtp_hdr_t *hdr = (srtp_hdr_t *)rtp_hdr; - uint32_t *enc_start; /* pointer to start of encrypted portion */ - uint32_t *auth_start; /* pointer to start of auth. portion */ + uint8_t *enc_start; /* pointer to start of encrypted portion */ + uint8_t *auth_start; /* pointer to start of auth. portion */ int enc_octet_len = 0; /* number of octets in encrypted portion */ srtp_xtd_seq_num_t est; /* estimated xtd_seq_num_t of *hdr */ int delta; /* delta of local pkt idx and that in hdr */ @@ -2166,8 +2174,6 @@ srtp_err_status_t srtp_protect_mki(srtp_ctx_t *ctx, debug_print0(mod_srtp, "function srtp_protect"); - /* we assume the hdr is 32-bit aligned to start */ - /* Verify RTP header */ status = srtp_validate_rtp_header(rtp_hdr, *pkt_octet_len); if (status) @@ -2275,16 +2281,15 @@ srtp_err_status_t srtp_protect_mki(srtp_ctx_t *ctx, * if we're not providing confidentiality, set enc_start to NULL */ if (stream->rtp_services & sec_serv_conf) { - enc_start = (uint32_t *)hdr + uint32s_in_rtp_header + hdr->cc; + enc_start = (uint8_t *)hdr + srtp_get_rtp_hdr_len(hdr); if (hdr->x == 1) { - xtn_hdr = (srtp_hdr_xtnd_t *)enc_start; - enc_start += (ntohs(xtn_hdr->length) + 1); + xtn_hdr = srtp_get_rtp_xtn_hdr(hdr); + enc_start += srtp_get_rtp_xtn_hdr_len(xtn_hdr); } /* note: the passed size is without the auth tag */ - if (!((uint8_t *)enc_start <= (uint8_t *)hdr + *pkt_octet_len)) + if (!(enc_start <= (uint8_t *)hdr + *pkt_octet_len)) return srtp_err_status_parse_err; - enc_octet_len = - (int)(*pkt_octet_len - ((uint8_t *)enc_start - (uint8_t *)hdr)); + enc_octet_len = (int)(*pkt_octet_len - (enc_start - (uint8_t *)hdr)); if (enc_octet_len < 0) return srtp_err_status_parse_err; } else { @@ -2300,7 +2305,7 @@ srtp_err_status_t srtp_protect_mki(srtp_ctx_t *ctx, * to indicate that no authentication is needed */ if (stream->rtp_services & sec_serv_auth) { - auth_start = (uint32_t *)hdr; + auth_start = (uint8_t *)hdr; auth_tag = (uint8_t *)hdr + *pkt_octet_len + mki_size; } else { auth_start = NULL; @@ -2417,9 +2422,8 @@ srtp_err_status_t srtp_protect_mki(srtp_ctx_t *ctx, /* if we're encrypting, exor keystream into the message */ if (enc_start) { - status = - srtp_cipher_encrypt(session_keys->rtp_cipher, (uint8_t *)enc_start, - (unsigned int *)&enc_octet_len); + status = srtp_cipher_encrypt(session_keys->rtp_cipher, enc_start, + (unsigned int *)&enc_octet_len); if (status) return srtp_err_status_cipher_fail; } @@ -2435,7 +2439,7 @@ srtp_err_status_t srtp_protect_mki(srtp_ctx_t *ctx, return status; /* run auth func over packet */ - status = srtp_auth_update(session_keys->rtp_auth, (uint8_t *)auth_start, + status = srtp_auth_update(session_keys->rtp_auth, auth_start, *pkt_octet_len); if (status) return status; @@ -2476,8 +2480,8 @@ srtp_err_status_t srtp_unprotect_mki(srtp_ctx_t *ctx, unsigned int use_mki) { srtp_hdr_t *hdr = (srtp_hdr_t *)srtp_hdr; - uint32_t *enc_start; /* pointer to start of encrypted portion */ - uint32_t *auth_start; /* pointer to start of auth. portion */ + uint8_t *enc_start; /* pointer to start of encrypted portion */ + uint8_t *auth_start; /* pointer to start of auth. portion */ unsigned int enc_octet_len = 0; /* number of octets in encrypted portion */ uint8_t *auth_tag = NULL; /* location of auth_tag within packet */ srtp_xtd_seq_num_t est; /* estimated xtd_seq_num_t of *hdr */ @@ -2496,8 +2500,6 @@ srtp_err_status_t srtp_unprotect_mki(srtp_ctx_t *ctx, debug_print0(mod_srtp, "function srtp_unprotect"); - /* we assume the hdr is 32-bit aligned to start */ - /* Verify RTP header */ status = srtp_validate_rtp_header(srtp_hdr, *pkt_octet_len); if (status) @@ -2650,16 +2652,16 @@ srtp_err_status_t srtp_unprotect_mki(srtp_ctx_t *ctx, * if we're not providing confidentiality, set enc_start to NULL */ if (stream->rtp_services & sec_serv_conf) { - enc_start = (uint32_t *)hdr + uint32s_in_rtp_header + hdr->cc; + enc_start = (uint8_t *)hdr + srtp_get_rtp_hdr_len(hdr); if (hdr->x == 1) { - xtn_hdr = (srtp_hdr_xtnd_t *)enc_start; - enc_start += (ntohs(xtn_hdr->length) + 1); + xtn_hdr = srtp_get_rtp_xtn_hdr(hdr); + enc_start += srtp_get_rtp_xtn_hdr_len(xtn_hdr); } - if (!((uint8_t *)enc_start <= + if (!(enc_start <= (uint8_t *)hdr + (*pkt_octet_len - tag_len - mki_size))) return srtp_err_status_parse_err; enc_octet_len = (uint32_t)(*pkt_octet_len - tag_len - mki_size - - ((uint8_t *)enc_start - (uint8_t *)hdr)); + (enc_start - (uint8_t *)hdr)); } else { enc_start = NULL; } @@ -2670,7 +2672,7 @@ srtp_err_status_t srtp_unprotect_mki(srtp_ctx_t *ctx, * to indicate that no authentication is needed */ if (stream->rtp_services & sec_serv_auth) { - auth_start = (uint32_t *)hdr; + auth_start = (uint8_t *)hdr; auth_tag = (uint8_t *)hdr + *pkt_octet_len - tag_len; } else { auth_start = NULL; @@ -2705,7 +2707,7 @@ srtp_err_status_t srtp_unprotect_mki(srtp_ctx_t *ctx, return status; /* now compute auth function over packet */ - status = srtp_auth_update(session_keys->rtp_auth, (uint8_t *)auth_start, + status = srtp_auth_update(session_keys->rtp_auth, auth_start, *pkt_octet_len - tag_len - mki_size); if (status) return status; @@ -2753,8 +2755,8 @@ srtp_err_status_t srtp_unprotect_mki(srtp_ctx_t *ctx, /* if we're decrypting, add keystream into ciphertext */ if (enc_start) { - status = srtp_cipher_decrypt(session_keys->rtp_cipher, - (uint8_t *)enc_start, &enc_octet_len); + status = srtp_cipher_decrypt(session_keys->rtp_cipher, enc_start, + &enc_octet_len); if (status) return srtp_err_status_cipher_fail; } @@ -3623,8 +3625,8 @@ static srtp_err_status_t srtp_protect_rtcp_aead( unsigned int use_mki) { srtcp_hdr_t *hdr = (srtcp_hdr_t *)rtcp_hdr; - uint32_t *enc_start; /* pointer to start of encrypted portion */ - uint32_t *trailer_p; /* pointer to start of trailer */ + uint8_t *enc_start; /* pointer to start of encrypted portion */ + uint8_t *trailer_p; /* pointer to start of trailer */ uint32_t trailer; /* trailer value */ unsigned int enc_octet_len = 0; /* number of octets in encrypted portion */ uint8_t *auth_tag = NULL; /* location of auth_tag within packet */ @@ -3642,13 +3644,13 @@ static srtp_err_status_t srtp_protect_rtcp_aead( * set encryption start and encryption length - if we're not * providing confidentiality, set enc_start to NULL */ - enc_start = (uint32_t *)hdr + uint32s_in_rtcp_header; + enc_start = (uint8_t *)hdr + octets_in_rtcp_header; enc_octet_len = *pkt_octet_len - octets_in_rtcp_header; /* NOTE: hdr->length is not usable - it refers to only the first * RTCP report in the compound packet! */ - trailer_p = (uint32_t *)((char *)enc_start + enc_octet_len + tag_len); + trailer_p = enc_start + enc_octet_len + tag_len; if (stream->rtcp_services & sec_serv_conf) { trailer = htonl(SRTCP_E_BIT); /* set encrypt bit */ @@ -3735,16 +3737,16 @@ static srtp_err_status_t srtp_protect_rtcp_aead( /* if we're encrypting, exor keystream into the message */ if (enc_start) { - status = srtp_cipher_encrypt(session_keys->rtcp_cipher, - (uint8_t *)enc_start, &enc_octet_len); + status = srtp_cipher_encrypt(session_keys->rtcp_cipher, enc_start, + &enc_octet_len); if (status) { return srtp_err_status_cipher_fail; } /* * Get the tag and append that to the output */ - status = srtp_cipher_get_tag(session_keys->rtcp_cipher, - (uint8_t *)auth_tag, &tag_len); + status = + srtp_cipher_get_tag(session_keys->rtcp_cipher, auth_tag, &tag_len); if (status) { return (srtp_err_status_cipher_fail); } @@ -3762,8 +3764,8 @@ static srtp_err_status_t srtp_protect_rtcp_aead( /* * Get the tag and append that to the output */ - status = srtp_cipher_get_tag(session_keys->rtcp_cipher, - (uint8_t *)auth_tag, &tag_len); + status = + srtp_cipher_get_tag(session_keys->rtcp_cipher, auth_tag, &tag_len); if (status) { return (srtp_err_status_cipher_fail); } @@ -3794,8 +3796,8 @@ static srtp_err_status_t srtp_unprotect_rtcp_aead( unsigned int use_mki) { srtcp_hdr_t *hdr = (srtcp_hdr_t *)srtcp_hdr; - uint32_t *enc_start; /* pointer to start of encrypted portion */ - uint32_t *trailer_p; /* pointer to start of trailer */ + uint8_t *enc_start; /* pointer to start of encrypted portion */ + uint8_t *trailer_p; /* pointer to start of trailer */ uint32_t trailer; /* trailer value */ unsigned int enc_octet_len = 0; /* number of octets in encrypted portion */ uint8_t *auth_tag = NULL; /* location of auth_tag within packet */ @@ -3822,8 +3824,8 @@ static srtp_err_status_t srtp_unprotect_rtcp_aead( */ /* This should point trailer to the word past the end of the normal data. */ /* This would need to be modified for optional mikey data */ - trailer_p = (uint32_t *)((char *)hdr + *pkt_octet_len - - sizeof(srtcp_trailer_t) - mki_size); + trailer_p = + (uint8_t *)hdr + *pkt_octet_len - sizeof(srtcp_trailer_t) - mki_size; memcpy(&trailer, trailer_p, sizeof(trailer)); /* @@ -3835,7 +3837,7 @@ static srtp_err_status_t srtp_unprotect_rtcp_aead( sizeof(srtcp_trailer_t); if (*((unsigned char *)trailer_p) & SRTCP_E_BYTE_BIT) { - enc_start = (uint32_t *)hdr + uint32s_in_rtcp_header; + enc_start = (uint8_t *)hdr + octets_in_rtcp_header; } else { enc_octet_len = 0; enc_start = NULL; /* this indicates that there's no encryption */ @@ -3905,8 +3907,8 @@ static srtp_err_status_t srtp_unprotect_rtcp_aead( /* if we're decrypting, exor keystream into the message */ if (enc_start) { - status = srtp_cipher_decrypt(session_keys->rtcp_cipher, - (uint8_t *)enc_start, &enc_octet_len); + status = srtp_cipher_decrypt(session_keys->rtcp_cipher, enc_start, + &enc_octet_len); if (status) { return status; } @@ -3915,8 +3917,8 @@ static srtp_err_status_t srtp_unprotect_rtcp_aead( * Still need to run the cipher to check the tag */ tmp_len = tag_len; - status = srtp_cipher_decrypt(session_keys->rtcp_cipher, - (uint8_t *)auth_tag, &tmp_len); + status = + srtp_cipher_decrypt(session_keys->rtcp_cipher, auth_tag, &tmp_len); if (status) { return status; } @@ -3995,9 +3997,9 @@ srtp_err_status_t srtp_protect_rtcp_mki(srtp_t ctx, unsigned int mki_index) { srtcp_hdr_t *hdr = (srtcp_hdr_t *)rtcp_hdr; - uint32_t *enc_start; /* pointer to start of encrypted portion */ - uint32_t *auth_start; /* pointer to start of auth. portion */ - uint32_t *trailer_p; /* pointer to start of trailer */ + uint8_t *enc_start; /* pointer to start of encrypted portion */ + uint8_t *auth_start; /* pointer to start of auth. portion */ + uint8_t *trailer_p; /* pointer to start of trailer */ uint32_t trailer; /* trailer value */ unsigned int enc_octet_len = 0; /* number of octets in encrypted portion */ uint8_t *auth_tag = NULL; /* location of auth_tag within packet */ @@ -4009,8 +4011,6 @@ srtp_err_status_t srtp_protect_rtcp_mki(srtp_t ctx, unsigned int mki_size = 0; srtp_session_keys_t *session_keys = NULL; - /* we assume the hdr is 32-bit aligned to start */ - /* check the packet length - it must at least contain a full header */ if (*pkt_octet_len < octets_in_rtcp_header) return srtp_err_status_bad_param; @@ -4086,7 +4086,7 @@ srtp_err_status_t srtp_protect_rtcp_mki(srtp_t ctx, * set encryption start and encryption length - if we're not * providing confidentiality, set enc_start to NULL */ - enc_start = (uint32_t *)hdr + uint32s_in_rtcp_header; + enc_start = (uint8_t *)hdr + octets_in_rtcp_header; enc_octet_len = *pkt_octet_len - octets_in_rtcp_header; /* all of the packet, except the header, gets encrypted */ @@ -4094,7 +4094,7 @@ srtp_err_status_t srtp_protect_rtcp_mki(srtp_t ctx, * NOTE: hdr->length is not usable - it refers to only the first RTCP report * in the compound packet! */ - trailer_p = (uint32_t *)((char *)enc_start + enc_octet_len); + trailer_p = enc_start + enc_octet_len; if (stream->rtcp_services & sec_serv_conf) { trailer = htonl(SRTCP_E_BIT); /* set encrypt bit */ @@ -4114,7 +4114,7 @@ srtp_err_status_t srtp_protect_rtcp_mki(srtp_t ctx, * (note that srtpc *always* provides authentication, unlike srtp) */ /* Note: This would need to change for optional mikey data */ - auth_start = (uint32_t *)hdr; + auth_start = (uint8_t *)hdr; auth_tag = (uint8_t *)hdr + *pkt_octet_len + sizeof(srtcp_trailer_t) + mki_size; @@ -4181,8 +4181,8 @@ srtp_err_status_t srtp_protect_rtcp_mki(srtp_t ctx, /* if we're encrypting, exor keystream into the message */ if (enc_start) { - status = srtp_cipher_encrypt(session_keys->rtcp_cipher, - (uint8_t *)enc_start, &enc_octet_len); + status = srtp_cipher_encrypt(session_keys->rtcp_cipher, enc_start, + &enc_octet_len); if (status) return srtp_err_status_cipher_fail; } @@ -4197,7 +4197,7 @@ srtp_err_status_t srtp_protect_rtcp_mki(srtp_t ctx, * result at auth_tag */ status = - srtp_auth_compute(session_keys->rtcp_auth, (uint8_t *)auth_start, + srtp_auth_compute(session_keys->rtcp_auth, auth_start, (*pkt_octet_len) + sizeof(srtcp_trailer_t), auth_tag); debug_print(mod_srtp, "srtcp auth tag: %s", srtp_octet_string_hex_string(auth_tag, tag_len)); @@ -4226,9 +4226,9 @@ srtp_err_status_t srtp_unprotect_rtcp_mki(srtp_t ctx, unsigned int use_mki) { srtcp_hdr_t *hdr = (srtcp_hdr_t *)srtcp_hdr; - uint32_t *enc_start; /* pointer to start of encrypted portion */ - uint32_t *auth_start; /* pointer to start of auth. portion */ - uint32_t *trailer_p; /* pointer to start of trailer */ + uint8_t *enc_start; /* pointer to start of encrypted portion */ + uint8_t *auth_start; /* pointer to start of auth. portion */ + uint8_t *trailer_p; /* pointer to start of trailer */ uint32_t trailer; /* trailer value */ unsigned int enc_octet_len = 0; /* number of octets in encrypted portion */ uint8_t *auth_tag = NULL; /* location of auth_tag within packet */ @@ -4244,8 +4244,6 @@ srtp_err_status_t srtp_unprotect_rtcp_mki(srtp_t ctx, unsigned int mki_size = 0; srtp_session_keys_t *session_keys = NULL; - /* we assume the hdr is 32-bit aligned to start */ - if (*pkt_octet_len < 0) return srtp_err_status_bad_param; @@ -4283,9 +4281,8 @@ 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(stream, (const uint8_t *)hdr, - (unsigned int)*pkt_octet_len, &mki_size); + session_keys = srtp_get_session_keys( + stream, (uint8_t *)hdr, (unsigned int)*pkt_octet_len, &mki_size); if (session_keys == NULL) return srtp_err_status_bad_mki; @@ -4329,17 +4326,16 @@ srtp_err_status_t srtp_unprotect_rtcp_mki(srtp_t ctx, */ /* This should point trailer to the word past the end of the normal data. */ /* This would need to be modified for optional mikey data */ - trailer_p = (uint32_t *)((char *)hdr + *pkt_octet_len - - (tag_len + mki_size + sizeof(srtcp_trailer_t))); + trailer_p = (uint8_t *)hdr + *pkt_octet_len - + (tag_len + mki_size + sizeof(srtcp_trailer_t)); memcpy(&trailer, trailer_p, sizeof(trailer)); - e_bit_in_packet = - (*((unsigned char *)trailer_p) & SRTCP_E_BYTE_BIT) == SRTCP_E_BYTE_BIT; + e_bit_in_packet = (*(trailer_p)&SRTCP_E_BYTE_BIT) == SRTCP_E_BYTE_BIT; if (e_bit_in_packet != sec_serv_confidentiality) { return srtp_err_status_cant_check; } if (sec_serv_confidentiality) { - enc_start = (uint32_t *)hdr + uint32s_in_rtcp_header; + enc_start = (uint8_t *)hdr + octets_in_rtcp_header; } else { enc_octet_len = 0; enc_start = NULL; /* this indicates that there's no encryption */ @@ -4349,7 +4345,7 @@ srtp_err_status_t srtp_unprotect_rtcp_mki(srtp_t ctx, * set the auth_start and auth_tag pointers to the proper locations * (note that srtcp *always* uses authentication, unlike srtp) */ - auth_start = (uint32_t *)hdr; + auth_start = (uint8_t *)hdr; /* * The location of the auth tag in the packet needs to know MKI @@ -4404,8 +4400,8 @@ srtp_err_status_t srtp_unprotect_rtcp_mki(srtp_t ctx, return status; /* run auth func over packet, put result into tmp_tag */ - status = srtp_auth_compute(session_keys->rtcp_auth, (uint8_t *)auth_start, - auth_len, tmp_tag); + status = srtp_auth_compute(session_keys->rtcp_auth, auth_start, auth_len, + tmp_tag); debug_print(mod_srtp, "srtcp computed tag: %s", srtp_octet_string_hex_string(tmp_tag, tag_len)); if (status) @@ -4433,8 +4429,8 @@ srtp_err_status_t srtp_unprotect_rtcp_mki(srtp_t ctx, /* if we're decrypting, exor keystream into the message */ if (enc_start) { - status = srtp_cipher_decrypt(session_keys->rtcp_cipher, - (uint8_t *)enc_start, &enc_octet_len); + status = srtp_cipher_decrypt(session_keys->rtcp_cipher, enc_start, + &enc_octet_len); if (status) return srtp_err_status_cipher_fail; } From d30461e907dd58077192384cefe4b1892d0733c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Wed, 20 Dec 2023 08:24:16 +0100 Subject: [PATCH 2/2] document xtn helper functions srtp_get_rtp_xtn_hdr & srtp_get_rtp_xtn_hdr_len both assume that the caller has verified that the input arguments are valid, ie that there is an extension present by previously checking the x bit of the RTP header. --- srtp/srtp.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/srtp/srtp.c b/srtp/srtp.c index dddf6b7e7..82f4a8303 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -82,12 +82,24 @@ static uint32_t srtp_get_rtp_hdr_len(const srtp_hdr_t *hdr) return octets_in_rtp_header + 4 * hdr->cc; } +/* + * Returns the location of the header extention cast too a srtp_hdr_xtnd_t + * struct. Will always return a value and assumes that the caller has already + * verified that a header extension is present by checking the x bit of + * srtp_hdr_t. + */ static srtp_hdr_xtnd_t *srtp_get_rtp_xtn_hdr(srtp_hdr_t *hdr) { uint32_t rtp_xtn_hdr_start = srtp_get_rtp_hdr_len(hdr); return (srtp_hdr_xtnd_t *)((uint8_t *)hdr + rtp_xtn_hdr_start); } +/* + * Returns the length of the extension header including the extension header + * header so will return a minium of 4. Assumes the srtp_hdr_xtnd_t is a valid + * pointer and that the caller has already verified that a header extension is + * valid by checking the x bit of the RTP header. + */ static uint32_t srtp_get_rtp_xtn_hdr_len(const srtp_hdr_xtnd_t *xtn_hdr) { return (ntohs(xtn_hdr->length) + 1) * 4;