Skip to content

Commit

Permalink
Better SRTP-SDES negotiation in SIP/NoSIP plugins (fixes #2726) (#2727)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lminiero
lminiero committed Jul 14, 2021
1 parent eb5b20a commit 861155b
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 13 deletions.
35 changes: 29 additions & 6 deletions plugins/janus_nosip.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
Expand Down
35 changes: 28 additions & 7 deletions plugins/janus_sip.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
}
}
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
Expand Down

0 comments on commit 861155b

Please sign in to comment.