From 35d613fae9cfb712914a2a14ca9f281801769ad2 Mon Sep 17 00:00:00 2001 From: Jehan Monnier Date: Thu, 18 Oct 2018 10:43:05 +0200 Subject: [PATCH] =?UTF-8?q?fix=20wrong=20selection=20of=20sender=20crypto?= =?UTF-8?q?=E2=80=99s=20key=20in=20case=20of=20an=20incoming=20offer=20wit?= =?UTF-8?q?h=20matching=20chypto=20also=20not=20the=20first.=20#=20Conflic?= =?UTF-8?q?ts:=20#=09src/sal/call-op.cpp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/conference/session/media-session-p.h | 1 - src/conference/session/media-session.cpp | 16 ++++------------ src/sal/call-op.cpp | 10 +++++++--- src/sal/sal.cpp | 7 +++++++ src/sal/sal.h | 2 ++ tester/call_single_tester.c | 8 ++++++++ tester/rcfiles/laure_tcp_rc | 1 + 7 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/conference/session/media-session-p.h b/src/conference/session/media-session-p.h index 1b65012a5..10cc209c3 100644 --- a/src/conference/session/media-session-p.h +++ b/src/conference/session/media-session-p.h @@ -169,7 +169,6 @@ private: SalMulticastRole getMulticastRole (SalStreamType type); void joinMulticastGroup (int streamIndex, MediaStream *ms); - int findCryptoIndexFromTag (const SalSrtpCryptoAlgo crypto[], unsigned char tag); void setDtlsFingerprint (MSMediaStreamSessions *sessions, const SalStreamDescription *sd, const SalStreamDescription *remote); void setDtlsFingerprintOnAllStreams (); void setupDtlsParams (MediaStream *ms); diff --git a/src/conference/session/media-session.cpp b/src/conference/session/media-session.cpp index f8216ca56..c3cd64b98 100644 --- a/src/conference/session/media-session.cpp +++ b/src/conference/session/media-session.cpp @@ -1710,14 +1710,6 @@ void MediaSessionPrivate::joinMulticastGroup (int streamIndex, MediaStream *ms) // ----------------------------------------------------------------------------- -int MediaSessionPrivate::findCryptoIndexFromTag (const SalSrtpCryptoAlgo crypto[], unsigned char tag) { - for (int i = 0; i < SAL_CRYPTO_ALGO_MAX; i++) { - if (crypto[i].tag == tag) - return i; - } - return -1; -} - void MediaSessionPrivate::setDtlsFingerprint (MSMediaStreamSessions *sessions, const SalStreamDescription *sd, const SalStreamDescription *remote) { if (sal_stream_description_has_dtls(sd)) { if (sd->dtls_role == SalDtlsRoleInvalid) @@ -1881,7 +1873,7 @@ void MediaSessionPrivate::updateCryptoParameters (SalMediaDescription *oldMd, Sa } bool MediaSessionPrivate::updateStreamCryptoParameters (const SalStreamDescription *localStreamDesc, SalStreamDescription *oldStream, SalStreamDescription *newStream, MediaStream *ms) { - int cryptoIdx = findCryptoIndexFromTag(localStreamDesc->crypto, static_cast(newStream->crypto_local_tag)); + int cryptoIdx = Sal::findCryptoIndexFromTag(localStreamDesc->crypto, static_cast(newStream->crypto_local_tag)); if (cryptoIdx >= 0) { if (localDescChanged & SAL_MEDIA_DESCRIPTION_CRYPTO_KEYS_CHANGED) ms_media_stream_sessions_set_srtp_send_key_b64(&ms->sessions, newStream->crypto[0].algo, localStreamDesc->crypto[cryptoIdx].master_key); @@ -2694,7 +2686,7 @@ void MediaSessionPrivate::startAudioStream (CallSession::State targetState, bool // Valid local tags are > 0 if (sal_stream_description_has_srtp(stream)) { const SalStreamDescription *localStreamDesc = sal_media_description_find_stream(localDesc, stream->proto, SalAudio); - int cryptoIdx = findCryptoIndexFromTag(localStreamDesc->crypto, static_cast(stream->crypto_local_tag)); + int cryptoIdx = Sal::findCryptoIndexFromTag(localStreamDesc->crypto, static_cast(stream->crypto_local_tag)); if (cryptoIdx >= 0) { ms_media_stream_sessions_set_srtp_recv_key_b64(&audioStream->ms.sessions, stream->crypto[0].algo, stream->crypto[0].master_key); ms_media_stream_sessions_set_srtp_send_key_b64(&audioStream->ms.sessions, stream->crypto[0].algo, localStreamDesc->crypto[cryptoIdx].master_key); @@ -2874,7 +2866,7 @@ void MediaSessionPrivate::startTextStream () { getCurrentParams()->getPrivate()->setUsedRealtimeTextCodec(rtp_profile_get_payload(textProfile, usedPt)); getCurrentParams()->enableRealtimeText(true); if (sal_stream_description_has_srtp(tstream)) { - int cryptoIdx = findCryptoIndexFromTag(localStreamDesc->crypto, static_cast(tstream->crypto_local_tag)); + int cryptoIdx = Sal::findCryptoIndexFromTag(localStreamDesc->crypto, static_cast(tstream->crypto_local_tag)); if (cryptoIdx >= 0) { ms_media_stream_sessions_set_srtp_recv_key_b64(&textStream->ms.sessions, tstream->crypto[0].algo, tstream->crypto[0].master_key); ms_media_stream_sessions_set_srtp_send_key_b64(&textStream->ms.sessions, tstream->crypto[0].algo, localStreamDesc->crypto[cryptoIdx].master_key); @@ -2967,7 +2959,7 @@ void MediaSessionPrivate::startVideoStream (CallSession::State targetState) { if (isActive) { if (sal_stream_description_has_srtp(vstream)) { const SalStreamDescription *localStreamDesc = sal_media_description_find_stream(localDesc, vstream->proto, SalVideo); - int cryptoIdx = findCryptoIndexFromTag(localStreamDesc->crypto, static_cast(vstream->crypto_local_tag)); + int cryptoIdx = Sal::findCryptoIndexFromTag(localStreamDesc->crypto, static_cast(vstream->crypto_local_tag)); if (cryptoIdx >= 0) { ms_media_stream_sessions_set_srtp_recv_key_b64(&videoStream->ms.sessions, vstream->crypto[0].algo, vstream->crypto[0].master_key); ms_media_stream_sessions_set_srtp_send_key_b64(&videoStream->ms.sessions, vstream->crypto[0].algo, localStreamDesc->crypto[cryptoIdx].master_key); diff --git a/src/sal/call-op.cpp b/src/sal/call-op.cpp index d92e4b9ec..cf986542a 100644 --- a/src/sal/call-op.cpp +++ b/src/sal/call-op.cpp @@ -339,9 +339,13 @@ void SalCallOp::sdpProcess(){ strcpy(mResult->streams[i].rtcp_addr,mRemoteMedia->streams[i].rtcp_addr); mResult->streams[i].rtcp_port=mRemoteMedia->streams[i].rtcp_port; - if (sal_stream_description_has_srtp(&mResult->streams[i])) { - mResult->streams[i].crypto[0] = mRemoteMedia->streams[i].crypto[0]; - } + if (sal_stream_description_has_srtp(&mResult->streams[i])) { + int cryptoIdx = Sal::findCryptoIndexFromTag( mRemoteMedia->streams[i].crypto, static_cast(mResult->streams[i].crypto[0].tag)); + if (cryptoIdx >= 0) + mResult->streams[i].crypto[0] = mRemoteMedia->streams[i].crypto[cryptoIdx]; + else + lError() << "Failed to find crypto algo with tag: " << mResult->streams[i].crypto_local_tag << "from resulting description [" << mResult << "]"; + } } } } diff --git a/src/sal/sal.cpp b/src/sal/sal.cpp index 193588ca4..a8a174d11 100644 --- a/src/sal/sal.cpp +++ b/src/sal/sal.cpp @@ -823,6 +823,13 @@ belle_sip_response_t* Sal::createResponseFromRequest (belle_sip_request_t* req, return resp; } +int Sal::findCryptoIndexFromTag (const SalSrtpCryptoAlgo crypto[], unsigned char tag) { + for (int i = 0; i < SAL_CRYPTO_ALGO_MAX; i++) { + if (crypto[i].tag == tag) + return i; + } + return -1; +} diff --git a/src/sal/sal.h b/src/sal/sal.h index 1de178d5f..b9e13dfe7 100644 --- a/src/sal/sal.h +++ b/src/sal/sal.h @@ -253,6 +253,8 @@ public: belle_sip_source_t *createTimer (belle_sip_source_func_t func, void *data, unsigned int timeoutValueMs, const std::string &timerName); void cancelTimer (belle_sip_source_t *timer); + //utils + static int findCryptoIndexFromTag (const SalSrtpCryptoAlgo crypto[], unsigned char tag); private: struct SalUuid { diff --git a/tester/call_single_tester.c b/tester/call_single_tester.c index 2408640ac..e3d082ef0 100644 --- a/tester/call_single_tester.c +++ b/tester/call_single_tester.c @@ -2665,6 +2665,13 @@ static void srtp_call(void) { call_base(LinphoneMediaEncryptionSRTP,FALSE,FALSE,LinphonePolicyNoFirewall,FALSE); } +/* + *Purpose of this test is to check that even if caller and callee does not have exactly the same crypto suite configured, the matching crypto suite is used. + */ +static void srtp_call_with_different_crypto_suite(void) { + call_base_with_configfile(LinphoneMediaEncryptionSRTP,FALSE,FALSE,LinphonePolicyNoFirewall,FALSE, "laure_tcp_rc", "marie_rc"); +} + static void zrtp_call(void) { call_base(LinphoneMediaEncryptionZRTP,FALSE,FALSE,LinphonePolicyNoFirewall,FALSE); } @@ -6542,6 +6549,7 @@ test_t call_tests[] = { TEST_NO_TAG("Call paused resumed with loss", call_paused_resumed_with_loss), TEST_NO_TAG("Call paused resumed from callee", call_paused_resumed_from_callee), TEST_NO_TAG("SRTP call", srtp_call), + TEST_NO_TAG("SRTP call with different crypto suite", srtp_call_with_different_crypto_suite), TEST_NO_TAG("ZRTP call", zrtp_call), TEST_NO_TAG("ZRTP silent call", zrtp_silent_call), TEST_NO_TAG("ZRTP SAS call", zrtp_sas_call), diff --git a/tester/rcfiles/laure_tcp_rc b/tester/rcfiles/laure_tcp_rc index 912db8271..3dddf03ed 100644 --- a/tester/rcfiles/laure_tcp_rc +++ b/tester/rcfiles/laure_tcp_rc @@ -4,6 +4,7 @@ sip_tcp_port=-1 sip_tls_port=-1 default_proxy=0 ping_with_options=0 +srtp_crypto_suites=AES_256_CM_HMAC_SHA1_80 [auth_info_0] username=laure