Skip to content

Commit d1170a9

Browse files
committed
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.
1 parent d922f4a commit d1170a9

4 files changed

Lines changed: 218 additions & 45 deletions

File tree

include/srtp.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,8 @@ typedef enum {
219219
/**< needed */
220220
srtp_err_status_buffer_small = 28, /**< out buffer is too small */
221221
srtp_err_status_cryptex_err = 29, /**< unsupported cryptex operation */
222+
srtp_err_status_direction_mismatch =
223+
30, /**< operation does not match stream direction */
222224
} srtp_err_status_t;
223225

224226
typedef struct srtp_ctx_t_ srtp_ctx_t;
@@ -637,6 +639,7 @@ srtp_err_status_t srtp_shutdown(void);
637639
* - srtp_err_status_replay_fail rtp sequence number was non-increasing
638640
* - srtp_err_status_buffer_small the srtp buffer is too small for the SRTP
639641
* packet
642+
* - srtp_err_status_direction_mismatch the stream is not a sender stream
640643
* - @e other failure in cryptographic mechanisms
641644
*/
642645
srtp_err_status_t srtp_protect(srtp_t ctx,
@@ -688,6 +691,8 @@ srtp_err_status_t srtp_protect(srtp_t ctx,
688691
* - srtp_err_status_replay_fail if the SRTP packet is a replay (e.g. packet
689692
* has already been processed and accepted).
690693
* - srtp_err_status_bad_mki if the MKI in the packet is not a known MKI id
694+
* - srtp_err_status_direction_mismatch if the stream is not a receiver
695+
* stream
691696
* - [other] if there has been an error in the cryptographic mechanisms.
692697
*
693698
*/
@@ -911,6 +916,7 @@ void srtp_append_salt_to_key(uint8_t *key,
911916
* - srtp_err_status_ok if there were no problems.
912917
* - srtp_err_status_buffer_small the srtcp buffer is too small for the
913918
* SRTCP packet
919+
* - srtp_err_status_direction_mismatch the stream is not a sender stream
914920
* - [other] if there was a failure in
915921
* the cryptographic mechanisms.
916922
*/
@@ -961,6 +967,8 @@ srtp_err_status_t srtp_protect_rtcp(srtp_t ctx,
961967
* already been processed and accepted).
962968
* - srtp_err_status_bad_mki if the MKI in the packet is not a known MKI
963969
* id
970+
* - srtp_err_status_direction_mismatch if the stream is not a receiver
971+
* stream
964972
* - [other] if there has been an error in the cryptographic mechanisms.
965973
*
966974
*/

srtp/srtp.c

Lines changed: 33 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2406,21 +2406,12 @@ static srtp_err_status_t srtp_unprotect_aead(srtp_ctx_t *ctx,
24062406
}
24072407

24082408
/*
2409-
* verify that stream is for received traffic - this check will
2410-
* detect SSRC collisions, since a stream that appears in both
2411-
* srtp_protect() and srtp_unprotect() will fail this test in one of
2412-
* those functions.
2413-
*
24142409
* we do this check *after* the authentication check, so that the
24152410
* latter check will catch any attempts to fool us into thinking
24162411
* that we've got a collision
24172412
*/
2418-
if (stream->direction != dir_srtp_receiver) {
2419-
if (stream->direction == dir_unknown) {
2420-
stream->direction = dir_srtp_receiver;
2421-
} else {
2422-
srtp_handle_event(ctx, stream, event_ssrc_collision);
2423-
}
2413+
if (stream->direction == dir_unknown) {
2414+
stream->direction = dir_srtp_receiver;
24242415
}
24252416

24262417
/*
@@ -2560,6 +2551,7 @@ srtp_err_status_t srtp_protect(srtp_t ctx,
25602551
stream->direction = dir_srtp_sender;
25612552
} else {
25622553
srtp_handle_event(ctx, stream, event_ssrc_collision);
2554+
return srtp_err_status_direction_mismatch;
25632555
}
25642556
}
25652557

@@ -2873,6 +2865,18 @@ srtp_err_status_t srtp_unprotect(srtp_t ctx,
28732865
return srtp_err_status_no_ctx;
28742866
}
28752867
} else {
2868+
/*
2869+
* Verify that stream is for received traffic - this check will
2870+
* detect SSRC collisions, since a stream that appears in both
2871+
* srtp_protect() and srtp_unprotect() will fail this test in one of
2872+
* those functions.
2873+
*
2874+
*/
2875+
if (stream->direction == dir_srtp_sender) {
2876+
srtp_handle_event(ctx, stream, event_ssrc_collision);
2877+
return srtp_err_status_direction_mismatch;
2878+
}
2879+
28762880
status = srtp_get_est_pkt_index(hdr, stream, &est, &delta);
28772881

28782882
if (status && (status != srtp_err_status_pkt_idx_adv)) {
@@ -3098,21 +3102,12 @@ srtp_err_status_t srtp_unprotect(srtp_t ctx,
30983102
}
30993103

31003104
/*
3101-
* verify that stream is for received traffic - this check will
3102-
* detect SSRC collisions, since a stream that appears in both
3103-
* srtp_protect() and srtp_unprotect() will fail this test in one of
3104-
* those functions.
3105-
*
31063105
* we do this check *after* the authentication check, so that the
31073106
* latter check will catch any attempts to fool us into thinking
31083107
* that we've got a collision
31093108
*/
3110-
if (stream->direction != dir_srtp_receiver) {
3111-
if (stream->direction == dir_unknown) {
3112-
stream->direction = dir_srtp_receiver;
3113-
} else {
3114-
srtp_handle_event(ctx, stream, event_ssrc_collision);
3115-
}
3109+
if (stream->direction == dir_unknown) {
3110+
stream->direction = dir_srtp_receiver;
31163111
}
31173112

31183113
/*
@@ -4009,21 +4004,12 @@ static srtp_err_status_t srtp_unprotect_rtcp_aead(
40094004
*rtcp_len -= (tag_len + sizeof(srtcp_trailer_t) + stream->mki_size);
40104005

40114006
/*
4012-
* verify that stream is for received traffic - this check will
4013-
* detect SSRC collisions, since a stream that appears in both
4014-
* srtp_protect() and srtp_unprotect() will fail this test in one of
4015-
* those functions.
4016-
*
40174007
* we do this check *after* the authentication check, so that the
40184008
* latter check will catch any attempts to fool us into thinking
40194009
* that we've got a collision
40204010
*/
4021-
if (stream->direction != dir_srtp_receiver) {
4022-
if (stream->direction == dir_unknown) {
4023-
stream->direction = dir_srtp_receiver;
4024-
} else {
4025-
srtp_handle_event(ctx, stream, event_ssrc_collision);
4026-
}
4011+
if (stream->direction == dir_unknown) {
4012+
stream->direction = dir_srtp_receiver;
40274013
}
40284014

40294015
/*
@@ -4139,6 +4125,7 @@ srtp_err_status_t srtp_protect_rtcp(srtp_t ctx,
41394125
stream->direction = dir_srtp_sender;
41404126
} else {
41414127
srtp_handle_event(ctx, stream, event_ssrc_collision);
4128+
return srtp_err_status_direction_mismatch;
41424129
}
41434130
}
41444131

@@ -4368,6 +4355,17 @@ srtp_err_status_t srtp_unprotect_rtcp(srtp_t ctx,
43684355
}
43694356
}
43704357

4358+
/*
4359+
* verify that stream is for received traffic - this check will
4360+
* detect SSRC collisions, since a stream that appears in both
4361+
* srtp_protect() and srtp_unprotect() will fail this test in one of
4362+
* those functions.
4363+
*/
4364+
if (stream->direction == dir_srtp_sender) {
4365+
srtp_handle_event(ctx, stream, event_ssrc_collision);
4366+
return srtp_err_status_direction_mismatch;
4367+
}
4368+
43714369
/*
43724370
* Determine if MKI is being used and what session keys should be used
43734371
*/
@@ -4547,21 +4545,12 @@ srtp_err_status_t srtp_unprotect_rtcp(srtp_t ctx,
45474545
*rtcp_len -= stream->mki_size;
45484546

45494547
/*
4550-
* verify that stream is for received traffic - this check will
4551-
* detect SSRC collisions, since a stream that appears in both
4552-
* srtp_protect() and srtp_unprotect() will fail this test in one of
4553-
* those functions.
4554-
*
45554548
* we do this check *after* the authentication check, so that the
45564549
* latter check will catch any attempts to fool us into thinking
45574550
* that we've got a collision
45584551
*/
4559-
if (stream->direction != dir_srtp_receiver) {
4560-
if (stream->direction == dir_unknown) {
4561-
stream->direction = dir_srtp_receiver;
4562-
} else {
4563-
srtp_handle_event(ctx, stream, event_ssrc_collision);
4564-
}
4552+
if (stream->direction == dir_unknown) {
4553+
stream->direction = dir_srtp_receiver;
45654554
}
45664555

45674556
/*

0 commit comments

Comments
 (0)