-
Notifications
You must be signed in to change notification settings - Fork 815
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
Add the option to check the source address of ClientHello message on DTLS-SRTP #4261
base: master
Are you sure you want to change the base?
Changes from 4 commits
7be1f21
dfe04eb
54d7f60
458c845
e590ecf
5043bab
9079b00
100d1cc
dcf9c65
c3cadef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -117,6 +117,12 @@ typedef struct dtls_srtp_channel | |||||||||||||||||||||||
unsigned channel; | ||||||||||||||||||||||||
} dtls_srtp_channel; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
typedef struct dtls_srtp_ice_cand | ||||||||||||||||||||||||
{ | ||||||||||||||||||||||||
unsigned cand_cnt; | ||||||||||||||||||||||||
pj_sockaddr cand_addr[PJ_ICE_MAX_CAND]; | ||||||||||||||||||||||||
} dtls_srtp_ice_cand; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. still used? |
||||||||||||||||||||||||
typedef struct dtls_srtp | ||||||||||||||||||||||||
{ | ||||||||||||||||||||||||
pjmedia_transport base; | ||||||||||||||||||||||||
|
@@ -144,6 +150,8 @@ typedef struct dtls_srtp | |||||||||||||||||||||||
char buf[NUM_CHANNEL][PJMEDIA_MAX_MTU]; | ||||||||||||||||||||||||
pjmedia_clock *clock[NUM_CHANNEL];/* Timer workaround for retrans */ | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
dtls_srtp_ice_cand ice_rem_cand[NUM_CHANNEL]; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. still used? |
||||||||||||||||||||||||
SSL_CTX *ossl_ctx[NUM_CHANNEL]; | ||||||||||||||||||||||||
SSL *ossl_ssl[NUM_CHANNEL]; | ||||||||||||||||||||||||
BIO *ossl_rbio[NUM_CHANNEL]; | ||||||||||||||||||||||||
|
@@ -1082,6 +1090,152 @@ static pj_status_t parse_setup_finger_attr(dtls_srtp *ds, | |||||||||||||||||||||||
return PJ_SUCCESS; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/* Parse a=candidate line */ | ||||||||||||||||||||||||
static pj_status_t parse_cand(dtls_srtp *ds, | ||||||||||||||||||||||||
sauwming marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
const pj_str_t *orig_input, | ||||||||||||||||||||||||
dtls_srtp_ice_cand cand[]) | ||||||||||||||||||||||||
{ | ||||||||||||||||||||||||
pj_str_t token, delim, host; | ||||||||||||||||||||||||
int af; | ||||||||||||||||||||||||
unsigned comp_id; | ||||||||||||||||||||||||
unsigned cand_cnt; | ||||||||||||||||||||||||
pj_ssize_t found_idx; | ||||||||||||||||||||||||
pj_status_t status = PJNATH_EICEINCANDSDP; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/* Foundation */ | ||||||||||||||||||||||||
delim = pj_str(" "); | ||||||||||||||||||||||||
found_idx = pj_strtok(orig_input, &delim, &token, 0); | ||||||||||||||||||||||||
if (found_idx == orig_input->slen) { | ||||||||||||||||||||||||
#if DTLS_DEBUG | ||||||||||||||||||||||||
PJ_LOG(2, (ds->base.name, "Parse ICE cand: " | ||||||||||||||||||||||||
"Expecting ICE foundation in candidate")); | ||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||
goto on_return; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/* Component ID */ | ||||||||||||||||||||||||
found_idx = pj_strtok(orig_input, &delim, &token, found_idx + token.slen); | ||||||||||||||||||||||||
if (found_idx == orig_input->slen) { | ||||||||||||||||||||||||
#if DTLS_DEBUG | ||||||||||||||||||||||||
PJ_LOG(2, (ds->base.name, "Parse ICE cand: " | ||||||||||||||||||||||||
"Expecting ICE component ID in candidate")); | ||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||
goto on_return; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
comp_id = (pj_uint8_t)pj_strtoul(&token) - 1; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (comp_id > NUM_CHANNEL) { | ||||||||||||||||||||||||
#if DTLS_DEBUG | ||||||||||||||||||||||||
PJ_LOG(2, (ds->base.name, "Parse ICE cand: " | ||||||||||||||||||||||||
"Invalid ICE component ID in candidate")); | ||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||
goto on_return; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
cand_cnt = cand[comp_id].cand_cnt; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/* Transport */ | ||||||||||||||||||||||||
found_idx = pj_strtok(orig_input, &delim, &token, found_idx + token.slen); | ||||||||||||||||||||||||
if (found_idx == orig_input->slen) { | ||||||||||||||||||||||||
#if DTLS_DEBUG | ||||||||||||||||||||||||
PJ_LOG(2, (ds->base.name, "Parse ICE cand: " | ||||||||||||||||||||||||
"Expecting ICE transport in candidate")); | ||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||
goto on_return; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
if (pj_stricmp2(&token, "UDP") != 0) { | ||||||||||||||||||||||||
#if DTLS_DEBUG | ||||||||||||||||||||||||
PJ_LOG(2, (ds->base.name, "Parse ICE cand: " | ||||||||||||||||||||||||
"Expecting ICE UDP transport only in candidate")); | ||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||
goto on_return; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/* Priority */ | ||||||||||||||||||||||||
found_idx = pj_strtok(orig_input, &delim, &token, found_idx + token.slen); | ||||||||||||||||||||||||
if (found_idx == orig_input->slen) { | ||||||||||||||||||||||||
#if DTLS_DEBUG | ||||||||||||||||||||||||
PJ_LOG(2, (ds->base.name, "Parse ICE cand: " | ||||||||||||||||||||||||
"Expecting ICE priority in candidate")); | ||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||
goto on_return; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/* Host */ | ||||||||||||||||||||||||
found_idx = pj_strtok(orig_input, &delim, &host, found_idx + token.slen); | ||||||||||||||||||||||||
if (found_idx == orig_input->slen) { | ||||||||||||||||||||||||
#if DTLS_DEBUG | ||||||||||||||||||||||||
PJ_LOG(2, (ds->base.name, "Parse ICE cand: " | ||||||||||||||||||||||||
"Expecting ICE priority in candidate")); | ||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||
goto on_return; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
/* Detect address family */ | ||||||||||||||||||||||||
if (pj_strchr(&host, ':')) | ||||||||||||||||||||||||
af = pj_AF_INET6(); | ||||||||||||||||||||||||
else | ||||||||||||||||||||||||
af = pj_AF_INET(); | ||||||||||||||||||||||||
/* Assign address */ | ||||||||||||||||||||||||
if (pj_sockaddr_init(af, &cand[comp_id].cand_addr[cand_cnt], | ||||||||||||||||||||||||
&host, 0)) { | ||||||||||||||||||||||||
#if DTLS_DEBUG | ||||||||||||||||||||||||
PJ_LOG(2, (ds->base.name, "Parse ICE cand: " | ||||||||||||||||||||||||
"Invalid ICE candidate address")); | ||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||
goto on_return; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/* Port */ | ||||||||||||||||||||||||
found_idx = pj_strtok(orig_input, &delim, &token, found_idx + host.slen); | ||||||||||||||||||||||||
if (found_idx == orig_input->slen) { | ||||||||||||||||||||||||
#if DTLS_DEBUG | ||||||||||||||||||||||||
PJ_LOG(2, (ds->base.name, "Parse ICE cand: " | ||||||||||||||||||||||||
"Expecting ICE port number in candidate")); | ||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||
goto on_return; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
pj_sockaddr_set_port(&cand[comp_id].cand_addr[cand_cnt], | ||||||||||||||||||||||||
(pj_uint16_t)pj_strtoul(&token)); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
cand[comp_id].cand_cnt++; | ||||||||||||||||||||||||
status = PJ_SUCCESS; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
on_return: | ||||||||||||||||||||||||
return status; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
static pj_status_t get_ice_rem_cand(dtls_srtp* ds, | ||||||||||||||||||||||||
const pjmedia_sdp_session* rem_sdp, | ||||||||||||||||||||||||
unsigned media_index) | ||||||||||||||||||||||||
{ | ||||||||||||||||||||||||
/* Get all candidates in the media */ | ||||||||||||||||||||||||
unsigned cand_cnt = 0; | ||||||||||||||||||||||||
unsigned i; | ||||||||||||||||||||||||
static const pj_str_t STR_CANDIDATE = { "candidate", 9 }; | ||||||||||||||||||||||||
pjmedia_sdp_media* rem_m = rem_sdp->media[media_index]; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
for (i = 0; i < rem_m->attr_count && cand_cnt < PJ_ICE_MAX_CAND; ++i) { | ||||||||||||||||||||||||
pjmedia_sdp_attr *attr; | ||||||||||||||||||||||||
pj_status_t status; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
attr = rem_m->attr[i]; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (pj_strcmp(&attr->name, &STR_CANDIDATE) != 0) | ||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/* Parse candidate */ | ||||||||||||||||||||||||
status = parse_cand(ds, &attr->value, ds->ice_rem_cand); | ||||||||||||||||||||||||
if (status != PJ_SUCCESS) { | ||||||||||||||||||||||||
PJ_PERROR(4, (ds->base.name, status, | ||||||||||||||||||||||||
"Error in parsing SDP candidate attribute '%.*s'", | ||||||||||||||||||||||||
(int)attr->value.slen, attr->value.ptr)); | ||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
cand_cnt++; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
return PJ_SUCCESS; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
static pj_status_t get_rem_addrs(dtls_srtp *ds, | ||||||||||||||||||||||||
const pjmedia_sdp_session *sdp_remote, | ||||||||||||||||||||||||
unsigned media_index, | ||||||||||||||||||||||||
|
@@ -1260,6 +1414,35 @@ static void on_ice_complete2(pjmedia_transport *tp, | |||||||||||||||||||||||
* | ||||||||||||||||||||||||
* *************************************/ | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
static pj_bool_t is_valid_src_addr(dtls_srtp *ds, unsigned idx, | ||||||||||||||||||||||||
pj_sockaddr *src_addr) | ||||||||||||||||||||||||
{ | ||||||||||||||||||||||||
if (ds->use_ice) { | ||||||||||||||||||||||||
dtls_srtp_ice_cand *ice_cand = &ds->ice_rem_cand[idx]; | ||||||||||||||||||||||||
unsigned i = 0; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (ds->srtp->use_rtcp_mux) | ||||||||||||||||||||||||
idx = RTP_CHANNEL; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
for (; i < ice_cand->cand_cnt && i < PJ_ARRAY_SIZE(ice_cand->cand_addr); | ||||||||||||||||||||||||
++i) | ||||||||||||||||||||||||
{ | ||||||||||||||||||||||||
if (pj_sockaddr_cmp(&ice_cand->cand_addr[i], src_addr) == 0) | ||||||||||||||||||||||||
return PJ_TRUE; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
return PJ_FALSE; | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
pj_sockaddr *rem_addr; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (idx == RTP_CHANNEL) { | ||||||||||||||||||||||||
rem_addr = &ds->rem_addr; | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
rem_addr = &ds->rem_rtcp; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
return (pj_sockaddr_cmp(rem_addr, src_addr) == 0); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
static pj_status_t dtls_on_recv(pjmedia_transport *tp, unsigned idx, | ||||||||||||||||||||||||
const void *pkt, pj_size_t size) | ||||||||||||||||||||||||
{ | ||||||||||||||||||||||||
|
@@ -1361,6 +1544,50 @@ static pj_status_t dtls_on_recv(pjmedia_transport *tp, unsigned idx, | |||||||||||||||||||||||
(ds->setup == DTLS_SETUP_ACTPASS || ds->setup == DTLS_SETUP_PASSIVE)) | ||||||||||||||||||||||||
{ | ||||||||||||||||||||||||
pj_status_t status; | ||||||||||||||||||||||||
pj_bool_t check_hello_addr = PJ_TRUE; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
#if defined(PJMEDIA_SRTP_DTLS_CHECK_HELLO_ADDR) && PJMEDIA_SRTP_DTLS_CHECK_HELLO_ADDR==0 | ||||||||||||||||||||||||
if (!ds->use_ice) | ||||||||||||||||||||||||
check_hello_addr = PJ_FALSE; | ||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||
if (check_hello_addr) { | ||||||||||||||||||||||||
pjmedia_transport_info info; | ||||||||||||||||||||||||
pj_sockaddr src_addr; | ||||||||||||||||||||||||
pj_bool_t src_addr_avail = PJ_TRUE; | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. source address must be available, because it's where the packet comes from. |
||||||||||||||||||||||||
|
||||||||||||||||||||||||
pjmedia_transport_get_info(ds->srtp->member_tp, &info); | ||||||||||||||||||||||||
if (idx == RTP_CHANNEL) { | ||||||||||||||||||||||||
if (!pj_sockaddr_has_addr(&info.src_rtp_name)) { | ||||||||||||||||||||||||
src_addr_avail = PJ_FALSE; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
else { | ||||||||||||||||||||||||
pj_sockaddr_cp(&src_addr, &info.src_rtp_name); | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is RTP address, not SDP address. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, it is to get the message's source address. pjproject/pjmedia/src/pjmedia/transport_srtp_dtls.c Lines 1181 to 1191 in dcf9c65
|
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
else { | ||||||||||||||||||||||||
if (!pj_sockaddr_has_addr(&info.src_rtcp_name)) { | ||||||||||||||||||||||||
src_addr_avail = PJ_FALSE; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
else { | ||||||||||||||||||||||||
pj_sockaddr_cp(&src_addr, &info.src_rtcp_name); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (!src_addr_avail || !is_valid_src_addr(ds, idx, &src_addr)) { | ||||||||||||||||||||||||
char psrc_addr[PJ_INET6_ADDRSTRLEN] = "Unknown"; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (src_addr_avail) { | ||||||||||||||||||||||||
pj_sockaddr_print(&src_addr, psrc_addr, | ||||||||||||||||||||||||
sizeof(psrc_addr), 3); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
PJ_LOG(2, (ds->base.name, "DTLS-SRTP %s ignoring %lu bytes, " | ||||||||||||||||||||||||
"from src addr [%s]", CHANNEL_TO_STRING(idx), | ||||||||||||||||||||||||
(unsigned long)size, psrc_addr)); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
DTLS_UNLOCK(ds); | ||||||||||||||||||||||||
return PJ_SUCCESS; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
ds->setup = DTLS_SETUP_PASSIVE; | ||||||||||||||||||||||||
status = ssl_handshake_channel(ds, idx); | ||||||||||||||||||||||||
if (status != PJ_SUCCESS) { | ||||||||||||||||||||||||
|
@@ -1782,6 +2009,13 @@ static pj_status_t dtls_media_start( pjmedia_transport *tp, | |||||||||||||||||||||||
/* Update remote RTP & RTCP addresses */ | ||||||||||||||||||||||||
get_rem_addrs(ds, sdp_remote, media_index, &ds->rem_addr, | ||||||||||||||||||||||||
&ds->rem_rtcp, NULL); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (ds->use_ice && | ||||||||||||||||||||||||
(ds->setup == DTLS_SETUP_ACTPASS || | ||||||||||||||||||||||||
ds->setup == DTLS_SETUP_PASSIVE)) | ||||||||||||||||||||||||
{ | ||||||||||||||||||||||||
get_ice_rem_cand(ds, sdp_remote, media_index); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/* Check if the background DTLS nego has completed */ | ||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need a compile-time setting? and why is the default 0 if it may cause a security issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might add a slight delay in starting the handshake (wait for the transport address and RTP source address to be available) and possible issues when using aggressive nomination.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reading the pdf, I believe the address check should be made mandatory for ICE, and the address should match one of the verified (valid?) candidates obtained during ICE check, so there should be no delay because ICE check has been done.
For no-ICE, then you can use the compile-time setting, because the best we can do is compare it with the SDP address, which may not be accurate.