From 28031f36d222d7c71720e4cc616c87616fc4bcd8 Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Tue, 6 Jul 2021 12:36:10 +0200 Subject: [PATCH] Better SRTP-SDES negotiation in SIP/NoSIP plugins (fixes #2726) --- plugins/janus_nosip.c | 35 +++++++++++++++++++++++++++++------ plugins/janus_sip.c | 35 ++++++++++++++++++++++++++++------- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/plugins/janus_nosip.c b/plugins/janus_nosip.c index 10bccf248e..b8330d3e19 100644 --- a/plugins/janus_nosip.c +++ b/plugins/janus_nosip.c @@ -298,6 +298,7 @@ typedef struct janus_nosip_media { guint32 audio_ssrc, audio_ssrc_peer; int audio_pt; const char *audio_pt_name; + gint32 audio_srtp_tag; srtp_t audio_srtp_in, audio_srtp_out; srtp_policy_t audio_remote_policy, audio_local_policy; char *audio_srtp_local_profile, *audio_srtp_local_crypto; @@ -310,6 +311,7 @@ typedef struct janus_nosip_media { guint32 simulcast_ssrc; int video_pt; const char *video_pt_name; + gint32 video_srtp_tag; srtp_t video_srtp_in, video_srtp_out; srtp_policy_t video_remote_policy, video_local_policy; char *video_srtp_local_profile, *video_srtp_local_crypto; @@ -503,10 +505,10 @@ static int janus_nosip_srtp_set_remote(janus_nosip_session *session, gboolean vi master_length = SRTP_AESGCM256_MASTER_LENGTH; #endif } else { - JANUS_LOG(LOG_ERR, "[NoSIP-%p] Unsupported SRTP profile %s\n", session, profile); + JANUS_LOG(LOG_WARN, "[NoSIP-%p] Unsupported SRTP profile %s\n", session, profile); return -2; } - JANUS_LOG(LOG_WARN, "[NoSIP-%p] Key/Salt/Master: %zu/%zu/%zu\n", + JANUS_LOG(LOG_VERB, "[NoSIP-%p] Key/Salt/Master: %zu/%zu/%zu\n", session, master_length, key_length, salt_length); /* Base64 decode the crypto string and set it as the remote SRTP context */ gsize len = 0; @@ -567,6 +569,7 @@ static void janus_nosip_srtp_cleanup(janus_nosip_session *session) { session->media.has_srtp_remote = FALSE; session->media.srtp_profile = 0; /* Audio */ + session->media.audio_srtp_tag = 0; if(session->media.audio_srtp_out) srtp_dealloc(session->media.audio_srtp_out); session->media.audio_srtp_out = NULL; @@ -586,6 +589,7 @@ static void janus_nosip_srtp_cleanup(janus_nosip_session *session) { session->media.audio_srtp_local_crypto = NULL; } /* Video */ + session->media.video_srtp_tag = 0; if(session->media.video_srtp_out) srtp_dealloc(session->media.video_srtp_out); session->media.video_srtp_out = NULL; @@ -1840,14 +1844,27 @@ void janus_nosip_sdp_process(janus_nosip_session *session, janus_sdp *sdp, gbool } gint32 tag = 0; char profile[101], crypto[101]; - /* FIXME inline can be more complex than that, and we're currently only offering SHA1_80 */ int res = a->value ? (sscanf(a->value, "%"SCNi32" %100s inline:%100s", &tag, profile, crypto)) : 0; if(res != 3) { JANUS_LOG(LOG_WARN, "Failed to parse crypto line, ignoring... %s\n", a->value); } else { gboolean video = (m->type == JANUS_SDP_VIDEO); - janus_nosip_srtp_set_remote(session, video, profile, crypto); + if(answer && ((!video && tag != session->media.audio_srtp_tag) || (video && tag != session->media.video_srtp_tag))) { + /* Not the tag for the crypto line we offered */ + tempA = tempA->next; + continue; + } + if(janus_nosip_srtp_set_remote(session, video, profile, crypto) < 0) { + /* Unsupported profile? */ + tempA = tempA->next; + continue; + } + if(!video) { + session->media.audio_srtp_tag = tag; + } else { + session->media.video_srtp_tag = tag; + } session->media.has_srtp_remote = TRUE; } } @@ -1903,7 +1920,10 @@ char *janus_nosip_sdp_manipulate(janus_nosip_session *session, janus_sdp *sdp, g if(!session->media.audio_srtp_local_profile || !session->media.audio_srtp_local_crypto) { janus_nosip_srtp_set_local(session, FALSE, &session->media.audio_srtp_local_profile, &session->media.audio_srtp_local_crypto); } - janus_sdp_attribute *a = janus_sdp_attribute_create("crypto", "1 %s inline:%s", session->media.audio_srtp_local_profile, session->media.audio_srtp_local_crypto); + if(session->media.audio_srtp_tag == 0) + session->media.audio_srtp_tag = 1; + janus_sdp_attribute *a = janus_sdp_attribute_create("crypto", "%"SCNi32" %s inline:%s", + session->media.audio_srtp_tag, session->media.audio_srtp_local_profile, session->media.audio_srtp_local_crypto); m->attributes = g_list_append(m->attributes, a); } } else if(m->type == JANUS_SDP_VIDEO) { @@ -1912,7 +1932,10 @@ char *janus_nosip_sdp_manipulate(janus_nosip_session *session, janus_sdp *sdp, g if(!session->media.video_srtp_local_profile || !session->media.video_srtp_local_crypto) { janus_nosip_srtp_set_local(session, TRUE, &session->media.video_srtp_local_profile, &session->media.video_srtp_local_crypto); } - janus_sdp_attribute *a = janus_sdp_attribute_create("crypto", "1 %s inline:%s", session->media.video_srtp_local_profile, session->media.video_srtp_local_crypto); + if(session->media.video_srtp_tag == 0) + session->media.video_srtp_tag = 1; + janus_sdp_attribute *a = janus_sdp_attribute_create("crypto", "%"SCNi32" %s inline:%s", + session->media.video_srtp_tag, session->media.video_srtp_local_profile, session->media.video_srtp_local_crypto); m->attributes = g_list_append(m->attributes, a); } } diff --git a/plugins/janus_sip.c b/plugins/janus_sip.c index f4e4c9368e..518c30640a 100644 --- a/plugins/janus_sip.c +++ b/plugins/janus_sip.c @@ -988,6 +988,7 @@ typedef struct janus_sip_media { guint32 audio_ssrc, audio_ssrc_peer; int audio_pt; const char *audio_pt_name; + gint32 audio_srtp_tag; srtp_t audio_srtp_in, audio_srtp_out; srtp_policy_t audio_remote_policy, audio_local_policy; char *audio_srtp_local_profile, *audio_srtp_local_crypto; @@ -1001,6 +1002,7 @@ typedef struct janus_sip_media { guint32 simulcast_ssrc; int video_pt; const char *video_pt_name; + gint32 video_srtp_tag; srtp_t video_srtp_in, video_srtp_out; srtp_policy_t video_remote_policy, video_local_policy; char *video_srtp_local_profile, *video_srtp_local_crypto; @@ -1307,7 +1309,7 @@ static int janus_sip_srtp_set_remote(janus_sip_session *session, gboolean video, master_length = SRTP_AESGCM256_MASTER_LENGTH; #endif } else { - JANUS_LOG(LOG_ERR, "[SIP-%s] Unsupported SRTP profile %s\n", session->account.username, profile); + JANUS_LOG(LOG_WARN, "[SIP-%s] Unsupported SRTP profile %s\n", session->account.username, profile); return -2; } JANUS_LOG(LOG_VERB, "[SIP-%s] Key/Salt/Master: %zu/%zu/%zu\n", @@ -1373,6 +1375,7 @@ static void janus_sip_srtp_cleanup(janus_sip_session *session) { session->media.has_srtp_remote_video = FALSE; session->media.srtp_profile = 0; /* Audio */ + session->media.audio_srtp_tag = 0; if(session->media.audio_srtp_out) srtp_dealloc(session->media.audio_srtp_out); session->media.audio_srtp_out = NULL; @@ -1392,6 +1395,7 @@ static void janus_sip_srtp_cleanup(janus_sip_session *session) { session->media.audio_srtp_local_crypto = NULL; } /* Video */ + session->media.video_srtp_tag = 0; if(session->media.video_srtp_out) srtp_dealloc(session->media.video_srtp_out); session->media.video_srtp_out = NULL; @@ -6038,18 +6042,29 @@ void janus_sip_sdp_process(janus_sip_session *session, janus_sdp *sdp, gboolean } gint32 tag = 0; char profile[101], crypto[101]; - /* FIXME inline can be more complex than that, and we're currently only offering SHA1_80 */ int res = a->value ? (sscanf(a->value, "%"SCNi32" %100s inline:%100s", &tag, profile, crypto)) : 0; if(res != 3) { JANUS_LOG(LOG_WARN, "Failed to parse crypto line, ignoring... %s\n", a->value); } else { gboolean video = (m->type == JANUS_SDP_VIDEO); - janus_sip_srtp_set_remote(session, video, profile, crypto); - if(!video) + if(answer && ((!video && tag != session->media.audio_srtp_tag) || (video && tag != session->media.video_srtp_tag))) { + /* Not the tag for the crypto line we offered */ + tempA = tempA->next; + continue; + } + if(janus_sip_srtp_set_remote(session, video, profile, crypto) < 0) { + /* Unsupported profile? */ + tempA = tempA->next; + continue; + } + if(!video) { + session->media.audio_srtp_tag = tag; session->media.has_srtp_remote_audio = TRUE; - else + } else { + session->media.video_srtp_tag = tag; session->media.has_srtp_remote_video = TRUE; + } } } } @@ -6108,7 +6123,10 @@ char *janus_sip_sdp_manipulate(janus_sip_session *session, janus_sdp *sdp, gbool if(!session->media.audio_srtp_local_profile || !session->media.audio_srtp_local_crypto) { janus_sip_srtp_set_local(session, FALSE, &session->media.audio_srtp_local_profile, &session->media.audio_srtp_local_crypto); } - janus_sdp_attribute *a = janus_sdp_attribute_create("crypto", "1 %s inline:%s", session->media.audio_srtp_local_profile, session->media.audio_srtp_local_crypto); + if(session->media.audio_srtp_tag == 0) + session->media.audio_srtp_tag = 1; + janus_sdp_attribute *a = janus_sdp_attribute_create("crypto", "%"SCNi32" %s inline:%s", + session->media.audio_srtp_tag, session->media.audio_srtp_local_profile, session->media.audio_srtp_local_crypto); m->attributes = g_list_append(m->attributes, a); } } else if(m->type == JANUS_SDP_VIDEO) { @@ -6117,7 +6135,10 @@ char *janus_sip_sdp_manipulate(janus_sip_session *session, janus_sdp *sdp, gbool if(!session->media.video_srtp_local_profile || !session->media.video_srtp_local_crypto) { janus_sip_srtp_set_local(session, TRUE, &session->media.video_srtp_local_profile, &session->media.video_srtp_local_crypto); } - janus_sdp_attribute *a = janus_sdp_attribute_create("crypto", "1 %s inline:%s", session->media.video_srtp_local_profile, session->media.video_srtp_local_crypto); + if(session->media.video_srtp_tag == 0) + session->media.video_srtp_tag = 1; + janus_sdp_attribute *a = janus_sdp_attribute_create("crypto", "%"SCNi32" %s inline:%s", + session->media.video_srtp_tag, session->media.video_srtp_local_profile, session->media.video_srtp_local_crypto); m->attributes = g_list_append(m->attributes, a); } }