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

Add the option to check the source address of ClientHello message on DTLS-SRTP #4261

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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
11 changes: 11 additions & 0 deletions pjmedia/include/pjmedia/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -1100,6 +1100,17 @@
# define PJMEDIA_SRTP_DTLS_OSSL_CIPHERS "DEFAULT"
#endif

/**
* Enabled this to check the source address of ClientHello message coming
* from a valid address. When ICE is enabled, the check will always be
* performed.
*
* Default value: 0
*/
#ifndef PJMEDIA_SRTP_DTLS_CHECK_HELLO_ADDR
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

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.

# define PJMEDIA_SRTP_DTLS_CHECK_HELLO_ADDR 0
#endif


/**
* Maximum number of SRTP cryptos.
Expand Down
24 changes: 24 additions & 0 deletions pjmedia/include/pjmedia/transport_ice.h
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,30 @@ PJ_DECL(pj_status_t) pjmedia_ice_trickle_send_local_cand(
pj_bool_t *p_end_of_cand);


/**
* Enumerate the candidates for the specified component.
*
* @param tp The ICE media transport.
* @param comp_id Component ID.
* @param lcount On input, it specifies the maximum number of
* local candidate elements. On output, it will be filled
* with the number of candidates copied to the array.
* @param lcand Array of local candidates.
* @param rcount On input, it specifies the maximum number of
* remote candidate elements. On output, it will be filled
* with the number of candidates copied to the array.
* @param rcand Array of remote candidates.
*
* @return PJ_SUCCESS, or the appropriate error code.
*/
PJ_DECL(pj_status_t) pj_ice_enum_cands(pjmedia_transport *tp,
unsigned comp_id,
unsigned *lcount,
pj_ice_sess_cand lcand[],
unsigned* rcount,
pj_ice_sess_cand rcand[]);


PJ_END_DECL


Expand Down
28 changes: 28 additions & 0 deletions pjmedia/src/pjmedia/transport_ice.c
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,34 @@ PJ_DEF(pj_status_t) pjmedia_ice_trickle_send_local_cand(
return PJ_SUCCESS;
}

PJ_DEF(pj_status_t) pj_ice_enum_cands(pjmedia_transport *tp,
unsigned comp_id,
unsigned *lcount,
pj_ice_sess_cand lcand[],
unsigned *rcount,
pj_ice_sess_cand rcand[])
{
struct transport_ice *tp_ice = (struct transport_ice*)tp;
pj_status_t status = PJ_SUCCESS;

PJ_ASSERT_RETURN(tp && comp_id && lcount && rcount, PJ_EINVAL);

if (*lcount > 0) {
status = pj_ice_strans_enum_cands(tp_ice->ice_st, comp_id,
lcount, lcand);

if (status != PJ_SUCCESS)
return status;
}

if (*rcount > 0) {
status = pj_ice_strans_enum_remote_cands(tp_ice->ice_st, comp_id,
rcount, rcand);
}

return status;
}


