From 9d9d6dee9a89bb98838d8956d8627822dd73afc9 Mon Sep 17 00:00:00 2001 From: Ulf Olsson Date: Mon, 29 May 2017 15:15:28 +0200 Subject: [PATCH 1/2] srtp.c: Save the ROC and sequence number before usage The ROC and the sequence number to set must be calculated before the estimated index is used otherwise the packets following the first one can't be decoded/authenticated Change-Id: Ib2950b37771d39607fdead33d32245fa08fb0ab1 --- srtp/srtp.c | 8 ++++-- test/srtp_driver.c | 70 +++++++++++++++++++++++++++++++++------------- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/srtp/srtp.c b/srtp/srtp.c index 446e4053d..3f8c865eb 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -2346,6 +2346,8 @@ srtp_unprotect_mki(srtp_ctx_t *ctx, void *srtp_hdr, int *pkt_octet_len, unsigned int mki_size = 0; srtp_session_keys_t *session_keys = NULL; int advance_packet_index = 0; + uint32_t roc_to_set; + uint16_t seq_to_set; debug_print(mod_srtp, "function srtp_unprotect", NULL); @@ -2404,6 +2406,8 @@ srtp_unprotect_mki(srtp_ctx_t *ctx, void *srtp_hdr, int *pkt_octet_len, if (status == srtp_err_status_pkt_idx_adv) { advance_packet_index = 1; + roc_to_set = (uint32_t)(est >> 16); + seq_to_set = (uint16_t)(est & 0xFFFF); } /* check replay database */ @@ -2664,8 +2668,8 @@ srtp_unprotect_mki(srtp_ctx_t *ctx, void *srtp_hdr, int *pkt_octet_len, */ if (advance_packet_index) { srtp_rdbx_set_roc_seq(&stream->rtp_rdbx, - (uint32_t)(est >> 16), - (uint16_t)(est & 0xFFFF)); + roc_to_set, + seq_to_set); stream->pending_roc = 0; srtp_rdbx_add_index(&stream->rtp_rdbx, 0); } else { diff --git a/test/srtp_driver.c b/test/srtp_driver.c index 631f6eb44..56052a632 100644 --- a/test/srtp_driver.c +++ b/test/srtp_driver.c @@ -3077,15 +3077,19 @@ test_set_receiver_roc(uint32_t packets, uint32_t roc_to_set) srtp_policy_t receiver_policy; srtp_t receiver_session; - srtp_hdr_t *pkt; - unsigned char *recv_pkt; + srtp_hdr_t *pkt_1; + unsigned char *recv_pkt_1; + + srtp_hdr_t *pkt_2; + unsigned char *recv_pkt_2; uint32_t i; uint32_t ts; uint16_t seq; int msg_len_octets = 32; - int protected_msg_len_octets; + int protected_msg_len_octets_1; + int protected_msg_len_octets_2; /* Create sender */ memset(&sender_policy, 0, sizeof(sender_policy)); @@ -3105,10 +3109,13 @@ test_set_receiver_roc(uint32_t packets, uint32_t roc_to_set) seq = 0; ts = 0; for (i = 0; i < packets; i++) { - pkt = srtp_create_test_packet_extended(msg_len_octets, sender_policy.ssrc.value, seq, ts); - protected_msg_len_octets = msg_len_octets; - status = srtp_protect(sender_session, pkt, &protected_msg_len_octets); - free(pkt); + srtp_hdr_t *tmp_pkt; + int tmp_len; + + tmp_pkt = srtp_create_test_packet_extended(msg_len_octets, sender_policy.ssrc.value, seq, ts); + tmp_len = msg_len_octets; + status = srtp_protect(sender_session, tmp_pkt, &tmp_len); + free(tmp_pkt); if (status) { return status; } @@ -3116,10 +3123,20 @@ test_set_receiver_roc(uint32_t packets, uint32_t roc_to_set) ts++; } - /* Create the packet to decrypt and test for ROC change */ - pkt = srtp_create_test_packet_extended(msg_len_octets, sender_policy.ssrc.value, seq, ts); - protected_msg_len_octets = msg_len_octets; - status = srtp_protect(sender_session, pkt, &protected_msg_len_octets); + /* Create the first packet to decrypt and test for ROC change */ + pkt_1 = srtp_create_test_packet_extended(msg_len_octets, sender_policy.ssrc.value, seq, ts); + protected_msg_len_octets_1 = msg_len_octets; + status = srtp_protect(sender_session, pkt_1, &protected_msg_len_octets_1); + if (status) { + return status; + } + + /* Create the second packet to decrypt and test for ROC change */ + seq++; + ts++; + pkt_2 = srtp_create_test_packet_extended(msg_len_octets, sender_policy.ssrc.value, seq, ts); + protected_msg_len_octets_2 = msg_len_octets; + status = srtp_protect(sender_session, pkt_2, &protected_msg_len_octets_2); if (status) { return status; } @@ -3138,12 +3155,19 @@ test_set_receiver_roc(uint32_t packets, uint32_t roc_to_set) return status; } - /* Make a copy of the sent protected packet */ - recv_pkt = malloc(protected_msg_len_octets); - if (recv_pkt == NULL) { + /* Make a copy of the first sent protected packet */ + recv_pkt_1 = malloc(protected_msg_len_octets_1); + if (recv_pkt_1 == NULL) { return srtp_err_status_fail; } - memcpy(recv_pkt, pkt, protected_msg_len_octets); + memcpy(recv_pkt_1, pkt_1, protected_msg_len_octets_1); + + /* Make a copy of the second sent protected packet */ + recv_pkt_2 = malloc(protected_msg_len_octets_2); + if (recv_pkt_2 == NULL) { + return srtp_err_status_fail; + } + memcpy(recv_pkt_2, pkt_2, protected_msg_len_octets_2); /* Set the ROC to the wanted value */ status = srtp_set_stream_roc(receiver_session, receiver_policy.ssrc.value, roc_to_set); @@ -3151,7 +3175,14 @@ test_set_receiver_roc(uint32_t packets, uint32_t roc_to_set) return status; } - status = srtp_unprotect(receiver_session, recv_pkt, &protected_msg_len_octets); + /* Unprotect the first packet */ + status = srtp_unprotect(receiver_session, recv_pkt_1, &protected_msg_len_octets_1); + if (status) { + return status; + } + + /* Unprotect the second packet */ + status = srtp_unprotect(receiver_session, recv_pkt_2, &protected_msg_len_octets_2); if (status) { return status; } @@ -3167,8 +3198,10 @@ test_set_receiver_roc(uint32_t packets, uint32_t roc_to_set) return status; } - free(pkt); - free(recv_pkt); + free(pkt_1); + free(recv_pkt_1); + free(pkt_2); + free(recv_pkt_2); return srtp_err_status_ok; } @@ -3326,7 +3359,6 @@ srtp_test_set_receiver_roc() { return status; } - return srtp_err_status_ok; } From 28209e2c4bc8c89b14470527ea91dc2ba570b47a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Wed, 7 Jun 2017 12:47:16 +0200 Subject: [PATCH 2/2] Silence "potentially uninitialized" compiler warning. The warning is incorrect in this case, but just to make it happy. --- srtp/srtp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/srtp/srtp.c b/srtp/srtp.c index 3f8c865eb..ede804f1d 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -2346,8 +2346,8 @@ srtp_unprotect_mki(srtp_ctx_t *ctx, void *srtp_hdr, int *pkt_octet_len, unsigned int mki_size = 0; srtp_session_keys_t *session_keys = NULL; int advance_packet_index = 0; - uint32_t roc_to_set; - uint16_t seq_to_set; + uint32_t roc_to_set = 0; + uint16_t seq_to_set = 0; debug_print(mod_srtp, "function srtp_unprotect", NULL);