From 70ad9cad8c938cb10bf8262059ce1e59b670b9ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Mon, 8 Jan 2024 23:54:01 +0100 Subject: [PATCH] remove last ekt reference Preserve the invalid policy test so more can be added. Merge srtp_valid_policy and srtp_validate_policy_master_keys functions. --- include/srtp.h | 2 - srtp/srtp.c | 92 +++++++++++++++++++++++----------------------- test/srtp_driver.c | 44 ++++------------------ 3 files changed, 53 insertions(+), 85 deletions(-) diff --git a/include/srtp.h b/include/srtp.h index 79ddfddac..24c54d5f0 100644 --- a/include/srtp.h +++ b/include/srtp.h @@ -337,8 +337,6 @@ typedef struct srtp_policy_t { /**< this stream. */ srtp_master_key_t **keys; /** Array of Master Key structures */ unsigned long num_master_keys; /** Number of master keys */ - void *deprecated_ekt; /**< DEPRECATED: pointer to the EKT */ - /**< policy structure for this stream */ unsigned long window_size; /**< The window size to use for replay */ /**< protection. */ int allow_repeat_tx; /**< Whether retransmissions of */ diff --git a/srtp/srtp.c b/srtp/srtp.c index f1ac1a8c3..fd3424a56 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -344,12 +344,31 @@ static srtp_err_status_t srtp_remove_and_dealloc_streams( return data.status; } -static srtp_err_status_t srtp_valid_policy(const srtp_policy_t *p) +static srtp_err_status_t srtp_valid_policy(const srtp_policy_t *policy) { - if (p != NULL && p->deprecated_ekt != NULL) { + if (policy == NULL) { return srtp_err_status_bad_param; } + if (policy->key == NULL) { + if (policy->num_master_keys <= 0) { + return srtp_err_status_bad_param; + } + + if (policy->num_master_keys > SRTP_MAX_NUM_MASTER_KEYS) { + return srtp_err_status_bad_param; + } + + for (unsigned long i = 0; i < policy->num_master_keys; i++) { + if (policy->keys[i]->key == NULL) { + return srtp_err_status_bad_param; + } + if (policy->keys[i]->mki_size > SRTP_MAX_MKI_LEN) { + return srtp_err_status_bad_param; + } + } + } + return srtp_err_status_ok; } @@ -862,29 +881,6 @@ static inline size_t full_key_length(const srtp_cipher_type_t *cipher) } } -static unsigned int srtp_validate_policy_master_keys( - const srtp_policy_t *policy) -{ - unsigned long i = 0; - - if (policy->key == NULL) { - if (policy->num_master_keys <= 0) - return 0; - - if (policy->num_master_keys > SRTP_MAX_NUM_MASTER_KEYS) - return 0; - - for (i = 0; i < policy->num_master_keys; i++) { - if (policy->keys[i]->key == NULL) - return 0; - if (policy->keys[i]->mki_size > SRTP_MAX_MKI_LEN) - return 0; - } - } - - return 1; -} - srtp_session_keys_t *srtp_get_session_keys_with_mki_index( srtp_stream_ctx_t *stream, unsigned int use_mki, @@ -2919,16 +2915,16 @@ srtp_err_status_t srtp_stream_add(srtp_t session, const srtp_policy_t *policy) srtp_err_status_t status; srtp_stream_t tmp; + /* sanity check arguments */ + if (session == NULL) { + return srtp_err_status_bad_param; + } + status = srtp_valid_policy(policy); if (status != srtp_err_status_ok) { return status; } - /* sanity check arguments */ - if ((session == NULL) || (policy == NULL) || - (!srtp_validate_policy_master_keys(policy))) - return srtp_err_status_bad_param; - /* allocate stream */ status = srtp_stream_alloc(&tmp, policy); if (status) { @@ -2989,14 +2985,17 @@ srtp_err_status_t srtp_create(srtp_t *session, /* handle for session */ srtp_err_status_t stat; srtp_ctx_t *ctx; - stat = srtp_valid_policy(policy); - if (stat != srtp_err_status_ok) { - return stat; - } - /* sanity check arguments */ - if (session == NULL) + if (session == NULL) { return srtp_err_status_bad_param; + } + + if (policy) { + stat = srtp_valid_policy(policy); + if (stat != srtp_err_status_ok) { + return stat; + } + } /* allocate srtp context and set ctx_ptr */ ctx = (srtp_ctx_t *)srtp_crypto_alloc(sizeof(srtp_ctx_t)); @@ -3066,17 +3065,16 @@ srtp_err_status_t srtp_update(srtp_t session, const srtp_policy_t *policy) { srtp_err_status_t stat; + /* sanity check arguments */ + if (session == NULL) { + return srtp_err_status_bad_param; + } + stat = srtp_valid_policy(policy); if (stat != srtp_err_status_ok) { return stat; } - /* sanity check arguments */ - if ((session == NULL) || (policy == NULL) || - (!srtp_validate_policy_master_keys(policy))) { - return srtp_err_status_bad_param; - } - while (policy != NULL) { stat = srtp_stream_update(session, policy); if (stat) { @@ -3258,16 +3256,16 @@ srtp_err_status_t srtp_stream_update(srtp_t session, { srtp_err_status_t status; + /* sanity check arguments */ + if (session == NULL) { + return srtp_err_status_bad_param; + } + status = srtp_valid_policy(policy); if (status != srtp_err_status_ok) { return status; } - /* sanity check arguments */ - if ((session == NULL) || (policy == NULL) || - (!srtp_validate_policy_master_keys(policy))) - return srtp_err_status_bad_param; - switch (policy->ssrc.type) { case (ssrc_any_outbound): case (ssrc_any_inbound): diff --git a/test/srtp_driver.c b/test/srtp_driver.c index 9c4fcd2ed..0ac243b0f 100644 --- a/test/srtp_driver.c +++ b/test/srtp_driver.c @@ -651,7 +651,6 @@ int main(int argc, char *argv[]) policy.ssrc.type = ssrc_specific; policy.ssrc.value = 0xdecafbad; policy.key = test_key; - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.next = NULL; @@ -1806,7 +1805,6 @@ srtp_err_status_t srtp_validate(void) policy.ssrc.type = ssrc_specific; policy.ssrc.value = 0xcafebabe; policy.key = test_key; - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.next = NULL; @@ -1966,7 +1964,6 @@ srtp_err_status_t srtp_validate_null(void) policy.ssrc.type = ssrc_specific; policy.ssrc.value = 0xcafebabe; policy.key = test_key; - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.next = NULL; @@ -2128,7 +2125,6 @@ srtp_err_status_t srtp_validate_gcm(void) policy.ssrc.type = ssrc_specific; policy.ssrc.value = 0xcafebabe; policy.key = test_key_gcm; - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.next = NULL; @@ -2291,7 +2287,6 @@ srtp_err_status_t srtp_validate_encrypted_extensions_headers(void) policy.ssrc.type = ssrc_specific; policy.ssrc.value = 0xcafebabe; policy.key = test_key_ext_headers; - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.enc_xtn_hdr = headers; @@ -2412,7 +2407,6 @@ srtp_err_status_t srtp_validate_encrypted_extensions_headers_gcm(void) policy.ssrc.type = ssrc_specific; policy.ssrc.value = 0xcafebabe; policy.key = test_key_ext_headers; - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.enc_xtn_hdr = headers; @@ -2528,7 +2522,6 @@ srtp_err_status_t srtp_validate_aes_256(void) policy.ssrc.type = ssrc_specific; policy.ssrc.value = 0xcafebabe; policy.key = aes_256_test_key; - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.next = NULL; @@ -2656,7 +2649,6 @@ srtp_err_status_t srtp_test_empty_payload(void) policy.ssrc.type = ssrc_specific; policy.ssrc.value = 0xcafebabe; policy.key = test_key; - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.next = NULL; @@ -2732,7 +2724,6 @@ srtp_err_status_t srtp_test_empty_payload_gcm(void) policy.ssrc.type = ssrc_specific; policy.ssrc.value = 0xcafebabe; policy.key = test_key; - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.next = NULL; @@ -2857,7 +2848,6 @@ srtp_err_status_t srtp_test_remove_stream(void) policy.ssrc.type = ssrc_specific; policy.ssrc.value = 0xcafebabe; policy.key = test_key; - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.next = NULL; @@ -2916,7 +2906,6 @@ srtp_err_status_t srtp_test_update(void) memset(&policy, 0, sizeof(policy)); srtp_crypto_policy_set_rtp_default(&policy.rtp); srtp_crypto_policy_set_rtcp_default(&policy.rtcp); - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.next = NULL; @@ -3083,7 +3072,6 @@ srtp_err_status_t srtp_test_setup_protect_trailer_streams( memset(&policy, 0, sizeof(policy)); srtp_crypto_policy_set_rtp_default(&policy.rtp); srtp_crypto_policy_set_rtcp_default(&policy.rtcp); - policy.deprecated_ekt = NULL; policy.window_size = 128; policy.allow_repeat_tx = 0; policy.next = NULL; @@ -3093,7 +3081,6 @@ srtp_err_status_t srtp_test_setup_protect_trailer_streams( memset(&policy_mki, 0, sizeof(policy_mki)); srtp_crypto_policy_set_rtp_default(&policy_mki.rtp); srtp_crypto_policy_set_rtcp_default(&policy_mki.rtcp); - policy_mki.deprecated_ekt = NULL; policy_mki.window_size = 128; policy_mki.allow_repeat_tx = 0; policy_mki.next = NULL; @@ -3106,7 +3093,6 @@ srtp_err_status_t srtp_test_setup_protect_trailer_streams( memset(&policy_aes_gcm, 0, sizeof(policy_aes_gcm)); srtp_crypto_policy_set_aes_gcm_128_16_auth(&policy_aes_gcm.rtp); srtp_crypto_policy_set_aes_gcm_128_16_auth(&policy_aes_gcm.rtcp); - policy_aes_gcm.deprecated_ekt = NULL; policy_aes_gcm.window_size = 128; policy_aes_gcm.allow_repeat_tx = 0; policy_aes_gcm.next = NULL; @@ -3116,7 +3102,6 @@ srtp_err_status_t srtp_test_setup_protect_trailer_streams( memset(&policy_aes_gcm_mki, 0, sizeof(policy_aes_gcm_mki)); srtp_crypto_policy_set_aes_gcm_128_16_auth(&policy_aes_gcm_mki.rtp); srtp_crypto_policy_set_aes_gcm_128_16_auth(&policy_aes_gcm_mki.rtcp); - policy_aes_gcm_mki.deprecated_ekt = NULL; policy_aes_gcm_mki.window_size = 128; policy_aes_gcm_mki.allow_repeat_tx = 0; policy_aes_gcm_mki.next = NULL; @@ -3989,7 +3974,6 @@ const srtp_policy_t default_policy = { NULL, (srtp_master_key_t **)test_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 */ @@ -4018,7 +4002,6 @@ const srtp_policy_t aes_only_policy = { NULL, (srtp_master_key_t **)test_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 */ @@ -4047,7 +4030,6 @@ const srtp_policy_t hmac_only_policy = { NULL, (srtp_master_key_t **)test_keys, 2, /* Number of Master keys associated with the policy */ - NULL, /* indicates that EKT is not in use */ 128, /* replay window size */ 0, /* retransmission not allowed */ NULL, /* no encrypted extension headers */ @@ -4079,7 +4061,6 @@ const srtp_policy_t aes128_gcm_8_policy = { NULL, (srtp_master_key_t **)test_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 */ @@ -4110,7 +4091,6 @@ const srtp_policy_t aes128_gcm_8_cauth_policy = { NULL, (srtp_master_key_t **)test_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 */ @@ -4141,7 +4121,6 @@ const srtp_policy_t aes256_gcm_8_policy = { NULL, (srtp_master_key_t **)test_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 */ @@ -4172,7 +4151,6 @@ const srtp_policy_t aes256_gcm_8_cauth_policy = { NULL, (srtp_master_key_t **)test_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 */ @@ -4202,7 +4180,6 @@ const srtp_policy_t null_policy = { NULL, (srtp_master_key_t **)test_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 */ @@ -4271,7 +4248,6 @@ const srtp_policy_t aes_256_hmac_policy = { 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 */ @@ -4279,9 +4255,7 @@ const srtp_policy_t aes_256_hmac_policy = { NULL }; -char ekt_test_policy = 'x'; - -const srtp_policy_t hmac_only_with_ekt_policy = { +const srtp_policy_t hmac_only_with_no_master_key = { { ssrc_any_outbound, 0 }, /* SSRC */ { SRTP_NULL_CIPHER, /* cipher type */ @@ -4300,13 +4274,12 @@ const srtp_policy_t hmac_only_with_ekt_policy = { sec_serv_auth /* security services flag */ }, NULL, - (srtp_master_key_t **)test_keys, - 2, /* indicates the number of Master keys */ - &ekt_test_policy, /* requests deprecated EKT functionality */ - 128, /* replay window size */ - 0, /* retransmission not allowed */ - NULL, /* no encrypted extension headers */ - 0, /* list of encrypted extension headers is empty */ + NULL, /* no master keys*/ + 0, /* indicates the number of Master keys */ + 128, /* replay window size */ + 0, /* retransmission not allowed */ + NULL, /* no encrypted extension headers */ + 0, /* list of encrypted extension headers is empty */ NULL }; @@ -4339,7 +4312,7 @@ const srtp_policy_t *policy_array[] = { // clang-format off const srtp_policy_t *invalid_policy_array[] = { - &hmac_only_with_ekt_policy, + &hmac_only_with_no_master_key, NULL }; // clang-format on @@ -4367,7 +4340,6 @@ const srtp_policy_t wildcard_policy = { test_key, NULL, 0, - NULL, 128, /* replay window size */ 0, /* retransmission not allowed */ NULL, /* no encrypted extension headers */