/* Disable ICE when SDP from remote doesn't contain a=candidate line */
static void set_no_ice(struct transport_ice *tp_ice, const char *reason,
Expand Down
91 changes: 91 additions & 0 deletions pjmedia/src/pjmedia/transport_srtp_dtls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

typedef struct dtls_srtp
{
pjmedia_transport base;
Expand Down Expand Up @@ -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];

SSL_CTX *ossl_ctx[NUM_CHANNEL];
SSL *ossl_ssl[NUM_CHANNEL];
BIO *ossl_rbio[NUM_CHANNEL];
Expand Down Expand Up @@ -1178,6 +1186,45 @@ static pj_status_t get_rem_addrs(dtls_srtp *ds,
return PJ_SUCCESS;
}

static pj_bool_t is_valid_src_addr(dtls_srtp *ds, unsigned idx,
pj_sockaddr *src_addr)
{
if (ds->use_ice) {
pj_status_t status;
pj_ice_sess_cand rcand[PJ_ICE_MAX_CAND];
unsigned i = 0;
unsigned lcount = 0;
unsigned rcount = PJ_ARRAY_SIZE(rcand);

if (ds->srtp->use_rtcp_mux)
idx = RTP_CHANNEL;

status = pj_ice_enum_cands(ds->srtp->member_tp, idx + 1,
&lcount, NULL, &rcount, rcand);

if (status != PJ_SUCCESS) {
pj_perror(4, ds->base.name, status,
"Failed getting ICE candidates");
return PJ_FALSE;
}

for (; i < rcount && i < PJ_ARRAY_SIZE(rcand); ++i) {
if (pj_sockaddr_cmp(&rcand[i].addr, 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);
}
}

/* Check if an incoming packet is a DTLS packet (rfc5764 section 5.1.2) */
#define IS_DTLS_PKT(pkt, pkt_len) (*(char*)pkt > 19 && *(char*)pkt < 64)

Expand Down Expand Up @@ -1361,6 +1408,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;

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);
}
}
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) {
Expand Down
18 changes: 18 additions & 0 deletions pjnath/include/pjnath/ice_strans.h
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,24 @@ PJ_DECL(pj_status_t) pj_ice_strans_enum_cands(pj_ice_strans *ice_st,
unsigned *count,
pj_ice_sess_cand cand[]);

/**
* Enumerate the remote candidates for the specified component.
*
* @param ice_st The ICE stream transport.
* @param comp_id Component ID.
* @param count On input, it specifies the maximum number of
* elements. On output, it will be filled with
* the number of candidates copied to the
* array.
* @param cand Array of candidates.
*
* @return PJ_SUCCESS, or the appropriate error code.
*/
PJ_DECL(pj_status_t) pj_ice_strans_enum_remote_cands(pj_ice_strans* ice_st,
unsigned comp_id,
unsigned *count,
pj_ice_sess_cand cand[]);

/**
* Get the default candidate for the specified component. When this
* function is called before ICE negotiation completes, the default
Expand Down
56 changes: 42 additions & 14 deletions pjnath/src/pjnath/ice_strans.c
Original file line number Diff line number Diff line change
Expand Up @@ -1478,32 +1478,60 @@ PJ_DEF(unsigned) pj_ice_strans_get_cands_count(pj_ice_strans *ice_st,
return cnt;
}

/*
* Enum candidates
*/
PJ_DEF(pj_status_t) pj_ice_strans_enum_cands(pj_ice_strans *ice_st,
unsigned comp_id,
unsigned *count,
pj_ice_sess_cand cand[])
static pj_status_t ice_strans_enum_cands(pj_ice_strans *ice_st,
pj_bool_t is_local,
unsigned comp_id,
unsigned *count,
pj_ice_sess_cand cand[])
{
unsigned i, cnt;
unsigned i, cnt, cand_cnt;
pj_ice_sess_cand *ice_cand;

PJ_ASSERT_RETURN(ice_st && ice_st->ice && comp_id &&
comp_id <= ice_st->comp_cnt && count && cand, PJ_EINVAL);
PJ_ASSERT_RETURN(ice_st&& ice_st->ice&& comp_id&&
comp_id <= ice_st->comp_cnt && count && cand, PJ_EINVAL);

if (is_local) {
cand_cnt = ice_st->ice->lcand_cnt;
ice_cand = ice_st->ice->lcand;
} else {
cand_cnt = ice_st->ice->rcand_cnt;
ice_cand = ice_st->ice->rcand;
}

cnt = 0;
for (i=0; i<ice_st->ice->lcand_cnt && cnt<*count; ++i) {
if (ice_st->ice->lcand[i].comp_id != comp_id)
for (i = 0; i < cand_cnt && cnt < *count; ++i) {
if (ice_cand[i].comp_id != comp_id)
continue;
pj_memcpy(&cand[cnt], &ice_st->ice->lcand[i],
sizeof(pj_ice_sess_cand));
pj_memcpy(&cand[cnt], &ice_cand[i], sizeof(pj_ice_sess_cand));
++cnt;
}

*count = cnt;
return PJ_SUCCESS;
}

/*
* Enum local candidates
*/
PJ_DEF(pj_status_t) pj_ice_strans_enum_cands(pj_ice_strans *ice_st,
unsigned comp_id,
unsigned *count,
pj_ice_sess_cand cand[])
{
return ice_strans_enum_cands(ice_st, PJ_TRUE, comp_id, count, cand);
}

/*
* Enum remote candidates
*/
PJ_DEF(pj_status_t) pj_ice_strans_enum_remote_cands(pj_ice_strans* ice_st,
unsigned comp_id,
unsigned* count,
pj_ice_sess_cand cand[])
{
return ice_strans_enum_cands(ice_st, PJ_FALSE, comp_id, count, cand);
}

/*
* Get default candidate.
*/
Expand Down
Loading