--- res_rtp_asterisk.c.orig 2015-02-25 14:47:25.772839327 +0100 +++ res/res_rtp_asterisk.c 2015-03-01 20:16:55.511645072 +0100 @@ -204,6 +204,7 @@ #ifdef HAVE_OPENSSL_SRTP struct dtls_details { + SSL *ssl; /*!< SSL session */ BIO *read_bio; /*!< Memory buffer for reading */ BIO *write_bio; /*!< Memory buffer for writing */ @@ -1202,6 +1203,7 @@ enum ast_rtp_dtls_setup setup) { dtls->dtls_setup = setup; + if (!(dtls->ssl = SSL_new(ssl_ctx))) { ast_log(LOG_ERROR, "Failed to allocate memory for SSL\n"); @@ -1232,6 +1234,9 @@ return 0; error: + ast_log(LOG_ERROR, "UNEXPECTED error occured in dtls_details_initialize, for dtls %p, dtls->ssl %p \n",dtls,dtls->ssl); + /* TODO, what's best here, as in patch from ASTERISK-24651 or as before? */ + if (dtls->read_bio) { BIO_free(dtls->read_bio); dtls->read_bio = NULL; @@ -1400,11 +1405,13 @@ } if (rtp->dtls.ssl) { + /* TODO, what's best here, as in patch from ASTERISK-24651 or as before? */ SSL_free(rtp->dtls.ssl); rtp->dtls.ssl = NULL; } if (rtp->rtcp && rtp->rtcp->dtls.ssl) { + /* TODO, what's best here, as in patch from ASTERISK-24651 or as before? */ SSL_free(rtp->rtcp->dtls.ssl); rtp->rtcp->dtls.ssl = NULL; } @@ -1475,9 +1482,13 @@ } if (*dtls_setup == AST_RTP_DTLS_SETUP_ACTIVE) { + SSL_set_connect_state(ssl); + } else if (*dtls_setup == AST_RTP_DTLS_SETUP_PASSIVE) { + SSL_set_accept_state(ssl); + } else { return; } @@ -1597,7 +1608,9 @@ dtls->connection = AST_RTP_DTLS_CONNECTION_NEW; } SSL_do_handshake(dtls->ssl); + dtls_srtp_check_pending(instance, rtp, rtcp); + } #endif @@ -1609,6 +1622,22 @@ struct ast_rtp_instance *instance = ice->user_data; struct ast_rtp *rtp = ast_rtp_instance_get_data(instance); +#ifdef HAVE_OPENSSL_SRTP + + + int count=0; + /*quickfix - this gives chan_sip at least 1 more second to call dtls_set_setup */ + /* TODO what if AST_RTP_DTLS_SETUP_ACTPASS never changes - just return? */ + while (( (rtp->dtls.dtls_setup == AST_RTP_DTLS_SETUP_ACTPASS) || (rtp->rtcp && rtp->rtcp->dtls.dtls_setup == AST_RTP_DTLS_SETUP_ACTPASS) ) && count < 10) { + ast_log(LOG_ERROR, "UNEXPECTED loop %d on ice complete for dtls %p \n",count,&rtp->dtls); + usleep(100000); + count++; + if (count==10) { + return; + } + } + +#endif if (status == PJ_SUCCESS) { struct ast_sockaddr remote_address; @@ -1621,14 +1650,26 @@ if (rtp->rtcp) { update_address_with_ice_candidate(rtp, AST_RTP_ICE_COMPONENT_RTCP, &rtp->rtcp->them); } - } + } -#ifdef HAVE_OPENSSL_SRTP - dtls_perform_handshake(instance, &rtp->dtls, 0); +#ifdef HAVE_OPENSSL_SRTP + if (&rtp->dtls && (rtp->dtls.dtls_setup != AST_RTP_DTLS_SETUP_PASSIVE)) { + /* ASSUMPTION the rtp->dtls->dtls_setup is passive if and only if rtcp->dtls->dtls_setup is passive too? */ + /* some other thread might do __rtp_rcvfrom at this instant, we might need mutexes? */ + + dtls_perform_handshake(instance, &rtp->dtls, 0); + + + if (rtp->rtcp) { + + dtls_perform_handshake(instance, &rtp->rtcp->dtls, 1); + + } + } else { + /* Let's not call perform_handshake for either rtcp or rtp if we are passive, not from this on_ice_complete callback anyways?*/ + } + - if (rtp->rtcp) { - dtls_perform_handshake(instance, &rtp->rtcp->dtls, 1); - } #endif if (!strictrtp) { @@ -1757,6 +1798,8 @@ struct ast_rtp_instance *instance = (struct ast_rtp_instance *)data; struct ast_rtp *rtp = ast_rtp_instance_get_data(instance); + /* TODO, potentially calling DTLSv1_handle_timeout(rtp->dtls.ssl) without some concurrency handling is dangerous? */ + /* TODO, what's best here, as in patch from ASTERISK-24651 or as before? */ if (!rtp) { return 0; @@ -1772,17 +1815,24 @@ rtp->dtlstimerid = -1; ast_mutex_unlock(&rtp->dtls_timer_lock); - if (rtp->dtls.ssl && !SSL_is_init_finished(rtp->dtls.ssl)) { DTLSv1_handle_timeout(rtp->dtls.ssl); + } + dtls_srtp_check_pending(instance, rtp, 0); + if (rtp->rtcp && rtp->rtcp->dtls.ssl && !SSL_is_init_finished(rtp->rtcp->dtls.ssl)) { DTLSv1_handle_timeout(rtp->rtcp->dtls.ssl); + } - dtls_srtp_check_pending(instance, rtp, 1); + if (rtp->rtcp && rtp->rtcp->dtls.ssl) { + + dtls_srtp_check_pending(instance, rtp, 1); + + } ao2_ref(instance, -1); return 0; @@ -1797,7 +1847,7 @@ if (!dtls->ssl || !dtls->write_bio) { return; } - + pending = BIO_ctrl_pending(dtls->write_bio); if (pending > 0) { @@ -1827,7 +1877,7 @@ } if (DTLSv1_get_timeout(dtls->ssl, &dtls_timeout)) { - int timeout = dtls_timeout.tv_sec * 1000 + dtls_timeout.tv_usec / 1000; + int timeout = dtls_timeout.tv_sec * 1000 + dtls_timeout.tv_usec / 1000; /* TODO maybe this time is too short and we will get many DTLS-failures? */ ao2_ref(instance, +1); if ((rtp->dtlstimerid = ast_sched_add(rtp->sched, timeout, dtls_srtp_handle_timeout, instance)) < 0) { ao2_ref(instance, -1); @@ -1842,6 +1892,7 @@ static int dtls_srtp_renegotiate(const void *data) { + ast_log(LOG_NOTICE, "Doing srtp_renegotiate \n"); struct ast_rtp_instance *instance = (struct ast_rtp_instance *)data; struct ast_rtp *rtp = ast_rtp_instance_get_data(instance); @@ -1969,7 +2020,7 @@ res_srtp_policy->set_ssrc(remote_policy, 0, 1); if (ast_rtp_instance_add_srtp_policy(instance, remote_policy, local_policy)) { - ast_log(LOG_WARNING, "Could not set policies when setting up DTLS-SRTP on '%p'\n", rtp); + ast_log(LOG_WARNING, "Could not set policies when setting up DTLS-SRTP on '%p'\n", rtp); goto error; } @@ -2009,7 +2060,11 @@ } #ifdef HAVE_OPENSSL_SRTP - dtls_srtp_check_pending(instance, rtp, rtcp); + /* TODO, what's best here, as in patch from ASTERISK-24651 or as before? This function could be called by thread A at the same time as thread B calls dtls_perform_handshake due to ice callback (at least before this patch was applied). Moreover, thread C could potentially call dtls_srtp_check_pending(instance, rtp, rtcp) concurrently from dtls_srtp_handle_timeout ?*/ + /* TODO, Why did ASTERISK-24651-patch use a special mutex lock protecting all calls to dtls_srtp_check_pending? When I tried this I seemed to get deadlocks...*/ + + dtls_srtp_check_pending(instance, rtp, rtcp); + /* If this is an SSL packet pass it to OpenSSL for processing */ if ((*in >= 20) && (*in <= 64)) { @@ -2029,27 +2084,40 @@ SSL_set_accept_state(dtls->ssl); } - dtls_srtp_check_pending(instance, rtp, rtcp); + + + dtls_srtp_check_pending(instance, rtp, rtcp); + + + BIO_write(dtls->read_bio, buf, len); len = SSL_read(dtls->ssl, buf, len); + if ((len < 0) && (SSL_get_error(dtls->ssl, len) == SSL_ERROR_SSL)) { unsigned long error = ERR_get_error(); - ast_log(LOG_ERROR, "DTLS failure occurred on RTP instance '%p' due to reason '%s', terminating\n", + ast_log(LOG_ERROR, "UNEXPECTED DTLS failure occurred on RTP instance '%p' due to reason '%s', terminating\n", instance, ERR_reason_error_string(error)); return -1; } - dtls_srtp_check_pending(instance, rtp, rtcp); + + + dtls_srtp_check_pending(instance, rtp, rtcp); + if (SSL_is_init_finished(dtls->ssl)) { /* Any further connections will be existing since this is now established */ dtls->connection = AST_RTP_DTLS_CONNECTION_EXISTING; if (!rtcp) { /* Use the keying material to set up key/salt information */ + + res = dtls_srtp_setup(rtp, srtp, instance); + + } } @@ -2484,7 +2552,7 @@ } static int ast_rtp_destroy(struct ast_rtp_instance *instance) -{ +{ struct ast_rtp *rtp = ast_rtp_instance_get_data(instance); #ifdef HAVE_PJPROJECT struct timeval wait = ast_tvadd(ast_tvnow(), ast_samp2tv(TURN_STATE_WAIT_TIME, 1000)); @@ -2511,8 +2579,12 @@ close(rtp->rtcp->s); #ifdef HAVE_OPENSSL_SRTP if (rtp->rtcp->dtls.ssl) { - SSL_free(rtp->rtcp->dtls.ssl); + /* TODO, what's best here, as in patch from ASTERISK-24651 or as before? */ + + SSL_free(rtp->rtcp->dtls.ssl); + } + #endif ast_free(rtp->rtcp); } @@ -2567,14 +2639,19 @@ #ifdef HAVE_OPENSSL_SRTP /* Destroy the SSL context if present */ + if (rtp->ssl_ctx) { SSL_CTX_free(rtp->ssl_ctx); } /* Destroy the SSL session if present */ if (rtp->dtls.ssl) { + /* TODO, what's best here, as in patch from ASTERISK-24651 or as before? */ SSL_free(rtp->dtls.ssl); } + + + #endif ao2_cleanup(rtp->lasttxformat); @@ -2583,6 +2660,7 @@ /* Destroy synchronization items */ ast_mutex_destroy(&rtp->lock); + ast_cond_destroy(&rtp->cond); /* Finally destroy ourselves */ @@ -4688,7 +4766,7 @@ #endif return; - } else { + } else { if (rtp->rtcp) { if (rtp->rtcp->schedid > 0) { if (!ast_sched_del(rtp->sched, rtp->rtcp->schedid)) { @@ -4703,8 +4781,11 @@ } close(rtp->rtcp->s); #ifdef HAVE_OPENSSL_SRTP - if (rtp->rtcp->dtls.ssl) { + if (rtp->rtcp->dtls.ssl) { + /* TODO, what's best here, as in patch from ASTERISK-24651 or as before? */ + SSL_free(rtp->rtcp->dtls.ssl); + } #endif ast_free(rtp->rtcp); @@ -4989,7 +5070,7 @@ struct ast_rtp *rtp = ast_rtp_instance_get_data(instance); /* If ICE negotiation is enabled the DTLS Handshake will be performed upon completion of it */ -#ifdef USE_PJPROJECT +#ifdef HAVE_PJPROJECT if (rtp->ice) { return 0; }