Skip to content

Respect the direction set in a template stream #618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bifurcation wants to merge 5 commits into
base: main
Choose a base branch
from
+565 −45
Open

Respect the direction set in a template stream #618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4155f4d
make test to verify direction is adhered too
pabuhler May 11, 2026
581f2c4
Respect the direction set in a template stream
bifurcation Nov 21, 2022
d922f4a
Also respect template direction for RTCP
pabuhler May 14, 2026
d1170a9
enforce directional stream operations
pabuhler May 14, 2026
c2fcc91
Preserve stream direction across srtp_update
pabuhler May 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions include/srtp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
*
*/
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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.
*
*/
Expand Down
127 changes: 83 additions & 44 deletions srtp/srtp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/*
Expand Down Expand Up @@ -2517,6 +2508,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 */
Expand Down Expand Up @@ -2556,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;
}
}

Expand Down Expand Up @@ -2847,6 +2843,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));
Expand All @@ -2865,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)) {
Expand Down Expand Up @@ -3090,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;
}

/*
Expand Down Expand Up @@ -3478,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)
{
Expand All @@ -3494,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) {
Expand All @@ -3506,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);
Expand Down Expand Up @@ -3546,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));
Expand All @@ -3561,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) {
Expand All @@ -3580,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;
}
Expand Down Expand Up @@ -4001,21 +4038,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;
}

/*
Expand Down Expand Up @@ -4092,6 +4120,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 */
Expand Down Expand Up @@ -4127,6 +4159,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;
}
}

Expand Down Expand Up @@ -4341,6 +4374,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,
Expand All @@ -4352,6 +4389,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
*/
Expand Down Expand Up @@ -4531,21 +4579,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;
}

/*
Expand Down
Loading
Loading