diff --git a/srtp/srtp.c b/srtp/srtp.c index d7e9f66e5..e0a4b510d 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -1978,28 +1978,39 @@ static srtp_err_status_t srtp_get_session_keys_for_packet( return srtp_err_status_bad_mki; } +/* + * Locating the MKI means knowing how many octets trail it, which RFC 4771 + * makes depend on the packet: for AES-GCM only the 4-octet ROC of a mode 3 + * ROC-carrying packet (RFC 7714 section 8.2), for a mode 1 packet that does + * not carry the ROC nothing at all (srtp_protect() appends no tag to those), + * otherwise the full authentication tag. + */ static srtp_err_status_t srtp_get_session_keys_for_rtp_packet( srtp_stream_ctx_t *stream, const uint8_t *hdr, size_t pkt_octet_len, + bool rcc_carry, srtp_session_keys_t **session_keys) { - size_t tag_len = 0; + size_t trailing_len; if (stream->num_master_keys == 0 || stream->session_keys == NULL) { return srtp_err_status_no_ctx; } - // Determine the authentication tag size if (stream->session_keys[0].rtp_cipher->algorithm == SRTP_AES_GCM_128 || stream->session_keys[0].rtp_cipher->algorithm == SRTP_AES_GCM_256) { - tag_len = 0; + trailing_len = + (stream->rcc_mode == srtp_rcc_mode_3 && rcc_carry) ? 4 : 0; + } else if (stream->rcc_mode == srtp_rcc_mode_1 && !rcc_carry) { + trailing_len = 0; } else { - tag_len = srtp_auth_get_tag_length(stream->session_keys[0].rtp_auth); + trailing_len = + srtp_auth_get_tag_length(stream->session_keys[0].rtp_auth); } - return srtp_get_session_keys_for_packet(stream, hdr, pkt_octet_len, tag_len, - session_keys); + return srtp_get_session_keys_for_packet(stream, hdr, pkt_octet_len, + trailing_len, session_keys); } static srtp_err_status_t srtp_get_session_keys_for_rtcp_packet( @@ -3050,20 +3061,11 @@ srtp_err_status_t srtp_unprotect(srtp_t ctx, /* * Determine if MKI is being used and what session keys should be used. - * For RFC 4771 mode 3 the sender's 4-octet ROC is carried in the SRTP - * authentication tag field, which RFC 7714 section 8.2 places after the - * MKI. Exclude that trailing ROC from the length so the MKI is located - * correctly (the MKI lookup expects the MKI to be the last field). + * How many octets trail the MKI depends on the RCC mode and on whether + * this packet carries the ROC, so the lookup is told which it is. */ - { - size_t mki_lookup_len = srtp_len; - if (stream->rcc_mode == srtp_rcc_mode_3 && rcc_carry && - srtp_len >= octets_in_rtp_header + 4) { - mki_lookup_len -= 4; - } - status = srtp_get_session_keys_for_rtp_packet( - stream, srtp, mki_lookup_len, &session_keys); - } + status = srtp_get_session_keys_for_rtp_packet(stream, srtp, srtp_len, + rcc_carry, &session_keys); if (status) { return status; } diff --git a/test/rcc_test.c b/test/rcc_test.c index 4daca6239..ff594fa17 100644 --- a/test/rcc_test.c +++ b/test/rcc_test.c @@ -78,9 +78,9 @@ static const uint8_t gcm_master_key[16] = { static const uint8_t gcm_master_salt[12] = { 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, }; +#endif static const uint8_t mki4[4] = { 0xde, 0xad, 0xbe, 0xef }; -#endif static void create_cm_rcc_policy_ssrc(srtp_policy_t *policy, srtp_rcc_mode_t mode, @@ -107,6 +107,24 @@ static void create_cm_rcc_policy(srtp_policy_t *policy, create_cm_rcc_policy_ssrc(policy, mode, rate, ssrc_specific); } +static void create_cm_rcc_policy_mki(srtp_policy_t *policy, + srtp_rcc_mode_t mode, + uint16_t rate) +{ + CHECK_OK(srtp_policy_create(policy)); + CHECK_OK(srtp_policy_set_profile(*policy, srtp_profile_aes128_cm_sha1_80)); + CHECK_OK(srtp_policy_set_sec_serv(*policy, sec_serv_conf_and_auth, + sec_serv_conf_and_auth)); + CHECK_OK(srtp_policy_set_ssrc(*policy, + (srtp_ssrc_t){ ssrc_specific, TEST_SSRC })); + CHECK_OK(srtp_policy_set_rcc_mode_tx_rate(*policy, mode, rate)); + CHECK_OK(srtp_policy_set_window_size(*policy, 128)); + CHECK_OK(srtp_policy_use_mki(*policy, sizeof(mki4))); + CHECK_OK(srtp_policy_add_key(*policy, cm_master_key, sizeof(cm_master_key), + cm_master_salt, sizeof(cm_master_salt), mki4, + sizeof(mki4))); +} + #ifdef GCM static void create_gcm_rcc_policy_ssrc(srtp_policy_t *policy, srtp_rcc_mode_t mode, @@ -420,6 +438,33 @@ static void rcc_mode1_rate4_carry_and_untagged(void) CHECK_OK(srtp_shutdown()); } +/* + * Mode 1, R == 4, with an MKI. A packet that does not carry the ROC has no + * authentication tag, so the MKI is its last field; the receiver must locate + * it there rather than a tag length before the end. + */ +static void rcc_mode1_rate4_mki_untagged(void) +{ + srtp_policy_t sp, rp; + srtp_t snd, rcv; + + CHECK_OK(srtp_init()); + create_cm_rcc_policy_mki(&sp, srtp_rcc_mode_1, 4); + create_cm_rcc_policy_mki(&rp, srtp_rcc_mode_1, 4); + CHECK_OK(srtp_create(&snd, sp)); + CHECK_OK(srtp_create(&rcv, rp)); + + for (uint16_t seq = 0; seq <= 12; seq++) { + rcc_roundtrip(snd, rcv, seq, "mode1 rate4 mki payload"); + } + + CHECK_OK(srtp_dealloc(snd)); + CHECK_OK(srtp_dealloc(rcv)); + srtp_policy_destroy(sp); + srtp_policy_destroy(rp); + CHECK_OK(srtp_shutdown()); +} + /* * Mode 2, R == 1, late-joining receiver. The sender first wraps its sequence * number past 65535 so its ROC becomes 1. A brand-new receiver (ROC 0) then @@ -1057,6 +1102,7 @@ TEST_LIST = { rcc_mode2_rate4_carry_and_noncarry }, { "rcc_mode1_rate4_carry_and_untagged()", rcc_mode1_rate4_carry_and_untagged }, + { "rcc_mode1_rate4_mki_untagged()", rcc_mode1_rate4_mki_untagged }, { "rcc_mode2_late_join_roc_sync()", rcc_mode2_late_join_roc_sync }, { "rcc_mode2_rate4_late_join()", rcc_mode2_rate4_late_join }, { "rcc_mode2_wildcard_inbound_late_join()",