Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better SRTP-SDES negotiation in SIP/NoSIP plugins (fixes #2726) #2727

Merged
merged 1 commit into from
Jul 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 29 additions & 6 deletions plugins/janus_nosip.c
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
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