From 65a358727c296e2fd9d2bd37afce41089feb664e Mon Sep 17 00:00:00 2001 From: Marc Soda Date: Fri, 8 May 2015 14:48:15 -0400 Subject: [PATCH 01/11] Use the proper ROC when encrypting. --- kernel-module/xt_RTPENGINE.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/kernel-module/xt_RTPENGINE.c b/kernel-module/xt_RTPENGINE.c index 713c34997..c203eb838 100644 --- a/kernel-module/xt_RTPENGINE.c +++ b/kernel-module/xt_RTPENGINE.c @@ -2264,11 +2264,19 @@ src_check_ok: rtp_pt_idx = rtp_payload_type(rtp.header, &g->target); - pkt_idx_u = pkt_idx = packet_index(&g->decrypt, &g->target.decrypt, rtp.header); - if (srtp_auth_validate(&g->decrypt, &g->target.decrypt, &rtp, &pkt_idx)) - goto skip_error; - if (pkt_idx != pkt_idx_u) - update_packet_index(&g->decrypt, &g->target.decrypt, pkt_idx); + if ((&g->decrypt)->cipher->decrypt) { + pkt_idx_u = pkt_idx = packet_index(&g->decrypt, &g->target.decrypt, rtp.header); + if (srtp_auth_validate(&g->decrypt, &g->target.decrypt, &rtp, &pkt_idx)) + goto skip_error; + if (pkt_idx != pkt_idx_u) + update_packet_index(&g->decrypt, &g->target.decrypt, pkt_idx); + } else { + pkt_idx_u = pkt_idx = packet_index(&g->encrypt, &g->target.encrypt, rtp.header); + + if (pkt_idx != pkt_idx_u) + update_packet_index(&g->encrypt, &g->target.encrypt, pkt_idx); + } + if (srtp_decrypt(&g->decrypt, &g->target.decrypt, &rtp, pkt_idx)) goto skip_error; From 16b91847f92d61d41a2d5e7c5ee854c8e0d917c7 Mon Sep 17 00:00:00 2001 From: Marc Soda Date: Fri, 8 May 2015 14:50:22 -0400 Subject: [PATCH 02/11] Track SSRCs and sequence numbers. Sync between userspace and the kernel. --- daemon/call.c | 15 +++++++++++++++ daemon/crypto.c | 33 +++++++++++++++++++++++++++++++++ daemon/crypto.h | 15 ++++++++++++--- daemon/rtp.c | 29 ++++++++++++++++++++++------- kernel-module/xt_RTPENGINE.c | 10 ++++++++++ kernel-module/xt_RTPENGINE.h | 1 + 6 files changed, 93 insertions(+), 10 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index b66a174d1..fdef6c20c 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -161,6 +161,7 @@ static int call_avpf2avp_rtcp(str *s, struct packet_stream *); static int call_savpf2avp_rtcp(str *s, struct packet_stream *); //static int call_savpf2savp_rtcp(str *s, struct packet_stream *); +static void unkernelize(struct packet_stream *); /* ********** */ @@ -435,6 +436,7 @@ void kernelize(struct packet_stream *stream) { reti.src_addr.family = reti.dst_addr.family; reti.src_addr.port = sink->sfd->fd.localport; + reti.ssrc = sink->crypto.ssrc; ifa = g_atomic_pointer_get(&sink->media->local_address); if (reti.src_addr.family == AF_INET) @@ -860,6 +862,12 @@ kernel_check: if (PS_ISSET(stream, NO_KERNEL_SUPPORT)) goto forward; + if (sink && sink->crypto.ssrc_mismatch && !stun_ret) { + ilog(LOG_INFO, "SSRC changed, unkernelizing media stream"); + __unkernelize(stream); + goto forward; + } + if (PS_ISSET(stream, CONFIRMED) && sink && PS_ARESET2(sink, CONFIRMED, FILLED)) kernelize(stream); @@ -1378,6 +1386,7 @@ static void callmaster_timer(void *ptr) { struct stream_fd *sfd; struct rtp_stats *rs; unsigned int pt; + struct rtp_ssrc_entry *cur_ssrc; ZERO(hlp); @@ -1444,6 +1453,11 @@ static void callmaster_timer(void *ptr) { mutex_lock(&sink->out_lock); if (sink->crypto.params.crypto_suite && ke->target.encrypt.last_index - sink->crypto.last_index > 0x4000) { + // Keep the SSRC list in sync too. + cur_ssrc = find_ssrc(ke->target.ssrc, sink->crypto.ssrc_list); + if (cur_ssrc) + cur_ssrc->index = ke->target.encrypt.last_index; + sink->crypto.last_index = ke->target.encrypt.last_index; update = 1; } @@ -2931,6 +2945,7 @@ void call_destroy(struct call *c) { dtls_shutdown(ps); ps->sfd = NULL; crypto_cleanup(&ps->crypto); + free_ssrc_list(ps->crypto.ssrc_list); ps->rtp_sink = NULL; ps->rtcp_sink = NULL; diff --git a/daemon/crypto.c b/daemon/crypto.c index 4caaf0c50..b069c112b 100644 --- a/daemon/crypto.c +++ b/daemon/crypto.c @@ -572,3 +572,36 @@ void crypto_dump_keys(struct crypto_context *in, struct crypto_context *out) { ilog(LOG_DEBUG, "SRTP keys, outgoing:"); dump_key(out); } + +struct rtp_ssrc_entry *find_ssrc(u_int32_t ssrc, struct rtp_ssrc_entry *list) { + for (; list; list = list->next) { + if (list->ssrc == ssrc) + return list; + } + return 0; +} + +void add_ssrc_entry(struct rtp_ssrc_entry *ent, struct rtp_ssrc_entry *list) { + struct rtp_ssrc_entry *cur; + for (cur = list; list; list = list->next) + cur = list; + cur->next = ent; +} + +struct rtp_ssrc_entry *create_ssrc_entry(u_int32_t ssrc, u_int64_t index) { + struct rtp_ssrc_entry *ent; + ent = malloc(sizeof(struct rtp_ssrc_entry)); + ent->ssrc = ssrc; + ent->index = index; + ent->next = 0; + return ent; +} + +void free_ssrc_list(struct rtp_ssrc_entry *list) { + struct rtp_ssrc_entry *tmp; + for (tmp = list; tmp; ) { + tmp = list->next; + free(list); + list = tmp; + } +} diff --git a/daemon/crypto.h b/daemon/crypto.h index 54aa37c62..04b28b66d 100644 --- a/daemon/crypto.h +++ b/daemon/crypto.h @@ -88,10 +88,16 @@ struct crypto_context { void *session_key_ctx[2]; int have_session_key:1; -}; - + struct rtp_ssrc_entry *ssrc_list; + int ssrc_mismatch; +}; +struct rtp_ssrc_entry { + u_int32_t ssrc; + u_int64_t index; + struct rtp_ssrc_entry *next; +}; extern const struct crypto_suite crypto_suites[]; extern const int num_crypto_suites; @@ -102,7 +108,10 @@ const struct crypto_suite *crypto_find_suite(const str *); int crypto_gen_session_key(struct crypto_context *, str *, unsigned char, int); void crypto_dump_keys(struct crypto_context *in, struct crypto_context *out); - +struct rtp_ssrc_entry *find_ssrc(u_int32_t, struct rtp_ssrc_entry *); +void add_ssrc_entry(struct rtp_ssrc_entry *, struct rtp_ssrc_entry *); +struct rtp_ssrc_entry *create_ssrc_entry(u_int32_t, u_int64_t); +void free_ssrc_list(struct rtp_ssrc_entry *); INLINE int crypto_encrypt_rtp(struct crypto_context *c, struct rtp_header *rtp, diff --git a/daemon/rtp.c b/daemon/rtp.c index a70544bee..289d6f508 100644 --- a/daemon/rtp.c +++ b/daemon/rtp.c @@ -182,26 +182,41 @@ void rtp_append_mki(str *s, struct crypto_context *c) { int rtp_avp2savp(str *s, struct crypto_context *c) { struct rtp_header *rtp; str payload, to_auth; - u_int64_t index; + struct rtp_ssrc_entry *cur_ssrc; if (rtp_payload(&rtp, &payload, s)) return -1; if (check_session_keys(c)) return -1; - /* SSRC is part of the crypto context and ROC must be reset when it changes */ + if (G_UNLIKELY(!c->ssrc_list)) + c->ssrc_list = create_ssrc_entry(rtp->ssrc, ntohs(rtp->seq_num)); + + // Find the entry for the current SSRC. + cur_ssrc = find_ssrc(rtp->ssrc, c->ssrc_list); + // If it doesn't exist, create a new entry. + if (G_UNLIKELY(!cur_ssrc)) { + cur_ssrc = create_ssrc_entry(rtp->ssrc, ntohs(rtp->seq_num)); + add_ssrc_entry(cur_ssrc, c->ssrc_list); + } + + // New SSRC, set the crypto context. if (G_UNLIKELY(!c->ssrc)) c->ssrc = rtp->ssrc; + // SSRC has changed. else if (G_UNLIKELY(c->ssrc != rtp->ssrc)) { - c->last_index = 0; + // Signal stream_packet() to unkernelize. + c->ssrc_mismatch = 1; c->ssrc = rtp->ssrc; + c->last_index = cur_ssrc->index; + } else { + c->ssrc_mismatch = 0; } - index = packet_index(c, rtp); + cur_ssrc->index = packet_index(c, rtp); /* rfc 3711 section 3.1 */ - - if (!c->params.session_params.unencrypted_srtp && crypto_encrypt_rtp(c, rtp, &payload, index)) + if (!c->params.session_params.unencrypted_srtp && crypto_encrypt_rtp(c, rtp, &payload, cur_ssrc->index)) return -1; to_auth = *s; @@ -209,7 +224,7 @@ int rtp_avp2savp(str *s, struct crypto_context *c) { rtp_append_mki(s, c); if (!c->params.session_params.unauthenticated_srtp && c->params.crypto_suite->srtp_auth_tag) { - c->params.crypto_suite->hash_rtp(c, s->s + s->len, &to_auth, index); + c->params.crypto_suite->hash_rtp(c, s->s + s->len, &to_auth, cur_ssrc->index); s->len += c->params.crypto_suite->srtp_auth_tag; } diff --git a/kernel-module/xt_RTPENGINE.c b/kernel-module/xt_RTPENGINE.c index c203eb838..f7db1826a 100644 --- a/kernel-module/xt_RTPENGINE.c +++ b/kernel-module/xt_RTPENGINE.c @@ -141,6 +141,7 @@ struct re_crypto_context { struct crypto_shash *shash; const struct re_cipher *cipher; const struct re_hmac *hmac; + u_int32_t ssrc; }; struct rtpengine_stats_a { @@ -1891,6 +1892,12 @@ static u_int64_t packet_index(struct re_crypto_context *c, long long int diff; unsigned long flags; + // Keep the crypt context SSRC in sync. + if (unlikely(!c->ssrc)) + c->ssrc = rtp->ssrc; + else if (unlikely(c->ssrc != rtp->ssrc)) + c->ssrc = rtp->ssrc; + seq = ntohs(rtp->seq_num); spin_lock_irqsave(&c->lock, flags); @@ -2271,6 +2278,9 @@ src_check_ok: if (pkt_idx != pkt_idx_u) update_packet_index(&g->decrypt, &g->target.decrypt, pkt_idx); } else { + // Pass to userspace if SSRC has changed. + if ((g->encrypt.ssrc) && (g->encrypt.ssrc != rtp.header->ssrc)) + goto skip_error; pkt_idx_u = pkt_idx = packet_index(&g->encrypt, &g->target.encrypt, rtp.header); if (pkt_idx != pkt_idx_u) diff --git a/kernel-module/xt_RTPENGINE.h b/kernel-module/xt_RTPENGINE.h index 361a56fb0..0dd2bd8e1 100644 --- a/kernel-module/xt_RTPENGINE.h +++ b/kernel-module/xt_RTPENGINE.h @@ -82,6 +82,7 @@ struct rtpengine_target_info { struct rtpengine_srtp decrypt; struct rtpengine_srtp encrypt; + u_int32_t ssrc; // Expose the SSRC to userspace when we resync. unsigned char payload_types[NUM_PAYLOAD_TYPES]; /* must be sorted */ unsigned int num_payload_types; From 965fa396d269610a7fc7e6eb1825bd34c23c470d Mon Sep 17 00:00:00 2001 From: Kevin McAllister Date: Fri, 8 May 2015 15:21:14 -0400 Subject: [PATCH 03/11] Change packet_index functions to more closely match RFC This became necessary because of the way Asterisk handles Sequence numbers when changing SSRC. They continue to increment a single sequence number even though the SSRC is different, on switching back this causes the packet_index function to interpret this as many lost packets. The previous function had dead-spots that would not adjust the packet_index at all if the difference fell in these ranges. These gaps always resulted in behavior contra what would happen in webrtc clients. --- daemon/rtp.c | 34 +++++++++++++++--------------- kernel-module/xt_RTPENGINE.c | 40 ++++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/daemon/rtp.c b/daemon/rtp.c index 289d6f508..00781428a 100644 --- a/daemon/rtp.c +++ b/daemon/rtp.c @@ -137,8 +137,6 @@ error: static u_int64_t packet_index(struct crypto_context *c, struct rtp_header *rtp) { u_int16_t seq; - u_int64_t index; - long long int diff; seq = ntohs(rtp->seq_num); /* rfc 3711 section 3.3.1 */ @@ -146,24 +144,24 @@ static u_int64_t packet_index(struct crypto_context *c, struct rtp_header *rtp) c->last_index = seq; /* rfc 3711 appendix A, modified, and sections 3.3 and 3.3.1 */ - index = (c->last_index & 0xffffffff0000ULL) | seq; - diff = index - c->last_index; - if (diff >= 0) { - if (diff < 0x8000) - c->last_index = index; - else if (index >= 0x10000) - index -= 0x10000; - } - else { - if (diff >= -0x8000) - ; - else { - index += 0x10000; - c->last_index = index; - } + u_int16_t s_l = (c->last_index & 0x00000000ffffULL); + u_int32_t roc = (c->last_index & 0xffffffff0000ULL) >> 16; + u_int32_t v = 0; + + if (s_l < 0x8000) { + if (((seq - s_l) > 0x8000) && roc > 0) + v = (roc - 1) % 0x10000; + else + v = roc; + } else { + if ((s_l - 0x8000) > seq) + v = (roc + 1) % 0x10000; + else + v = roc; } - return index; + c->last_index = (u_int64_t)(((v << 16) | seq) & 0xffffffffffffULL); + return c->last_index; } void rtp_append_mki(str *s, struct crypto_context *c) { diff --git a/kernel-module/xt_RTPENGINE.c b/kernel-module/xt_RTPENGINE.c index f7db1826a..b50124816 100644 --- a/kernel-module/xt_RTPENGINE.c +++ b/kernel-module/xt_RTPENGINE.c @@ -1889,8 +1889,10 @@ static u_int64_t packet_index(struct re_crypto_context *c, { u_int16_t seq; u_int64_t index; - long long int diff; unsigned long flags; + u_int16_t s_l; + u_int32_t roc; + u_int32_t v; // Keep the crypt context SSRC in sync. if (unlikely(!c->ssrc)) @@ -1906,25 +1908,27 @@ static u_int64_t packet_index(struct re_crypto_context *c, if (unlikely(!s->last_index)) s->last_index = seq; - /* rfc 3711 appendix A, modified, and sections 3.3 and 3.3.1 */ - index = ((u_int64_t) c->roc << 16) | seq; - diff = index - s->last_index; - if (diff >= 0) { - if (diff < 0x8000) - s->last_index = index; - else if (index >= 0x10000) - index -= 0x10000; - } - else { - if (diff >= -0x8000) - ; - else { - index += 0x10000; - c->roc++; - s->last_index = index; - } + /* rfc 3711 appendix A, modified, and sections 3.3 and 3.3.1 */ + s_l = (s->last_index & 0x00000000ffffULL); + roc = (s->last_index & 0xffffffff0000ULL) >> 16; + v = 0; + + if (s_l < 0x8000) { + if (((seq - s_l) > 0x8000) && roc > 0) + v = (roc - 1) % 0x10000; + else + v = roc; + } else { + if ((s_l - 0x8000) > seq) + v = (roc + 1) % 0x10000; + else + v = roc; } + index = (v << 16) | seq; + s->last_index = index; + c->roc = v; + spin_unlock_irqrestore(&c->lock, flags); return index; From 3097f71b8ca8a92baed3f06952e7464ad9e1c83a Mon Sep 17 00:00:00 2001 From: Kevin McAllister Date: Fri, 8 May 2015 15:24:04 -0400 Subject: [PATCH 04/11] convert spaces to tab on comment. --- kernel-module/xt_RTPENGINE.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel-module/xt_RTPENGINE.c b/kernel-module/xt_RTPENGINE.c index b50124816..12dd9c0dd 100644 --- a/kernel-module/xt_RTPENGINE.c +++ b/kernel-module/xt_RTPENGINE.c @@ -1908,7 +1908,7 @@ static u_int64_t packet_index(struct re_crypto_context *c, if (unlikely(!s->last_index)) s->last_index = seq; - /* rfc 3711 appendix A, modified, and sections 3.3 and 3.3.1 */ + /* rfc 3711 appendix A, modified, and sections 3.3 and 3.3.1 */ s_l = (s->last_index & 0x00000000ffffULL); roc = (s->last_index & 0xffffffff0000ULL) >> 16; v = 0; From 0ac1ed24192d1dca9efc3289541231475d1613b7 Mon Sep 17 00:00:00 2001 From: Marc Soda Date: Fri, 8 May 2015 15:36:57 -0400 Subject: [PATCH 05/11] Remove unnecessary prototype. --- daemon/call.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index fdef6c20c..a12b42ce4 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -161,8 +161,6 @@ static int call_avpf2avp_rtcp(str *s, struct packet_stream *); static int call_savpf2avp_rtcp(str *s, struct packet_stream *); //static int call_savpf2savp_rtcp(str *s, struct packet_stream *); -static void unkernelize(struct packet_stream *); - /* ********** */ static const struct streamhandler_io __shio_noop = { From 9c3ad508f7ab1c5d562ef7202868ffaac4f2ba3a Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Tue, 12 May 2015 10:45:15 -0400 Subject: [PATCH 06/11] use appropriate crypto context for encryption and decryption ensures that the correct ROC is used --- kernel-module/xt_RTPENGINE.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/kernel-module/xt_RTPENGINE.c b/kernel-module/xt_RTPENGINE.c index 12dd9c0dd..d8baaa4c5 100644 --- a/kernel-module/xt_RTPENGINE.c +++ b/kernel-module/xt_RTPENGINE.c @@ -2071,26 +2071,28 @@ static int srtp_auth_validate(struct re_crypto_context *c, if (srtp_hash(hmac, c, s, r, pkt_idx)) return -1; if (!memcmp(auth_tag, hmac, s->auth_tag_len)) - goto ok; + goto ok_update; /* or maybe we did a rollover too many */ if (pkt_idx >= 0x20000) { pkt_idx -= 0x20000; if (srtp_hash(hmac, c, s, r, pkt_idx)) return -1; if (!memcmp(auth_tag, hmac, s->auth_tag_len)) - goto ok; + goto ok_update; } /* last guess: reset ROC to zero */ pkt_idx &= 0xffff; if (srtp_hash(hmac, c, s, r, pkt_idx)) return -1; if (!memcmp(auth_tag, hmac, s->auth_tag_len)) - goto ok; + goto ok_update; return -1; -ok: +ok_update: *pkt_idx_p = pkt_idx; + update_packet_index(c, s, pkt_idx); +ok: return 0; } @@ -2203,7 +2205,7 @@ static unsigned int rtpengine46(struct sk_buff *skb, struct rtpengine_table *t, unsigned int datalen; u_int32_t *u32; struct rtp_parsed rtp; - u_int64_t pkt_idx = 0, pkt_idx_u; + u_int64_t pkt_idx; skb_reset_transport_header(skb); uh = udp_hdr(skb); @@ -2275,22 +2277,13 @@ src_check_ok: rtp_pt_idx = rtp_payload_type(rtp.header, &g->target); - if ((&g->decrypt)->cipher->decrypt) { - pkt_idx_u = pkt_idx = packet_index(&g->decrypt, &g->target.decrypt, rtp.header); - if (srtp_auth_validate(&g->decrypt, &g->target.decrypt, &rtp, &pkt_idx)) - goto skip_error; - if (pkt_idx != pkt_idx_u) - update_packet_index(&g->decrypt, &g->target.decrypt, pkt_idx); - } else { - // Pass to userspace if SSRC has changed. - if ((g->encrypt.ssrc) && (g->encrypt.ssrc != rtp.header->ssrc)) - goto skip_error; - pkt_idx_u = pkt_idx = packet_index(&g->encrypt, &g->target.encrypt, rtp.header); - - if (pkt_idx != pkt_idx_u) - update_packet_index(&g->encrypt, &g->target.encrypt, pkt_idx); - } + // Pass to userspace if SSRC has changed. + if ((g->encrypt.ssrc) && (g->encrypt.ssrc != rtp.header->ssrc)) + goto skip_error; + pkt_idx = packet_index(&g->decrypt, &g->target.decrypt, rtp.header); + if (srtp_auth_validate(&g->decrypt, &g->target.decrypt, &rtp, &pkt_idx)) + goto skip_error; if (srtp_decrypt(&g->decrypt, &g->target.decrypt, &rtp, pkt_idx)) goto skip_error; @@ -2313,6 +2306,7 @@ not_rtp: } if (rtp.ok) { + pkt_idx = packet_index(&g->encrypt, &g->target.encrypt, rtp.header); srtp_encrypt(&g->encrypt, &g->target.encrypt, &rtp, pkt_idx); skb_put(skb, g->target.encrypt.mki_len + g->target.encrypt.auth_tag_len); srtp_authenticate(&g->encrypt, &g->target.encrypt, &rtp, pkt_idx); From 76f3893464bc8938c21618cb072f235a3df2d752 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Tue, 12 May 2015 15:38:42 -0400 Subject: [PATCH 07/11] ssrc tracking improvements change ssrc tracking code to a hash table and delay creation of hash table and ssrc lookup until a change in ssrc is actually seen --- daemon/aux.c | 8 ++++++ daemon/aux.h | 2 ++ daemon/call.c | 19 +++---------- daemon/crypto.c | 36 +++++++++++------------- daemon/crypto.h | 13 ++++----- daemon/rtp.c | 53 ++++++++++++++++++++++-------------- kernel-module/xt_RTPENGINE.c | 9 +----- 7 files changed, 70 insertions(+), 70 deletions(-) diff --git a/daemon/aux.c b/daemon/aux.c index cbb645024..459f781a6 100644 --- a/daemon/aux.c +++ b/daemon/aux.c @@ -212,3 +212,11 @@ int g_tree_find_all_cmp(void *k, void *v, void *d) { g_queue_push_tail(q, v); return FALSE; } +unsigned int uint32_hash(const void *p) { + const u_int32_t *a = p; + return *a; +} +int uint32_eq(const void *a, const void *b) { + const u_int32_t *A = a, *B = b; + return (*A == *B) ? TRUE : FALSE; +} diff --git a/daemon/aux.h b/daemon/aux.h index 80a731457..615a4474d 100644 --- a/daemon/aux.h +++ b/daemon/aux.h @@ -67,6 +67,8 @@ INLINE void strdupfree(char **, const char *); char *get_thread_buf(void); unsigned int in6_addr_hash(const void *p); int in6_addr_eq(const void *a, const void *b); +unsigned int uint32_hash(const void *p); +int uint32_eq(const void *a, const void *b); diff --git a/daemon/call.c b/daemon/call.c index a12b42ce4..fbaee346d 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -764,14 +764,16 @@ loop_ok: mutex_lock(&out_srtp->out_lock); /* return values are: 0 = forward packet, -1 = error/dont forward, - * 1 = forward and push update to redis */ + * 1 = forward and push update to redis and kernel */ if (rwf_in) handler_ret = rwf_in(s, in_srtp); if (handler_ret >= 0 && rwf_out) handler_ret += rwf_out(s, out_srtp); - if (handler_ret > 0) + if (handler_ret > 0) { + __unkernelize(stream); update = 1; + } mutex_unlock(&out_srtp->out_lock); mutex_unlock(&in_srtp->in_lock); @@ -860,12 +862,6 @@ kernel_check: if (PS_ISSET(stream, NO_KERNEL_SUPPORT)) goto forward; - if (sink && sink->crypto.ssrc_mismatch && !stun_ret) { - ilog(LOG_INFO, "SSRC changed, unkernelizing media stream"); - __unkernelize(stream); - goto forward; - } - if (PS_ISSET(stream, CONFIRMED) && sink && PS_ARESET2(sink, CONFIRMED, FILLED)) kernelize(stream); @@ -1384,7 +1380,6 @@ static void callmaster_timer(void *ptr) { struct stream_fd *sfd; struct rtp_stats *rs; unsigned int pt; - struct rtp_ssrc_entry *cur_ssrc; ZERO(hlp); @@ -1451,11 +1446,6 @@ static void callmaster_timer(void *ptr) { mutex_lock(&sink->out_lock); if (sink->crypto.params.crypto_suite && ke->target.encrypt.last_index - sink->crypto.last_index > 0x4000) { - // Keep the SSRC list in sync too. - cur_ssrc = find_ssrc(ke->target.ssrc, sink->crypto.ssrc_list); - if (cur_ssrc) - cur_ssrc->index = ke->target.encrypt.last_index; - sink->crypto.last_index = ke->target.encrypt.last_index; update = 1; } @@ -2943,7 +2933,6 @@ void call_destroy(struct call *c) { dtls_shutdown(ps); ps->sfd = NULL; crypto_cleanup(&ps->crypto); - free_ssrc_list(ps->crypto.ssrc_list); ps->rtp_sink = NULL; ps->rtcp_sink = NULL; diff --git a/daemon/crypto.c b/daemon/crypto.c index b069c112b..41f8f7ba8 100644 --- a/daemon/crypto.c +++ b/daemon/crypto.c @@ -573,35 +573,31 @@ void crypto_dump_keys(struct crypto_context *in, struct crypto_context *out) { dump_key(out); } -struct rtp_ssrc_entry *find_ssrc(u_int32_t ssrc, struct rtp_ssrc_entry *list) { - for (; list; list = list->next) { - if (list->ssrc == ssrc) - return list; - } - return 0; +struct rtp_ssrc_entry *find_ssrc(u_int32_t ssrc, GHashTable *ht) { + return g_hash_table_lookup(ht, &ssrc); } -void add_ssrc_entry(struct rtp_ssrc_entry *ent, struct rtp_ssrc_entry *list) { - struct rtp_ssrc_entry *cur; - for (cur = list; list; list = list->next) - cur = list; - cur->next = ent; +void add_ssrc_entry(struct rtp_ssrc_entry *ent, GHashTable *ht) { + g_hash_table_insert(ht, &ent->ssrc, ent); } struct rtp_ssrc_entry *create_ssrc_entry(u_int32_t ssrc, u_int64_t index) { struct rtp_ssrc_entry *ent; - ent = malloc(sizeof(struct rtp_ssrc_entry)); + ent = g_slice_alloc(sizeof(struct rtp_ssrc_entry)); ent->ssrc = ssrc; ent->index = index; - ent->next = 0; return ent; } -void free_ssrc_list(struct rtp_ssrc_entry *list) { - struct rtp_ssrc_entry *tmp; - for (tmp = list; tmp; ) { - tmp = list->next; - free(list); - list = tmp; - } +void free_ssrc_table(GHashTable **ht) { + if (!*ht) + return; + g_hash_table_destroy(*ht); + *ht = NULL; +} +static void free_ssrc_entry(void *p) { + g_slice_free1(sizeof(struct rtp_ssrc_entry), p); +} +GHashTable *create_ssrc_table(void) { + return g_hash_table_new_full(uint32_hash, uint32_eq, free_ssrc_entry, NULL); } diff --git a/daemon/crypto.h b/daemon/crypto.h index 04b28b66d..b0211bca1 100644 --- a/daemon/crypto.h +++ b/daemon/crypto.h @@ -86,17 +86,14 @@ struct crypto_context { /* ? */ void *session_key_ctx[2]; + GHashTable *ssrc_hash; int have_session_key:1; - - struct rtp_ssrc_entry *ssrc_list; - int ssrc_mismatch; }; struct rtp_ssrc_entry { u_int32_t ssrc; u_int64_t index; - struct rtp_ssrc_entry *next; }; extern const struct crypto_suite crypto_suites[]; @@ -108,10 +105,11 @@ const struct crypto_suite *crypto_find_suite(const str *); int crypto_gen_session_key(struct crypto_context *, str *, unsigned char, int); void crypto_dump_keys(struct crypto_context *in, struct crypto_context *out); -struct rtp_ssrc_entry *find_ssrc(u_int32_t, struct rtp_ssrc_entry *); -void add_ssrc_entry(struct rtp_ssrc_entry *, struct rtp_ssrc_entry *); +struct rtp_ssrc_entry *find_ssrc(u_int32_t, GHashTable *); +void add_ssrc_entry(struct rtp_ssrc_entry *, GHashTable *); struct rtp_ssrc_entry *create_ssrc_entry(u_int32_t, u_int64_t); -void free_ssrc_list(struct rtp_ssrc_entry *); +void free_ssrc_table(GHashTable **); +GHashTable *create_ssrc_table(void); INLINE int crypto_encrypt_rtp(struct crypto_context *c, struct rtp_header *rtp, @@ -145,6 +143,7 @@ INLINE void crypto_params_cleanup(struct crypto_params *p) { } INLINE void crypto_cleanup(struct crypto_context *c) { crypto_params_cleanup(&c->params); + free_ssrc_table(&c->ssrc_hash); if (!c->params.crypto_suite) return; if (c->params.crypto_suite->session_key_cleanup) diff --git a/daemon/rtp.c b/daemon/rtp.c index 00781428a..3b27ce915 100644 --- a/daemon/rtp.c +++ b/daemon/rtp.c @@ -180,41 +180,54 @@ void rtp_append_mki(str *s, struct crypto_context *c) { int rtp_avp2savp(str *s, struct crypto_context *c) { struct rtp_header *rtp; str payload, to_auth; + u_int64_t index; struct rtp_ssrc_entry *cur_ssrc; + int update_kernel = 0; if (rtp_payload(&rtp, &payload, s)) return -1; if (check_session_keys(c)) return -1; - if (G_UNLIKELY(!c->ssrc_list)) - c->ssrc_list = create_ssrc_entry(rtp->ssrc, ntohs(rtp->seq_num)); + /* check last known SSRC */ + if (G_LIKELY(rtp->ssrc == c->ssrc)) + goto ssrc_ok; + if (!c->ssrc) { + c->ssrc = rtp->ssrc; + update_kernel = 1; + goto ssrc_ok; + } + + /* SSRC mismatch. stash away last know info */ + ilog(LOG_DEBUG, "SSRC changed, updating SRTP crypto contexts"); + if (G_UNLIKELY(!c->ssrc_hash)) + c->ssrc_hash = create_ssrc_table(); - // Find the entry for the current SSRC. - cur_ssrc = find_ssrc(rtp->ssrc, c->ssrc_list); + // Find the entry for the last SSRC. + cur_ssrc = find_ssrc(c->ssrc, c->ssrc_hash); // If it doesn't exist, create a new entry. if (G_UNLIKELY(!cur_ssrc)) { - cur_ssrc = create_ssrc_entry(rtp->ssrc, ntohs(rtp->seq_num)); - add_ssrc_entry(cur_ssrc, c->ssrc_list); + cur_ssrc = create_ssrc_entry(c->ssrc, c->last_index); + add_ssrc_entry(cur_ssrc, c->ssrc_hash); } + else + cur_ssrc->index = c->last_index; // New SSRC, set the crypto context. - if (G_UNLIKELY(!c->ssrc)) - c->ssrc = rtp->ssrc; - // SSRC has changed. - else if (G_UNLIKELY(c->ssrc != rtp->ssrc)) { - // Signal stream_packet() to unkernelize. - c->ssrc_mismatch = 1; - c->ssrc = rtp->ssrc; + c->ssrc = rtp->ssrc; + cur_ssrc = find_ssrc(rtp->ssrc, c->ssrc_hash); + if (G_UNLIKELY(!cur_ssrc)) + c->last_index = 0; + else c->last_index = cur_ssrc->index; - } else { - c->ssrc_mismatch = 0; - } - cur_ssrc->index = packet_index(c, rtp); + update_kernel = 1; + +ssrc_ok: + index = packet_index(c, rtp); /* rfc 3711 section 3.1 */ - if (!c->params.session_params.unencrypted_srtp && crypto_encrypt_rtp(c, rtp, &payload, cur_ssrc->index)) + if (!c->params.session_params.unencrypted_srtp && crypto_encrypt_rtp(c, rtp, &payload, index)) return -1; to_auth = *s; @@ -222,11 +235,11 @@ int rtp_avp2savp(str *s, struct crypto_context *c) { rtp_append_mki(s, c); if (!c->params.session_params.unauthenticated_srtp && c->params.crypto_suite->srtp_auth_tag) { - c->params.crypto_suite->hash_rtp(c, s->s + s->len, &to_auth, cur_ssrc->index); + c->params.crypto_suite->hash_rtp(c, s->s + s->len, &to_auth, index); s->len += c->params.crypto_suite->srtp_auth_tag; } - return 0; + return update_kernel ? 1 : 0; } /* rfc 3711, section 3.3 */ diff --git a/kernel-module/xt_RTPENGINE.c b/kernel-module/xt_RTPENGINE.c index d8baaa4c5..b67688307 100644 --- a/kernel-module/xt_RTPENGINE.c +++ b/kernel-module/xt_RTPENGINE.c @@ -141,7 +141,6 @@ struct re_crypto_context { struct crypto_shash *shash; const struct re_cipher *cipher; const struct re_hmac *hmac; - u_int32_t ssrc; }; struct rtpengine_stats_a { @@ -1894,12 +1893,6 @@ static u_int64_t packet_index(struct re_crypto_context *c, u_int32_t roc; u_int32_t v; - // Keep the crypt context SSRC in sync. - if (unlikely(!c->ssrc)) - c->ssrc = rtp->ssrc; - else if (unlikely(c->ssrc != rtp->ssrc)) - c->ssrc = rtp->ssrc; - seq = ntohs(rtp->seq_num); spin_lock_irqsave(&c->lock, flags); @@ -2278,7 +2271,7 @@ src_check_ok: rtp_pt_idx = rtp_payload_type(rtp.header, &g->target); // Pass to userspace if SSRC has changed. - if ((g->encrypt.ssrc) && (g->encrypt.ssrc != rtp.header->ssrc)) + if (unlikely((g->target.ssrc) && (g->target.ssrc != rtp.header->ssrc))) goto skip_error; pkt_idx = packet_index(&g->decrypt, &g->target.decrypt, rtp.header); From c3c6d37c7dcf0af12cd3079b582d506f317b5e13 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Wed, 13 May 2015 11:45:21 -0400 Subject: [PATCH 08/11] convert ssrc tracking functions to inline --- daemon/crypto.c | 29 ----------------------------- daemon/crypto.h | 38 +++++++++++++++++++++++++++++++++----- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/daemon/crypto.c b/daemon/crypto.c index 41f8f7ba8..4caaf0c50 100644 --- a/daemon/crypto.c +++ b/daemon/crypto.c @@ -572,32 +572,3 @@ void crypto_dump_keys(struct crypto_context *in, struct crypto_context *out) { ilog(LOG_DEBUG, "SRTP keys, outgoing:"); dump_key(out); } - -struct rtp_ssrc_entry *find_ssrc(u_int32_t ssrc, GHashTable *ht) { - return g_hash_table_lookup(ht, &ssrc); -} - -void add_ssrc_entry(struct rtp_ssrc_entry *ent, GHashTable *ht) { - g_hash_table_insert(ht, &ent->ssrc, ent); -} - -struct rtp_ssrc_entry *create_ssrc_entry(u_int32_t ssrc, u_int64_t index) { - struct rtp_ssrc_entry *ent; - ent = g_slice_alloc(sizeof(struct rtp_ssrc_entry)); - ent->ssrc = ssrc; - ent->index = index; - return ent; -} - -void free_ssrc_table(GHashTable **ht) { - if (!*ht) - return; - g_hash_table_destroy(*ht); - *ht = NULL; -} -static void free_ssrc_entry(void *p) { - g_slice_free1(sizeof(struct rtp_ssrc_entry), p); -} -GHashTable *create_ssrc_table(void) { - return g_hash_table_new_full(uint32_hash, uint32_eq, free_ssrc_entry, NULL); -} diff --git a/daemon/crypto.h b/daemon/crypto.h index b0211bca1..b723857f4 100644 --- a/daemon/crypto.h +++ b/daemon/crypto.h @@ -7,6 +7,7 @@ #include #include "compat.h" #include "str.h" +#include "aux.h" @@ -105,11 +106,11 @@ const struct crypto_suite *crypto_find_suite(const str *); int crypto_gen_session_key(struct crypto_context *, str *, unsigned char, int); void crypto_dump_keys(struct crypto_context *in, struct crypto_context *out); -struct rtp_ssrc_entry *find_ssrc(u_int32_t, GHashTable *); -void add_ssrc_entry(struct rtp_ssrc_entry *, GHashTable *); -struct rtp_ssrc_entry *create_ssrc_entry(u_int32_t, u_int64_t); -void free_ssrc_table(GHashTable **); -GHashTable *create_ssrc_table(void); +INLINE struct rtp_ssrc_entry *find_ssrc(u_int32_t, GHashTable *); +INLINE void add_ssrc_entry(struct rtp_ssrc_entry *, GHashTable *); +INLINE struct rtp_ssrc_entry *create_ssrc_entry(u_int32_t, u_int64_t); +INLINE void free_ssrc_table(GHashTable **); +INLINE GHashTable *create_ssrc_table(void); INLINE int crypto_encrypt_rtp(struct crypto_context *c, struct rtp_header *rtp, @@ -198,4 +199,31 @@ INLINE int crypto_params_cmp(const struct crypto_params *a, const struct crypto_ +INLINE struct rtp_ssrc_entry *find_ssrc(u_int32_t ssrc, GHashTable *ht) { + return g_hash_table_lookup(ht, &ssrc); +} +INLINE void add_ssrc_entry(struct rtp_ssrc_entry *ent, GHashTable *ht) { + g_hash_table_insert(ht, &ent->ssrc, ent); +} +INLINE struct rtp_ssrc_entry *create_ssrc_entry(u_int32_t ssrc, u_int64_t index) { + struct rtp_ssrc_entry *ent; + ent = g_slice_alloc(sizeof(struct rtp_ssrc_entry)); + ent->ssrc = ssrc; + ent->index = index; + return ent; +} +INLINE void free_ssrc_table(GHashTable **ht) { + if (!*ht) + return; + g_hash_table_destroy(*ht); + *ht = NULL; +} +INLINE void free_ssrc_entry(void *p) { + g_slice_free1(sizeof(struct rtp_ssrc_entry), p); +} +INLINE GHashTable *create_ssrc_table(void) { + return g_hash_table_new_full(uint32_hash, uint32_eq, free_ssrc_entry, NULL); +} + + #endif From 39392e2b983e167bdae5133c7784236c39149838 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Wed, 13 May 2015 12:07:59 -0400 Subject: [PATCH 09/11] fix ssrc in simulator-ng.pl --- tests/simulator-ng.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/simulator-ng.pl b/tests/simulator-ng.pl index 1885ddddd..32a4a38c9 100755 --- a/tests/simulator-ng.pl +++ b/tests/simulator-ng.pl @@ -275,9 +275,10 @@ sub rtcp_savpf { sub rtp { my ($ctx) = @_; + my $ssrc = $$ctx{ssrc} // ($$ctx{ssrc} = rand(2**32)); my $seq = $$ctx{rtp_seqnum}; defined($seq) or $seq = int(rand(0xfffe)) + 1; - my $hdr = pack("CCnNN", 0x80, 0x00, $seq, rand(2**32), rand(2**32)); + my $hdr = pack("CCnNN", 0x80, 0x00, $seq, rand(2**32), $ssrc); my $pack = $hdr . rand_str($PAYLOAD); $$ctx{rtp_seqnum} = (++$seq & 0xffff); return $pack; From 5d6604a816c285e7e411a0c7e72ea4261339d1c9 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Wed, 13 May 2015 13:09:03 -0400 Subject: [PATCH 10/11] SSRC/ROC debug tools for simulator-ng --- tests/simulator-ng.pl | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/simulator-ng.pl b/tests/simulator-ng.pl index 32a4a38c9..3176c21ea 100755 --- a/tests/simulator-ng.pl +++ b/tests/simulator-ng.pl @@ -17,7 +17,8 @@ use SRTP; my ($NUM, $RUNTIME, $STREAMS, $PAYLOAD, $INTERVAL, $RTCP_INTERVAL, $STATS_INTERVAL) = (1000, 30, 1, 160, 20, 5, 5); -my ($NODEL, $IP, $IPV6, $KEEPGOING, $REINVITES, $PROTOS, $DEST, $SUITES, $NOENC, $RTCPMUX, $BUNDLE, $LAZY); +my ($NODEL, $IP, $IPV6, $KEEPGOING, $REINVITES, $PROTOS, $DEST, $SUITES, $NOENC, $RTCPMUX, $BUNDLE, $LAZY, + $CHANGE_SSRC); GetOptions( 'no-delete' => \$NODEL, 'num-calls=i' => \$NUM, @@ -38,6 +39,7 @@ GetOptions( 'rtcp-mux' => \$RTCPMUX, 'bundle' => \$BUNDLE, 'lazy-params' => \$LAZY, + 'change-ssrc' => \$CHANGE_SSRC, ) or die; ($IP || $IPV6) or die("at least one of --local-ip or --local-ipv6 must be given"); @@ -154,6 +156,7 @@ sub rtp_encrypt { my ($pkt, $roc) = SRTP::encrypt_rtp(@$dctx{qw(crypto_suite rtp_session_key rtp_session_salt rtp_session_auth_key rtp_roc rtp_mki rtp_mki_len unenc_srtp unauth_srtp)}, $r); + $roc == ($$dctx{rtp_roc} // 0) or print("ROC is now $roc\n"); $$dctx{rtp_roc} = $roc; $NOENC{rtp_packet} = $pkt; @@ -769,6 +772,21 @@ while (time() < $end) { offer($c, 0, 1); answer($c, 1, 0); } + + if ($CHANGE_SSRC && rand() < .001) { + my $c = $calls[rand(@calls)]; + my $s = $$c{sides}[rand(2)]; + my $st = rand($$s{num_streams}); + my $d = (qw(in out))[rand(2)]; + my $stc = $$s{trans_contexts}[$st]; + my $ct = $$stc{$d}; + if (defined($$ct{rtp_roc}) && $$stc{ssrc}) { + my $nssrc = rand(2 ** 32); + print("change SSRC from $$stc{ssrc} to $nssrc\n"); + $$stc{ssrc} = $nssrc; + $$ct{roc} = 0; + } + } } if (!$NODEL) { From 87fbbb85ecfbd9d261251a70adcbf985c92db6dc Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Wed, 13 May 2015 13:15:07 -0400 Subject: [PATCH 11/11] implement ssrc/roc tracking in encryption as well --- daemon/rtp.c | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/daemon/rtp.c b/daemon/rtp.c index 3b27ce915..561651879 100644 --- a/daemon/rtp.c +++ b/daemon/rtp.c @@ -176,26 +176,15 @@ void rtp_append_mki(str *s, struct crypto_context *c) { s->len += c->params.mki_len; } -/* rfc 3711, section 3.3 */ -int rtp_avp2savp(str *s, struct crypto_context *c) { - struct rtp_header *rtp; - str payload, to_auth; - u_int64_t index; +static int rtp_ssrc_check(const struct rtp_header *rtp, struct crypto_context *c) { struct rtp_ssrc_entry *cur_ssrc; - int update_kernel = 0; - - if (rtp_payload(&rtp, &payload, s)) - return -1; - if (check_session_keys(c)) - return -1; /* check last known SSRC */ if (G_LIKELY(rtp->ssrc == c->ssrc)) - goto ssrc_ok; + return 0; if (!c->ssrc) { c->ssrc = rtp->ssrc; - update_kernel = 1; - goto ssrc_ok; + return 1; } /* SSRC mismatch. stash away last know info */ @@ -221,9 +210,22 @@ int rtp_avp2savp(str *s, struct crypto_context *c) { else c->last_index = cur_ssrc->index; - update_kernel = 1; + return 1; +} + +/* rfc 3711, section 3.3 */ +int rtp_avp2savp(str *s, struct crypto_context *c) { + struct rtp_header *rtp; + str payload, to_auth; + u_int64_t index; + int ret = 0; + + if (rtp_payload(&rtp, &payload, s)) + return -1; + if (check_session_keys(c)) + return -1; -ssrc_ok: + ret = rtp_ssrc_check(rtp, c); index = packet_index(c, rtp); /* rfc 3711 section 3.1 */ @@ -239,7 +241,7 @@ ssrc_ok: s->len += c->params.crypto_suite->srtp_auth_tag; } - return update_kernel ? 1 : 0; + return ret; } /* rfc 3711, section 3.3 */ @@ -248,12 +250,14 @@ int rtp_savp2avp(str *s, struct crypto_context *c) { u_int64_t index; str payload, to_auth, to_decrypt, auth_tag; char hmac[20]; + int ret = 0; if (rtp_payload(&rtp, &payload, s)) return -1; if (check_session_keys(c)) return -1; + ret = rtp_ssrc_check(rtp, c); index = packet_index(c, rtp); if (srtp_payloads(&to_auth, &to_decrypt, &auth_tag, NULL, c->params.session_params.unauthenticated_srtp ? 0 : c->params.crypto_suite->srtp_auth_tag, @@ -297,7 +301,7 @@ decrypt: *s = to_auth; - return 0; + return ret; error: ilog(LOG_WARNING | LOG_FLAG_LIMIT, "Discarded invalid SRTP packet: authentication failed");