From e84fd8612ba49f151ad0a904e94f7236ddd38ce8 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Fri, 9 Feb 2018 14:44:35 -0500 Subject: [PATCH] fix incorrect DTLS context being used with ICE fixes #451 Change-Id: I4db956bd9e8234862a7dd3ee492a6ed2778150d3 --- daemon/call.c | 55 +++++++++++------------ daemon/call_interfaces.c | 3 -- daemon/dtls.c | 95 +++++++++++++++++++++++++++------------- daemon/dtls.h | 6 ++- daemon/ice.c | 1 + daemon/media_socket.c | 2 +- 6 files changed, 100 insertions(+), 62 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index d2dcc9cc2..56ae9b511 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -930,8 +930,9 @@ enum call_stream_state call_stream_state_machine(struct packet_stream *ps) { if (MEDIA_ISSET(media, DTLS)) { mutex_lock(&ps->in_lock); - if (ps->selected_sfd->dtls.init && !ps->selected_sfd->dtls.connected) { - dtls(ps, NULL, NULL); + struct stream_fd *sfd = dtls_sfd(ps); + if (sfd->dtls.init && !sfd->dtls.connected) { + dtls(sfd, NULL, NULL); mutex_unlock(&ps->in_lock); return CSS_DTLS; } @@ -953,34 +954,34 @@ static int __init_stream(struct packet_stream *ps) { struct call *call = ps->call; int active; - if (ps->selected_sfd) { - // XXX apply SDES parms to all sfds? - if (MEDIA_ISSET(media, SDES)) - crypto_init(&ps->selected_sfd->crypto, &media->sdes_in.params); - - if (MEDIA_ISSET(media, DTLS) && !PS_ISSET(ps, FALLBACK_RTCP)) { - active = dtls_is_active(&ps->selected_sfd->dtls); - // we try to retain our role if possible, but must handle a role switch - if ((active && !MEDIA_ISSET(media, SETUP_ACTIVE)) - || (!active && !MEDIA_ISSET(media, SETUP_PASSIVE))) - active = -1; - if (active == -1) - active = (PS_ISSET(ps, FILLED) && MEDIA_ISSET(media, SETUP_ACTIVE)); - dtls_connection_init(ps, active, call->dtls_cert); - - if (!PS_ISSET(ps, FINGERPRINT_VERIFIED) && media->fingerprint.hash_func - && ps->dtls_cert) - { - if (dtls_verify_cert(ps)) - return -1; - } - - call_stream_state_machine(ps); + if (MEDIA_ISSET(media, SDES)) { + for (GList *l = ps->sfds.head; l; l = l->next) { + struct stream_fd *sfd = l->data; + crypto_init(&sfd->crypto, &media->sdes_in.params); } + crypto_init(&ps->crypto, &media->sdes_out.params); } - if (MEDIA_ISSET(media, SDES)) - crypto_init(&ps->crypto, &media->sdes_out.params); + if (MEDIA_ISSET(media, DTLS) && !PS_ISSET(ps, FALLBACK_RTCP)) { + struct stream_fd *sfd = dtls_sfd(ps); + active = dtls_is_active(&sfd->dtls); + // we try to retain our role if possible, but must handle a role switch + if ((active && !MEDIA_ISSET(media, SETUP_ACTIVE)) + || (!active && !MEDIA_ISSET(media, SETUP_PASSIVE))) + active = -1; + if (active == -1) + active = (PS_ISSET(ps, FILLED) && MEDIA_ISSET(media, SETUP_ACTIVE)); + dtls_connection_init(ps, active, call->dtls_cert); + + if (!PS_ISSET(ps, FINGERPRINT_VERIFIED) && media->fingerprint.hash_func + && ps->dtls_cert) + { + if (dtls_verify_cert(ps)) + return -1; + } + + call_stream_state_machine(ps); + } return 0; } diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index 4895e1cd5..b644f4f66 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -686,8 +686,6 @@ static void call_ng_process_flags(struct sdp_ng_flags *out, bencode_item_t *inpu } call_ng_flags_list(out, input, "rtcp-mux", call_ng_flags_rtcp_mux, NULL); - - /* XXX module still needs to support this list */ call_ng_flags_list(out, input, "SDES", ng_sdes_option, NULL); bencode_get_alt(input, "transport-protocol", "transport protocol", &out->transport_protocol_str); @@ -701,7 +699,6 @@ static void call_ng_process_flags(struct sdp_ng_flags *out, bencode_item_t *inpu out->ptime = bencode_dictionary_get_int_str(input, "ptime", 0); if ((dict = bencode_dictionary_get_expect(input, "codec", BENCODE_DICTIONARY))) { - /* XXX module still needs to support these */ call_ng_flags_list(out, dict, "strip", call_ng_flags_codec_ht, out->codec_strip); call_ng_flags_list(out, dict, "offer", call_ng_flags_codec_list, &out->codec_offer); #ifdef WITH_TRANSCODING diff --git a/daemon/dtls.c b/daemon/dtls.c index 3915b39fe..669ff7217 100644 --- a/daemon/dtls.c +++ b/daemon/dtls.c @@ -35,6 +35,32 @@ #define CERT_EXPIRY_TIME (60*60*24*30) /* 30 days */ +INLINE struct stream_fd *dtls_primary(struct packet_stream *ps) { + if (!ps->sfds.length) + return NULL; + return ps->sfds.head->data; +} +// determine the sfd to hold our DTLS context if we don't know the sfd. +// it's either the "selected_sfd" for regular multi-homed streams, or +// the first sfd in the list in case ICE is in use +struct stream_fd *dtls_sfd(struct packet_stream *ps) { + if (!ps) + return NULL; + if (PS_ISSET(ps, ICE)) + return dtls_primary(ps); + return ps->selected_sfd; +} +// determine the DTLS context if we do have an sfd. can be sfd->dtls, +// or in case ICE is in use, the first sfd's context. +struct dtls_connection *dtls_ptr(struct stream_fd *sfd) { + if (!sfd) + return NULL; + struct packet_stream *ps = sfd->stream; + if (PS_ISSET(ps, ICE)) // ignore which sfd we were given + sfd = dtls_primary(ps); + return &sfd->dtls; +} + @@ -477,13 +503,13 @@ int dtls_connection_init(struct packet_stream *ps, int active, struct dtls_cert struct dtls_connection *d; unsigned long err; - if (!ps || !ps->selected_sfd) + struct stream_fd *sfd = dtls_sfd(ps); + if (!sfd) return 0; + d = &sfd->dtls; __DBG("dtls_connection_init(%i)", active); - d = &ps->selected_sfd->dtls; - if (d->init) { if ((d->active && active) || (!d->active && !active)) goto done; @@ -522,7 +548,7 @@ int dtls_connection_init(struct packet_stream *ps, int active, struct dtls_cert if (!d->r_bio || !d->w_bio) goto error; - SSL_set_app_data(d->ssl, ps->selected_sfd); /* XXX obj reference here? */ + SSL_set_app_data(d->ssl, sfd); /* XXX obj reference here? */ SSL_set_bio(d->ssl, d->r_bio, d->w_bio); d->init = 1; SSL_set_mode(d->ssl, SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); @@ -618,15 +644,18 @@ found: ilog(LOG_INFO, "DTLS-SRTP successfully negotiated"); - if (d->active) { - /* we're the client */ - crypto_init(&ps->crypto, &client); - crypto_init(&ps->selected_sfd->crypto, &server); - } - else { - /* we're the server */ - crypto_init(&ps->crypto, &server); - crypto_init(&ps->selected_sfd->crypto, &client); + for (GList *l = ps->sfds.head; l; l = l->next) { + struct stream_fd *sfd = l->data; + if (d->active) { + /* we're the client */ + crypto_init(&ps->crypto, &client); + crypto_init(&sfd->crypto, &server); + } + else { + /* we're the server */ + crypto_init(&ps->crypto, &server); + crypto_init(&sfd->crypto, &client); + } } crypto_dump_keys(&ps->crypto, &ps->selected_sfd->crypto); @@ -643,17 +672,18 @@ error: } /* called with call locked in W or R with ps->in_lock held */ -int dtls(struct packet_stream *ps, const str *s, const endpoint_t *fsin) { - struct dtls_connection *d; +int dtls(struct stream_fd *sfd, const str *s, const endpoint_t *fsin) { + struct packet_stream *ps = sfd->stream; int ret; unsigned char buf[0x10000]; - if (!ps || !ps->selected_sfd) + if (!ps) return 0; if (!MEDIA_ISSET(ps->media, DTLS)) return 0; - - d = &ps->selected_sfd->dtls; + struct dtls_connection *d = dtls_ptr(sfd); + if (!d) + return 0; if (s) __DBG("dtls packet input: len %u %02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x", @@ -675,7 +705,7 @@ int dtls(struct packet_stream *ps, const str *s, const endpoint_t *fsin) { ret = try_connect(d); if (ret == -1) { - ilog(LOG_ERROR, "DTLS error on local port %u", ps->selected_sfd->socket.local.port); + ilog(LOG_ERROR, "DTLS error on local port %u", sfd->socket.local.port); /* fatal error */ dtls_connection_cleanup(d); return 0; @@ -727,7 +757,7 @@ int dtls(struct packet_stream *ps, const str *s, const endpoint_t *fsin) { fsin = &ps->endpoint; ilog(LOG_DEBUG, "Sending DTLS packet"); - socket_sendto(&ps->selected_sfd->socket, buf, ret, fsin); + socket_sendto(&sfd->socket, buf, ret, fsin); } return 0; @@ -735,23 +765,29 @@ int dtls(struct packet_stream *ps, const str *s, const endpoint_t *fsin) { /* call must be locked */ void dtls_shutdown(struct packet_stream *ps) { - struct dtls_connection *d; - if (!ps || !ps->selected_sfd) + if (!ps) return; __DBG("dtls_shutdown"); - d = &ps->selected_sfd->dtls; - if (!d->init) - return; + for (GList *l = ps->sfds.head; l; l = l->next) { + struct stream_fd *sfd = l->data; + + struct dtls_connection *d = &sfd->dtls; + if (!d->init) + continue; + + if (d->connected && d->ssl) { + SSL_shutdown(d->ssl); + dtls(sfd, NULL, &ps->endpoint); + } + + dtls_connection_cleanup(d); - if (d->connected && d->ssl) { - SSL_shutdown(d->ssl); - dtls(ps, NULL, &ps->endpoint); + crypto_reset(&sfd->crypto); } - dtls_connection_cleanup(d); if (ps->dtls_cert) { X509_free(ps->dtls_cert); @@ -759,7 +795,6 @@ void dtls_shutdown(struct packet_stream *ps) { } crypto_reset(&ps->crypto); - crypto_reset(&ps->selected_sfd->crypto); } void dtls_connection_cleanup(struct dtls_connection *c) { diff --git a/daemon/dtls.h b/daemon/dtls.h index d546310c7..68b662826 100644 --- a/daemon/dtls.h +++ b/daemon/dtls.h @@ -66,7 +66,7 @@ const struct dtls_hash_func *dtls_find_hash_func(const str *); struct dtls_cert *dtls_cert(void); int dtls_connection_init(struct packet_stream *, int active, struct dtls_cert *cert); -int dtls(struct packet_stream *, const str *s, const endpoint_t *sin); +int dtls(struct stream_fd *, const str *s, const endpoint_t *sin); void dtls_connection_cleanup(struct dtls_connection *); void dtls_shutdown(struct packet_stream *ps); @@ -109,6 +109,10 @@ INLINE int dtls_is_active(const struct dtls_connection *d) { } +struct stream_fd *dtls_sfd(struct packet_stream *ps); +struct dtls_connection *dtls_ptr(struct stream_fd *sfd); + + #endif diff --git a/daemon/ice.c b/daemon/ice.c index 7238f562e..a4dfa4fee 100644 --- a/daemon/ice.c +++ b/daemon/ice.c @@ -1055,6 +1055,7 @@ static int __check_valid(struct ice_agent *ag) { if (ps->component == 1) ilog(LOG_INFO, "ICE negotiated: local interface %s", sockaddr_print_buf(&pair->local_intf->spec->local_address.addr)); + break; } } diff --git a/daemon/media_socket.c b/daemon/media_socket.c index ac46bf6c6..161f05558 100644 --- a/daemon/media_socket.c +++ b/daemon/media_socket.c @@ -1183,7 +1183,7 @@ static void __stream_ssrc(struct packet_stream *in_srtp, struct packet_stream *o static int media_demux_protocols(struct packet_handler_ctx *phc) { if (MEDIA_ISSET(phc->mp.media, DTLS) && is_dtls(&phc->s)) { mutex_lock(&phc->mp.stream->in_lock); - int ret = dtls(phc->mp.stream, &phc->s, &phc->mp.fsin); + int ret = dtls(phc->mp.sfd, &phc->s, &phc->mp.fsin); mutex_unlock(&phc->mp.stream->in_lock); if (!ret) return 0;