From 4155f4da78789d69e7a527dc8c24e6e464768ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Tue, 12 May 2026 00:20:04 +0200 Subject: [PATCH 1/5] make test to verify direction is adhered too This is a failing test for #618 --- test/srtp_driver.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/test/srtp_driver.c b/test/srtp_driver.c index 1e354013d..0af1f1699 100644 --- a/test/srtp_driver.c +++ b/test/srtp_driver.c @@ -115,6 +115,10 @@ srtp_err_status_t srtp_test_remove_stream(void); srtp_err_status_t srtp_test_update(void); +srtp_err_status_t srtp_test_template_inbound_direction(void); + +srtp_err_status_t srtp_test_template_outbound_direction(void); + srtp_err_status_t srtp_test_update_mki(void); srtp_err_status_t srtp_test_protect_trailer_length(void); @@ -862,6 +866,22 @@ int main(int argc, char *argv[]) exit(1); } + printf("testing inbound wildcard template stream direction checks..."); + if (srtp_test_template_inbound_direction() == srtp_err_status_ok) { + printf("passed\n"); + } else { + printf("failed\n"); + exit(1); + } + + printf("testing outbound wildcard template stream direction checks..."); + if (srtp_test_template_outbound_direction() == srtp_err_status_ok) { + printf("passed\n"); + } else { + printf("failed\n"); + exit(1); + } + /* * test the function srtp_update() */ @@ -4846,6 +4866,71 @@ srtp_err_status_t srtp_test_update(void) return srtp_err_status_ok; } +srtp_err_status_t srtp_test_template_inbound_direction(void) +{ + const uint32_t ssrc = 0x12121212; + srtp_policy_t policy; + srtp_t inbound_session; + uint8_t *rtp; + size_t rtp_len; + + CHECK_OK(srtp_policy_create(&policy)); + CHECK_OK(srtp_policy_set_profile(policy, srtp_profile_aes128_cm_sha1_80)); + CHECK_OK(policy_set_key(policy, test_key)); + + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_inbound, 0 })); + CHECK_OK(srtp_create(&inbound_session, policy)); + + rtp = create_rtp_test_packet(32, ssrc, 1, 1, false, &rtp_len, NULL); + if (rtp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_RETURN(call_srtp_protect(inbound_session, rtp, &rtp_len, 0), + srtp_err_status_no_ctx); + free(rtp); + + CHECK_OK(srtp_dealloc(inbound_session)); + srtp_policy_destroy(policy); + + return srtp_err_status_ok; +} + +srtp_err_status_t srtp_test_template_outbound_direction(void) +{ + const uint32_t ssrc = 0x12121212; + srtp_policy_t policy; + srtp_t outbound_session; + srtp_t sender_session; + uint8_t *rtp; + size_t rtp_len; + + CHECK_OK(srtp_policy_create(&policy)); + CHECK_OK(srtp_policy_set_profile(policy, srtp_profile_aes128_cm_sha1_80)); + CHECK_OK(policy_set_key(policy, test_key)); + + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_outbound, 0 })); + CHECK_OK(srtp_create(&sender_session, policy)); + CHECK_OK(srtp_create(&outbound_session, policy)); + + rtp = create_rtp_test_packet(32, ssrc, 2, 1, false, &rtp_len, NULL); + if (rtp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect(sender_session, rtp, &rtp_len, 0)); + + CHECK_RETURN(call_srtp_unprotect(outbound_session, rtp, &rtp_len), + srtp_err_status_no_ctx); + free(rtp); + + CHECK_OK(srtp_dealloc(sender_session)); + CHECK_OK(srtp_dealloc(outbound_session)); + srtp_policy_destroy(policy); + + return srtp_err_status_ok; +} + srtp_err_status_t srtp_test_update_mki(void) { srtp_err_status_t status; From 581f2c4743e316e62ead99e016d433ba0f0480b0 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Mon, 21 Nov 2022 11:46:50 -0500 Subject: [PATCH 2/5] Respect the direction set in a template stream --- srtp/srtp.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/srtp/srtp.c b/srtp/srtp.c index 3d14d3364..5866ad298 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -2517,6 +2517,10 @@ srtp_err_status_t srtp_protect(srtp_t ctx, stream = srtp_get_stream(ctx, hdr->ssrc); if (stream == NULL) { if (ctx->stream_template != NULL) { + if (ctx->stream_template->direction == dir_srtp_receiver) { + return srtp_err_status_no_ctx; + } + srtp_stream_ctx_t *new_stream; /* allocate and initialize a new stream */ @@ -2847,6 +2851,10 @@ srtp_err_status_t srtp_unprotect(srtp_t ctx, stream = srtp_get_stream(ctx, hdr->ssrc); if (stream == NULL) { if (ctx->stream_template != NULL) { + if (ctx->stream_template->direction == dir_srtp_sender) { + return srtp_err_status_no_ctx; + } + stream = ctx->stream_template; debug_print(mod_srtp, "using provisional stream (SSRC: 0x%08x)", (unsigned int)ntohl(hdr->ssrc)); From d922f4ae776e55eb430315c66110a3824ab0fc17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Thu, 14 May 2026 09:29:08 +0200 Subject: [PATCH 3/5] Also respect template direction for RTCP --- srtp/srtp.c | 8 ++++++++ test/srtp_driver.c | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/srtp/srtp.c b/srtp/srtp.c index 5866ad298..0d475c78f 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -4100,6 +4100,10 @@ srtp_err_status_t srtp_protect_rtcp(srtp_t ctx, stream = srtp_get_stream(ctx, hdr->ssrc); if (stream == NULL) { if (ctx->stream_template != NULL) { + if (ctx->stream_template->direction == dir_srtp_receiver) { + return srtp_err_status_no_ctx; + } + srtp_stream_ctx_t *new_stream; /* allocate and initialize a new stream */ @@ -4349,6 +4353,10 @@ srtp_err_status_t srtp_unprotect_rtcp(srtp_t ctx, stream = srtp_get_stream(ctx, hdr->ssrc); if (stream == NULL) { if (ctx->stream_template != NULL) { + if (ctx->stream_template->direction == dir_srtp_sender) { + return srtp_err_status_no_ctx; + } + stream = ctx->stream_template; debug_print(mod_srtp, diff --git a/test/srtp_driver.c b/test/srtp_driver.c index 0af1f1699..fda909c25 100644 --- a/test/srtp_driver.c +++ b/test/srtp_driver.c @@ -4872,7 +4872,9 @@ srtp_err_status_t srtp_test_template_inbound_direction(void) srtp_policy_t policy; srtp_t inbound_session; uint8_t *rtp; + uint8_t *rtcp; size_t rtp_len; + size_t rtcp_len; CHECK_OK(srtp_policy_create(&policy)); CHECK_OK(srtp_policy_set_profile(policy, srtp_profile_aes128_cm_sha1_80)); @@ -4890,6 +4892,14 @@ srtp_err_status_t srtp_test_template_inbound_direction(void) srtp_err_status_no_ctx); free(rtp); + rtcp = create_rtcp_test_packet(32, ssrc, &rtcp_len, NULL); + if (rtcp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_RETURN(call_srtp_protect_rtcp(inbound_session, rtcp, &rtcp_len, 0), + srtp_err_status_no_ctx); + free(rtcp); + CHECK_OK(srtp_dealloc(inbound_session)); srtp_policy_destroy(policy); @@ -4903,7 +4913,9 @@ srtp_err_status_t srtp_test_template_outbound_direction(void) srtp_t outbound_session; srtp_t sender_session; uint8_t *rtp; + uint8_t *rtcp; size_t rtp_len; + size_t rtcp_len; CHECK_OK(srtp_policy_create(&policy)); CHECK_OK(srtp_policy_set_profile(policy, srtp_profile_aes128_cm_sha1_80)); @@ -4924,6 +4936,16 @@ srtp_err_status_t srtp_test_template_outbound_direction(void) srtp_err_status_no_ctx); free(rtp); + rtcp = create_rtcp_test_packet(32, ssrc, &rtcp_len, NULL); + if (rtcp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect_rtcp(sender_session, rtcp, &rtcp_len, 0)); + + CHECK_RETURN(call_srtp_unprotect_rtcp(outbound_session, rtcp, &rtcp_len), + srtp_err_status_no_ctx); + free(rtcp); + CHECK_OK(srtp_dealloc(sender_session)); CHECK_OK(srtp_dealloc(outbound_session)); srtp_policy_destroy(policy); From d1170a956fe789ddee1b13a657beafdf7e3eda9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Thu, 14 May 2026 16:51:55 +0200 Subject: [PATCH 4/5] enforce directional stream operations A stream can no be used for both protecting and unprotecting. Treat it as an error as well as rasing the collision event. --- include/srtp.h | 8 +++ srtp/srtp.c | 77 +++++++++----------- test/srtp_driver.c | 175 +++++++++++++++++++++++++++++++++++++++++++++ test/util.c | 3 +- 4 files changed, 218 insertions(+), 45 deletions(-) diff --git a/include/srtp.h b/include/srtp.h index 8e0ab1fca..2faf8aaba 100644 --- a/include/srtp.h +++ b/include/srtp.h @@ -219,6 +219,8 @@ typedef enum { /**< needed */ srtp_err_status_buffer_small = 28, /**< out buffer is too small */ srtp_err_status_cryptex_err = 29, /**< unsupported cryptex operation */ + srtp_err_status_direction_mismatch = + 30, /**< operation does not match stream direction */ } srtp_err_status_t; typedef struct srtp_ctx_t_ srtp_ctx_t; @@ -637,6 +639,7 @@ srtp_err_status_t srtp_shutdown(void); * - srtp_err_status_replay_fail rtp sequence number was non-increasing * - srtp_err_status_buffer_small the srtp buffer is too small for the SRTP * packet + * - srtp_err_status_direction_mismatch the stream is not a sender stream * - @e other failure in cryptographic mechanisms */ srtp_err_status_t srtp_protect(srtp_t ctx, @@ -688,6 +691,8 @@ srtp_err_status_t srtp_protect(srtp_t ctx, * - srtp_err_status_replay_fail if the SRTP packet is a replay (e.g. packet * has already been processed and accepted). * - srtp_err_status_bad_mki if the MKI in the packet is not a known MKI id + * - srtp_err_status_direction_mismatch if the stream is not a receiver + * stream * - [other] if there has been an error in the cryptographic mechanisms. * */ @@ -911,6 +916,7 @@ void srtp_append_salt_to_key(uint8_t *key, * - srtp_err_status_ok if there were no problems. * - srtp_err_status_buffer_small the srtcp buffer is too small for the * SRTCP packet + * - srtp_err_status_direction_mismatch the stream is not a sender stream * - [other] if there was a failure in * the cryptographic mechanisms. */ @@ -961,6 +967,8 @@ srtp_err_status_t srtp_protect_rtcp(srtp_t ctx, * already been processed and accepted). * - srtp_err_status_bad_mki if the MKI in the packet is not a known MKI * id + * - srtp_err_status_direction_mismatch if the stream is not a receiver + * stream * - [other] if there has been an error in the cryptographic mechanisms. * */ diff --git a/srtp/srtp.c b/srtp/srtp.c index 0d475c78f..0bddd307a 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -2406,21 +2406,12 @@ static srtp_err_status_t srtp_unprotect_aead(srtp_ctx_t *ctx, } /* - * verify that stream is for received traffic - this check will - * detect SSRC collisions, since a stream that appears in both - * srtp_protect() and srtp_unprotect() will fail this test in one of - * those functions. - * * we do this check *after* the authentication check, so that the * latter check will catch any attempts to fool us into thinking * that we've got a collision */ - if (stream->direction != dir_srtp_receiver) { - if (stream->direction == dir_unknown) { - stream->direction = dir_srtp_receiver; - } else { - srtp_handle_event(ctx, stream, event_ssrc_collision); - } + if (stream->direction == dir_unknown) { + stream->direction = dir_srtp_receiver; } /* @@ -2560,6 +2551,7 @@ srtp_err_status_t srtp_protect(srtp_t ctx, stream->direction = dir_srtp_sender; } else { srtp_handle_event(ctx, stream, event_ssrc_collision); + return srtp_err_status_direction_mismatch; } } @@ -2873,6 +2865,18 @@ srtp_err_status_t srtp_unprotect(srtp_t ctx, return srtp_err_status_no_ctx; } } else { + /* + * Verify that stream is for received traffic - this check will + * detect SSRC collisions, since a stream that appears in both + * srtp_protect() and srtp_unprotect() will fail this test in one of + * those functions. + * + */ + if (stream->direction == dir_srtp_sender) { + srtp_handle_event(ctx, stream, event_ssrc_collision); + return srtp_err_status_direction_mismatch; + } + status = srtp_get_est_pkt_index(hdr, stream, &est, &delta); if (status && (status != srtp_err_status_pkt_idx_adv)) { @@ -3098,21 +3102,12 @@ srtp_err_status_t srtp_unprotect(srtp_t ctx, } /* - * verify that stream is for received traffic - this check will - * detect SSRC collisions, since a stream that appears in both - * srtp_protect() and srtp_unprotect() will fail this test in one of - * those functions. - * * we do this check *after* the authentication check, so that the * latter check will catch any attempts to fool us into thinking * that we've got a collision */ - if (stream->direction != dir_srtp_receiver) { - if (stream->direction == dir_unknown) { - stream->direction = dir_srtp_receiver; - } else { - srtp_handle_event(ctx, stream, event_ssrc_collision); - } + if (stream->direction == dir_unknown) { + stream->direction = dir_srtp_receiver; } /* @@ -4009,21 +4004,12 @@ static srtp_err_status_t srtp_unprotect_rtcp_aead( *rtcp_len -= (tag_len + sizeof(srtcp_trailer_t) + stream->mki_size); /* - * verify that stream is for received traffic - this check will - * detect SSRC collisions, since a stream that appears in both - * srtp_protect() and srtp_unprotect() will fail this test in one of - * those functions. - * * we do this check *after* the authentication check, so that the * latter check will catch any attempts to fool us into thinking * that we've got a collision */ - if (stream->direction != dir_srtp_receiver) { - if (stream->direction == dir_unknown) { - stream->direction = dir_srtp_receiver; - } else { - srtp_handle_event(ctx, stream, event_ssrc_collision); - } + if (stream->direction == dir_unknown) { + stream->direction = dir_srtp_receiver; } /* @@ -4139,6 +4125,7 @@ srtp_err_status_t srtp_protect_rtcp(srtp_t ctx, stream->direction = dir_srtp_sender; } else { srtp_handle_event(ctx, stream, event_ssrc_collision); + return srtp_err_status_direction_mismatch; } } @@ -4368,6 +4355,17 @@ srtp_err_status_t srtp_unprotect_rtcp(srtp_t ctx, } } + /* + * verify that stream is for received traffic - this check will + * detect SSRC collisions, since a stream that appears in both + * srtp_protect() and srtp_unprotect() will fail this test in one of + * those functions. + */ + if (stream->direction == dir_srtp_sender) { + srtp_handle_event(ctx, stream, event_ssrc_collision); + return srtp_err_status_direction_mismatch; + } + /* * Determine if MKI is being used and what session keys should be used */ @@ -4547,21 +4545,12 @@ srtp_err_status_t srtp_unprotect_rtcp(srtp_t ctx, *rtcp_len -= stream->mki_size; /* - * verify that stream is for received traffic - this check will - * detect SSRC collisions, since a stream that appears in both - * srtp_protect() and srtp_unprotect() will fail this test in one of - * those functions. - * * we do this check *after* the authentication check, so that the * latter check will catch any attempts to fool us into thinking * that we've got a collision */ - if (stream->direction != dir_srtp_receiver) { - if (stream->direction == dir_unknown) { - stream->direction = dir_srtp_receiver; - } else { - srtp_handle_event(ctx, stream, event_ssrc_collision); - } + if (stream->direction == dir_unknown) { + stream->direction = dir_srtp_receiver; } /* diff --git a/test/srtp_driver.c b/test/srtp_driver.c index fda909c25..7e9641e41 100644 --- a/test/srtp_driver.c +++ b/test/srtp_driver.c @@ -115,6 +115,10 @@ srtp_err_status_t srtp_test_remove_stream(void); srtp_err_status_t srtp_test_update(void); +srtp_err_status_t srtp_test_inbound_direction(void); + +srtp_err_status_t srtp_test_outbound_direction(void); + srtp_err_status_t srtp_test_template_inbound_direction(void); srtp_err_status_t srtp_test_template_outbound_direction(void); @@ -866,6 +870,22 @@ int main(int argc, char *argv[]) exit(1); } + printf("testing inbound stream direction checks..."); + if (srtp_test_inbound_direction() == srtp_err_status_ok) { + printf("passed\n"); + } else { + printf("failed\n"); + exit(1); + } + + printf("testing outbound stream direction checks..."); + if (srtp_test_outbound_direction() == srtp_err_status_ok) { + printf("passed\n"); + } else { + printf("failed\n"); + exit(1); + } + printf("testing inbound wildcard template stream direction checks..."); if (srtp_test_template_inbound_direction() == srtp_err_status_ok) { printf("passed\n"); @@ -4866,11 +4886,123 @@ srtp_err_status_t srtp_test_update(void) return srtp_err_status_ok; } +srtp_err_status_t srtp_test_inbound_direction(void) +{ + const uint32_t ssrc = 0x12121212; + srtp_policy_t policy; + srtp_t inbound_session; + srtp_t sender_session; + uint8_t *rtp; + uint8_t *rtcp; + size_t rtp_len; + size_t rtcp_len; + + CHECK_OK(srtp_policy_create(&policy)); + CHECK_OK(srtp_policy_set_profile(policy, srtp_profile_aes128_cm_sha1_80)); + CHECK_OK(policy_set_key(policy, test_key)); + + CHECK_OK(srtp_policy_set_ssrc( + policy, (srtp_ssrc_t){ ssrc_any_outbound, 0x12121212 })); + CHECK_OK(srtp_create(&sender_session, policy)); + + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_specific, ssrc })); + CHECK_OK(srtp_create(&inbound_session, policy)); + + rtp = create_rtp_test_packet(32, ssrc, 1, 1, false, &rtp_len, NULL); + if (rtp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect(sender_session, rtp, &rtp_len, 0)); + CHECK_OK(call_srtp_unprotect(inbound_session, rtp, &rtp_len)); + CHECK_RETURN(call_srtp_protect(inbound_session, rtp, &rtp_len, 0), + srtp_err_status_direction_mismatch); + free(rtp); + + rtcp = create_rtcp_test_packet(32, ssrc, &rtcp_len, NULL); + if (rtcp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect_rtcp(sender_session, rtcp, &rtcp_len, 0)); + CHECK_OK(call_srtp_unprotect_rtcp(inbound_session, rtcp, &rtcp_len)); + CHECK_RETURN(call_srtp_protect_rtcp(inbound_session, rtcp, &rtcp_len, 0), + srtp_err_status_direction_mismatch); + free(rtcp); + + CHECK_OK(srtp_dealloc(sender_session)); + CHECK_OK(srtp_dealloc(inbound_session)); + srtp_policy_destroy(policy); + + return srtp_err_status_ok; +} + +srtp_err_status_t srtp_test_outbound_direction(void) +{ + const uint32_t ssrc = 0x12121212; + srtp_policy_t policy; + srtp_t outbound_session; + srtp_t sender_session; + uint8_t *rtp; + uint8_t *rtcp; + size_t rtp_len; + size_t rtcp_len; + + CHECK_OK(srtp_policy_create(&policy)); + CHECK_OK(srtp_policy_set_profile(policy, srtp_profile_aes128_cm_sha1_80)); + CHECK_OK(policy_set_key(policy, test_key)); + + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_outbound, 0 })); + CHECK_OK(srtp_create(&sender_session, policy)); + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_specific, ssrc })); + CHECK_OK(srtp_create(&outbound_session, policy)); + + rtp = create_rtp_test_packet(32, ssrc, 2, 1, false, &rtp_len, NULL); + if (rtp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect(outbound_session, rtp, &rtp_len, 0)); + free(rtp); + + rtcp = create_rtcp_test_packet(32, ssrc, &rtcp_len, NULL); + if (rtcp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect_rtcp(outbound_session, rtcp, &rtcp_len, 0)); + free(rtcp); + + rtp = create_rtp_test_packet(32, ssrc, 3, 1, false, &rtp_len, NULL); + if (rtp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect(sender_session, rtp, &rtp_len, 0)); + CHECK_RETURN(call_srtp_unprotect(outbound_session, rtp, &rtp_len), + srtp_err_status_direction_mismatch); + free(rtp); + + rtcp = create_rtcp_test_packet(32, ssrc, &rtcp_len, NULL); + if (rtcp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect_rtcp(sender_session, rtcp, &rtcp_len, 0)); + CHECK_RETURN(call_srtp_unprotect_rtcp(outbound_session, rtcp, &rtcp_len), + srtp_err_status_direction_mismatch); + free(rtcp); + + CHECK_OK(srtp_dealloc(sender_session)); + CHECK_OK(srtp_dealloc(outbound_session)); + srtp_policy_destroy(policy); + + return srtp_err_status_ok; +} + srtp_err_status_t srtp_test_template_inbound_direction(void) { const uint32_t ssrc = 0x12121212; srtp_policy_t policy; srtp_t inbound_session; + srtp_t sender_session; uint8_t *rtp; uint8_t *rtcp; size_t rtp_len; @@ -4880,6 +5012,10 @@ srtp_err_status_t srtp_test_template_inbound_direction(void) CHECK_OK(srtp_policy_set_profile(policy, srtp_profile_aes128_cm_sha1_80)); CHECK_OK(policy_set_key(policy, test_key)); + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_outbound, 0 })); + CHECK_OK(srtp_create(&sender_session, policy)); + CHECK_OK( srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_inbound, 0 })); CHECK_OK(srtp_create(&inbound_session, policy)); @@ -4900,6 +5036,27 @@ srtp_err_status_t srtp_test_template_inbound_direction(void) srtp_err_status_no_ctx); free(rtcp); + rtp = create_rtp_test_packet(32, ssrc, 2, 1, false, &rtp_len, NULL); + if (rtp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect(sender_session, rtp, &rtp_len, 0)); + CHECK_OK(call_srtp_unprotect(inbound_session, rtp, &rtp_len)); + CHECK_RETURN(call_srtp_protect(inbound_session, rtp, &rtp_len, 0), + srtp_err_status_direction_mismatch); + free(rtp); + + rtcp = create_rtcp_test_packet(32, ssrc, &rtcp_len, NULL); + if (rtcp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect_rtcp(sender_session, rtcp, &rtcp_len, 0)); + CHECK_OK(call_srtp_unprotect_rtcp(inbound_session, rtcp, &rtcp_len)); + CHECK_RETURN(call_srtp_protect_rtcp(inbound_session, rtcp, &rtcp_len, 0), + srtp_err_status_direction_mismatch); + free(rtcp); + + CHECK_OK(srtp_dealloc(sender_session)); CHECK_OK(srtp_dealloc(inbound_session)); srtp_policy_destroy(policy); @@ -4946,6 +5103,24 @@ srtp_err_status_t srtp_test_template_outbound_direction(void) srtp_err_status_no_ctx); free(rtcp); + rtp = create_rtp_test_packet(32, ssrc, 3, 1, false, &rtp_len, NULL); + if (rtp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect(outbound_session, rtp, &rtp_len, 0)); + CHECK_RETURN(call_srtp_unprotect(outbound_session, rtp, &rtp_len), + srtp_err_status_direction_mismatch); + free(rtp); + + rtcp = create_rtcp_test_packet(32, ssrc, &rtcp_len, NULL); + if (rtcp == NULL) { + return srtp_err_status_alloc_fail; + } + CHECK_OK(call_srtp_protect_rtcp(outbound_session, rtcp, &rtcp_len, 0)); + CHECK_RETURN(call_srtp_unprotect_rtcp(outbound_session, rtcp, &rtcp_len), + srtp_err_status_direction_mismatch); + free(rtcp); + CHECK_OK(srtp_dealloc(sender_session)); CHECK_OK(srtp_dealloc(outbound_session)); srtp_policy_destroy(policy); diff --git a/test/util.c b/test/util.c index 51abf6a25..280622e44 100644 --- a/test/util.c +++ b/test/util.c @@ -92,8 +92,9 @@ const char *err_status_string(srtp_err_status_t status) ERR_STATUS_STRING(pkt_idx_adv); ERR_STATUS_STRING(buffer_small); ERR_STATUS_STRING(cryptex_err); + ERR_STATUS_STRING(direction_mismatch); } - return "unkown srtp_err_status"; + return "unknown srtp_err_status"; } void check_ok_impl(srtp_err_status_t status, const char *file, int line) From c2fcc91f0c3942e1eecf6eb34cfda20484878111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Thu, 14 May 2026 19:00:28 +0200 Subject: [PATCH 5/5] Preserve stream direction across srtp_update Keep existing stream directions unchanged when srtp_update rebuilds specific streams, and reject wildcard template updates that attempt to flip inbound/outbound direction. Add regression coverage for specific and template-derived streams to verify direction preservation and template direction mismatch rejection. --- srtp/srtp.c | 34 ++++++++ test/srtp_driver.c | 190 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 224 insertions(+) diff --git a/srtp/srtp.c b/srtp/srtp.c index 0bddd307a..58fefc686 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -3481,6 +3481,30 @@ static srtp_err_status_t is_update_policy_compatable(srtp_stream_t stream, return srtp_err_status_ok; } +static srtp_err_status_t is_update_template_direction_compatible( + srtp_stream_t stream_template, + const srtp_policy_t policy) +{ + switch (policy->ssrc.type) { + case (ssrc_any_outbound): + if (stream_template->direction != dir_srtp_sender) { + return srtp_err_status_bad_param; + } + break; + case (ssrc_any_inbound): + if (stream_template->direction != dir_srtp_receiver) { + return srtp_err_status_bad_param; + } + break; + case (ssrc_specific): + case (ssrc_undefined): + default: + return srtp_err_status_bad_param; + } + + return srtp_err_status_ok; +} + static srtp_err_status_t update_template_streams(srtp_t session, const srtp_policy_t policy) { @@ -3497,6 +3521,12 @@ static srtp_err_status_t update_template_streams(srtp_t session, return status; } + status = is_update_template_direction_compatible(session->stream_template, + policy); + if (status != srtp_err_status_ok) { + return status; + } + /* allocate new template stream */ status = srtp_stream_alloc(&new_stream_template, policy); if (status) { @@ -3509,6 +3539,7 @@ static srtp_err_status_t update_template_streams(srtp_t session, srtp_crypto_free(new_stream_template); return status; } + new_stream_template->direction = session->stream_template->direction; /* allocate new stream list */ status = srtp_stream_list_alloc(&new_stream_list); @@ -3549,6 +3580,7 @@ static srtp_err_status_t stream_update(srtp_t session, srtp_err_status_t status; srtp_xtd_seq_num_t old_index; srtp_rdb_t old_rtcp_rdb; + direction_t old_direction; srtp_stream_t stream; stream = srtp_get_stream(session, htonl(policy->ssrc.value)); @@ -3564,6 +3596,7 @@ static srtp_err_status_t stream_update(srtp_t session, /* save old extendard seq */ old_index = stream->rtp_rdbx.index; old_rtcp_rdb = stream->rtcp_rdb; + old_direction = stream->direction; status = srtp_stream_remove(session, policy->ssrc.value); if (status) { @@ -3583,6 +3616,7 @@ static srtp_err_status_t stream_update(srtp_t session, /* restore old extended seq */ stream->rtp_rdbx.index = old_index; stream->rtcp_rdb = old_rtcp_rdb; + stream->direction = old_direction; return srtp_err_status_ok; } diff --git a/test/srtp_driver.c b/test/srtp_driver.c index 7e9641e41..5cd65cffd 100644 --- a/test/srtp_driver.c +++ b/test/srtp_driver.c @@ -115,6 +115,8 @@ srtp_err_status_t srtp_test_remove_stream(void); srtp_err_status_t srtp_test_update(void); +srtp_err_status_t srtp_test_update_preserves_direction(void); + srtp_err_status_t srtp_test_inbound_direction(void); srtp_err_status_t srtp_test_outbound_direction(void); @@ -870,6 +872,14 @@ int main(int argc, char *argv[]) exit(1); } + printf("testing srtp_update() stream direction preservation..."); + if (srtp_test_update_preserves_direction() == srtp_err_status_ok) { + printf("passed\n"); + } else { + printf("failed\n"); + exit(1); + } + printf("testing inbound stream direction checks..."); if (srtp_test_inbound_direction() == srtp_err_status_ok) { printf("passed\n"); @@ -4886,6 +4896,186 @@ srtp_err_status_t srtp_test_update(void) return srtp_err_status_ok; } +static srtp_err_status_t create_direction_update_policy(srtp_policy_t *policy) +{ + CHECK_OK(srtp_policy_create(policy)); + CHECK_OK(srtp_policy_set_profile(*policy, srtp_profile_aes128_cm_sha1_80)); + CHECK_OK(policy_set_key(*policy, test_key)); + + return srtp_err_status_ok; +} + +static srtp_err_status_t check_stream_direction(srtp_t session, + uint32_t ssrc, + direction_t expected_direction) +{ + srtp_stream_t stream = srtp_get_stream(session, htonl(ssrc)); + CHECK(stream != NULL); + CHECK(stream->direction == expected_direction); + + return srtp_err_status_ok; +} + +static srtp_err_status_t protect_one_rtp(srtp_t session, + uint32_t ssrc, + uint16_t seq) +{ + size_t rtp_len; + uint8_t *rtp = + create_rtp_test_packet(32, ssrc, seq, 1, false, &rtp_len, NULL); + if (rtp == NULL) { + return srtp_err_status_alloc_fail; + } + + CHECK_OK(call_srtp_protect(session, rtp, &rtp_len, 0)); + free(rtp); + + return srtp_err_status_ok; +} + +static srtp_err_status_t protect_unprotect_one_rtp(srtp_t sender, + srtp_t receiver, + uint32_t ssrc, + uint16_t seq) +{ + size_t rtp_len; + uint8_t *rtp = + create_rtp_test_packet(32, ssrc, seq, 1, false, &rtp_len, NULL); + if (rtp == NULL) { + return srtp_err_status_alloc_fail; + } + + CHECK_OK(call_srtp_protect(sender, rtp, &rtp_len, 0)); + CHECK_OK(call_srtp_unprotect(receiver, rtp, &rtp_len)); + free(rtp); + + return srtp_err_status_ok; +} + +static srtp_err_status_t srtp_test_update_preserves_specific_inbound_direction( + void) +{ + const uint32_t ssrc = 0x12121212; + srtp_policy_t policy; + srtp_t session; + srtp_t sender_session; + + CHECK_OK(create_direction_update_policy(&policy)); + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_outbound, 0 })); + CHECK_OK(srtp_create(&sender_session, policy)); + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_specific, ssrc })); + CHECK_OK(srtp_create(&session, policy)); + + CHECK_OK(protect_unprotect_one_rtp(sender_session, session, ssrc, 1)); + CHECK_OK(check_stream_direction(session, ssrc, dir_srtp_receiver)); + CHECK_OK(srtp_update(session, policy)); + CHECK_OK(check_stream_direction(session, ssrc, dir_srtp_receiver)); + + CHECK_OK(srtp_dealloc(session)); + CHECK_OK(srtp_dealloc(sender_session)); + srtp_policy_destroy(policy); + + return srtp_err_status_ok; +} + +static srtp_err_status_t srtp_test_update_preserves_specific_outbound_direction( + void) +{ + const uint32_t ssrc = 0x12121212; + srtp_policy_t policy; + srtp_t session; + + CHECK_OK(create_direction_update_policy(&policy)); + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_specific, ssrc })); + CHECK_OK(srtp_create(&session, policy)); + + CHECK_OK(protect_one_rtp(session, ssrc, 2)); + CHECK_OK(check_stream_direction(session, ssrc, dir_srtp_sender)); + CHECK_OK(srtp_update(session, policy)); + CHECK_OK(check_stream_direction(session, ssrc, dir_srtp_sender)); + + CHECK_OK(srtp_dealloc(session)); + srtp_policy_destroy(policy); + + return srtp_err_status_ok; +} + +static srtp_err_status_t srtp_test_update_preserves_template_inbound_direction( + void) +{ + const uint32_t ssrc = 0x12121212; + srtp_policy_t policy; + srtp_t session; + srtp_t sender_session; + + CHECK_OK(create_direction_update_policy(&policy)); + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_outbound, 0 })); + CHECK_OK(srtp_create(&sender_session, policy)); + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_inbound, 0 })); + CHECK_OK(srtp_create(&session, policy)); + + CHECK_OK(protect_unprotect_one_rtp(sender_session, session, ssrc, 3)); + CHECK_OK(check_stream_direction(session, ssrc, dir_srtp_receiver)); + CHECK_OK(srtp_update(session, policy)); + CHECK(session->stream_template->direction == dir_srtp_receiver); + CHECK_OK(check_stream_direction(session, ssrc, dir_srtp_receiver)); + + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_outbound, 0 })); + CHECK_RETURN(srtp_update(session, policy), srtp_err_status_bad_param); + CHECK(session->stream_template->direction == dir_srtp_receiver); + + CHECK_OK(srtp_dealloc(session)); + CHECK_OK(srtp_dealloc(sender_session)); + srtp_policy_destroy(policy); + + return srtp_err_status_ok; +} + +static srtp_err_status_t srtp_test_update_preserves_template_outbound_direction( + void) +{ + const uint32_t ssrc = 0x12121212; + srtp_policy_t policy; + srtp_t session; + + CHECK_OK(create_direction_update_policy(&policy)); + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_outbound, 0 })); + CHECK_OK(srtp_create(&session, policy)); + + CHECK_OK(protect_one_rtp(session, ssrc, 4)); + CHECK_OK(check_stream_direction(session, ssrc, dir_srtp_sender)); + CHECK_OK(srtp_update(session, policy)); + CHECK(session->stream_template->direction == dir_srtp_sender); + CHECK_OK(check_stream_direction(session, ssrc, dir_srtp_sender)); + + CHECK_OK( + srtp_policy_set_ssrc(policy, (srtp_ssrc_t){ ssrc_any_inbound, 0 })); + CHECK_RETURN(srtp_update(session, policy), srtp_err_status_bad_param); + CHECK(session->stream_template->direction == dir_srtp_sender); + + CHECK_OK(srtp_dealloc(session)); + srtp_policy_destroy(policy); + + return srtp_err_status_ok; +} + +srtp_err_status_t srtp_test_update_preserves_direction(void) +{ + CHECK_OK(srtp_test_update_preserves_specific_inbound_direction()); + CHECK_OK(srtp_test_update_preserves_specific_outbound_direction()); + CHECK_OK(srtp_test_update_preserves_template_inbound_direction()); + CHECK_OK(srtp_test_update_preserves_template_outbound_direction()); + + return srtp_err_status_ok; +} + srtp_err_status_t srtp_test_inbound_direction(void) { const uint32_t ssrc = 0x12121212;