From 41f0390f931534f9ae4872816e1449c354b79bf5 Mon Sep 17 00:00:00 2001 From: Simon Morlat Date: Wed, 27 Nov 2013 16:48:57 +0100 Subject: [PATCH] generate SRTP keys more securely simplify management of SalMediaDescription take into account changes in local encryption keys while updating streams. --- coreapi/bellesip_sal/sal_impl.c | 5 ++++ coreapi/callbacks.c | 42 ++++++--------------------- coreapi/linphonecall.c | 50 ++++++++++++++++++--------------- coreapi/private.h | 2 ++ include/sal/sal.h | 6 +++- 5 files changed, 47 insertions(+), 58 deletions(-) diff --git a/coreapi/bellesip_sal/sal_impl.c b/coreapi/bellesip_sal/sal_impl.c index 80f577490..e69359e9c 100644 --- a/coreapi/bellesip_sal/sal_impl.c +++ b/coreapi/bellesip_sal/sal_impl.c @@ -890,3 +890,8 @@ void sal_signing_key_parse_file(SalAuthInfo* auth_info, const char* path, const auth_info->key = (SalSigningKey *) belle_sip_signing_key_parse_file(path, passwd); if (auth_info->key) belle_sip_object_ref((belle_sip_object_t *) auth_info->key); } + +unsigned char * sal_get_random_bytes(unsigned char *ret, size_t size){ + return belle_sip_random_bytes(ret,size); +} + diff --git a/coreapi/callbacks.c b/coreapi/callbacks.c index ee125717f..183371130 100644 --- a/coreapi/callbacks.c +++ b/coreapi/callbacks.c @@ -37,24 +37,16 @@ static void register_failure(SalOp *op, SalError error, SalReason reason, const static int media_parameters_changed(LinphoneCall *call, SalMediaDescription *oldmd, SalMediaDescription *newmd) { if (call->params.in_conference != call->current_params.in_conference) return SAL_MEDIA_DESCRIPTION_CHANGED; if (call->up_bw != linphone_core_get_upload_bandwidth(call->core)) return SAL_MEDIA_DESCRIPTION_CHANGED; - return sal_media_description_equals(oldmd, newmd); + if (call->localdesc_changed) ms_message("Local description has changed: %i", call->localdesc_changed); + return call->localdesc_changed | sal_media_description_equals(oldmd, newmd); } void linphone_core_update_streams_destinations(LinphoneCore *lc, LinphoneCall *call, SalMediaDescription *old_md, SalMediaDescription *new_md) { - SalStreamDescription *old_audiodesc = NULL; - SalStreamDescription *old_videodesc = NULL; SalStreamDescription *new_audiodesc = NULL; SalStreamDescription *new_videodesc = NULL; char *rtp_addr, *rtcp_addr; int i; - for (i = 0; i < old_md->n_active_streams; i++) { - if (old_md->streams[i].type == SalAudio) { - old_audiodesc = &old_md->streams[i]; - } else if (old_md->streams[i].type == SalVideo) { - old_videodesc = &old_md->streams[i]; - } - } for (i = 0; i < new_md->n_active_streams; i++) { if (new_md->streams[i].type == SalAudio) { new_audiodesc = &new_md->streams[i]; @@ -76,21 +68,6 @@ void linphone_core_update_streams_destinations(LinphoneCore *lc, LinphoneCall *c rtp_session_set_remote_addr_full(call->videostream->ms.session, rtp_addr, new_videodesc->rtp_port, rtcp_addr, new_videodesc->rtcp_port); } #endif - - /* Copy address and port values from new_md to old_md since we will keep old_md as resultdesc */ - strcpy(old_md->addr, new_md->addr); - if (old_audiodesc && new_audiodesc) { - strcpy(old_audiodesc->rtp_addr, new_audiodesc->rtp_addr); - strcpy(old_audiodesc->rtcp_addr, new_audiodesc->rtcp_addr); - old_audiodesc->rtp_port = new_audiodesc->rtp_port; - old_audiodesc->rtcp_port = new_audiodesc->rtcp_port; - } - if (old_videodesc && new_videodesc) { - strcpy(old_videodesc->rtp_addr, new_videodesc->rtp_addr); - strcpy(old_videodesc->rtcp_addr, new_videodesc->rtcp_addr); - old_videodesc->rtp_port = new_videodesc->rtp_port; - old_videodesc->rtcp_port = new_videodesc->rtcp_port; - } } void linphone_core_update_streams(LinphoneCore *lc, LinphoneCall *call, SalMediaDescription *new_md){ @@ -112,9 +89,6 @@ void linphone_core_update_streams(LinphoneCore *lc, LinphoneCall *call, SalMedia ms_message("Media descriptions are different, need to restart the streams."); } else { if (md_changed == SAL_MEDIA_DESCRIPTION_UNCHANGED) { - /*as nothing has changed, keep the oldmd */ - call->resultdesc=oldmd; - sal_media_description_unref(new_md); if (call->all_muted){ ms_message("Early media finished, unmuting inputs..."); /*we were in early media, now we want to enable real media */ @@ -127,7 +101,7 @@ void linphone_core_update_streams(LinphoneCore *lc, LinphoneCall *call, SalMedia #endif } ms_message("No need to restart streams, SDP is unchanged."); - return; + goto end; }else { if (md_changed & SAL_MEDIA_DESCRIPTION_NETWORK_CHANGED) { ms_message("Network parameters have changed, update them."); @@ -137,17 +111,13 @@ void linphone_core_update_streams(LinphoneCore *lc, LinphoneCall *call, SalMedia ms_message("Crypto parameters have changed, update them."); linphone_call_update_crypto_parameters(call, oldmd, new_md); } - call->resultdesc = oldmd; - sal_media_description_unref(new_md); - return; + goto end; } } } linphone_call_stop_media_streams (call); linphone_call_init_media_streams (call); } - if (oldmd) - sal_media_description_unref(oldmd); if (new_md) { bool_t all_muted=FALSE; @@ -169,6 +139,10 @@ void linphone_core_update_streams(LinphoneCore *lc, LinphoneCall *call, SalMedia if (call->state==LinphoneCallPausing && call->paused_by_app && ms_list_size(lc->calls)==1){ linphone_core_play_named_tone(lc,LinphoneToneCallOnHold); } + end: + if (oldmd) + sal_media_description_unref(oldmd); + } #if 0 static bool_t is_duplicate_call(LinphoneCore *lc, const LinphoneAddress *from, const LinphoneAddress *to){ diff --git a/coreapi/linphonecall.c b/coreapi/linphonecall.c index 6786209cd..82274f22d 100644 --- a/coreapi/linphonecall.c +++ b/coreapi/linphonecall.c @@ -43,24 +43,34 @@ static MSWebCam *get_nowebcam_device(){ } #endif -static bool_t generate_b64_crypto_key(int key_length, char* key_out) { +static bool_t generate_b64_crypto_key(int key_length, char* key_out, size_t key_out_size) { int b64_size; - uint8_t* tmp = (uint8_t*) malloc(key_length); - if (ortp_crypto_get_random(tmp, key_length)!=0) { + uint8_t* tmp = (uint8_t*) ms_malloc0(key_length); + if (sal_get_random_bytes(tmp, key_length)==NULL) { ms_error("Failed to generate random key"); - free(tmp); + ms_free(tmp); return FALSE; } b64_size = b64_encode((const char*)tmp, key_length, NULL, 0); + if (b64_size == 0) { + ms_error("Failed to get b64 result size"); + ms_free(tmp); + return FALSE; + } + if (b64_size>=key_out_size){ + ms_error("Insufficient room for writing base64 SRTP key"); + ms_free(tmp); + return FALSE; + } + b64_size=b64_encode((const char*)tmp, key_length, key_out, key_out_size); if (b64_size == 0) { ms_error("Failed to b64 encode key"); - free(tmp); + ms_free(tmp); return FALSE; } key_out[b64_size] = '\0'; - b64_encode((const char*)tmp, key_length, key_out, 40); - free(tmp); + ms_free(tmp); return TRUE; } @@ -293,17 +303,18 @@ void linphone_call_make_local_media_description(LinphoneCore *lc, LinphoneCall * if (md->streams[i].proto == SalProtoRtpSavp) { if (keep_srtp_keys && old_md && old_md->streams[i].proto==SalProtoRtpSavp){ int j; + ms_message("Keeping same crypto keys."); for(j=0;jstreams[i].crypto[j],&old_md->streams[i].crypto[j],sizeof(SalSrtpCryptoAlgo)); } }else{ md->streams[i].crypto[0].tag = 1; md->streams[i].crypto[0].algo = AES_128_SHA1_80; - if (!generate_b64_crypto_key(30, md->streams[i].crypto[0].master_key)) + if (!generate_b64_crypto_key(30, md->streams[i].crypto[0].master_key, SAL_SRTP_KEY_SIZE)) md->streams[i].crypto[0].algo = 0; md->streams[i].crypto[1].tag = 2; md->streams[i].crypto[1].algo = AES_128_SHA1_32; - if (!generate_b64_crypto_key(30, md->streams[i].crypto[1].master_key)) + if (!generate_b64_crypto_key(30, md->streams[i].crypto[1].master_key, SAL_SRTP_KEY_SIZE)) md->streams[i].crypto[1].algo = 0; md->streams[i].crypto[2].algo = 0; } @@ -322,7 +333,10 @@ void linphone_call_make_local_media_description(LinphoneCore *lc, LinphoneCall * #endif //BUILD_UPNP linphone_address_destroy(addr); call->localdesc=md; - if (old_md) sal_media_description_unref(old_md); + if (old_md){ + call->localdesc_changed=sal_media_description_equals(md,old_md); + sal_media_description_unref(old_md); + } } static int find_port_offset(LinphoneCore *lc, SalStreamType type){ @@ -1538,6 +1552,9 @@ static RtpProfile *make_profile(LinphoneCall *call, const SalMediaDescription *m for(elem=desc->payloads;elem!=NULL;elem=elem->next){ PayloadType *pt=(PayloadType*)elem->data; int number; + /* make a copy of the payload type, so that we left the ones from the SalStreamDescription unchanged. + If the SalStreamDescription is freed, this will have no impact on the running streams*/ + pt=payload_type_clone(pt); if ((pt->flags & PAYLOAD_TYPE_FLAG_CAN_SEND) && first) { if (desc->type==SalAudio){ @@ -1926,7 +1943,6 @@ void linphone_call_stop_media_streams_for_ice_gathering(LinphoneCall *call){ void linphone_call_update_crypto_parameters(LinphoneCall *call, SalMediaDescription *old_md, SalMediaDescription *new_md) { SalStreamDescription *old_stream; SalStreamDescription *new_stream; - int i; old_stream = sal_media_description_find_stream(old_md, SalProtoRtpSavp, SalAudio); new_stream = sal_media_description_find_stream(new_md, SalProtoRtpSavp, SalAudio); @@ -1941,11 +1957,6 @@ void linphone_call_update_crypto_parameters(LinphoneCall *call, SalMediaDescript ms_warning("Failed to find local crypto algo with tag: %d", new_stream->crypto_local_tag); call->audiostream_encrypted = FALSE; } - for (i = 0; i < SAL_CRYPTO_ALGO_MAX; i++) { - old_stream->crypto[i].tag = new_stream->crypto[i].tag; - old_stream->crypto[i].algo = new_stream->crypto[i].algo; - strncpy(old_stream->crypto[i].master_key, new_stream->crypto[i].master_key, sizeof(old_stream->crypto[i].master_key) - 1); - } } } @@ -1963,11 +1974,6 @@ void linphone_call_update_crypto_parameters(LinphoneCall *call, SalMediaDescript ms_warning("Failed to find local crypto algo with tag: %d", new_stream->crypto_local_tag); call->videostream_encrypted = FALSE; } - for (i = 0; i < SAL_CRYPTO_ALGO_MAX; i++) { - old_stream->crypto[i].tag = new_stream->crypto[i].tag; - old_stream->crypto[i].algo = new_stream->crypto[i].algo; - strncpy(old_stream->crypto[i].master_key, new_stream->crypto[i].master_key, sizeof(old_stream->crypto[i].master_key) - 1); - } } } #endif @@ -2057,12 +2063,10 @@ void linphone_call_stop_media_streams(LinphoneCall *call){ ms_event_queue_skip(call->core->msevq); } if (call->audio_profile){ - rtp_profile_clear_all(call->audio_profile); rtp_profile_destroy(call->audio_profile); call->audio_profile=NULL; } if (call->video_profile){ - rtp_profile_clear_all(call->video_profile); rtp_profile_destroy(call->video_profile); call->video_profile=NULL; } diff --git a/coreapi/private.h b/coreapi/private.h index 69eead0f1..e4ddd59c0 100644 --- a/coreapi/private.h +++ b/coreapi/private.h @@ -199,6 +199,8 @@ struct _LinphoneCall unsigned int remote_session_ver; LinphoneCall *referer; /*when this call is the result of a transfer, referer is set to the original call that caused the transfer*/ LinphoneCall *transfer_target;/*if this call received a transfer request, then transfer_target points to the new call created to the refer target */ + int localdesc_changed; + bool_t refer_pending; bool_t media_pending; bool_t audio_muted; diff --git a/include/sal/sal.h b/include/sal/sal.h index 70146cd05..046f2928f 100644 --- a/include/sal/sal.h +++ b/include/sal/sal.h @@ -169,11 +169,13 @@ typedef struct SalIceRemoteCandidate { #define SAL_MEDIA_DESCRIPTION_MAX_ICE_UFRAG_LEN 256 #define SAL_MEDIA_DESCRIPTION_MAX_ICE_PWD_LEN 256 +#define SAL_SRTP_KEY_SIZE 41 + typedef struct SalSrtpCryptoAlgo { unsigned int tag; enum ortp_srtp_crypto_suite_t algo; /* 41= 40 max(key_length for all algo) + '\0' */ - char master_key[41]; + char master_key[SAL_SRTP_KEY_SIZE]; } SalSrtpCryptoAlgo; #define SAL_CRYPTO_ALGO_MAX 4 @@ -687,4 +689,6 @@ LINPHONE_PUBLIC void sal_set_dns_timeout(Sal* sal,int timeout); LINPHONE_PUBLIC int sal_get_dns_timeout(const Sal* sal); LINPHONE_PUBLIC void sal_set_dns_user_hosts_file(Sal *sal, const char *hosts_file); LINPHONE_PUBLIC const char *sal_get_dns_user_hosts_file(const Sal *sal); +unsigned char * sal_get_random_bytes(unsigned char *ret, size_t size); + #endif